diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index 48ef6bf318..a9be11b72f 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -49,10 +49,11 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain { public stoppable auth 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(!isOwnExtension(_to), "colony-cannot-target-own-extensions"); // Prevent transactions to transfer held tokens bytes4 sig; @@ -63,16 +64,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); } @@ -345,23 +336,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 f3be842e97..ce7b965f08 100755 --- a/contracts/colony/ColonyStorage.sol +++ b/contracts/colony/ColonyStorage.sol @@ -221,20 +221,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"); _; } @@ -283,6 +270,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 7827de9178..0b33dbfba0 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -250,21 +250,40 @@ interface IColony is ColonyDataTypes, IRecovery { /// @return extension The address of the extension installation function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); + /// @dev DEPRECATED + /// @notice Upgrade an extension in a colony. Secured function to authorised members. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + /// @param newVersion The version to upgrade to (must be one larger than the current version) + function upgradeExtension(bytes32 extensionId, uint256 newVersion) external; + /// @notice Upgrade an extension in a colony. Secured function to authorised members. /// @param extension The address of the extension installation /// @param newVersion The version to upgrade to (must be one larger than the current version) - function upgradeExtension(address payable extension, uint256 newVersion) external; + function upgradeExtension(address extension, uint256 newVersion) external; + + /// @dev DEPRECATED + /// @notice Set the deprecation of an extension in a colony. Secured function to authorised members. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + /// @param deprecated Whether to deprecate the extension or not + function deprecateExtension(bytes32 extensionId, bool deprecated) external; /// @notice Set the deprecation of an extension in a colony. Secured function to authorised members. /// @param extension The address of the extension installation /// @param deprecated Whether to deprecate the extension or not - function deprecateExtension(address payable extension, bool deprecated) external; + function deprecateExtension(address extension, bool deprecated) external; + + /// @dev DEPRECATED + /// @notice Uninstall an extension from a colony. Secured function to authorised members. + /// @dev This is a permanent action -- re-installing the extension will deploy a new contract + /// @dev It is recommended to deprecate an extension before uninstalling to allow active objects to be resolved + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + function uninstallExtension(bytes32 extensionId) external; /// @notice Uninstall an extension from a colony. Secured function to authorised members. /// @dev This is a permanent action -- re-installing the extension will deploy a new contract /// @dev It is recommended to deprecate an extension before uninstalling to allow active objects to be resolved /// @param extension The address of the extension installation - function uninstallExtension(address payable extension) external; + function uninstallExtension(address extension) external; /// @notice Add a colony domain, and its respective local skill under skill with id `_parentSkillId`. /// New funding pot is created and associated with the domain here. diff --git a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol index 6b3b1446d2..2094f8feff 100755 --- a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol +++ b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol @@ -121,18 +121,38 @@ interface ColonyNetworkDataTypes { /// @param version The version of the extension event ExtensionInstalled(bytes32 indexed extensionId, address indexed extension, address indexed colony, uint256 version); + /// @dev DEPRECATED + /// @notice Event logged when an extension is upgraded in a colony + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + /// @param version The new version of the extension + event ExtensionUpgraded(bytes32 indexed extensionId, address indexed colony, uint256 version); + /// @notice Event logged when an extension is upgraded in a colony /// @param extension Address of the extension installation /// @param colony The address of the colony /// @param version The new version of the extension event ExtensionUpgraded(address indexed extension, address indexed colony, uint256 version); + /// @dev DEPRECATED + /// @notice Event logged when an extension is (un)deprecated in a colony + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + /// @param deprecated Whether the extension is deprecated or not + event ExtensionDeprecated(bytes32 indexed extensionId, address indexed colony, bool deprecated); + /// @notice Event logged when an extension is (un)deprecated in a colony /// @param extension Address of the extension installation /// @param colony The address of the colony /// @param deprecated Whether the extension is deprecated or not event ExtensionDeprecated(address indexed extension, address indexed colony, bool deprecated); + /// @dev DEPRECATED + /// @notice Event logged when an extension is uninstalled from a colony + /// @param extensionId The identifier for the extension + /// @param colony The address of the colony + event ExtensionUninstalled(bytes32 indexed extensionId, address indexed colony); + /// @notice Event logged when an extension is uninstalled from a colony /// @param extension Address of the extension installation /// @param colony The address of the colony diff --git a/contracts/colonyNetwork/ColonyNetworkExtensions.sol b/contracts/colonyNetwork/ColonyNetworkExtensions.sol index 63656a3ca0..59a5a367cf 100644 --- a/contracts/colonyNetwork/ColonyNetworkExtensions.sol +++ b/contracts/colonyNetwork/ColonyNetworkExtensions.sol @@ -65,7 +65,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { return address(extension); } - function upgradeExtension(address payable _extension, uint256 _newVersion) + // Deprecated + function upgradeExtension(bytes32 _extensionId, uint256 _newVersion) + public + stoppable + { + address extension = migrateToMultiExtension(_extensionId); + upgradeExtension(extension, _newVersion); + + emit ExtensionUpgraded(_extensionId, msg.sender, _newVersion); + } + + function upgradeExtension(address _extension, uint256 _newVersion) public stoppable calledByColony @@ -77,7 +88,7 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { require(_newVersion == ColonyExtension(_extension).version() + 1, "colony-network-extension-bad-increment"); require(resolvers[extensionId][_newVersion] != address(0x0), "colony-network-extension-bad-version"); - EtherRouter(_extension).setResolver(resolvers[extensionId][_newVersion]); + EtherRouter(payable(_extension)).setResolver(resolvers[extensionId][_newVersion]); ColonyExtension(_extension).finishUpgrade(); assert(ColonyExtension(_extension).version() == _newVersion); @@ -85,7 +96,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { emit ExtensionUpgraded(_extension, msg.sender, _newVersion); } - function deprecateExtension(address payable _extension, bool _deprecated) + // Deprecated + function deprecateExtension(bytes32 _extensionId, bool _deprecated) + public + stoppable + { + address extension = migrateToMultiExtension(_extensionId); + deprecateExtension(extension, _deprecated); + + emit ExtensionDeprecated(_extensionId, msg.sender, _deprecated); + } + + function deprecateExtension(address _extension, bool _deprecated) public stoppable calledByColony @@ -95,7 +117,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { emit ExtensionDeprecated(_extension, msg.sender, _deprecated); } - function uninstallExtension(address payable _extension) + // Deprecated + function uninstallExtension(bytes32 _extensionId) + public + stoppable + { + address extension = migrateToMultiExtension(_extensionId); + uninstallExtension(extension); + + emit ExtensionUninstalled(_extensionId, msg.sender); + } + + function uninstallExtension(address _extension) public stoppable calledByColony @@ -150,4 +183,12 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage { address extension = Resolver(_resolver).lookup(VERSION_SIG); return ColonyExtension(extension).version(); } + + function migrateToMultiExtension(bytes32 _extensionId) internal returns (address) { + address extension = installations[_extensionId][msg.sender]; + require(extension != address(0x0), "colony-network-extension-not-installed"); + + multiInstallations[extension] = msg.sender; + return extension; + } } diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol index 8cfdd62677..71c9b293ed 100644 --- a/contracts/colonyNetwork/IColonyNetwork.sol +++ b/contracts/colonyNetwork/IColonyNetwork.sol @@ -319,19 +319,36 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery { /// @return extension The address of the extension installation function installExtension(bytes32 extensionId, uint256 version) external returns (address extension); + /// @dev DEPRECATED + /// @notice Upgrade an extension in a colony. Can only be called by a Colony. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + /// @param newVersion Version of the extension to upgrade to (must be one greater than current) + function upgradeExtension(bytes32 extensionId, uint256 newVersion) external; + /// @notice Upgrade an extension in a colony. Can only be called by a Colony. /// @param extension Address of the extension installation /// @param newVersion Version of the extension to upgrade to (must be one greater than current) - function upgradeExtension(address payable extension, uint256 newVersion) external; + function upgradeExtension(address extension, uint256 newVersion) external; + + /// @dev DEPRECATED + /// @notice Set the deprecation of an extension in a colony. Can only be called by a Colony. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + /// @param deprecated Whether to deprecate the extension or not + function deprecateExtension(bytes32 extensionId, bool deprecated) external; /// @notice Set the deprecation of an extension in a colony. Can only be called by a Colony. /// @param extension Address of the extension installation /// @param deprecated Whether to deprecate the extension or not - function deprecateExtension(address payable extension, bool deprecated) external; + function deprecateExtension(address extension, bool deprecated) external; + + /// @dev DEPRECATED + /// @notice Uninstall an extension in a colony. Can only be called by a Colony. + /// @param extensionId keccak256 hash of the extension name, used as an indentifier + function uninstallExtension(bytes32 extensionId) external; /// @notice Uninstall an extension in a colony. Can only be called by a Colony. /// @param extension Address of the extension installation - function uninstallExtension(address payable extension) external; + function uninstallExtension(address extension) external; /// @notice Get an extension's resolver. /// @param extensionId keccak256 hash of the extension name, used as an indentifier diff --git a/docs/_Interface_IColony.md b/docs/_Interface_IColony.md index f95637093e..65f998fb66 100644 --- a/docs/_Interface_IColony.md +++ b/docs/_Interface_IColony.md @@ -261,6 +261,19 @@ Set the deprecation of an extension in a colony. Secured function to authorised |deprecated|bool|Whether to deprecate the extension or not +### `deprecateExtension` + +Set the deprecation of an extension in a colony. Secured function to authorised members. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|deprecated|bool|Whether to deprecate the extension or not + + ### `editColony` Called to change the metadata associated with a colony. Expected to be a IPFS hash of a JSON blob, but not enforced to any degree by the contracts @@ -1761,6 +1774,19 @@ Uninstall an extension from a colony. Secured function to authorised members. |extension|address|The address of the extension installation +### `uninstallExtension` + +Uninstall an extension from a colony. Secured function to authorised members. + +*Note: This is a permanent action -- re-installing the extension will deploy a new contract* + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier + + ### `unlockToken` unlock the native colony token, if possible @@ -1832,6 +1858,19 @@ Upgrade an extension in a colony. Secured function to authorised members. |newVersion|uint256|The version to upgrade to (must be one larger than the current version) +### `upgradeExtension` + +Upgrade an extension in a colony. Secured function to authorised members. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|newVersion|uint256|The version to upgrade to (must be one larger than the current version) + + ### `userCanSetRoles` Check whether a given user can modify roles in the target domain `_childDomainId`. Mostly a convenience function to provide a uniform interface for extension contracts validating permissions diff --git a/docs/_Interface_IColonyNetwork.md b/docs/_Interface_IColonyNetwork.md index ec931e7a70..5f28de4103 100644 --- a/docs/_Interface_IColonyNetwork.md +++ b/docs/_Interface_IColonyNetwork.md @@ -234,6 +234,19 @@ Set the deprecation of an extension in a colony. Can only be called by a Colony. |deprecated|bool|Whether to deprecate the extension or not +### `deprecateExtension` + +Set the deprecation of an extension in a colony. Can only be called by a Colony. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier +|deprecated|bool|Whether to deprecate the extension or not + + ### `deprecateSkill` Mark a global skill as deprecated which stops new tasks and payments from using it. @@ -959,6 +972,18 @@ Uninstall an extension in a colony. Can only be called by a Colony. |extension|address|Address of the extension installation +### `uninstallExtension` + +Uninstall an extension in a colony. Can only be called by a Colony. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier + + ### `unstakeForMining` Unstake CLNY currently staked for reputation mining. @@ -1005,4 +1030,17 @@ Upgrade an extension in a colony. Can only be called by a Colony. |Name|Type|Description| |---|---|---| |extension|address|Address of the extension installation +|newVersion|uint256|Version of the extension to upgrade to (must be one greater than current) + + +### `upgradeExtension` + +Upgrade an extension in a colony. Can only be called by a Colony. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier |newVersion|uint256|Version of the extension to upgrade to (must be one greater than current) \ No newline at end of file diff --git a/test/contracts-network/colony-arbitrary-transactions.js b/test/contracts-network/colony-arbitrary-transactions.js index 9f096f8681..9c492fd57e 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; @@ -159,16 +159,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..072a5f7950 100644 --- a/test/contracts-network/colony-network-extensions.js +++ b/test/contracts-network/colony-network-extensions.js @@ -25,9 +25,11 @@ const TestExtension3 = artifacts.require("TestExtension3"); const TestVotingToken = artifacts.require("TestVotingToken"); const Resolver = artifacts.require("Resolver"); const RequireExecuteCall = artifacts.require("RequireExecuteCall"); +const ContractEditing = artifacts.require("ContractEditing"); contract("Colony Network Extensions", (accounts) => { let colonyNetwork; + let editableColonyNetwork; let metaColony; let colony; let token; @@ -71,6 +73,13 @@ contract("Colony Network Extensions", (accounts) => { colonyNetwork = await setupColonyNetwork(); ({ metaColony } = await setupMetaColonyWithLockedCLNYToken(colonyNetwork)); + const colonyNetworkAsER = await EtherRouter.at(colonyNetwork.address); + const colonyNetworkResolverAddress = await colonyNetworkAsER.resolver(); + const colonyNetworkResolver = await Resolver.at(colonyNetworkResolverAddress); + const contractEditing = await ContractEditing.new(); + await colonyNetworkResolver.register("setStorageSlot(uint256,bytes32)", contractEditing.address); + editableColonyNetwork = await ContractEditing.at(colonyNetwork.address); + ({ colony, token } = await setupRandomColony(colonyNetwork)); await colony.addDomain(1, UINT256_MAX, 1); // Domain 2 @@ -191,6 +200,26 @@ contract("Colony Network Extensions", (accounts) => { expect(version).to.eq.BN(2); }); + it("allows root users to upgrade an extension, with the deprecated interface", async () => { + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + + const extensionAddress = getExtensionAddressFromTx(tx); + let extension = await ColonyExtension.at(extensionAddress); + let version = await extension.version(); + expect(version).to.eq.BN(1); + + // Set up `installations` mapping in the old style + const slot = soliditySha3(`0x000000000000000000000000${colony.address.slice(2)}`, soliditySha3(TEST_EXTENSION, 39)); + const value = `0x000000000000000000000000${extensionAddress.slice(2)}`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await colony.methods["upgradeExtension(bytes32,uint256)"](TEST_EXTENSION, 2, { from: ROOT }); + + extension = await ColonyExtension.at(extensionAddress); + version = await extension.version(); + expect(version).to.eq.BN(2); + }); + it("does not allow non-root users to upgrade an extension", async () => { const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); @@ -232,11 +261,33 @@ contract("Colony Network Extensions", (accounts) => { await extension.foo(); - await colony.deprecateExtension(extensionAddress, true, { from: ROOT }); + await colony.methods["deprecateExtension(address,bool)"](extensionAddress, true, { from: ROOT }); await checkErrorRevert(extension.foo(), "colony-extension-deprecated"); - await colony.deprecateExtension(extensionAddress, false, { from: ROOT }); + await colony.methods["deprecateExtension(address,bool)"](extensionAddress, false, { from: ROOT }); + + await extension.foo(); + }); + + it("allows root users to deprecate and undeprecate an extension, with the deprecated interface", async () => { + const tx = await colony.installExtension(TEST_EXTENSION, 1, { from: ROOT }); + + const extensionAddress = getExtensionAddressFromTx(tx); + const extension = await TestExtension1.at(extensionAddress); + + // Set up `installations` mapping in the old style + const slot = soliditySha3(`0x000000000000000000000000${colony.address.slice(2)}`, soliditySha3(TEST_EXTENSION, 39)); + const value = `0x000000000000000000000000${extensionAddress.slice(2)}`; + await editableColonyNetwork.setStorageSlot(slot, value); + + await extension.foo(); + + await colony.methods["deprecateExtension(bytes32,bool)"](TEST_EXTENSION, true, { from: ROOT }); + + await checkErrorRevert(extension.foo(), "colony-extension-deprecated"); + + await colony.methods["deprecateExtension(bytes32,bool)"](TEST_EXTENSION, false, { from: ROOT }); await extension.foo(); }); @@ -264,7 +315,28 @@ 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); 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..4f40901773 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! @@ -272,8 +272,8 @@ contract("Coin Machine", (accounts) => { 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); + 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); @@ -291,8 +291,8 @@ 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); + 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 +319,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]); @@ -355,8 +355,8 @@ 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); + 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 +461,11 @@ contract("Coin Machine", (accounts) => { }); it("cannot adjust prices while the token balance is zero", async () => { + let tx; + await colony.uninstallExtension(COIN_MACHINE, { from: USER0 }); - await colony.installExtension(COIN_MACHINE, coinMachineVersion, { from: USER0 }); - const coinMachineAddress = await colonyNetwork.getExtensionInstallation(COIN_MACHINE, colony.address); + 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 +479,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 }); @@ -706,8 +707,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); @@ -728,8 +729,8 @@ contract("Coin Machine", (accounts) => { 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); + 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); @@ -798,8 +799,8 @@ 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); + 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)); @@ -872,8 +873,8 @@ 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); + 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); @@ -884,8 +885,8 @@ 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); + 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..64669f1ac2 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); diff --git a/test/extensions/one-tx-payment.js b/test/extensions/one-tx-payment.js index 65459f1d43..14f872f2da 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 diff --git a/test/extensions/token-supplier.js b/test/extensions/token-supplier.js index e21de7518c..fdb453b885 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); diff --git a/test/extensions/voting-rep.js b/test/extensions/voting-rep.js index 9ea68a128b..138ca48e3a 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( diff --git a/test/extensions/whitelist.js b/test/extensions/whitelist.js index 93bdf83f1a..67270b1380 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);