-
Notifications
You must be signed in to change notification settings - Fork 371
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
Allowed Slippage for limit swaps #1451
base: release-53
Are you sure you want to change the base?
Conversation
hubert-de-lalye
commented
Dec 12, 2024
•
edited by xhiroz
Loading
edited by xhiroz
- To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1208764027793564
✅ Deploy Preview for vigilant-albattani-df38ec ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deploying gmx-interface with Cloudflare Pages
|
…ed slippage on token input change
616045d
to
13e8a75
Compare
swapPath = allPaths[0].path; | ||
const sortedPaths = opts.order | ||
? [...allPaths].sort((a, b) => { | ||
for (const field of opts.order ?? []) { |
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.
oh, maybe create different sort functions for sort each strategy?
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 moved this to separated getSwapPathComparator
and added unit tests for it, it's a generic function and strategy passed as priority list of liquidity and/or path length
initialSwapImpactFeeBps?: bigint; | ||
setAllowedSwapSlippageBps: (value: bigint) => void; | ||
totalSwapImpactBps: bigint; | ||
notAvailable?: boolean; |
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.
not critial, but, probably rename it to “disabled”? would be a more familiar name
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.
this inherited from AcceptableSwapImpact component, and I think naming is ok cuz it will render N/A, it's differs from just disabled state