Fix #15050 - MV3: Keep the user logged in when service worker restarts by darkwing · Pull Request #15558 · MetaMask/metamask-extension

darkwing · 2022-08-11T18:44:41Z

Explanation

Currently, when the service worker restarts, the user gets logged out. This PR is the extension portion of the login fix. More details will be added as we get further along.

More Information

Fixes MV3: Ensure user remains logged in after service work restart #15050
See: #67890

Depends on:

eth-keyring-controller (Keep User Logged In: Export key for encrypted key login KeyringController#152)

Manual Testing Steps

Start the extension in mv3 mode: yarn start:mv3
Login to MetaMask
Lock MetaMask
Log in again
Open the DevTools "Application" panel and kill the service worker (it will restart)
Execute actions that would require login without refreshing the page ("Create Account"), for example
See that you're still logged in, actions working correctly

Pre-Merge Checklist

PR template is filled out
IF this PR fixes a bug, a test that would have caught the bug has been added
PR is linked to the appropriate GitHub issue
PR has been added to the appropriate release Milestone

+ If there are functional changes:

Manual testing complete & passed
"Extension QA Board" label has been applied

github-actions · 2022-08-11T18:44:55Z

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.

app/scripts/metamask-controller.js

seaona · 2022-09-22T08:03:31Z

I've noticed the following behaviors:

If I have the extension full window open, and I wait +5min idle, MM does not logout
If I don't have the extension full window open, and I wait +5min idle, MM does logout
If I terminate the Service Worker manually, and have the extension full window open, MM does logout
If I terminate the Service Worker manually, and I don't have the extension full window open, MM does logout

About 3 and 4, I am not completely sure this would ever happen in a real life scenario, as I don't think anyone would terminate the Service Worker manually. Just including it here in case we do need to consider it @darkwing @danjm

Behavior 2

mm-popup-idle.webm

Behavior 3

mm-full-manual-restart.webm

Behavior 4

mm-popup-manual-restart.webm

app/scripts/metamask-controller.js

seaona · 2022-10-11T09:21:49Z

Some QA finding with last commits, I am getting the error Uncaught (in promise) Error: key is not extractable on the two Onboarding paths:

Onboarding Flow with Create Wallet

key-not-extractable-create.webm

Onboarding Flow with an Imported Account

key-not-extractable.webm

seaona · 2022-10-26T10:48:56Z

Started re-testing with the last changes and all previous errors mentioned above disappeared ⭐

I saw that the error vault.map is not a function is triggered after I log out and try to log in again. Not sure if it's something about my build or a global error for everyone.

Repro steps:

Load MM
Go to Create Wallet onboarding flow
Lock MM
Try to unlock MM -- see error on the UI

vault-map-notfunc.webm

darkwing · 2022-10-26T13:29:22Z

Thanks @seaona ! I spotted this issue yesterday, as I was missing an await in the browser-passworder PR; this should be fixed. If you're still seeing it, I think it's because NPM is using cached information

lavamoat/browserify/policy-override.json

yarn.lock

…to mv3-login

metamaskbot · 2022-11-22T22:27:56Z

Builds ready [71a087f]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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, 8, 9
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2140 ± 87 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	87	135	107	14	7
		domContentLoaded	1752	2393	2120	171	82
		load	1753	2393	2140	182	87
		domInteractive	1752	2393	2120	171	82

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

background: 21911 bytes
ui: 15 bytes
common: 136 bytes

app/scripts/metamask-controller.js

metamaskbot · 2022-11-23T00:30:54Z

Builds ready [c2577d9]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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, 8, 9
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2223 ± 113 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	96	2004	216	411	197
		domContentLoaded	1760	2494	2205	231	111
		load	1760	2494	2223	235	113
		domInteractive	1760	2494	2205	231	111

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

background: 22109 bytes
ui: 15 bytes
common: 136 bytes

jpuri · 2022-11-23T03:18:39Z

@darkwing : Code changes look good and seems to work well 👍

I see one scenario breaking. If I login in MV2 and then start extension in MV3, I get this error while trying to login again using password:

darkwing · 2022-11-23T14:03:04Z

I see one scenario breaking. If I login in MV2 and then start extension in MV3, I get this error while trying to login again using password

That should be the Ledger hardware wallet error, correct? That's a separate issue being worked on by the hardware wallets team.

darkwing · 2022-11-23T14:10:13Z

Specifically, commenting out this line prevents the problem:

https://github.com/MetaMask/eth-ledger-bridge-keyring/blob/main/index.js#L40

The issue described isn't specific to or introduced by this PR.

Gudahtt

LGTM!

metamaskbot · 2022-11-23T17:45:48Z

Builds ready [62fc4a2]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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, 8, 9
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2342 ± 170 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	91	1141	183	221	106
		domContentLoaded	1727	3487	2327	363	175
		load	1727	3487	2342	354	170
		domInteractive	1727	3487	2327	363	175

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

background: 22121 bytes
ui: 15 bytes
common: 136 bytes

darkwing requested a review from a team as a code owner August 11, 2022 18:44

darkwing requested a review from ryanml August 11, 2022 18:44

darkwing marked this pull request as draft August 11, 2022 18:52

darkwing force-pushed the mv3-login branch from 2767a81 to 798a0c2 Compare August 12, 2022 16:07

hilvmason added the PR - P1 identifies PRs deemed priority for Extension team label Aug 15, 2022

segun reviewed Aug 23, 2022

View reviewed changes

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

danjm self-assigned this Sep 6, 2022

darkwing force-pushed the mv3-login branch from 798a0c2 to 75392b1 Compare September 20, 2022 16:55

segun mentioned this pull request Sep 21, 2022

Keep memstore contents after service worker restarts #15913

Merged

6 tasks

darkwing added the DO-NOT-MERGE Pull requests that should not be merged label Sep 22, 2022

darkwing force-pushed the mv3-login branch from 46655bc to bcdb88d Compare September 22, 2022 18:24

darkwing added the MV3 label Sep 26, 2022

darkwing marked this pull request as ready for review September 26, 2022 17:21

darkwing force-pushed the mv3-login branch from ce279dd to c3359bb Compare September 27, 2022 14:55

seaona mentioned this pull request Oct 4, 2022

[Bug]: Buy option on all test networks throws an error when I try it #16017

Closed

darkwing force-pushed the mv3-login branch 2 times, most recently from 538ae0a to 4d4ba9c Compare October 8, 2022 22:04

naugtur reviewed Oct 10, 2022

View reviewed changes

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

naugtur reviewed Oct 10, 2022

View reviewed changes

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

darkwing force-pushed the mv3-login branch 4 times, most recently from b01909e to cacb5ca Compare October 19, 2022 19:00

darkwing force-pushed the mv3-login branch 2 times, most recently from 9f689d7 to 20c066a Compare October 25, 2022 13:59

seaona and others added 5 commits November 22, 2022 11:59

snap policies

36a8965

Fix UI sync (#16612)

5eb1cd1

Fix policies

afcbd26

Fix lint

1947d65

Fix policies

1eaecc9

darkwing force-pushed the mv3-login branch from 028abb1 to 1eaecc9 Compare November 22, 2022 18:11

lavamoat

4a82542

Gudahtt reviewed Nov 22, 2022

View reviewed changes

lavamoat/browserify/policy-override.json Show resolved Hide resolved

Gudahtt reviewed Nov 22, 2022

View reviewed changes

yarn.lock Show resolved Hide resolved

darkwing added 3 commits November 22, 2022 15:29

Restore policy overrides

c6f6388

Merge branch 'mv3-login' of github.com:MetaMask/metamask-extension in…

a5f5a67

…to mv3-login

Regenerate policies

71a087f

Gudahtt reviewed Nov 22, 2022

View reviewed changes

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

Gudahtt reviewed Nov 22, 2022

View reviewed changes

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

darkwing added 2 commits November 22, 2022 17:41

Warn of invalid salt

333b770

Add salt validation, move startUISync

c2577d9

Add return

3bcaf31

Gudahtt approved these changes Nov 23, 2022

View reviewed changes

darkwing added 2 commits November 23, 2022 11:03

Merge branch 'develop' into mv3-login

f09a411

Merge branch 'develop' into mv3-login

62fc4a2

kumavis approved these changes Nov 24, 2022

View reviewed changes

darkwing merged commit 266d7d9 into develop Nov 24, 2022

darkwing deleted the mv3-login branch November 24, 2022 00:49

github-actions bot locked and limited conversation to collaborators Nov 24, 2022

Conversation

darkwing commented Aug 11, 2022 • edited by danjm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

More Information

Depends on:

Manual Testing Steps

Pre-Merge Checklist

+ If there are functional changes:

Uh oh!

github-actions bot commented Aug 11, 2022

Uh oh!

Uh oh!

seaona commented Sep 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seaona commented Oct 11, 2022

Uh oh!

seaona commented Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darkwing commented Oct 26, 2022

Uh oh!

Uh oh!

Uh oh!

metamaskbot commented Nov 22, 2022

Uh oh!

Uh oh!

Uh oh!

metamaskbot commented Nov 23, 2022

Uh oh!

jpuri commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darkwing commented Nov 23, 2022

Uh oh!

darkwing commented Nov 23, 2022

Uh oh!

Gudahtt left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Nov 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

darkwing commented Aug 11, 2022 •

edited by danjm

Loading

seaona commented Sep 22, 2022 •

edited

Loading

seaona commented Oct 26, 2022 •

edited

Loading

jpuri commented Nov 23, 2022 •

edited

Loading