Impact: High
Likelihood: High
Root + Impact
Description
-
It is stated in a protocol documentation that when a contest is closed and a manager takes his cut, 'the remainder is distributed equally to those who claimed in time!' but it is not distributed fairly.
-
The remainder is divided by an amount of all players, not claimant, and this means that an eligible claimant will receive less than he should.
-
The pot balance holds climantCuts for players that didn't claim.
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
@> uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
Risk
Likelihood:
Impact:
Proof of Concept
Please, add the following test to TestMyCut.sol:
function test_claimantReceiveLess_potHoldsDifference() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
uint256 potBalBefore = ERC20Mock(weth).balanceOf(contest);
console.log("potBalBefore ", potBalBefore.toString());
ContestManager(conMan).fundContest(0);
vm.stopPrank();
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
uint256 remainingRewards = Pot(contest).getRemainingRewards();
uint256 managerPercent = 10;
uint256 amountOfClaimers = 1;
uint256 managerCut = remainingRewards / managerPercent;
uint256 userClaimCutExpected = (remainingRewards - managerCut) / amountOfClaimers;
uint256 userClaimCutActual = (remainingRewards - managerCut) / players.length;
assertGt(userClaimCutExpected, userClaimCutActual);
uint256 potBalAfter = ERC20Mock(weth).balanceOf(contest);
console.log("potBalAfter ", potBalAfter.toString());
assertEq(potBalBefore, 0);
assertEq(potBalAfter, userClaimCutActual);
}
Recommended Mitigation
Consider to divide remaining pool by the amount of claimants, not all players:
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
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);
}
}
