feat: thorchain streaming swap progress bar #5192

woodenfurniture · 2023-08-30T23:38:54Z

Description

This adds a progress bar to thorchain streaming swaps.

Includes progress bar and ability to convey the concept of partial fulfilment with reasons for each failure - a value add over viewblock and other interfaces.

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 #5191

Risk

Risk of broken trade confirmation UI

Testing

Test streaming swaps and ensure progress bar appears and works as expected
Test non-streaming swaps and ensure progress bar does not appear

Engineering

☝️

Operations

☝️

Screenshots (if applicable)

Before confirmation

On confirmation

On streaming data loaded

On send tx confirmed

Progress increasing, matches viewblock

Streaming complete

woodenfurniture · 2023-08-30T23:39:05Z

Current dependencies on/for this PR:

develop
- PR feat: thorchain streaming swap progress bar #5192 👈

This comment was auto-generated by Graphite.

0xApotheosis

Code-wise looking good, will do a functional pass now.

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx

0xApotheosis

Super cool feature, though I did end up in this state on a small swap:

The stream status never moved on from loading. It might be because the trade was so small it complete in one step?

woodenfurniture · 2023-09-01T02:49:44Z

Super cool feature, though I did end up in this state on a small swap:
The stream status never moved on from loading. It might be because the trade was so small it complete in one step?

This is intended, the transaction succeeds before the streaming completes. Likely you didnt see the progress bar initialise because it takes about 3-5 mins to pick up the tx from the chain.

If the streaming completes before it's able to see the tx it will stay in that indeterminate loading state, but that isnt possible to work around without wiring this to a websocket watching the receiving chain. I think that should be ok to leave as-is because the user will see the funds, and the progress bar doesnt explicitly say "0%" in this case

I do think we will need to modify the tx status to not say "completed" while it's still streaming, but I hope we can do that in a follow-up because I'm working on changes that will conflict very heavily with that :P

src/assets/translations/en/main.json

gomesalexandre · 2023-09-01T09:41:02Z

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx

+      isAnimated: !isComplete,
+      colorScheme: isComplete ? 'green' : 'blue',
+    },
+    attemptedSwaps: count ?? 0,


Wondering if this is the best vernacular, these were not attempted but actually completed internal swaps?

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx

gomesalexandre · 2023-09-01T09:57:05Z

The stream status never moved on from loading. It might be because the trade was so small it complete in one step?

Tangentially, is this monkey patched? Such a small swap should obviously not show up as a streaming swap option

gomesalexandre · 2023-09-01T10:26:34Z

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx

+  quantity?: number
+  count?: number


These seem undefined for non-streaming swaps, can we perhaps use them as discriminator on whether or not to use the progress bar / "Loading" copy?

Waited a few minutes to ensure the Tx status is properly picked by the THOR node and still got the same response.

This might be an edge case, but may be nice to tackle regardless 🙏🏽

In this case I erred on the side of safety and used the schema from the swagger docs to define them as optional or required, and it seems there are no required fields defined for this response.

https://daemon.thorchain.shapeshift.com/lcd/thorchain/doc

gomesalexandre

Tested locally, LGTM minus a few naming/types comments and a possible improvement of the flow. Didn't test the actual progress bar with a large swap because something something testing funds, will leave this for ops testing.

Non-streaming swap

Such regular swaps correctly have no "Loading" nor progress bar

Streaming swaps

Tested with the following monkey-patch

diff --git a/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts b/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts
index a2e8e57910..708f7d8d21 100644
--- a/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts
+++ b/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts
@@ -160,9 +160,7 @@ export const getThorTradeQuote = async (
 
   const perRouteValues = [getRouteValues(swapQuote, false)]
 
-  streamingSwapQuote &&
-    swapQuote.expected_amount_out !== streamingSwapQuote.expected_amount_out &&
-    perRouteValues.push(getRouteValues(streamingSwapQuote, true))
+  streamingSwapQuote && perRouteValues.push(getRouteValues(streamingSwapQuote, true))
 
   const getRouteRate = (expectedAmountOutThorBaseUnit: string) => {
     const THOR_PRECISION = 8

Can confirm the flow stated by @0xApotheosis above, though I believe this to be an inherent issue of testing this with a monkey-patched streaming swaps for small amounts that aren't actually streaming. Added a possible fix for it in https://github.com/shapeshift/web/pull/5192/files#r1312857963

0xApotheosis · 2023-09-01T19:20:47Z

The stream status never moved on from loading. It might be because the trade was so small it complete in one step?

Tangentially, is this monkey patched? Such a small swap should obviously not show up as a streaming swap option

Yep, she was indeed monkey patched!

0xApotheosis

Approved as the current issues are intended. Address the suggestions at your discretion.

…rnode

woodenfurniture · 2023-09-03T21:32:55Z

Tested locally, LGTM minus a few naming/types comments and a possible improvement of the flow. Didn't test the actual progress bar with a large swap because something something testing funds, will leave this for ops testing.

Non-streaming swap

Such regular swaps correctly have no "Loading" nor progress bar

Streaming swaps

Tested with the following monkey-patch
diff --git a/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts b/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts
index a2e8e57910..708f7d8d21 100644
--- a/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts
+++ b/src/lib/swapper/swappers/ThorchainSwapper/getThorTradeQuote/getTradeQuote.ts
@@ -160,9 +160,7 @@ export const getThorTradeQuote = async (
 
   const perRouteValues = [getRouteValues(swapQuote, false)]
 
-  streamingSwapQuote &&
-    swapQuote.expected_amount_out !== streamingSwapQuote.expected_amount_out &&
-    perRouteValues.push(getRouteValues(streamingSwapQuote, true))
+  streamingSwapQuote && perRouteValues.push(getRouteValues(streamingSwapQuote, true))
 
   const getRouteRate = (expectedAmountOutThorBaseUnit: string) => {
     const THOR_PRECISION = 8
Can confirm the flow stated by @0xApotheosis above, though I believe this to be an inherent issue of testing this with a monkey-patched streaming swaps for small amounts that aren't actually streaming. Added a possible fix for it in #5192 (files)

For reference some of the quirks are noted here for fix in follow-up 🙂
#5192 (comment)

woodenfurniture force-pushed the streaming-swap-progress-bar branch from eba708a to 027c0f7 Compare September 1, 2023 00:31

woodenfurniture changed the title ~~feat: initial implementation of streaming progress bar~~ feat: thorchain streaming swap progress bar Sep 1, 2023

woodenfurniture marked this pull request as ready for review September 1, 2023 01:55

woodenfurniture requested a review from a team as a code owner September 1, 2023 01:55

0xApotheosis reviewed Sep 1, 2023

View reviewed changes

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx Outdated Show resolved Hide resolved

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx Outdated Show resolved Hide resolved

0xApotheosis requested changes Sep 1, 2023

View reviewed changes

gomesalexandre reviewed Sep 1, 2023

View reviewed changes

src/assets/translations/en/main.json Outdated Show resolved Hide resolved

gomesalexandre reviewed Sep 1, 2023

View reviewed changes

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx Outdated Show resolved Hide resolved

gomesalexandre reviewed Sep 1, 2023

View reviewed changes

src/components/MultiHopTrade/hooks/useThorStreamingProgress/useThorStreamingProgress.tsx Show resolved Hide resolved

gomesalexandre reviewed Sep 1, 2023

View reviewed changes

gomesalexandre approved these changes Sep 1, 2023

View reviewed changes

0xApotheosis approved these changes Sep 1, 2023

View reviewed changes

woodenfurniture and others added 5 commits September 4, 2023 07:09

feat: initial implementation of streaming progress bar

ece973c

update layout

293b584

chore: remove monkey patch and comment out learn more button

1918781

fix: dont show progress bar until a valid result is received from tho…

bc74be1

…rnode

fix: everything

8bdea1e

chore: action code review feedback

6fd8856

woodenfurniture force-pushed the streaming-swap-progress-bar branch from 1cd9b07 to 6fd8856 Compare September 3, 2023 21:34

woodenfurniture enabled auto-merge (squash) September 3, 2023 21:35

0xApotheosis disabled auto-merge September 3, 2023 22:43

0xApotheosis merged commit e342faf into develop Sep 3, 2023
3 checks passed

0xApotheosis deleted the streaming-swap-progress-bar branch September 3, 2023 22:44

0xApotheosis mentioned this pull request Sep 3, 2023

chore: release v1.425.0 #5207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: thorchain streaming swap progress bar #5192

feat: thorchain streaming swap progress bar #5192

woodenfurniture commented Aug 30, 2023 •

edited

Loading

woodenfurniture commented Aug 30, 2023

0xApotheosis left a comment

0xApotheosis left a comment

woodenfurniture commented Sep 1, 2023 •

edited

Loading

gomesalexandre Sep 1, 2023

gomesalexandre commented Sep 1, 2023

gomesalexandre Sep 1, 2023

woodenfurniture Sep 3, 2023 •

edited

Loading

gomesalexandre left a comment

0xApotheosis commented Sep 1, 2023

0xApotheosis left a comment

woodenfurniture commented Sep 3, 2023

Non-streaming swap

Streaming swaps

feat: thorchain streaming swap progress bar #5192

feat: thorchain streaming swap progress bar #5192

Conversation

woodenfurniture commented Aug 30, 2023 • edited Loading

Description

Pull Request Type

Issue (if applicable)

Risk

Testing

Engineering

Operations

Screenshots (if applicable)

woodenfurniture commented Aug 30, 2023

0xApotheosis left a comment

Choose a reason for hiding this comment

0xApotheosis left a comment

Choose a reason for hiding this comment

woodenfurniture commented Sep 1, 2023 • edited Loading

gomesalexandre Sep 1, 2023

Choose a reason for hiding this comment

gomesalexandre commented Sep 1, 2023

gomesalexandre Sep 1, 2023

Choose a reason for hiding this comment

woodenfurniture Sep 3, 2023 • edited Loading

Choose a reason for hiding this comment

gomesalexandre left a comment

Choose a reason for hiding this comment

Non-streaming swap

Streaming swaps

0xApotheosis commented Sep 1, 2023

0xApotheosis left a comment

Choose a reason for hiding this comment

woodenfurniture commented Sep 3, 2023

Non-streaming swap

Streaming swaps

woodenfurniture commented Aug 30, 2023 •

edited

Loading

woodenfurniture commented Sep 1, 2023 •

edited

Loading

woodenfurniture Sep 3, 2023 •

edited

Loading