Skip to content

fix: batch bulkAcceptShare - #9510

Open
abk404 wants to merge 1 commit into
masterfrom
POL-929
Open

fix: batch bulkAcceptShare #9510
abk404 wants to merge 1 commit into
masterfrom
POL-929

Conversation

@abk404

@abk404abk404 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

fix(sdk-core): batch bulkAcceptShare to prevent WASM OOM in browser

Problem

Users with a large number of shared wallets (e.g. 96) get the following error when clicking "Accept Keys" in Kintsugi:

RangeError: WebAssembly.instantiate(): Out of memory: Cannot allocate Wasm memory for new instance

Root cause:bulkAcceptShare decrypts each wallet's shared keychain using Argon2id — a WASM-based KDF. Each decrypt call creates a fresh WebAssembly.Instance that commits ~64 MB of real memory and reserves ~2 GB of virtual address space. With Promise.all launching all shares simultaneously, a user with 96 wallets triggers ~192 concurrent WASM instances (decrypt + re-encrypt per share) — exhausting the browser tab's virtual address space (~256 GB on V8) and crashing.

This was introduced in WCN-174 (feat: make encrypt/decrypt async) when the original sequential flatMap loop was converted to Promise.all to handle the newly async encrypt/decrypt calls, without accounting for the per-call WASM memory cost.

Before (broken):

// All 96 shares run simultaneously — up to 192 WASM instances at onceconstkeysForWalletShares=(awaitPromise.all(walletShares.map(async(walletShare)=>{constdecryptedPrv=awaitthis.bitgo.decrypt(...)// new WASM instance per callconstnewEncryptedPrv=awaitthis.bitgo.encrypt(...)// another WASM instance...}))).flat();

Fix

Process shares in batches of 16. At most 32 WASM instances are alive at once (~2 GB real, ~64 GB virtual), well within browser limits.

Batch size rationale: Benchmarking shows a Chrome tab on a modern machine hits OOM at ~124 concurrent WASM instances (~248 GB virtual). A typical 8–16 GB machine under normal browser load has a lower effective ceiling. BATCH_SIZE = 16 → 32 concurrent instances → ~2 GB RAM committed — safe across devices while being 4× faster than a conservative batch of 4.

After:

constBATCH_SIZE=16;constkeysForWalletShares: AcceptShareOptionsRequest[]=[];for(constbatchof_.chunk(walletShares,BATCH_SIZE)){constbatchResults=awaitPromise.all(batch.map((walletShare)=>processShare(walletShare)));keysForWalletShares.push(...batchResults.flat());}

Per-share logic is identical — no behaviour change for any individual share, no API changes.

Performance impact

For 96 wallets: 6 batches × ~3s per batch ≈ ~18s vs. a crash. Acceptable for a one-time accept-keys action. Previously when the flow was synchronous (before WCN-174), latency was similar — the async conversion didn't reduce total time for a single user, it just introduced the concurrency bug.

Testing

  • Existing encryptionVersion threading tests continue to pass
  • Added test asserting all 20 shares are processed when count exceeds BATCH_SIZE
  • Added concurrency test asserting max in-flight encrypt calls never exceeds 16

Related

  • Bug introduced: WCN-174
  • Reported via: Bullish (GI) Limited — user with 96 wallets unable to accept keyshares

@linear-code

Copy link
Copy Markdown
Contributor

POL-929

@abk404abk404 changed the title fix: add batching for keys accept decryptfix: batch bulkAcceptShare Aug 17, 2026
@abk404
abk404 marked this pull request as ready for review August 17, 2026 10:41
@abk404
abk404 requested review from a team as code ownersAugust 17, 2026 10:41

@s84krishs84krish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

Sign up for freeto 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.

3 participants

@abk404@davidkaplanbitgo@s84krish