MyCut

AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Submission Details
Impact: low
Likelihood: high
Invalid

`remainingRewards` Is Not Updated After `closePot()` Distribution, Causing Stale Accounting

remainingRewards Is Not Updated After closePot() Distribution, Causing Stale Accounting

Description

closePot() uses remainingRewards to compute manager and claimant payouts, but it never updates remainingRewards after performing those transfers.

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

Because the state variable is left unchanged, getRemainingRewards() reports outdated values after closure and no longer reflects actual token balance in the pot. This creates accounting drift between state and reality, which can mislead monitoring, off-chain automation, and admin decision-making.


Risk

Likelihood: High

The issue occurs on every successful close where distribution executes, because the state variable is never synchronized post-transfer.

Impact: Low

This is primarily a state-accounting correctness issue. It does not directly create a new public theft path by itself, but it degrades observability and lifecycle correctness.


Proof of Concept

The stale accounting is demonstrated by the following PoC test:

function testClosePot_UnderDistributes_WhenFewClaimantsAndManyPlayers() public {
address[] memory players = new address[](3);
players[0] = p1;
players[1] = p2;
players[2] = p3;
uint256[] memory rewards = new uint256[](3);
rewards[0] = 40;
rewards[1] = 30;
rewards[2] = 30;
(address contest, Pot pot) = _createAndFund(players, rewards, 100);
vm.prank(p1);
pot.claimCut(); // remainingRewards = 60, claimants.length = 1
_close(contest);
assertEq(token.balanceOf(contest), 36);
assertEq(pot.getRemainingRewards(), 60);
}

After closure, the contract balance is 36, while getRemainingRewards() still returns 60, proving the value is stale.

Run:

forge test --match-test testClosePot_UnderDistributes_WhenFewClaimantsAndManyPlayers -vv

Output:

[PASS] testClosePot_UnderDistributes_WhenFewClaimantsAndManyPlayers() (gas: 1122444)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.22ms (665.34µs CPU time)
Ran 1 test suite in 20.78ms (2.22ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Finalize remainingRewards as part of closure so state matches post-settlement reality.

A practical approach is to calculate total distributed amount and subtract it from remainingRewards, or set it to zero if closure is intended to be terminal.

uint256 managerCut = remainingRewards / managerCutPercent;
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
+uint256 distributedToClaimants = claimantCut * claimants.length;
+uint256 totalDistributed = managerCut + distributedToClaimants;
... // transfers
+remainingRewards -= totalDistributed;

If closure should strictly finalize the pot, combine this with a one-time close guard and set remainingRewards to zero after handling residual dust by policy.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!