Root + Impact
The GameEnded event always emits pot after it has been reset to 0. This means the emitted event shows no prize, even though the winner actually receives the real pot amount in pendingWinnings
Description
-
Normal Behavior:
When a winner is declared, the full pot balance should be assigned to the winner’s pendingWinnings and the GameEnded event should transparently log the amount won.
-
Specific Issue:
The code assigns the pot to pendingWinnings, then resets the pot to 0, then emits GameEnded with pot — so the event always shows 0 instead of the true pot value.
function declareWinner() external gameNotEnded {
require(currentKing != address(0), "Game: No one has claimed the throne yet.");
require(
block.timestamp > lastClaimTime + gracePeriod,
"Game: Grace period has not expired yet."
);
gameEnded = true;
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0;
emit GameEnded(currentKing, pot, block.timestamp, gameRound);
}
Risk
Likelihood:
Impact:
-
Low impact: Funds are not lost.
-
But it misleads off-chain listeners (frontends, indexers, analytics, or audit logs) who rely on the event for the real prize value.
-
Reduces transparency of game outcomes.
Proof Of Code
.
.
.
event GameEnded(
address indexed winner,
uint256 prizeAmount,
uint256 timestamp,
uint256 round
);
.
.
.
function test_declareWinnerEmitsZeroReward() public {
vm.startPrank(player1);
uint256 claimFee = game.claimFee();
game.claimThrone{value: claimFee}();
vm.stopPrank();
vm.warp(block.timestamp + 2 days);
vm.expectEmit(address(game));
emit GameEnded(address(player1), 0, block.timestamp, game.gameRound());
game.declareWinner();
}
Result:
Traces:
[227572] GameTest::test_declareWinnerEmitsZeroPot()
├─ [0] VM::startPrank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [2514] Game::claimFee() [staticcall]
│ └─ ← [Return] 100000000000000000 [1e17]
├─ [150621] Game::claimThrone{value: 100000000000000000}()
│ ├─ emit ThroneClaimed(newKing: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], claimAmount: 100000000000000000 [1e17], newClaimFee: 110000000000000000 [1.1e17], newPot: 95000000000000000 [9.5e16], timestamp: 1)
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(172801 [1.728e5])
│ └─ ← [Return]
├─ [0] VM::expectEmit(Game: [0x8Ad159a275AEE56fb2334DBb69036E9c7baCEe9b])
│ └─ ← [Return]
├─ [2471] Game::gameRound() [staticcall]
│ └─ ← [Return] 1
├─ emit GameEnded(winner: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], prizeAmount: 0, timestamp: 172801 [1.728e5], round: 1)
├─ [48689] Game::declareWinner()
│ ├─ emit GameEnded(winner: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], prizeAmount: 0, timestamp: 172801 [1.728e5], round: 1)
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 165.35ms (11.35ms CPU time)
Ran 1 test suite in 400.97ms (165.35ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, pendingWinnings[currentKing], block.timestamp, gameRound);
