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

Fix 10010 - Fetch swap quote refresh time from API #10017

Closed
wants to merge 1 commit into from

Conversation

darkwing
Copy link
Contributor

@darkwing darkwing commented Dec 8, 2020

Fixes: #10010

The MetaSwap team has implemented a new endpoint which tells us how often quotes should refresh. We were hardcoding 60 seconds as the refresh value, but moving that value to the MetaSwap API would prevent the MetaMask team from needing to create and deploy a new release in each browser to adjust that value.

As to why I've gone this route to accomplish this functionality, I like this strategy because we aren't pinging MetaSwap until we absolutely need to (i.e. when we're fetching quotes to display). We also benefit from fetchWithCache caching that value, so that we aren't pinging MetaSwap each time. @cloudonshore had mentioned this value wouldn't often change, so a fetch with cache only when we need that value should make us performant.

@darkwing darkwing requested a review from a team as a code owner December 8, 2020 18:50
@darkwing darkwing requested a review from brad-decker December 8, 2020 18:50
@darkwing darkwing marked this pull request as draft December 8, 2020 18:50
@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2020

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@darkwing darkwing force-pushed the 10010-api-refresh-time branch from 313ff30 to 71d9290 Compare December 9, 2020 17:55
@darkwing darkwing force-pushed the 10010-api-refresh-time branch from 71d9290 to a734732 Compare December 9, 2020 18:10
@darkwing darkwing requested review from danjm and brad-decker and removed request for brad-decker December 10, 2020 16:10
@darkwing darkwing marked this pull request as ready for review December 10, 2020 18:16
@brad-decker brad-decker self-assigned this Dec 10, 2020
Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

this looks good to me!

Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

There is also some code in the swaps controller related to the refresh time that is currently hard coded. Look at the variable called QUOTE_POLLING_INTERVAL. It is set to 10 seconds less than the quote refresh time. So either we need to manage state for this in the background controller, or pass the value retrieved from the api to the fetchAndSetQuotes method.

@darkwing darkwing closed this Dec 14, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fetch swaps quote refresh time from the API
3 participants