fix: token detection and import #23798

bergeron · 2024-03-29T01:11:37Z

Description

Fixes issues with detecting and manually importing tokens, caused by recent controller changes.

Hotfix for 11.14.0

This state publish had an undefined variable, and had the wrong function signature:

Which prevented the controllers from processing account switches.

A test was failing because RPC requests were sent before onboarding. Delayed the enabling of token detection controller to triggerNetworkrequests.

Also patching this fix for detection during account switch on mainnet when autodetection=off:
MetaMask/core#4133

Related issues

Fixes:

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

I’ve followed MetaMask 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-03-29T01:11:51Z

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.

salimtb · 2024-03-29T09:52:50Z

@bergeron can you check why your e2e tests are failing ?

app/scripts/metamask-controller.js

MajorLift · 2024-03-29T17:44:27Z

Probably unrelated to the bugs, but I noticed another error related to the preferences-controller that was introduced in #22928.

diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 89905caaef..ddecaf42f5 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -500,8 +500,8 @@ export default class MetamaskController extends EventEmitter {
 
     const preferencesMessenger = this.controllerMessenger.getRestricted({
       name: 'PreferencesController',
-      allowedActions: ['PreferencesController:getState'],
-      allowedEvents: ['PreferencesController:stateChange'],
+      allowedActions: [],
+      allowedEvents: [],
     });
 
     this.preferencesController = new PreferencesController({

bergeron · 2024-03-29T20:11:26Z

Test failing because we're sending RPC requests before onboarding finishes. Test was recently added while detection was broken. Will see if I can delay the controller's start.

codecov · 2024-04-01T17:36:24Z

Codecov Report

Attention: Patch coverage is 75.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 69.03%. Comparing base (f29a78e) to head (2f5c68b).

Files	Patch %	Lines
app/scripts/metamask-controller.js	75.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #23798      +/-   ##
===========================================
+ Coverage    68.99%   69.03%   +0.04%     
===========================================
  Files         1164     1164              
  Lines        44515    44491      -24     
  Branches     11890    11888       -2     
===========================================
  Hits         30711    30711              
+ Misses       13804    13780      -24

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

metamaskbot · 2024-04-01T17:40:08Z

Builds ready [f402e0f]

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

Page Load Metrics (1192 ± 583 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	82	592	185	111	53
		domContentLoaded	16	77	35	18	9
		load	67	2994	1192	1215	583
		domInteractive	16	77	35	18	9

Bundle size diffs [🚀 Bundle size reduced!]

background: -82 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

… bergeron-patch-1

metamaskbot · 2024-04-03T04:36:14Z

Builds ready [6ffbb8c]

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

Page Load Metrics (355 ± 304 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	67	164	105	22	11
		domContentLoaded	9	62	31	16	8
		load	53	1969	355	633	304
		domInteractive	9	62	31	16	8

Bundle size diffs [🚀 Bundle size reduced!]

background: -45 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

socket-security · 2024-04-03T06:03:22Z

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

Package	New capabilities	Transitives	Size	Publisher
npm/@types/[email protected]	None	`0`	5.96 kB	types

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

View full report↗︎

metamaskbot · 2024-04-03T07:00:12Z

Builds ready [2f5c68b]

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

Page Load Metrics (631 ± 449 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	273	126	46	22
		domContentLoaded	10	60	29	14	7
		load	58	2407	631	934	449
		domInteractive	10	60	29	14	7

Bundle size diffs [🚀 Bundle size reduced!]

background: -45 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: -18 Bytes (-0.00%)

## Explanation During account change, the token detection trigger was guarded by the `useTokenDetection` setting. But on mainnet, detection should run against the static token list even if the setting is disabled. So remove the guard. ## References MetaMask/metamask-extension#23798 ## Changelog ### `@metamask/assets-controllers` - **FIXED**: Token detection during account change ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate

MajorLift

LGTM! Thanks for making these fixes.

Update metamask-controller.js

e1ee300

bergeron marked this pull request as ready for review March 29, 2024 01:14

bergeron requested a review from a team as a code owner March 29, 2024 01:14

bergeron requested review from MajorLift, salimtb and sahar-fehri March 29, 2024 01:14

bergeron added the team-assets label Mar 29, 2024

lint

9ba0e75

salimtb previously approved these changes Mar 29, 2024

View reviewed changes

salimtb self-requested a review March 29, 2024 10:01

publish state, not array

2ec8e70

bergeron dismissed salimtb’s stale review via 2ec8e70 March 29, 2024 15:18

bergeron commented Mar 29, 2024

View reviewed changes

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

MajorLift reviewed Mar 29, 2024

View reviewed changes

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

Fix state publish

a9fd10e

move token detection enable to post onboarding

f402e0f

bergeron changed the title ~~fix: preferences controller messaging~~ fix: token detection and import Apr 1, 2024

bergeron added 4 commits April 2, 2024 11:33

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

07c7d61

… bergeron-patch-1

allow token detection during onboarding of existing seed

10ac840

.

405c2c9

.

6ffbb8c

patch core changes

40a68af

bergeron mentioned this pull request Apr 3, 2024

fix: token detection during account change MetaMask/core#4132

Closed

3 tasks

.

8aef11f

bergeron mentioned this pull request Apr 3, 2024

fix: token detection during account change MetaMask/core#4133

Merged

3 tasks

patch

2f5c68b

MajorLift approved these changes Apr 3, 2024

View reviewed changes

sahar-fehri approved these changes Apr 4, 2024

View reviewed changes

brad-decker approved these changes Apr 5, 2024

View reviewed changes

bergeron merged commit f327149 into develop Apr 5, 2024
72 checks passed

bergeron deleted the bergeron-patch-1 branch April 5, 2024 19:38

github-actions bot locked and limited conversation to collaborators Apr 5, 2024

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: token detection and import #23798

fix: token detection and import #23798

bergeron commented Mar 29, 2024 •

edited

Loading

github-actions bot commented Mar 29, 2024

salimtb commented Mar 29, 2024

MajorLift commented Mar 29, 2024 •

edited

Loading

bergeron commented Mar 29, 2024

codecov bot commented Apr 1, 2024 •

edited

Loading

metamaskbot commented Apr 1, 2024

metamaskbot commented Apr 3, 2024

socket-security bot commented Apr 3, 2024 •

edited

Loading

metamaskbot commented Apr 3, 2024

MajorLift left a comment

fix: token detection and import #23798

fix: token detection and import #23798

Conversation

bergeron commented Mar 29, 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 Mar 29, 2024

salimtb commented Mar 29, 2024

MajorLift commented Mar 29, 2024 • edited Loading

bergeron commented Mar 29, 2024

codecov bot commented Apr 1, 2024 • edited Loading

Codecov Report

metamaskbot commented Apr 1, 2024

metamaskbot commented Apr 3, 2024

socket-security bot commented Apr 3, 2024 • edited Loading

metamaskbot commented Apr 3, 2024

MajorLift left a comment

Choose a reason for hiding this comment

bergeron commented Mar 29, 2024 •

edited

Loading

MajorLift commented Mar 29, 2024 •

edited

Loading

codecov bot commented Apr 1, 2024 •

edited

Loading

socket-security bot commented Apr 3, 2024 •

edited

Loading