Fix denominator constraint #256
Conversation
test/colony-funding.js
Outdated
| "colony-reward-payout-invalid-parametar-numerator", | ||
| "colony-reward-payout-invalid-parametar-denominator" | ||
| "colony-reward-payout-invalid-parametar-denominator", |
| @@ -313,7 +313,7 @@ contract("All", accounts => { | |||
| }); | |||
|
|
|||
| it("when working with reward payouts", async () => { | |||
| const totalReputation = toBN(350 * 1e18); | |||
| const totalReputation = toBN(300 * 1e18); | |||
There was a problem hiding this comment.
You've changed this to be accurate, but I don't think this variable is used anywhere in the test?
There was a problem hiding this comment.
We are passing it to claimRewardPayout
helpers/test-helper.js
Outdated
| @@ -253,5 +253,9 @@ export function bnSqrt(bn) { | |||
| .add(a) | |||
| .div(web3Utils.toBN(2)); | |||
| } | |||
| // Do we need while loop? Not sure how precise the formula above is | |||
There was a problem hiding this comment.
I don't believe the loop is needed (though obviously the addition is). I believe you could also replace the addition with b = b.addn(1).
test/colony-funding.js
Outdated
| const blockTimestamp = await currentBlockTime(); | ||
|
|
||
| const info = await colony.getRewardPayoutInfo(payoutId); | ||
| assert.equal(info[1].toString(), userTokens.toString()); |
There was a problem hiding this comment.
Is there a reason we're not checking info[0]? We could compare it to colonyNetwork.getReputationRootHash(), right? (Although without going through all the mining setup, I suppose that's going to just be 0x0, which might be the reason you've skipped it?)
There was a problem hiding this comment.
Yea, i thought i can add it when we add proofs (which will require mining process), but i guess we can do it now
Fixed a bug where user was able to pass denominator smaller than the correct value, therefore maximizing his reward and draining reward pot.