-
Notifications
You must be signed in to change notification settings - Fork 177
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: upgrade swappers to support returning multiple quotes #5095
Conversation
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
796c336
to
af7cd77
Compare
ff5533f
to
c328f23
Compare
af7cd77
to
e00ed08
Compare
77fdf63
to
b3ca2fa
Compare
9599b3f
to
e8d3737
Compare
e8d3737
to
99fae41
Compare
src/components/MultiHopTrade/components/TradeInput/components/TradeQuotes/TradeQuote.tsx
Show resolved
Hide resolved
src/lib/swapper/swappers/ThorchainSwapper/utils/makeSwapMemo/assertIsValidMemo.ts
Show resolved
Hide resolved
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.
First reviewy pass, looks excellent overall 🐐
Will continue with a second functional pass after the PR under this one is in, and after tackling those two PRs discussed in DM:
1. need to calculate the number of swaps instead of hardcoded 10
2. need to store isStreaming flag somewhere on the thor quote so we dont use my hacky .endsWith on the quote source
9b09e6c
to
7a67469
Compare
7a67469
to
630995b
Compare
src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts
Show resolved
Hide resolved
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.
THOR / streaming swaps pass.
- Streaming is shown as option / selected swapper
- Streaming memo is used
i.e 137633406 RUNE in base unit limit, 10 blocks streaming, number of swaps automatically determined by the network
Note for a follow-up PR: streaming swaps isn't available here given the smallish amount, so we should make sure to only programmatically display them when available, see THORSwap with a similar quote:
- Trade goes through
- Selecting the regular non-streaming option shows no regressions
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.
Li.Fi multiple routes pass:
- Multiple routes (3 currently) are displayed:
- NOK - quotes aren't consistently displaying the underlying tool, see https://github.com/shapeshift/web/pull/5095/files#r1298680978 for context - fixed in 6752a7b
- Trades are going through and the selected tool is displayed at confirm step
- The correct tool is used
- Selecting an alternative tool works similarly
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.
c51714b
to
80e2ff3
Compare
Description
Upgrades swappers to return mutliple quotes if there are any:
The UI will display the quotes with a label to indicate a difference between them.
Note: information about thorchain streaming swaps for users will be added in a follow-up PR and is out of scope for the first release.
Pull Request Type
Issue (if applicable)
closes #5090
Risk
Testing
Engineering
Please assist by performing in-depth testing of this, especially streaming swaps.
Operations
See testing notes :)
Screenshots (if applicable)
UI changes:
Streaming swap on thor swapper
Executing the first lifi trade (of multiple)
Trade executed on 1inch as requested:
https://optimistic.etherscan.io/tx/0xb8ddbd6fc6101778c6c8018a257fe4b7f6d8a407d4cb1fcabe82099ef411aa74
Excecuting another lifi trade in the list (of multiple)
Trade executed on 0x as requested:
https://optimistic.etherscan.io/tx/0x3cb044c0f103828ee676039432a1f2d549c9320a035af33c1c541adc591fc32a