First Flight #9: Soulmate
First Flight #9
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Staking rewards can be lost

honour

Description:
Stakers who accidentally/unintentionally withdraw some/all of their tokens from the staking contract without first claiming their rewards will lose some/all of their staking rewards.

Proof of Concept:
Add test to StakingTest.t.sol

function test_RewardsLoss() public {
//stakers deposit 10 loveTokens to stake
_depositTokenToStake(10 ether);
​
//soulmate1 claims before withdrawing stake ✅
vm.startPrank(soulmate1);
uint256 soulmate1BalanceBeforeClaim = loveToken.balanceOf(soulmate1);
stakingContract.claimRewards();
uint256 soulmate1BalanceAfterClaim = loveToken.balanceOf(soulmate1);
stakingContract.withdraw(10 ether);
vm.stopPrank();
//soulmate2 withdraws stake before claiming loosing their rewards
vm.startPrank(soulmate2);
stakingContract.withdraw(10 ether);
uint256 soulmate2BalanceBeforeClaim = loveToken.balanceOf(soulmate2);
stakingContract.claimRewards();
uint256 soulmate2BalanceAfterClaim = loveToken.balanceOf(soulmate2);
vm.stopPrank();
assertTrue(soulmate1BalanceAfterClaim > soulmate1BalanceBeforeClaim);
assertTrue(soulmate2BalanceBeforeClaim == soulmate2BalanceAfterClaim);
}

Tools Used:
Manual Review

Recommendation:
Staking::claimRewards should be called on withdraw if the staker has any rewards i.e lastClaim > 1 week

function withdraw(uint256 amount) public {
// No require needed because of overflow protection
//? should also claimRewards on withdraw to prevent users from loosing their stake
if((block.timestamp - lastClaim[msg.sender])/1 weeks >= 1) claimRewards();
userStakes[msg.sender] -= amount;
loveToken.transfer(msg.sender, amount);
emit Withdrew(msg.sender, amount);
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-withdraw-before-claimReward

If we we implement a correct claimRewards function with its intended logic, this would indeed be an issue. I believe low severity for this findings and its duplicates to be appropriate given it is dependent on users lack of understanding of claiming rewards first before a withdrawal.

Support

FAQs

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