Submission Details
Severity:
**[M-1] Remaining rewards in claimant cuts are not distributed correctly.**
**Description:** As per the documentation *"the remainder is distributed equally to those who claimed in time"*, this is not true. The rewards are distributed equally, however not the *remainder* of the rewards. This is because the denominator we use to determine the claimant cut is `i_players.length` and it should actually be `claimants.length`.
**Impact:** By using `i_players.length` we are reducing the remaining rewards that the claimants should actually be receiving. As the claimants are a subset of players, the remaining rewards should only be split amongst them by the number of claimants not players.
**Proof of Concept:**
The below test can be added to the TestMyCut.t.sol:TestMyCut contract’s test suite. It demonstrates the scenario whereby we have 2 players, each entitled to a reward of 50 WEI of wETH. A single user claims their cut ($50 , \text{wWEI}$), and then the contest is closed. Once the manager takes their cut $\left(\frac{50 , \text{wWEI}}{10} = 5 , \text{wWEI}\right)$, the remaining reward $\left(45 , \text{wWEI}\right)$ should be sent to the claimant. However, it is divided by 2, highlighting a loss in precision. This results in only $22 , \text{wWEI}$ being incorrectly sent to the claimant (with $1 , \text{wWEI}$ lost due to precision loss), when the claimant is actually entitled to $45 , \text{wWEI}$.
```solidity
event Transfer(address indexed from, address indexed to, uint256 value);
/**
* Remaining rewards are incorrectly distributed.
*/
function testRemainingRewardsAreIncorrectlyDistributed() public mintAndApproveTokens {
// Arrange - A single user claims the reward and we close the contest.
rewards = [50, 50];
totalRewards = 100;
ContestManager contestManager = ContestManager(conMan);
vm.startPrank(user);
contest = contestManager.createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
contestManager.fundContest(0);
vm.stopPrank();
Pot pot = Pot(contest);
vm.prank(player1);
pot.claimCut();
uint256 contestManagerInitialBalance = weth.balanceOf(user);
uint256 playerOneInitialBalance = weth.balanceOf(player1);
uint256 playerTwoInitialBalance = weth.balanceOf(player2);
// Act
vm.warp(91 days);
vm.prank(user);
vm.expectEmit(true, true, false, true);
emit Transfer(contest, player1, 22); // Highlighting a loss in precision
contestManager.closeContest(contest);
// Assert
uint256 contestManagerFinalBalance = weth.balanceOf(user);
uint256 playerOneFinalBalance = weth.balanceOf(player1);
uint256 playerTwoFinalBalance = weth.balanceOf(player2);
uint256 managersCut = 50 / 10;
uint256 playerOneExpectedFinalBalance = rewards[0] + (50 - managersCut);
assertFalse(playerOneFinalBalance == playerOneExpectedFinalBalance);
}
```
**Recommended Mitigation:**
```diff
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
// audit: q - magic number hard-coded.
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent; // audit: q - Division is throwing me off, SafeMath? This can go wrong leading to precision loss.
i_token.transfer(msg.sender, managerCut);
- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
+ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
```
Updates
Lead Judging Commences
equious about 1 year ago
Submission Judgement Published Assigned finding tags:
Incorrect distribution in closePot()
Rewards breakdown
nSLOC
106This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.