Skip to content

Commit 3e04965

Browse files
areaarea
committed
Payout scalar changes
1 parent 19e82c5 commit 3e04965

9 files changed

Lines changed: 105 additions & 32 deletions

File tree

‎contracts/colony/ColonyExpenditure.sol‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ contract ColonyExpenditure is ColonyStorage {
214214
require(_slots.length == _payoutModifiers.length, "colony-expenditure-bad-slots");
215215

216216
for (uint256 i; i < _slots.length; i++) {
217+
require(_payoutModifiers[i] <= 0, "colony-expenditure-bad-payout-modifier");
217218
expenditureSlots[_id][_slots[i]].payoutModifier = _payoutModifiers[i];
218219

219220
emit ExpenditurePayoutModifierSet(msgSender(), _id, _slots[i], _payoutModifiers[i]);
@@ -315,15 +316,14 @@ contract ColonyExpenditure is ColonyStorage {
315316

316317
// Validate payout modifier
317318
if (offset == 2) {
318-
require(
319-
int256(uint256(_value)) <= MAX_PAYOUT_MODIFIER &&
320-
int256(uint256(_value)) >= MIN_PAYOUT_MODIFIER,
321-
"colony-expenditure-bad-payout-modifier"
322-
);
319+
if (!ColonyAuthority(address(authority)).hasUserRole(msgSender(), 1, uint8(ColonyDataTypes.ColonyRole.Root))){
320+
require(int256(uint256(_value)) <= 0, "colony-expenditure-bad-payout-modifier");
321+
}
322+
require(int256(uint256(_value)) >= MIN_PAYOUT_MODIFIER, "colony-expenditure-bad-payout-modifier");
323323
}
324324

325325
} else {
326-
require(false, "colony-expenditure-bad-slot");
326+
revert("colony-expenditure-bad-slot");
327327
}
328328

329329
executeStateChange(keccak256(abi.encode(_id, _storageSlot)), _mask, _keys, _value);

‎contracts/colony/ColonyFunding.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ contract ColonyFunding is ColonyStorage { // ignore-swc-123
209209
uint256 initialPayout = expenditureSlotPayouts[_id][_slot][_token];
210210
delete expenditureSlotPayouts[_id][_slot][_token];
211211

212-
int256 payoutModifier = imin(imax(slot.payoutModifier, MIN_PAYOUT_MODIFIER), MAX_PAYOUT_MODIFIER);
212+
int256 payoutModifier = imax(slot.payoutModifier, MIN_PAYOUT_MODIFIER);
213213
uint256 payoutScalar = uint256(payoutModifier + int256(WAD));
214214

215215
uint256 repPayout = wmul(initialPayout, payoutScalar);

‎contracts/colonyNetwork/ColonyNetworkENS.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,6 @@ contract ColonyNetworkENS is ColonyNetworkStorage, MultiChain {
152152
} else if (isXdai()) {
153153
return "joincolony.colonyxdai";
154154
}
155-
require(false, "colony-network-unsupported-network");
155+
revert("colony-network-unsupported-network");
156156
}
157157
}

‎contracts/extensions/EvaluatedExpenditure.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ contract EvaluatedExpenditure is ColonyExtension, BasicMetaTransaction {
8282
/// @param _childSkillIndex The index that the `_domainId` is relative to `_permissionDomainId`
8383
/// @param _id Expenditure identifier
8484
/// @param _slots Array of slots to set payout modifiers
85-
/// @param _payoutModifiers Values (between +/- WAD) to modify the payout & reputation bonus
85+
/// @param _payoutModifiers Values (between -WAD and 0) to modify the payout & reputation bonus
8686
function setExpenditurePayoutModifiers(
8787
uint256 _permissionDomainId,
8888
uint256 _childSkillIndex,

‎contracts/testHelpers/RequireExecuteCall.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ contract RequireExecuteCall {
3333
// Slice the sighash.
3434
returndata := add(returndata, 0x04)
3535
}
36-
require(false, abi.decode(returndata, (string))); // All that remains is the revert string
36+
revert(abi.decode(returndata, (string))); // All that remains is the revert string
3737
}
38-
require(false, "require-execute-call-reverted-with-no-error");
38+
revert("require-execute-call-reverted-with-no-error");
3939
}
4040
}
4141
}

‎docs/interfaces/extensions/evaluatedexpenditure.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ Sets the payout modifiers in given expenditure slots, using the arbitration perm
8282
|_childSkillIndex|uint256|The index that the `_domainId` is relative to `_permissionDomainId`
8383
|_id|uint256|Expenditure identifier
8484
|_slots|uint256[]|Array of slots to set payout modifiers
85-
|_payoutModifiers|int256[]|Values (between +/- WAD) to modify the payout & reputation bonus
85+
|_payoutModifiers|int256[]|Values (between -WAD and 0) to modify the payout & reputation bonus
8686

8787

8888
### ▸ `uninstall()`

‎helpers/test-helper.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1113,7 +1113,7 @@ exports.getRewardClaimSquareRootsAndProofs = async function getRewardClaimSquare
11131113
};
11141114

11151115
exports.bn2bytes32 = function bn2bytes32(x, size = 64) {
1116-
return `0x${x.toString(16, size)}`;
1116+
return `0x${x.toTwos(size * 4).toString(16, size)}`;
11171117
};
11181118

11191119
exports.rolesToBytes32 = function rolesToBytes32(roles) {

‎test/contracts-network/colony-expenditure.js‎

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -340,14 +340,21 @@ contract("Colony Expenditure", (accounts) => {
340340
"colony-expenditure-not-owner"
341341
);
342342

343-
await colony.setExpenditurePayoutModifiers(expenditureId, [SLOT1, SLOT2], [WAD.divn(2), WAD], { from: ADMIN });
343+
await colony.setExpenditurePayoutModifiers(expenditureId, [SLOT1, SLOT2], [WAD.divn(-2), WAD.muln(-1)], { from: ADMIN });
344344

345345
expenditureSlot = await colony.getExpenditureSlot(expenditureId, SLOT0);
346346
expect(expenditureSlot.payoutModifier).to.be.zero;
347347
expenditureSlot = await colony.getExpenditureSlot(expenditureId, SLOT1);
348-
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.divn(2));
348+
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.divn(-2));
349349
expenditureSlot = await colony.getExpenditureSlot(expenditureId, SLOT2);
350-
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD);
350+
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(-1));
351+
});
352+
353+
it("should not allow owners to update many slot payout modifiers at once if one is >0", async () => {
354+
await checkErrorRevert(
355+
colony.setExpenditurePayoutModifiers(expenditureId, [SLOT1, SLOT2], [0, 1], { from: ADMIN }),
356+
"colony-expenditure-bad-payout-modifier"
357+
);
351358
});
352359

353360
it("should not allow owners to update many slot payout modifiers with mismatched arguments", async () => {
@@ -463,7 +470,7 @@ contract("Colony Expenditure", (accounts) => {
463470
[SLOT0, SLOT1],
464471
[10, 20],
465472
[SLOT0, SLOT2],
466-
[WAD.divn(3), WAD.divn(2)],
473+
[WAD.divn(-3), WAD.divn(-2)],
467474
[token.address, otherToken.address],
468475
[
469476
[SLOT0, SLOT1],
@@ -481,7 +488,7 @@ contract("Colony Expenditure", (accounts) => {
481488
expect(slot.recipient).to.equal(RECIPIENT);
482489
expect(slot.skills[0]).to.be.zero;
483490
expect(slot.claimDelay).to.eq.BN(10);
484-
expect(slot.payoutModifier).to.eq.BN(WAD.divn(3));
491+
expect(slot.payoutModifier).to.eq.BN(WAD.divn(-3));
485492

486493
slot = await colony.getExpenditureSlot(expenditureId, SLOT1);
487494
expect(slot.recipient).to.equal(USER);
@@ -493,7 +500,7 @@ contract("Colony Expenditure", (accounts) => {
493500
expect(slot.recipient).to.equal(ADMIN);
494501
expect(slot.skills[0]).to.eq.BN(GLOBAL_SKILL_ID);
495502
expect(slot.claimDelay).to.be.zero;
496-
expect(slot.payoutModifier).to.eq.BN(WAD.divn(2));
503+
expect(slot.payoutModifier).to.eq.BN(WAD.divn(-2));
497504

498505
let payout;
499506
payout = await colony.getExpenditureSlotPayout(expenditureId, SLOT0, token.address);
@@ -522,7 +529,7 @@ contract("Colony Expenditure", (accounts) => {
522529
[SLOT0, SLOT1],
523530
[10, 20],
524531
[SLOT0, SLOT2],
525-
[WAD.divn(3), WAD.divn(2)],
532+
[WAD.divn(-3), WAD.divn(-2)],
526533
[token.address, otherToken.address],
527534
[[SLOT0], [SLOT1, SLOT2]],
528535
[
@@ -561,7 +568,7 @@ contract("Colony Expenditure", (accounts) => {
561568
[SLOT0, SLOT1],
562569
[10, 20],
563570
[SLOT0, SLOT2],
564-
[WAD.divn(3), WAD.divn(2)],
571+
[WAD.divn(-3), WAD.divn(-2)],
565572
[token.address, otherToken.address],
566573
[
567574
[SLOT0, SLOT1],
@@ -582,7 +589,7 @@ contract("Colony Expenditure", (accounts) => {
582589
expect(slot.recipient).to.equal(RECIPIENT);
583590
expect(slot.skills[0]).to.be.zero;
584591
expect(slot.claimDelay).to.eq.BN(10);
585-
expect(slot.payoutModifier).to.eq.BN(WAD.divn(3));
592+
expect(slot.payoutModifier).to.eq.BN(WAD.divn(-3));
586593

587594
slot = await colony.getExpenditureSlot(expenditureId, SLOT1);
588595
expect(slot.recipient).to.equal(USER);
@@ -594,7 +601,7 @@ contract("Colony Expenditure", (accounts) => {
594601
expect(slot.recipient).to.equal(ADMIN);
595602
expect(slot.skills[0]).to.eq.BN(GLOBAL_SKILL_ID);
596603
expect(slot.claimDelay).to.be.zero;
597-
expect(slot.payoutModifier).to.eq.BN(WAD.divn(2));
604+
expect(slot.payoutModifier).to.eq.BN(WAD.divn(-2));
598605

599606
let payout;
600607
payout = await colony.getExpenditureSlotPayout(expenditureId, SLOT0, token.address);
@@ -1274,12 +1281,48 @@ contract("Colony Expenditure", (accounts) => {
12741281
it("should allow arbitration users to update expenditure slot payoutModifier", async () => {
12751282
const mask = [MAPPING, ARRAY];
12761283
const keys = ["0x0", bn2bytes32(new BN(2))];
1277-
const value = bn2bytes32(new BN(100));
1284+
const value = bn2bytes32(new BN(100).muln(-1));
12781285

12791286
await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR });
12801287

12811288
const expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
1282-
expect(expenditureSlot.payoutModifier).to.eq.BN(100);
1289+
expect(expenditureSlot.payoutModifier).to.eq.BN(new BN(100).muln(-1));
1290+
});
1291+
1292+
it("should allow not allow arbitration users to update expenditure slot payoutModifier to a value greater than 1", async () => {
1293+
const mask = [MAPPING, ARRAY];
1294+
const keys = ["0x0", bn2bytes32(new BN(2))];
1295+
const value = bn2bytes32(WAD.muln(2));
1296+
1297+
await checkErrorRevert(
1298+
colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR }),
1299+
"colony-expenditure-bad-payout-modifier"
1300+
);
1301+
1302+
const expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
1303+
expect(expenditureSlot.payoutModifier).to.eq.BN(0);
1304+
});
1305+
1306+
it("should allow root users to update expenditure slot payoutModifier to a value greater than 1", async () => {
1307+
const mask = [MAPPING, ARRAY];
1308+
const keys = ["0x0", bn2bytes32(new BN(2))];
1309+
const value = bn2bytes32(WAD.muln(2));
1310+
1311+
await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
1312+
1313+
const expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
1314+
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(2));
1315+
});
1316+
1317+
it("not even root users should be allowed to update expenditure slot payoutModifier to a value less than -1", async () => {
1318+
const mask = [MAPPING, ARRAY];
1319+
const keys = ["0x0", bn2bytes32(new BN(2))];
1320+
const value = bn2bytes32(WAD.muln(-2));
1321+
1322+
await checkErrorRevert(
1323+
colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT }),
1324+
"colony-expenditure-bad-payout-modifier"
1325+
);
12831326
});
12841327

12851328
it("should not allow arbitration users to pass an invalid payoutModifier", async () => {
@@ -1451,7 +1494,7 @@ contract("Colony Expenditure", (accounts) => {
14511494
const mask = [MAPPING, ARRAY];
14521495
const keys = ["0x0", bn2bytes32(new BN(2))];
14531496
const value = bn2bytes32(WAD);
1454-
await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR });
1497+
await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
14551498

14561499
const expenditure = await colony.getExpenditure(expenditureId);
14571500
await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, domain1.fundingPotId, expenditure.fundingPotId, WAD, token.address);
@@ -1473,14 +1516,44 @@ contract("Colony Expenditure", (accounts) => {
14731516
expect(entry.amount).to.eq.BN(WAD.muln(2));
14741517
});
14751518

1519+
it("should scale up payout by payoutScalar for large payouts", async () => {
1520+
await colony.setExpenditureRecipient(expenditureId, SLOT0, RECIPIENT, { from: ADMIN });
1521+
await colony.setExpenditurePayout(expenditureId, SLOT0, token.address, WAD, { from: ADMIN });
1522+
1523+
// Modifier of 1 WAD translates to scalar of 2 WAD
1524+
const mask = [MAPPING, ARRAY];
1525+
const keys = ["0x0", bn2bytes32(new BN(2))];
1526+
const value = bn2bytes32(WAD.muln(100));
1527+
await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
1528+
1529+
const expenditure = await colony.getExpenditure(expenditureId);
1530+
await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, domain1.fundingPotId, expenditure.fundingPotId, WAD, token.address);
1531+
await colony.finalizeExpenditure(expenditureId, { from: ADMIN });
1532+
1533+
const recipientBalanceBefore = await token.balanceOf(RECIPIENT);
1534+
await colony.claimExpenditurePayout(expenditureId, SLOT0, token.address);
1535+
1536+
// Cash payout maxes out at payout
1537+
const recipientBalanceAfter = await token.balanceOf(RECIPIENT);
1538+
expect(recipientBalanceAfter.sub(recipientBalanceBefore)).to.eq.BN(WAD.divn(100).muln(99).subn(1)); // eslint-disable-line prettier/prettier
1539+
1540+
// But reputation gets a boost
1541+
const addr = await colonyNetwork.getReputationMiningCycle(false);
1542+
const repCycle = await IReputationMiningCycle.at(addr);
1543+
const numEntries = await repCycle.getReputationUpdateLogLength();
1544+
const entry = await repCycle.getReputationUpdateLogEntry(numEntries.subn(1));
1545+
expect(entry.user).to.equal(RECIPIENT);
1546+
expect(entry.amount).to.eq.BN(WAD.muln(101));
1547+
});
1548+
14761549
it("should not overflow when using the maximum payout * modifier", async () => {
14771550
await colony.setExpenditureRecipient(expenditureId, SLOT0, RECIPIENT, { from: ADMIN });
14781551
await colony.setExpenditurePayout(expenditureId, SLOT0, token.address, MAX_PAYOUT, { from: ADMIN });
14791552

14801553
const mask = [MAPPING, ARRAY];
14811554
const keys = ["0x0", bn2bytes32(new BN(2))];
1482-
const value = bn2bytes32(WAD);
1483-
await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ARBITRATOR });
1555+
const value = bn2bytes32(UINT256_MAX);
1556+
await colony.setExpenditureState(1, UINT256_MAX, expenditureId, EXPENDITURESLOTS_SLOT, mask, keys, value, { from: ROOT });
14841557

14851558
const expenditure = await colony.getExpenditure(expenditureId);
14861559
await colony.moveFundsBetweenPots(1, UINT256_MAX, UINT256_MAX, domain1.fundingPotId, expenditure.fundingPotId, MAX_PAYOUT, token.address);

‎test/extensions/evaluated-expenditure.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ contract("EvaluatedExpenditure", (accounts) => {
101101
expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
102102
expect(expenditureSlot.payoutModifier).to.be.zero;
103103

104-
await evaluatedExpenditure.setExpenditurePayoutModifiers(1, UINT256_MAX, expenditureId, [0], [WAD], { from: USER0 });
104+
await evaluatedExpenditure.setExpenditurePayoutModifiers(1, UINT256_MAX, expenditureId, [0], [WAD.muln(-1)], { from: USER0 });
105105

106106
expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
107-
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD);
107+
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(-1));
108108
});
109109

110110
it("cannot set the payout modifier with bad arguments", async () => {
@@ -123,7 +123,7 @@ contract("EvaluatedExpenditure", (accounts) => {
123123

124124
it("can set the payout modifier via metatransaction", async () => {
125125
const txData = await evaluatedExpenditure.contract.methods
126-
.setExpenditurePayoutModifiers(1, UINT256_MAX.toString(), expenditureId.toString(), [0], [WAD.toString()])
126+
.setExpenditurePayoutModifiers(1, UINT256_MAX.toString(), expenditureId.toString(), [0], [WAD.muln(-1)])
127127
.encodeABI();
128128

129129
const { r, s, v } = await getMetaTransactionParameters(txData, USER0, evaluatedExpenditure.address);
@@ -135,7 +135,7 @@ contract("EvaluatedExpenditure", (accounts) => {
135135
await evaluatedExpenditure.executeMetaTransaction(USER0, txData, r, s, v, { from: USER1 });
136136

137137
expenditureSlot = await colony.getExpenditureSlot(expenditureId, 0);
138-
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD);
138+
expect(expenditureSlot.payoutModifier).to.eq.BN(WAD.muln(-1));
139139
});
140140
});
141141
});

0 commit comments

Comments
 (0)