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.

Sign up for GitHub

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

Jump to bottom

Block and eth_call balance polling initiated by the account tracker continue after UI is closed #25951

Closed
danjm opened this issue Jul 18, 2024 · 2 comments
Closed

Block and eth_call balance polling initiated by the account tracker continue after UI is closed #25951

danjm opened this issue Jul 18, 2024 · 2 comments
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. regression-beta-12.0.0 Regression bug that was found in beta in release 12.0.0 release-blocker This bug is blocking the next release Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-extension-platform type-bug

Comments

@danjm
Copy link
Contributor

danjm commented Jul 18, 2024 •
edited
Loading

A bug was observed on testing v12.0.0 with the immediate symptom of:

On the 12.0.0 beta release, there is a periodic polling task that seems to be running every 20 seconds (including when the app is closed) that generates 1 API request to Etherscan and 3 to RPC provider

This results in many more requests going to the rpc provider than it should.

It turns out that some of those requests have different causes. This ticket is only concerned with the eth_call and eth_getblockbynumber requests that stem from the account tracker, and continue to occur after the UI is closed.

To repro:

  1. Build the v12.0.0 branch. A dist build gives the simplest repro
  2. Install and onboard
  3. Close the UI
  4. Open the service worker console and observe network requests to infura
@danjm danjm added type-bug release-blocker This bug is blocking the next release regression-beta-12.0.0 Regression bug that was found in beta in release 12.0.0 labels Jul 18, 2024
@danjm danjm changed the title Block and etherscan polling initiated by the account tracker continue after UI is closed Block and eth_call balance polling initiated by the account tracker continue after UI is closed Jul 18, 2024
@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Jul 18, 2024
@danjm danjm mentioned this issue Jul 22, 2024
Closed
@danjm
Copy link
Contributor Author

danjm commented Jul 22, 2024

This is partially addressed by #25909, but a deeper root cause needs to be addressed as well, which #26002 points at more directly

@gauthierpetetin gauthierpetetin added the Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. label Jul 25, 2024
@chloeYue chloeYue mentioned this issue Jul 30, 2024
Merged
7 tasks
chloeYue added a commit that referenced this issue Jul 30, 2024
@chloeYue @DDDDDanica
test: Add manual scenario for network polling scenario (#26195)
aceea3b 
## **Description**

This PR introduces a new test scenario which relates to a
release-blocker for v12.0.0. This scenario is to verify that once the
MetaMask UI is closed, the extension stops all network requests,
including `etherscan`, `eth_call`, `eth_getBalance`, `eth_blockNumber`,
and `eth_getBlockByNumber` to Infura or any other RPC provider. This
check is vital for reducing unnecessary network traffic and enhancing
both privacy and efficiency within the extension's operation.


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26195?quickstart=1)

## **Related issues**

Fixes: #25964
Related release blocker: #25951 

## **Manual testing steps**
Follow testing steps in this  manual scenario

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

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.

---------

Co-authored-by: Danica Shen <zhaodanica@gmail.com>
dawnseeker8 pushed a commit that referenced this issue Aug 12, 2024
@chloeYue @DDDDDanica @dawnseeker8
test: Add manual scenario for network polling scenario (#26195)
8871567 
## **Description**

This PR introduces a new test scenario which relates to a
release-blocker for v12.0.0. This scenario is to verify that once the
MetaMask UI is closed, the extension stops all network requests,
including `etherscan`, `eth_call`, `eth_getBalance`, `eth_blockNumber`,
and `eth_getBlockByNumber` to Infura or any other RPC provider. This
check is vital for reducing unnecessary network traffic and enhancing
both privacy and efficiency within the extension's operation.


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26195?quickstart=1)

## **Related issues**

Fixes: #25964
Related release blocker: #25951 

## **Manual testing steps**
Follow testing steps in this  manual scenario

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

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.

---------

Co-authored-by: Danica Shen <zhaodanica@gmail.com>
@danjm
Copy link
Contributor Author

danjm commented Aug 13, 2024

Fixed by #26141

@danjm danjm closed this as completed Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. regression-beta-12.0.0 Regression bug that was found in beta in release 12.0.0 release-blocker This bug is blocking the next release Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-extension-platform type-bug
Projects
Bugs by severity
Archived in project
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gauthierpetetin @danjm @metamaskbot @hjetpoluru