diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index f65655d47e..5710665c54 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -365,9 +365,9 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { } function installExtension(bytes32 _extensionId, uint256 _version) - public stoppable auth returns (address) + public stoppable auth { - return IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version); + IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version); } function upgradeExtension(address _extension, uint256 _newVersion) @@ -388,6 +388,12 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extension); } + function migrateToMultiExtension(bytes32 _extensionId) + public stoppable auth + { + IColonyNetwork(colonyNetworkAddress).migrateToMultiExtension(_extensionId); + } + function addDomain(uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _parentDomainId) public stoppable authDomain(_permissionDomainId, _childSkillIndex, _parentDomainId) diff --git a/contracts/colony/ColonyAuthority.sol b/contracts/colony/ColonyAuthority.sol index 2abecefe0a..0b040f60fb 100644 --- a/contracts/colony/ColonyAuthority.sol +++ b/contracts/colony/ColonyAuthority.sol @@ -120,6 +120,7 @@ contract ColonyAuthority is CommonAuthority { addRoleCapability(ROOT_ROLE, "upgradeExtension(address,uint256)"); addRoleCapability(ROOT_ROLE, "deprecateExtension(address,bool)"); addRoleCapability(ROOT_ROLE, "uninstallExtension(address)"); + addRoleCapability(ROOT_ROLE, "migrateToMultiExtension(bytes32)"); addRoleCapability(ARBITRATION_ROLE, "setExpenditureMetadata(uint256,uint256,uint256,string)"); } diff --git a/contracts/colony/ColonyStorage.sol b/contracts/colony/ColonyStorage.sol index 6b874cb71d..53fbd3c5df 100755 --- a/contracts/colony/ColonyStorage.sol +++ b/contracts/colony/ColonyStorage.sol @@ -296,7 +296,7 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes // Ensure addr is an extension installed in the colony, must check old & new formats try ColonyExtension(addr).identifier() returns (bytes32 extensionId) { return ( - IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(addr) == address(this) || + IColonyNetwork(colonyNetworkAddress).getExtensionColony(addr) == address(this) || IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == addr ); } catch { diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol index 85c1eb6a21..d16c742b0f 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -263,8 +263,7 @@ interface IColony is ColonyDataTypes, IRecovery { /// @notice Install an extension to the colony. Secured function to authorised members. /// @param extensionId keccak256 hash of the extension name, used as an indentifier /// @param version The new extension version to install - /// @return extension The address of the extension installation - function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); + function installExtension(bytes32 extensionId, uint256 version) external; /// @notice Upgrade an extension in a colony. Secured function to authorised members. /// @param extension The address of the extension installation @@ -282,6 +281,10 @@ interface IColony is ColonyDataTypes, IRecovery { /// @param extension The address of the extension installation function uninstallExtension(address extension) external; + /// @notice Migrate extension bookkeeping to multiExtension. Secured function to authorised members. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + function migrateToMultiExtension(bytes32 extensionId) external; + /// @notice Add a colony domain, and its respective local skill under skill with id `_parentSkillId`. /// New funding pot is created and associated with the domain here. /// @param _permissionDomainId The domainId in which I have the permission to take this action diff --git a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol index 2094f8feff..7a4d1294e2 100755 --- a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol +++ b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol @@ -114,7 +114,13 @@ interface ColonyNetworkDataTypes { /// @param version The version of the extension event ExtensionAddedToNetwork(bytes32 indexed extensionId, uint256 version); - /// @notice Event logged when an extension is installed in a colony + /// @notice Event logged when an extension is installed in a colony (for v7 and below) + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + /// @param version The version of the extension + event ExtensionInstalled(bytes32 indexed extensionId, address indexed colony, uint256 version); + + /// @notice Event logged when an extension is installed in a colony (for v8 and above) /// @param extensionId The identifier for the extension /// @param extension Address of the extension installation /// @param colony The address of the colony diff --git a/contracts/colonyNetwork/ColonyNetworkExtensions.sol b/contracts/colonyNetwork/ColonyNetworkExtensions.sol index 9e4f0c3202..40b9dad30f 100644 --- a/contracts/colonyNetwork/ColonyNetworkExtensions.sol +++ b/contracts/colonyNetwork/ColonyNetworkExtensions.sol @@ -60,14 +60,17 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { if (IColony(msg.sender).version() <= 7) { require(installations[_extensionId][msg.sender] == address(0x0), "colony-network-extension-already-installed"); installations[_extensionId][msg.sender] = address(extension); + + emit ExtensionInstalled(_extensionId, address(extension), _version); } else { multiInstallations[address(extension)] = msg.sender; + + emit ExtensionInstalled(_extensionId, address(extension), msg.sender, _version); } extension.setResolver(resolvers[_extensionId][_version]); ColonyExtension(address(extension)).install(msg.sender); - emit ExtensionInstalled(_extensionId, address(extension), msg.sender, _version); return address(extension); } @@ -160,15 +163,16 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { emit ExtensionUninstalled(_extension, msg.sender); } - function migrateToMultiExtension(bytes32 _extensionId, address _colony) + function migrateToMultiExtension(bytes32 _extensionId) public stoppable + calledByColony { - require(installations[_extensionId][_colony] != address(0x0), "colony-network-extension-not-installed"); + require(installations[_extensionId][msg.sender] != address(0x0), "colony-network-extension-not-installed"); - multiInstallations[installations[_extensionId][_colony]] = payable(_colony); + multiInstallations[installations[_extensionId][msg.sender]] = payable(msg.sender); - delete installations[_extensionId][_colony]; + delete installations[_extensionId][msg.sender]; } // Public view functions @@ -181,7 +185,7 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { return resolvers[_extensionId][_version]; } - function getExtensionMultiInstallation(address _extension) + function getExtensionColony(address _extension) public view returns (address) diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol index e8f94728b7..4fb12210a7 100644 --- a/contracts/colonyNetwork/IColonyNetwork.sol +++ b/contracts/colonyNetwork/IColonyNetwork.sol @@ -316,8 +316,7 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery { /// @notice Install an extension in a colony. Can only be called by a Colony. /// @param extensionId keccak256 hash of the extension name, used as an indentifier /// @param version Version of the extension to install - /// @return extension The address of the extension installation - function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); + function installExtension(bytes32 extensionId, uint256 version) external; /// @dev DEPRECATED /// @notice Upgrade an extension in a colony. Can only be called by a Colony. @@ -350,10 +349,9 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery { /// @param extension Address of the extension installation function uninstallExtension(address extension) external; - /// @notice Migrate extension bookkeeping to multiExtension + /// @notice Migrate extension bookkeeping to multiExtension. Can only be called by a Colony. /// @param extensionId keccak256 hash of the extension name, used as an indentifier - /// @param colony Address of the colony the extension is installed in - function migrateToMultiExtension(bytes32 extensionId, address colony) external; + function migrateToMultiExtension(bytes32 extensionId) external; /// @notice Get an extension's resolver. /// @param extensionId keccak256 hash of the extension name, used as an indentifier @@ -370,7 +368,7 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery { /// @notice Get an extension's installed colony. /// @param extension Address of the extension installation /// @return colony Address of the colony the extension is installed in - function getExtensionMultiInstallation(address extension) external view returns (address colony); + function getExtensionColony(address extension) external view returns (address colony); /// @notice Return 1 / the fee to pay to the network. e.g. if the fee is 1% (or 0.01), return 100. /// @return _feeInverse The inverse of the network fee diff --git a/docs/_Interface_IColony.md b/docs/_Interface_IColony.md index 09d7834339..ab8ccee0ca 100644 --- a/docs/_Interface_IColony.md +++ b/docs/_Interface_IColony.md @@ -1038,11 +1038,6 @@ Install an extension to the colony. Secured function to authorised members. |extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier |version|uint256|The new extension version to install -**Return Parameters** - -|Name|Type|Description| -|---|---|---| -|extension|address|The address of the extension installation ### `lockExpenditure` @@ -1160,6 +1155,18 @@ Make a new task in the colony. Secured function to authorised members. |_dueDate|uint256|The due date of the task, can set to `0` for no-op +### `migrateToMultiExtension` + +Migrate extension bookkeeping to multiExtension. Secured function to authorised members. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier + + ### `mintTokens` Mint `_wad` amount of colony tokens. Secured function to authorised members. diff --git a/docs/_Interface_IColonyNetwork.md b/docs/_Interface_IColonyNetwork.md index 1f359eb5a6..a15923d2c4 100644 --- a/docs/_Interface_IColonyNetwork.md +++ b/docs/_Interface_IColonyNetwork.md @@ -347,40 +347,40 @@ Returns the address of the ENSRegistrar for the Network. |---|---|---| |address|address|The address the ENSRegistrar resolves to -### `getExtensionInstallation` +### `getExtensionColony` -Get an extension's installation. +Get an extension's installed colony. **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 +|extension|address|Address of the extension installation **Return Parameters** |Name|Type|Description| |---|---|---| -|installation|address|The address of the installed extension +|colony|address|Address of the colony the extension is installed in -### `getExtensionMultiInstallation` +### `getExtensionInstallation` -Get an extension's installed colony. +Get an extension's installation. **Parameters** |Name|Type|Description| |---|---|---| -|extension|address|Address of the extension installation +|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| |---|---|---| -|colony|address|Address of the colony the extension is installed in +|installation|address|The address of the installed extension ### `getExtensionResolver` @@ -693,11 +693,6 @@ Install an extension in a colony. Can only be called by a Colony. |extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier |version|uint256|Version of the extension to install -**Return Parameters** - -|Name|Type|Description| -|---|---|---| -|extension|address|The address of the extension installation ### `isColony` @@ -735,7 +730,7 @@ Reverse lookup a username from an address. ### `migrateToMultiExtension` -Migrate extension bookkeeping to multiExtension +Migrate extension bookkeeping to multiExtension. Can only be called by a Colony. **Parameters** @@ -743,7 +738,6 @@ Migrate extension bookkeeping to multiExtension |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 ### `punishStakers` diff --git a/test/contracts-network/colony-network-extensions.js b/test/contracts-network/colony-network-extensions.js index e3312eba5b..5df768df22 100644 --- a/test/contracts-network/colony-network-extensions.js +++ b/test/contracts-network/colony-network-extensions.js @@ -196,21 +196,21 @@ contract("Colony Network Extensions", (accounts) => { let colonyAddress; - colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extension.address); + colonyAddress = await colonyNetwork.getExtensionColony(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)}`; + const slot = soliditySha3(ethers.utils.hexZeroPad(colony.address, 32), soliditySha3(TEST_EXTENSION, 39)); + const value = ethers.utils.hexZeroPad(extension.address, 32); await editableColonyNetwork.setStorageSlot(slot, value); let extensionAddress; extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); expect(extensionAddress).to.not.equal(ethers.constants.AddressZero); - await colonyNetwork.migrateToMultiExtension(TEST_EXTENSION, colony.address); + await colony.migrateToMultiExtension(TEST_EXTENSION); - colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extension.address); + colonyAddress = await colonyNetwork.getExtensionColony(extension.address); expect(colonyAddress).to.equal(colony.address); extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); @@ -221,8 +221,8 @@ contract("Colony Network Extensions", (accounts) => { const version7Colony = await Version7.new(colonyNetwork.address); // Add version7Colony to _isColony mapping - const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); - const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19); + const value = ethers.utils.zeroPad(1, 32); await editableColonyNetwork.setStorageSlot(slot, value); await version7Colony.installExtension(TEST_EXTENSION, 1); @@ -297,8 +297,8 @@ contract("Colony Network Extensions", (accounts) => { const version7Colony = await Version7.new(colonyNetwork.address); // Add version7Colony to _isColony mapping - const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); - const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19); + const value = ethers.utils.zeroPad(1, 32); await editableColonyNetwork.setStorageSlot(slot, value); await version7Colony.installExtension(TEST_EXTENSION, 1); @@ -346,8 +346,8 @@ contract("Colony Network Extensions", (accounts) => { const version7Colony = await Version7.new(colonyNetwork.address); // Add version7Colony to _isColony mapping - const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); - const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19); + const value = ethers.utils.zeroPad(1, 32); await editableColonyNetwork.setStorageSlot(slot, value); await version7Colony.installExtension(TEST_EXTENSION, 1); @@ -397,8 +397,8 @@ contract("Colony Network Extensions", (accounts) => { const version7Colony = await Version7.new(colonyNetwork.address); // Add version7Colony to _isColony mapping - const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); - const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19); + const value = ethers.utils.zeroPad(1, 32); await editableColonyNetwork.setStorageSlot(slot, value); await version7Colony.installExtension(TEST_EXTENSION, 1); diff --git a/test/extensions/coin-machine.js b/test/extensions/coin-machine.js index 3ecf15272d..7ecebe00b6 100644 --- a/test/extensions/coin-machine.js +++ b/test/extensions/coin-machine.js @@ -55,6 +55,14 @@ contract("Coin Machine", (accounts) => { const ADDRESS_ZERO = ethers.constants.AddressZero; + async function installCoinMachine() { + const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion); + const coinMachineAddress = getExtensionAddressFromTx(tx); + + coinMachine = await CoinMachine.at(coinMachineAddress); + return coinMachine; + } + before(async () => { colonyNetwork = await setupColonyNetwork(); const { metaColony } = await setupMetaColonyWithLockedCLNYToken(colonyNetwork); @@ -73,10 +81,7 @@ contract("Coin Machine", (accounts) => { beforeEach(async () => { ({ colony, token } = await setupRandomColony(colonyNetwork)); purchaseToken = await setupRandomToken(); - - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); // Forward time to start of a Coin Machine period - so long a test doesn't take an hour to run, should be reproducible! // (I still don't like this functionality of CoinMachine though!) @@ -268,9 +273,7 @@ contract("Coin Machine", (accounts) => { await otherToken.unlock(); await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); await otherToken.mint(coinMachine.address, UINT128_MAX); @@ -287,9 +290,7 @@ contract("Coin Machine", (accounts) => { it("cannot buy more than the balance of tokens", async () => { await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); await token.mint(coinMachine.address, WAD); @@ -351,9 +352,7 @@ contract("Coin Machine", (accounts) => { it("can buy tokens with eth", async () => { await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); await token.mint(coinMachine.address, UINT128_MAX); @@ -703,9 +702,7 @@ contract("Coin Machine", (accounts) => { colony = await setupColony(colonyNetwork, token.address); await token.unlock(); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); await token.mint(coinMachine.address, UINT128_MAX); @@ -725,9 +722,7 @@ contract("Coin Machine", (accounts) => { await purchaseToken.unlock(); await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); await token.mint(coinMachine.address, UINT128_MAX); @@ -795,9 +790,7 @@ contract("Coin Machine", (accounts) => { it("cannot buy more than their user limit allows", async () => { await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); await token.mint(coinMachine.address, WAD.muln(1000)); @@ -869,9 +862,7 @@ contract("Coin Machine", (accounts) => { it("cannot set a user limit without a whitelist", async () => { await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); await coinMachine.initialise(token.address, purchaseToken.address, 60 * 60, 10, WAD.muln(100), WAD.muln(200), WAD.divn(2), WAD, ADDRESS_ZERO); @@ -881,9 +872,7 @@ contract("Coin Machine", (accounts) => { it("can calculate the max purchase at any given time", async () => { await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); - const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = getExtensionAddressFromTx(tx); - coinMachine = await CoinMachine.at(coinMachineAddress); + coinMachine = await installCoinMachine(); // Initial supply of 250 tokens await token.mint(coinMachine.address, WAD.muln(250)); diff --git a/test/extensions/token-supplier.js b/test/extensions/token-supplier.js index 6d76e75123..13f26fda8c 100644 --- a/test/extensions/token-supplier.js +++ b/test/extensions/token-supplier.js @@ -131,7 +131,7 @@ contract("Token Supplier", (accounts) => { token.mint(SUPPLY_CEILING.muln(3)); - await checkErrorRevert(tokenSupplier.setTokenSupplyCeiling(SUPPLY_CEILING.muln(2).subn(1)), "token-supplier-ceiling-too-low"); + await checkErrorRevert(tokenSupplier.setTokenSupplyCeiling(SUPPLY_CEILING.muln(2)), "token-supplier-ceiling-too-low"); }); it("can update the tokenIssuanceRate if root", async () => { diff --git a/test/extensions/voting-rep.js b/test/extensions/voting-rep.js index 6cd1814b32..0c8be8770e 100644 --- a/test/extensions/voting-rep.js +++ b/test/extensions/voting-rep.js @@ -1251,7 +1251,7 @@ contract("Voting Reputation", (accounts) => { expect(tx.logs[1].args.executed).to.be.true; const extensionAddress = getExtensionAddressFromTx(tx); - const colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extensionAddress); + const colonyAddress = await colonyNetwork.getExtensionColony(extensionAddress); expect(colonyAddress).to.equal(colony.address); });