MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Remaining rewards should be divided by the number of claimers

ivanonchain

Description:

When Pot.sol::closePot function distribute the remaining rewards is dividing that amount into the number of players to distribute into the claimants. When the number of players is greater than the number of claimants a amount will remaining in the contract

Impact:

The Readme of this project claims that the remainder is distributed equally to those who claimed in time, but this is not the case because always will be a locked remaining in the contract if there are players that didn't claim, duet o the remaining amount is divided into the number of players when it should be divided into the number of claimants.
This will happens always this scenario happens and it is not an expected behavior

The README of this project states that the remainder is distributed equally to those who claimed in time. However, but this is not the case because. A portion of the remaining balance will always be locked in the contract if some players do not claim, as the remainder is divided among the total number of players rather than just the claimants. This issue will consistently occur in such scenarios and is not the expected behavior

Proof of Concept:

Paste next code in the TestMyCut.sol file

function testRemainingRewardsShouldBeDividedByTheNumberOfClaimers() mintAndApproveTokens public {
address player3 = makeAddr("player3");
​
vm.startPrank(user);
rewards = [20, 50, 100];
players = [player1, player2, player3];
totalRewards = 170;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.startPrank(player2);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
uint256 finalPotBalance = IERC20(ERC20Mock(weth)).balanceOf(contest);
console.log('finalPotBalance: ', finalPotBalance);
// The remaining amount is 30
// ((100 - (100/10)) /3 ) => (100 - 10)/ 3 => 90/3 = 30
// So 30 for each claimer, as there are 2 claimer will remain 30 in the contract
assertEq(finalPotBalance, 30);
}

Recommended Mitigation:

Consider divide into the number of claimant instead of number of 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);
}
}
}
Updates

Lead Judging Commences

equious Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect distribution in closePot()

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.