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

The `unstakeAll()` function cannot immediately return the staked `fjordToken` at `currentEpoch`

shaflow01

Summary

unstakeAll() allows users to unstake from all epochs at once, but it cannot withdraw tokens staked in the currentEpoch. Additionally, while the function's comment states that it will 'also claim all the pending rewards,' in reality, it does not claim the pending rewards but only calculates them.

Vulnerability Detail

unstakeAll() allows users to unstake from all epochs at once, but due to the following constraints, it cannot withdraw tokens staked in the currentEpoch.

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L583

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

Additionally, the function’s comment inaccurately describes its functionality.

Impact

unstakeAll() allows users to unstake from all epochs at once, but it cannot withdraw tokens staked in the currentEpoch

Tools Used

Manual Review

Recommendations

/// @notice Unstake from all epochs.
/// @dev This function allows users to unstake from all the epochs at once,
- /// while also claiming all the pending rewards.
/// @return totalStakedAmount The total amount that has been unstaked.
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 && epoch != currentEpoch )) continue;
- 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;
}
}
​
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)
);
}
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.