Skip to content

refactor(multichain-account-service)!: replace withKeyring usage with withKeyringV2#8491

Merged
hmalik88 merged 32 commits intomainfrom
hm/mul-1545
May 4, 2026
Merged

refactor(multichain-account-service)!: replace withKeyring usage with withKeyringV2#8491
hmalik88 merged 32 commits intomainfrom
hm/mul-1545

Conversation

@hmalik88
Copy link
Copy Markdown
Contributor

@hmalik88 hmalik88 commented Apr 16, 2026

Explanation

Changes

Base provider & types

  • Add KeyringControllerWithKeyringV2Action to the messenger's AllowedActions
  • Split BaseBip44AccountProvider.withKeyring into two helpers:
  • withKeyring; calls KeyringController:withKeyring (V1). Used by SnapAccountProvider until its V2 migration lands.
  • withKeyringV2<SelectedKeyring extends KeyringV2>; calls KeyringController:withKeyringV2. Used by the EVM provider.

EvmAccountProvider

  • All call sites moved to withKeyringV2
  • Callbacks use V2 keyring methods: getAccounts() returns KeyringAccount[], createAccounts({ type: 'bip44:derive-index', entropySource, groupIndex }) replaces addAccounts()
  • Range creation loops with bip44:derive-index since HdKeyringV2 does not support bip44:derive-index-range
  • #getAddressFromGroupIndex is now typed against HdKeyring from @metamask/eth-hd-keyring/v2 and peeks via keyring.root.deriveChild(groupIndex) (the V2 wrapper forwards the legacy root getter)
  • discoverAccounts keeps the V1-style peek-then-create pattern: peek the address → check on-chain activity outside the keyring mutex → create the account only if there is activity → look up the InternalAccount from AccountsController after the withKeyringV2 callback completes
  • KeyringCapabilities and Keyring types now come from @metamask/keyring-api/v2

References

N/a

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Moderate risk because it changes the EVM account-creation and discovery path to use the V2 keyring interface and different keyring methods; regressions could affect account derivation/indexing and discovery behavior.

Overview
BREAKING: Migrates the EVM account provider from KeyringController:withKeyring (V1) to KeyringController:withKeyringV2 (V2) and updates types/messenger wiring to allow the new action.

EvmAccountProvider now uses V2 keyring APIs (getAccounts returning KeyringAccount[], createAccounts replacing addAccounts, and deriving IDs from returned accounts) and changes range creation to loop per index (since HdKeyringV2 doesn’t support range creation). Discovery now “peeks” the derived address via the V2 HD keyring root before creating the account, and adds explicit assertions/error handling when keyring creation returns no account.

Tests were updated accordingly, including new V2 keyring mocks and deterministic derivation fixtures, and the changelog documents the breaking migration.

Reviewed by Cursor Bugbot for commit cdf3ac5. Bugbot is set up for automated code reviews on this repo. Configure here.

@hmalik88 hmalik88 changed the title refactor(multichain-account-service): replace withKeyring usage with withKeyringV2 refactor(multichain-account-service)!: replace withKeyring usage with withKeyringV2 Apr 16, 2026
@hmalik88 hmalik88 marked this pull request as ready for review April 16, 2026 20:45
@hmalik88 hmalik88 requested review from a team as code owners April 16, 2026 20:45
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.ts Outdated
Comment thread packages/multichain-account-service/CHANGELOG.md Outdated
Comment thread packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.ts Outdated
Comment thread packages/multichain-account-service/src/MultichainAccountService.test.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.ts Outdated
Comment thread packages/multichain-account-service/src/providers/EvmAccountProvider.ts Outdated
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit efd0921. Configure here.

@hmalik88 hmalik88 enabled auto-merge May 4, 2026 16:28
@hmalik88 hmalik88 added this pull request to the merge queue May 4, 2026
Merged via the queue into main with commit 83eb70c May 4, 2026
366 checks passed
@hmalik88 hmalik88 deleted the hm/mul-1545 branch May 4, 2026 16:33
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.

2 participants