feat(snap-account-service): add migration logic + keyring v2 support#8732
Conversation
96269d0 to
43c238b
Compare
8452267 to
ce71df1
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 2adb638. Configure here.
|
|
||
| // Remove the legacy Snap keyring after migration. | ||
| log('Removing legacy Snap keyring...'); | ||
| await controller.removeKeyring(legacySnapKeyringEntry.metadata.id); |
There was a problem hiding this comment.
Migration breaks proactive selected account forwarding to snaps
Medium Severity
The #migrate() method removes the legacy snap keyring via controller.removeKeyring(...), but #forwardSelectedAccounts — which is wired to fire on selectedAccountGroupChange, unlock, and account group lifecycle events — still calls getLegacySnapKeyring(). That method is get-or-create, so after migration it silently spins up a new, empty legacy keyring and calls setSelectedAccounts on it. Since no snaps are connected to this empty keyring, proactive selected-account notifications stop reaching any snap after migration completes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2adb638. Configure here.


Explanation
Add migration logic to migrate from the legacy Snap keyring to the new architecture with 1 Snap keyring v2 per Snaps.
Also enhance
:ensureReadyso consumers can safely call:withKeyringV2to get the associated Snap keyring v2 instance before using it.References
N/A
Checklist
Note
High Risk
High risk because it changes Snap account/keyring lifecycle (migration, auto keyring creation, and message routing) which directly affects account persistence and Snap-driven account operations.
Overview
Implements a legacy Snap keyring → per-Snap v2 keyring migration in
SnapAccountService(migrate), designed to be concurrent-safe, retried on failure, and automatically invoked byensureReady.Updates
ensureReadyto run migration first and then atomically create the v2 keyring for a Snap if missing before waiting on platform readiness;handleKeyringSnapMessageis switched to forward messages viaKeyringController:withKeyringV2(with special-casing forGetSelectedAccountsand auto-create onAccountCreated).In
multichain-account-service, Snap-based providers dropKeyringController:withKeyringusage entirely:BaseBip44AccountProviderremoves the v1 helper,SnapAccountProvidernow targetseth-snap-keyring/v2APIs (includingdeleteAccount), and BTC/SOL/TRX/Snap provider tests are updated accordingly; changelogs and dependencies are bumped to reflect these breaking API shifts.Reviewed by Cursor Bugbot for commit 2adb638. Bugbot is set up for automated code reviews on this repo. Configure here.