Update in response to review comments

kronosapiens · kronosapiens · commit 34c3b52ad3d6 · 2021-07-12T13:11:28.000-07:00
diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol
@@ -340,39 +340,18 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain {
     return IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version);
   }
 
-  // Deprecated
-  function upgradeExtension(bytes32 _extensionId, uint256 _newVersion)
-  public stoppable auth
-  {
-    IColonyNetwork(colonyNetworkAddress).upgradeExtension(_extensionId, _newVersion);
-  }
-
   function upgradeExtension(address _extension, uint256 _newVersion)
   public stoppable auth
   {
     IColonyNetwork(colonyNetworkAddress).upgradeExtension(_extension, _newVersion);
   }
 
-  // Deprecated
-  function deprecateExtension(bytes32 _extensionId, bool _deprecated)
-  public stoppable auth
-  {
-    IColonyNetwork(colonyNetworkAddress).deprecateExtension(_extensionId, _deprecated);
-  }
-
   function deprecateExtension(address _extension, bool _deprecated)
   public stoppable auth
   {
     IColonyNetwork(colonyNetworkAddress).deprecateExtension(_extension, _deprecated);
   }
 
-  // Deprecated
-  function uninstallExtension(bytes32 _extensionId)
-  public stoppable auth
-  {
-    IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extensionId);
-  }
-
   function uninstallExtension(address _extension)
   public stoppable auth
   {
@@ -477,6 +456,15 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain {
   function finishUpgrade() public always {
     ColonyAuthority colonyAuthority = ColonyAuthority(address(authority));
     bytes4 sig;
+
+    sig = bytes4(keccak256("upgradeExtension(address,uint256)"));
+    colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true);
+
+    sig = bytes4(keccak256("deprecateExtension(address,bool)"));
+    colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true);
+
+    sig = bytes4(keccak256("uninstallExtension(address)"));
+    colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true);
   }
 
   function checkNotAdditionalProtectedVariable(uint256 _slot) public view recovery {
diff --git a/contracts/colony/ColonyAuthority.sol b/contracts/colony/ColonyAuthority.sol
@@ -111,7 +111,7 @@ contract ColonyAuthority is CommonAuthority {
     addRoleCapability(ROOT_ROLE, "burnTokens(address,uint256)");
     addRoleCapability(ROOT_ROLE, "unlockToken()");
 
-    // Added in colony v6 (dandelion-lwss)
+    // Added in colony v7 (dandelion-lwss)
     addRoleCapability(FUNDING_ROLE, "moveFundsBetweenPots(uint256,uint256,uint256,uint256,uint256,uint256,uint256,uint256,address)");
 
     // Added in colony v8 (e-lwss)
diff --git a/contracts/colonyNetwork/ColonyNetworkExtensions.sol b/contracts/colonyNetwork/ColonyNetworkExtensions.sol
@@ -70,7 +70,7 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     public
     stoppable
   {
-    address extension = migrateToMultiExtension(_extensionId);
+    address extension = migrateToMultiExtension(_extensionId, msg.sender);
     upgradeExtension(extension, _newVersion);
 
     emit ExtensionUpgraded(_extensionId, msg.sender, _newVersion);
@@ -101,7 +101,7 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     public
     stoppable
   {
-    address extension = migrateToMultiExtension(_extensionId);
+    address extension = migrateToMultiExtension(_extensionId, msg.sender);
     deprecateExtension(extension, _deprecated);
 
     emit ExtensionDeprecated(_extensionId, msg.sender, _deprecated);
@@ -122,7 +122,7 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     public
     stoppable
   {
-    address extension = migrateToMultiExtension(_extensionId);
+    address extension = migrateToMultiExtension(_extensionId, msg.sender);
     uninstallExtension(extension);
 
     emit ExtensionUninstalled(_extensionId, msg.sender);
@@ -142,6 +142,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     emit ExtensionUninstalled(_extension, msg.sender);
   }
 
+  function migrateToMultiExtension(bytes32 _extensionId, address _colony)
+    public
+    stoppable
+    returns (address)
+  {
+    address extension = installations[_extensionId][_colony];
+    require(extension != address(0x0), "colony-network-extension-not-installed");
+
+    multiInstallations[extension] = payable(_colony);
+    return extension;
+  }
+
   // Public view functions
 
   function getExtensionResolver(bytes32 _extensionId, uint256 _version)
@@ -183,12 +195,4 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     address extension = Resolver(_resolver).lookup(VERSION_SIG);
     return ColonyExtension(extension).version();
   }
-
-  function migrateToMultiExtension(bytes32 _extensionId) internal returns (address) {
-    address extension = installations[_extensionId][msg.sender];
-    require(extension != address(0x0), "colony-network-extension-not-installed");
-
-    multiInstallations[extension] = msg.sender;
-    return extension;
-  }
 }
diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol
@@ -350,6 +350,12 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery {
   /// @param extension Address of the extension installation
   function uninstallExtension(address extension) external;
 
+  /// @notice Migrate extension bookkeeping to multiExtension
+  /// @param extensionId keccak256 hash of the extension name, used as an indentifier
+  /// @param colony Address of the colony the extension is installed in
+  /// @return extension The address of the extension
+  function migrateToMultiExtension(bytes32 extensionId, address colony) external returns (address extension);
+
   /// @notice Get an extension's resolver.
   /// @param extensionId keccak256 hash of the extension name, used as an indentifier
   /// @param version Version of the extension
diff --git a/docs/_Interface_IColonyNetwork.md b/docs/_Interface_IColonyNetwork.md
@@ -733,6 +733,24 @@ Reverse lookup a username from an address.
 |---|---|---|
 |domain|string|A string containing the colony-based ENS name corresponding to addr
 
+### `migrateToMultiExtension`
+
+Migrate extension bookkeeping to multiExtension
+
+
+**Parameters**
+
+|Name|Type|Description|
+|---|---|---|
+|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
+|colony|address|Address of the colony the extension is installed in
+
+**Return Parameters**
+
+|Name|Type|Description|
+|---|---|---|
+|extension|address|The address of the extension
+
 ### `punishStakers`
 
 Function called to punish people who staked against a new reputation root hash that turned out to be incorrect.
diff --git a/test/contracts-network/colony-network-extensions.js b/test/contracts-network/colony-network-extensions.js
@@ -154,28 +154,60 @@ contract("Colony Network Extensions", (accounts) => {
       await metaColony.addExtensionToNetwork(TEST_EXTENSION, testExtension2Resolver.address);
     });
 
-    it("allows a root user to install an extension with any version", async () => {
-      const tx = await colony.installExtension(TEST_EXTENSION, 2, { from: ROOT });
+    it("allows a root user to install an extension", async () => {
+      const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT });
 
       const extensionAddress = getExtensionAddressFromTx(tx);
-      const extension = await TestExtension2.at(extensionAddress);
+      const extension = await TestExtension1.at(extensionAddress);
       const owner = await extension.owner();
       expect(owner).to.equal(colonyNetwork.address);
 
       const identifier = await extension.identifier();
       const version = await extension.version();
       const colonyAddress = await extension.getColony();
       expect(identifier).to.equal(TEST_EXTENSION);
-      expect(version).to.eq.BN(2);
+      expect(version).to.eq.BN(1);
       expect(colonyAddress).to.equal(colony.address);
 
       // Only colonyNetwork can install the extension
       await checkErrorRevert(extension.install(colony.address), "ds-auth-unauthorized");
     });
 
+    it("allows a root user to install an extension with any version", async () => {
+      const tx = await colony.installExtension(TEST_EXTENSION, 2, { from: ROOT });
+
+      const extensionAddress = getExtensionAddressFromTx(tx);
+      const extension = await TestExtension2.at(extensionAddress);
+
+      const identifier = await extension.identifier();
+      const version = await extension.version();
+      expect(identifier).to.equal(TEST_EXTENSION);
+      expect(version).to.eq.BN(2);
+    });
+
     it("does not allow an extension to be installed with a nonexistent resolver", async () => {
       await checkErrorRevert(colony.installExtension(TEST_EXTENSION, 0, { from: ROOT }), "colony-network-extension-bad-version");
     });
+
+    it("allows colonies to migrate to multiExtension bookkeeping", async () => {
+      const extension = await TestExtension1.new();
+      await extension.install(colony.address);
+
+      let colonyAddress;
+
+      colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extension.address);
+      expect(colonyAddress).to.equal(ethers.constants.AddressZero);
+
+      // Set up `installations` mapping in the old style
+      const slot = soliditySha3(`0x000000000000000000000000${colony.address.slice(2)}`, soliditySha3(TEST_EXTENSION, 39));
+      const value = `0x000000000000000000000000${extension.address.slice(2)}`;
+      await editableColonyNetwork.setStorageSlot(slot, value);
+
+      await colonyNetwork.migrateToMultiExtension(TEST_EXTENSION, colony.address);
+
+      colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extension.address);
+      expect(colonyAddress).to.equal(colony.address);
+    });
   });
 
   describe("upgrading extensions", () => {
