Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: insert tolerance into redeemShare() checks [SUP-8853] #645

Merged
merged 13 commits into from
Oct 24, 2024

Conversation

TamaraRingas
Copy link

https://github.com/yAudit/superform-report/issues/2

deposit4626() is used to redeem vault assets and deposit them in a superform vault. However, in _redeemShare(), there is a strict equality balance check, that, in case of stETH, will always revert.

Technical Details

Lido stETH has a know rounding down problem as stated in their docs. When trying to redeem stETH with deposit4626(), the balance check will always fail leading the function to revert.

Impact

Low. It is not possible to use deposit4626() with a stETH vault and any other token that have a similar behaviour.

Recommendation

Avoid to use strict equality and insert a tolerance in the balance check to handle the 1-2 wei corner case in the same way that is done here.

Copy link
Contributor

@0xTimepunk 0xTimepunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there!

src/router-plus/SuperformRouterPlus.sol Outdated Show resolved Hide resolved
src/router-plus/SuperformRouterPlus.sol Outdated Show resolved Hide resolved
@0xTimepunk 0xTimepunk changed the title fix: insert tolerance into redeemShare() checks [SUP-8835] fix: insert tolerance into redeemShare() checks [SUP-8853] Oct 24, 2024
Copy link

linear bot commented Oct 24, 2024

@0xTimepunk 0xTimepunk merged commit 0a7a58b into v1.5 Oct 24, 2024
6 checks passed
@TamaraRingas TamaraRingas deleted the tamara-low-info-fixes branch October 24, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants