[Bug]: MM is sending RPC requests to another custom network, aside from the one that's currently selected

seaona commented 1 year ago

Describe the bug

Problem: after adding a custom localhost network, if I change to another network and terminate the ganache server for localhost, I still can see how requests are being send to the localhost custom network together with the requests to the currently selected network.

https://user-images.githubusercontent.com/54408225/209115431-80f25d71-ef80-4ffc-98d3-fa05be22e1a9.mp4

Steps to reproduce

[investigating]

Setup a ganache server with a different port number than the default ganache port 8546
On MM, go to Add Network
Add the new server pointing to localhost 8546
Change to another network
Terminate ganache server
Observe network requests on the background console -- requests to the localhost net are still being sent, alongside with requests to the current selected network

Error messages or log output

No response

Version

10.24.0

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

This duplicate ticket also provides steps to reproduce: https://github.com/MetaMask/metamask-extension/issues/23415

seaona commented 1 year ago

Added 10.24.0 to have eyes on it, but cannot confirm it's an RC regression. Issue is hard to repro

seaona commented 1 year ago

Another example here:

https://user-images.githubusercontent.com/54408225/224253719-d3ea254d-81fb-4aca-a706-bb6ef8683a58.mp4

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

github-actions[bot] commented 12 months ago

This issue was closed because there has been no follow up activity in the last 45 days. If you feel this was closed in error, please reopen and provide evidence on the latest release of the extension. Thank you for your contributions.

seaona commented 8 months ago

This issue is still happening. I just encountered it again in prod 11.10, having a Tenderly network selected, which was not working, then switching to Mainnet, and I still see failed requests being sent to Tenderly RPC

https://github.com/MetaMask/metamask-extension/assets/54408225/11e76f7f-287f-40ff-9374-926843b9f04c

gauthierpetetin commented 8 months ago

This duplicate ticket also provides steps to reproduce: https://github.com/MetaMask/metamask-extension/issues/23415

Gudahtt commented 5 months ago

This may be fixed by https://github.com/MetaMask/eth-block-tracker/pull/208, which is in eth-block-tracker@9.0.3

mcmire commented 5 months ago

I tested this and I wasn't able to replicate the behavior described here and in #23415, so it appears that the PR mentioned above did indeed improve the situation. There are some improvements we can make on top of this, such as:

Reducing the frequency that the "latest block" request is retried on an unavailable network. The fetch middleware, Infura middleware, and the PollingBlockTracker all have retry logic. For the fetch and Infura middleware, the retry interval is 1 second (see here and here); for the PollingBlockTracker it's the polling interval / 10 (see here). Therefore, if you switch to an unavailable network, inspect the background process, and switch to the Network tab, you will notice 5 retries within 5 seconds, then after a couple of seconds, 5 more retries within 5 seconds, etc. We can reduce the frequency of retries by passing both a pollingInterval and retryTimeout to PollingBlockTracker and making them the same.
Stopping the PollingBlockTracker after a while if the network is truly down.
Adding a timeout to getLatestBlock in PollingBlockTracker so that if the network is down, the promise that this method returns gets eventually rejected. Without this, if you are connected to an unavailable network and restart the extension, MetaMask will show a spinner forever, but with this timeout, MetaMask will eventually show a "could not connect" error and let you switch to an available network.

But, I think all of these problems are unrelated to this exact issue. Therefore, I am inclined to close this ticket. Any last thoughts before I do so?

mcmire commented 5 months ago

https://github.com/MetaMask/metamask-extension/commit/04c2e90094802b23b4265e2ad303978cd3e132c0 (from https://github.com/MetaMask/metamask-extension/pull/24861) is the commit that upgraded @metamask/network-controller to a new version that switched from eth-block-tracker to @metamask/eth-block-tracker, incorporating the fix made in https://github.com/MetaMask/eth-block-tracker/pull/208. I tested the commit before this and the commit after this, and confirmed that the problem went away with that fix to @metamask/eth-block-tracker.

Closing.

mcmire commented 5 months ago

I have confirmed that https://github.com/MetaMask/metamask-extension/pull/24861 will go into 12.0.0.

Gudahtt commented 4 months ago

Unfortunately I am still able to reproduce this bug even on v9.0.3 of eth-block-tracker. I was able to reproduce it using these steps:

Add BNB and switch to it
Block requests to that domain in the dev console
Rapidly switch back and forth between networks until I see it get into a failing loop

It's inconsistent, it can take many attempts. Usually I just see 3 failures after switching, and then it stops. But it can get stuck in an infinite loop of failures still.

desi commented 4 months ago

Note: For reference weekly checkin recording for WF team - time 9:25 discussion around this issue.

desi commented 4 months ago

@mikesposito - reference from Mark https://github.com/MetaMask/eth-block-tracker/issues/163

DDDDDanica commented 3 months ago

Update: this issue is still occurring on 11.16.15

https://github.com/user-attachments/assets/b6f73e2b-883f-4a59-a7f2-12b7875d8342

mikesposito commented 2 months ago

I'm unable to repro this on v12.1.2 - I'm trying to see if I can find the commit that potentially fixed it

EDIT: The issue is still present unfortunately, though it is not visible if background.html is used instead of a service worker (e.g. when running yarn webpack --watch

mikesposito commented 3 days ago

This PR represents the first step to fix this: https://github.com/MetaMask/eth-block-tracker/pull/284

MetaMask / metamask-extension