Skip to content

feat: Skip wallet onboarding for developers#23781

Merged
pedronfigueiredo merged 5 commits intodevelopfrom
pnf/skip-onboarding
May 27, 2024
Merged

feat: Skip wallet onboarding for developers#23781
pedronfigueiredo merged 5 commits intodevelopfrom
pnf/skip-onboarding

Conversation

@pedronfigueiredo
Copy link
Copy Markdown
Contributor

@pedronfigueiredo pedronfigueiredo commented Mar 28, 2024 •
edited by github-actions bot
Loading

Description

  • Implements a new development build script that skips onboarding by preloading state from the end-to-end test fixtures.
  • The vault state is dynamically generated from the environment variables TEST_SRP and PASSWORD.
  • This build hides all the "What's New?" modals and the "Product Tour" as well as all onboarding screens.
  • The network is set to "Sepolia" by default.
  • Alternatively, onboarding can be skipped without setting a TEST_SRP, in which case the e2e tests SRP and accounts are used.

Open in GitHub Codespaces

Related issues

Related: #23615, #19389

Manual testing steps

  1. Checkout this branch
  2. Set a TEST_SRP environment variable on your .metamaskrc file as well as PASSWORD if you haven't already
  3. Run yarn start:skip-onboarding
  4. Load the extension on your favorite browser and open the popup

Screenshots/Recordings

After

asdf.mov

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.

@pedronfigueiredo pedronfigueiredo added the team-extension-platform Extension Platform team label Mar 28, 2024
@pedronfigueiredo pedronfigueiredo self-assigned this Mar 28, 2024
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner March 28, 2024 14:06
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 28, 2024

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.

@gauthierpetetin gauthierpetetin added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 11, 2024
digiwand
digiwand previously approved these changes Apr 11, 2024
View reviewed changes
@seaona
Copy link
Copy Markdown
Member

seaona commented Apr 15, 2024

hey cool idea 🚀 I think we could add a section in the README about this script. Somewhere here maybe?
Also, adding a couple of comments (no-changes needed) but maybe this could also be useful to clarify in the documentation.

  • it seems to work only with fresh installs: if I have an older wallet instance and I re-build with a new SRP, I see the old wallet accounts and if I generate new accounts, it's using the old SRP
skip-onboarding-reload.mp4
  • it loads only 1 Account, despite having more balance in subsequent accounts (similarly as in the e2e). Usually in regular builds, we load as many accounts as they have balance, in the following way:
    • Account1 is loaded
    • Account2 has balance -- is loaded
    • Account3 has balance --is loaded
    • Account4 doesn't have balance -- it is not loaded and we stop loading accounts at this point, no matter if the next account has balance

However, I think for the purposes of this script, it's totally fine to load only 1 account.

Screenshot from 2024-04-15 10-21-09

Screenshot from 2024-04-15 10-23-40

legobeat
legobeat reviewed Apr 15, 2024
View reviewed changes
app/scripts/skip-onboarding.js Outdated
import { encryptorFactory } from './lib/encryptor-factory';

const E2E_SRP =
'spread raise short crane omit tent fringe mandate neglect detail suspect cradle';
Copy link
Copy Markdown
Contributor

@legobeat legobeat Apr 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be shared with the existing definition rather than duplicated? In order to avoid avoid churn arising from a future change changing only one of them.

@seaona seaona mentioned this pull request May 8, 2024
Closed
7 tasks
OGPoyraz
OGPoyraz previously approved these changes May 9, 2024
View reviewed changes
HUAWE33
HUAWE33 reviewed May 23, 2024
View reviewed changes
Copy link
Copy Markdown

@HUAWE33 HUAWE33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#24739 #24737

@pedronfigueiredo pedronfigueiredo force-pushed the pnf/skip-onboarding branch 2 times, most recently from 1985598 to 9ddaecf Compare May 24, 2024 08:30
DDDDDanica
DDDDDanica previously approved these changes May 24, 2024
View reviewed changes
@DDDDDanica
Copy link
Copy Markdown
Contributor

DDDDDanica commented May 24, 2024

LGTM ! Thanks for the changes, gonna be such a life-changing

seaona
seaona reviewed May 27, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@seaona seaona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Some conflicts need to be resolved and update branch with develop, since most of the tests should be fixed

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented May 27, 2024

Builds ready [c44d152]
Page Load Metrics (530 ± 438 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6756611810450
domContentLoaded8331263
load522414530912438
domInteractive8331263
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 7.74 KiB (0.22%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@pedronfigueiredo pedronfigueiredo merged commit 6dcfa71 into develop May 27, 2024
@pedronfigueiredo pedronfigueiredo deleted the pnf/skip-onboarding branch May 27, 2024 14:08
@github-actions github-actions bot locked and limited conversation to collaborators May 27, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label May 27, 2024
@gauthierpetetin gauthierpetetin added the release-12.0.0 Issue or pull request that will be included in release 12.0.0 label Jun 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DDDDDanica DDDDDanica DDDDDanica approved these changes

@seaona seaona seaona approved these changes

@OGPoyraz OGPoyraz OGPoyraz left review comments

+3 more reviewers

@legobeat legobeat legobeat left review comments

@HUAWE33 HUAWE33 HUAWE33 left review comments

@digiwand digiwand digiwand left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@pedronfigueiredo pedronfigueiredo

Labels

release-12.0.0 Issue or pull request that will be included in release 12.0.0 team-extension-platform Extension Platform team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@pedronfigueiredo @seaona @DDDDDanica @metamaskbot @OGPoyraz @digiwand @legobeat @HUAWE33 @gauthierpetetin