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

Incorrect Staking Logic Implementation in `Staking.sol`

robertodf99

[H-1] Incorrect Staking Logic Implementation in Staking.sol

Description: The current implementation of staking logic allows users to claim more tokens than they should be entitled to. Additionally, if rewards are claimed before the completion of the last week, users lose the rewards for that week.

Impact: The issues with the staking logic lead to the following consequences:

  1. The claimRewards() function assigns the wrong timestamp to users claiming rewards for the first time, resulting in incorrect reward calculations.

if (lastClaim[msg.sender] == 0) {
lastClaim[msg.sender] = soulmateContract.idToCreationTimestamp(
soulmateId
);

For these users, the timestamp when the soulmate token was minted is assigned as the last claim. Since the rewards for each user are calculated as:

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
);

This implies that users don't need to deposit tokens in the contract to accrue love tokens, it will be enough depositing them the same day they want to claim the rewards (they have to at least wait for a week). This is in line with the next issue.

  1. After the initial claim, the user's lastClaim timestamp is updated. Subsequently, as the reward is solely updated upon calling Staking.sol::claimRewards(), users can strategically deposit tokens on the same day they intend to claim rewards. This behavior implies that users don't necessarily need to maintain token deposits for longer than a week if they choose not to. This compounds with the issue highlighted earlier, exacerbating the potential for users to exploit the system by minimizing their token deposits to optimize reward accumulation.

  2. In the same function Staking.sol::claimRewards(), users lose the tokens claimed after the completion of the last week, if they don't want to loose any of their rewards they have to claim them at exactly the end of the week:

uint256 timeInWeeksSinceLastClaim = ((block.timestamp -
lastClaim[msg.sender]) / 1 weeks);
if (timeInWeeksSinceLastClaim < 1)
revert Staking__StakingPeriodTooShort();
​
lastClaim[msg.sender] = block.timestamp;

The amount lost is equal to (block.timestamp-lastClaim[msg.sender])%1 weeks*userStakes[msg.sender]/ 1 weeks.

Proof of Concept: Place the following in StakingTest.t.sol:

Code
function test_FirstClaimIsMisscalculated() public {
uint256 balancePerSoulmates = 1 ether;
_giveLoveTokenToSoulmates(balancePerSoulmates);
uint256 weekOfStaking = 2;
vm.warp(block.timestamp + weekOfStaking * 1 weeks);
vm.startPrank(soulmate1);
airdropContract.claim();
loveToken.approve(address(stakingContract), 1 ether);
stakingContract.deposit(1 ether);
stakingContract.claimRewards();
vm.stopPrank();
assertTrue(loveToken.balanceOf(soulmate1) == 14 ether + 2 ether);
}
​
function test_userDepositsTokenWhenClaiming() public {
uint256 balancePerSoulmates = 1 ether;
_depositTokenToStake(balancePerSoulmates);
uint256 weekOfStaking = 1;
vm.warp(block.timestamp + weekOfStaking * 1 weeks);
vm.startPrank(soulmate1);
airdropContract.claim();
loveToken.approve(address(stakingContract), 1 ether);
stakingContract.deposit(1 ether);
stakingContract.claimRewards();
vm.stopPrank();
assertTrue(loveToken.balanceOf(soulmate1) == 8 ether); // 6 accrued during a week and 2 from the staking rewards
}
​
function test_stakedTokensAreLost() public {
uint256 balancePerSoulmates = 1 ether;
_depositTokenToStake(balancePerSoulmates);
uint256 weekOfStaking = 2;
vm.warp(block.timestamp + weekOfStaking * 1 weeks - 1);
vm.prank(soulmate1);
stakingContract.claimRewards();
uint256 tokensLost = (((block.timestamp -
stakingContract.lastClaim(msg.sender)) % 1 weeks) *
balancePerSoulmates) / 1 weeks;
assertTrue(
loveToken.balanceOf(soulmate1) ==
((block.timestamp - stakingContract.lastClaim(msg.sender)) *
balancePerSoulmates) /
1 weeks -
tokensLost
);
}

Recommended Mitigation: Keep track of users' rewards in different points in time by implementing a modifier updateReward() and a function earned() as shown in the example below:

Code
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
​
import {IVault} from "./interface/IVault.sol";
import {ISoulmate} from "./interface/ISoulmate.sol";
import {ILoveToken} from "./interface/ILoveToken.sol";
​
/// @title Staking Contract for LoveToken.
/// @author n0kto
contract CorrectStaking {
/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
error CorrectStaking__NoMoreRewards();
​
/*//////////////////////////////////////////////////////////////
STATE VARIABLES
//////////////////////////////////////////////////////////////*/
​
ILoveToken public immutable loveToken;
ISoulmate public immutable soulmateContract;
IVault public immutable stakingVault;
​
mapping(address user => uint256 loveToken) public userStakes;
mapping(address => uint) public activeAt;
uint256 public constant rewardPerToken = 1;
mapping(address => uint) public rewardsPerAccount;
​
/*//////////////////////////////////////////////////////////////
EVENTS
//////////////////////////////////////////////////////////////*/
event Deposited(address indexed user, uint256 amount);
event Withdrew(address indexed user, uint256 amount);
event RewardsClaimed(address indexed user, uint256 amount);
​
/*//////////////////////////////////////////////////////////////
MODIFIERS
//////////////////////////////////////////////////////////////*/
modifier updateReward(address _account) {
if (_account != address(0)) {
rewardsPerAccount[_account] = earned(_account);
activeAt[_account] = block.timestamp;
}
​
_;
}
​
/*//////////////////////////////////////////////////////////////
FUNCTIONS
//////////////////////////////////////////////////////////////*/
constructor(
ILoveToken _loveToken,
ISoulmate _soulmateContract,
IVault _stakingVault
) {
loveToken = _loveToken;
soulmateContract = _soulmateContract;
stakingVault = _stakingVault;
}
​
/// @notice Increase the userStakes variable and transfer LoveToken to this contract.
function deposit(uint256 amount) public updateReward(msg.sender) {
if (loveToken.balanceOf(address(stakingVault)) == 0)
revert CorrectStaking__NoMoreRewards();
// No require needed because of overflow protection
userStakes[msg.sender] += amount;
emit Deposited(msg.sender, amount);
loveToken.transferFrom(msg.sender, address(this), amount);
}
​
/// @notice Decrease the userStakes variable and transfer LoveToken to the user withdrawing.
function withdraw(uint256 amount) public updateReward(msg.sender) {
// No require needed because of overflow protection
userStakes[msg.sender] -= amount;
emit Withdrew(msg.sender, amount);
loveToken.transfer(msg.sender, amount);
}
​
/// @notice Claim rewards for staking.
/// @notice Users can claim 1 token per staking token per week.
function claimRewards() public updateReward(msg.sender) {
if (loveToken.balanceOf(address(stakingVault)) == 0)
revert CorrectStaking__NoMoreRewards();
uint256 amountToClaim;
// Dust collector
if (
loveToken.balanceOf(address(stakingVault)) <=
rewardsPerAccount[msg.sender]
) {
amountToClaim = loveToken.balanceOf(address(stakingVault));
} else {
amountToClaim = rewardsPerAccount[msg.sender];
}
​
rewardsPerAccount[msg.sender] = 0;
​
emit RewardsClaimed(msg.sender, amountToClaim);
​
loveToken.transferFrom(
address(stakingVault),
msg.sender,
amountToClaim
);
}
​
function earned(address _account) public view returns (uint) {
uint256 elapsedSeconds = block.timestamp - activeAt[_account];
return
((userStakes[_account] * rewardPerToken) * elapsedSeconds) /
1 weeks +
rewardsPerAccount[_account];
}
}
​
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.