Description
The problem is that when someone claims the throne, the remaining time resets to the gracePeriod, which leads to incorrect calculation of the remaining time and can cause the game to not finish after the grace period, or even never finish at all.
In this POC, we show the Remaining Time when player1 claims the throne immediately after the game starts, and player2 claims it after half a day has passed.
Proof of Concept
But before running the test, you need to correct this issue in claimThrone():
- require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
+ require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
Then, add this test to Game.t.sol:
function testGetRemainingTime() public {
vm.startPrank(player1);
uint256 lastClaimTime = game.lastClaimTime();
uint256 currentTimestamp = block.timestamp;
uint256 remainingTime = game.getRemainingTime();
console2.log("When game started:");
console2.log("Last Claim Time:", lastClaimTime);
console2.log("Current Block Timestamp:", currentTimestamp);
console2.log("Grace Time:", GRACE_PERIOD);
console2.log("=> Remaining Time:", remainingTime);
vm.stopPrank();
vm.warp(block.timestamp + 43200);
vm.startPrank(player2);
game.claimThrone{value: 0.1 ether}();
uint256 newLastClaimTime = game.lastClaimTime();
uint256 newTimestamp = block.timestamp;
uint256 newRemainingTime = game.getRemainingTime();
console2.log("______________________________________________________");
console2.log("User claims throne after half a day:");
console2.log("New Last Claim Time:", newLastClaimTime);
console2.log("New Current Block Timestamp:", newTimestamp);
console2.log("Grace Time:", GRACE_PERIOD);
console2.log("=> New Remaining Time:", newRemainingTime);
vm.stopPrank();
}
Then run:
forge test --match-test testGetRemainingTime -vvv
Output
Ran 1 test for test/Game.t.sol:GameTest
[PASS] testGetRemainingTime() (gas: 175102)
Logs:
When game started:
Last Claim Time: 1
Current Block Timestamp: 1
Grace Time: 86400
=> Remaining Time: 86400
______________________________________________________
User claims throne after half a day:
New Last Claim Time: 43201
New Current Block Timestamp: 43201
Grace Time: 86400
=> New Remaining Time: 86400
As we see in the output, the remaining time did not decrease correctly.
Recommended Mitigation
You need to separate the game's startTime from lastClaimTime. Do not update it when the throne is claimed. Just declare it once at the start of the game using block.timestamp.
+ uint256 public startGame;
- lastClaimTime = block.timestamp;
+ startGame = block.timestamp;
- uint256 endTime = lastClaimTime + gracePeriod;
+ uint256 endTime = startGame + gracePeriod;
