Merge pull request #771 from JoinColony/maint/better-proofs

UINT256_MAX in Domain Proofs

contracts/colony/Colony.sol

@@ -101,7 +101,7 @@ contract Colony is ColonyStorage, PatriciaTreeProofs {
   {
     return (
       hasUserRole(_user, _domainId, _role) &&
-      (_domainId == _childDomainId || validateDomainInheritance(_domainId, _childSkillIndex, _childDomainId))
+      validateDomainInheritance(_domainId, _childSkillIndex, _childDomainId)
     );
   }
 

contracts/colony/ColonyStorage.sol

@@ -195,9 +195,7 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
   // Note that these require messages currently cannot propogate up because of the `executeTaskRoleAssignment` logic
   modifier isAdmin(uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _id, address _user) {
     require(ColonyAuthority(address(authority)).hasUserRole(_user, _permissionDomainId, uint8(ColonyRole.Administration)), "colony-not-admin");
-    if (_permissionDomainId != tasks[_id].domainId) {
-      require(validateDomainInheritance(_permissionDomainId, _childSkillIndex, tasks[_id].domainId), "ds-auth-invalid-domain-inheritence");
-    }
+    require(validateDomainInheritance(_permissionDomainId, _childSkillIndex, tasks[_id].domainId), "ds-auth-invalid-domain-inheritence");
     _;
   }
 
@@ -215,9 +213,7 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
     require(domainExists(_permissionDomainId), "ds-auth-permission-domain-does-not-exist");
     require(domainExists(_childDomainId), "ds-auth-child-domain-does-not-exist");
     require(isAuthorized(msg.sender, _permissionDomainId, msg.sig), "ds-auth-unauthorized");
-    if (_permissionDomainId != _childDomainId) {
-      require(validateDomainInheritance(_permissionDomainId, _childSkillIndex, _childDomainId), "ds-auth-invalid-domain-inheritence");
-    }
+    require(validateDomainInheritance(_permissionDomainId, _childSkillIndex, _childDomainId), "ds-auth-invalid-domain-inheritence");
     _;
   }
 
@@ -230,8 +226,12 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
 
   // Evaluates a "domain proof" which checks that childDomainId is part of the subtree starting at permissionDomainId
   function validateDomainInheritance(uint256 permissionDomainId, uint256 childSkillIndex, uint256 childDomainId) internal view returns (bool) {
-    uint256 childSkillId = IColonyNetwork(colonyNetworkAddress).getChildSkillId(domains[permissionDomainId].skillId, childSkillIndex);
-    return childSkillId == domains[childDomainId].skillId;
+    if (permissionDomainId == childDomainId) {
+      return childSkillIndex == UINT256_MAX;
+    } else {
+      uint256 childSkillId = IColonyNetwork(colonyNetworkAddress).getChildSkillId(domains[permissionDomainId].skillId, childSkillIndex);
+      return childSkillId == domains[childDomainId].skillId;
+    }
   }
 
   // Checks to see if the permission comes ONLY from the Architecture role (i.e. user does not have root, etc.)

contracts/colonyNetwork/ColonyNetwork.sol

@@ -334,10 +334,10 @@ contract ColonyNetwork is ColonyNetworkStorage {
     IColony colony = IColony(_colonyAddress);
     colony.setRecoveryRole(msg.sender);
     colony.setRootRole(msg.sender, true);
-    colony.setArbitrationRole(1, 0, msg.sender, 1, true);
-    colony.setArchitectureRole(1, 0, msg.sender, 1, true);
-    colony.setFundingRole(1, 0, msg.sender, 1, true);
-    colony.setAdministrationRole(1, 0, msg.sender, 1, true);
+    colony.setArbitrationRole(1, UINT256_MAX, msg.sender, 1, true);
+    colony.setArchitectureRole(1, UINT256_MAX, msg.sender, 1, true);
+    colony.setFundingRole(1, UINT256_MAX, msg.sender, 1, true);
+    colony.setAdministrationRole(1, UINT256_MAX, msg.sender, 1, true);
 
     // Colony will not have owner
     DSAuth dsauth = DSAuth(_colonyAddress);

contracts/extensions/FundingQueue.sol

@@ -105,12 +105,12 @@ contract FundingQueue is DSMath, PatriciaTreeProofs {
     uint256 toSkillId = colony.getDomain(toDomain).skillId;
 
     require(
-      domainSkillId == fromSkillId ||
+      (domainSkillId == fromSkillId && _fromChildSkillIndex == UINT256_MAX) ||
       fromSkillId == colonyNetwork.getChildSkillId(domainSkillId, _fromChildSkillIndex),
       "funding-queue-bad-inheritence-from"
     );
     require(
-      domainSkillId == toSkillId ||
+      (domainSkillId == toSkillId && _toChildSkillIndex == UINT256_MAX) ||
       toSkillId == colonyNetwork.getChildSkillId(domainSkillId, _toChildSkillIndex),
       "funding-queue-bad-inheritence-to"
     );
@@ -269,8 +269,8 @@ contract FundingQueue is DSMath, PatriciaTreeProofs {
 
     colony.moveFundsBetweenPots(
       proposal.domainId,
-      proposal.toChildSkillIndex,
       proposal.fromChildSkillIndex,
+      proposal.toChildSkillIndex,
       proposal.fromPot,
       proposal.toPot,
       actualFundingToTransfer,

contracts/extensions/OneTxPayment.sol

@@ -27,6 +27,8 @@ import "./../colonyNetwork/IColonyNetwork.sol";
 
 
 contract OneTxPayment {
+
+  uint256 constant UINT256_MAX = 2**256 - 1;
   bytes4 constant ADD_PAYMENT_SIG = bytes4(keccak256("addPayment(uint256,uint256,address,address,uint256,uint256,uint256)"));
   bytes4 constant MOVE_FUNDS_SIG = bytes4(keccak256("moveFundsBetweenPots(uint256,uint256,uint256,uint256,uint256,uint256,address)"));
 
@@ -113,7 +115,7 @@ contract OneTxPayment {
       // Fund the payment
       colony.moveFundsBetweenPots(
         1, // Root domain always 1
-        0, // Not used, this extension must have funding permission in the root for this function to work
+        UINT256_MAX, // Not used, this extension must have funding permission in the root for this function to work
         _childSkillIndex,
         1, // Root domain funding pot is always 1
         paymentData[2],

helpers/test-data-generator.js

@@ -4,6 +4,7 @@ import BN from "bn.js";
 import { ethers } from "ethers";
 
 import {
+  UINT256_MAX,
   MANAGER_PAYOUT,
   EVALUATOR_PAYOUT,
   WORKER_PAYOUT,
@@ -152,8 +153,8 @@ export async function setupFundedTask({
   const totalPayouts = managerPayoutBN.add(workerPayoutBN).add(evaluatorPayoutBN);
 
   const childSkillIndex = await getChildSkillIndex(colonyNetwork, colony, 1, task.domainId);
-  await colony.setFundingRole(1, 0, manager, 1, true);
-  await colony.moveFundsBetweenPots(1, 0, childSkillIndex, 1, task.fundingPotId, totalPayouts, tokenAddress, { from: manager });
+  await colony.setFundingRole(1, UINT256_MAX, manager, 1, true);
+  await colony.moveFundsBetweenPots(1, UINT256_MAX, childSkillIndex, 1, task.fundingPotId, totalPayouts, tokenAddress, { from: manager });
   await colony.setAllTaskPayouts(taskId, tokenAddress, managerPayout, evaluatorPayout, workerPayout, { from: manager });
   await assignRoles({ colony, taskId, manager, evaluator, worker });
 

helpers/test-helper.js

@@ -704,7 +704,7 @@ export async function getChildSkillIndex(colonyNetwork, colony, _parentDomainId,
   const childDomainId = new BN(_childDomainId);
 
   if (parentDomainId.eq(childDomainId)) {
-    return 0;
+    return UINT256_MAX;
   }
 
   const parentDomain = await colony.getDomain(parentDomainId);

test-gas-costs/gasCosts.js

@@ -4,6 +4,7 @@ import path from "path";
 import { TruffleLoader } from "@colony/colony-js-contract-loader-fs";
 
 import {
+  UINT256_MAX,
   WAD,
   MANAGER_ROLE,
   WORKER_ROLE,
@@ -107,7 +108,7 @@ contract("All", function (accounts) {
     it("when working with a Colony", async function () {
       await colony.mintTokens(200);
       await colony.claimColonyFunds(token.address);
-      await colony.setAdministrationRole(1, 0, EVALUATOR, 1, true);
+      await colony.setAdministrationRole(1, UINT256_MAX, EVALUATOR, 1, true);
     });
 
     it("when working with a Task", async function () {
@@ -147,7 +148,7 @@ contract("All", function (accounts) {
       });
 
       // moveFundsBetweenPots
-      await colony.moveFundsBetweenPots(1, 0, 0, 1, 2, 190, token.address);
+      await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, 1, 2, 190, token.address);
 
       // setTaskManagerPayout
       await executeSignedTaskChange({
@@ -205,20 +206,20 @@ contract("All", function (accounts) {
 
     it("when working with a Payment", async function () {
       // 4 transactions payment
-      await colony.addPayment(1, 0, WORKER, token.address, WAD, 1, 0);
+      await colony.addPayment(1, UINT256_MAX, WORKER, token.address, WAD, 1, 0);
       const paymentId = await colony.getPaymentCount();
       const payment = await colony.getPayment(paymentId);
 
-      await colony.moveFundsBetweenPots(1, 0, 0, 1, payment.fundingPotId, WAD.add(WAD.divn(10)), token.address);
-      await colony.finalizePayment(1, 0, paymentId);
+      await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, 1, payment.fundingPotId, WAD.add(WAD.divn(10)), token.address);
+      await colony.finalizePayment(1, UINT256_MAX, paymentId);
       await colony.claimPayment(paymentId, token.address);
 
       // 1 transaction payment
       const oneTxExtension = await OneTxPayment.new(colony.address);
-      await colony.setAdministrationRole(1, 0, oneTxExtension.address, 1, true);
-      await colony.setFundingRole(1, 0, oneTxExtension.address, 1, true);
+      await colony.setAdministrationRole(1, UINT256_MAX, oneTxExtension.address, 1, true);
+      await colony.setFundingRole(1, UINT256_MAX, oneTxExtension.address, 1, true);
 
-      await oneTxExtension.makePayment(1, 0, 1, 0, [WORKER], [token.address], [10], 1, GLOBAL_SKILL_ID);
+      await oneTxExtension.makePayment(1, UINT256_MAX, 1, UINT256_MAX, [WORKER], [token.address], [10], 1, GLOBAL_SKILL_ID);
     });
 
     it("when working with staking", async function () {
@@ -313,7 +314,7 @@ contract("All", function (accounts) {
 
       await fundColonyWithTokens(newColony, otherToken, 300);
 
-      await newColony.moveFundsBetweenPots(1, 0, 0, 1, 0, 100, otherToken.address);
+      await newColony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, 1, 0, 100, otherToken.address);
 
       const tx = await newColony.startNextRewardPayout(otherToken.address, ...colonyWideReputationProof);
       const payoutId = tx.logs[0].args.rewardPayoutId;
@@ -347,7 +348,7 @@ contract("All", function (accounts) {
       await forwardTime(5184001);
       await newColony.finalizeRewardPayout(payoutId);
 
-      await newColony.moveFundsBetweenPots(1, 0, 0, 1, 0, 100, otherToken.address);
+      await newColony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, 1, 0, 100, otherToken.address);
 
       const tx2 = await newColony.startNextRewardPayout(otherToken.address, ...colonyWideReputationProof);
       const payoutId2 = tx2.logs[0].args.rewardPayoutId;