Extra tests, mostly around permissions

Author: area

contracts/common/DomainReceiverManagement.sol

@@ -37,8 +37,8 @@ abstract contract DomainReceiverManagement is MetaTransactionMsgSender, IsContra
   function checkDomainTokenReceiverDeployed(
     uint256 _domainId
   ) public returns (address domainTokenReceiverAddress) {
-    require(msgSenderIsColony(), "colony-domain-receiver-management-not-colony");
     require(!isStopped(), "colony-domain-receiver-management-stopped");
+    require(msgSenderIsColony(), "colony-domain-receiver-management-not-colony");
 
     // Calculate the address the domain should be receiving funds at
     domainTokenReceiverAddress = getDomainTokenReceiverAddress(msgSender(), _domainId);

test/contracts-network/colony-arbitrary-transactions.js

@@ -48,6 +48,19 @@ contract("Colony Arbitrary Transactions", (accounts) => {
     expect(balancePost.sub(balancePre)).to.eq.BN(WAD);
   });
 
+  it("makeSingleArbitraryTransaction can only be called by the colony on itself", async () => {
+    const action = await encodeTxData(token, "mint", [WAD]);
+    await checkErrorRevert(colony.makeSingleArbitraryTransaction(token.address, action, { from: USER0 }), "colony-not-self");
+  });
+
+  it("makeArbitraryTransactions must be called with the same number of addresses and actions", async () => {
+    const action = await encodeTxData(token, "mint", [WAD]);
+    await checkErrorRevert(
+      colony.makeArbitraryTransactions([token.address, token.address], [action], true),
+      "colony-targets-and-actions-length-mismatch",
+    );
+  });
+
   it("should be able to make multiple arbitrary transactions", async () => {
     const action = await encodeTxData(token, "mint", [WAD]);
     const action2 = await encodeTxData(token, "mint", [WAD.muln(2)]);

test/contracts-network/colony-expenditure.js

@@ -234,6 +234,10 @@ contract("Colony Expenditure", (accounts) => {
 
     it("should allow arbitrators to update the metadata", async () => {
       const setExpenditureMetadata = colony.methods["setExpenditureMetadata(uint256,uint256,uint256,string)"];
+
+      // Try with a bad proof
+      await checkErrorRevert(setExpenditureMetadata(1, 0, expenditureId, IPFS_HASH, { from: ARBITRATOR }), "ds-auth-invalid-domain-inheritance");
+
       const tx = await setExpenditureMetadata(1, UINT256_MAX, expenditureId, IPFS_HASH, { from: ARBITRATOR });
 
       await expectEvent(tx, "ExpenditureMetadataSet", [ARBITRATOR, expenditureId, IPFS_HASH]);
@@ -588,6 +592,15 @@ contract("Colony Expenditure", (accounts) => {
       await checkErrorRevert(colony.finalizeExpenditure(expenditureId, { from: ADMIN }), "colony-expenditure-not-draft-or-locked");
     });
 
+    it("should not allow expenditures to be finalized if they are not fully funded", async () => {
+      await colony.setExpenditurePayout(expenditureId, SLOT0, token.address, WAD, { from: ADMIN });
+      await checkErrorRevert(colony.finalizeExpenditure(expenditureId, { from: ADMIN }), "colony-expenditure-not-funded");
+      await checkErrorRevert(
+        colony.finalizeExpenditureViaArbitration(1, UINT256_MAX, expenditureId, { from: ARBITRATOR }),
+        "colony-expenditure-not-funded",
+      );
+    });
+
     it("should allow owners to finalize expenditures from locked state", async () => {
       await colony.lockExpenditure(expenditureId, { from: ADMIN });
 

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

@@ -197,6 +197,7 @@ contract("Colony Network Recovery", (accounts) => {
       await checkErrorRevert(colonyNetwork.bridgeMessage(1, "0x00000000"), "colony-in-recovery-mode");
       await checkErrorRevert(colonyNetwork.bridgeMessageToNetwork(1, "0x00000000"), "colony-in-recovery-mode");
       await checkErrorRevert(colonyNetwork.createProxyColony(1, HASHZERO), "colony-in-recovery-mode");
+      await checkErrorRevert(colonyNetwork.checkDomainTokenReceiverDeployed(1), "colony-domain-receiver-management-stopped");
 
       await colonyNetwork.approveExitRecovery();
       await colonyNetwork.exitRecoveryMode();

test/contracts-network/colony-recovery.js

@@ -197,20 +197,24 @@ contract("Colony Recovery", (accounts) => {
       await checkErrorRevert(metaColony.claimExpenditurePayout(0, 0, ADDRESS_ZERO), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.moveFundsBetweenPots(0, 0, 0, 0, 0, 0, 0, 0, ADDRESS_ZERO), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.moveFundsBetweenPots(0, 0, 0, 0, 0, 0, ADDRESS_ZERO), "colony-in-recovery-mode");
+      await checkErrorRevert(metaColony.moveFundsBetweenPots(0, 0, 0, 0, 0, 0, 0, 0, 0, ADDRESS_ZERO), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.claimColonyFunds(ADDRESS_ZERO), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.startNextRewardPayout(ADDRESS_ZERO, HASHZERO, HASHZERO, 0, []), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.claimRewardPayout(0, [0, 0, 0, 0, 0, 0, 0], HASHZERO, HASHZERO, 0, []), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.setRewardInverse(0), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.setExpenditurePayouts(0, [], ADDRESS_ZERO, []), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.setExpenditurePayout(0, 0, ADDRESS_ZERO, 0), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.setExpenditurePayout(1, UINT256_MAX, 0, 0, ADDRESS_ZERO, 0), "colony-in-recovery-mode");
+      await checkErrorRevert(metaColony.setExpenditureSkill(1, 1, 1), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.enterRecoveryMode(), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.burnTokens(ADDRESS_ZERO, 0), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.registerColonyLabel("", ""), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.updateColonyOrbitDB(""), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.makeArbitraryTransactions([], [], true), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.makeSingleArbitraryTransaction(ADDRESS_ZERO, HASHZERO), "colony-in-recovery-mode");
+      await checkErrorRevert(metaColony.makeProxyArbitraryTransactions(1, [], []), "colony-in-recovery-mode");
+      await checkErrorRevert(metaColony.callProxyNetwork(1, []), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.updateApprovalAmount(ADDRESS_ZERO, ADDRESS_ZERO), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.finalizeRewardPayout(1), "colony-in-recovery-mode");
     });

test/contracts-network/colony.js

@@ -13,6 +13,7 @@ const {
   expectAllEvents,
   expectEvent,
   upgradeColonyOnceThenToLatest,
+  bn2bytes32,
 } = require("../../helpers/test-helper");
 const {
   setupRandomColony,
@@ -216,6 +217,14 @@ contract("Colony", (accounts) => {
       await expectEvent(tx, "FundingPotAdded", [fundingPotCount]);
       await expectEvent(tx, "DomainMetadata", [accounts[0], domainCount, IPFS_HASH]);
     });
+
+    it("should require a valid permission proof", async () => {
+      await colony.addDomain(1, UINT256_MAX, 1);
+
+      // Remove permission
+      await colony.setUserRoles(1, UINT256_MAX, USER0, 1, bn2bytes32(0));
+      await checkErrorRevert(colony.addDomain(1, UINT256_MAX, 1), "ds-auth-unauthorized");
+    });
   });
 
   describe("when editing domains", () => {
@@ -229,6 +238,15 @@ contract("Colony", (accounts) => {
       await colony.addDomain(1, UINT256_MAX, 1);
       await expectNoEvent(colony.editDomain(1, 0, 2, ""), "DomainMetadata");
     });
+
+    it("should require a valid permission proof", async () => {
+      await colony.addDomain(1, UINT256_MAX, 1);
+      await colony.editDomain(1, 0, 2, IPFS_HASH);
+
+      // Remove permission
+      await colony.setUserRoles(1, UINT256_MAX, USER0, 1, bn2bytes32(0));
+      await checkErrorRevert(colony.editDomain(1, 0, 2, IPFS_HASH), "ds-auth-unauthorized");
+    });
   });
 
   describe.skip("when deprecating domains", () => {

test/cross-chain/cross-chain.js

@@ -927,6 +927,17 @@ contract("Cross-chain", (accounts) => {
       expect(balanceAfter.sub(balanceBefore).toHexString()).to.equal(ethers.utils.parseEther("100").toHexString());
     });
 
+    it("root permissions are required for makeProxyArbitraryTransactions", async () => {
+      const p = guardianSpy.getPromiseForNextBridgedTransaction();
+      let tx = await colony.makeProxyArbitraryTransactions(foreignChainId, [foreignToken.address], ["0x00000000"]);
+      await tx.wait();
+      await p;
+
+      await colony.setUserRoles(1, UINT256_MAX_ETHERS, accounts[0], 1, ethers.utils.hexZeroPad("0x00", 32));
+      tx = await colony.makeProxyArbitraryTransactions(foreignChainId, [foreignToken.address], ["0x00000000"], { gasLimit: 1000000 });
+      await checkErrorRevertEthers(tx.wait(), "ds-auth-unauthorized");
+    });
+
     it("arbitrary transactions on the foreign chain must go to contracts", async () => {
       const p = guardianSpy.getPromiseForNextBridgedTransaction();
 
@@ -1079,6 +1090,13 @@ contract("Cross-chain", (accounts) => {
     });
   });
 
+  describe("ColonyNetwork functions are secure", async () => {
+    it("a non-colony address cannot call bridgeMessage", async () => {
+      const tx = await homeColonyNetwork.bridgeMessage(1, HASHZERO, { gasLimit: 1000000 });
+      await checkErrorRevertEthers(tx.wait(), "colony-caller-must-be-colony");
+    });
+  });
+
   describe("Invalid interactions with bridging system are handled appropriately", async () => {
     it("Can't bridge to a chain that's not supported", async () => {
       const tx = await homeColony.makeProxyArbitraryTransactions(111, [ADDRESS_ZERO], ["0x00000000"], { gasLimit: 1000000 });