refactor: use core profile syncing controllers. #26370

Prithpal-Sooriya · 2024-08-12T15:19:43Z

Description

1st migration to shared controllers. This adds the reusable controllers from core. Migrated AuthenticationController and UserStorageController

Related issues

Fixes: N/A

Manual testing steps

Go to this page...

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.

github-actions · 2024-08-12T15:19:54Z

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-08-12T15:20:55Z

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

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/[email protected]	network	`0`	668 kB	metamaskbot
npm/@metamask/[email protected]	None	`0`	128 kB	metamaskbot
npm/@metamask/[email protected]	network	`0`	1.04 MB	metamaskbot
npm/@stablelib/[email protected]	None	`0`	103 kB	dchest
npm/@stablelib/[email protected]	None	`0`	13.2 kB	dchest
npm/@stablelib/[email protected]	None	`0`	33.3 kB	dchest
npm/@stablelib/[email protected]	None	`0`	7.06 kB	dchest
npm/[email protected]	filesystem	`0`	1.46 MB	ldthomas
npm/[email protected]	filesystem	`0`	148 kB	harrisiirak
npm/[email protected]	None	`0`	923 kB	tonybrix
npm/[email protected]	None	`0`	24 kB	davidmarkclements
npm/[email protected]	None	`0`	85.7 kB	sbihel

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

View full report↗︎

socket-security · 2024-08-12T15:20:58Z

🚨 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`	🚫

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.

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]

Prithpal-Sooriya · 2024-08-12T15:34:21Z

@metamaskbot update-policies

metamaskbot · 2024-08-12T15:43:57Z

Policies updated

Prithpal-Sooriya · 2024-08-12T17:27:44Z

package.json

@@ -365,7 +366,6 @@
    "@metamask/user-operation-controller": "^13.0.0",
    "@metamask/utils": "^8.2.1",
    "@ngraveio/bc-ur": "^1.1.12",
-    "@noble/ciphers": "^0.5.2",


This package was only used as part of our controllers. Now that we are using these shared controllers, we can remove this dep.

Prithpal-Sooriya · 2024-08-12T17:29:44Z

ui/selectors/metamask-notifications/authentication.ts


 type AppState = {
-  metamask: AuthenticationControllerState & {
+  metamask: AuthenticationController.AuthenticationControllerState & {


Yep I get it, this syntax is weird. We were fighting to get multiple file exports working for this package, but ran into issues and time constraints, so opted for named exports/namespaced exports.

In near future we will support multiple exports. E.g.

import { AuthenticationControllerState } from '@metamask/profile-sync-controller/auth'

Prithpal-Sooriya · 2024-08-12T17:36:00Z

app/scripts/metamask-controller.js

+    // Temporary add missing methods (due to notification controller migration)
+    this.controllerMessenger.registerActionHandler(
+      'NotificationServicesController:disableNotificationServices',
+      () => this.metamaskNotificationsController.disableMetamaskNotifications(),
+    );
+    this.controllerMessenger.registerActionHandler(
+      'NotificationServicesController:selectIsNotificationServicesEnabled',
+      () =>
+        this.metamaskNotificationsController.selectIsMetamaskNotificationsEnabled(),
+    );


We are slowly migrating controllers to use the shared controllers.
This addition is so that this PR works standalone (there was some controller renaming that was done in shared libraries)

metamaskbot · 2024-08-12T17:43:38Z

Builds ready [ee2b73e]

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

Page Load Metrics (356 ± 313 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	89	181	118	21	10
		domContentLoaded	10	73	28	16	8
		load	45	2165	356	652	313
		domInteractive	10	73	28	16	8

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

background: 106.53 KiB (3.01%)
ui: 0 Bytes (0.00%)
common: 5.9 KiB (0.08%)

codecov · 2024-08-12T17:44:12Z

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.98%. Comparing base (b3c2323) to head (190c72c).
Report is 3 commits behind head on develop.

Files	Patch %	Lines
app/scripts/metamask-controller.js	57.14%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #26370      +/-   ##
===========================================
- Coverage    70.13%   69.98%   -0.16%     
===========================================
  Files         1435     1422      -13     
  Lines        50309    49937     -372     
  Branches     13897    13862      -35     
===========================================
- Hits         35283    34944     -339     
+ Misses       15026    14993      -33

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

matteoscurati

LGTM

metamaskbot · 2024-08-13T12:48:45Z

Builds ready [5ad6097]

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

Page Load Metrics (281 ± 278 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	85	192	121	27	13
		domContentLoaded	10	96	31	24	12
		load	51	2157	281	580	278
		domInteractive	10	96	31	24	12

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

background: 106.53 KiB (3.01%)
ui: 0 Bytes (0.00%)
common: 5.9 KiB (0.08%)

mathieuartu

YESSS

sonarqubecloud · 2024-08-14T08:42:11Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-08-14T09:13:11Z

Builds ready [190c72c]

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

Page Load Metrics (60 ± 7 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	58	125	90	16	8
		domContentLoaded	9	55	26	13	6
		load	38	89	60	14	7
		domInteractive	9	55	26	13	6

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

background: 106.53 KiB (3.01%)
ui: 0 Bytes (0.00%)
common: 5.9 KiB (0.08%)

Prithpal-Sooriya added 3 commits August 12, 2024 15:18

refactor: use core profile sync controller

bbfa164

refactor: use core AuthenticationController

b377dc4

refactor: use core UserStorageController

86cc3ba

metamaskbot added the team-notifications Notifications team label Aug 12, 2024

Update LavaMoat policies

6c57755

Prithpal-Sooriya added 2 commits August 12, 2024 16:46

refactor: dedupe lock file

968dd4f

refactor: update lavamoat policies

ee2b73e

Prithpal-Sooriya commented Aug 12, 2024

View reviewed changes

Prithpal-Sooriya added 2 commits August 13, 2024 09:25

Merge branch 'develop' into notify_840_use_profile_sync_controller

964ebd2

Merge branch 'develop' into notify_840_use_profile_sync_controller

5ad6097

Prithpal-Sooriya marked this pull request as ready for review August 13, 2024 12:39

Prithpal-Sooriya requested review from a team as code owners August 13, 2024 12:39

Prithpal-Sooriya requested review from matteoscurati and mathieuartu August 13, 2024 12:40

matteoscurati approved these changes Aug 13, 2024

View reviewed changes

mathieuartu approved these changes Aug 13, 2024

View reviewed changes

Merge branch 'develop' into notify_840_use_profile_sync_controller

190c72c

Prithpal-Sooriya merged commit f9353f4 into develop Aug 14, 2024
77 of 78 checks passed

Prithpal-Sooriya deleted the notify_840_use_profile_sync_controller branch August 14, 2024 09:29

github-actions bot locked and limited conversation to collaborators Aug 14, 2024

metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Aug 14, 2024

gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: use core profile syncing controllers. #26370

refactor: use core profile syncing controllers. #26370

Prithpal-Sooriya commented Aug 12, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Aug 12, 2024

socket-security bot commented Aug 12, 2024 •

edited

Loading

socket-security bot commented Aug 12, 2024 •

edited

Loading

Prithpal-Sooriya commented Aug 12, 2024

metamaskbot commented Aug 12, 2024

Prithpal-Sooriya Aug 12, 2024

Prithpal-Sooriya Aug 12, 2024 •

edited

Loading

Prithpal-Sooriya Aug 12, 2024

metamaskbot commented Aug 12, 2024

codecov bot commented Aug 12, 2024 •

edited

Loading

matteoscurati left a comment

metamaskbot commented Aug 13, 2024

mathieuartu left a comment

sonarqubecloud bot commented Aug 14, 2024

metamaskbot commented Aug 14, 2024

refactor: use core profile syncing controllers. #26370

refactor: use core profile syncing controllers. #26370

Conversation

Prithpal-Sooriya commented Aug 12, 2024 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Aug 12, 2024

socket-security bot commented Aug 12, 2024 • edited Loading

socket-security bot commented Aug 12, 2024 • edited Loading

Next steps

Prithpal-Sooriya commented Aug 12, 2024

metamaskbot commented Aug 12, 2024

Prithpal-Sooriya Aug 12, 2024

Choose a reason for hiding this comment

Prithpal-Sooriya Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

Prithpal-Sooriya Aug 12, 2024

Choose a reason for hiding this comment

metamaskbot commented Aug 12, 2024

codecov bot commented Aug 12, 2024 • edited Loading

Codecov Report

matteoscurati left a comment

Choose a reason for hiding this comment

metamaskbot commented Aug 13, 2024

mathieuartu left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Aug 14, 2024

Quality Gate passed

metamaskbot commented Aug 14, 2024

Prithpal-Sooriya commented Aug 12, 2024 •

edited by github-actions bot

Loading

socket-security bot commented Aug 12, 2024 •

edited

Loading

socket-security bot commented Aug 12, 2024 •

edited

Loading

Prithpal-Sooriya Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading