Add fast price fetching #2477

nenadV91 · 2022-02-19T15:59:02Z

Summary

Fixes #2437

This will create a quote request to the fast API endpoint parallel to the current best quote request, and its going to be resolved faster and display the quote results in the UI also faster. Once the best quote request is also resolved it will update the quote data with those results.
It will also add separate quote loader that will be displayed when new quote request is made and it will hide when the best quote request is finished (This is just a placeholder, Michel could take a look at this)

github-actions · 2022-02-19T15:59:16Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2022-02-19T16:09:58Z

🔭 GP Swap: CoW Protocol v2 Swap UI

W3stside · 2022-02-21T15:11:53Z

as we discussed in the call, please revert the new loader SVG and use the nose picker default. if you'd like, open a new PR with the new loader

elena-zh · 2022-02-22T11:58:55Z

@nenadV91 , changes LGTM!
Agree not to remove "cow picking nose" icon when the best price is calculated.

alfetopito

Seems to work fine, not as fast as expected though.

Also, please keep the nose picking cow 🥺

nenadV91 · 2022-02-24T18:24:56Z

@alfetopito Updated with the cow gif

anxolin

In principle works good, but im a bit concern with racing conditions.

Also, @biocom do you think we could experiment and add a different loader for the first loading?
I'd love that we show the logos of 1inch, paraswap, uni, sushi, etc making the impression we are FASTLY checking on those

anxolin · 2022-02-25T12:31:49Z

src/custom/components/ArrowWrapperLoader/index.tsx

@@ -63,7 +65,7 @@ export const Wrapper = styled.div<ShowLoaderProp>`
  > div > img {
    height: 100%;
    width: 100%;
-    padding: 2px 2px 0;
+    padding: ${({ noPadding }) => (noPadding ? `0px` : `2px 2px 0`)};


curious why we need this? u end up leaving the nose picking GIF right?

Good catch, reverted this also

anxolin · 2022-02-25T12:47:59Z

src/custom/hooks/useRefetchPriceCallback.tsx

+      // Get the best quote
+      getBestQuoteResolveOnlyLastCall(bestQuoteParams)
+        .then((res) => handleResponse(res, true))
+        .catch(handleError)


Can there be a race condition?

getFastQuoteResolveOnlyLastCall is sent in parallel to getBestQuoteResolveOnlyLastCall. What happens if getBestQuoteResolveOnlyLastCall finish before the fast one? Wouldn't we update using the wrong price in this case?

It would, the solution for this is in latest commit and I tested it in a way by using setTimeout on fast price update action. And the update will check inside of update action code, if there is already a quote price amount, which means that the best quote request was already done and if the current action params are bestQuote update or not.

anxolin · 2022-02-25T12:49:23Z

src/custom/hooks/useRefetchPriceCallback.tsx

@@ -202,7 +190,9 @@ export function useRefetchQuoteCallback() {

        // Update quote
        updateQuote(quoteData)


Do we ALWAY update the price? no matter if its worse or better?

Addressed in the comment up ☝️

elena-zh

LGTM now!

fairlighteth · 2022-02-28T10:13:46Z

@nenadV91 My first impression in terms of faster price loading, is positive. To me it feels I quickly get feedback in terms of seeing a quoted price. Here some comments I would consider:

When a price is being fetched and keeps on fetching, it would be good to have the shimmer effect also on the Receive (incl. fee) section. A strong indicator that the receive amount might be subject to an update and is not neccesarily final.

Screen.Recording.2022-02-28.at.10.04.45.mov

Perhaps out of scope, but once a price has been fetched, the section below the swap container is created and shown (Price, fees..). This container is removed/added quickly whenever you change an input amount. We could consider keeping this section consistent, beyond a first price being fetched. And use the 'shimmer' effects instead on it:

Screen.Recording.2022-02-28.at.10.10.12.mov

This prevents the feeling of 'flashing' content. A similar approach can be seen on 1inch.

@anxolin

Also, @biocom do you think we could experiment and add a different loader for the first loading?
I'd love that we show the logos of 1inch, paraswap, uni, sushi, etc making the impression we are FASTLY checking on those

Yes we definitely could. I would suggest to keep it out of this PR and try to address this separately. At first glance it seems more involved, in terms of what it takes to add a different loader visually to the UI.

anxolin · 2022-02-28T13:32:40Z

Im also PRO shimmer effect and keeping the container. Also agree we should do it in another PR

anxolin

Soft Approve. Looks ok, but i have some small nitpick comments.

Also, another comment that probably don't need to be addressed in this PR (and maybe we can get by for now not addressing it, if this solution is something we like and the other is hard to implement). However, I believe this solution is not super general. My first idea was to evolve the logic we have of making this price competition, see:

_extractPriceAndErrorPromiseValues

My thinking was, that we only need to update on every new price, and the update logic would only improve the price (never makes the price worse).
This would be more general, so we could launch the call to ANY number of services and update as the prices are coming back, so we don't differentiate between FAST and GOOD api

anxolin · 2022-02-28T13:44:47Z

src/custom/state/price/reducer.ts

+      // Flag to not update the quote when the there is already a quote price and the
+      // current quote in action is not the best quote, meaning the best quote for
+      // some reason was already loaded before fast quote and we want to keep best quote data
+      const shouldNotUpdate = quote && quote[sellToken]?.price?.amount && !isBestQuote


Bad name, NOT in a var name makes it hard to read. Try to read:!shouldNotUpdate

can u also give a better name to quote[sellToken]?.price?.amount, like:
const hadPrice = !!quote[sellToken]?.price?.amount

also for readability

anxolin · 2022-02-28T13:47:14Z

src/custom/state/price/reducer.ts

+      // Flag to not update the quote when the there is already a quote price and the
+      // current quote in action is not the best quote, meaning the best quote for
+      // some reason was already loaded before fast quote and we want to keep best quote data
+      const shouldNotUpdate = quote && quote[sellToken]?.price?.amount && !isBestQuote


Also, nitpick. Move the !isBestQuote to the first condition, so its evaluated first in short-circuit

Add fast price fetching

16c42f0

nenadV91 added 2 commits February 19, 2022 19:54

Add best quote loader

a95eec6

Loader style update

ad7213c

nenadV91 marked this pull request as ready for review February 19, 2022 19:25

nenadV91 requested review from a team February 19, 2022 19:25

alfetopito reviewed Feb 23, 2022

View reviewed changes

Change quote loader to old cow loader

e1d6fad

nenadV91 requested a review from alfetopito February 24, 2022 18:25

anxolin reviewed Feb 25, 2022

View reviewed changes

elena-zh approved these changes Feb 25, 2022

View reviewed changes

Fix for potential race condition issue

36524aa

elena-zh mentioned this pull request Feb 28, 2022

Show swap container at the top of the page #2505

Closed

anxolin approved these changes Feb 28, 2022

View reviewed changes

nenadV91 added 2 commits February 28, 2022 16:20

PR updates

7fc7885

Merge branch 'develop' into 2437/fast-quote-price

c4f8610

nenadV91 merged commit 588ccf2 into develop Feb 28, 2022

github-actions bot locked and limited conversation to collaborators Feb 28, 2022

alfetopito deleted the 2437/fast-quote-price branch March 2, 2022 19:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fast price fetching #2477

Add fast price fetching #2477

nenadV91 commented Feb 19, 2022 •

edited

Loading

github-actions bot commented Feb 19, 2022

github-actions bot commented Feb 19, 2022

W3stside commented Feb 21, 2022

elena-zh commented Feb 22, 2022

alfetopito left a comment

nenadV91 commented Feb 24, 2022

anxolin left a comment

anxolin Feb 25, 2022

nenadV91 Feb 25, 2022

anxolin Feb 25, 2022

nenadV91 Feb 25, 2022

anxolin Feb 25, 2022

nenadV91 Feb 25, 2022 •

edited

Loading

elena-zh left a comment

fairlighteth commented Feb 28, 2022

anxolin commented Feb 28, 2022

anxolin left a comment

anxolin Feb 28, 2022

anxolin Feb 28, 2022

anxolin Feb 28, 2022

		@@ -202,7 +190,9 @@ export function useRefetchQuoteCallback() {

		// Update quote
		updateQuote(quoteData)

Add fast price fetching #2477

Add fast price fetching #2477

Conversation

nenadV91 commented Feb 19, 2022 • edited Loading

Summary

github-actions bot commented Feb 19, 2022

github-actions bot commented Feb 19, 2022

W3stside commented Feb 21, 2022

elena-zh commented Feb 22, 2022

alfetopito left a comment

Choose a reason for hiding this comment

nenadV91 commented Feb 24, 2022

anxolin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nenadV91 Feb 25, 2022 • edited Loading

Choose a reason for hiding this comment

elena-zh left a comment

Choose a reason for hiding this comment

fairlighteth commented Feb 28, 2022

anxolin commented Feb 28, 2022

anxolin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nenadV91 commented Feb 19, 2022 •

edited

Loading

nenadV91 Feb 25, 2022 •

edited

Loading