Payout scalar changes

JoinColony · May 26, 2023 · c3d2961 · c3d2961
1 parent 00c1665
commit c3d2961
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 32 deletions.
diff --git a/contracts/colony/ColonyExpenditure.sol b/contracts/colony/ColonyExpenditure.sol
@@ -214,6 +214,7 @@ contract ColonyExpenditure is ColonyStorage {
     require(_slots.length == _payoutModifiers.length, "colony-expenditure-bad-slots");
 
     for (uint256 i; i < _slots.length; i++) {
+      require(_payoutModifiers[i] <= 0, "colony-expenditure-bad-payout-modifier");
       expenditureSlots[_id][_slots[i]].payoutModifier = _payoutModifiers[i];
 
       emit ExpenditurePayoutModifierSet(msgSender(), _id, _slots[i], _payoutModifiers[i]);
@@ -315,15 +316,14 @@ contract ColonyExpenditure is ColonyStorage {
 
       // Validate payout modifier
       if (offset == 2) {
-        require(
-          int256(uint256(_value)) <= MAX_PAYOUT_MODIFIER &&
-          int256(uint256(_value)) >= MIN_PAYOUT_MODIFIER,
-          "colony-expenditure-bad-payout-modifier"
-        );
+        if (!ColonyAuthority(address(authority)).hasUserRole(msgSender(), 1, uint8(ColonyDataTypes.ColonyRole.Root))){
+          require(int256(uint256(_value)) <= 0, "colony-expenditure-bad-payout-modifier");
+        }
+        require(int256(uint256(_value)) >= MIN_PAYOUT_MODIFIER, "colony-expenditure-bad-payout-modifier");
       }
 
     } else {
-      require(false, "colony-expenditure-bad-slot");
+      revert("colony-expenditure-bad-slot");
     }
 
     executeStateChange(keccak256(abi.encode(_id, _storageSlot)), _mask, _keys, _value);

diff --git a/contracts/colony/ColonyFunding.sol b/contracts/colony/ColonyFunding.sol
@@ -209,7 +209,7 @@ contract ColonyFunding is ColonyStorage { // ignore-swc-123
     uint256 initialPayout = expenditureSlotPayouts[_id][_slot][_token];
     delete expenditureSlotPayouts[_id][_slot][_token];
 
-    int256 payoutModifier = imin(imax(slot.payoutModifier, MIN_PAYOUT_MODIFIER), MAX_PAYOUT_MODIFIER);
+    int256 payoutModifier = imax(slot.payoutModifier, MIN_PAYOUT_MODIFIER);
     uint256 payoutScalar = uint256(payoutModifier + int256(WAD));
 
     uint256 repPayout = wmul(initialPayout, payoutScalar);

diff --git a/contracts/colonyNetwork/ColonyNetworkENS.sol b/contracts/colonyNetwork/ColonyNetworkENS.sol
@@ -152,6 +152,6 @@ contract ColonyNetworkENS is ColonyNetworkStorage, MultiChain {
     } else if (isXdai()) {
       return "joincolony.colonyxdai";
     }
-    require(false, "colony-network-unsupported-network");
+    revert("colony-network-unsupported-network");
   }
 }
diff --git a/contracts/extensions/EvaluatedExpenditure.sol b/contracts/extensions/EvaluatedExpenditure.sol
@@ -82,7 +82,7 @@ contract EvaluatedExpenditure is ColonyExtension, BasicMetaTransaction {
   /// @param _childSkillIndex The index that the `_domainId` is relative to `_permissionDomainId`
   /// @param _id Expenditure identifier
   /// @param _slots Array of slots to set payout modifiers
-  /// @param _payoutModifiers Values (between +/- WAD) to modify the payout & reputation bonus
+  /// @param _payoutModifiers Values (between -WAD and 0) to modify the payout & reputation bonus
   function setExpenditurePayoutModifiers(
     uint256 _permissionDomainId,
     uint256 _childSkillIndex,

diff --git a/contracts/testHelpers/RequireExecuteCall.sol b/contracts/testHelpers/RequireExecuteCall.sol
@@ -33,9 +33,9 @@ contract RequireExecuteCall {
           // Slice the sighash.
           returndata := add(returndata, 0x04)
         }
-        require(false, abi.decode(returndata, (string))); // All that remains is the revert string
+        revert(abi.decode(returndata, (string))); // All that remains is the revert string
       }
-      require(false, "require-execute-call-reverted-with-no-error");
+      revert("require-execute-call-reverted-with-no-error");
     }
   }
 }
diff --git a/docs/interfaces/extensions/evaluatedexpenditure.md b/docs/interfaces/extensions/evaluatedexpenditure.md
@@ -82,7 +82,7 @@ Sets the payout modifiers in given expenditure slots, using the arbitration perm
 |_childSkillIndex|uint256|The index that the `_domainId` is relative to `_permissionDomainId`
 |_id|uint256|Expenditure identifier
 |_slots|uint256[]|Array of slots to set payout modifiers
-|_payoutModifiers|int256[]|Values (between +/- WAD) to modify the payout & reputation bonus
+|_payoutModifiers|int256[]|Values (between -WAD and 0) to modify the payout & reputation bonus
 
 
 ### ▸ `uninstall()`

diff --git a/helpers/test-helper.js b/helpers/test-helper.js
@@ -1113,7 +1113,7 @@ exports.getRewardClaimSquareRootsAndProofs = async function getRewardClaimSquare
 };
 
 exports.bn2bytes32 = function bn2bytes32(x, size = 64) {
-  return `0x${x.toString(16, size)}`;
+  return `0x${x.toTwos(size * 4).toString(16, size)}`;
 };
 
 exports.rolesToBytes32 = function rolesToBytes32(roles) {

diff --git a/test/contracts-network/colony-expenditure.js b/test/contracts-network/colony-expenditure.js
@@ -340,14 +340,21 @@ contract("Colony Expenditure", (accounts) => {
         "colony-expenditure-not-owner"
       );
 
-      await colony.setExpenditurePayoutModifiers(expenditureId, [SLOT1, SLOT2], [WAD.divn(2), WAD], { from: ADMIN });
+      await colony.setExpenditurePayoutModifiers(expenditureId, [SLOT1, SLOT2], [WAD.divn(-2), WAD.muln(-1)], { from: ADMIN });
 
       expenditureSlot = await colony.getExpenditureSlot(expenditureId, SLOT0);
       expect(expenditureSlot.payoutModifier).to.be.zero;
       expenditureSlot = await colony.getExpenditureSlot(expenditureId, SLOT1);
-      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.divn(2));
+      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.divn(-2));
       expenditureSlot = await colony.getExpenditureSlot(expenditureId, SLOT2);
-      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD);
+      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(-1));
+    });
+
+    it("should not allow owners to update many slot payout modifiers at once if one is >0", async () => {
+      await checkErrorRevert(
+        colony.setExpenditurePayoutModifiers(expenditureId, [SLOT1, SLOT2], [0, 1], { from: ADMIN }),
+        "colony-expenditure-bad-payout-modifier"
+      );
     });
 
     it("should not allow owners to update many slot payout modifiers with mismatched arguments", async () => {
@@ -463,7 +470,7 @@ contract("Colony Expenditure", (accounts) => {
         [SLOT0, SLOT1],
         [10, 20],
         [SLOT0, SLOT2],
-        [WAD.divn(3), WAD.divn(2)],
+        [WAD.divn(-3), WAD.divn(-2)],
         [token.address, otherToken.address],
         [
           [SLOT0, SLOT1],
@@ -481,7 +488,7 @@ contract("Colony Expenditure", (accounts) => {
       expect(slot.recipient).to.equal(RECIPIENT);
       expect(slot.skills[0]).to.be.zero;
       expect(slot.claimDelay).to.eq.BN(10);
-      expect(slot.payoutModifier).to.eq.BN(WAD.divn(3));
+      expect(slot.payoutModifier).to.eq.BN(WAD.divn(-3));
 
       slot = await colony.getExpenditureSlot(expenditureId, SLOT1);
       expect(slot.recipient).to.equal(USER);
@@ -493,7 +500,7 @@ contract("Colony Expenditure", (accounts) => {
       expect(slot.recipient).to.equal(ADMIN);
       expect(slot.skills[0]).to.eq.BN(GLOBAL_SKILL_ID);
       expect(slot.claimDelay).to.be.zero;
-      expect(slot.payoutModifier).to.eq.BN(WAD.divn(2));
+      expect(slot.payoutModifier).to.eq.BN(WAD.divn(-2));
 
       let payout;
       payout = await colony.getExpenditureSlotPayout(expenditureId, SLOT0, token.address);
@@ -522,7 +529,7 @@ contract("Colony Expenditure", (accounts) => {
           [SLOT0, SLOT1],
           [10, 20],
           [SLOT0, SLOT2],
-          [WAD.divn(3), WAD.divn(2)],
+          [WAD.divn(-3), WAD.divn(-2)],
           [token.address, otherToken.address],
           [[SLOT0], [SLOT1, SLOT2]],
           [
@@ -561,7 +568,7 @@ contract("Colony Expenditure", (accounts) => {
         [SLOT0, SLOT1],
         [10, 20],
         [SLOT0, SLOT2],
-        [WAD.divn(3), WAD.divn(2)],
+        [WAD.divn(-3), WAD.divn(-2)],
         [token.address, otherToken.address],
         [
           [SLOT0, SLOT1],
@@ -582,7 +589,7 @@ contract("Colony Expenditure", (accounts) => {
       expect(slot.recipient).to.equal(RECIPIENT);
       expect(slot.skills[0]).to.be.zero;
       expect(slot.claimDelay).to.eq.BN(10);
-      expect(slot.payoutModifier).to.eq.BN(WAD.divn(3));
+      expect(slot.payoutModifier).to.eq.BN(WAD.divn(-3));
 
       slot = await colony.getExpenditureSlot(expenditureId, SLOT1);
       expect(slot.recipient).to.equal(USER);
@@ -594,7 +601,7 @@ contract("Colony Expenditure", (accounts) => {
       expect(slot.recipient).to.equal(ADMIN);
       expect(slot.skills[0]).to.eq.BN(GLOBAL_SKILL_ID);
       expect(slot.claimDelay).to.be.zero;
-      expect(slot.payoutModifier).to.eq.BN(WAD.divn(2));
+      expect(slot.payoutModifier).to.eq.BN(WAD.divn(-2));
 
       let payout;
       payout = await colony.getExpenditureSlotPayout(expenditureId, SLOT0, token.address);
@@ -1274,12 +1281,48 @@ contract("Colony Expenditure", (accounts) => {
     it("should allow arbitration users to update expenditure slot payoutModifier", async () => {
       const mask = [MAPPING, ARRAY];
       const keys = ["0x0", bn2bytes32(new BN(2))];
-      const value = bn2bytes32(new BN(100));
+      const value = bn2bytes32(new BN(100).muln(-1));
 
       await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR });
 
       const expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
-      expect(expenditureSlot.payoutModifier).to.eq.BN(100);
+      expect(expenditureSlot.payoutModifier).to.eq.BN(new BN(100).muln(-1));
+    });
+
+    it("should allow not allow arbitration users to update expenditure slot payoutModifier to a value greater than 1", async () => {
+      const mask = [MAPPING, ARRAY];
+      const keys = ["0x0", bn2bytes32(new BN(2))];
+      const value = bn2bytes32(WAD.muln(2));
+
+      await checkErrorRevert(
+        colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR }),
+        "colony-expenditure-bad-payout-modifier"
+      );
+
+      const expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
+      expect(expenditureSlot.payoutModifier).to.eq.BN(0);
+    });
+
+    it("should allow root users to update expenditure slot payoutModifier to a value greater than 1", async () => {
+      const mask = [MAPPING, ARRAY];
+      const keys = ["0x0", bn2bytes32(new BN(2))];
+      const value = bn2bytes32(WAD.muln(2));
+
+      await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
+
+      const expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
+      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(2));
+    });
+
+    it("not even root users should be allowed to update expenditure slot payoutModifier to a value less than -1", async () => {
+      const mask = [MAPPING, ARRAY];
+      const keys = ["0x0", bn2bytes32(new BN(2))];
+      const value = bn2bytes32(WAD.muln(-2));
+
+      await checkErrorRevert(
+        colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT }),
+        "colony-expenditure-bad-payout-modifier"
+      );
     });
 
     it("should not allow arbitration users to pass an invalid payoutModifier", async () => {
@@ -1451,7 +1494,7 @@ contract("Colony Expenditure", (accounts) => {
       const mask = [MAPPING, ARRAY];
       const keys = ["0x0", bn2bytes32(new BN(2))];
       const value = bn2bytes32(WAD);
-      await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR });
+      await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
 
       const expenditure = await colony.getExpenditure(expenditureId);
       await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, domain1.fundingPotId, expenditure.fundingPotId, WAD, token.address);
@@ -1473,14 +1516,44 @@ contract("Colony Expenditure", (accounts) => {
       expect(entry.amount).to.eq.BN(WAD.muln(2));
     });
 
+    it("should scale up payout by payoutScalar for large payouts", async () => {
+      await colony.setExpenditureRecipient(expenditureId, SLOT0, RECIPIENT, { from: ADMIN });
+      await colony.setExpenditurePayout(expenditureId, SLOT0, token.address, WAD, { from: ADMIN });
+
+      // Modifier of 1 WAD translates to scalar of 2 WAD
+      const mask = [MAPPING, ARRAY];
+      const keys = ["0x0", bn2bytes32(new BN(2))];
+      const value = bn2bytes32(WAD.muln(100));
+      await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
+
+      const expenditure = await colony.getExpenditure(expenditureId);
+      await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, domain1.fundingPotId, expenditure.fundingPotId, WAD, token.address);
+      await colony.finalizeExpenditure(expenditureId, { from: ADMIN });
+
+      const recipientBalanceBefore = await token.balanceOf(RECIPIENT);
+      await colony.claimExpenditurePayout(expenditureId, SLOT0, token.address);
+
+      // Cash payout maxes out at payout
+      const recipientBalanceAfter = await token.balanceOf(RECIPIENT);
+      expect(recipientBalanceAfter.sub(recipientBalanceBefore)).to.eq.BN(WAD.divn(100).muln(99).subn(1)); // eslint-disable-line prettier/prettier
+
+      // But reputation gets a boost
+      const addr = await colonyNetwork.getReputationMiningCycle(false);
+      const repCycle = await IReputationMiningCycle.at(addr);
+      const numEntries = await repCycle.getReputationUpdateLogLength();
+      const entry = await repCycle.getReputationUpdateLogEntry(numEntries.subn(1));
+      expect(entry.user).to.equal(RECIPIENT);
+      expect(entry.amount).to.eq.BN(WAD.muln(101));
+    });
+
     it("should not overflow when using the maximum payout * modifier", async () => {
       await colony.setExpenditureRecipient(expenditureId, SLOT0, RECIPIENT, { from: ADMIN });
       await colony.setExpenditurePayout(expenditureId, SLOT0, token.address, MAX_PAYOUT, { from: ADMIN });
 
       const mask = [MAPPING, ARRAY];
       const keys = ["0x0", bn2bytes32(new BN(2))];
-      const value = bn2bytes32(WAD);
-      await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR });
+      const value = bn2bytes32(UINT256_MAX);
+      await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
 
       const expenditure = await colony.getExpenditure(expenditureId);
       await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, domain1.fundingPotId, expenditure.fundingPotId, MAX_PAYOUT, token.address);

diff --git a/test/extensions/evaluated-expenditure.js b/test/extensions/evaluated-expenditure.js
@@ -101,10 +101,10 @@ contract("EvaluatedExpenditure", (accounts) => {
       expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
       expect(expenditureSlot.payoutModifier).to.be.zero;
 
-      await evaluatedExpenditure.setExpenditurePayoutModifiers(1, UINT256_MAX, expenditureId, [0], [WAD], { from: USER0 });
+      await evaluatedExpenditure.setExpenditurePayoutModifiers(1, UINT256_MAX, expenditureId, [0], [WAD.muln(-1)], { from: USER0 });
 
       expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
-      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD);
+      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(-1));
     });
 
     it("cannot set the payout modifier with bad arguments", async () => {
@@ -123,7 +123,7 @@ contract("EvaluatedExpenditure", (accounts) => {
 
     it("can set the payout modifier via metatransaction", async () => {
       const txData = await evaluatedExpenditure.contract.methods
-        .setExpenditurePayoutModifiers(1, UINT256_MAX.toString(), expenditureId.toString(), [0], [WAD.toString()])
+        .setExpenditurePayoutModifiers(1, UINT256_MAX.toString(), expenditureId.toString(), [0], [WAD.muln(-1)])
         .encodeABI();
 
       const { r, s, v } = await getMetaTransactionParameters(txData, USER0, evaluatedExpenditure.address);
@@ -135,7 +135,7 @@ contract("EvaluatedExpenditure", (accounts) => {
       await evaluatedExpenditure.executeMetaTransaction(USER0, txData, r, s, v, { from: USER1 });
 
       expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
-      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD);
+      expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(-1));
     });
   });
 });