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

extension doesn't stop polling currency rates/token rates when the UI is closed #21895

Closed
danjm opened this issue Nov 20, 2023 · 0 comments · Fixed by #22123
Closed

extension doesn't stop polling currency rates/token rates when the UI is closed #21895

danjm opened this issue Nov 20, 2023 · 0 comments · Fixed by #22123
Assignees
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. release-11.8.0 Issue or pull request that will be included in release 11.8.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform type-bug

Comments

@danjm
Copy link
Contributor

danjm commented Nov 20, 2023

Extension doesn't stop polling currency rates/token rates when the UI is closed. all of our polling should be disabled when the UI is closed.

@danjm danjm added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform labels Nov 20, 2023
@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Nov 20, 2023
@DDDDDanica DDDDDanica self-assigned this Nov 30, 2023
DDDDDanica added a commit that referenced this issue Dec 4, 2023
## **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 when
- _add a new token_ when auto token detection is enabled + have tokens
in account
- _hard reload_ (reload from extensions management page) when auto token
detection is enabled + have tokens in account

Tracing back to the request , it comes from `const prices = await
this.fetchExchangeRate(slug, nativeCurrency);` inside
`TokenRatesController`, which is called from `updateExchangeRates`,
which will be polled with an interval of 3 mins. However, this polling
request `await safelyExecute(() => this.updateExchangeRates());` is not
stopped when UI is closed as it is not found in `stopNetworkRequests`.

Hence the fix will be stopped the controller `TokenRatesController`
which UI is closed, with the condition of `useTokenDetection` is
enabled.

## **Related issues**

Fixes: #21895

## **Manual testing steps**

1. Load MM and open backgound.html console
2. Enable auto token detection
3. Import 1 ~ many tokens
4. See in the network there is a request to get token prices similar as 
```
https://api.coingecko.com/api/v3/simple/token_price/ethereum?contract_addresses=0xC011a73ee8576Fb46F5E1c5751cA3B9Fe0af2a6F,0x7Fc66500c84A76Ad7e9c93437bFc5Ac33E2DDaE9,0xfF20817765cB7f73d4bde2e66e067E58D11095C2&vs_currencies=eth
```
5. Close MM extension
6. Waited for at least 3 mins
7. There is no request sent to `coingecko` any more

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**


https://github.com/MetaMask/metamask-extension/assets/12678455/610653a6-fd10-48cb-be83-891b1c172ac0


## **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.
@metamaskbot metamaskbot added the release-11.8.0 Issue or pull request that will be included in release 11.8.0 label Dec 4, 2023
DDDDDanica added a commit that referenced this issue Dec 5, 2023
## **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 when
- _add a new token_ when auto token detection is enabled + have tokens
in account
- _hard reload_ (reload from extensions management page) when auto token
detection is enabled + have tokens in account

Tracing back to the request , it comes from `const prices = await
this.fetchExchangeRate(slug, nativeCurrency);` inside
`TokenRatesController`, which is called from `updateExchangeRates`,
which will be polled with an interval of 3 mins. However, this polling
request `await safelyExecute(() => this.updateExchangeRates());` is not
stopped when UI is closed as it is not found in `stopNetworkRequests`.

Hence the fix will be stopped the controller `TokenRatesController`
which UI is closed, with the condition of `useTokenDetection` is
enabled.

## **Related issues**

Fixes: #21895

## **Manual testing steps**

1. Load MM and open backgound.html console
2. Enable auto token detection
3. Import 1 ~ many tokens
4. See in the network there is a request to get token prices similar as 
```
https://api.coingecko.com/api/v3/simple/token_price/ethereum?contract_addresses=0xC011a73ee8576Fb46F5E1c5751cA3B9Fe0af2a6F,0x7Fc66500c84A76Ad7e9c93437bFc5Ac33E2DDaE9,0xfF20817765cB7f73d4bde2e66e067E58D11095C2&vs_currencies=eth
```
5. Close MM extension
6. Waited for at least 3 mins
7. There is no request sent to `coingecko` any more

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**


https://github.com/MetaMask/metamask-extension/assets/12678455/610653a6-fd10-48cb-be83-891b1c172ac0


## **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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. release-11.8.0 Issue or pull request that will be included in release 11.8.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants