-
Notifications
You must be signed in to change notification settings - Fork 108
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
feat(slippage): smart slippage for high fee orders #4911
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -61,27 +61,60 @@ function quoteUsingSameParameters(currentParams: FeeQuoteParams, quoteInfo: Quot | |||
} = currentParams | |||
const { amount, buyToken, sellToken, kind, userAddress, receiver, appData } = quoteInfo | |||
const hasSameReceiver = currentReceiver && receiver ? currentReceiver === receiver : true | |||
const hasSameAppData = compareAppDataWithoutQuoteData(appData, currentAppData) |
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.
Needed to drop the quote from appData otherwise it enters into an infinite loop.
if (percentage === undefined) { | ||
// Unset, return undefined | ||
return | ||
} | ||
if (percentage < 1) { | ||
// bigger volume compared to the fee, trust on smart slippage from BFF | ||
return | ||
} else if (percentage < 5) { | ||
// Between 1 and 5, 2% | ||
return 200 | ||
} else if (percentage < 10) { | ||
// Between 5 and 10, 5% | ||
return 500 | ||
} else if (percentage < 20) { | ||
// Between 10 and 20, 10% | ||
return 1000 | ||
} | ||
// TODO: more granularity? | ||
|
||
// > 30%, cap it at 20% slippage | ||
return 2000 |
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.
Open for debate. Too much? Too little? More levels?
Hey @alfetopito , great job! Besides the question I reported in the thread, there is a nitpick I'd like to report: I can see that a slippage percentage can be changed when a final quote is loaded Not sure what we can do with this.. Maybe ad a 'loading' effect to these fields? But, AFAIU, in a separate task. Btw slippage of 20% looks a bit to scary for me :) |
00e0eda
to
94fd7fa
Compare
94fd7fa
to
209773f
Compare
209773f
to
786eae4
Compare
I added a crude loading indicator when either:
🤷 |
Hey @alfetopito , thank you for adding the loading effect! Looks the way better now! |
I think the suggested values make sense. Maybe a more generalizable strategy would be something like:
This way we would allow at least 50% price movement of the gas estimate itself (which usually should become irrelevant or equal to default slippage when fees are <1%). I think this approach would be more flexible and not necessarily override the smart slippage (in case the underlying token is very volatile) Was bucketing things an attempt to reduce the number of discrete values we have? |
fa43c2f
to
1cbe7a3
Compare
Superseded by #4982 |
Summary
Fixes #4736
Calculates smart slippage based on fee percentage relative to sell amount.
This change intends to cover small orders where a small network cost variation can make them untradable.
The proposed logic, which still need to be discussed and reviewed, works as follows:
These will take precedence over smart slippage from BFF.
To Test
Note: In gchain/arb1 the fees are very small, so the trade amounts need to be tiny in order to test it. I observed that even with high slippage they won't trade, likely because the trades are too small to be considered by any solver.