MV3 Hardware Wallet Changes #17608

andrewpeters9 · 2023-02-05T18:24:35Z

❗️This MR currently serves as a backup to #17687 , don't spend time reviewing this unless you're wanting to look at the ideas that were previously proposed

Currently a draft. See the below GH Issue for an explanation/diagrams etc.

# Conflicts: # app/scripts/metamask-controller.js

# Conflicts: # .yarn/patches/@MetaMask+eth-ledger-bridge-keyring+0.13.0.patch # .yarn/patches/eth-keyring-controller+8.1.0.patch # .yarn/patches/eth-trezor-keyring+0.10.0.patch # app/scripts/controllers/preferences.js

# Conflicts: # app/scripts/controllers/preferences.js # app/scripts/metamask-controller.js # app/scripts/migrations/066.js # package.json # shared/constants/hardware-wallets.js # yarn.lock

# Conflicts: # app/scripts/controllers/preferences.js

# Conflicts: # ui/store/action-queue/index.ts

github-actions · 2023-02-05T18:24:47Z

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.

andrewpeters9 · 2023-02-06T13:04:09Z

.yarn/patches/@metamask-eth-ledger-bridge-keyring-npm-0.13.0-189684913d.patch

@@ -0,0 +1,169 @@
+diff --git a/index.js b/index.js


Ignore these patches - will be replaced with package updates. Discussions on changes are happening in their respective repos

andrewpeters9 · 2023-02-06T13:05:20Z

.yarn/patches/eth-trezor-keyring-npm-0.10.0-1bf9aa3f4d.patch

@@ -0,0 +1,87 @@
+diff --git a/index.js b/index.js


Ignore this patch - a discussion regarding changes is occurring in the related repo.

andrewpeters9 · 2023-02-06T13:06:15Z

app/scripts/controllers/keyring-events/handlers.js

+  },
+  backgroundSync: async (keyring, newState) => {
+    await keyring.deserialize(newState);
+    console.log('⬆️ State update for keyring', keyring.type, newState);


Ignore these console logs - they'll be removed post-draft.

andrewpeters9 · 2023-02-06T13:09:40Z

app/scripts/controllers/keyring-events/utils.ts

+ *
+ * @param error
+ */
+export const isServiceWorkerMv3Error = (error: any): boolean => {


I'm in two minds as to whether this should occur, and instead, we could just pass every error that occurs on the backend to the frontend.

andrewpeters9 · 2023-02-06T13:13:50Z

app/scripts/controllers/keyring-events/utils.ts

+  if (isUserSet) {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
+    return isMv3ErrorMessage(error.cause.message);


Error.cause isn't implemented in the current TS version, and seems like lodash's type-narrowing doesn't help TS much here.

Might be worth creating a separate issue for a TS extension lib, such as https://github.com/andnp/SimplyTyped to aid with type-narrowing (as it has a helper isKeyOf that assists with these situations)

andrewpeters9 · 2023-02-06T13:30:29Z

ui/store/client-keyrings.js

+  }
+
+  // @TODO, handle responding to specific JSON-RPC messages
+  clientKeyringController.handleMethodCall(params);


@todo, pending resolution of an issue regarding transaction confirmation, this context of the function will only respond to specific JSON-RPC messages, so that multiple clients aren't being called simultaneously.

socket-security · 2023-02-08T13:23:37Z

Socket Security Pull Request Report

Dependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again.

😵‍💫 Bin script confusion

This package has multiple bin scripts with the same name. This can cause non-deterministic behavior when installing or could be a sign of a supply chain attack

Consider removing one of the conflicting packages. Packages should only export bin scripts with their name

Package	Bin script	Source
jest@29.4.2 (upgraded)	`jest`	`package.json` via @storybook/test-runner@0.9.4
jest-cli@28.1.3 (upgraded)	`jest`	`package.json` via @storybook/test-runner@0.9.4
jest-cli@29.4.2 (upgraded)	`jest`	`package.json` via @storybook/test-runner@0.9.4, jest@29.4.2

Pull request report summary

Issue	Status
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script confusion	⚠️ 3 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
Potential typo squat	✅ 0 issues
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@2.4.2

@SocketSecurity ignore jest@29.4.2
@SocketSecurity ignore jest-cli@28.1.3
@SocketSecurity ignore jest-cli@29.4.2

Powered by socket.dev

brad-decker · 2023-04-20T18:28:20Z

@andrewpeters9 can this be updated to be considered for inclusion? Otherwise can we close?

andrewpeters9 · 2023-04-21T13:03:09Z

@andrewpeters9 can this be updated to be considered for inclusion? Otherwise can we close?

@brad-decker Could we leave this open until #17687 is no longer a draft or merged? As it's intended to be a backup for that MR in the case that MR's approach can't be implemented for whatever reason.

github-actions · 2023-07-20T11:50:05Z

This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions.

github-actions · 2023-08-03T12:05:28Z

This PR was closed because there has been no follow up activity in the last 14 days. Thank you for your contributions.

andrewpeters9 added 30 commits October 27, 2022 01:17

proof of concept - WIP

da6f595

small fixes

76d75f3

return more appropriate errors

e789cd7

cleanup, changes to initialization

693a708

Merge branch 'develop' into hardware-wallet-fixes

f0983d2

refactoring of keyring events controller - wip

ceb7493

backend cleaned up

adcf6e0

General cleanup of KeyringEventsController

1c11c7e

init methods forcefully invoked client-side

1ed68ce

ignore methods elaborated, client-side cleanup

8e95997

gas fixes

f71da2b

force webHid if mv3

dd29817

transaction fixes

ca12c35

deserialization and serialization cleanup

30a9382

Merge remote-tracking branch 'origin/develop' into hardware-wallet-fixes

2ff493c

refactoring, small improvements and comments

8497735

Merge branch 'develop' into hardware-wallet-fixes

99f48c2

# Conflicts: # app/scripts/metamask-controller.js

new patches, refactoring, vendor script fixes

61b863a

updated patches, performance improvements

05db0bc

Merge remote-tracking branch 'origin/develop' into hardware-wallet-fixes

22aeddc

patch update

e366bd0

refactoring and package patches

ebfe36a

small refactorings and comment removal

bf46ef4

remove unused import

4883d5b

patches migrated

4596f46

lock file change commit

27bcdf4

lattice fixes and testing

3837380

small CSS fix for error messages

1f1dbaa

Merge remote-tracking branch 'origin/develop' into hardware-wallet-fixes

a85165f

# Conflicts: # app/scripts/controllers/preferences.js # app/scripts/metamask-controller.js # app/scripts/migrations/066.js # package.json # shared/constants/hardware-wallets.js # yarn.lock

andrewpeters9 added 8 commits January 31, 2023 14:06

lattice patch applied

338264d

duplicate data removed

2e6f9cc

Merge remote-tracking branch 'origin/develop' into hardware-wallet-fixes

e3cbf93

# Conflicts: # app/scripts/controllers/preferences.js

ts migration for constants

13a25cb

refreshing

ee56138

Merge remote-tracking branch 'origin/develop' into hardware-wallet-fixes

bb0bf6b

# Conflicts: # ui/store/action-queue/index.ts

keyring-controller fixes

4e9ba4b

small method fix

5398762

andrewpeters9 requested a review from a team as a code owner February 5, 2023 18:24

andrewpeters9 requested a review from pedronfigueiredo February 5, 2023 18:24

andrewpeters9 marked this pull request as draft February 5, 2023 18:56

remove scss change

c9c4465

andrewpeters9 added area-hardware MV3 MV3-non_perf MV3 issues not related to performance. labels Feb 6, 2023

andrewpeters9 linked an issue Feb 6, 2023 that may be closed by this pull request

[Bug]: [MV3] Hardware Wallet Support #16217

Closed

andrewpeters9 added 2 commits February 6, 2023 14:18

diff fixes

a93e8fa

todo’s fixed

0c3714a

andrewpeters9 commented Feb 6, 2023

View reviewed changes

Update add-to-chrome.md

71baee8

github-actions bot added the stale issues and PRs marked as stale label Jul 20, 2023

github-actions bot closed this Aug 3, 2023

HowardBraham deleted the hardware-wallet-fixes branch January 19, 2026 19:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MV3 Hardware Wallet Changes #17608

MV3 Hardware Wallet Changes #17608

andrewpeters9 commented Feb 5, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Feb 5, 2023

Uh oh!

andrewpeters9 Feb 6, 2023

Uh oh!

andrewpeters9 Feb 6, 2023

Uh oh!

andrewpeters9 Feb 6, 2023

Uh oh!

andrewpeters9 Feb 6, 2023

Uh oh!

andrewpeters9 Feb 6, 2023

Uh oh!

andrewpeters9 Feb 6, 2023

Uh oh!

socket-security bot commented Feb 8, 2023

Uh oh!

brad-decker commented Apr 20, 2023

Uh oh!

andrewpeters9 commented Apr 21, 2023

Uh oh!

github-actions bot commented Jul 20, 2023

Uh oh!

github-actions bot commented Aug 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MV3 Hardware Wallet Changes #17608

MV3 Hardware Wallet Changes #17608

Conversation

andrewpeters9 commented Feb 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❗️This MR currently serves as a backup to #17687 , don't spend time reviewing this unless you're wanting to look at the ideas that were previously proposed

Currently a draft. See the below GH Issue for an explanation/diagrams etc.

Uh oh!

github-actions bot commented Feb 5, 2023

Uh oh!

andrewpeters9 Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

andrewpeters9 Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

andrewpeters9 Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

andrewpeters9 Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

andrewpeters9 Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

andrewpeters9 Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

socket-security bot commented Feb 8, 2023

Socket Security Pull Request Report

Uh oh!

brad-decker commented Apr 20, 2023

Uh oh!

andrewpeters9 commented Apr 21, 2023

Uh oh!

github-actions bot commented Jul 20, 2023

Uh oh!

github-actions bot commented Aug 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrewpeters9 commented Feb 5, 2023 •

edited

Loading