feat: account syncing various updates #28541

mathieuartu · 2024-11-19T12:38:49Z

Description

This PR adds various account syncing house keeping improvements in order to be ready for re-enablement in a subsequent PR.
The number of files changed by this PR is large, but none affects a user facing feature, since account syncing is disabled in production.

Bump @metamask/profile-sync-controller to version 1.0.2
Add two new state keys linked to UserStorageController, isAccountSyncingReadyToBeDispatched and hasAccountSyncingSyncedAtLeastOnce
Wait for _addAccountsWithBalance to finish adding accounts after onboarding, then set isAccountSyncingReadyToBeDispatched to true
Use USER_STORAGE_FEATURE_NAMES exported constant from @metamask/profile-sync-controller to define user storage paths everywhere (no more magic strings)
Add batch delete delete support for E2E util UserStorageMockttpController
Update all account sync E2E tests in order to wait for account sync to have successfully been completed once before going on with the rest of the instructions.

⚠️ Please note that this PR does NOT re-enable account syncing in production. This will be done in a subsequent PR.

Related issues

Manual testing steps

Import your SRP
Add accounts, rename accounts
Uninstall & reinstall
Import the same SRP
Verify that previous updates made in step 2 are there

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
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.

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.

…ait for _addAccountsWithBalance before dispatching the first account sync

github-actions · 2024-11-19T12:39:02Z

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-11-19T12:39:52Z

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

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/[email protected]	network	`0`	44.1 kB	lgbot
npm/@metamask/[email protected]	network	`0`	172 kB	metamaskbot
npm/@metamask/[email protected]	environment	`+2`	761 kB	metamaskbot
npm/@metamask/[email protected]	network	`+2`	1.85 MB	metamaskbot
npm/[email protected]	None	`0`	23.2 kB	lukeed
npm/[email protected]	None	`0`	646 kB	acemarke

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

View full report↗︎

socket-security · 2024-11-19T12:39:54Z

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert	Package	Note	Source	CI
Network access	npm/@metamask/[email protected]	Module: globalThis["fetch"] Location: Package overview	`package.json` `yarn.lock`	🚫
New author	npm/@metamask/[email protected]	New Author: lgbot Previous Author: mcmire	`yarn.lock`	🚫
Unstable ownership	npm/@metamask/[email protected]	Author: lgbot	`yarn.lock`	🚫
Network access	npm/@metamask/[email protected]	Module: globalThis["fetch"] Location: Package overview	`yarn.lock`	🚫

View full report↗︎

Next steps

What is network access?

This module accesses the network.

Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

What is new author?

A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package.

Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights.

What is unstable ownership?

A new collaborator has begun publishing package versions. Package stability and security risk may be elevated.

Try to reduce the amount of authors you depend on to reduce the risk to malicious actors gaining access to your supply chain. Packages should remove inactive collaborators with publishing rights from packages on npm.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

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

test/e2e/helpers/user-storage/userStorageMockttpController.ts

mathieuartu · 2024-11-19T12:43:15Z

app/scripts/metamask-controller.js

+            if (count !== 1) {
+              await this.removeAccount(address);
+            }
+            break;


This whole function appears to be fully modified but it has just been wrapped in a try - catch - finally block

mathieuartu · 2024-11-19T12:44:12Z

app/scripts/metamask-controller.js

+    } catch (e) {
+      log.warn(`Failed to add accounts with balance. Error: ${e}`);
+    } finally {
+      this.preferencesController.setHasFinishedAddingAccountsWithBalance(true);


In order to prevent conflicts with different actors adding accounts at the end of onboarding, we're only dispatching account syncing when _addAccountsWithBalance is done.

mathieuartu · 2024-11-19T12:52:38Z

@metamaskbot update-policies

metamaskbot · 2024-11-19T13:05:39Z

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

…etaMask/metamask-extension into improv/account_syncing_various_updates

metamaskbot · 2024-11-20T08:55:34Z

Builds ready [32286e0]

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, 6, 7
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2341 ± 112 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	329	2701	1955	800	384
		domContentLoaded	1828	2689	2302	236	113
		load	1841	2702	2341	233	112
		domInteractive	32	67	48	11	5
		backgroundConnect	9	90	32	22	10
		firstReactRender	68	377	158	104	50
		getState	82	214	128	44	21
		initialActions	0	1	0	0	0
		loadScripts	1332	2061	1725	199	96
		setupStore	7	81	20	24	11
		uiStartup	2149	3244	2830	310	149

Bundle size diffs [🚀 Bundle size reduced!]

background: 171 Bytes (0.00%)
ui: 225 Bytes (0.00%)
common: -55.86 KiB (-0.67%)

…etaMask/metamask-extension into improv/account_syncing_various_updates

metamaskbot · 2024-11-20T12:41:51Z

Builds ready [0ed48a0]

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, 6, 7
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1897 ± 66 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1644	2243	1905	135	65
		domContentLoaded	1634	2203	1857	124	59
		load	1644	2248	1897	137	66
		domInteractive	28	95	42	18	9
		backgroundConnect	13	86	40	25	12
		firstReactRender	68	387	142	92	44
		getState	45	250	114	55	26
		initialActions	0	1	0	0	0
		loadScripts	1212	1694	1400	104	50
		setupStore	6	85	22	29	14
		uiStartup	1939	2951	2327	311	149

Bundle size diffs [🚀 Bundle size reduced!]

background: 171 Bytes (0.00%)
ui: 225 Bytes (0.00%)
common: -55.86 KiB (-0.66%)

…etaMask/metamask-extension into improv/account_syncing_various_updates

app/scripts/constants/sentry-state.ts

metamaskbot · 2024-11-20T15:02:25Z

Builds ready [c2f24fd]

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, 6, 7
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2110 ± 64 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	523	2341	2031	370	178
		domContentLoaded	1793	2243	2048	123	59
		load	1871	2355	2110	134	64
		domInteractive	15	172	47	33	16
		backgroundConnect	10	198	61	43	21
		firstReactRender	66	366	127	77	37
		getState	48	204	110	46	22
		initialActions	0	0	0	0	0
		loadScripts	1283	1713	1548	117	56
		setupStore	6	39	10	7	3
		uiStartup	2165	3021	2517	199	96

Bundle size diffs [🚀 Bundle size reduced!]

background: 412 Bytes (0.01%)
ui: 225 Bytes (0.00%)
common: -55.79 KiB (-0.66%)

cmd-ob

Look good! Thanks for sorting.

mathieuartu added 3 commits November 19, 2024 13:33

improv: use profile-sync-controller config for user storage paths + w…

8a6691c

…ait for _addAccountsWithBalance before dispatching the first account sync

Merge branch 'develop' into improv/account_syncing_various_updates

5346252

fix: yarn.lock

104219f

mathieuartu added the team-identity label Nov 19, 2024

mathieuartu self-assigned this Nov 19, 2024

mathieuartu changed the title ~~Improv: account syncing various updates~~ feat: account syncing various updates Nov 19, 2024

github-advanced-security bot found potential problems Nov 19, 2024

View reviewed changes

test/e2e/helpers/user-storage/userStorageMockttpController.ts Fixed Show fixed Hide fixed

mathieuartu commented Nov 19, 2024

View reviewed changes

fix: regex escaping

0d8ecbd

Update LavaMoat policies

c7dc7f7

mathieuartu added 14 commits November 19, 2024 15:27

fix: unit test

b5b2f33

Merge branch 'develop' into improv/account_syncing_various_updates

c6ca069

fix: tests

b4eab67

Merge branch 'improv/account_syncing_various_updates' of github.com:M…

52592ed

…etaMask/metamask-extension into improv/account_syncing_various_updates

fix: bump profile-sync-controller and update code accordingly

8560b40

Merge branch 'develop' into improv/account_syncing_various_updates

33e992f

fix: test helper

61b6e70

Merge branch 'improv/account_syncing_various_updates' of github.com:M…

aa5992f

…etaMask/metamask-extension into improv/account_syncing_various_updates

fix: test

4a1ddb2

Merge branch 'develop' into improv/account_syncing_various_updates

7896fc1

fix: e2e & integration

af3ae62

Merge branch 'improv/account_syncing_various_updates' of github.com:M…

44b2874

…etaMask/metamask-extension into improv/account_syncing_various_updates

fix: e2e test

d3081ac

Merge branch 'develop' into improv/account_syncing_various_updates

32286e0

mathieuartu added 4 commits November 20, 2024 11:52

fix: yarn.lock

97e2787

Merge branch 'improv/account_syncing_various_updates' of github.com:M…

a743f3a

…etaMask/metamask-extension into improv/account_syncing_various_updates

Merge branch 'develop' into improv/account_syncing_various_updates

d6c5aef

Merge branch 'develop' into improv/account_syncing_various_updates

0ed48a0

mathieuartu added 3 commits November 20, 2024 15:23

feat: add analytics event

570ee0a

Merge branch 'improv/account_syncing_various_updates' of github.com:M…

d3e1925

…etaMask/metamask-extension into improv/account_syncing_various_updates

Merge branch 'develop' into improv/account_syncing_various_updates

c2f24fd

Prithpal-Sooriya reviewed Nov 20, 2024

View reviewed changes

app/scripts/constants/sentry-state.ts Show resolved Hide resolved

Prithpal-Sooriya approved these changes Nov 20, 2024

View reviewed changes

cmd-ob approved these changes Nov 20, 2024

View reviewed changes

mathieuartu marked this pull request as ready for review November 20, 2024 15:14

mathieuartu requested review from a team as code owners November 20, 2024 15:14

mathieuartu added this pull request to the merge queue Nov 20, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2024

mathieuartu added this pull request to the merge queue Nov 20, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2024

mathieuartu added this pull request to the merge queue Nov 20, 2024

Merged via the queue into develop with commit fac35b8 Nov 20, 2024
74 of 75 checks passed

mathieuartu deleted the improv/account_syncing_various_updates branch November 20, 2024 16:42

github-actions bot locked and limited conversation to collaborators Nov 20, 2024

metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: account syncing various updates #28541

feat: account syncing various updates #28541

mathieuartu commented Nov 19, 2024 •

edited

Loading

github-actions bot commented Nov 19, 2024

socket-security bot commented Nov 19, 2024 •

edited

Loading

socket-security bot commented Nov 19, 2024 •

edited

Loading

mathieuartu Nov 19, 2024 •

edited

Loading

mathieuartu Nov 19, 2024

mathieuartu commented Nov 19, 2024

metamaskbot commented Nov 19, 2024

metamaskbot commented Nov 20, 2024

metamaskbot commented Nov 20, 2024

metamaskbot commented Nov 20, 2024

cmd-ob left a comment

feat: account syncing various updates #28541

feat: account syncing various updates #28541

Conversation

mathieuartu commented Nov 19, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Nov 19, 2024

socket-security bot commented Nov 19, 2024 • edited Loading

socket-security bot commented Nov 19, 2024 • edited Loading

Next steps

mathieuartu Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

mathieuartu Nov 19, 2024

Choose a reason for hiding this comment

mathieuartu commented Nov 19, 2024

metamaskbot commented Nov 19, 2024

metamaskbot commented Nov 20, 2024

metamaskbot commented Nov 20, 2024

metamaskbot commented Nov 20, 2024

cmd-ob left a comment

Choose a reason for hiding this comment

mathieuartu commented Nov 19, 2024 •

edited

Loading

socket-security bot commented Nov 19, 2024 •

edited

Loading

socket-security bot commented Nov 19, 2024 •

edited

Loading

mathieuartu Nov 19, 2024 •

edited

Loading