Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: low
Invalid

LP provider may struggle to `unstake` because their vault can change value with every user action

Summary

In VaultRouterBranch::unstakeThere is a check that ensures a LP provider can only unstake if they have no available rewards.

The rewards are accounted in Distributio, which keeps track of distributing value to the vault (rewards to be claimed by LP providers based on how many shares they have).

But, the vault's value gets updated with every user action. This makes the vault's value to distribute and the LP providers rewards ever-changing.

This constant change in value can make it very difficult to pass the check in unstake-> which would revert an unstake.

Vulnerability Details

In unstakethere is this check that ensures that the user does not have any pending/claimable rewards, if they do they cant unstake:

// get the claimable amount of fees
UD60x18 amountToClaimX18 = vault.wethRewardDistribution.getActorValueChange(actorId).intoUD60x18();
// reverts if the claimable amount is NOT 0
if (!amountToClaimX18.isZero()) revert Errors.UserHasPendingRewards(actorId, amountToClaimX18.intoUint256());

This value (amount of rewards for LP provider) gets updated via this process:

  • The vault calculates and updates various amounts, but for this case, we are focusing on the vault updating and distributing value to the Distributorcontract. That is the vault the LP provider has shares in. Everytime this function is called, the vault can have new value distributed to it. This function is called during every user action, meaning the vault can receive value and change the rewards amount for a user at every action.

// updates the vault's credit capacity and perform all vault
// state transitions before updating `msg.sender` staked shares
Vault.recalculateVaultsCreditCapacity(vaultsIds);
  • The Distributorcontract distributesthe new value by updating valuePerShare

SD59x18 deltaValuePerShare = value.div(totalShares.intoSD59x18());
self.valuePerShare = sd59x18(self.valuePerShare).add(deltaValuePerShare).intoInt256();
  • That value update is used to determine new rewards for LP provider

SD59x18 deltaValuePerShare = sd59x18(self.valuePerShare).sub(sd59x18(actor.lastValuePerShare));
valueChange = deltaValuePerShare.mul(ud60x18(actor.shares).intoSD59x18());

So, every time recalculateVaultsCreditCapacityis called for that vault, their can be new pending rewards for LP providers.

IF unstakefails because of this, the user can go to the FeeDistributionBranchand claimFeesto try and claim the fee's so that they can unstake.

But FeeDistributionBranch::claimFeesdoes not call recalculateVaultsCreditCapacity-> so the reward amount will be at the current state (before updating Distribution)

The user will have their accounting in Distributionupdated so that their lastValuePerShare= the contracts current (not most current, becasue the update didnt happen) valuePerShare-> which will equate to them not having any new rewards.

This would allow the check in unstaketo pass.

BUT, any user action in that vault will update the vaults rewards and subsequetly adding new pending rewards to the user.

Also, when the user goes back to unstakeafter claiming their rewards, recalculateVaultsCreditCapacityis called again and can update the rewards they have pending. Making them fail the check in unstakeagain and not allowing them to unstake.

Impact

It can be difficult for LP providers to unstake and even if they claim their rewards, by the time they unstaketheir vault could have been updated and their unstakewould fail.

Only allowing them to unstakeif no updates to the vault are made.

Tools Used

Manual Review

Recommendations

Becasue the rewards are paid through FeeDistributionBranchand not VaultBranchRouter-> if the user has pending rewards after the update which is called at the beginning of unstake:

The claimable rewards amount can be stored seperately, allowing the user to them claimFeeeven after their unstakehas processed. The user wont be able to generate any new rewards, but only able to claim the rewards they have earned (which is what the check in unstakeis allowing / enforcing anyways).

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID] Inside `VaultRouterBranch`::`unstake` it's checked that a LP provider can only call if they have no available rewards, this could lead to DoS.

Support

FAQs

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