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

Staking Rewards Misaligned with Staking Period

paprikrumplikas

Summary

The staking mechanism within Staking.sol incorrectly calculates rewards based on the duration since the soulmates were paired, rather than from when the users actually began staking their tokens. This misalignment can lead to users receiving rewards for a period during which their tokens were not actively staked, contrary to the intended functionality of rewarding users based on the time their capital is locked in the staking contract.

Vulnerability Details

Staking::claimRewards determines the amount of rewards a user is eligible for based on the time elapsed since the soulmates were paired (as indicated by the timestamp of pairing). This approach does not account for the actual commencement of staking, potentially granting rewards for a period prior to the staking action. The provided test code demonstrates that rewards can be claimed immediately after depositing tokens into the staking contract, without a requisite waiting period reflective of actual staking duration:

function test_faultyStakingLogic() public {
uint256 balanceUser1;
uint256 balanceUser1_beforeRewardsClaim;
uint256 balanceUser1_afterRewardsClaim;
​
// pair up user 1 and 2
vm.prank(user1);
soulmateContract.mintSoulmateToken();
vm.prank(user2);
soulmateContract.mintSoulmateToken();
​
vm.warp(block.timestamp + 4 weeks); // fast fwd time with 1 month
vm.startPrank(user1);
airdropContract.claim(); // claim airdrop
balanceUser1 = loveToken.balanceOf(user1);
loveToken.approve(address(stakingContract), type(uint256).max); // approve spending
stakingContract.deposit(balanceUser1); // deposit all
vm.stopPrank();
​
// to claim rewards, we dont need to advance time after starting staking
​
vm.startPrank(user1);
balanceUser1_beforeRewardsClaim = loveToken.balanceOf(user1);
stakingContract.claimRewards();
balanceUser1_afterRewardsClaim = loveToken.balanceOf(user1);
vm.stopPrank();
​
assertLt(balanceUser1_beforeRewardsClaim, balanceUser1_afterRewardsClaim);
}

Impact

This flaw impacts the staking system in several ways:

  1. Unwarranted Rewards: Users can claim rewards not duly earned through staking, undermining the fairness and integrity of the rewards distribution mechanism.

  2. Resource Drain: The premature or unwarranted distribution of rewards can lead to a faster depletion of the rewards pool, affecting the sustainability of the staking program.

  3. Incentive Structure Distortion: The misalignment between staking period and rewards calculation can distort the intended incentive structure, potentially rewarding short-term interactions over genuine long-term participation.

Tools Used

Manual review.

Recommendations

The Staking contract should be amended to calculate rewards based on the actual period tokens have been staked. This involves tracking the timestamp at which each user's tokens are deposited into the staking contract and using this timestamp to calculate rewards.

+ mapping(address => uint256) private stakingStartTimes;
​
function deposit(uint256 amount) public {
if (loveToken.balanceOf(address(stakingVault)) == 0) {
revert Staking__NoMoreRewards();
}
+ if (stakingStartTimes[msg.sender] == 0) {
+ stakingStartTimes[msg.sender] = block.timestamp;
+ }
// No require needed because of overflow protection
userStakes[msg.sender] += amount;
loveToken.transferFrom(msg.sender, address(this), amount);
​
emit Deposited(msg.sender, amount);
}
​
​
function claimRewards() public {
uint256 soulmateId = soulmateContract.ownerToId(msg.sender);
// first claim
if (lastClaim[msg.sender] == 0) {
- lastClaim[msg.sender] = soulmateContract.idToCreationTimestamp(soulmateId);
+ lastClaim[msg.sender] = stakingStartTimes[msg.sender];
}
​
// How many weeks passed since the last claim.
uint256 timeInWeeksSinceLastClaim = ((block.timestamp - lastClaim[msg.sender]) / 1 weeks);
​
if (timeInWeeksSinceLastClaim < 1) {
revert Staking__StakingPeriodTooShort();
}
​
lastClaim[msg.sender] = block.timestamp;
​
// Send the same amount of LoveToken as the week waited times the number of token staked
uint256 amountToClaim = userStakes[msg.sender] * timeInWeeksSinceLastClaim;
loveToken.transferFrom(address(stakingVault), msg.sender, amountToClaim);
​
emit RewardsClaimed(msg.sender, amountToClaim);
}
​

Note that the suggestion above has a limitation: it does not account for multiple deposits made by the same user at different times. To address this and accurately calculate rewards for each deposit, a more nuanced approach is required. This can involve tracking each deposit individually along with its timestamp.

Updates

Lead Judging Commences

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

finding-claimRewards-multi-deposits-time

High severity, this allows users to claim additional rewards without committing to intended weekly staking period via multi-deposit/deposit right before claiming rewards.

Support

FAQs

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