metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

Published 20 hours ago • verified publisher icon MetaMask

fix: UX: Multichain: Fix dead network problem when switching tabs

Open darkwing opened this issue 1 year ago • 6 comments

Description

This PR ensures that a connection to a given network does not block the UI when switching to a dapp tab that remembers that down network, thus preventing a non-respondent network from not displaying the MetaMask UI

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/metamask-extension/issues/25588

Manual testing steps

Scenario 1: Kill network, manually click MetaMask extension icon

  1. Start local ganache
  2. Add the local network to your network listing (Settings -> Networks -> Add custom network)
  3. In tab 1, connect to dapp 1 on Localhost
  4. In tab 2, connect to dapp 2 on Ethereum Mainnet
  5. Kill local ganache
  6. Click tab 1
  7. Click the MetaMask icon in the extension bar
  8. See UI pop up as expected

Scenario 2: Kill network, trigger transaction from dapp

  1. Start local ganache
  2. Add the local network to your network listing (Settings -> Networks -> Add custom network)
  3. In tab 1, connect to dapp 1 on Localhost
  4. In tab 2, connect to dapp 2 on Ethereum Mainnet
  5. Kill local ganache
  6. Click tab 1
  7. Trigger a transaction from the dapp
  8. See UI pop up quickly, with confirmation, as expected

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

darkwing avatar Jun 19 '24 15:06 darkwing

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.

github-actions[bot] avatar Jun 19 '24 15:06 github-actions[bot]

I noticed that this test was still taking a long time on Chrome so I did some more manual testing and it appears as though (assuming you kill local ganache):

  1. Click the send button in test dapp
  2. (random, sometimes long wait time)
  3. The confirmation eventually pops up

Spying on the service worker between clicking the send button and the confirmation finally coming up, it appears the confirmation doesn't appear until the following network request(s) complete:

  • https://tx-sentinel-ethereum-mainnet.api.cx.metamask.io/networks
  • https://gas.api.cx.metamask.io/networks/1337/suggestedGasFees
SCR-20240625-rfkn

darkwing avatar Jun 26 '24 00:06 darkwing

When trying to send a transaction on a network with dead RPC, I see 2 RPC calls that each take ~5 seconds to fail, which together cause the ~10 second delay in receiving the metamask popup:

  • Click send on test dapp
  • extension executes addTransactionWithController
  • Calls TransactionController.addTransaction
    • determineTransactionType runs the eth_getCode RPC query
    • updateGasProperties runs the eth_estimateGas RPC query

I don't know why the RPC queries take 5 seconds instead of failing fast. Perhaps somewhere lower level is doing retry. But that's the only code I see slowing down the popup.

https://github.com/MetaMask/metamask-extension/assets/3500406/ede4eeb9-bae9-4506-99aa-724a898317fa

bergeron avatar Jun 27 '24 21:06 bergeron

on the test environment I am seeing a great amount of requests for mostly eth_estimateGas, eth_gasPrice and eth_blockNumber. Those return very fast, but we keep making requests until ~10seconds.

https://github.com/MetaMask/metamask-extension/assets/54408225/b91d21a3-a57b-443c-b5b1-682c1fd3d50f

and on the dev build from this branch, I am seeing a similar behaviour as I see in production: sometimes the popup is open fast, but sometimes it takes ~5 seconds

https://github.com/MetaMask/metamask-extension/assets/54408225/d7106023-dc9d-4f1b-a890-443698fd38b3

seaona avatar Jun 28 '24 12:06 seaona

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 69.56%. Comparing base (4a9c480) to head (84786eb). Report is 25 commits behind head on develop.

Files Patch % Lines
ui/store/actions.ts 0.00% 4 Missing :warning:
app/scripts/metamask-controller.js 0.00% 2 Missing :warning:
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #25425      +/-   ##
===========================================
- Coverage    69.57%   69.56%   -0.01%     
===========================================
  Files         1360     1360              
  Lines        48172    48177       +5     
  Branches     13296    13296              
===========================================
  Hits         33513    33513              
- Misses       14659    14664       +5

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jun 28 '24 21:06 codecov[bot]

Builds ready [84786eb]
Page Load Metrics (263 ± 280 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint653551165928
domContentLoaded97530199
load412024263582280
domInteractive97530199
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 80 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 194 Bytes (0.00%)

metamaskbot avatar Jun 28 '24 21:06 metamaskbot

Missing release label release-12.0.0 on PR. Adding release label release-12.0.0 on PR and removing other release labels(release-12.2.0), as PR was cherry-picked in branch 12.0.0.

metamaskbot avatar Jul 09 '24 17:07 metamaskbot