More proxy-colony permission tests (and others)

.circleci/config.yml

@@ -318,6 +318,11 @@ jobs:
           command: CHAIN_ID=100 pnpm run test:contracts:chainid:coverage
           environment:
             NODE_OPTIONS: --max-old-space-size=6144
+      - run:
+          name: "Running chainid tests for Mainnet"
+          command: CHAIN_ID=1 pnpm run test:contracts:chainid:coverage
+          environment:
+            NODE_OPTIONS: --max-old-space-size=6144
       - persist_to_workspace:
           root: ./
           paths:

contracts/bridging/ProxyColony.sol

@@ -96,9 +96,13 @@ contract ProxyColony is DSAuth, Multicall, CallWithGuards {
       require(_targets[i] != bridgeAddress, "colony-cannot-target-bridge");
       require(_targets[i] != owner, "colony-cannot-target-network");
 
-      (bool success, ) = callWithGuards(_targets[i], _payloads[i]);
+      (bool success, bytes memory returndata) = callWithGuards(_targets[i], _payloads[i]);
 
-      require(success, "colony-arbitrary-transaction-failed");
+      // Note that this is not a require because returndata might not be a string, and if we try
+      // to decode it we'll get a revert.
+      if (!success) {
+        revert(abi.decode(returndata, (string)));
+      }
     }
   }
 }

contracts/colony/ColonyArbitraryTransaction.sol

@@ -61,6 +61,7 @@ contract ColonyArbitraryTransaction is ColonyStorage {
     bytes memory payload = abi.encodeWithSignature("multicall(bytes[])", _actions);
 
     IColonyNetwork(colonyNetworkAddress).bridgeMessageToNetwork(_chainId, payload);
+    return true;
   }
 
   function makeArbitraryTransactions(

contracts/colonyNetwork/ColonyNetworkSkills.sol

@@ -22,7 +22,6 @@ import "./../reputationMiningCycle/IReputationMiningCycle.sol";
 import "./../common/Multicall.sol";
 import "./ColonyNetworkStorage.sol";
 import { IColonyBridge } from "./../bridging/IColonyBridge.sol";
-import { CallWithGuards } from "../common/CallWithGuards.sol";
 
 contract ColonyNetworkSkills is ColonyNetworkStorage, Multicall {
   // Skills

contracts/colonyNetwork/ColonyNetworkStorage.sol

@@ -24,18 +24,11 @@ import { CommonStorage } from "./../common/CommonStorage.sol";
 import { MultiChain } from "./../common/MultiChain.sol";
 import { ERC20Extended } from "./../common/ERC20Extended.sol";
 import { ColonyNetworkDataTypes } from "./ColonyNetworkDataTypes.sol";
-import { CallWithGuards } from "../common/CallWithGuards.sol";
 
 // ignore-file-swc-131
 // ignore-file-swc-108
 
-contract ColonyNetworkStorage is
-  ColonyNetworkDataTypes,
-  DSMath,
-  CommonStorage,
-  MultiChain,
-  CallWithGuards
-{
+contract ColonyNetworkStorage is ColonyNetworkDataTypes, DSMath, CommonStorage, MultiChain {
   // Number of colonies in the network
   uint256 colonyCount; // Storage slot 6
   // uint256 version number of the latest deployed Colony contract, used in creating new colonies
@@ -193,10 +186,6 @@ contract ColonyNetworkStorage is
     return _skillId >> 128;
   }
 
-  function isMiningChain() internal view returns (bool) {
-    return block.chainid == getMiningChainId();
-  }
-
   function getMiningChainId() public view returns (uint256) {
     return reputationMiningChainId;
   }

test/contracts-network/colony-network-extensions.js

@@ -305,6 +305,13 @@ contract("Colony Network Extensions", (accounts) => {
   });
 
   describe("using extensions", () => {
+    it("extension-managing functions on Network cannot be called by non-colony addresses", async () => {
+      await checkErrorRevert(colonyNetwork.installExtension(TEST_EXTENSION, 1, { from: ROOT }), "colony-caller-must-be-colony");
+      await checkErrorRevert(colonyNetwork.upgradeExtension(TEST_EXTENSION, 2, { from: ROOT }), "colony-caller-must-be-colony");
+      await checkErrorRevert(colonyNetwork.deprecateExtension(TEST_EXTENSION, true, { from: ROOT }), "colony-caller-must-be-colony");
+      await checkErrorRevert(colonyNetwork.uninstallExtension(TEST_EXTENSION, { from: ROOT }), "colony-caller-must-be-colony");
+    });
+
     it("allows network-managed extensions to lock and unlock tokens", async () => {
       const tokenLockingAddress = await colonyNetwork.getTokenLocking();
       const tokenLocking = await ITokenLocking.at(tokenLockingAddress);

test/contracts-network/colony-network.js

@@ -26,7 +26,7 @@ const {
   setStorageSlot,
 } = require("../../helpers/test-helper");
 
-const { CURR_VERSION, MIN_STAKE, IPFS_HASH, ADDRESS_ZERO, WAD } = require("../../helpers/constants");
+const { CURR_VERSION, IPFS_HASH, ADDRESS_ZERO, WAD } = require("../../helpers/constants");
 const { setupENSRegistrar } = require("../../helpers/upgradable-contracts");
 
 const { expect } = chai;
@@ -203,16 +203,6 @@ contract("Colony Network", (accounts) => {
 
       await checkErrorRevert(colonyNetwork.startNextCycle(), "colony-reputation-mining-clny-token-invalid-address");
     });
-
-    it('should not allow "punishStakers" to be called from an account that is not the mining cycle', async () => {
-      const chainId = await getChainId();
-      await metaColony.initialiseReputationMining(chainId, ethers.constants.HashZero, 0);
-
-      await checkErrorRevert(
-        colonyNetwork.punishStakers([accounts[0], accounts[1]], MIN_STAKE),
-        "colony-reputation-mining-sender-not-active-reputation-cycle",
-      );
-    });
   });
 
   describe("when creating new colonies at a specific version", () => {

test/contracts-network/metatx-token.js

@@ -289,6 +289,12 @@ contract("MetaTxToken", (accounts) => {
         const locked = await metaTxToken.locked();
         expect(locked).to.be.true;
       });
+
+      it("only owner can call setOwner", async () => {
+        await checkErrorRevert(metaTxToken.setOwner(USER1, { from: USER1 }), "ds-auth-unauthorized");
+        await metaTxToken.setOwner(USER1, { from: USER0 });
+        await metaTxToken.setOwner(USER0, { from: USER1 });
+      });
     });
 
     describe("when working with ether transfers", () => {

test/contracts-network/reputation-basic-functionality.js

@@ -6,7 +6,7 @@ const bnChai = require("bn-chai");
 const { ethers } = require("ethers");
 
 const { giveUserCLNYTokens, giveUserCLNYTokensAndStake } = require("../../helpers/test-data-generator");
-const { MIN_STAKE, MINING_CYCLE_DURATION, DECAY_RATE, CHALLENGE_RESPONSE_WINDOW_DURATION } = require("../../helpers/constants");
+const { MIN_STAKE, MINING_CYCLE_DURATION, DECAY_RATE, CHALLENGE_RESPONSE_WINDOW_DURATION, ADDRESS_ZERO } = require("../../helpers/constants");
 const { forwardTime, checkErrorRevert, getActiveRepCycle, advanceMiningCycleNoContest, getBlockTime } = require("../../helpers/test-helper");
 
 const { expect } = chai;
@@ -213,9 +213,23 @@ contract("Reputation mining - basic functionality", (accounts) => {
       await checkErrorRevert(repCycle.resetWindow(), "colony-reputation-mining-sender-not-network");
     });
 
-    it('should not allow "setReputationRootHash" to be called from an account that is not a ReputationMiningCycle', async () => {
+    it("should not allow reputation-mining functions to be called from an account that is not an active ReputationMiningCycle", async () => {
+      const inactiveRepCycleAddr = await colonyNetwork.getReputationMiningCycle(false);
+
+      await checkErrorRevert(
+        colonyNetwork.setReputationRootHash("0x000001", 10, [accounts[0], accounts[1]], { from: inactiveRepCycleAddr }),
+        "colony-reputation-mining-sender-not-active-reputation-cycle",
+      );
+      await checkErrorRevert(
+        colonyNetwork.punishStakers([accounts[0], accounts[1]], MIN_STAKE, { from: inactiveRepCycleAddr }),
+        "colony-reputation-mining-sender-not-active-reputation-cycle",
+      );
+      await checkErrorRevert(
+        colonyNetwork.reward(accounts[0], MIN_STAKE, { from: inactiveRepCycleAddr }),
+        "colony-reputation-mining-sender-not-active-reputation-cycle",
+      );
       await checkErrorRevert(
-        colonyNetwork.setReputationRootHash("0x000001", 10, [accounts[0], accounts[1]]),
+        colonyNetwork.burnUnneededRewards(0, { from: inactiveRepCycleAddr }),
         "colony-reputation-mining-sender-not-active-reputation-cycle",
       );
     });
@@ -258,6 +272,38 @@ contract("Reputation mining - basic functionality", (accounts) => {
 
       await checkErrorRevert(inactiveRepCycle.initialise(MINER1, MINER2), "colony-reputation-mining-cycle-already-initialised");
     });
+
+    it("functions that should only be after initialisation cannot be called before", async () => {
+      const before = await web3.eth.getStorageAt(colonyNetwork.address, 17);
+      const ethersProvider = new ethers.providers.Web3Provider(web3.currentProvider);
+      await ethersProvider.send("hardhat_setStorageAt", [
+        colonyNetwork.address,
+        ethers.utils.hexZeroPad(ethers.utils.hexlify(17), 32),
+        ethers.utils.hexZeroPad(ethers.utils.hexlify(0), 32),
+      ]);
+
+      await checkErrorRevert(colonyNetwork.startNextCycle(), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.setMiningDelegate(ADDRESS_ZERO, false), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.setReputationRootHash("0x00", 0, [ADDRESS_ZERO]), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.punishStakers([ADDRESS_ZERO], 0), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.reward(ADDRESS_ZERO, 0), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.claimMiningReward(ADDRESS_ZERO), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.unstakeForMining(0), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.burnUnneededRewards(0), "colony-reputation-mining-not-initialised");
+      await checkErrorRevert(colonyNetwork.setReputationMiningCycleReward(0), "colony-reputation-mining-not-initialised");
+
+      // Put in to recovery mode
+      await colonyNetwork.enterRecoveryMode();
+
+      await checkErrorRevert(
+        colonyNetwork.setReplacementReputationUpdateLogEntry(ADDRESS_ZERO, 0, ADDRESS_ZERO, 0, 0, ADDRESS_ZERO, 0, 0),
+        "colony-reputation-mining-not-initialised",
+      );
+      await colonyNetwork.approveExitRecovery();
+      await colonyNetwork.exitRecoveryMode();
+
+      await ethersProvider.send("hardhat_setStorageAt", [colonyNetwork.address, ethers.utils.hexZeroPad(ethers.utils.hexlify(17), 32), before]);
+    });
   });
 
   describe("when reading reputation mining constant properties", async () => {

test/cross-chain/cross-chain.js

@@ -315,6 +315,19 @@ contract("Cross-chain", (accounts) => {
       await checkErrorRevertEthers(tx.wait(), "colony-caller-must-be-meta-colony");
     });
 
+    it("callProxyNetwork can only be called through the metacolony", async () => {
+      const payload = homeColonyNetwork.interface.encodeFunctionData("setColonyBridgeAddress", [ADDRESS_ZERO]);
+      let tx = await homeColonyNetwork.createColonyForFrontend(ADDRESS_ZERO, "A", "A", 18, CURR_VERSION, "", "");
+      await tx.wait();
+
+      const colonyCount = await homeColonyNetwork.getColonyCount();
+      const colonyAddress = await homeColonyNetwork.getColony(colonyCount);
+      const fakeMetaColony = new ethers.Contract(colonyAddress, IMetaColony.abi, ethersHomeSigner);
+
+      tx = await fakeMetaColony.callProxyNetwork(foreignChainId, [payload], { gasLimit: 1000000 });
+      await checkErrorRevertEthers(tx.wait(), "colony-caller-must-be-meta-colony");
+    });
+
     it("callProxyNetwork can only be called by root permissions on the metacolony", async () => {
       const payload = remoteColonyNetwork.interface.encodeFunctionData("setColonyBridgeAddress", [ADDRESS_ZERO]);
       const homeMetacolony2 = new ethers.Contract(homeMetacolony.address, IMetaColony.abi, ethersHomeSigner2);
@@ -698,6 +711,50 @@ contract("Cross-chain", (accounts) => {
       expect(colonyBalance.toHexString()).to.equal(ethers.utils.parseEther("0.7").toHexString());
       expect(recipientBalanceAfter.sub(receipientBalanceBefore).toHexString()).to.equal(ethers.utils.parseEther("0.3").toHexString());
     });
+
+    it("a bookkeeping error will mean that tokens can no longer be claimed until tokens are returned", async () => {
+      const tokenAmount = ethers.utils.parseEther("100");
+
+      let tx = await foreignToken["mint(address,uint256)"](proxyColony.address, tokenAmount);
+      await tx.wait();
+
+      // Claim on the foreign chain
+      const p = bridgeMonitor.getPromiseForNextBridgedTransaction();
+      tx = await proxyColony.claimTokens(foreignToken.address);
+      await tx.wait();
+      await p;
+
+      // Now remove the tokens
+      const balanceSlot = ethers.utils.keccak256(
+        ethers.utils.concat([ethers.utils.hexZeroPad(proxyColony.address, 32), ethers.utils.hexZeroPad(1, 32)]),
+      );
+
+      await ethersForeignProvider.send("hardhat_setStorageAt", [
+        foreignToken.address,
+        balanceSlot,
+        ethers.utils.hexZeroPad(ethers.utils.parseEther("30").toHexString(), 32),
+      ]);
+
+      tx = await proxyColony.claimTokens(foreignToken.address, { gasLimit: 1000000 });
+      await checkErrorRevertEthers(tx.wait(), "colony-shell-token-bookkeeping-error");
+
+      // Now return the tokens
+      await ethersForeignProvider.send("hardhat_setStorageAt", [
+        foreignToken.address,
+        balanceSlot,
+        ethers.utils.hexZeroPad(ethers.utils.parseEther("100").toHexString(), 32),
+      ]);
+
+      // Mint some more tokens
+      tx = await foreignToken["mint(address,uint256)"](proxyColony.address, ethers.utils.parseEther("100"));
+      await tx.wait();
+
+      // Can now claim
+      const p2 = bridgeMonitor.getPromiseForNextBridgedTransaction();
+      tx = await proxyColony.claimTokens(foreignToken.address);
+      await tx.wait();
+      await p2;
+    });
   });
 
   describe("making arbitrary transactions on another chain", async () => {
@@ -741,6 +798,18 @@ contract("Cross-chain", (accounts) => {
       expect(balanceAfter.sub(balanceBefore).toHexString()).to.equal(ethers.utils.parseEther("100").toHexString());
     });
 
+    it("arbitrary transactions on the foreign chain must go to contracts", async () => {
+      const p = bridgeMonitor.getPromiseForNextBridgedTransaction();
+
+      const payload = foreignToken.interface.encodeFunctionData("mint(address,uint256)", [proxyColony.address, ethers.utils.parseEther("100")]);
+
+      const tx = await colony.makeProxyArbitraryTransactions(foreignChainId, [accounts[0]], [payload]);
+      await tx.wait();
+      await p;
+
+      await checkErrorRevertEthers(p, "require-execute-call-target-not-contract");
+    });
+
     it("can make multiple arbitrary transactions on the foreign chain in one go", async () => {
       const shellBalanceBefore = await foreignToken.balanceOf(proxyColony.address);
       const colonyBalanceBefore = await colony.getFundingPotProxyBalance(1, foreignChainId, foreignToken.address);
@@ -790,7 +859,7 @@ contract("Cross-chain", (accounts) => {
       const tx4 = await colony.makeProxyArbitraryTransactions(foreignChainId, [foreignToken.address], ["0x00000000"]);
       await tx4.wait();
 
-      await checkErrorRevertEthers(p, "colony-arbitrary-transaction-failed");
+      await checkErrorRevertEthers(p, "require-execute-call-reverted-with-no-error");
     });
   });
 

test/misc/chainid.js

@@ -99,13 +99,23 @@ contract("Contract Storage", (accounts) => {
     it("should handle tokens appropriately if auction is initialised for the CLNY token", async () => {
       await giveUserCLNYTokens(colonyNetwork, colonyNetwork.address, WAD);
       const supplyBefore = await clnyToken.totalSupply();
+      const balanceBefore = await clnyToken.balanceOf(colonyNetwork.address);
       const tx = await colonyNetwork.startTokenAuction(clnyToken.address);
 
       const supplyAfter = await clnyToken.totalSupply();
+      const balanceAfter = await clnyToken.balanceOf(colonyNetwork.address);
 
-      // tokens should be transferred to metacolony
-      expect(supplyBefore).to.eq.BN(supplyAfter);
-      await expectEvent(tx, "Transfer(address indexed,address indexed,uint256)", [colonyNetwork.address, metaColony.address, WAD]);
+      if (await isMainnet()) {
+        // tokens should be burned.
+        expect(supplyBefore.sub(supplyAfter)).to.eq.BN(balanceBefore);
+        await expectEvent(tx, "Burn(address indexed,uint256)", [colonyNetwork.address, WAD]);
+        expect(balanceAfter).to.be.zero;
+        expect(supplyBefore.sub(balanceBefore)).to.eq.BN(supplyAfter);
+      } else {
+        // tokens should be transferred to metacolony
+        expect(supplyBefore).to.eq.BN(supplyAfter);
+        await expectEvent(tx, "Transfer(address indexed,address indexed,uint256)", [colonyNetwork.address, metaColony.address, WAD]);
+      }
     });
 
     it("CLNY raised from auctions is dealt with appropriately", async () => {
@@ -125,6 +135,7 @@ contract("Contract Storage", (accounts) => {
       await clnyToken.approve(tokenAuction.address, clnyNeededForMaxPriceAuctionSellout, { from: accounts[1] });
       await tokenAuction.bid(clnyNeededForMaxPriceAuctionSellout, { from: accounts[1] });
 
+      const balanceBefore = await clnyToken.balanceOf(tokenAuction.address);
       const supplyBefore = await clnyToken.totalSupply();
       const receivedTotal = await tokenAuction.receivedTotal();
       expect(receivedTotal).to.not.be.zero;
@@ -133,9 +144,15 @@ contract("Contract Storage", (accounts) => {
       const balanceAfter = await clnyToken.balanceOf(tokenAuction.address);
       expect(balanceAfter).to.be.zero;
       const supplyAfter = await clnyToken.totalSupply();
-      // tokens should be transferred to metacolony
-      expect(supplyBefore).to.eq.BN(supplyAfter);
-      await expectEvent(tx, "Transfer(address indexed,address indexed,uint256)", [tokenAuction.address, metaColony.address, receivedTotal]);
+      if (await isMainnet()) {
+        // tokens should be burned.
+        expect(supplyBefore.sub(supplyAfter)).to.eq.BN(balanceBefore);
+        await expectEvent(tx, "Burn(address indexed,uint256)", [tokenAuction.address, receivedTotal]);
+      } else {
+        // tokens should be transferred to metacolony
+        expect(supplyBefore).to.eq.BN(supplyAfter);
+        await expectEvent(tx, "Transfer(address indexed,address indexed,uint256)", [tokenAuction.address, metaColony.address, receivedTotal]);
+      }
     });
   });
 });