From d96e4b891b87489340ae5e3c631f1886aab98bda Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Mon, 28 Jun 2021 13:24:37 -0700 Subject: [PATCH] Add support for deprecated interface --- contracts/colony/Colony.sol | 30 ++++++-- contracts/colony/IColony.sol | 25 +++++- .../colonyNetwork/ColonyNetworkDataTypes.sol | 20 +++++ .../colonyNetwork/ColonyNetworkExtensions.sol | 49 +++++++++++- contracts/colonyNetwork/IColonyNetwork.sol | 23 +++++- docs/_Interface_IColony.md | 39 ++++++++++ docs/_Interface_IColonyNetwork.md | 38 ++++++++++ .../colony-network-extensions.js | 76 ++++++++++++++++++- 8 files changed, 283 insertions(+), 17 deletions(-) diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index 48ef6bf318..16aa615cf2 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -345,23 +345,43 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { function installExtension(bytes32 _extensionId, uint256 _version) public stoppable auth returns (address) { - address extension = IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version); - return extension; + return IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version); } - function upgradeExtension(address payable _extension, uint256 _newVersion) + // 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); } - function deprecateExtension(address payable _extension, bool _deprecated) + // 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); } - function uninstallExtension(address payable _extension) + // Deprecated + function uninstallExtension(bytes32 _extensionId) + public stoppable auth + { + IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extensionId); + } + + function uninstallExtension(address _extension) public stoppable auth { IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extension); diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol index 7827de9178..0b33dbfba0 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -250,21 +250,40 @@ interface IColony is ColonyDataTypes, IRecovery { /// @return extension The address of the extension installation function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); + /// @dev DEPRECATED + /// @notice Upgrade an extension in a colony. Secured function to authorised members. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + /// @param newVersion The version to upgrade to (must be one larger than the current version) + function upgradeExtension(bytes32 extensionId, uint256 newVersion) external; + /// @notice Upgrade an extension in a colony. Secured function to authorised members. /// @param extension The address of the extension installation /// @param newVersion The version to upgrade to (must be one larger than the current version) - function upgradeExtension(address payable extension, uint256 newVersion) external; + function upgradeExtension(address extension, uint256 newVersion) external; + + /// @dev DEPRECATED + /// @notice Set the deprecation of an extension in a colony. Secured function to authorised members. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + /// @param deprecated Whether to deprecate the extension or not + function deprecateExtension(bytes32 extensionId, bool deprecated) external; /// @notice Set the deprecation of an extension in a colony. Secured function to authorised members. /// @param extension The address of the extension installation /// @param deprecated Whether to deprecate the extension or not - function deprecateExtension(address payable extension, bool deprecated) external; + function deprecateExtension(address extension, bool deprecated) external; + + /// @dev DEPRECATED + /// @notice Uninstall an extension from a colony. Secured function to authorised members. + /// @dev This is a permanent action -- re-installing the extension will deploy a new contract + /// @dev It is recommended to deprecate an extension before uninstalling to allow active objects to be resolved + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + function uninstallExtension(bytes32 extensionId) external; /// @notice Uninstall an extension from a colony. Secured function to authorised members. /// @dev This is a permanent action -- re-installing the extension will deploy a new contract /// @dev It is recommended to deprecate an extension before uninstalling to allow active objects to be resolved /// @param extension The address of the extension installation - function uninstallExtension(address payable extension) external; + function uninstallExtension(address extension) 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. diff --git a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol index 6b3b1446d2..2094f8feff 100755 --- a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol +++ b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol @@ -121,18 +121,38 @@ interface ColonyNetworkDataTypes { /// @param version The version of the extension event ExtensionInstalled(bytes32 indexed extensionId, address indexed extension, address indexed colony, uint256 version); + /// @dev DEPRECATED + /// @notice Event logged when an extension is upgraded in a colony + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + /// @param version The new version of the extension + event ExtensionUpgraded(bytes32 indexed extensionId, address indexed colony, uint256 version); + /// @notice Event logged when an extension is upgraded in a colony /// @param extension Address of the extension installation /// @param colony The address of the colony /// @param version The new version of the extension event ExtensionUpgraded(address indexed extension, address indexed colony, uint256 version); + /// @dev DEPRECATED + /// @notice Event logged when an extension is (un)deprecated in a colony + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + /// @param deprecated Whether the extension is deprecated or not + event ExtensionDeprecated(bytes32 indexed extensionId, address indexed colony, bool deprecated); + /// @notice Event logged when an extension is (un)deprecated in a colony /// @param extension Address of the extension installation /// @param colony The address of the colony /// @param deprecated Whether the extension is deprecated or not event ExtensionDeprecated(address indexed extension, address indexed colony, bool deprecated); + /// @dev DEPRECATED + /// @notice Event logged when an extension is uninstalled from a colony + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + event ExtensionUninstalled(bytes32 indexed extensionId, address indexed colony); + /// @notice Event logged when an extension is uninstalled from a colony /// @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 63656a3ca0..59a5a367cf 100644 --- a/contracts/colonyNetwork/ColonyNetworkExtensions.sol +++ b/contracts/colonyNetwork/ColonyNetworkExtensions.sol @@ -65,7 +65,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { return address(extension); } - function upgradeExtension(address payable _extension, uint256 _newVersion) + // Deprecated + function upgradeExtension(bytes32 _extensionId, uint256 _newVersion) + public + stoppable + { + address extension = migrateToMultiExtension(_extensionId); + upgradeExtension(extension, _newVersion); + + emit ExtensionUpgraded(_extensionId, msg.sender, _newVersion); + } + + function upgradeExtension(address _extension, uint256 _newVersion) public stoppable calledByColony @@ -77,7 +88,7 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { require(_newVersion == ColonyExtension(_extension).version() + 1, "colony-network-extension-bad-increment"); require(resolvers[extensionId][_newVersion] != address(0x0), "colony-network-extension-bad-version"); - EtherRouter(_extension).setResolver(resolvers[extensionId][_newVersion]); + EtherRouter(payable(_extension)).setResolver(resolvers[extensionId][_newVersion]); ColonyExtension(_extension).finishUpgrade(); assert(ColonyExtension(_extension).version() == _newVersion); @@ -85,7 +96,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { emit ExtensionUpgraded(_extension, msg.sender, _newVersion); } - function deprecateExtension(address payable _extension, bool _deprecated) + // Deprecated + function deprecateExtension(bytes32 _extensionId, bool _deprecated) + public + stoppable + { + address extension = migrateToMultiExtension(_extensionId); + deprecateExtension(extension, _deprecated); + + emit ExtensionDeprecated(_extensionId, msg.sender, _deprecated); + } + + function deprecateExtension(address _extension, bool _deprecated) public stoppable calledByColony @@ -95,7 +117,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { emit ExtensionDeprecated(_extension, msg.sender, _deprecated); } - function uninstallExtension(address payable _extension) + // Deprecated + function uninstallExtension(bytes32 _extensionId) + public + stoppable + { + address extension = migrateToMultiExtension(_extensionId); + uninstallExtension(extension); + + emit ExtensionUninstalled(_extensionId, msg.sender); + } + + function uninstallExtension(address _extension) public stoppable calledByColony @@ -150,4 +183,12 @@ 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 index 8cfdd62677..71c9b293ed 100644 --- a/contracts/colonyNetwork/IColonyNetwork.sol +++ b/contracts/colonyNetwork/IColonyNetwork.sol @@ -319,19 +319,36 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery { /// @return extension The address of the extension installation function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); + /// @dev DEPRECATED + /// @notice Upgrade 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 newVersion Version of the extension to upgrade to (must be one greater than current) + function upgradeExtension(bytes32 extensionId, uint256 newVersion) external; + /// @notice Upgrade an extension in a colony. Can only be called by a Colony. /// @param extension Address of the extension installation /// @param newVersion Version of the extension to upgrade to (must be one greater than current) - function upgradeExtension(address payable extension, uint256 newVersion) external; + function upgradeExtension(address extension, uint256 newVersion) external; + + /// @dev DEPRECATED + /// @notice Set the deprecation of 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 deprecated Whether to deprecate the extension or not + function deprecateExtension(bytes32 extensionId, bool deprecated) external; /// @notice Set the deprecation of an extension in a colony. Can only be called by a Colony. /// @param extension Address of the extension installation /// @param deprecated Whether to deprecate the extension or not - function deprecateExtension(address payable extension, bool deprecated) external; + function deprecateExtension(address extension, bool deprecated) external; + + /// @dev DEPRECATED + /// @notice Uninstall an extension in a colony. Can only be called by a Colony. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + function uninstallExtension(bytes32 extensionId) external; /// @notice Uninstall an extension in a colony. Can only be called by a Colony. /// @param extension Address of the extension installation - function uninstallExtension(address payable extension) external; + function uninstallExtension(address extension) external; /// @notice Get an extension's resolver. /// @param extensionId keccak256 hash of the extension name, used as an indentifier diff --git a/docs/_Interface_IColony.md b/docs/_Interface_IColony.md index f95637093e..65f998fb66 100644 --- a/docs/_Interface_IColony.md +++ b/docs/_Interface_IColony.md @@ -261,6 +261,19 @@ Set the deprecation of an extension in a colony. Secured function to authorised |deprecated|bool|Whether to deprecate the extension or not +### `deprecateExtension` + +Set the deprecation of an extension in a colony. Secured function to authorised members. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|deprecated|bool|Whether to deprecate the extension or not + + ### `editColony` Called to change the metadata associated with a colony. Expected to be a IPFS hash of a JSON blob, but not enforced to any degree by the contracts @@ -1761,6 +1774,19 @@ Uninstall an extension from a colony. Secured function to authorised members. |extension|address|The address of the extension installation +### `uninstallExtension` + +Uninstall an extension from a colony. Secured function to authorised members. + +*Note: This is a permanent action -- re-installing the extension will deploy a new contract* + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier + + ### `unlockToken` unlock the native colony token, if possible @@ -1832,6 +1858,19 @@ Upgrade an extension in a colony. Secured function to authorised members. |newVersion|uint256|The version to upgrade to (must be one larger than the current version) +### `upgradeExtension` + +Upgrade an extension in a colony. Secured function to authorised members. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|newVersion|uint256|The version to upgrade to (must be one larger than the current version) + + ### `userCanSetRoles` Check whether a given user can modify roles in the target domain `_childDomainId`. Mostly a convenience function to provide a uniform interface for extension contracts validating permissions diff --git a/docs/_Interface_IColonyNetwork.md b/docs/_Interface_IColonyNetwork.md index ec931e7a70..5f28de4103 100644 --- a/docs/_Interface_IColonyNetwork.md +++ b/docs/_Interface_IColonyNetwork.md @@ -234,6 +234,19 @@ Set the deprecation of an extension in a colony. Can only be called by a Colony. |deprecated|bool|Whether to deprecate the extension or not +### `deprecateExtension` + +Set the deprecation of an extension in a colony. Can only be called by a Colony. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|deprecated|bool|Whether to deprecate the extension or not + + ### `deprecateSkill` Mark a global skill as deprecated which stops new tasks and payments from using it. @@ -959,6 +972,18 @@ Uninstall an extension in a colony. Can only be called by a Colony. |extension|address|Address of the extension installation +### `uninstallExtension` + +Uninstall an extension in a colony. Can only be called by a Colony. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier + + ### `unstakeForMining` Unstake CLNY currently staked for reputation mining. @@ -1005,4 +1030,17 @@ Upgrade an extension in a colony. Can only be called by a Colony. |Name|Type|Description| |---|---|---| |extension|address|Address of the extension installation +|newVersion|uint256|Version of the extension to upgrade to (must be one greater than current) + + +### `upgradeExtension` + +Upgrade an extension in a colony. Can only be called by a Colony. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier |newVersion|uint256|Version of the extension to upgrade to (must be one greater than current) \ No newline at end of file diff --git a/test/contracts-network/colony-network-extensions.js b/test/contracts-network/colony-network-extensions.js index e5eff9f973..b8b43ea967 100644 --- a/test/contracts-network/colony-network-extensions.js +++ b/test/contracts-network/colony-network-extensions.js @@ -25,9 +25,11 @@ const TestExtension3 = artifacts.require("TestExtension3"); const TestVotingToken = artifacts.require("TestVotingToken"); const Resolver = artifacts.require("Resolver"); const RequireExecuteCall = artifacts.require("RequireExecuteCall"); +const ContractEditing = artifacts.require("ContractEditing"); contract("Colony Network Extensions", (accounts) => { let colonyNetwork; + let editableColonyNetwork; let metaColony; let colony; let token; @@ -71,6 +73,13 @@ contract("Colony Network Extensions", (accounts) => { colonyNetwork = await setupColonyNetwork(); ({ metaColony } = await setupMetaColonyWithLockedCLNYToken(colonyNetwork)); + const colonyNetworkAsER = await EtherRouter.at(colonyNetwork.address); + const colonyNetworkResolverAddress = await colonyNetworkAsER.resolver(); + const colonyNetworkResolver = await Resolver.at(colonyNetworkResolverAddress); + const contractEditing = await ContractEditing.new(); + await colonyNetworkResolver.register("setStorageSlot(uint256,bytes32)", contractEditing.address); + editableColonyNetwork = await ContractEditing.at(colonyNetwork.address); + ({ colony, token } = await setupRandomColony(colonyNetwork)); await colony.addDomain(1, UINT256_MAX, 1); // Domain 2 @@ -191,6 +200,26 @@ contract("Colony Network Extensions", (accounts) => { expect(version).to.eq.BN(2); }); + it("allows root users to upgrade an extension, with the deprecated interface", async () => { + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + + const extensionAddress = getExtensionAddressFromTx(tx); + let extension = await ColonyExtension.at(extensionAddress); + let version = await extension.version(); + expect(version).to.eq.BN(1); + + // Set up `installations` mapping in the old style + const slot = soliditySha3(`0x000000000000000000000000${colony.address.slice(2)}`, soliditySha3(TEST_EXTENSION, 39)); + const value = `0x000000000000000000000000${extensionAddress.slice(2)}`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await colony.methods["upgradeExtension(bytes32,uint256)"](TEST_EXTENSION, 2, { from: ROOT }); + + extension = await ColonyExtension.at(extensionAddress); + version = await extension.version(); + expect(version).to.eq.BN(2); + }); + it("does not allow non-root users to upgrade an extension", async () => { const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); @@ -232,11 +261,33 @@ contract("Colony Network Extensions", (accounts) => { await extension.foo(); - await colony.deprecateExtension(extensionAddress, true, { from: ROOT }); + await colony.methods["deprecateExtension(address,bool)"](extensionAddress, true, { from: ROOT }); await checkErrorRevert(extension.foo(), "colony-extension-deprecated"); - await colony.deprecateExtension(extensionAddress, false, { from: ROOT }); + await colony.methods["deprecateExtension(address,bool)"](extensionAddress, false, { from: ROOT }); + + await extension.foo(); + }); + + it("allows root users to deprecate and undeprecate an extension, with the deprecated interface", async () => { + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + + const extensionAddress = getExtensionAddressFromTx(tx); + const extension = await TestExtension1.at(extensionAddress); + + // Set up `installations` mapping in the old style + const slot = soliditySha3(`0x000000000000000000000000${colony.address.slice(2)}`, soliditySha3(TEST_EXTENSION, 39)); + const value = `0x000000000000000000000000${extensionAddress.slice(2)}`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await extension.foo(); + + await colony.methods["deprecateExtension(bytes32,bool)"](TEST_EXTENSION, true, { from: ROOT }); + + await checkErrorRevert(extension.foo(), "colony-extension-deprecated"); + + await colony.methods["deprecateExtension(bytes32,bool)"](TEST_EXTENSION, false, { from: ROOT }); await extension.foo(); }); @@ -270,6 +321,27 @@ contract("Colony Network Extensions", (accounts) => { expect(new BN(colonyBalance)).to.eq.BN(100); }); + it("allows root users to uninstall an extension and send ether to the beneficiary, with the deprecated interface", async () => { + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + + const extensionAddress = getExtensionAddressFromTx(tx); + const extension = await TestExtension1.at(extensionAddress); + await extension.send(100); + + // Set up `installations` mapping in the old style + const slot = soliditySha3(`0x000000000000000000000000${colony.address.slice(2)}`, soliditySha3(TEST_EXTENSION, 39)); + const value = `0x000000000000000000000000${extensionAddress.slice(2)}`; + await editableColonyNetwork.setStorageSlot(slot, value); + + // Only colonyNetwork can uninstall + await checkErrorRevert(extension.uninstall(), "ds-auth-unauthorized"); + + await colony.methods["uninstallExtension(bytes32)"](TEST_EXTENSION, { from: ROOT }); + + const colonyBalance = await web3GetBalance(colony.address); + expect(new BN(colonyBalance)).to.eq.BN(100); + }); + it("does not allow non-root users to uninstall an extension", async () => { await checkErrorRevert(colony.uninstallExtension(ethers.constants.AddressZero, { from: ARCHITECT }), "ds-auth-unauthorized"); await checkErrorRevert(colony.uninstallExtension(ethers.constants.AddressZero, { from: USER }), "ds-auth-unauthorized");