metamask-extension Test RPC queue updates [WIP]

Description

Testing RPC queue refactor that batches request by domain.

This PR is used just to make it easier for me to review the diff, review CI results, and to get access to metamaskbot comment.

Related issues

Fixes:

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

[ ] I’ve followed MetaMask Coding Standards.
[ ] 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 format if applicable
[ ] I’ve applied the right labels on the PR (see labeling guidelines). 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.

Feb 08 '24 01:02 Gudahtt

@metamaskbot update-policies

Feb 14 '24 06:02 Gudahtt

Policies updated

Feb 14 '24 06:02 metamaskbot

Builds ready [5076901]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1261 ± 129 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	124	424	213	74	36
		domContentLoaded	21	166	67	35	17
		load	970	2164	1261	269	129
		domInteractive	21	166	67	35	17

Bundle size diffs [🚀 Bundle size reduced!]

background: -631 Bytes (-0.02%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

Feb 14 '24 06:02 metamaskbot

@metamaskbot update-policies

Feb 27 '24 19:02 Gudahtt

Policies updated

Feb 27 '24 19:02 metamaskbot

@metamaskbot update-policies

Feb 29 '24 15:02 Gudahtt

Policies updated

Feb 29 '24 15:02 metamaskbot

@metamaskbot update-policies

Feb 29 '24 17:02 Gudahtt

Policies updated

Feb 29 '24 17:02 metamaskbot

@metamaskbot update-policies

Mar 08 '24 22:03 Gudahtt

No policy changes

Mar 08 '24 22:03 metamaskbot

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/[email protected]	Transitive: filesystem	`+5`	2.78 MB	metamaskbot

🚮 Removed packages: npm/@metamask/[email protected]

View full report↗︎

Mar 12 '24 20:03 socket-security[bot]

@metamaskbot update-policies

Mar 13 '24 11:03 Gudahtt

Policies updated

Mar 13 '24 11:03 metamaskbot

@metamaskbot update-policies

Mar 13 '24 12:03 Gudahtt

No policy changes

Mar 13 '24 12:03 metamaskbot

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 68.72%. Comparing base (0604181) to head (e3b58ab).

:exclamation: Current head e3b58ab differs from pull request most recent head 0c58d7a. Consider uploading reports for the commit 0c58d7a to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #22865      +/-   ##
===========================================
- Coverage    68.80%   68.72%   -0.08%     
===========================================
  Files         1127     1124       -3     
  Lines        43595    43624      +29     
  Branches     11661    11673      +12     
===========================================
- Hits         29993    29977      -16     
- Misses       13602    13647      +45

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

Mar 13 '24 12:03 codecov[bot]

Builds ready [e3b58ab]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1292 ± 334 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	83	284	145	44	21
		domContentLoaded	11	81	35	21	10
		load	81	2009	1292	695	334
		domInteractive	11	81	35	21	10

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 26.92 KiB (0.72%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

Mar 13 '24 12:03 metamaskbot

Just noticed a bug:

turn on queuing feature flag
go to docs site https://docs.metamask.io/wallet/reference/ & clear permissions (disconnect from dapp)
check the docs selected network (call eth_chain id)
switch the wallet to a different network than the dapp
make a wallet_switchEthereumChain call from docs, switch to a chain that's not the same as the wallet's selected network, minimize the notification
make a eth_requestPermissions call for eth_accounts

-> double confirmation for switch, and the wallet ends up being switched to the docs' original selected network.

Mar 13 '24 19:03 BelfordZ

problem 1: wallet_requestPermissions should not switch chain before executing

problem 2: the networkClientId for a request that is queued after a switchEthereumChain request is incorrect

Consider the scenario where a dapp makes the following calls, in order, but without waiting for the response of one before sending the next (assume the dapp selected network is0xe708, and the wallet is on 0xa):

wallet_switchEthereumChain(0x1)
wallet_requestPermissions({ eth_accounts: {} }) In this scenario, what we expect to occur is:
confirmation dialog for switching from 0xa to 0x1
confirmation dialog for requesting the accounts
if the user opens the wallet, they will see that the network is 0x1
if the dapp calls eth_chainId, they will get back 0x1

What occurs atm though:

confirmation dialog for switching from 0xa to 0x1
confirmation dialog for switching from 0x1 to 0xe708
confirmation dialog for requesting the accounts
if the user opens the wallet, they will see that the network is 0xe708
if the dapp calls eth_chainId, they will get back 0xe708

Mar 13 '24 23:03 BelfordZ

Rebased to resolve conflicts

Mar 14 '24 15:03 Gudahtt

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

Mar 14 '24 15:03 socket-security[bot]

@SocketSecurity ignore npm/@metamask/[email protected]

Known maintainer

Mar 14 '24 16:03 Gudahtt

I believe I've found a bug related to our provider RPC pipeline and the SelectedNetworkController. It may be related to the bugs you've found here.

When testing this PR, I'd recommend doing a hard-refresh of all dapps after granting or clearing permissions. That should work around the bug that I've found (See the bug for more details: #23509)

Mar 14 '24 21:03 Gudahtt

Addresses problem 1:

https://github.com/MetaMask/core/pull/4066

Mar 15 '24 22:03 jiexi

update: Ive re-tested the 2nd issue and it doesn't seem to be a problem anymore. Not sure what was going on before, but it seems to work as expected now. (just make sure the dapp has permissions, otherwise everything is pretty messed up)

Mar 19 '24 18:03 BelfordZ

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.

Mar 19 '24 19:03 github-actions[bot]

Rebased to resolve conflicts

Mar 19 '24 19:03 BelfordZ

@metamaskbot update-policies

Mar 19 '24 19:03 BelfordZ

Policies updated

Mar 19 '24 19:03 metamaskbot