Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Dead Code in Platform Fee Adjustment Logic

joe23

Root

The claimThrone() function contains a conditional check:

if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}

However:

  • previousKingPayout is always initialized to 0 and never updated.

  • currentPlatformFee is calculated as a percentage of sentAmount, making it impossible for the condition to be true.

This renders the if block dead code, as it will never execute.

Impact

Code Bloat: The presence of unreachable code increases contract bytecode size and gas usage for no benefit.

  • Developer Confusion: Suggests misleading logic or an incomplete feature, which may introduce bugs if future devs attempt to "fix" it incorrectly.

  • Audit Overhead: Dead code makes it harder to audit and reason about intended behavior.

Description

  • Normal Behavior:
    In the claimThrone() function, a platform fee is calculated from the ETH sent by a new player. A defensive check is included to ensure that this fee does not exceed the remaining ETH after a payout to the previous king.

    Issue:
    However, the previousKingPayout variable is hardcoded to 0 and never updated.

  • As a result, the condition: can never be true, making this check dead code that will never execute.

if (currentPlatformFee > (sentAmount - previousKingPayout))
function claimThrone() external payable gameNotEnded nonReentrant {
...
// Calculate platform fee
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
​
// @> Dead code: this condition is never true because previousKingPayout is always 0
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
​
...
}
​

Risk

Likelihood:

  • This code path is present in every call to claimThrone(), making it highly visible and frequently encountered.

  • Since previousKingPayout is always set to 0, the condition in question will never evaluate to true, regardless of input or state, making the issue deterministically dead.Impact:

  • Impact:

    • Wasted gas on evaluating a condition that can never be true.

    • Increased bytecode size for no functional purpose.

Proof of Concept

Goal:
Demonstrate that the if condition inside claimThrone() is dead code and can never be triggered.

Test Explanation:
We simulate multiple sentAmount values and compute the currentPlatformFee. Since previousKingPayout is always zero, we check whether the condition currentPlatformFee > (sentAmount - previousKingPayout) can ever be true. It never is.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import "forge-std/Test.sol";
import "../src/Game.sol"; // Update path if your Game contract is located elsewhere
​
contract DeadCodeTest is Test {
Game game;
address owner = address(0xABCD);
​
function setUp() public {
vm.prank(owner);
game = new Game(
1 ether, // initialClaimFee
1 days, // gracePeriod
20, // feeIncreasePercentage
10 // platformFeePercentage
);
}
​
function testPlatformFeeCheckNeverTriggers() public {
// Simulate various sent amounts
uint256[5] memory testFees = [
uint256(1 ether),
uint256(2 ether),
uint256(5 ether),
uint256(10 ether),
uint256(100 ether)
];
​
for (uint256 i = 0; i < testFees.length; i++) {
uint256 sentAmount = testFees[i];
uint256 platformFeePercentage = game.platformFeePercentage(); // 10%
uint256 currentPlatformFee = (sentAmount * platformFeePercentage) /
100;
uint256 previousKingPayout = 0;
​
console.log("==== Test #%s ====", i + 1);
console.log("Sent: %s ETH", sentAmount / 1 ether);
console.log("Platform Fee: %s ETH", currentPlatformFee / 1 ether);
console.log(
"Sent - Prev King Payout: %s ETH",
(sentAmount - previousKingPayout) / 1 ether
);
​
// This condition in the Game contract will never be true
bool condition = currentPlatformFee >
(sentAmount - previousKingPayout);
assertFalse(
condition,
"Dead code triggered: condition unexpectedly true"
);
}
}
}

Output of the POC

[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/DeadCodeTest.t.sol:DeadCodeTest
[PASS] testPlatformFeeCheckNeverTriggers() (gas: 43278)
Logs:
==== Test #1 ====
Sent: 1 ETH
Platform Fee: 0 ETH
Sent - Prev King Payout: 1 ETH
==== Test #2 ====
Sent: 2 ETH
Platform Fee: 0 ETH
Sent - Prev King Payout: 2 ETH
==== Test #3 ====
Sent: 5 ETH
Platform Fee: 0 ETH
Sent - Prev King Payout: 5 ETH
==== Test #4 ====
Sent: 10 ETH
Platform Fee: 1 ETH
Sent - Prev King Payout: 10 ETH
==== Test #5 ====
Sent: 100 ETH
Platform Fee: 10 ETH
Sent - Prev King Payout: 100 ETH
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.08ms (382.35µs CPU time)
​
Ran 1 test suite in 10.87ms (1.08ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

- if (currentPlatformFee > (sentAmount - previousKingPayout)) {
- currentPlatformFee = sentAmount - previousKingPayout;
- }
Updates

Appeal created

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Missing Previous King Payout Functionality

Support

FAQs

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

Give us feedback!