-
Notifications
You must be signed in to change notification settings - Fork 89
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): small order slippage v2 #4934
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
apps/cowswap-frontend/src/modules/swap/updaters/SmartSlippageUpdater.ts
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/modules/swap/pure/Row/RowSlippageContent/index.tsx
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/modules/swap/updaters/SmartSlippageUpdater.ts
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/modules/swap/updaters/SmartSlippageUpdater.ts
Outdated
Show resolved
Hide resolved
Hey @alfetopito , I have some questions to clarify/suggestions:
|
c08b061
to
bfb70b9
Compare
Hey @alfetopito , it works :) Also I've noticed that when: |
0e02672
to
f57253c
Compare
Already explained over the call.
Doesn't it match the calculations in the spreadsheet? I mean, apart from BFF, you can disregard that.
Good point, will fix it
It's ok, as it comes from different sources
Added the limitation when both are added together to not go over 50%. |
Targeting off => serve 0
The slippage was not being reset when both were null and there was a previously set value. |
@elena-zh point 3 from the initial comment is fixed 🤞 |
|
||
import { calculateBpsFromFeeMultiplier } from './calculateBpsFromFeeMultiplier' | ||
|
||
import { useReceiveAmountInfo } from '../../../trade' |
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.
import { useReceiveAmountInfo } from '../../../trade' | |
import { useReceiveAmountInfo } from 'modules/trade' |
import { useDerivedTradeState, useIsWrapOrUnwrap } from 'modules/trade' | ||
|
||
import { smartSwapSlippageAtom } from '../state/slippageValueAndTypeAtom' | ||
import { useDerivedTradeState, useIsWrapOrUnwrap } from '../../../trade' |
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.
import { useDerivedTradeState, useIsWrapOrUnwrap } from '../../../trade' | |
import { useDerivedTradeState, useIsWrapOrUnwrap } from 'modules/trade' |
* More relevant for small orders in relation to fee amount, negligent for larger orders. | ||
*/ | ||
export function useSmartSlippageFromFeeMultiplier(): number | undefined { | ||
const { beforeNetworkCosts, afterNetworkCosts, costs, isSell } = useReceiveAmountInfo() || {} |
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.
Awesome! Thanks for the using useReceiveAmountInfo
!
Update 2024/10/03
Added feature flag
smartSlippageFeeMultiplierPercentage
, that returns the multiplier value and can still be used to feature flag the individual feature of small orders slippage.Also, slippage is now capped at 50%.
New testing steps
smartSlippageFeeMultiplierPercentage
and disableisSmartSlippageEnabled
5000
One more fun test is to add/pick new values to be served when on for the new feature flag:
And then observe the slippage change accordingly in the UI.
Summary
Supersedes #4911
Instead of using the fee % in relation to order size, calculate how much slippage is needed if the fee is 50% higher.
This value is added to the one returned by BFF.
To Test
You can also observe all the smart slippage values by filtering the console logs by
SmartSlippage