fix: Prevent rpc request before onboarding is completed #23622

pedronfigueiredo · 2024-03-21T09:38:10Z

Description

The temporary solution is to:

Prevent two network controller requests by:
- Patch the network controller so that the lookup on initializeProvidercan be avoided.
- Call that initializeProvider with the lookup: false argument right after the network controller is constructed
- Call await this.networkController.lookupNetwork(); at the end of the metamask controller if the onboarding has been completed
- Call await this.networkController.lookupNetwork(); once the onboarding complete state changes
Prevent one accounts tracker request by:
- Moving the blockTracker event listener from the constructor of the controller to the start function

The long term solution will be #23005 (currently blocked by #4004 on core.

Related issues

Fixes: https://github.com/MetaMask/metamask-planning/issues/881

Manual testing steps

Add await new Promise((resolve) => setTimeout(resolve, 10000)); to the top of initialize on the background.js file
Build the app
Load extension and immediately open the network tab on the background console before the app initializes
No requests should happen

Screenshots/Recordings

Before

Peek.2023-12-05.06-44.mp4

After

Peek.2023-12-05.06-31.mp4

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.

github-actions · 2024-03-21T09:38:24Z

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.

socket-security · 2024-03-21T09:54:52Z

👍 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↗︎

pedronfigueiredo · 2024-03-21T09:59:36Z

@SocketSecurity ignore npm/@metamask/safe-event-emitter@3.0.0

metamaskbot · 2024-03-21T10:01:44Z

Builds ready [91acfcc]

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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1381 ± 563 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	78	413	157	76	37
		domContentLoaded	13	77	29	15	7
		load	58	2639	1381	1172	563
		domInteractive	13	77	29	15	7

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

background: 113 Bytes (0.00%)
ui: 9 Bytes (0.00%)
common: 23 Bytes (0.00%)

app/scripts/metamask-controller.js

danjm

I left two comments

.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch

app/scripts/metamask-controller.js

.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch

metamaskbot · 2024-03-25T10:33:49Z

Builds ready [7e532de]

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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1019 ± 588 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	72	272	153	58	28
		domContentLoaded	11	116	40	28	14
		load	57	3144	1019	1225	588
		domInteractive	11	116	40	28	14

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

background: 171 Bytes (0.00%)
ui: 9 Bytes (0.00%)
common: 23 Bytes (0.00%)

app/scripts/metamask-controller.js

…mplete

metamaskbot · 2024-03-26T13:01:55Z

Builds ready [3988377]

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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1232 ± 496 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	81	275	148	49	23
		domContentLoaded	16	62	30	14	7
		load	67	2606	1232	1032	496
		domInteractive	16	62	30	14	7

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

background: 169 Bytes (0.00%)
ui: 9 Bytes (0.00%)
common: -27 Bytes (-0.00%)

test/e2e/tests/onboarding/onboarding.spec.js

pedronfigueiredo added the team-extension-platform label Mar 21, 2024

pedronfigueiredo requested a review from danjm March 21, 2024 09:38

pedronfigueiredo self-assigned this Mar 21, 2024

pedronfigueiredo requested a review from a team as a code owner March 21, 2024 09:38

pedronfigueiredo force-pushed the prevent-rpc-request-before-onboard branch from d16dad8 to 91acfcc Compare March 21, 2024 09:38

pedronfigueiredo changed the title ~~fix: Prevent rpc request before onboarding is complete~~ fix: Prevent rpc request before onboarding is completed Mar 21, 2024

danjm reviewed Mar 21, 2024

View reviewed changes

app/scripts/metamask-controller.js Outdated Show resolved Hide resolved

danjm reviewed Mar 21, 2024

View reviewed changes

app/scripts/metamask-controller.js Outdated Show resolved Hide resolved

danjm requested changes Mar 21, 2024

View reviewed changes

pedronfigueiredo force-pushed the prevent-rpc-request-before-onboard branch from 1b78a96 to 837e12d Compare March 21, 2024 16:07

pedronfigueiredo requested a review from danjm March 21, 2024 16:19

Gudahtt reviewed Mar 22, 2024

View reviewed changes

.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch Show resolved Hide resolved

Gudahtt reviewed Mar 22, 2024

View reviewed changes

app/scripts/metamask-controller.js Show resolved Hide resolved

Gudahtt reviewed Mar 22, 2024

View reviewed changes

app/scripts/metamask-controller.js Outdated Show resolved Hide resolved

Gudahtt reviewed Mar 22, 2024

View reviewed changes

.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch Outdated Show resolved Hide resolved

pedronfigueiredo force-pushed the prevent-rpc-request-before-onboard branch 2 times, most recently from 63af236 to 536ab0a Compare March 25, 2024 10:39

pedronfigueiredo requested a review from Gudahtt March 25, 2024 10:39

DDDDDanica reviewed Mar 25, 2024

View reviewed changes

app/scripts/metamask-controller.js Show resolved Hide resolved

brad-decker previously approved these changes Mar 25, 2024

View reviewed changes

pedronfigueiredo dismissed brad-decker’s stale review via 430c5cd March 26, 2024 12:39

danjm and others added 4 commits March 26, 2024 12:41

Prevent requests to the network provider until after onboarding is co…

51e01d8

…mplete

update patch after rebase

2735b04

update yarn.lock after running yarn for the first time

f51f7e8

perform network lookup on extension reload and onboarding complete

094c4fd

pedronfigueiredo added 8 commits March 26, 2024 12:41

delay block polling start in the account tracker

d09935b

remove unecessary network lookup

6783798

address feedback

a13e6e4

update lockfile

80084d3

revert moving deprecatedNetworkId assignement

ea70a84

extract postOnboardingInitialization

d885be6

Update patch and add patch docs

c03b926

Add onboarding e2e for infura network requests

3988377

pedronfigueiredo force-pushed the prevent-rpc-request-before-onboard branch from 430c5cd to 3988377 Compare March 26, 2024 12:41

DDDDDanica approved these changes Mar 26, 2024

View reviewed changes

danjm approved these changes Mar 26, 2024

View reviewed changes

pedronfigueiredo merged commit c947630 into develop Mar 26, 2024
65 of 66 checks passed

pedronfigueiredo deleted the prevent-rpc-request-before-onboard branch March 26, 2024 14:50

github-actions bot locked and limited conversation to collaborators Mar 26, 2024

metamaskbot added the release-11.15.0 Issue or pull request that will be included in release 11.15.0 label Mar 26, 2024

seaona reviewed Mar 27, 2024

View reviewed changes

test/e2e/tests/onboarding/onboarding.spec.js Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Prevent rpc request before onboarding is completed #23622

fix: Prevent rpc request before onboarding is completed #23622

pedronfigueiredo commented Mar 21, 2024

github-actions bot commented Mar 21, 2024

socket-security bot commented Mar 21, 2024 •

edited

Loading

pedronfigueiredo commented Mar 21, 2024

metamaskbot commented Mar 21, 2024

danjm left a comment

metamaskbot commented Mar 25, 2024

metamaskbot commented Mar 26, 2024

fix: Prevent rpc request before onboarding is completed #23622

fix: Prevent rpc request before onboarding is completed #23622

Conversation

pedronfigueiredo commented Mar 21, 2024

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Mar 21, 2024

socket-security bot commented Mar 21, 2024 • edited Loading

pedronfigueiredo commented Mar 21, 2024

metamaskbot commented Mar 21, 2024

danjm left a comment

Choose a reason for hiding this comment

metamaskbot commented Mar 25, 2024

metamaskbot commented Mar 26, 2024

socket-security bot commented Mar 21, 2024 •

edited

Loading