chore: Add eth hd keyring and key tree to decrease unlock time by tommasini · Pull Request #12428 · MetaMask/metamask-mobile

tommasini · 2024-11-26T14:15:33Z

Description

This PR is a draft and it still needs to

~~Remove key-tree patch and update to the new release (still pending)~~
~~Remove eth-hd-keyring patch and install the latest release that includes this PR~~
Updated patch version of Keyring Controller to 17.2.2 (Added a patch to the Keyring Controller awaiting for the generation of random mnemonic, that will be removed once this work is done, in this PR or worst scenario, following up work)

App launch times pipeline: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/5b450f2a-91a3-4a94-8479-729655a2cf0b?tab=workflows

Follow up work:

Remove patch of keyring controller added
Decrease baseline of performance e2e pipeline (app_launch_times) for android and ios (Android less than 2.9 seconds cold app start for wallet view)

Related issues

Fixes:

Manual testing steps

Build used for testing: https://app.bitrise.io/build/6ed111dd-6bbb-4492-a73e-a11347335e8e?tab=log

create new acount generating an SRP, ✅
create a new account when the SRP account is created ✅
https://github.com/user-attachments/assets/71a45633-6b2a-446f-ac7a-6aed99c97b54
import with SRP, ✅
add with private key,✅
https://github.com/user-attachments/assets/5ed51268-7b2a-47c1-abb7-1f1da19bda05
update the app (from v4) with 1 account imported via SRP ,✅
https://github.com/user-attachments/assets/2364461d-869f-40c5-85a4-ef2db3433ceb
update the app with 3 accounts (imported via SRP and private key and added) ,✅
https://github.com/user-attachments/assets/fa452f36-48d8-4573-8c1f-d97a1938baf5

Screenshots/Recordings

Before

After

Pre-merge author checklist

I’ve followed MetaMask Contributor Docs and MetaMask Mobile 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-11-26T14:15:46Z

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.

github-actions · 2024-11-26T14:16:16Z

Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 4ed8536
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/98aac5df-803c-45b9-bd5e-46cb8ba376a1

Note

This comment will auto-update when build completes
You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

socket-security · 2024-11-26T14:16:35Z

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

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/eth-hd-keyring@9.0.0	None	`+2`	1.16 MB	metamaskbot

View full report↗︎

github-actions · 2024-11-26T14:19:31Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 2834fba
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/5351aa81-0e8d-4484-ae5b-55fe6c12e115

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions · 2024-11-26T19:59:19Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e6e8c10
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/77897a67-becb-48b3-9c11-d48bd4809ce1

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions · 2024-11-27T00:13:04Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c1704e9
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/72e36820-36ed-449a-bbdf-2799ce676253

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

app/core/Encryptor/lib.ts

…cenarios. Added patch to keyring controller to make generateRandomMnemonic awaited since it is async in hd-keyring

github-actions · 2024-11-27T12:47:20Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 38a192b
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6394a7c9-6458-4f25-b2b8-910206ec513c

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

mikesposito

I'm thinking that we should also test an update from a previous version, to check that the accounts derived from the mnemonic stay the same

app/core/Encryptor/pbkdf2.ts

Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>

codecov-commenter · 2024-11-28T18:36:41Z

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 57.00%. Comparing base (22a4989) to head (64ed517).
Report is 66 commits behind head on main.

Files with missing lines	Patch %	Lines
app/core/Engine/Engine.ts	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12428      +/-   ##
==========================================
+ Coverage   56.41%   57.00%   +0.58%     
==========================================
  Files        1797     1815      +18     
  Lines       40586    40852     +266     
  Branches     5097     5161      +64     
==========================================
+ Hits        22896    23287     +391     
+ Misses      16134    15981     -153     
- Partials     1556     1584      +28

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

github-actions · 2024-11-29T11:03:51Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 0716d9f
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/17e0ec19-a9c7-4aed-b38b-e4044b384f3e

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions · 2024-12-03T19:14:55Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 1dba714
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/39ac67f8-a293-46c2-99c4-2e6400e1ca38

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

sonarqubecloud · 2024-12-03T19:26:16Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

Cal-L

LGTM

mikesposito

LGTM!

add eth hd keyring and key tree to decrease unlock time

4ed8536

tommasini added Run Smoke E2E team-tiger-deprecated DEPRECATED: team no longer exists labels Nov 26, 2024

metamaskbot added the team-mobile-platform Mobile Platform team label Nov 26, 2024

Merge branch 'main' into chore/add-eth-hd-keyring

2834fba

tommasini added Run Smoke E2E and removed Run Smoke E2E labels Nov 26, 2024

tommasini and others added 2 commits November 26, 2024 19:56

update types and tests of encryptor

1bc2376

Merge branch 'main' into chore/add-eth-hd-keyring

e6e8c10

tommasini added Run Smoke E2E and removed Run Smoke E2E labels Nov 26, 2024

tommasini and others added 3 commits November 27, 2024 00:04

remove patches and update eth hd keyring to v9

3e0c818

deduplicate

e3d3f5a

Merge branch 'main' into chore/add-eth-hd-keyring

c1704e9

tommasini added Run Smoke E2E and removed Run Smoke E2E labels Nov 27, 2024

FrederikBolding reviewed Nov 27, 2024

View reviewed changes

app/core/Encryptor/lib.ts Outdated Show resolved Hide resolved

tommasini and others added 3 commits November 27, 2024 12:43

Isolate pbkdf2 in a single file, add test coverage to test multiple s…

e3970d3

…cenarios. Added patch to keyring controller to make generateRandomMnemonic awaited since it is async in hd-keyring

Merge branch 'main' into chore/add-eth-hd-keyring

66fda61

remove unnecessary tsdocs in test file

38a192b

tommasini added Run Smoke E2E and removed Run Smoke E2E labels Nov 27, 2024

mikesposito reviewed Nov 27, 2024

View reviewed changes

app/core/Encryptor/pbkdf2.ts Outdated Show resolved Hide resolved

app/core/Encryptor/pbkdf2.ts Outdated Show resolved Hide resolved

tommasini and others added 7 commits November 28, 2024 15:06

Update app/core/Encryptor/pbkdf2.test.ts

2d533a0

Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>

Update app/core/Encryptor/pbkdf2.test.ts

2cc6d96

Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>

Update app/core/Encryptor/pbkdf2.test.ts

085e8e1

Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>

Update app/core/Encryptor/pbkdf2.test.ts

d901073

Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>

Update app/util/bytes.test.ts

9fdc49b

Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>

rename bytesToBits to bytesLengthToBitsLength

6447f42

Merge branch 'main' into chore/add-eth-hd-keyring

64ed517

Merge branch 'main' into chore/add-eth-hd-keyring

0716d9f

tommasini added Run Smoke E2E and removed Run Smoke E2E labels Nov 29, 2024

Merge branch 'main' into chore/add-eth-hd-keyring

1dba714

tommasini added Run Smoke E2E and removed Run Smoke E2E labels Dec 3, 2024

Cal-L approved these changes Dec 3, 2024

View reviewed changes

tommasini enabled auto-merge December 3, 2024 21:30

mikesposito approved these changes Dec 3, 2024

View reviewed changes

owencraston approved these changes Dec 3, 2024

View reviewed changes

tommasini added this pull request to the merge queue Dec 3, 2024

Merged via the queue into main with commit dc310af Dec 3, 2024

tommasini deleted the chore/add-eth-hd-keyring branch December 3, 2024 23:22

github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Dec 3, 2024

github-actions bot locked and limited conversation to collaborators Dec 3, 2024

metamaskbot added the release-7.38.0 Issue or pull request that will be included in release 7.38.0 label Dec 3, 2024

Uh oh!

Conversation

tommasini commented Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Nov 26, 2024

Uh oh!

github-actions bot commented Nov 26, 2024

Bitrise

Uh oh!

socket-security bot commented Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 26, 2024 • edited by metamaskbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitrise

Uh oh!

github-actions bot commented Nov 26, 2024 • edited by metamaskbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitrise

Uh oh!

github-actions bot commented Nov 27, 2024 • edited by metamaskbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitrise

Uh oh!

Uh oh!

github-actions bot commented Nov 27, 2024 • edited by metamaskbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitrise

Uh oh!

mikesposito left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Nov 28, 2024

Codecov Report

Uh oh!

github-actions bot commented Nov 29, 2024 • edited by metamaskbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitrise

Uh oh!

github-actions bot commented Dec 3, 2024 • edited by metamaskbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitrise

Uh oh!

sonarqubecloud bot commented Dec 3, 2024

Quality Gate passed

Uh oh!

Cal-L left a comment

Choose a reason for hiding this comment

Uh oh!

mikesposito left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

tommasini commented Nov 26, 2024 •

edited

Loading

socket-security bot commented Nov 26, 2024 •

edited

Loading

github-actions bot commented Nov 26, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Nov 26, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Nov 27, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Nov 27, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Nov 29, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Dec 3, 2024 •

edited by metamaskbot

Loading