Skip to content

feat(accounts-controller): use KeyringV1Adapter for SnapKeyring (v2) accounts#8703

Merged
ccharly merged 6 commits intomainfrom
cc/feat/use-keyring-v1-adapter
May 5, 2026
Merged

feat(accounts-controller): use KeyringV1Adapter for SnapKeyring (v2) accounts#8703
ccharly merged 6 commits intomainfrom
cc/feat/use-keyring-v1-adapter

Conversation

@ccharly
Copy link
Copy Markdown
Contributor

@ccharly ccharly commented May 5, 2026

Explanation

I introduced a new v1 adapter for SnapKeyringV2 since the v2 interface has an incompatible getAccounts method that cannot be used by the KeyringController.

So the new SnapKeyringV2 will now be "wrapped" by their v1 adapter AND also be available as normal v2 keyring instances (using the same ref).

This allows us to query them with :getKeyringsByType and to make synchronous calls (something that we will have to change someday) AND to make them compatible with the KeyrignController keyrings lifecycle!

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
Changes how Snap v2 accounts are discovered by unwrapping KeyringV1Adapter instances returned from KeyringController:getKeyringsByType, which can affect account synchronization during KeyringController:stateChange. Dependency bumps (including adding @metamask/keyring-sdk) may introduce subtle behavior changes in keyring interop.

Overview
Snap v2 account lookup is now routed through KeyringV1Adapter. AccountsController’s Snap v2 path (#getAccountFromSnapKeyringV2) now expects KeyringController:getKeyringsByType(KeyringType.Snap) to return v1-adapted keyrings and uses KeyringV1Adapter.unwrap() to reach the underlying SnapKeyringV2 for lookupByAddress and snapId.

Tests were updated to wrap mocked SnapKeyringV2 instances in KeyringV1Adapter, and package deps/changelogs were updated (adds @metamask/keyring-sdk, bumps @metamask/keyring-utils across affected packages, and updates yarn.lock).

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

@socket-security
Copy link
Copy Markdown

socket-security Bot commented May 5, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​metamask/​keyring-utils@​3.2.0 ⏵ 3.3.1100 +110074 +192 +7100
Updated@​metamask/​keyring-sdk@​2.0.2 ⏵ 2.1.175 +110085 +195 +1100

View full report

@ccharly ccharly marked this pull request as ready for review May 5, 2026 20:50
@ccharly ccharly requested review from a team as code owners May 5, 2026 20:50
@ccharly ccharly enabled auto-merge May 5, 2026 20:58
@ccharly ccharly changed the title feat(accounts-controller): use KeyringV1Adapter for SnapKeyringV2 accounts feat(accounts-controller): use KeyringV1Adapter for SnapKeyring (v2) accounts May 5, 2026
@ccharly ccharly added this pull request to the merge queue May 5, 2026
Merged via the queue into main with commit b268cb6 May 5, 2026
366 checks passed
@ccharly ccharly deleted the cc/feat/use-keyring-v1-adapter branch May 5, 2026 21:20
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