-
Notifications
You must be signed in to change notification settings - Fork 321
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
Prevent bLUSD LP withdrawal MEV sandwiching #1005
Conversation
It looks good, although I'm not sure how precise the withdrawal prediction is (particularly those cases where we rely on Have you tried each case (bLUSD, LUSD, both)? |
Good suggestion on fixing a min slippage. This should reduce transaction failure. With a slippage of 0.1% I didn't receive any failed transactions on a mainnet fork, for each case: bLUSD, LUSD and both. |
const minBLusdAmount = withdrawal.get(BLusdAmmTokenIndex.BLUSD)?.mul(1 - curveSlippage); | ||
const minLusdAmount = withdrawal.get(BLusdAmmTokenIndex.LUSD)?.mul(1 - curveSlippage); | ||
|
||
if (minBLusdAmount === undefined || minBLusdAmount === Decimal.ZERO) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under which conditions could this happen? Would the user get any error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory it shouldn't happen since withdrawal
should be initialized by the time handleConfirmPressed
is invoked, however this check ensures that for whatever potential reason withdrawal
is zero (e.g. a future dev mistake, or unknown bug), this function won't let the user withdraw with a 0 value.
}); | ||
} | ||
}; | ||
};; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m curious, why does it need two “;”?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me! 👍 👍
2246a3f
to
5b3229e
Compare
Specify min out parameters for Curve liquidity withdrawals.