Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

L01-Incorrect GameEnded Event Emission

Root + Impact

Description

Normal behavior:
When the game ends after the grace period, the declareWinner() function should emit the GameEnded event with accurate details: the winner’s address, the total prize amount (the pot), the timestamp, and the game round.

Issue:
Due to the ordering of operations in declareWinner(), the pot is set to 0 before it is emitted in the GameEnded event. This causes the event to always report a prize amount of 0, even when the winner has received a non-zero payout. This misleads users, explorers, and off-chain indexers about actual winnings and can make it appear that the winner received nothing.

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;
<@ audit
pot = 0; // Reset pot after assigning to winner's pending winnings
emit GameEnded(currentKing, pot, block.timestamp, gameRound);
}



Risk

Likelihood:

  • Happens every time declareWinner() is called and the pot > 0.

  • Will always occur because pot = 0 is set before the event emission.

Impact:

  • All GameEnded events on-chain will misrepresent the prize amount as 0.

  • Reduces transparency and trust in the game contract, especially from off-chain UIs, analytics platforms, and players relying on event logs.

  • Makes post-mortem accounting and leaderboard logic unreliable.


Proof of Concept

// Setup
game.claimThrone{value: 1 ether}(); // Player becomes king
vm.warp(block.timestamp + game.gracePeriod() + 1); // Simulate time passing
// Act
game.declareWinner(); // Ends the game
// Inspect event logs
// Expected: GameEnded(winner, 1 ether, timestamp, round)
// Actual: GameEnded(winner, 0, timestamp, round)

Recommended Mitigation

Move the pot value into a local variable before zeroing it, and emit that in the event:

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;
+ uint256 prizeAmount = pot;
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0;
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, prizeAmount, block.timestamp, gameRound);
}

This ensures accurate logging and restores event consistency for external users and platforms.

Updates

Appeal created

inallhonesty Lead Judge 9 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Game::declareWinner emits GameEnded event with pot = 0 always

Support

FAQs

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