Fjord Token Staking

Summary

Discrepency between the unstakeAll() function and unstake(), unstakeVested() functions related to immediate unstaking thus putting users at a disadvantage

Vulnerability Details

The FjordStaking.unstake and FjordStaking.unstakeVested functions allow user to unstake immediately if the currentEpoch == unstaking epoch. This can be seen in the following code snippets.

But the issue is FjordStaking.unstakeAll function which is used to unstake from all epochs (as its natspec comments describe) does not allow for immediate unstaking. It allows to unstake from the epochs which are beyond the lockCycle period only. Hence there is a discrepency between the unstakeAll() function and unstake(), unstakeVested() functions when it comes to immediate unstaking in the currentEpoch.

Impact

The users can get confused as a result of the above discrepancy. A user might need to unstake from all epochs including the currentEpoch. Since the unstake and vestedUnstake allow for immediate unstaking from the currentEpoch the user might expect the same behaviour from the unstakeAll function. He will call the unstakeAll to unstake all his staked FjordTokens in all epochs before lockCycle period and the staked amount in the currentEpoch. But calling the FjordStaking.unstakeAll() function will not unstake from the currentEpoch immediately and if he is not aware of this fact then his FjordTokens funds will get locked for a duration of lockCycle from the next epoch onwards against his will.

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L579-L583
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L462-L466
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L511-L515

Tools Used

Manual Review and VSCode

Recommendations

Hence it is recommended to follow the following steps:

1. Modify the FjordStaking.unstakeAll() function to enable immediate unstaking from the currentEpoch such that the functionalities of unstakeAll, unstake and unstakeVested functions align with each other thus eliminating any confusion related to unstaking.

2. Document this discrpency between the unstakeAll() function and unstake(), unstakeVested() functions related to immediate unstaking such that users are fully aware of this behaviour.