Skip to content
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

Merged
merged 16 commits into from
Aug 19, 2023

Conversation

woodenfurniture
Copy link
Member

@woodenfurniture woodenfurniture commented Aug 17, 2023

Description

Upgrades swappers to return mutliple quotes if there are any:

  • Thorchain: return regular quote and streaming quote
  • Lifi: return the first 3 quotes available (discard the rest)

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

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

closes #5090

Risk

  • risk of broken quotes and incorrect information displayed against them
  • risk of incorrect route being executed (e.g select thorchain streaming swap and regular one gets sent)
  • risk of incorrect values in trades
  • risk of broken quote UI generally

Testing

  • Test all swappers are working as normal
  • Test streaming swaps execution in great detail - note, the quote itself is not using streaming swaps data, meaning it looks as if streaming swaps are the same as current swaps. This will be tackled in a direct follow-up
  • Test alternative lifi routes and check the correct tool (e.g lifi, startgate, 1inch via lifi) is used, check correct gas and amounts were executed

Engineering

Please assist by performing in-depth testing of this, especially streaming swaps.

Operations

See testing notes :)

Screenshots (if applicable)

UI changes:

image
image

Streaming swap on thor swapper

image
image
image
image

Executing the first lifi trade (of multiple)

Trade executed on 1inch as requested:
https://optimistic.etherscan.io/tx/0xb8ddbd6fc6101778c6c8018a257fe4b7f6d8a407d4cb1fcabe82099ef411aa74
image
image
image
image

Excecuting another lifi trade in the list (of multiple)

Trade executed on 0x as requested:
https://optimistic.etherscan.io/tx/0x3cb044c0f103828ee676039432a1f2d549c9320a035af33c1c541adc591fc32a
image
image
image
image

@woodenfurniture
Copy link
Member Author

woodenfurniture commented Aug 17, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

Copy link
Contributor

@gomesalexandre gomesalexandre left a 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

Base automatically changed from 08-17-chore_remove_manual_memo_calcs to develop August 18, 2023 11:59
Copy link
Contributor

@gomesalexandre gomesalexandre left a 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
Screenshot 2023-08-18 at 18 36 50 Screenshot 2023-08-18 at 18 36 59
  • Streaming memo is used

https://live.blockcypher.com/doge/tx/d8499bf6933dcbae22367b228506780c5f76057bda34d4efb9c8a54dd5e36537/

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:

image image
  • Trade goes through
image
  • Selecting the regular non-streaming option shows no regressions
image image image

Copy link
Contributor

@gomesalexandre gomesalexandre left a 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:
image image
  • Trades are going through and the selected tool is displayed at confirm step
image
  • The correct tool is used
image
  • Selecting an alternative tool works similarly
image image image image

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final pass, testing remaining swappers:

0x

image image image image

Osmosis

image image image

1inch - wasn't able to get any quote because of 301s - will tackle in a follow-up

image

get in :fridaydoge:

@woodenfurniture woodenfurniture enabled auto-merge (squash) August 19, 2023 10:49
@woodenfurniture woodenfurniture merged commit badb5c7 into develop Aug 19, 2023
3 checks passed
@woodenfurniture woodenfurniture deleted the streaming-swaps branch August 19, 2023 10:53
This was referenced Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Thorchain streaming swaps
2 participants