-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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(21895): stop polling token price request from coingecko #22123
Conversation
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. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #22123 +/- ##
===========================================
+ Coverage 67.59% 67.60% +0.01%
===========================================
Files 1053 1053
Lines 40810 40804 -6
Branches 10941 10939 -2
===========================================
Hits 27584 27584
+ Misses 13226 13220 -6 ☔ View full report in Codecov by Sentry. |
## **Description** Cherry-pick features to stop pulling after UI is closed #22123 and add headers to request going to CoinGecko #22151 See more info here: https://consensys.slack.com/archives/C065W3877E3/p1701450197502839 <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Related issues** Fixes: ## **Manual testing steps** Refer to #22123 and #22151 ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
Description
There's a request to coingecko after extension is closed starting as
https://api.coingecko.com/api/v3/simple/token_price/
, and this is dispatched whenTracing back to the request , it comes from
const prices = await this.fetchExchangeRate(slug, nativeCurrency);
insideTokenRatesController
, which is called fromupdateExchangeRates
, which will be polled with an interval of 3 mins. However, this polling requestawait safelyExecute(() => this.updateExchangeRates());
is not stopped when UI is closed as it is not found instopNetworkRequests
.Hence the fix will be stopped the controller
TokenRatesController
which UI is closed, with the condition ofuseTokenDetection
is enabled.Related issues
Fixes: #21895
Manual testing steps
coingecko
any moreScreenshots/Recordings
Before
After
fetch-fix.mp4
Pre-merge author checklist
Pre-merge reviewer checklist