Description:
The claimThrone() function declares a variable previousKingPayout but never assigns it any value beyond its default of zero. As a result, the previousKing never receives any share of the incoming claim fee, even though the comments and intended design suggest they should. All of the ETH beyond the platform fee is funneled into the pot, depriving players of any direct reward for having held the throne.
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
@> uint256 previousKingPayout = 0;
uint256 currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
uint256 amountToPot = sentAmount - currentPlatformFee;
platformFeesBalance += currentPlatformFee;
pot += amountToPot;
}
Impact: HIGH
-
No participant payouts: A player who successfully holds the throne and is then dethroned receives zero ETH from subsequent claims, contrary to game design.
-
Reduced player incentive: Without any direct payout, participants only contribute to the pot (and platform fees), disincentivizing play.
-
Pot skew: The pot grows larger than intended by the missing “king’s cut,” potentially disrupting prize mechanics.
Proof of Concept:
function testPreviousKingGetsNoPayout() public {
vm.prank(player1);
game.claimThrone{value: 1 ether}();
vm.prank(player2);
uint256 fee2 = game.claimFee();
game.claimThrone{value: fee2}();
uint256 payout1 = game.pendingWinnings(player1);
assertEq(payout1, 0, "Previous king should have received a payout but got none");
}
[PASS] testPreviousKingGetsNoPayout() (gas: 189666)
Traces:
[229466] GameTest::testPreviousKingGetsNoPayout()
├─ [0] VM::prank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [150533] Game::claimThrone{value: 1000000000000000000}()
│ ├─ emit ThroneClaimed(newKing: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], claimAmount: 1000000000000000000 [1e18], newClaimFee: 110000000000000000 [1.1e17], newPot: 950000000000000000 [9.5e17], timestamp: 1)
│ └─ ← [Stop]
├─ [0] VM::prank(player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324])
│ └─ ← [Return]
├─ [372] Game::claimFee() [staticcall]
│ └─ ← [Return] 110000000000000000 [1.1e17]
├─ [48033] Game::claimThrone{value: 110000000000000000}()
│ ├─ emit ThroneClaimed(newKing: GameTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], claimAmount: 110000000000000000 [1.1e17], newClaimFee: 121000000000000000 [1.21e17], newPot: 1054500000000000000 [1.054e18], timestamp: 1)
│ └─ ← [Stop]
├─ [2516] Game::pendingWinnings(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84]) [staticcall]
│ └─ ← [Return] 0
└─ ← [Stop]
Recommended Mitigation:
Calculate and distribute a percentage of each new claim fee to the outgoing king before adding the remainder to the pot.
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
- uint256 previousKingPayout = 0;
+ uint256 previousKingPayout = (sentAmount * previousKingPercentage) / 100;
uint256 currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
// Pay outgoing king
+ if (currentKing != address(0) && previousKingPayout > 0) {
+ pendingWinnings[currentKing] += previousKingPayout;
+ }
// Collect platform fee
platformFeesBalance += currentPlatformFee;
// Add remaining to pot
- uint256 amountToPot = sentAmount - currentPlatformFee;
+ uint256 amountToPot = sentAmount - currentPlatformFee - previousKingPayout;
pot += amountToPot;
}
Define previousKingPercentage in contract state (uint256 public previousKingPercentage;)
This ensures that dethroned players receive their intended reward.
