From d5744931b5508ca47d2a266050aa937f501bd1fa Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Thu, 24 Jun 2021 11:53:32 -0700 Subject: [PATCH 01/11] Introduce a multiExtension approach --- contracts/colony/Colony.sol | 26 +++++-- contracts/colony/ColonyAuthority.sol | 3 + contracts/colony/ColonyStorage.sol | 5 +- contracts/colony/IColony.sol | 15 ++-- .../colonyNetwork/ColonyNetworkDataTypes.sol | 15 ++-- .../colonyNetwork/ColonyNetworkExtensions.sol | 54 +++++++------ .../colonyNetwork/ColonyNetworkStorage.sol | 7 +- contracts/colonyNetwork/IColonyNetwork.sol | 20 +++-- docs/_Interface_IColony.md | 11 ++- docs/_Interface_IColonyNetwork.md | 28 ++++++- helpers/test-helper.js | 4 + .../colony-network-extensions.js | 75 +++++++++---------- 12 files changed, 164 insertions(+), 99 deletions(-) diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index e426a58d22..6c349fed31 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -373,27 +373,28 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { } function installExtension(bytes32 _extensionId, uint256 _version) - public stoppable auth + public stoppable auth returns (address) { - IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version); + address extension = IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version); + return extension; } - function upgradeExtension(bytes32 _extensionId, uint256 _newVersion) + function upgradeExtension(address payable _extension, uint256 _newVersion) public stoppable auth { - IColonyNetwork(colonyNetworkAddress).upgradeExtension(_extensionId, _newVersion); + IColonyNetwork(colonyNetworkAddress).upgradeExtension(_extension, _newVersion); } - function deprecateExtension(bytes32 _extensionId, bool _deprecated) + function deprecateExtension(address payable _extension, bool _deprecated) public stoppable auth { - IColonyNetwork(colonyNetworkAddress).deprecateExtension(_extensionId, _deprecated); + IColonyNetwork(colonyNetworkAddress).deprecateExtension(_extension, _deprecated); } - function uninstallExtension(bytes32 _extensionId) + function uninstallExtension(address payable _extension) public stoppable auth { - IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extensionId); + IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extension); } function addDomain(uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _parentDomainId) public @@ -501,6 +502,15 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { sig = bytes4(keccak256("setDefaultGlobalClaimDelay(uint256)")); colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true); + 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); + sig = bytes4(keccak256("setExpenditureMetadata(uint256,uint256,uint256,string)")); colonyAuthority.setRoleCapability(uint8(ColonyRole.Arbitration), address(this), sig, true); } diff --git a/contracts/colony/ColonyAuthority.sol b/contracts/colony/ColonyAuthority.sol index b89f53aeaa..2abecefe0a 100644 --- a/contracts/colony/ColonyAuthority.sol +++ b/contracts/colony/ColonyAuthority.sol @@ -117,6 +117,9 @@ contract ColonyAuthority is CommonAuthority { // Added in colony v8 (ebony-lwss) addRoleCapability(ROOT_ROLE, "makeArbitraryTransactions(address[],bytes[],bool)"); addRoleCapability(ROOT_ROLE, "setDefaultGlobalClaimDelay(uint256)"); + addRoleCapability(ROOT_ROLE, "upgradeExtension(address,uint256)"); + addRoleCapability(ROOT_ROLE, "deprecateExtension(address,bool)"); + addRoleCapability(ROOT_ROLE, "uninstallExtension(address)"); addRoleCapability(ARBITRATION_ROLE, "setExpenditureMetadata(uint256,uint256,uint256,string)"); } diff --git a/contracts/colony/ColonyStorage.sol b/contracts/colony/ColonyStorage.sol index 805d9e1e4e..f45e10f1a4 100755 --- a/contracts/colony/ColonyStorage.sol +++ b/contracts/colony/ColonyStorage.sol @@ -242,11 +242,12 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes // Ensure msg.sender is a contract require(isContract(msg.sender), "colony-sender-must-be-contract"); - // Ensure msg.sender is an extension + // Ensure msg.sender is an extension, must check old & new formats // slither-disable-next-line unused-return try ColonyExtension(msg.sender).identifier() returns (bytes32 extensionId) { require( - IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == msg.sender, + IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == msg.sender || + IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(msg.sender) == address(this), "colony-must-be-extension" ); } catch { diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol index c56760608c..9b1ccfdfe9 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -263,23 +263,24 @@ 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 - function installExtension(bytes32 extensionId, uint256 version) external; + /// @return extension The address of the extension installation + function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); /// @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 extension The address of the extension installation /// @param newVersion The version to upgrade to (must be one larger than the current version) - function upgradeExtension(bytes32 extensionId, uint256 newVersion) external; + function upgradeExtension(address payable extension, uint256 newVersion) external; /// @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 extension The address of the extension installation /// @param deprecated Whether to deprecate the extension or not - function deprecateExtension(bytes32 extensionId, bool deprecated) external; + function deprecateExtension(address payable extension, bool deprecated) 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 extensionId keccak256 hash of the extension name, used as an indentifier - function uninstallExtension(bytes32 extensionId) external; + /// @param extension The address of the extension installation + function uninstallExtension(address payable 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 2ef049b925..6b3b1446d2 100755 --- a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol +++ b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol @@ -116,26 +116,27 @@ interface ColonyNetworkDataTypes { /// @notice Event logged when an extension is installed in a colony /// @param extensionId The identifier for the extension + /// @param extension Address of the extension installation /// @param colony The address of the colony /// @param version The version of the extension - event ExtensionInstalled(bytes32 indexed extensionId, address indexed colony, uint256 version); + event ExtensionInstalled(bytes32 indexed extensionId, address indexed extension, address indexed colony, uint256 version); /// @notice Event logged when an extension is upgraded in a colony - /// @param extensionId The identifier for the extension + /// @param extension Address of the extension installation /// @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); + event ExtensionUpgraded(address indexed extension, address indexed colony, uint256 version); /// @notice Event logged when an extension is (un)deprecated in a colony - /// @param extensionId The identifier for the extension + /// @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(bytes32 indexed extensionId, address indexed colony, bool deprecated); + event ExtensionDeprecated(address indexed extension, address indexed colony, bool deprecated); /// @notice Event logged when an extension is uninstalled from a colony - /// @param extensionId The identifier for the extension + /// @param extension Address of the extension installation /// @param colony The address of the colony - event ExtensionUninstalled(bytes32 indexed extensionId, address indexed colony); + event ExtensionUninstalled(address indexed extension, address indexed colony); struct Skill { // total number of parent skills diff --git a/contracts/colonyNetwork/ColonyNetworkExtensions.sol b/contracts/colonyNetwork/ColonyNetworkExtensions.sol index 6c27b8b75b..63656a3ca0 100644 --- a/contracts/colonyNetwork/ColonyNetworkExtensions.sol +++ b/contracts/colonyNetwork/ColonyNetworkExtensions.sol @@ -50,59 +50,63 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { public stoppable calledByColony + returns (address) { require(resolvers[_extensionId][_version] != address(0x0), "colony-network-extension-bad-version"); - require(installations[_extensionId][msg.sender] == address(0x0), "colony-network-extension-already-installed"); EtherRouter extension = new EtherRouter(); - installations[_extensionId][msg.sender] = address(extension); + multiInstallations[address(extension)] = msg.sender; extension.setResolver(resolvers[_extensionId][_version]); ColonyExtension(address(extension)).install(msg.sender); - emit ExtensionInstalled(_extensionId, msg.sender, _version); + emit ExtensionInstalled(_extensionId, address(extension), msg.sender, _version); + + return address(extension); } - function upgradeExtension(bytes32 _extensionId, uint256 _newVersion) + function upgradeExtension(address payable _extension, uint256 _newVersion) public stoppable calledByColony { - require(installations[_extensionId][msg.sender] != address(0x0), "colony-network-extension-not-installed"); + require(multiInstallations[_extension] == msg.sender, "colony-network-extension-not-installed"); + + bytes32 extensionId = ColonyExtension(_extension).identifier(); - address payable extension = installations[_extensionId][msg.sender]; - require(_newVersion == ColonyExtension(extension).version() + 1, "colony-network-extension-bad-increment"); - require(resolvers[_extensionId][_newVersion] != address(0x0), "colony-network-extension-bad-version"); + 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]); - ColonyExtension(extension).finishUpgrade(); - assert(ColonyExtension(extension).version() == _newVersion); + EtherRouter(_extension).setResolver(resolvers[extensionId][_newVersion]); + ColonyExtension(_extension).finishUpgrade(); - emit ExtensionUpgraded(_extensionId, msg.sender, _newVersion); + assert(ColonyExtension(_extension).version() == _newVersion); + + emit ExtensionUpgraded(_extension, msg.sender, _newVersion); } - function deprecateExtension(bytes32 _extensionId, bool _deprecated) + function deprecateExtension(address payable _extension, bool _deprecated) public stoppable calledByColony { - ColonyExtension(installations[_extensionId][msg.sender]).deprecate(_deprecated); + ColonyExtension(_extension).deprecate(_deprecated); - emit ExtensionDeprecated(_extensionId, msg.sender, _deprecated); + emit ExtensionDeprecated(_extension, msg.sender, _deprecated); } - function uninstallExtension(bytes32 _extensionId) + function uninstallExtension(address payable _extension) public stoppable calledByColony { - require(installations[_extensionId][msg.sender] != address(0x0), "colony-network-extension-not-installed"); + require(multiInstallations[_extension] == msg.sender, "colony-network-extension-not-installed"); + + delete multiInstallations[_extension]; - ColonyExtension extension = ColonyExtension(installations[_extensionId][msg.sender]); - installations[_extensionId][msg.sender] = address(0x0); - extension.uninstall(); + ColonyExtension(_extension).uninstall(); - emit ExtensionUninstalled(_extensionId, msg.sender); + emit ExtensionUninstalled(_extension, msg.sender); } // Public view functions @@ -115,6 +119,14 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { return resolvers[_extensionId][_version]; } + function getExtensionMultiInstallation(address _extension) + public + view + returns (address) + { + return multiInstallations[_extension]; + } + function getExtensionInstallation(bytes32 _extensionId, address _colony) public view diff --git a/contracts/colonyNetwork/ColonyNetworkStorage.sol b/contracts/colonyNetwork/ColonyNetworkStorage.sol index dc846174e3..b8f726afdb 100644 --- a/contracts/colonyNetwork/ColonyNetworkStorage.sol +++ b/contracts/colonyNetwork/ColonyNetworkStorage.sol @@ -93,13 +93,16 @@ contract ColonyNetworkStorage is CommonStorage, ColonyNetworkDataTypes, DSMath { uint256 DEPRECATED_lastMetaColonyStipendIssued; // Storage slot 37 // [_extensionId][version] => resolver - mapping(bytes32 => mapping(uint256 => address)) resolvers; // Storage slot 38 + mapping (bytes32 => mapping(uint256 => address)) resolvers; // Storage slot 38 // [_extensionId][colony] => address - mapping(bytes32 => mapping(address => address payable)) installations; // Storage slot 39 + mapping (bytes32 => mapping(address => address payable)) installations; // Storage slot 39 // Used for whitelisting payout tokens mapping (address => bool) payoutWhitelist; // Storage slot 40 + // [_extension] => colony + mapping (address => address payable) multiInstallations; // Storage slot 41 + modifier calledByColony() { require(_isColony[msg.sender], "colony-caller-must-be-colony"); _; diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol index 3a5a696b9f..8cfdd62677 100644 --- a/contracts/colonyNetwork/IColonyNetwork.sol +++ b/contracts/colonyNetwork/IColonyNetwork.sol @@ -316,21 +316,22 @@ 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 - function installExtension(bytes32 extensionId, uint256 version) external; + /// @return extension The address of the extension installation + function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); /// @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 extension Address of the extension installation /// @param newVersion Version of the extension to upgrade to (must be one greater than current) - function upgradeExtension(bytes32 extensionId, uint256 newVersion) external; + function upgradeExtension(address payable extension, uint256 newVersion) external; /// @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 extension Address of the extension installation /// @param deprecated Whether to deprecate the extension or not - function deprecateExtension(bytes32 extensionId, bool deprecated) external; + function deprecateExtension(address payable extension, bool deprecated) external; /// @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; + /// @param extension Address of the extension installation + function uninstallExtension(address payable extension) external; /// @notice Get an extension's resolver. /// @param extensionId keccak256 hash of the extension name, used as an indentifier @@ -344,6 +345,11 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery { /// @return installation The address of the installed extension function getExtensionInstallation(bytes32 extensionId, address colony) external view returns (address installation); + /// @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); + /// @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 function getFeeInverse() external view returns (uint256 _feeInverse); diff --git a/docs/_Interface_IColony.md b/docs/_Interface_IColony.md index 0f6d187444..09d7834339 100644 --- a/docs/_Interface_IColony.md +++ b/docs/_Interface_IColony.md @@ -257,7 +257,7 @@ Set the deprecation of an extension in a colony. Secured function to authorised |Name|Type|Description| |---|---|---| -|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|extension|address|The address of the extension installation |deprecated|bool|Whether to deprecate the extension or not @@ -1038,6 +1038,11 @@ 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` @@ -1894,7 +1899,7 @@ Uninstall an extension from a colony. Secured function to authorised members. |Name|Type|Description| |---|---|---| -|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|extension|address|The address of the extension installation ### `unlockToken` @@ -1964,7 +1969,7 @@ Upgrade an extension in a colony. Secured function to authorised members. |Name|Type|Description| |---|---|---| -|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|extension|address|The address of the extension installation |newVersion|uint256|The version to upgrade to (must be one larger than the current version) diff --git a/docs/_Interface_IColonyNetwork.md b/docs/_Interface_IColonyNetwork.md index e0a507aa88..ec931e7a70 100644 --- a/docs/_Interface_IColonyNetwork.md +++ b/docs/_Interface_IColonyNetwork.md @@ -230,7 +230,7 @@ Set the deprecation of an extension in a colony. Can only be called by a Colony. |Name|Type|Description| |---|---|---| -|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|extension|address|Address of the extension installation |deprecated|bool|Whether to deprecate the extension or not @@ -352,6 +352,23 @@ Get an extension's installation. |---|---|---| |installation|address|The address of the installed extension +### `getExtensionMultiInstallation` + +Get an extension's installed colony. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extension|address|Address of the extension installation + +**Return Parameters** + +|Name|Type|Description| +|---|---|---| +|colony|address|Address of the colony the extension is installed in + ### `getExtensionResolver` Get an extension's resolver. @@ -663,6 +680,11 @@ 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` @@ -934,7 +956,7 @@ Uninstall an extension in a colony. Can only be called by a Colony. |Name|Type|Description| |---|---|---| -|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|extension|address|Address of the extension installation ### `unstakeForMining` @@ -982,5 +1004,5 @@ Upgrade an extension in a colony. Can only be called by a Colony. |Name|Type|Description| |---|---|---| -|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|extension|address|Address of the extension installation |newVersion|uint256|Version of the extension to upgrade to (must be one greater than current) \ No newline at end of file diff --git a/helpers/test-helper.js b/helpers/test-helper.js index 69b4c86dae..44f1ac31a0 100644 --- a/helpers/test-helper.js +++ b/helpers/test-helper.js @@ -1004,3 +1004,7 @@ export function bn2bytes32(x, size = 64) { export function rolesToBytes32(roles) { return `0x${new BN(roles.map((role) => new BN(1).shln(role)).reduce((a, b) => a.or(b), new BN(0))).toString(16, 64)}`; } + +export function getExtensionAddressFromTx(installExtensionTx) { + return `0x${installExtensionTx.receipt.rawLogs[1].topics[2].slice(26)}`; +} diff --git a/test/contracts-network/colony-network-extensions.js b/test/contracts-network/colony-network-extensions.js index 9e89c309ef..e5eff9f973 100644 --- a/test/contracts-network/colony-network-extensions.js +++ b/test/contracts-network/colony-network-extensions.js @@ -6,7 +6,7 @@ import { BN } from "bn.js"; import { ethers } from "ethers"; import { soliditySha3 } from "web3-utils"; -import { checkErrorRevert, web3GetBalance, encodeTxData } from "../../helpers/test-helper"; +import { checkErrorRevert, web3GetBalance, encodeTxData, getExtensionAddressFromTx } from "../../helpers/test-helper"; import { setupEtherRouter } from "../../helpers/upgradable-contracts"; import { setupColonyNetwork, setupMetaColonyWithLockedCLNYToken, setupRandomColony } from "../../helpers/test-data-generator"; import { UINT256_MAX } from "../../helpers/constants"; @@ -146,9 +146,9 @@ contract("Colony Network Extensions", (accounts) => { }); it("allows a root user to install an extension with any version", async () => { - await colony.installExtension(TEST_EXTENSION, 2, { from: ROOT }); + const tx = await colony.installExtension(TEST_EXTENSION, 2, { from: ROOT }); - const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); + const extensionAddress = getExtensionAddressFromTx(tx); const extension = await TestExtension2.at(extensionAddress); const owner = await extension.owner(); expect(owner).to.equal(colonyNetwork.address); @@ -167,12 +167,6 @@ contract("Colony Network Extensions", (accounts) => { 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("does not allow an extension to be installed twice", async () => { - await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); - - await checkErrorRevert(colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }), "colony-network-extension-already-installed"); - }); }); describe("upgrading extensions", () => { @@ -183,16 +177,14 @@ contract("Colony Network Extensions", (accounts) => { }); it("allows root users to upgrade an extension", async () => { - await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); - - const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); - expect(extensionAddress).to.not.equal(ethers.constants.AddressZero); + 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); - await colony.upgradeExtension(TEST_EXTENSION, 2, { from: ROOT }); + await colony.upgradeExtension(extensionAddress, 2, { from: ROOT }); extension = await ColonyExtension.at(extensionAddress); version = await extension.version(); @@ -200,27 +192,30 @@ contract("Colony Network Extensions", (accounts) => { }); it("does not allow non-root users to upgrade an extension", async () => { - await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); - await checkErrorRevert(colony.upgradeExtension(TEST_EXTENSION, 2, { from: ARCHITECT }), "ds-auth-unauthorized"); - await checkErrorRevert(colony.upgradeExtension(TEST_EXTENSION, 2, { from: USER }), "ds-auth-unauthorized"); + const extensionAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.upgradeExtension(extensionAddress, 2, { from: ARCHITECT }), "ds-auth-unauthorized"); + await checkErrorRevert(colony.upgradeExtension(extensionAddress, 2, { from: USER }), "ds-auth-unauthorized"); }); it("does not allow upgrading a extension which is not installed", async () => { - await checkErrorRevert(colony.upgradeExtension(TEST_EXTENSION, 2, { from: ROOT }), "colony-network-extension-not-installed"); + await checkErrorRevert(colony.upgradeExtension(ethers.constants.AddressZero, 2, { from: ROOT }), "colony-network-extension-not-installed"); }); it("does not allow upgrading a extension to a version which does not exist", async () => { - await colony.installExtension(TEST_EXTENSION, 3, { from: ROOT }); + const tx = await colony.installExtension(TEST_EXTENSION, 3, { from: ROOT }); // Can't upgrade from version 3 to nonexistent 4 - await checkErrorRevert(colony.upgradeExtension(TEST_EXTENSION, 4, { from: ROOT }), "colony-network-extension-bad-version"); + const extensionAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.upgradeExtension(extensionAddress, 4, { from: ROOT }), "colony-network-extension-bad-version"); }); it("does not allow upgrading a extension out of order", async () => { - await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); - await checkErrorRevert(colony.upgradeExtension(TEST_EXTENSION, 3, { from: ROOT }), "colony-network-extension-bad-increment"); + const extensionAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.upgradeExtension(extensionAddress, 3, { from: ROOT }), "colony-network-extension-bad-increment"); }); }); @@ -230,26 +225,27 @@ contract("Colony Network Extensions", (accounts) => { }); it("allows root users to deprecate and undeprecate an extension", async () => { - await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); - const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); + const extensionAddress = getExtensionAddressFromTx(tx); const extension = await TestExtension1.at(extensionAddress); await extension.foo(); - await colony.deprecateExtension(TEST_EXTENSION, true, { from: ROOT }); + await colony.deprecateExtension(extensionAddress, true, { from: ROOT }); await checkErrorRevert(extension.foo(), "colony-extension-deprecated"); - await colony.deprecateExtension(TEST_EXTENSION, false, { from: ROOT }); + await colony.deprecateExtension(extensionAddress, false, { from: ROOT }); await extension.foo(); }); it("does not allow non-root users to deprecate an extension", async () => { - await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); - await checkErrorRevert(colony.deprecateExtension(TEST_EXTENSION, true, { from: ARCHITECT }), "ds-auth-unauthorized"); + const extensionAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.deprecateExtension(extensionAddress, true, { from: ARCHITECT }), "ds-auth-unauthorized"); }); }); @@ -259,29 +255,28 @@ contract("Colony Network Extensions", (accounts) => { }); it("allows root users to uninstall an extension and send ether to the beneficiary", async () => { - await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); - const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); + const extensionAddress = getExtensionAddressFromTx(tx); const extension = await TestExtension1.at(extensionAddress); await extension.send(100); // Only colonyNetwork can uninstall await checkErrorRevert(extension.uninstall(), "ds-auth-unauthorized"); - await colony.uninstallExtension(TEST_EXTENSION, { from: ROOT }); + await colony.uninstallExtension(extensionAddress, { 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(TEST_EXTENSION, { from: ARCHITECT }), "ds-auth-unauthorized"); - - await checkErrorRevert(colony.uninstallExtension(TEST_EXTENSION, { from: USER }), "ds-auth-unauthorized"); + await checkErrorRevert(colony.uninstallExtension(ethers.constants.AddressZero, { from: ARCHITECT }), "ds-auth-unauthorized"); + await checkErrorRevert(colony.uninstallExtension(ethers.constants.AddressZero, { from: USER }), "ds-auth-unauthorized"); }); it("does not allow root users to uninstall an extension which is not installed", async () => { - await checkErrorRevert(colony.uninstallExtension(TEST_EXTENSION, { from: ROOT }), "colony-network-extension-not-installed"); + await checkErrorRevert(colony.uninstallExtension(ethers.constants.AddressZero, { from: ROOT }), "colony-network-extension-not-installed"); }); }); @@ -294,8 +289,9 @@ contract("Colony Network Extensions", (accounts) => { const tokenLockingAddress = await colonyNetwork.getTokenLocking(); const tokenLocking = await ITokenLocking.at(tokenLockingAddress); - await colony.installExtension(TEST_VOTING_TOKEN, 1, { from: ROOT }); - const testVotingTokenAddress = await colonyNetwork.getExtensionInstallation(TEST_VOTING_TOKEN, colony.address); + const tx = await colony.installExtension(TEST_VOTING_TOKEN, 1, { from: ROOT }); + + const testVotingTokenAddress = getExtensionAddressFromTx(tx); const testVotingToken = await TestVotingToken.at(testVotingTokenAddress); const lockCountPre = await tokenLocking.getTotalLockCount(token.address); @@ -337,8 +333,9 @@ contract("Colony Network Extensions", (accounts) => { const tokenLockingAddress = await colonyNetwork.getTokenLocking(); const tokenLocking = await ITokenLocking.at(tokenLockingAddress); - await colony.installExtension(TEST_VOTING_TOKEN, 1, { from: ROOT }); - const testVotingTokenAddress = await colonyNetwork.getExtensionInstallation(TEST_VOTING_TOKEN, colony.address); + const tx = await colony.installExtension(TEST_VOTING_TOKEN, 1, { from: ROOT }); + + const testVotingTokenAddress = getExtensionAddressFromTx(tx); const testVotingToken = await TestVotingToken.at(testVotingTokenAddress); await testVotingToken.lockToken(); From 5a7b35c25d9f9d4fb8b15cf310b032aeec217d53 Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Mon, 28 Jun 2021 13:24:37 -0700 Subject: [PATCH 02/11] Add support for deprecated interface --- contracts/colony/Colony.sol | 44 ++++--- contracts/colony/ColonyStorage.sol | 32 ++--- 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-arbitrary-transactions.js | 10 +- .../colony-network-extensions.js | 113 +++++++++++++++--- test/contracts-network/token-locking.js | 20 +++- test/extensions/coin-machine.js | 85 +++++++------ test/extensions/funding-queue.js | 18 ++- test/extensions/one-tx-payment.js | 14 +-- test/extensions/token-supplier.js | 27 +++-- test/extensions/voting-rep.js | 20 ++-- test/extensions/whitelist.js | 17 ++- 17 files changed, 436 insertions(+), 158 deletions(-) diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index 6c349fed31..1092ddac8a 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -79,10 +79,12 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { external stoppable self returns (bool) { - // Prevent transactions to network contracts + // Prevent transactions to network contracts or network-managed extensions installed in this colony require(_to != address(this), "colony-cannot-target-self"); require(_to != colonyNetworkAddress, "colony-cannot-target-network"); require(_to != tokenLockingAddress, "colony-cannot-target-token-locking"); + require(isContract(_to), "colony-must-target-contract"); + require(!isOwnExtension(_to), "colony-cannot-target-own-extensions"); // Prevent transactions to transfer held tokens bytes4 sig; @@ -93,16 +95,6 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { else if (sig == TRANSFER_SIG) { transferTransactionPreparation(_to, _action); } else if (sig == BURN_GUY_SIG || sig == TRANSFER_FROM_SIG) { burnGuyOrTransferFromTransactionPreparation(_action); } - // Prevent transactions to network-managed extensions installed in this colony - require(isContract(_to), "colony-to-must-be-contract"); - // slither-disable-next-line unused-return - try ColonyExtension(_to).identifier() returns (bytes32 extensionId) { - require( - IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) != _to, - "colony-cannot-target-extensions" - ); - } catch {} - bool res = executeCall(_to, 0, _action); if (sig == APPROVE_SIG) { approveTransactionCleanup(_to, _action); } @@ -375,23 +367,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/ColonyStorage.sol b/contracts/colony/ColonyStorage.sol index f45e10f1a4..ab4c418209 100755 --- a/contracts/colony/ColonyStorage.sol +++ b/contracts/colony/ColonyStorage.sol @@ -239,20 +239,7 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes } modifier onlyExtension() { - // Ensure msg.sender is a contract - require(isContract(msg.sender), "colony-sender-must-be-contract"); - - // Ensure msg.sender is an extension, must check old & new formats - // slither-disable-next-line unused-return - try ColonyExtension(msg.sender).identifier() returns (bytes32 extensionId) { - require( - IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == msg.sender || - IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(msg.sender) == address(this), - "colony-must-be-extension" - ); - } catch { - require(false, "colony-must-be-extension"); - } + require(isOwnExtension(msg.sender), "colony-must-be-extension"); _; } @@ -301,6 +288,23 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes return size > 0; } + function isOwnExtension(address addr) internal returns (bool) { + // Ensure addr is a contract first, otherwise `try` block will revert + if (!isContract(addr)) { return false; } + + // // Ensure addr is an extension installed in the colony, must check old & new formats + // // slither-disable-next-line unused-return + try ColonyExtension(addr).identifier() returns (bytes32 extensionId) { + return ( + IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == addr || + IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(addr) == address(this) + ); + } catch { + return false; + } + + } + function domainExists(uint256 domainId) internal view returns (bool) { return domainId > 0 && domainId <= domainCount; } diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol index 9b1ccfdfe9..16eecfbb76 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -266,21 +266,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 09d7834339..ca24f8499c 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 @@ -1902,6 +1915,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 @@ -1973,6 +1999,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-arbitrary-transactions.js b/test/contracts-network/colony-arbitrary-transactions.js index a29e7ee53e..ffffb164ee 100644 --- a/test/contracts-network/colony-arbitrary-transactions.js +++ b/test/contracts-network/colony-arbitrary-transactions.js @@ -6,7 +6,7 @@ import { ethers } from "ethers"; import { soliditySha3 } from "web3-utils"; import { UINT256_MAX, WAD } from "../../helpers/constants"; -import { checkErrorRevert, encodeTxData } from "../../helpers/test-helper"; +import { checkErrorRevert, encodeTxData, getExtensionAddressFromTx } from "../../helpers/test-helper"; import { setupRandomColony, fundColonyWithTokens } from "../../helpers/test-data-generator"; const { expect } = chai; @@ -103,7 +103,7 @@ contract("Colony Arbitrary Transactions", (accounts) => { }); it("should not be able to make arbitrary transactions to a user address", async () => { - await checkErrorRevert(colony.makeArbitraryTransaction(accounts[0], "0x0"), "colony-to-must-be-contract"); + await checkErrorRevert(colony.makeArbitraryTransaction(accounts[0], "0x0"), "colony-must-target-contract"); }); it("should not be able to make arbitrary transactions to network or token locking", async () => { @@ -206,16 +206,16 @@ contract("Colony Arbitrary Transactions", (accounts) => { it("should not be able to make arbitrary transactions to the colony's own extensions", async () => { const COIN_MACHINE = soliditySha3("CoinMachine"); - await colony.installExtension(COIN_MACHINE, 2); + const tx = await colony.installExtension(COIN_MACHINE, 2); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + const coinMachineAddress = getExtensionAddressFromTx(tx); const coinMachine = await CoinMachine.at(coinMachineAddress); await coinMachine.initialise(token.address, ethers.constants.AddressZero, 60 * 60, 10, WAD, WAD, WAD, WAD, ADDRESS_ZERO); await token.mint(coinMachine.address, WAD); const action = await encodeTxData(coinMachine, "buyTokens", [WAD]); - await checkErrorRevert(colony.makeArbitraryTransaction(coinMachine.address, action), "colony-cannot-target-extensions"); + await checkErrorRevert(colony.makeArbitraryTransaction(coinMachine.address, action), "colony-cannot-target-own-extensions"); // But other colonies can const { colony: otherColony } = await setupRandomColony(colonyNetwork); diff --git a/test/contracts-network/colony-network-extensions.js b/test/contracts-network/colony-network-extensions.js index e5eff9f973..d46b29b42e 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 @@ -184,7 +193,27 @@ contract("Colony Network Extensions", (accounts) => { let version = await extension.version(); expect(version).to.eq.BN(1); - await colony.upgradeExtension(extensionAddress, 2, { from: ROOT }); + await colony.methods["upgradeExtension(address,uint256)"](extensionAddress, 2, { from: ROOT }); + + extension = await ColonyExtension.at(extensionAddress); + version = await extension.version(); + 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(); @@ -195,12 +224,15 @@ contract("Colony Network Extensions", (accounts) => { const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); const extensionAddress = getExtensionAddressFromTx(tx); - await checkErrorRevert(colony.upgradeExtension(extensionAddress, 2, { from: ARCHITECT }), "ds-auth-unauthorized"); - await checkErrorRevert(colony.upgradeExtension(extensionAddress, 2, { from: USER }), "ds-auth-unauthorized"); + await checkErrorRevert(colony.methods["upgradeExtension(address,uint256)"](extensionAddress, 2, { from: ARCHITECT }), "ds-auth-unauthorized"); + await checkErrorRevert(colony.methods["upgradeExtension(address,uint256)"](extensionAddress, 2, { from: USER }), "ds-auth-unauthorized"); }); it("does not allow upgrading a extension which is not installed", async () => { - await checkErrorRevert(colony.upgradeExtension(ethers.constants.AddressZero, 2, { from: ROOT }), "colony-network-extension-not-installed"); + await checkErrorRevert( + colony.methods["upgradeExtension(address,uint256)"](ethers.constants.AddressZero, 2, { from: ROOT }), + "colony-network-extension-not-installed" + ); }); it("does not allow upgrading a extension to a version which does not exist", async () => { @@ -208,14 +240,20 @@ contract("Colony Network Extensions", (accounts) => { // Can't upgrade from version 3 to nonexistent 4 const extensionAddress = getExtensionAddressFromTx(tx); - await checkErrorRevert(colony.upgradeExtension(extensionAddress, 4, { from: ROOT }), "colony-network-extension-bad-version"); + await checkErrorRevert( + colony.methods["upgradeExtension(address,uint256)"](extensionAddress, 4, { from: ROOT }), + "colony-network-extension-bad-version" + ); }); it("does not allow upgrading a extension out of order", async () => { const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); const extensionAddress = getExtensionAddressFromTx(tx); - await checkErrorRevert(colony.upgradeExtension(extensionAddress, 3, { from: ROOT }), "colony-network-extension-bad-increment"); + await checkErrorRevert( + colony.methods["upgradeExtension(address,uint256)"](extensionAddress, 3, { from: ROOT }), + "colony-network-extension-bad-increment" + ); }); }); @@ -232,11 +270,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(); }); @@ -245,7 +305,7 @@ contract("Colony Network Extensions", (accounts) => { const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); const extensionAddress = getExtensionAddressFromTx(tx); - await checkErrorRevert(colony.deprecateExtension(extensionAddress, true, { from: ARCHITECT }), "ds-auth-unauthorized"); + await checkErrorRevert(colony.methods["deprecateExtension(address,bool)"](extensionAddress, true, { from: ARCHITECT }), "ds-auth-unauthorized"); }); }); @@ -264,19 +324,42 @@ contract("Colony Network Extensions", (accounts) => { // Only colonyNetwork can uninstall await checkErrorRevert(extension.uninstall(), "ds-auth-unauthorized"); - await colony.uninstallExtension(extensionAddress, { from: ROOT }); + await colony.methods["uninstallExtension(address)"](extensionAddress, { from: ROOT }); + + const colonyBalance = await web3GetBalance(colony.address); + 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"); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](ethers.constants.AddressZero, { from: USER }), "ds-auth-unauthorized"); }); it("does not allow root users to uninstall an extension which is not installed", async () => { - await checkErrorRevert(colony.uninstallExtension(ethers.constants.AddressZero, { from: ROOT }), "colony-network-extension-not-installed"); + await checkErrorRevert( + colony.methods["uninstallExtension(address)"](ethers.constants.AddressZero, { from: ROOT }), + "colony-network-extension-not-installed" + ); }); }); @@ -325,8 +408,8 @@ contract("Colony Network Extensions", (accounts) => { }); it("does not allow users to lock and unlock tokens", async () => { - await checkErrorRevert(colony.lockToken(), "colony-sender-must-be-contract"); - await checkErrorRevert(colony.unlockTokenForUser(ROOT, 0), "colony-sender-must-be-contract"); + await checkErrorRevert(colony.lockToken(), "colony-must-be-extension"); + await checkErrorRevert(colony.unlockTokenForUser(ROOT, 0), "colony-must-be-extension"); }); it("does not allow a colony to unlock a lock placed by another colony", async () => { diff --git a/test/contracts-network/token-locking.js b/test/contracts-network/token-locking.js index 85bd76415a..ef3b22cd3c 100644 --- a/test/contracts-network/token-locking.js +++ b/test/contracts-network/token-locking.js @@ -6,12 +6,20 @@ import { ethers } from "ethers"; import { soliditySha3 } from "web3-utils"; import TruffleLoader from "../../packages/reputation-miner/TruffleLoader"; -import { getTokenArgs, checkErrorRevert, makeReputationKey, advanceMiningCycleNoContest, expectEvent } from "../../helpers/test-helper"; +import ReputationMinerTestWrapper from "../../packages/reputation-miner/test/ReputationMinerTestWrapper"; + import { giveUserCLNYTokensAndStake, setupColony, setupRandomColony, fundColonyWithTokens } from "../../helpers/test-data-generator"; import { UINT256_MAX, DEFAULT_STAKE } from "../../helpers/constants"; import { setupEtherRouter } from "../../helpers/upgradable-contracts"; -import ReputationMinerTestWrapper from "../../packages/reputation-miner/test/ReputationMinerTestWrapper"; +import { + getTokenArgs, + checkErrorRevert, + makeReputationKey, + advanceMiningCycleNoContest, + expectEvent, + getExtensionAddressFromTx, +} from "../../helpers/test-helper"; const { expect } = chai; chai.use(bnChai(web3.utils.BN)); @@ -331,8 +339,8 @@ contract("Token Locking", (addresses) => { await token.approve(tokenLocking.address, usersTokens, { from: userAddress }); await tokenLocking.methods["deposit(address,uint256,bool)"](token.address, usersTokens, true, { from: userAddress }); - await colony.installExtension(TEST_VOTING_TOKEN, 1); - const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_VOTING_TOKEN, colony.address); + const tx = await colony.installExtension(TEST_VOTING_TOKEN, 1); + const extensionAddress = getExtensionAddressFromTx(tx); const votingToken = await TestVotingToken.at(extensionAddress); await votingToken.lockToken(); @@ -361,8 +369,8 @@ contract("Token Locking", (addresses) => { await token.approve(tokenLocking.address, usersTokens, { from: userAddress }); await tokenLocking.methods["deposit(address,uint256,bool)"](token.address, usersTokens, true, { from: userAddress }); - await colony.installExtension(TEST_VOTING_TOKEN, 1); - const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_VOTING_TOKEN, colony.address); + const tx = await colony.installExtension(TEST_VOTING_TOKEN, 1); + const extensionAddress = getExtensionAddressFromTx(tx); const votingToken = await TestVotingToken.at(extensionAddress); await votingToken.lockToken(); diff --git a/test/extensions/coin-machine.js b/test/extensions/coin-machine.js index 2b569dd177..3ecf15272d 100644 --- a/test/extensions/coin-machine.js +++ b/test/extensions/coin-machine.js @@ -18,6 +18,7 @@ import { currentBlockTime, forwardTimeTo, expectEvent, + getExtensionAddressFromTx, } from "../../helpers/test-helper"; import { @@ -73,9 +74,8 @@ contract("Coin Machine", (accounts) => { ({ colony, token } = await setupRandomColony(colonyNetwork)); purchaseToken = await setupRandomToken(); - await colony.installExtension(COIN_MACHINE, coinMachineVersion); - - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion); + const coinMachineAddress = getExtensionAddressFromTx(tx); coinMachine = await CoinMachine.at(coinMachineAddress); // Forward time to start of a Coin Machine period - so long a test doesn't take an hour to run, should be reproducible! @@ -109,16 +109,12 @@ contract("Coin Machine", (accounts) => { it("can install the extension with the extension manager", async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - - await checkErrorRevert( - colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }), - "colony-network-extension-already-installed" - ); + const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - await checkErrorRevert(colony.uninstallExtension(COIN_MACHINE, { from: USER1 }), "ds-auth-unauthorized"); + const coinMachineAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](coinMachineAddress, { from: USER1 }), "ds-auth-unauthorized"); - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); + await colony.methods["uninstallExtension(address)"](coinMachineAddress, { from: USER0 }); }); it("can send unsold tokens back to the colony", async () => { @@ -126,7 +122,7 @@ contract("Coin Machine", (accounts) => { await coinMachine.initialise(token.address, ADDRESS_ZERO, 60 * 60, 10, WAD, WAD, WAD, WAD, ADDRESS_ZERO); - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); + await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); const balance = await token.balanceOf(colony.address); expect(balance).to.eq.BN(WAD); @@ -271,9 +267,9 @@ contract("Coin Machine", (accounts) => { const otherToken = await Token.new("", "", 18); await otherToken.unlock(); - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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); await otherToken.mint(coinMachine.address, UINT128_MAX); @@ -290,9 +286,9 @@ contract("Coin Machine", (accounts) => { }); it("cannot buy more than the balance of tokens", async () => { - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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); await token.mint(coinMachine.address, WAD); @@ -319,8 +315,8 @@ contract("Coin Machine", (accounts) => { expect(locked).to.equal(true); colony = await setupColony(colonyNetwork, token.address); - await colony.installExtension(COIN_MACHINE, coinMachineVersion); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion); + const coinMachineAddress = getExtensionAddressFromTx(tx); coinMachine = await CoinMachine.at(coinMachineAddress); const tokenAuthority = await TokenAuthority.new(token.address, colony.address, [coinMachine.address]); @@ -346,7 +342,7 @@ contract("Coin Machine", (accounts) => { let deprecated = await coinMachine.getDeprecated(); expect(deprecated).to.equal(false); - await colony.deprecateExtension(COIN_MACHINE, true); + await colony.methods["deprecateExtension(address,bool)"](coinMachine.address, true); await checkErrorRevert(coinMachine.buyTokens(WAD, { from: USER0 }), "colony-extension-deprecated"); deprecated = await coinMachine.getDeprecated(); @@ -354,9 +350,9 @@ contract("Coin Machine", (accounts) => { }); it("can buy tokens with eth", async () => { - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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); await token.mint(coinMachine.address, UINT128_MAX); @@ -461,9 +457,11 @@ contract("Coin Machine", (accounts) => { }); it("cannot adjust prices while the token balance is zero", async () => { - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + let tx; + + await colony.methods["uninstallExtension(address)"](coinMachine.address, { from: USER0 }); + tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); + const coinMachineAddress = getExtensionAddressFromTx(tx); coinMachine = await CoinMachine.at(coinMachineAddress); await token.mint(coinMachine.address, WAD.muln(200)); @@ -477,7 +475,6 @@ contract("Coin Machine", (accounts) => { let currentPrice; let evolvePrice; - let tx; await purchaseToken.mint(USER0, maxPerPeriod.muln(10), { from: USER0 }); await purchaseToken.approve(coinMachine.address, maxPerPeriod.muln(10), { from: USER0 }); @@ -539,7 +536,7 @@ contract("Coin Machine", (accounts) => { expect(currentPrice).to.eq.BN(WAD); await coinMachine.buyTokens(maxPerPeriod, { from: USER0 }); - tx = await colony.deprecateExtension(COIN_MACHINE, true, { from: USER0 }); + tx = await colony.methods["deprecateExtension(address,bool)"](coinMachine.address, true, { from: USER0 }); // Full event signature because we bounce the call through the colony await expectEvent(tx, "PriceEvolutionSet(bool)", [false]); @@ -555,7 +552,7 @@ contract("Coin Machine", (accounts) => { // Now we undeprecate the extension (and advance a period) // Price doesn't adjust in the period you update - await colony.deprecateExtension(COIN_MACHINE, false, { from: USER0 }); + tx = await colony.methods["deprecateExtension(address,bool)"](coinMachine.address, false, { from: USER0 }); await forwardTime(periodLength.toNumber(), this); tx = await coinMachine.updatePeriod(); @@ -706,8 +703,8 @@ contract("Coin Machine", (accounts) => { colony = await setupColony(colonyNetwork, token.address); await token.unlock(); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + const tx = await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); + const coinMachineAddress = getExtensionAddressFromTx(tx); coinMachine = await CoinMachine.at(coinMachineAddress); await token.mint(coinMachine.address, UINT128_MAX); @@ -727,9 +724,9 @@ contract("Coin Machine", (accounts) => { purchaseToken = await Token.new("Test Token", "TEST", 9); await purchaseToken.unlock(); - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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); await token.mint(coinMachine.address, UINT128_MAX); @@ -797,9 +794,9 @@ contract("Coin Machine", (accounts) => { }); it("cannot buy more than their user limit allows", async () => { - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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); await token.mint(coinMachine.address, WAD.muln(1000)); @@ -871,9 +868,9 @@ contract("Coin Machine", (accounts) => { }); it("cannot set a user limit without a whitelist", async () => { - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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); await coinMachine.initialise(token.address, purchaseToken.address, 60 * 60, 10, WAD.muln(100), WAD.muln(200), WAD.divn(2), WAD, ADDRESS_ZERO); @@ -883,9 +880,9 @@ contract("Coin Machine", (accounts) => { }); it("can calculate the max purchase at any given time", async () => { - await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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); // Initial supply of 250 tokens diff --git a/test/extensions/funding-queue.js b/test/extensions/funding-queue.js index 86ef0211ec..c479f202d1 100644 --- a/test/extensions/funding-queue.js +++ b/test/extensions/funding-queue.js @@ -17,6 +17,7 @@ import { forwardTime, getBlockTime, removeSubdomainLimit, + getExtensionAddressFromTx, } from "../../helpers/test-helper"; import { @@ -112,9 +113,9 @@ contract("Funding Queues", (accounts) => { await colony.addDomain(1, 0, 2); domain1 = await colony.getDomain(1); domain2 = await colony.getDomain(2); - await colony.installExtension(FUNDING_QUEUE, fundingQueueVersion); - const fundingQueueAddress = await colonyNetwork.getExtensionInstallation(FUNDING_QUEUE, colony.address); + const tx = await colony.installExtension(FUNDING_QUEUE, fundingQueueVersion); + const fundingQueueAddress = getExtensionAddressFromTx(tx); fundingQueue = await FundingQueue.at(fundingQueueAddress); await colony.setFundingRole(1, UINT256_MAX, fundingQueue.address, 1, true); @@ -206,15 +207,12 @@ contract("Funding Queues", (accounts) => { it("can install the extension with the extension manager", async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(FUNDING_QUEUE, fundingQueueVersion, { from: USER0 }); + const tx = await colony.installExtension(FUNDING_QUEUE, fundingQueueVersion, { from: USER0 }); - await checkErrorRevert( - colony.installExtension(FUNDING_QUEUE, fundingQueueVersion, { from: USER0 }), - "colony-network-extension-already-installed" - ); - await checkErrorRevert(colony.uninstallExtension(FUNDING_QUEUE, { from: USER1 }), "ds-auth-unauthorized"); + const fundingQueueAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](fundingQueueAddress, { from: USER1 }), "ds-auth-unauthorized"); - await colony.uninstallExtension(FUNDING_QUEUE, { from: USER0 }); + await colony.methods["uninstallExtension(address)"](fundingQueueAddress, { from: USER0 }); }); }); @@ -232,7 +230,7 @@ contract("Funding Queues", (accounts) => { let deprecated = await fundingQueue.getDeprecated(); expect(deprecated).to.equal(false); - await colony.deprecateExtension(FUNDING_QUEUE, true); + await colony.methods["deprecateExtension(address,bool)"](fundingQueue.address, true); await checkErrorRevert( fundingQueue.createProposal(1, UINT256_MAX, 0, 1, 2, WAD, token.address, { from: USER0 }), diff --git a/test/extensions/one-tx-payment.js b/test/extensions/one-tx-payment.js index 65459f1d43..259df2c276 100644 --- a/test/extensions/one-tx-payment.js +++ b/test/extensions/one-tx-payment.js @@ -6,7 +6,7 @@ import { ethers } from "ethers"; import { soliditySha3 } from "web3-utils"; import { UINT256_MAX, WAD, INITIAL_FUNDING, GLOBAL_SKILL_ID, FUNDING_ROLE, ADMINISTRATION_ROLE } from "../../helpers/constants"; -import { checkErrorRevert, web3GetCode, rolesToBytes32, expectEvent } from "../../helpers/test-helper"; +import { checkErrorRevert, web3GetCode, rolesToBytes32, expectEvent, getExtensionAddressFromTx } from "../../helpers/test-helper"; import { setupColonyNetwork, setupMetaColonyWithLockedCLNYToken, setupRandomColony, fundColonyWithTokens } from "../../helpers/test-data-generator"; import { setupEtherRouter } from "../../helpers/upgradable-contracts"; @@ -57,8 +57,8 @@ contract("One transaction payments", (accounts) => { await fundColonyWithTokens(colony, token, INITIAL_FUNDING); - await colony.installExtension(ONE_TX_PAYMENT, oneTxPaymentVersion); - const oneTxPaymentAddress = await colonyNetwork.getExtensionInstallation(ONE_TX_PAYMENT, colony.address); + const tx = await colony.installExtension(ONE_TX_PAYMENT, oneTxPaymentVersion); + const oneTxPaymentAddress = getExtensionAddressFromTx(tx); oneTxPayment = await OneTxPayment.at(oneTxPaymentAddress); // Give extension funding and administration rights @@ -90,12 +90,12 @@ contract("One transaction payments", (accounts) => { it("can install the extension with the extension manager", async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(ONE_TX_PAYMENT, oneTxPaymentVersion); + const tx = await colony.installExtension(ONE_TX_PAYMENT, oneTxPaymentVersion); - await checkErrorRevert(colony.installExtension(ONE_TX_PAYMENT, oneTxPaymentVersion), "colony-network-extension-already-installed"); - await checkErrorRevert(colony.uninstallExtension(ONE_TX_PAYMENT, { from: USER1 }), "ds-auth-unauthorized"); + const oneTxPaymentAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](oneTxPaymentAddress, { from: USER1 }), "ds-auth-unauthorized"); - await colony.uninstallExtension(ONE_TX_PAYMENT); + await colony.methods["uninstallExtension(address)"](oneTxPaymentAddress); }); }); diff --git a/test/extensions/token-supplier.js b/test/extensions/token-supplier.js index e21de7518c..6d76e75123 100644 --- a/test/extensions/token-supplier.js +++ b/test/extensions/token-supplier.js @@ -7,10 +7,18 @@ import { ethers } from "ethers"; import { soliditySha3 } from "web3-utils"; import { UINT256_MAX, WAD, SECONDS_PER_DAY } from "../../helpers/constants"; -import { checkErrorRevert, currentBlockTime, makeTxAtTimestamp, getBlockTime, forwardTime } from "../../helpers/test-helper"; import { setupColonyNetwork, setupRandomColony, setupMetaColonyWithLockedCLNYToken } from "../../helpers/test-data-generator"; import { setupEtherRouter } from "../../helpers/upgradable-contracts"; +import { + checkErrorRevert, + currentBlockTime, + makeTxAtTimestamp, + getBlockTime, + forwardTime, + getExtensionAddressFromTx, +} from "../../helpers/test-helper"; + const { expect } = chai; chai.use(bnChai(web3.utils.BN)); @@ -50,10 +58,10 @@ contract("Token Supplier", (accounts) => { beforeEach(async () => { ({ colony, token } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(TOKEN_SUPPLIER, tokenSupplierVersion); await colony.addDomain(1, UINT256_MAX, 1); - const tokenSupplierAddress = await colonyNetwork.getExtensionInstallation(TOKEN_SUPPLIER, colony.address); + const tx = await colony.installExtension(TOKEN_SUPPLIER, tokenSupplierVersion); + const tokenSupplierAddress = getExtensionAddressFromTx(tx); tokenSupplier = await TokenSupplier.at(tokenSupplierAddress); await colony.setRootRole(tokenSupplier.address, true); @@ -76,15 +84,12 @@ contract("Token Supplier", (accounts) => { it("can install the extension with the extension manager", async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(TOKEN_SUPPLIER, tokenSupplierVersion, { from: USER0 }); + const tx = await colony.installExtension(TOKEN_SUPPLIER, tokenSupplierVersion, { from: USER0 }); - await checkErrorRevert( - colony.installExtension(TOKEN_SUPPLIER, tokenSupplierVersion, { from: USER0 }), - "colony-network-extension-already-installed" - ); - await checkErrorRevert(colony.uninstallExtension(TOKEN_SUPPLIER, { from: USER1 }), "ds-auth-unauthorized"); + const tokenSupplierAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](tokenSupplierAddress, { from: USER1 }), "ds-auth-unauthorized"); - await colony.uninstallExtension(TOKEN_SUPPLIER, { from: USER0 }); + await colony.methods["uninstallExtension(address)"](tokenSupplierAddress, { from: USER0 }); }); it("can initialise", async () => { @@ -126,7 +131,7 @@ contract("Token Supplier", (accounts) => { token.mint(SUPPLY_CEILING.muln(3)); - await checkErrorRevert(tokenSupplier.setTokenSupplyCeiling(SUPPLY_CEILING.muln(2)), "token-supplier-ceiling-too-low"); + await checkErrorRevert(tokenSupplier.setTokenSupplyCeiling(SUPPLY_CEILING.muln(2).subn(1)), "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 75160d125d..420f67faaf 100644 --- a/test/extensions/voting-rep.js +++ b/test/extensions/voting-rep.js @@ -19,6 +19,7 @@ import { encodeTxData, bn2bytes32, expectEvent, + getExtensionAddressFromTx, } from "../../helpers/test-helper"; import { @@ -143,8 +144,8 @@ contract("Voting Reputation", (accounts) => { domain2 = await colony.getDomain(2); domain3 = await colony.getDomain(3); - await colony.installExtension(VOTING_REPUTATION, reputationVotingVersion); - const votingAddress = await colonyNetwork.getExtensionInstallation(VOTING_REPUTATION, colony.address); + const tx = await colony.installExtension(VOTING_REPUTATION, reputationVotingVersion); + const votingAddress = getExtensionAddressFromTx(tx); voting = await VotingReputation.at(votingAddress); await voting.initialise( @@ -275,23 +276,20 @@ contract("Voting Reputation", (accounts) => { it("can install the extension with the extension manager", async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(VOTING_REPUTATION, reputationVotingVersion, { from: USER0 }); + const tx = await colony.installExtension(VOTING_REPUTATION, reputationVotingVersion, { from: USER0 }); - await checkErrorRevert( - colony.installExtension(VOTING_REPUTATION, reputationVotingVersion, { from: USER0 }), - "colony-network-extension-already-installed" - ); - await checkErrorRevert(colony.uninstallExtension(VOTING_REPUTATION, { from: USER1 }), "ds-auth-unauthorized"); + const votingAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](votingAddress, { from: USER1 }), "ds-auth-unauthorized"); - await colony.uninstallExtension(VOTING_REPUTATION, { from: USER0 }); + await colony.methods["uninstallExtension(address)"](votingAddress, { from: USER0 }); }); it("can deprecate the extension if root", async () => { let deprecated = await voting.getDeprecated(); expect(deprecated).to.equal(false); - await checkErrorRevert(colony.deprecateExtension(VOTING_REPUTATION, true, { from: USER2 }), "ds-auth-unauthorized"); - await colony.deprecateExtension(VOTING_REPUTATION, true); + await checkErrorRevert(colony.methods["deprecateExtension(address,bool)"](voting.address, true, { from: USER2 }), "ds-auth-unauthorized"); + await colony.methods["deprecateExtension(address,bool)"](voting.address, true, { from: USER0 }); // Can't make new motions! const action = await encodeTxData(colony, "mintTokens", [WAD]); diff --git a/test/extensions/whitelist.js b/test/extensions/whitelist.js index 93bdf83f1a..c18dfd4ac9 100644 --- a/test/extensions/whitelist.js +++ b/test/extensions/whitelist.js @@ -7,7 +7,7 @@ import { soliditySha3 } from "web3-utils"; import { UINT256_MAX, IPFS_HASH } from "../../helpers/constants"; import { setupEtherRouter } from "../../helpers/upgradable-contracts"; -import { checkErrorRevert, web3GetCode } from "../../helpers/test-helper"; +import { checkErrorRevert, web3GetCode, getExtensionAddressFromTx } from "../../helpers/test-helper"; import { setupColonyNetwork, setupRandomColony, setupMetaColonyWithLockedCLNYToken } from "../../helpers/test-data-generator"; const { expect } = chai; @@ -46,9 +46,8 @@ contract("Whitelist", (accounts) => { beforeEach(async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(WHITELIST, whitelistVersion); - - const whitelistAddress = await colonyNetwork.getExtensionInstallation(WHITELIST, colony.address); + const tx = await colony.installExtension(WHITELIST, whitelistVersion); + const whitelistAddress = getExtensionAddressFromTx(tx); whitelist = await Whitelist.at(whitelistAddress); await colony.setAdministrationRole(1, UINT256_MAX, USER0, 1, true); @@ -79,12 +78,12 @@ contract("Whitelist", (accounts) => { it("can install the extension with the extension manager", async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(WHITELIST, whitelistVersion, { from: USER0 }); + const tx = await colony.installExtension(WHITELIST, whitelistVersion, { from: USER0 }); - await checkErrorRevert(colony.installExtension(WHITELIST, whitelistVersion, { from: USER0 }), "colony-network-extension-already-installed"); - await checkErrorRevert(colony.uninstallExtension(WHITELIST, { from: USER1 }), "ds-auth-unauthorized"); + const whitelistAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](whitelistAddress, { from: USER1 }), "ds-auth-unauthorized"); - await colony.uninstallExtension(WHITELIST, { from: USER0 }); + await colony.methods["uninstallExtension(address)"](whitelistAddress, { from: USER0 }); }); }); @@ -221,7 +220,7 @@ contract("Whitelist", (accounts) => { status = await whitelist.isApproved(USER1); expect(status).to.be.true; - await colony.deprecateExtension(WHITELIST, true, { from: USER0 }); + await colony.methods["deprecateExtension(address,bool)"](whitelist.address, true, { from: USER0 }); status = await whitelist.isApproved(USER1); expect(status).to.be.false; From 4de5fc8061261e65d0b13550afa5e871bb2a25fb Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Thu, 8 Jul 2021 10:19:48 -0700 Subject: [PATCH 03/11] Update slither and smoke tests --- contracts/colony/ColonyStorage.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/colony/ColonyStorage.sol b/contracts/colony/ColonyStorage.sol index ab4c418209..6b874cb71d 100755 --- a/contracts/colony/ColonyStorage.sol +++ b/contracts/colony/ColonyStorage.sol @@ -288,16 +288,16 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes return size > 0; } + // slither-disable-next-line unused-return function isOwnExtension(address addr) internal returns (bool) { // Ensure addr is a contract first, otherwise `try` block will revert if (!isContract(addr)) { return false; } - // // Ensure addr is an extension installed in the colony, must check old & new formats - // // slither-disable-next-line unused-return + // Ensure addr is an extension installed in the colony, must check old & new formats try ColonyExtension(addr).identifier() returns (bytes32 extensionId) { return ( - IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == addr || - IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(addr) == address(this) + IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(addr) == address(this) || + IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == addr ); } catch { return false; From edd490f4d3a70bffcb6a19a18a22d70b8f0a761a Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Fri, 9 Jul 2021 15:43:39 -0700 Subject: [PATCH 04/11] Update in response to review comments --- contracts/colony/Colony.sol | 21 ---- .../colonyNetwork/ColonyNetworkExtensions.sol | 26 ++-- contracts/colonyNetwork/IColonyNetwork.sol | 6 + docs/_Interface_IColonyNetwork.md | 18 +++ .../colony-network-extensions.js | 118 +++++++----------- 5 files changed, 82 insertions(+), 107 deletions(-) diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index 1092ddac8a..f65655d47e 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -370,39 +370,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 { diff --git a/contracts/colonyNetwork/ColonyNetworkExtensions.sol b/contracts/colonyNetwork/ColonyNetworkExtensions.sol index 59a5a367cf..c43592838f 100644 --- 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 index 71c9b293ed..2e3df8e107 100644 --- 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 index 5f28de4103..2de0e5711a 100644 --- 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 index d46b29b42e..c8fb4a9df5 100644 --- a/test/contracts-network/colony-network-extensions.js +++ b/test/contracts-network/colony-network-extensions.js @@ -29,7 +29,6 @@ const ContractEditing = artifacts.require("ContractEditing"); contract("Colony Network Extensions", (accounts) => { let colonyNetwork; - let editableColonyNetwork; let metaColony; let colony; let token; @@ -73,13 +72,6 @@ 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 @@ -154,11 +146,11 @@ 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); @@ -166,16 +158,55 @@ contract("Colony Network Extensions", (accounts) => { 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 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); + const editableColonyNetwork = await ContractEditing.at(colonyNetwork.address); + + 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", () => { @@ -200,26 +231,6 @@ 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 }); @@ -279,28 +290,6 @@ contract("Colony Network Extensions", (accounts) => { 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(); - }); - it("does not allow non-root users to deprecate an extension", async () => { const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); @@ -330,27 +319,6 @@ 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.methods["uninstallExtension(address)"](ethers.constants.AddressZero, { from: USER }), "ds-auth-unauthorized"); }); From b4a18b5d0a8382aa178ee5aaf3ba9b20eb84aa1d Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Fri, 9 Jul 2021 15:49:23 -0700 Subject: [PATCH 05/11] Update smoke tests --- test-smoke/colony-storage-consistent.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test-smoke/colony-storage-consistent.js b/test-smoke/colony-storage-consistent.js index 78eb1800f9..a4616ff266 100644 --- a/test-smoke/colony-storage-consistent.js +++ b/test-smoke/colony-storage-consistent.js @@ -154,11 +154,11 @@ contract("Contract Storage", (accounts) => { console.log("miningCycleStateHash:", miningCycleAccount.stateRoot.toString("hex")); console.log("tokenLockingStateHash:", tokenLockingAccount.stateRoot.toString("hex")); - expect(colonyNetworkAccount.stateRoot.toString("hex")).to.equal("55779b88bac4206c6ed791b93fa3a386c2e71b52d896d262c6b69c476a6968fe"); - expect(colonyAccount.stateRoot.toString("hex")).to.equal("abe2e24d6c366741cfb171ba03e452a705ef3ffce1715348fdfaccbdfd44fab2"); - expect(metaColonyAccount.stateRoot.toString("hex")).to.equal("78294685a492256887e3159e26071ba06d62883c72ccb26fd323a883eefc30fd"); - expect(miningCycleAccount.stateRoot.toString("hex")).to.equal("e105190bcd647989da1579ac209ae54ed63e08224fbb2469bad9f596773fe558"); - expect(tokenLockingAccount.stateRoot.toString("hex")).to.equal("8bab6ab2024de44a08765ee14ec3d6bdc0fa5ae2a5ee221c9f928345a3710658"); + expect(colonyNetworkAccount.stateRoot.toString("hex")).to.equal("82e6b44f4f0ffa1697daea594a335e85ee5f695fb996feaf65ad70db15f3c28f"); + expect(colonyAccount.stateRoot.toString("hex")).to.equal("f3bc877e660cc81eda6a406288d66cbd8b8793c6d0617645e1e1d782f029944e"); + expect(metaColonyAccount.stateRoot.toString("hex")).to.equal("d5fe57a5453202aa6bf438133e9d0b8399b42480e26529898aff08134b0a978f"); + expect(miningCycleAccount.stateRoot.toString("hex")).to.equal("1d957c0b904f02cccbd707bac8922cfb7c297170c4216da1b6f517b42c8c6871"); + expect(tokenLockingAccount.stateRoot.toString("hex")).to.equal("b8bd6248291c5618df041910f06c7344f20f2472596e06ce1ad991504f197a63"); }); }); }); From adaeabbc416ed66b9fb61dbdf116f52d555155e6 Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Tue, 27 Jul 2021 14:35:23 -0700 Subject: [PATCH 06/11] Update after rebasing --- test/extensions/evaluated-expenditures.js | 19 +++++++------------ test/extensions/voting-rep.js | 12 +++++------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/test/extensions/evaluated-expenditures.js b/test/extensions/evaluated-expenditures.js index 7bebe12aee..ffbba868ec 100644 --- a/test/extensions/evaluated-expenditures.js +++ b/test/extensions/evaluated-expenditures.js @@ -7,7 +7,7 @@ import { soliditySha3 } from "web3-utils"; import { UINT256_MAX, WAD } from "../../helpers/constants"; import { setupEtherRouter } from "../../helpers/upgradable-contracts"; -import { checkErrorRevert, web3GetCode } from "../../helpers/test-helper"; +import { checkErrorRevert, web3GetCode, getExtensionAddressFromTx } from "../../helpers/test-helper"; import { setupColonyNetwork, setupRandomColony, setupMetaColonyWithLockedCLNYToken } from "../../helpers/test-data-generator"; const { expect } = chai; @@ -46,9 +46,8 @@ contract("EvaluatedExpenditure", (accounts) => { beforeEach(async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(EVALUATED_EXPENDITURE, evaluatedExpenditureVersion); - - const evaluatedExpenditureAddress = await colonyNetwork.getExtensionInstallation(EVALUATED_EXPENDITURE, colony.address); + const tx = await colony.installExtension(EVALUATED_EXPENDITURE, evaluatedExpenditureVersion); + const evaluatedExpenditureAddress = getExtensionAddressFromTx(tx); evaluatedExpenditure = await EvaluatedExpenditure.at(evaluatedExpenditureAddress); await colony.setArbitrationRole(1, UINT256_MAX, evaluatedExpenditure.address, 1, true); @@ -79,16 +78,12 @@ contract("EvaluatedExpenditure", (accounts) => { it("can install the extension with the extension manager", async () => { ({ colony } = await setupRandomColony(colonyNetwork)); - await colony.installExtension(EVALUATED_EXPENDITURE, evaluatedExpenditureVersion, { from: USER0 }); - - await checkErrorRevert( - colony.installExtension(EVALUATED_EXPENDITURE, evaluatedExpenditureVersion, { from: USER0 }), - "colony-network-extension-already-installed" - ); + const tx = await colony.installExtension(EVALUATED_EXPENDITURE, evaluatedExpenditureVersion); - await checkErrorRevert(colony.uninstallExtension(EVALUATED_EXPENDITURE, { from: USER1 }), "ds-auth-unauthorized"); + const evaluatedExpenditureAddress = getExtensionAddressFromTx(tx); + await checkErrorRevert(colony.methods["uninstallExtension(address)"](evaluatedExpenditureAddress, { from: USER1 }), "ds-auth-unauthorized"); - await colony.uninstallExtension(EVALUATED_EXPENDITURE, { from: USER0 }); + await colony.methods["uninstallExtension(address)"](evaluatedExpenditureAddress, { from: USER0 }); }); }); diff --git a/test/extensions/voting-rep.js b/test/extensions/voting-rep.js index 420f67faaf..6cd1814b32 100644 --- a/test/extensions/voting-rep.js +++ b/test/extensions/voting-rep.js @@ -1232,9 +1232,6 @@ contract("Voting Reputation", (accounts) => { }); it("can take an action to install an extension", async () => { - let installation = await colonyNetwork.getExtensionInstallation(soliditySha3("OneTxPayment"), colony.address); - expect(installation).to.be.equal(ADDRESS_ZERO); - const oneTxPaymentImplementation = await OneTxPayment.new(); const resolver = await Resolver.new(); await setupEtherRouter("OneTxPayment", { OneTxPayment: oneTxPaymentImplementation.address }, resolver); @@ -1250,11 +1247,12 @@ contract("Voting Reputation", (accounts) => { await forwardTime(STAKE_PERIOD, this); - const { logs } = await voting.finalizeMotion(motionId); - expect(logs[1].args.executed).to.be.true; + const tx = await voting.finalizeMotion(motionId); + expect(tx.logs[1].args.executed).to.be.true; - installation = await colonyNetwork.getExtensionInstallation(soliditySha3("OneTxPayment"), colony.address); - expect(installation).to.not.be.equal(ADDRESS_ZERO); + const extensionAddress = getExtensionAddressFromTx(tx); + const colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extensionAddress); + expect(colonyAddress).to.equal(colony.address); }); it("can take an action with an arbitrary target", async () => { From 2e4d1c886036558ac34ec2c23cc95f81e87995ab Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Tue, 3 Aug 2021 10:37:56 -0700 Subject: [PATCH 07/11] Add support for old and new extension management schemes simultaneously --- contracts/colony/IColony.sol | 19 ---- .../colonyNetwork/ColonyNetworkExtensions.sol | 43 +++++--- contracts/colonyNetwork/IColonyNetwork.sol | 3 +- contracts/testHelpers/PreviousVersion.sol | 32 +++++- docs/_Interface_IColony.md | 39 -------- docs/_Interface_IColonyNetwork.md | 5 - .../colony-network-extensions.js | 97 +++++++++++++++++-- 7 files changed, 152 insertions(+), 86 deletions(-) diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol index 16eecfbb76..85c1eb6a21 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -266,35 +266,16 @@ 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 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 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 diff --git a/contracts/colonyNetwork/ColonyNetworkExtensions.sol b/contracts/colonyNetwork/ColonyNetworkExtensions.sol index c43592838f..9e4f0c3202 100644 --- a/contracts/colonyNetwork/ColonyNetworkExtensions.sol +++ b/contracts/colonyNetwork/ColonyNetworkExtensions.sol @@ -55,7 +55,14 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { require(resolvers[_extensionId][_version] != address(0x0), "colony-network-extension-bad-version"); EtherRouter extension = new EtherRouter(); - multiInstallations[address(extension)] = msg.sender; + + // Install in old installations mapping if version 7 or below + if (IColony(msg.sender).version() <= 7) { + require(installations[_extensionId][msg.sender] == address(0x0), "colony-network-extension-already-installed"); + installations[_extensionId][msg.sender] = address(extension); + } else { + multiInstallations[address(extension)] = msg.sender; + } extension.setResolver(resolvers[_extensionId][_version]); ColonyExtension(address(extension)).install(msg.sender); @@ -69,9 +76,17 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { function upgradeExtension(bytes32 _extensionId, uint256 _newVersion) public stoppable + calledByColony { - address extension = migrateToMultiExtension(_extensionId, msg.sender); - upgradeExtension(extension, _newVersion); + require(installations[_extensionId][msg.sender] != address(0x0), "colony-network-extension-not-installed"); + + address payable extension = installations[_extensionId][msg.sender]; + 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]); + ColonyExtension(extension).finishUpgrade(); + assert(ColonyExtension(extension).version() == _newVersion); emit ExtensionUpgraded(_extensionId, msg.sender, _newVersion); } @@ -100,9 +115,9 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { function deprecateExtension(bytes32 _extensionId, bool _deprecated) public stoppable + calledByColony { - address extension = migrateToMultiExtension(_extensionId, msg.sender); - deprecateExtension(extension, _deprecated); + ColonyExtension(installations[_extensionId][msg.sender]).deprecate(_deprecated); emit ExtensionDeprecated(_extensionId, msg.sender, _deprecated); } @@ -121,9 +136,13 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { function uninstallExtension(bytes32 _extensionId) public stoppable + calledByColony { - address extension = migrateToMultiExtension(_extensionId, msg.sender); - uninstallExtension(extension); + require(installations[_extensionId][msg.sender] != address(0x0), "colony-network-extension-not-installed"); + + ColonyExtension extension = ColonyExtension(installations[_extensionId][msg.sender]); + delete installations[_extensionId][msg.sender]; + extension.uninstall(); emit ExtensionUninstalled(_extensionId, msg.sender); } @@ -136,7 +155,6 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { require(multiInstallations[_extension] == msg.sender, "colony-network-extension-not-installed"); delete multiInstallations[_extension]; - ColonyExtension(_extension).uninstall(); emit ExtensionUninstalled(_extension, msg.sender); @@ -145,13 +163,12 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { function migrateToMultiExtension(bytes32 _extensionId, address _colony) public stoppable - returns (address) { - address extension = installations[_extensionId][_colony]; - require(extension != address(0x0), "colony-network-extension-not-installed"); + require(installations[_extensionId][_colony] != address(0x0), "colony-network-extension-not-installed"); + + multiInstallations[installations[_extensionId][_colony]] = payable(_colony); - multiInstallations[extension] = payable(_colony); - return extension; + delete installations[_extensionId][_colony]; } // Public view functions diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol index 2e3df8e107..e8f94728b7 100644 --- a/contracts/colonyNetwork/IColonyNetwork.sol +++ b/contracts/colonyNetwork/IColonyNetwork.sol @@ -353,8 +353,7 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery { /// @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); + function migrateToMultiExtension(bytes32 extensionId, address colony) external; /// @notice Get an extension's resolver. /// @param extensionId keccak256 hash of the extension name, used as an indentifier diff --git a/contracts/testHelpers/PreviousVersion.sol b/contracts/testHelpers/PreviousVersion.sol index bcb10479ab..7fec757173 100644 --- a/contracts/testHelpers/PreviousVersion.sol +++ b/contracts/testHelpers/PreviousVersion.sol @@ -1,5 +1,7 @@ pragma solidity 0.7.3; +import "./../colonyNetwork/IColonyNetwork.sol"; + contract Version3 { function version() pure external returns (uint256) { return 3; @@ -10,4 +12,32 @@ contract Version4 { function version() pure external returns (uint256) { return 4; } -} \ No newline at end of file +} + +contract Version7 { + function version() public pure returns (uint256) { + return 7; + } + + address colonyNetworkAddress; + + constructor(address _colonyNetworkAddress) public { + colonyNetworkAddress = _colonyNetworkAddress; + } + + function installExtension(bytes32 _extensionId, uint256 _version) public { + IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version); + } + + function upgradeExtension(bytes32 _extensionId, uint256 _newVersion) public { + IColonyNetwork(colonyNetworkAddress).upgradeExtension(_extensionId, _newVersion); + } + + function deprecateExtension(bytes32 _extensionId, bool _deprecated) public { + IColonyNetwork(colonyNetworkAddress).deprecateExtension(_extensionId, _deprecated); + } + + function uninstallExtension(bytes32 _extensionId) public { + IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extensionId); + } +} diff --git a/docs/_Interface_IColony.md b/docs/_Interface_IColony.md index ca24f8499c..09d7834339 100644 --- a/docs/_Interface_IColony.md +++ b/docs/_Interface_IColony.md @@ -261,19 +261,6 @@ 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 @@ -1915,19 +1902,6 @@ 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 @@ -1999,19 +1973,6 @@ 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 2de0e5711a..1f359eb5a6 100644 --- a/docs/_Interface_IColonyNetwork.md +++ b/docs/_Interface_IColonyNetwork.md @@ -745,11 +745,6 @@ Migrate extension bookkeeping to multiExtension |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` diff --git a/test/contracts-network/colony-network-extensions.js b/test/contracts-network/colony-network-extensions.js index c8fb4a9df5..e3312eba5b 100644 --- a/test/contracts-network/colony-network-extensions.js +++ b/test/contracts-network/colony-network-extensions.js @@ -26,9 +26,11 @@ const TestVotingToken = artifacts.require("TestVotingToken"); const Resolver = artifacts.require("Resolver"); const RequireExecuteCall = artifacts.require("RequireExecuteCall"); const ContractEditing = artifacts.require("ContractEditing"); +const Version7 = artifacts.require("Version7"); contract("Colony Network Extensions", (accounts) => { let colonyNetwork; + let editableColonyNetwork; let metaColony; let colony; let token; @@ -72,6 +74,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 @@ -182,13 +191,6 @@ contract("Colony Network Extensions", (accounts) => { }); it("allows colonies to migrate to multiExtension bookkeeping", async () => { - 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); - const editableColonyNetwork = await ContractEditing.at(colonyNetwork.address); - const extension = await TestExtension1.new(); await extension.install(colony.address); @@ -202,10 +204,34 @@ contract("Colony Network Extensions", (accounts) => { const value = `0x000000000000000000000000${extension.address.slice(2)}`; 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); colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extension.address); expect(colonyAddress).to.equal(colony.address); + + extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); + expect(extensionAddress).to.equal(ethers.constants.AddressZero); + }); + + it("allows old colonies to install extensions correctly", async () => { + const version7Colony = await Version7.new(colonyNetwork.address); + + // Add version7Colony to _isColony mapping + const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); + const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await version7Colony.installExtension(TEST_EXTENSION, 1); + + const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, version7Colony.address); + expect(extensionAddress).to.not.equal(ethers.constants.AddressZero); + + // But not twice + await checkErrorRevert(version7Colony.installExtension(TEST_EXTENSION, 1), "colony-network-extension-already-installed"); }); }); @@ -266,6 +292,25 @@ contract("Colony Network Extensions", (accounts) => { "colony-network-extension-bad-increment" ); }); + + it("allows old colonies to upgrade extensions correctly", async () => { + const version7Colony = await Version7.new(colonyNetwork.address); + + // Add version7Colony to _isColony mapping + const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); + const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await version7Colony.installExtension(TEST_EXTENSION, 1); + + const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, version7Colony.address); + + await version7Colony.upgradeExtension(TEST_EXTENSION, 2); + + const extension = await ColonyExtension.at(extensionAddress); + const version = await extension.version(); + expect(version).to.eq.BN(2); + }); }); describe("deprecating extensions", () => { @@ -296,6 +341,24 @@ contract("Colony Network Extensions", (accounts) => { const extensionAddress = getExtensionAddressFromTx(tx); await checkErrorRevert(colony.methods["deprecateExtension(address,bool)"](extensionAddress, true, { from: ARCHITECT }), "ds-auth-unauthorized"); }); + + it("allows old colonies to deprecate extensions correctly", async () => { + const version7Colony = await Version7.new(colonyNetwork.address); + + // Add version7Colony to _isColony mapping + const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); + const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await version7Colony.installExtension(TEST_EXTENSION, 1); + + const extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, version7Colony.address); + const extension = await TestExtension1.at(extensionAddress); + + await version7Colony.deprecateExtension(TEST_EXTENSION, true); + + await checkErrorRevert(extension.foo(), "colony-extension-deprecated"); + }); }); describe("uninstalling extensions", () => { @@ -329,6 +392,26 @@ contract("Colony Network Extensions", (accounts) => { "colony-network-extension-not-installed" ); }); + + it("allows old colonies to uninstall extensions correctly", async () => { + const version7Colony = await Version7.new(colonyNetwork.address); + + // Add version7Colony to _isColony mapping + const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19); + const value = `0x0000000000000000000000000000000000000000000000000000000000000001`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await version7Colony.installExtension(TEST_EXTENSION, 1); + + let extensionAddress; + extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, version7Colony.address); + expect(extensionAddress).to.not.equal(ethers.constants.AddressZero); + + await version7Colony.uninstallExtension(TEST_EXTENSION); + + extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, version7Colony.address); + expect(extensionAddress).to.equal(ethers.constants.AddressZero); + }); }); describe("using extensions", () => { From 693818c6bc99198730c456e4fc9e9ddfb09d7517 Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Tue, 3 Aug 2021 12:26:33 -0700 Subject: [PATCH 08/11] Update smoke tests --- test-smoke/colony-storage-consistent.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test-smoke/colony-storage-consistent.js b/test-smoke/colony-storage-consistent.js index a4616ff266..3e7bd1208d 100644 --- a/test-smoke/colony-storage-consistent.js +++ b/test-smoke/colony-storage-consistent.js @@ -154,11 +154,11 @@ contract("Contract Storage", (accounts) => { console.log("miningCycleStateHash:", miningCycleAccount.stateRoot.toString("hex")); console.log("tokenLockingStateHash:", tokenLockingAccount.stateRoot.toString("hex")); - expect(colonyNetworkAccount.stateRoot.toString("hex")).to.equal("82e6b44f4f0ffa1697daea594a335e85ee5f695fb996feaf65ad70db15f3c28f"); - expect(colonyAccount.stateRoot.toString("hex")).to.equal("f3bc877e660cc81eda6a406288d66cbd8b8793c6d0617645e1e1d782f029944e"); - expect(metaColonyAccount.stateRoot.toString("hex")).to.equal("d5fe57a5453202aa6bf438133e9d0b8399b42480e26529898aff08134b0a978f"); - expect(miningCycleAccount.stateRoot.toString("hex")).to.equal("1d957c0b904f02cccbd707bac8922cfb7c297170c4216da1b6f517b42c8c6871"); - expect(tokenLockingAccount.stateRoot.toString("hex")).to.equal("b8bd6248291c5618df041910f06c7344f20f2472596e06ce1ad991504f197a63"); + expect(colonyNetworkAccount.stateRoot.toString("hex")).to.equal("1679b16cb0efcdee6c8d387a8661918c010ed4cc8771ec660d3595e0c9a49ade"); + expect(colonyAccount.stateRoot.toString("hex")).to.equal("42d53a9f20f8425b12bff82beb45f7cc913aa9aa000d2d9c40b383c14f35701c"); + expect(metaColonyAccount.stateRoot.toString("hex")).to.equal("aa200a0942d03ee788ed61a51202576f2bfec1f9aa38d600706773e8b255ae64"); + expect(miningCycleAccount.stateRoot.toString("hex")).to.equal("71ac589b08983ae20a27996a781060274da4cd88a36a2a55d4f499379a8ab866"); + expect(tokenLockingAccount.stateRoot.toString("hex")).to.equal("252ae36ae857089fb21f0140041b815225f9ecd1f0e32f0e9c6716349fa74a4e"); }); }); }); From 30c0a32aa4aef0ca9da195288bcd04f79834c3f0 Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Wed, 11 Aug 2021 15:38:04 -0700 Subject: [PATCH 09/11] Respond to review comments II --- contracts/colony/Colony.sol | 10 ++++- contracts/colony/ColonyAuthority.sol | 1 + contracts/colony/ColonyStorage.sol | 2 +- contracts/colony/IColony.sol | 7 ++- .../colonyNetwork/ColonyNetworkDataTypes.sol | 8 +++- .../colonyNetwork/ColonyNetworkExtensions.sol | 16 ++++--- contracts/colonyNetwork/IColonyNetwork.sol | 10 ++--- docs/_Interface_IColony.md | 17 ++++--- docs/_Interface_IColonyNetwork.md | 26 +++++------ .../colony-network-extensions.js | 26 +++++------ test/extensions/coin-machine.js | 45 +++++++------------ test/extensions/token-supplier.js | 2 +- test/extensions/voting-rep.js | 2 +- 13 files changed, 90 insertions(+), 82 deletions(-) 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); }); From 2d6e41fdb4b5f6a51769456fe707aec9f1a6b05a Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Thu, 12 Aug 2021 12:28:40 -0700 Subject: [PATCH 10/11] Update smoke tests --- test-smoke/colony-storage-consistent.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test-smoke/colony-storage-consistent.js b/test-smoke/colony-storage-consistent.js index 3e7bd1208d..4d6c0136fb 100644 --- a/test-smoke/colony-storage-consistent.js +++ b/test-smoke/colony-storage-consistent.js @@ -154,11 +154,11 @@ contract("Contract Storage", (accounts) => { console.log("miningCycleStateHash:", miningCycleAccount.stateRoot.toString("hex")); console.log("tokenLockingStateHash:", tokenLockingAccount.stateRoot.toString("hex")); - expect(colonyNetworkAccount.stateRoot.toString("hex")).to.equal("1679b16cb0efcdee6c8d387a8661918c010ed4cc8771ec660d3595e0c9a49ade"); - expect(colonyAccount.stateRoot.toString("hex")).to.equal("42d53a9f20f8425b12bff82beb45f7cc913aa9aa000d2d9c40b383c14f35701c"); - expect(metaColonyAccount.stateRoot.toString("hex")).to.equal("aa200a0942d03ee788ed61a51202576f2bfec1f9aa38d600706773e8b255ae64"); - expect(miningCycleAccount.stateRoot.toString("hex")).to.equal("71ac589b08983ae20a27996a781060274da4cd88a36a2a55d4f499379a8ab866"); - expect(tokenLockingAccount.stateRoot.toString("hex")).to.equal("252ae36ae857089fb21f0140041b815225f9ecd1f0e32f0e9c6716349fa74a4e"); + expect(colonyNetworkAccount.stateRoot.toString("hex")).to.equal("20aafab74e93166b4453d850146ef4821336bd0ba3126700553bcf72dd919461"); + expect(colonyAccount.stateRoot.toString("hex")).to.equal("819d48ad7078741c91b128b1797f746568531a49f062d3bb251c02194b3974ac"); + expect(metaColonyAccount.stateRoot.toString("hex")).to.equal("9667e38b9b92fd7a405f8ed66ceb154d835ada404751b24a3186f2cdc665b9eb"); + expect(miningCycleAccount.stateRoot.toString("hex")).to.equal("6b36b3736e5360f9cd3af153a820728423e1e4589db8a65d2ac267447ee5b7c5"); + expect(tokenLockingAccount.stateRoot.toString("hex")).to.equal("01074ed59377000ae7e9222e7c5c8d25c6df11f851e15db9d8d56bd2915b182d"); }); }); }); From 5bd63748b822a89fef79b73f73266d8635a584fe Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Wed, 18 Aug 2021 07:42:01 -0700 Subject: [PATCH 11/11] Add ExtensionMigrated event --- contracts/colonyNetwork/ColonyNetworkDataTypes.sol | 6 ++++++ contracts/colonyNetwork/ColonyNetworkExtensions.sol | 8 ++++++-- test/contracts-network/colony-network-extensions.js | 5 +++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol index 7a4d1294e2..ff57758b88 100755 --- a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol +++ b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol @@ -164,6 +164,12 @@ interface ColonyNetworkDataTypes { /// @param colony The address of the colony event ExtensionUninstalled(address indexed extension, address indexed colony); + /// @notice Event logged when an extension is migrated from old to new storage schemes + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + /// @param extension Address of the extension installation + event ExtensionMigrated(bytes32 indexed extensionId, address indexed colony, address extension); + struct Skill { // total number of parent skills uint128 nParents; diff --git a/contracts/colonyNetwork/ColonyNetworkExtensions.sol b/contracts/colonyNetwork/ColonyNetworkExtensions.sol index 40b9dad30f..b3ba26fa05 100644 --- a/contracts/colonyNetwork/ColonyNetworkExtensions.sol +++ b/contracts/colonyNetwork/ColonyNetworkExtensions.sol @@ -168,11 +168,15 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { stoppable calledByColony { - require(installations[_extensionId][msg.sender] != address(0x0), "colony-network-extension-not-installed"); + address extension = installations[_extensionId][msg.sender]; + + require(extension != address(0x0), "colony-network-extension-not-installed"); - multiInstallations[installations[_extensionId][msg.sender]] = payable(msg.sender); + multiInstallations[extension] = payable(msg.sender); delete installations[_extensionId][msg.sender]; + + emit ExtensionMigrated(_extensionId, msg.sender, extension); } // Public view functions diff --git a/test/contracts-network/colony-network-extensions.js b/test/contracts-network/colony-network-extensions.js index 5df768df22..bc14246989 100644 --- a/test/contracts-network/colony-network-extensions.js +++ b/test/contracts-network/colony-network-extensions.js @@ -6,7 +6,7 @@ import { BN } from "bn.js"; import { ethers } from "ethers"; import { soliditySha3 } from "web3-utils"; -import { checkErrorRevert, web3GetBalance, encodeTxData, getExtensionAddressFromTx } from "../../helpers/test-helper"; +import { checkErrorRevert, web3GetBalance, encodeTxData, expectEvent, getExtensionAddressFromTx } from "../../helpers/test-helper"; import { setupEtherRouter } from "../../helpers/upgradable-contracts"; import { setupColonyNetwork, setupMetaColonyWithLockedCLNYToken, setupRandomColony } from "../../helpers/test-data-generator"; import { UINT256_MAX } from "../../helpers/constants"; @@ -208,7 +208,8 @@ contract("Colony Network Extensions", (accounts) => { extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address); expect(extensionAddress).to.not.equal(ethers.constants.AddressZero); - await colony.migrateToMultiExtension(TEST_EXTENSION); + const tx = await colony.migrateToMultiExtension(TEST_EXTENSION); + await expectEvent(tx, "ExtensionMigrated(bytes32 indexed,address indexed,address)", [TEST_EXTENSION, colony.address, extensionAddress]); colonyAddress = await colonyNetwork.getExtensionColony(extension.address); expect(colonyAddress).to.equal(colony.address);