feat: enable Ledger integration by dawnseeker8 · Pull Request #8246 · MetaMask/metamask-mobile

dawnseeker8 · 2024-01-11T05:21:01Z

Description

Previous PR have removed ledger features and its dependancies in main branch, this PR will re-enable thee ledger feature in those modify files.

Related issues

Fixes:

Manual testing steps

In Connect a hareware wallet screen, tap Select a hardware wallet you'd like to use label 7 times to enable ledger feature.
tap ledger icon to enable bluetooth to import account from ledger
Go to metamask test-dapp to test most of functions still work fine with ledger account.

Screenshots/Recordings

Connect a hareware wallet screen.

account dropdown list:

transation

personal sign

Pre-merge author checklist

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.

Also fix the code issue in ledger.ts file.

github-actions · 2024-01-11T05:21:16Z

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.

codecov-commenter · 2024-01-11T05:32:59Z

Codecov Report

Attention: 40 lines in your changes are missing coverage. Please review.

Comparison is base (b0fbb66) 40.80% compared to head (e76150a) 40.90%.

Files	Patch %	Lines
...nts/Views/ConnectHardware/SelectHardware/index.tsx	28.57%	10 Missing ⚠️
app/components/hooks/Ledger/useBluetoothDevices.ts	0.00%	10 Missing ⚠️
app/components/hooks/Ledger/useBluetooth.ts	20.00%	8 Missing ⚠️
app/components/Nav/Main/RootRPCMethodsUI.js	0.00%	5 Missing ⚠️
app/components/Views/LedgerConnect/index.tsx	0.00%	4 Missing ⚠️
...onents/UI/LedgerModals/LedgerConfirmationModal.tsx	0.00%	1 Missing ⚠️
app/components/hooks/Ledger/useLedgerBluetooth.ts	0.00%	1 Missing ⚠️
app/core/Vault.js	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8246      +/-   ##
==========================================
+ Coverage   40.80%   40.90%   +0.10%     
==========================================
  Files        1245     1245              
  Lines       30119    30148      +29     
  Branches     2889     2894       +5     
==========================================
+ Hits        12289    12333      +44     
+ Misses      17127    17112      -15     
  Partials      703      703

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

github-actions · 2024-01-11T05:33:52Z

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/135bbe62-765c-49e8-a3f9-4f1387609db1
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

socket-security · 2024-01-11T07:15:23Z

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

Package	New capabilities	Transitives	Size	Publisher
npm/@consensys/ledgerhq-metamask-keyring@0.0.7	Transitive: environment, filesystem, network	`+95`	28.6 MB
npm/@ledgerhq/logs@6.12.0	None	`0`	36.3 kB	sergii-shkolin
npm/@ledgerhq/react-native-hw-transport-ble@6.32.0	Transitive: environment, eval, filesystem, network, shell, unsafe	`+411`	160 MB	sergii-shkolin
npm/@noble/hashes@1.3.2	None	`0`	747 kB	paulmillr
npm/react-native-ble-plx@3.1.1	Transitive: environment, eval, filesystem, network, shell, unsafe	`+406`	155 MB	dominik-czupryna-withintent

View full report↗︎

socket-security · 2024-01-11T07:15:59Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/@expo/json-file@8.2.37

View full report↗︎

Next steps

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/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

Change Ledger code to remove EthKeyringController, FIx some issue for TypedSignV4

github-actions · 2024-01-27T00:14:24Z

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/35d650cc-e2e4-4395-8866-1f8db01d7d09
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

…nModal

…pace

…gerCommands may be delay to become true.

…ndition during ledger command sending.

gantunesr

LGTM. Left a small nit comment

mikesposito

Looks good!

github-actions · 2024-02-06T12:35:42Z

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c469a6f2-9170-4188-8275-9a6c6f8e2aed
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

gantunesr

lgtm!

gantunesr · 2024-02-06T12:42:01Z

@SocketSecurity ignore npm/@expo/json-file@8.2.37

gantunesr · 2024-02-06T12:42:50Z

The new publisher is part of the maintainers.

vivek-consensys · 2024-02-06T15:45:03Z

QA passed on Android and iOS using QA build:- https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c3c20b4c-eb95-4a9a-979e-ad62f01fe2b1

Test flows include:-

Connect Ledger
Disconnect Ledger
Send transaction using Ledger
Sign typed data v4 using Ledger
Sign permit using Ledger
SIWE using Ledger
Swap using Ledger

sonarqubecloud · 2024-02-07T01:54:22Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
29.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

dawnseeker8 · 2024-02-07T03:51:48Z

Latest e2e pipeline result from bitrise, and they all passed.

e2e smoke tests: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a9090a22-01bc-4b30-aed4-06b7db39a021
regression e2e pipeline: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c14876a1-6cbf-4da1-9628-1f54bacd858a

feat: Reenable Ledger feature by revert previous PR code.

b7917b3

Also fix the code issue in ledger.ts file.

dawnseeker8 added team-accounts-framework Accounts team Run Smoke E2E labels Jan 11, 2024

dawnseeker8 added 2 commits January 11, 2024 14:10

feat: Fix lint:tsc error in pipeline.

a8d9a73

feat: run yarn deduplicate to remove duplicate dependancies.

5670680

dawnseeker8 added 2 commits January 16, 2024 16:51

Merge branch 'main' into feat/reenable-ledger-feature

b0dba79

Merge branch 'main' into feat/reenable-ledger-feature

f9f1622

gantunesr changed the title ~~feat: Reenable Ledger feature by revert previous PR code.~~ feat: enable Ledger integration Jan 23, 2024

Cal-L removed the Run E2E Smoke label Jan 23, 2024

mikesposito reviewed Jan 25, 2024

View reviewed changes

Comment thread app/core/Ledger/Ledger.ts Outdated

Comment thread app/core/Ledger/Ledger.ts Outdated

mikesposito reviewed Jan 25, 2024

View reviewed changes

Comment thread app/core/Ledger/Ledger.ts

dawnseeker8 added 6 commits January 25, 2024 19:52

Merge branch 'main' into feat/reenable-ledger-feature

ff2deeb

feat: Remove all patch code for keyringController 8.1,

a38c460

Change Ledger code to remove EthKeyringController, FIx some issue for TypedSignV4

feat: remove all console code in ledger.ts

ea85e10

Merge branch 'main' into feat/reenable-ledger-feature

e5a0892

feat: push updated podfile.lock

4da6d18

feat; fix the lint dequeue error

494fe4c

mikesposito reviewed Jan 26, 2024

View reviewed changes

Comment thread app/core/Ledger/Ledger.ts

Comment thread app/core/Ledger/Ledger.ts Outdated

gantunesr reviewed Jan 27, 2024

View reviewed changes

Comment thread app/core/Vault.js Outdated

Comment thread app/core/Ledger/Ledger.ts Outdated

Comment thread app/core/Ledger/Ledger.ts

Comment thread app/core/Ledger/Ledger.ts

gantunesr marked this pull request as ready for review January 27, 2024 00:12

gantunesr requested a review from a team as a code owner January 27, 2024 00:12

github-actions Bot added the Run Smoke E2E label Jan 27, 2024

gantunesr assigned dawnseeker8 Jan 27, 2024

gantunesr added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jan 27, 2024

dawnseeker8 added 4 commits February 5, 2024 21:23

feat: Add paddingHorizontal and remove height in the LedgerTransactio…

d35c6f2

…nModal

feat: Update the UI based on Gustova feedback with marginHorizontal s…

971c1ea

…pace

feat: Fix the button not disabled during loading because isSendingLed…

1ff5157

…gerCommands may be delay to become true.

feat: add one more condition in disabled to prevent some edge case co…

755c339

…ndition during ledger command sending.

gantunesr previously approved these changes Feb 6, 2024

View reviewed changes

Comment thread app/core/Ledger/Ledger.ts

feat: Fix the ledger scan click help not consistance with QR hardware

29b4ef4

dawnseeker8 dismissed gantunesr’s stale review via 29b4ef4 February 6, 2024 03:13

dawnseeker8 added 2 commits February 6, 2024 11:20

Merge branch 'main' into feat/reenable-ledger-feature

142dec0

feat: Prettier format the file to make sure it following the standard.

9536aa8

mikesposito previously approved these changes Feb 6, 2024

View reviewed changes

gantunesr added Run Smoke E2E and removed Run Smoke E2E labels Feb 6, 2024

gantunesr previously approved these changes Feb 6, 2024

View reviewed changes

vivek-consensys added QA Passed QA testing has been completed and passed and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Feb 6, 2024

Merge branch 'main' into feat/reenable-ledger-feature

e76150a

dawnseeker8 dismissed stale reviews from gantunesr and mikesposito via e76150a February 7, 2024 01:41

gantunesr approved these changes Feb 7, 2024

View reviewed changes

dawnseeker8 merged commit d2e5846 into main Feb 7, 2024

dawnseeker8 deleted the feat/reenable-ledger-feature branch February 7, 2024 11:36

github-actions Bot locked and limited conversation to collaborators Feb 7, 2024

metamaskbot added the release-7.17.0 Issue or pull request that will be included in release 7.17.0 label Feb 7, 2024

angelcheung22 added the ledger-story label Apr 10, 2024

Uh oh!

Conversation

dawnseeker8 commented Jan 11, 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

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions Bot commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Jan 11, 2024

Uh oh!

socket-security Bot commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

socket-security Bot commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Next steps

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jan 27, 2024

Uh oh!

gantunesr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mikesposito left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 6, 2024

Uh oh!

gantunesr left a comment

Choose a reason for hiding this comment

Uh oh!

gantunesr commented Feb 6, 2024

Uh oh!

gantunesr commented Feb 6, 2024

Uh oh!

vivek-consensys commented Feb 6, 2024

Uh oh!

sonarqubecloud Bot commented Feb 7, 2024

Quality Gate passed

Uh oh!

dawnseeker8 commented Feb 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

dawnseeker8 commented Jan 11, 2024 •

edited

Loading

github-actions Bot commented Jan 11, 2024 •

edited

Loading

codecov-commenter commented Jan 11, 2024 •

edited

Loading

socket-security Bot commented Jan 11, 2024 •

edited

Loading

socket-security Bot commented Jan 11, 2024 •

edited

Loading