Skip to content

fix(keyring-snap-bridge): prevent deadlock in createAccounts (v2)#591

Merged
ccharly merged 3 commits into
mainfrom
cc/fix/deadlock
Jul 9, 2026
Merged

fix(keyring-snap-bridge): prevent deadlock in createAccounts (v2)#591
ccharly merged 3 commits into
mainfrom
cc/fix/deadlock

Conversation

@ccharly

@ccharly ccharly commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

We don't need to call saveState here. That was an old trick used by v1 async createAccount flow.

Here, we expect every SnapKeyring.createAccounts to be naturally wrapped in a :withKeyringV2 or :withKeyring call.

This should also have a positive impact in terms of performance since updates can be pinpointed to a specific keyrings, rather than re-persisting everything for 1 keyring change (though, the KeyringController still re-serialize all keyrings today... So that might only be a perf improvement in case of non-change/idempotent calls).

We stumbled upon this bug with the last change we made on the multichain-account-service (wrapping createAccounts call in :withKeyringV2), see:


Note

Medium Risk
Changes when keyring state is persisted for batch account creation; callers that invoke createAccounts outside a :withKeyringV2 transaction would no longer get automatic persistence.

Overview
Fixes a deadlock when batch account creation runs inside a :withKeyringV2 (or :withKeyring) transaction by removing the saveState callback from v2 SnapKeyring.createAccounts after new accounts are written to in-memory state.

createAccounts still updates the keyring and returns new plus existing accounts, but persistence is left to the caller’s transaction wrapper—the same pattern as v1 batch createAccounts, unlike async v1 createAccount which still persists on its own.

Tests and the changelog now document that saveState must not be invoked from createAccounts (success, idempotent, concurrent batches, and rollback paths).

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

@ccharly ccharly marked this pull request as ready for review July 9, 2026 10:15
@ccharly ccharly requested a review from a team as a code owner July 9, 2026 10:15
@ccharly ccharly changed the title fix(keyring-snap-bridge): prevent deadlock in createAccounts fix(keyring-snap-bridge): prevent deadlock in createAccounts (v2) Jul 9, 2026
hmalik88
hmalik88 previously approved these changes Jul 9, 2026
@ccharly ccharly enabled auto-merge July 9, 2026 10:19
@ccharly ccharly added this pull request to the merge queue Jul 9, 2026
Merged via the queue into main with commit f4a3e6e Jul 9, 2026
40 checks passed
@ccharly ccharly deleted the cc/fix/deadlock branch July 9, 2026 10: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.

2 participants