Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Users will not be able to claim from current epoch in unstakeAll

atoko

Summary

Users will not be able to claim from current epoch in unstakeAll

Vulnerability Details

Based on the comments below the unstakeAll function is meant to enable users unstake from all functions

/// @dev This function allows users to unstake from all the epochs at once,
/// while also claiming all the pending rewards.

However, based on the current implementation it is not possible for users to claim upto and including the current epoch if they just staked in it

function unstakeAll()
external
checkEpochRollover
redeemPendingRewards
returns (uint256 totalStakedAmount)
{
uint256[] memory activeDeposits = getActiveDeposits(msg.sender);
if (activeDeposits.length == 0) revert NoActiveDeposit();
​
for (uint16 i = 0; i < activeDeposits.length; i++) {
uint16 epoch = uint16(activeDeposits[i]);
DepositReceipt storage dr = deposits[msg.sender][epoch];
​
if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue;
​
totalStakedAmount += dr.staked;
​
// no vested staked and stake is 0 then delete the deposit
if (dr.vestedStaked == 0) {
delete deposits[msg.sender][epoch];
_activeDeposits[msg.sender].remove(epoch);
} else {
// still have vested staked, then only delete the staked
dr.staked = 0; // @audit current epoch?
}
}
​
totalStaked -= totalStakedAmount;
userData[msg.sender].totalStaked -= totalStakedAmount;
​
fjordToken.transfer(msg.sender, totalStakedAmount);
points.onUnstaked(msg.sender, totalStakedAmount);
​
// emit event
emit UnstakedAll(
msg.sender, totalStakedAmount, activeDeposits, getActiveDeposits(msg.sender)
);
}

Lets focus on this part of the function

if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue;

With this check in place it will not be possible for users to unstake from the current epoch if they have some stake in it.
Lets take an example of a user that decides to stake in an epoch x then call the unstakeAll function, based on the check above this will not be possible, if the desire is to prevent users from staking immediately, then i will discuss some issues that will occur in another report.

Impact

Users will not be able to claim from current epoch

Tools Used

manual review

Recommendations

If the desire is to allow users to call this function even if they are unstaking immediately from current epoch then dont skip the epoch in the loop

// If epoch is same as current epoch then user can unstake immediately
if (currentEpoch != epoch) {
// If epoch less than current epoch then user can unstake after at complete lockCycle
if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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