-
Notifications
You must be signed in to change notification settings - Fork 101
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(swap): fix swap out of market #3576
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
// Don't quote if there's nothing left to match in this order | ||
if (amount === '0') return null |
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.
Bonus: fix for this issue I observed while testing.
The order would match but still be in the pending list.
This prevent from quoting it and getting unnecessary 400s.
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.
Cool, thanks
appData: order.appData ?? undefined, | ||
appDataHash: order.appDataHash ?? undefined, |
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.
Bonus: pass the original appData so we can get more precise quotes.
It could have for example the permit hook, which affects the fee price.
const pendingLimit = useOnlyPendingOrders(chainId, UiOrderType.LIMIT) | ||
const pendingTwap = useOnlyPendingOrders(chainId, UiOrderType.TWAP) | ||
const pending = useMemo(() => pendingLimit.concat(pendingTwap), [pendingLimit, pendingTwap]) | ||
const pending = useOnlyPendingOrders(chainId) |
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.
We stopped checking for SWAPs since the preparation for TWAP.
This change checks for all order types once again, since we need the indication for all pending orders.
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.
Code looks great. It would be nice if someone, like Elena can test it
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.
Great, works!
82d5bab
to
7c03e7e
Compare
Summary
Fixes #3561
Swap orders should show out of the market again.
To Test
Check the instructions on #3561