Skip to content

Conversation

@mathieuartu
Copy link
Contributor

@mathieuartu mathieuartu commented Dec 11, 2025

Related to: https://consensyssoftware.atlassian.net/browse/MUL-1321

Examples


Note

Implements V2 adapters over legacy hardware keyrings to expose accounts via the unified API.

  • Add TrezorKeyringV2 with capabilities for bip44:derive-index and bip44:derive-path, derivation path parsing/validation (m/44'/60'/0'/0, legacy MEW, SLIP-0044 testnet), account registry caching, and graceful behavior when the device is locked
  • Add OneKeyKeyringV2 extending TrezorKeyringV2 with KeyringType.OneKey
  • Export new wrappers from src/index.ts; update package deps to @metamask/keyring-api and @metamask/account-api, tsconfig references, and README dependency graph
  • Enhance legacy trezor-keyring by exposing getIndexForAddress and refactoring path lookup; keep serialization behavior intact
  • Add comprehensive tests for account listing/creation, derivation paths, deletion, serialization, and locked-device scenarios; raise Jest coverage thresholds

Written by Cursor Bugbot for commit 3432867. This will update automatically on new commits. Configure here.

@mathieuartu mathieuartu self-assigned this Dec 11, 2025
@mathieuartu mathieuartu marked this pull request as ready for review December 15, 2025 14:11
@mathieuartu mathieuartu requested a review from a team as a code owner December 15, 2025 14:11
@mathieuartu mathieuartu force-pushed the feat/trezor-keyring-v2 branch from 0beaae5 to 5dfac85 Compare January 6, 2026 11:42
@mathieuartu mathieuartu force-pushed the feat/trezor-keyring-v2 branch from 5dfac85 to 9d07ffc Compare January 6, 2026 11:49
gantunesr
gantunesr previously approved these changes Jan 7, 2026
@MetaMask MetaMask deleted a comment from cursor bot Jan 14, 2026
When using bip44:derive-path, normalize the derivation path after parsing
to avoid mismatches when the input path has leading zeros in the index
(e.g., 'm/44'/60'/0'/0/007' becomes 'm/44'/60'/0'/0/7').

This ensures the existing account check correctly identifies duplicate
requests even when the path is specified with leading zeros.

Co-authored-by: mathieu.artu <[email protected]>
cursoragent and others added 2 commits January 15, 2026 06:39
- Define individual path constants (BIP44_HD_PATH_PREFIX, LEGACY_MEW_PATH, SLIP0044_TESTNET_PATH)
- Create ALLOWED_HD_PATHS const array from these paths
- Derive AllowedHdPath type from the const array using typeof
- Use simpler regex pattern to extract prefix/index, then validate prefix with isAllowedHdPath helper
- Improves consistency with legacy keyring pattern and makes allowed paths more maintainable

Co-authored-by: mathieu.artu <[email protected]>
@mathieuartu
Copy link
Contributor Author

@cursor following commits 972590d and 913af51 (LedgerKeyringV2 now present in the branch), and knowing that your latest changes introduced those lint errors, please iron out your first pass on this comment (#412 (comment)) and make it better.

Error:   44:30  error  This assertion is unnecessary since it does not change the type of the expression  @typescript-eslint/no-unnecessary-type-assertion
Error:   49:25  error  This assertion is unnecessary since it does not change the type of the expression  @typescript-eslint/no-unnecessary-type-assertion
Error:   54:31  error  This assertion is unnecessary since it does not change the type of the expression  @typescript-eslint/no-unnecessary-type-assertion

@cursor
Copy link

cursor bot commented Jan 15, 2026

Fixed the lint errors by removing the unnecessary as const assertions from the HD path constants. TypeScript already infers template literal strings like `m/44'/60'/0'/0` as their literal type, so the as const assertions were redundant.

Changes:

  • Removed as const from BIP44_HD_PATH_PREFIX, LEGACY_MEW_PATH, and SLIP0044_TESTNET_PATH constant declarations
  • The ALLOWED_HD_PATHS array still uses as const to ensure it's a readonly tuple, which is needed for the AllowedHdPath type derivation

All 91 tests pass and lint now succeeds.


Open in Cursor Open in Web

cursoragent and others added 3 commits January 15, 2026 06:46
TypeScript already infers template literal strings as literal types,
making the 'as const' assertions redundant and triggering lint errors.

Co-authored-by: mathieu.artu <[email protected]>
Similar to Ledger keyring v2 approach - use a single regex pattern that
both validates and parses the derivation path, ensuring it matches one
of the allowed Trezor HD paths:
- m/44'/60'/0'/0/{index} (BIP44 standard)
- m/44'/60'/0'/{index} (legacy MEW)
- m/44'/1'/0'/0/{index} (SLIP0044 testnet)

This removes the need for a separate ALLOWED_HD_PATHS const array and
isAllowedHdPath helper function, making the code more consistent with
the Ledger keyring v2 implementation.

Co-authored-by: mathieu.artu <[email protected]>
@mathieuartu mathieuartu force-pushed the feat/trezor-keyring-v2 branch from fb7ac91 to 6180102 Compare January 15, 2026 07:44
Copy link
Contributor

@ccharly ccharly left a comment

Choose a reason for hiding this comment

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

Awesome 💪

@mathieuartu mathieuartu added this pull request to the merge queue Jan 15, 2026
Merged via the queue into main with commit 59347b0 Jan 15, 2026
37 checks passed
@mathieuartu mathieuartu deleted the feat/trezor-keyring-v2 branch January 15, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants