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

Rounding error in `claimant` Cut calculation.

oxenzo

Description:

Similarly, rounding errors can occur while calculating each claimant's cut, potentially leading to underpayment due to truncation. While this works against the favor of the user, it can still result in dissatisfaction and potential disputes if claimants feel they are not receiving their fair share.

​
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);
}
}
}
​
​

Impact:

claimants may receive less than their entitled share of the remaining funds, which could lead to loss of trust in the protocol.

Proof of Concept:

If the remaining funds are not evenly divisible by the number of claimants, the truncation could result in under-distribution, leading some funds undistributed.

Add this to the TestMyCut.t.sol file.

​
function testClaimantGetsUnfairZeroShare() public mintAndApproveTokens {
vm.startPrank(user);
// small rewards for each player which is 2,2 and 2
rewards = [2, 2, 2];
// total reward which is 6
totalRewards = 6;
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.startPrank(player3);
Pot(contest).claimCut();
vm.stopPrank();
​
​
uint256 claimantBalanceBefore1 = ERC20Mock(weth).balanceOf(player1);
// here the balance is 2
uint256 claimantBalanceBefore2 = ERC20Mock(weth).balanceOf(player2);
// here the balance is 2
uint256 claimantBalanceBefore3 = ERC20Mock(weth).balanceOf(player3);
// here the balance is 2
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
// Around here the managercut is 0 which is unfair.
​
uint256 claimantBalanceAfter1 = ERC20Mock(weth).balanceOf(player1);
// Here the player1 after the manager closes the contest his balance is 2
​
uint256 claimantBalanceAfter2 = ERC20Mock(weth).balanceOf(player2);
// Here the player2 balance after the manager closes the contest is 2
​
uint256 claimantBalanceAfter3 = ERC20Mock(weth).balanceOf(player3);
// Here the player3 balance after the manager closes the contest is 2
​
console.log("This is the claimantBalanceBefore1::", claimantBalanceBefore1);
console.log("This is the claimantBalanceBefore2::", claimantBalanceBefore2);
console.log("This is the claimantBalanceBefore3::", claimantBalanceBefore3);
console.log("This is the claimantBalanceAfter1::", claimantBalanceAfter1);
console.log("This is the claimantBalanceAfter2::", claimantBalanceAfter2);
console.log("This is the claimantBalanceAfter3::", claimantBalanceAfter3);
}
​

These are the following logs that were outputted::

Traces:
[1723902] TestMyCut::setUp()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Return]
├─ [1103083] → new ContestManager@0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353
│ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Return] 5391 bytes of code
├─ [517943] → new ERC20Mock@0x5929B14F2984bBE5309c2eC9E7819060C31c970f
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: DefaultSender: [0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38], value: 100000000000 [1e11])
│ └─ ← [Return] 2124 bytes of code
├─ [0] console::log("User Address: ", user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Contest Manager Address 1: ", ContestManager: [0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]
​
[1076746] TestMyCut::testClaimantGetsUnfairZeroShare()
├─ [0] console::log("Minting tokens to: ", user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Return]
├─ [29712] ERC20Mock::mint(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 1000000000000000000000 [1e21])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], value: 1000000000000000000000 [1e21])
│ └─ ← [Stop]
├─ [24739] ERC20Mock::approve(ContestManager: [0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353], 1000000000000000000000 [1e21])
│ ├─ emit Approval(owner: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], spender: ContestManager: [0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353], value: 1000000000000000000000 [1e21])
│ └─ ← [Return] true
├─ [0] console::log("Approved tokens to: ", ContestManager: [0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Return]
├─ [702007] ContestManager::createContest([0x7026B763CBE7d4E72049EA67E89326432a50ef84, 0xEb0A3b7B96C1883858292F0039161abD287E3324, 0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3], [2, 2, 2], ERC20Mock: [0x5929B14F2984bBE5309c2eC9E7819060C31c970f], 6)
│ ├─ [597792] → new Pot@0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0
│ │ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: ContestManager: [0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353])
│ │ └─ ← [Return] 1513 bytes of code
│ └─ ← [Return] Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0]
├─ [29108] ContestManager::fundContest(0)
│ ├─ [192] Pot::getToken() [staticcall]
│ │ └─ ← [Return] ERC20Mock: [0x5929B14F2984bBE5309c2eC9E7819060C31c970f]
│ ├─ [641] ERC20Mock::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ │ └─ ← [Return] 1000000000000000000000 [1e21]
│ ├─ [26084] ERC20Mock::transferFrom(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0], 6)
│ │ ├─ emit Transfer(from: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], to: Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0], value: 6)
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [70849] Pot::claimCut()
│ ├─ [25188] ERC20Mock::transfer(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], 2)
│ │ ├─ emit Transfer(from: Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0], to: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], value: 2)
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324])
│ └─ ← [Return]
├─ [48949] Pot::claimCut()
│ ├─ [25188] ERC20Mock::transfer(player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324], 2)
│ │ ├─ emit Transfer(from: Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0], to: player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324], value: 2)
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(player3: [0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3])
│ └─ ← [Return]
├─ [48949] Pot::claimCut()
│ ├─ [25188] ERC20Mock::transfer(player3: [0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3], 2)
│ │ ├─ emit Transfer(from: Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0], to: player3: [0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3], value: 2)
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [641] ERC20Mock::balanceOf(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84]) [staticcall]
│ └─ ← [Return] 2
├─ [641] ERC20Mock::balanceOf(player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324]) [staticcall]
│ └─ ← [Return] 2
├─ [641] ERC20Mock::balanceOf(player3: [0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3]) [staticcall]
│ └─ ← [Return] 2
├─ [0] VM::warp(7862400 [7.862e6])
│ └─ ← [Return]
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Return]
├─ [1425] ContestManager::closeContest(Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0])
│ ├─ [570] Pot::closePot()
│ │ └─ ← [Stop]
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [641] ERC20Mock::balanceOf(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84]) [staticcall]
│ └─ ← [Return] 2
├─ [641] ERC20Mock::balanceOf(player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324]) [staticcall]
│ └─ ← [Return] 2
├─ [641] ERC20Mock::balanceOf(player3: [0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3]) [staticcall]
│ └─ ← [Return] 2
├─ [0] console::log("This is the claimantBalanceBefore1::", 2) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("This is the claimantBalanceBefore2::", 2) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("This is the claimantBalanceBefore3::", 2) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("This is the claimantBalanceAfter1::", 2) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("This is the claimantBalanceAfter2::", 2) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("This is the claimantBalanceAfter3::", 2) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
​

As you can see where we have the traces for the claimant cut::

​
─ [641] ERC20Mock::balanceOf(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84]) [staticcall]
│ └─ ← [Return] 2
├─ [641] ERC20Mock::balanceOf(player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324]) [staticcall]
│ └─ ← [Return] 2
├─ [641] ERC20Mock::balanceOf(player3: [0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3]) [staticcall]
│ └─ ← [Return] 2
​

From the above trace, it proves that none of the 3 claimants got any share mainly because of the truncation that occurs in the claimant cut formula below:

​
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
​

From above, we all know that the manager cut gets truncated to 0.

So this how the formula works it out...

remainingRewards = 0
managerCut = 0
i_players.length = 3

0 / 3 = 0

So the claimantCut ends up being 0.

Recommended Mitigation:

Rounding the final claimantCut division upwards would fix this.

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Dusty Pot

Support

FAQs

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