Add scenario changes and a forking simulation; remove _becomeG3

It will be easier for someone to make a proposal if they don't need to worry about the _becomeG3 arguments. The proposal may now be made, as in the forking simulation. We also fix up the test harnesses for the new Comptroller impl generation, and add a storage check for it.
compound-finance · Jun 27, 2020 · 1c770f8 · 1c770f8
1 parent 79efeca
commit 1c770f8
Show file tree

Hide file tree

Showing 17 changed files with 471 additions and 157 deletions.
diff --git a/contracts/Comptroller.sol b/contracts/Comptroller.sol
@@ -168,7 +168,7 @@ contract Comptroller is ComptrollerV3Storage, ComptrollerInterface, ComptrollerE
     /**
      * @notice Removes asset from sender's account liquidity calculation
      * @dev Sender must not have an outstanding borrow balance in the asset,
-     *  or be providing neccessary collateral for an outstanding borrow.
+     *  or be providing necessary collateral for an outstanding borrow.
      * @param cTokenAddress The address of the asset to be removed
      * @return Whether or not the account successfully exited the market
      */
@@ -1081,26 +1081,9 @@ contract Comptroller is ComptrollerV3Storage, ComptrollerInterface, ComptrollerE
         return state;
     }
 
-    function _become(Unitroller unitroller, uint compRate_, address[] memory compMarketsToAdd, address[] memory otherMarketsToAdd) public {
+    function _become(Unitroller unitroller) public {
         require(msg.sender == unitroller.admin(), "only unitroller admin can change brains");
         require(unitroller._acceptImplementation() == 0, "change not authorized");
-
-        Comptroller(address(unitroller))._becomeG3(compRate_, compMarketsToAdd, otherMarketsToAdd);
-    }
-
-    function _becomeG3(uint compRate_, address[] memory compMarketsToAdd, address[] memory otherMarketsToAdd) public {
-        require(msg.sender == comptrollerImplementation, "only brains can become itself");
-
-        for (uint i = 0; i < compMarketsToAdd.length; i++) {
-            _addMarketInternal(address(compMarketsToAdd[i]));
-        }
-
-        for (uint i = 0; i < otherMarketsToAdd.length; i++) {
-            _addMarketInternal(address(otherMarketsToAdd[i]));
-        }
-
-        _setCompRate(compRate_);
-        _addCompMarkets(compMarketsToAdd);
     }
 
     /**

diff --git a/scenario/src/Builder/ComptrollerImplBuilder.ts b/scenario/src/Builder/ComptrollerImplBuilder.ts
@@ -14,6 +14,9 @@ const ComptrollerScenarioG1Contract = getTestContract('ComptrollerScenarioG1');
 const ComptrollerG2Contract = getContract('ComptrollerG2');
 const ComptrollerScenarioG2Contract = getContract('ComptrollerScenarioG2');
 
+const ComptrollerG3Contract = getContract('ComptrollerG3');
+const ComptrollerScenarioG3Contract = getContract('ComptrollerScenarioG3');
+
 const ComptrollerScenarioContract = getTestContract('ComptrollerScenario');
 const ComptrollerContract = getContract('Comptroller');
 
@@ -48,6 +51,7 @@ export async function buildComptrollerImpl(
         description: 'ScenarioG1 Comptroller Impl'
       })
     ),
+
     new Fetcher<{ name: StringV }, ComptrollerImplData>(
       `
         #### ScenarioG2
@@ -65,6 +69,23 @@ export async function buildComptrollerImpl(
       })
     ),
 
+    new Fetcher<{ name: StringV }, ComptrollerImplData>(
+      `
+        #### ScenarioG3
+
+        * "ScenarioG3 name:<String>" - The Comptroller Scenario for local testing (G3)
+          * E.g. "ComptrollerImpl Deploy ScenarioG3 MyScen"
+      `,
+      'ScenarioG3',
+      [new Arg('name', getStringV)],
+      async (world, { name }) => ({
+        invokation: await ComptrollerScenarioG3Contract.deploy<ComptrollerImpl>(world, from, []),
+        name: name.val,
+        contract: 'ComptrollerScenarioG3Contract',
+        description: 'ScenarioG3 Comptroller Impl'
+      })
+    ),
+
     new Fetcher<{ name: StringV }, ComptrollerImplData>(
       `
         #### Scenario
@@ -81,6 +102,26 @@ export async function buildComptrollerImpl(
         description: 'Scenario Comptroller Impl'
       })
     ),
+
+    new Fetcher<{ name: StringV }, ComptrollerImplData>(
+      `
+        #### StandardG1
+
+        * "StandardG1 name:<String>" - The standard generation 1 Comptroller contract
+          * E.g. "Comptroller Deploy StandardG1 MyStandard"
+      `,
+      'StandardG1',
+      [new Arg('name', getStringV)],
+      async (world, { name }) => {
+        return {
+          invokation: await ComptrollerG1Contract.deploy<ComptrollerImpl>(world, from, []),
+          name: name.val,
+          contract: 'ComptrollerG1',
+          description: 'StandardG1 Comptroller Impl'
+        };
+      }
+    ),
+
     new Fetcher<{ name: StringV }, ComptrollerImplData>(
       `
         #### StandardG2
@@ -99,24 +140,26 @@ export async function buildComptrollerImpl(
         };
       }
     ),
-        new Fetcher<{ name: StringV }, ComptrollerImplData>(
+
+    new Fetcher<{ name: StringV }, ComptrollerImplData>(
       `
-        #### StandardG1
+        #### StandardG3
 
-        * "StandardG1 name:<String>" - The standard generation 1 Comptroller contract
-          * E.g. "Comptroller Deploy StandardG1 MyStandard"
+        * "StandardG3 name:<String>" - The standard generation 2 Comptroller contract
+          * E.g. "Comptroller Deploy StandardG3 MyStandard"
       `,
-      'StandardG1',
+      'StandardG3',
       [new Arg('name', getStringV)],
       async (world, { name }) => {
         return {
-          invokation: await ComptrollerG1Contract.deploy<ComptrollerImpl>(world, from, []),
+          invokation: await ComptrollerG3Contract.deploy<ComptrollerImpl>(world, from, []),
           name: name.val,
-          contract: 'ComptrollerG1',
-          description: 'StandardG1 Comptroller Impl'
+          contract: 'ComptrollerG3',
+          description: 'StandardG3 Comptroller Impl'
         };
       }
     ),
+
     new Fetcher<{ name: StringV }, ComptrollerImplData>(
       `
         #### Standard
@@ -135,6 +178,7 @@ export async function buildComptrollerImpl(
         };
       }
     ),
+
     new Fetcher<{ name: StringV }, ComptrollerImplData>(
       `
         #### Borked

diff --git a/scenario/src/ContractLookup.ts b/scenario/src/ContractLookup.ts
@@ -106,7 +106,7 @@ export function getErc20Address(world: World, erc20Arg: string): string {
 }
 
 export function getGovernorAddress(world: World, governorArg: string): string {
-  return getContractDataString(world, [['Governor', governorArg, 'address']]);
+  return getContractDataString(world, [['Contracts', governorArg]]);
 }
 
 export async function getPriceOracleProxy(world: World): Promise<PriceOracle> {

diff --git a/scenario/src/Event/ComptrollerImplEvent.ts b/scenario/src/Event/ComptrollerImplEvent.ts
@@ -31,43 +31,72 @@ async function genComptrollerImpl(world: World, from: string, params: Event): Pr
   return world;
 }
 
-async function become(
+async function mergeABI(
+  world: World,
+  from: string,
+  comptrollerImpl: ComptrollerImpl,
+  unitroller: Unitroller
+): Promise<World> {
+  if (!world.dryRun) {
+    // Skip this specifically on dry runs since it's likely to crash due to a number of reasons
+    world = await mergeContractABI(world, 'Comptroller', unitroller, unitroller.name, comptrollerImpl.name);
+  }
+
+  return world;
+}
+
+async function becomeG1(
   world: World,
   from: string,
   comptrollerImpl: ComptrollerImpl,
   unitroller: Unitroller,
-  compRate: encodedNumber,
-  compMarkets: string[],
-  otherMarkets: string[]
+  priceOracleAddr: string,
+  closeFactor: encodedNumber,
+  maxAssets: encodedNumber
 ): Promise<World> {
   let invokation = await invoke(
     world,
-    comptrollerImpl.methods._become(unitroller._address, compRate, compMarkets, otherMarkets),
+    comptrollerImpl.methods._become(unitroller._address, priceOracleAddr, closeFactor, maxAssets, false),
     from,
     ComptrollerErrorReporter
   );
-
   if (!world.dryRun) {
     // Skip this specifically on dry runs since it's likely to crash due to a number of reasons
     world = await mergeContractABI(world, 'Comptroller', unitroller, unitroller.name, comptrollerImpl.name);
   }
 
-  world = addAction(world, `Become ${unitroller._address}'s Comptroller Impl`, invokation);
+  world = addAction(
+    world,
+    `Become ${unitroller._address}'s Comptroller Impl with priceOracle=${priceOracleAddr},closeFactor=${closeFactor},maxAssets=${maxAssets}`,
+    invokation
+  );
 
   return world;
 }
 
-
-async function mergeABI(
+// Recome calls `become` on the G1 Comptroller, but passes a flag to not modify any of the initialization variables.
+async function recome(
   world: World,
   from: string,
   comptrollerImpl: ComptrollerImpl,
   unitroller: Unitroller
 ): Promise<World> {
-  if (!world.dryRun) {
-    // Skip this specifically on dry runs since it's likely to crash due to a number of reasons
-    world = await mergeContractABI(world, 'Comptroller', unitroller, unitroller.name, comptrollerImpl.name);
-  }
+  let invokation = await invoke(
+    world,
+    comptrollerImpl.methods._become(
+      unitroller._address,
+      '0x0000000000000000000000000000000000000000',
+      0,
+      0,
+      true
+    ),
+    from,
+    ComptrollerErrorReporter
+  );
+
+  world = await mergeContractABI(world, 'Comptroller', unitroller, unitroller.name, comptrollerImpl.name);
+
+  world = addAction(world, `Recome ${unitroller._address}'s Comptroller Impl`, invokation);
 
   return world;
 }
@@ -95,57 +124,51 @@ async function becomeG2(
   return world;
 }
 
-async function becomeG1(
+async function becomeG3(
   world: World,
   from: string,
   comptrollerImpl: ComptrollerImpl,
   unitroller: Unitroller,
-  priceOracleAddr: string,
-  closeFactor: encodedNumber,
-  maxAssets: encodedNumber
+  compRate: encodedNumber,
+  compMarkets: string[],
+  otherMarkets: string[]
 ): Promise<World> {
   let invokation = await invoke(
     world,
-    comptrollerImpl.methods._become(unitroller._address, priceOracleAddr, closeFactor, maxAssets, false),
+    comptrollerImpl.methods._become(unitroller._address, compRate, compMarkets, otherMarkets),
     from,
     ComptrollerErrorReporter
   );
+
   if (!world.dryRun) {
     // Skip this specifically on dry runs since it's likely to crash due to a number of reasons
     world = await mergeContractABI(world, 'Comptroller', unitroller, unitroller.name, comptrollerImpl.name);
   }
 
-  world = addAction(
-    world,
-    `Become ${unitroller._address}'s Comptroller Impl with priceOracle=${priceOracleAddr},closeFactor=${closeFactor},maxAssets=${maxAssets}`,
-    invokation
-  );
+  world = addAction(world, `Become ${unitroller._address}'s Comptroller Impl`, invokation);
 
   return world;
 }
-// Recome calls `become` on the G1 Comptroller, but passes a flag to not modify any of the initialization variables.
-async function recome(
+
+async function become(
   world: World,
   from: string,
   comptrollerImpl: ComptrollerImpl,
   unitroller: Unitroller
 ): Promise<World> {
   let invokation = await invoke(
     world,
-    comptrollerImpl.methods._become(
-      unitroller._address,
-      '0x0000000000000000000000000000000000000000',
-      0,
-      0,
-      true
-    ),
+    comptrollerImpl.methods._become(unitroller._address),
     from,
     ComptrollerErrorReporter
   );
 
-  world = await mergeContractABI(world, 'Comptroller', unitroller, unitroller.name, comptrollerImpl.name);
+  if (!world.dryRun) {
+    // Skip this specifically on dry runs since it's likely to crash due to a number of reasons
+    world = await mergeContractABI(world, 'Comptroller', unitroller, unitroller.name, comptrollerImpl.name);
+  }
 
-  world = addAction(world, `Recome ${unitroller._address}'s Comptroller Impl`, invokation);
+  world = addAction(world, `Become ${unitroller._address}'s Comptroller Impl`, invokation);
 
   return world;
 }
@@ -228,6 +251,7 @@ export function comptrollerImplCommands() {
         ),
       { namePos: 1 }
     ),
+
     new Command<{
       unitroller: Unitroller;
       comptrollerImpl: ComptrollerImpl;
@@ -255,12 +279,12 @@ export function comptrollerImplCommands() {
       otherMarkets: ArrayV<AddressV>;
     }>(
       `
-        #### Become
+        #### BecomeG3
 
-        * "ComptrollerImpl <Impl> Become <Rate> <CompMarkets> <OtherMarkets>" - Become the comptroller, if possible.
-          * E.g. "ComptrollerImpl MyImpl Become 0.1e18 [cDAI, cETH, cUSDC]
+        * "ComptrollerImpl <Impl> BecomeG3 <Rate> <CompMarkets> <OtherMarkets>" - Become the comptroller, if possible.
+          * E.g. "ComptrollerImpl MyImpl BecomeG3 0.1e18 [cDAI, cETH, cUSDC]
       `,
-      'Become',
+      'BecomeG3',
       [
         new Arg('unitroller', getUnitroller, { implicit: true }),
         new Arg('comptrollerImpl', getComptrollerImpl),
@@ -269,7 +293,28 @@ export function comptrollerImplCommands() {
         new Arg('otherMarkets', getArrayV(getAddressV), { default: new ArrayV([]) })
       ],
       (world, from, { unitroller, comptrollerImpl, compRate, compMarkets, otherMarkets }) => {
-        return become(world, from, comptrollerImpl, unitroller, compRate.encode(), compMarkets.val.map(a => a.val), otherMarkets.val.map(a => a.val))
+        return becomeG3(world, from, comptrollerImpl, unitroller, compRate.encode(), compMarkets.val.map(a => a.val), otherMarkets.val.map(a => a.val))
+      },
+      { namePos: 1 }
+    ),
+
+    new Command<{
+      unitroller: Unitroller;
+      comptrollerImpl: ComptrollerImpl;
+    }>(
+      `
+        #### Become
+
+        * "ComptrollerImpl <Impl> Become <Rate> <CompMarkets> <OtherMarkets>" - Become the comptroller, if possible.
+          * E.g. "ComptrollerImpl MyImpl Become 0.1e18 [cDAI, cETH, cUSDC]
+      `,
+      'Become',
+      [
+        new Arg('unitroller', getUnitroller, { implicit: true }),
+        new Arg('comptrollerImpl', getComptrollerImpl)
+      ],
+      (world, from, { unitroller, comptrollerImpl }) => {
+        return become(world, from, comptrollerImpl, unitroller)
       },
       { namePos: 1 }
     ),

diff --git a/spec/scenario/CoreMacros b/spec/scenario/CoreMacros
@@ -15,7 +15,7 @@ Macro PricedComptroller closeFactor=0.1 maxAssets=20
     ComptrollerImpl ScenComptrollerG1 BecomeG1 (PriceOracleProxy Address) closeFactor maxAssets
     ComptrollerImpl Deploy Scenario ScenComptroller
     Unitroller SetPendingImpl ScenComptroller
-    ComptrollerImpl ScenComptroller Become 1e18 []
+    ComptrollerImpl ScenComptroller Become
 
 Macro NewComptroller price=1.0 closeFactor=0.1 maxAssets=20
     --g1
@@ -30,9 +30,13 @@ Macro NewComptroller price=1.0 closeFactor=0.1 maxAssets=20
     Unitroller SetPendingImpl ComptrollerG2
     ComptrollerImpl ComptrollerG2 BecomeG2
     --g3
+    ComptrollerImpl Deploy StandardG3 ComptrollerG3
+    Unitroller SetPendingImpl ComptrollerG3
+    ComptrollerImpl ComptrollerG3 BecomeG3 1e18 []
+    --g4
     ComptrollerImpl Deploy Scenario ScenComptroller
     Unitroller SetPendingImpl ScenComptroller
-    ComptrollerImpl ScenComptroller Become 1e18 []
+    ComptrollerImpl ScenComptroller Become
 
 Macro NewCToken erc20 cToken borrowRate=0.000005 initialExchangeRate=2e9 decimals=8 tokenType=Standard delegatorType=CErc20DelegatorScenario cTokenType=CErc20DelegateScenario admin=Admin becomeImplementationData="0x0"
     Erc20 Deploy tokenType erc20 erc20