perf(bitcoin-wallet-snap): reduce batch account creation RPC and state overhead - #221
Open
hmalik88 wants to merge 20 commits into
Open
perf(bitcoin-wallet-snap): reduce batch account creation RPC and state overhead#221hmalik88 wants to merge 20 commits into
hmalik88 wants to merge 20 commits into
Conversation
…2 account deletion
… path AccountUseCases.create had no production caller (KeyringHandler routes keyring_createAccounts through createMany/discover) and emitted the v1 AccountCreated lifecycle event after persisting, with no rollback — a latent orphaned-account bug against keyring v2 clients. Removes create and the now-unused emitAccountCreatedEvent/emitAccountDeletedEvent from SnapClientAdapter and the SnapClient interface.
…h account creation
montelaidev
approved these changes
Aug 28, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Improves Bitcoin account batch creation performance by reducing entropy RPCs and state operations, while updating synchronization and keyring v2 deletion behavior.
Changes:
- Derives account children locally from shared parent entropy.
- Processes account ranges in one batch and reuses state snapshots.
- Adds synchronization coalescing and removes legacy deletion events.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
AccountUseCases.ts |
Batches creation and reuses state snapshots. |
AccountUseCases.test.ts |
Updates batch creation and deletion tests. |
BdkAccountRepository.ts |
Adds batched derivation and snapshot persistence. |
BdkAccountRepository.test.ts |
Tests derivation and snapshot merging. |
SnapClientAdapter.ts |
Removes legacy lifecycle event methods. |
KeyringHandler.ts |
Processes ranges as single batches. |
KeyringHandler.test.ts |
Tests unchunked range creation. |
CronHandler.ts |
Coalesces matching synchronization calls. |
CronHandler.test.ts |
Tests synchronization coalescing. |
entities/snap.ts |
Defines account-state snapshots. |
entities/account.ts |
Extends repository contracts for batching. |
snap.manifest.json |
Updates the bundle checksum. |
CHANGELOG.md |
Documents performance and lifecycle changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+139
to
+141
| const key = `syncSelectedAccounts:${[...accountIds].sort().join(',')}`; | ||
|
|
||
| await this.#syncCoalescer.run(key, async () => { |
Comment on lines
+254
to
+256
| // Reuse the lookup's state snapshot: we're inside the account | ||
| // mutation, so it cannot have been changed by another creation. | ||
| await this.#repository.insertMany(newAccounts, snapshot); |
Contributor
There was a problem hiding this comment.
this one should worth to be considered, a sync may have an impact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves Bitcoin Snap account creation performance, especially for large
keyring_createAccountsranges used during SRP import.The main change is that batch creation now fetches entropy once per distinct parent derivation path, then derives hardened account children locally. This replaces the previous shape where each account creation performed its own
snap_getBip32Entropycall. The insert path also reuses the state snapshot loaded during the existing-account lookup, avoiding duplicate full-state reads inside the same account mutation.This PR also removes the 100-account handler chunking so the requested range is processed as one batch, while still preserving idempotency, result ordering, and synchronization behavior.
What Changed
createManybatch instead of handler-level chunks of 100.AccountDeletedevent emission from the delete flow, matching keyring v2 behavior.Why
Before this change, creating N Bitcoin accounts required roughly N entropy RPCs plus repeated full-state reads/writes per 100-account chunk. For large imports, that made account creation scale poorly and increased contention in the extension/Snap RPC queue.
After this change, the dominant repeated cross-process entropy calls are replaced with one parent entropy fetch plus local child derivation. State I/O is also reduced by reusing the mutation snapshot already loaded for idempotency checks.
Notes
The private parent node is held transiently in Snap memory during batch creation. This is the same trust boundary as the previous per-account implementation, which already fetched private entropy before neutering it. The parent node is not persisted or logged, and account descriptors are still built from neutered child nodes.
Test Plan
yarn workspace @metamask/snap-networks-utils run buildyarn workspace @metamask/bitcoin-wallet-snap run buildyarn workspace @metamask/bitcoin-wallet-snap run testkeyring_createAccountsfor a large range returns accounts in requested index order.References
N/A
Checklist