Skip to content

Fix wallet connect performance regression with bounded address data fetching#757

Merged
jamespepper81 merged 3 commits into
devfrom
claude/fervent-goodall-2pRUP
May 29, 2026
Merged

Fix wallet connect performance regression with bounded address data fetching#757
jamespepper81 merged 3 commits into
devfrom
claude/fervent-goodall-2pRUP

Conversation

@jamespepper81
Copy link
Copy Markdown
Contributor

Summary

Fixes a critical performance regression in the progressive wallet connect flow where per-address data fetches (txs, utxos, address stats) were firing with unbounded concurrency, flooding the Esplora provider with 429/5xx errors and triggering retry-backoff that stretched wallet loading into minutes. Additionally, the unbounded fetches raced the snapshot build, causing data loss for the last-discovered addresses.

Root cause: The onAddressFound callback in getWalletDataProgressive was immediately fetching per-address data for each discovered address without concurrency limits, and these fetches were not awaited before the snapshot build completed.

Solution:

  1. Extract address discovery (cheap) from per-address data fetching (expensive)
  2. Introduce fetchAddressDataConcurrent() — a shared bounded worker pool (default 6 concurrent) used by both the cached and progressive paths
  3. Reuse /address stats already fetched during discovery to avoid redundant API calls
  4. Wrap progressive discovery in the same timeout as the cached path for parity
  5. Stream partial wallet data to the UI during the fetch phase (throttled every 5 addresses) so the dashboard shows live progress instead of a frozen skeleton

Type of Change

  • Bug fix
  • Refactor / cleanup

Changes

Core Fixes

  • fetchAddressDataConcurrent() (new): Bounded worker pool for fetching txs/utxos/address stats with configurable concurrency. Reuses cached /address responses from discovery to skip redundant API calls.
  • getWalletDataProgressive(): Refactored to separate discovery (collect addresses) from data fetching (bounded pool). Discovery now wrapped in timeout for parity with cached path. Partial wallet data streamed to UI during fetch phase.
  • onAddressFound callback: Now receives stats parameter (the /address response already fetched during discovery) so callers can reuse it instead of re-fetching.
  • fetchWalletSnapshot(): Updated to use the shared fetchAddressDataConcurrent() helper.

Constants

  • PARALLEL_BATCH_SIZE: Increased from 10 to 20 (matches GAP_LIMIT so a full gap window resolves in one round-trip)
  • ADDRESS_DATA_CONCURRENCY: New constant (6) for bounded per-address data fetching

UI Improvements

  • wallet-context.tsx: Only update wallet data when there's content to show (discovery phase emits progress-only updates; dashboard keeps skeleton until data arrives or discovery completes)

Test Plan

  • Unit tests (address-data-concurrency.test.ts): Validates bounded concurrency, data completeness (no dropped addresses even when slow), stats reuse, and UTXO retry logic
  • Regression guards (address-discovery-unit.test.ts): Ensures both paths use the shared fetcher, discovery callback doesn't fetch per-address data, and progressive discovery is wrapped in timeout
  • Validation (validate-initial-check-limit.test.ts): Updated to reflect PARALLEL_BATCH_SIZE = 20
  • Existing tests pass; no breaking changes to public APIs

Checklist

  • npm run typecheck passes
  • npm run lint passes
  • npm run test passes (new tests added)
  • No documentation updates needed (internal refactor)

https://claude.ai/code/session_013r8mPRVH2dz81nP9VBm8uC

claude added 2 commits May 29, 2026 08:37
The fresh-connect path (getWalletDataProgressive) fetched per-address
data inside discovery's onAddressFound callback, which was invoked
without await. This fired 3 Esplora calls per address with unbounded
concurrency, flooding the provider (429/5xx -> retry-backoff) and
racing the snapshot build so the last-discovered addresses' data could
be dropped. For wallets with many addresses this stretched the
connect->dashboard flow into minutes.

Changes:
- Extract the bounded worker-queue (concurrency 6) from
  fetchWalletSnapshot into a shared fetchAddressDataConcurrent helper;
  refactor fetchWalletSnapshot to use it.
- Progressive discovery now only COLLECTS addresses (plus the /address
  stats already fetched during discovery); heavy /txs + /utxo fetching
  runs afterwards via the bounded pool and is awaited before building
  the snapshot, removing both the flood and the race.
- Reuse discovery stats as the address `info`, skipping a redundant
  /address call per address.
- Restore live progress: emit discovery-phase counts (skeleton keeps
  showing "Found N addresses") and stream real data during the fetch
  phase. Guard the wallet-context handler so empty discovery-phase
  updates don't clobber the loading skeleton.
- Wrap progressive discovery in the 4-minute discovery timeout for
  parity with the cached path.
- Bump PARALLEL_BATCH_SIZE to 20 so a full gap window resolves in one
  parallel round-trip.

Adds behavioral tests for the bounded fetcher (concurrency cap, race
fix, stats reuse, utxo retry) and source-level regression guards.

https://claude.ai/code/session_013r8mPRVH2dz81nP9VBm8uC
mock.calls is typed any[][]; the destructured [path]: [string]
parameter annotation didn't satisfy the filter overload, failing
the CI typecheck step. Index the call args directly instead.

https://claude.ai/code/session_013r8mPRVH2dz81nP9VBm8uC
@jamespepper81
Copy link
Copy Markdown
Contributor Author

@claude can you code review the pr 757 and make any changes required

Code review of the progressive-connect change surfaced three issues:

1. (Bug) Discovery fires a final onBatchComplete with isComplete:true
   when address discovery ends - but the heavy per-address fetch still
   follows. emitDiscoveryProgress forwarded that as an empty-data
   isComplete:true update, so the dashboard dropped its loading state
   and showed a zero-balance wallet (clobbering any cached snapshot
   already on screen) before real data arrived. Discovery-phase emits
   now force isComplete:false; only the genuine final update completes.

2. (Hardening) Added a receivedComplete latch in the wallet-context
   onProgress handler so a late or out-of-order partial emit can't
   revert a finished dashboard back to a partial view.

3. (Efficiency) Hoisted `new Set(usedAddresses)` out of the per-tx map
   in buildPartialWalletData; it was rebuilt for every transaction, and
   that function now runs repeatedly as data streams in during connect.

Adds a regression guard that discovery-phase emits never report
isComplete:true.

https://claude.ai/code/session_013r8mPRVH2dz81nP9VBm8uC
Copy link
Copy Markdown
Contributor Author

Ran a code review of the PR. Found one real bug it introduced plus a couple of hardening/perf items — all fixed in 53f0f4f:

1. Premature "complete" with empty data (bug). Address discovery fires a final onBatchComplete with isComplete: true when discovery ends — but the heavy per-address /txs+/utxo fetch still follows. The discovery-phase progress emit was forwarding that through, so the dashboard dropped its loading state and briefly showed a zero-balance / no-tx wallet (and clobbered any cached snapshot already on screen) before the real data arrived seconds later. Fix: discovery-phase emits now force isComplete: false; only the genuine final update completes the load.

2. Late-emit hardening. Added a receivedComplete latch in the onProgress handler so a late/out-of-order partial emit can't revert a finished dashboard back to a partial (lower-balance) view. The fire-and-forget progress emits are ordering-safe as written, but this guards against the fragility cheaply.

3. Efficiency. Hoisted new Set(usedAddresses) out of the per-transaction map in buildPartialWalletData — it was being rebuilt for every transaction, and that function now runs repeatedly as data streams in during connect.

Added a regression guard that discovery-phase emits never report isComplete: true. typecheck, lint, and 60 unit tests pass.

Lower-severity items I considered but left as-is: no streaming feedback for wallets with <5 addresses (the final emit covers it; those load fast), and the pre-existing finalSnapshot! non-null assertion (unchanged by this PR).


Generated by Claude Code

@jamespepper81 jamespepper81 merged commit 3262b66 into dev May 29, 2026
5 checks passed
@jamespepper81 jamespepper81 deleted the claude/fervent-goodall-2pRUP branch May 29, 2026 09:32
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