From 0bfa15d7b0feea49fcdba499d63d4655e272fec4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 08:37:25 +0000 Subject: [PATCH 1/3] Fix slow XPUB connect: bound per-address fetch concurrency 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 --- src/contexts/wallet-context.tsx | 15 +- src/lib/blockchain.ts | 254 ++++++++++++++------- tests/address-data-concurrency.test.ts | 134 +++++++++++ tests/address-discovery-unit.test.ts | 36 +++ tests/validate-initial-check-limit.test.ts | 4 +- 5 files changed, 362 insertions(+), 81 deletions(-) create mode 100644 tests/address-data-concurrency.test.ts diff --git a/src/contexts/wallet-context.tsx b/src/contexts/wallet-context.tsx index b217f4a0..3148147b 100644 --- a/src/contexts/wallet-context.tsx +++ b/src/contexts/wallet-context.tsx @@ -768,11 +768,22 @@ export const WalletProvider = ({ children, testXpub }: { children: ReactNode; te // Update discovery progress setDiscoveryProgress(partialData.discoveryProgress); - + // Update wallet data with partial results // Convert PartialWalletData to WalletData by removing progressive fields const { discoveryProgress, isComplete, ...walletData } = partialData; - setData(walletData as WalletData); + // During the discovery phase we receive progress-only updates with no + // wallet data yet. Don't clobber the loading skeleton with an empty + // snapshot - only surface data once there's something to show (or when + // discovery has completed, which also covers the empty-wallet case). + const hasContent = + isComplete || + walletData.transactions.length > 0 || + walletData.balanceBTC > 0 || + walletData.usedAddressCount > 0; + if (hasContent) { + setData(walletData as WalletData); + } // If complete, mark as no longer discovering if (partialData.isComplete) { diff --git a/src/lib/blockchain.ts b/src/lib/blockchain.ts index 6d97fdd4..bf6bfd08 100644 --- a/src/lib/blockchain.ts +++ b/src/lib/blockchain.ts @@ -16,9 +16,10 @@ type AddressType = 'native' | 'legacy' | 'nested'; // Performance-optimized address discovery constants // The discovery process is the main bottleneck during wallet login/connection -const GAP_LIMIT = 20; // Standard gap limit for address discovery +const GAP_LIMIT = 20; // Standard BIP44 gap limit for address discovery const INITIAL_CHECK_LIMIT = 5; // Number of addresses to check per type for detection (5 provides better accuracy than 3) -const PARALLEL_BATCH_SIZE = 10; // How many addresses to check in parallel +const PARALLEL_BATCH_SIZE = 20; // How many addresses to check in parallel (matches GAP_LIMIT so a full gap window resolves in one round-trip) +const ADDRESS_DATA_CONCURRENCY = 6; // Bounded concurrency for per-address data fetches (avoids provider throttling) const ADDRESS_DISCOVERY_CACHE_TTL_MS = 10 * 60 * 1000; // Cache discovered addresses for 10 minutes const XPUB_LOG_PREFIX_LENGTH = 12; // How many characters of XPUB to show in logs (balance of readability vs privacy) @@ -155,7 +156,9 @@ export interface DiscoveryProgress { } export interface ProgressiveDiscoveryCallbacks { - onAddressFound?: (address: string, txCount: number) => void; + // `stats` is the raw esplora /address response already fetched during discovery, + // passed through so callers can reuse it instead of re-fetching /address/{addr}. + onAddressFound?: (address: string, txCount: number, stats?: any) => void; onBatchComplete?: (progress: DiscoveryProgress) => void; } @@ -217,9 +220,10 @@ async function performDiscoveryForTypesProgressive( gap = 0; foundInBatch = true; - // Notify callback about found address + // Notify callback about found address, passing the stats we + // already fetched so the caller can avoid a redundant /address call. if (callbacks?.onAddressFound) { - callbacks.onAddressFound(batch[i], totalTxCount); + callbacks.onAddressFound(batch[i], totalTxCount, stats); } } else { gap++; @@ -480,32 +484,41 @@ function createEmptySnapshot(xpub: string): WalletSnapshot { }; } -async function fetchWalletSnapshot(xpub: string, currency: Currency): Promise { - const usedAddresses = await getCachedUsedAddresses(xpub); - - if (usedAddresses.length === 0) { - return createEmptySnapshot(xpub); - } - - // Fetch non-critical data, allowing for graceful failure. - const latestBlockHeight = await esploraGet(`/blocks/tip/height`, 60).catch(() => null); - - const allTxs = new Map(); - const utxos: UTXO[] = []; - const addressInfos: AddressInfo[] = []; +/** + * Fetch txs/utxos/info for a set of addresses using a bounded worker pool. + * + * Concurrency is capped (default {@link ADDRESS_DATA_CONCURRENCY}) to avoid + * flooding the Esplora provider, which otherwise responds with 429/5xx and + * triggers retry-backoff that stretches wallet loading into minutes. + * + * When `statsByAddress` already contains the `/address` response for an address + * (e.g. fetched during discovery), it is reused as `info` and the redundant + * `/address/{addr}` call is skipped. + */ +export async function fetchAddressDataConcurrent( + addresses: string[], + statsByAddress?: Map, + concurrency: number = ADDRESS_DATA_CONCURRENCY, + onAddressProcessed?: (processed: number, soFar: Map) => void +): Promise> { + const result = new Map(); + if (addresses.length === 0) return result; - // Fetch all data for used addresses with limited concurrency to avoid provider throttling - const concurrency = 6; let idx = 0; + let processed = 0; async function worker() { - while (idx < usedAddresses.length) { - const address = usedAddresses[idx++]; + while (idx < addresses.length) { + const address = addresses[idx++]; try { - const [txs, initialAddressUtxos, addressInfo] = await Promise.all([ + const cachedInfo = statsByAddress?.get(address); + const [txs, initialAddressUtxos, fetchedInfo] = await Promise.all([ esploraGet(`/address/${address}/txs`).catch(() => []), esploraGet(`/address/${address}/utxo`).catch(() => []), - esploraGet(`/address/${address}`).catch(() => null) + cachedInfo !== undefined + ? Promise.resolve(cachedInfo) + : esploraGet(`/address/${address}`).catch(() => null), ]); + const addressInfo = fetchedInfo; // If the address shows a positive balance but the first UTXO call failed, // try a dedicated retry to avoid silently undercounting UTXOs. @@ -518,27 +531,60 @@ async function fetchWalletSnapshot(xpub: string, currency: Currency): Promise 0 ? await esploraGet(`/address/${address}/utxo`).catch(() => []) : []; - if (Array.isArray(txs)) { - txs.forEach((tx: any) => allTxs.set(tx.txid, tx)); - } - if (Array.isArray(addressUtxos)) { - addressUtxos.forEach((utxo: any) => { - utxos.push({ txid: utxo.txid, vout: utxo.vout, address, value: utxo.value }); - }); - } - if (addressInfo && addressInfo.chain_stats?.tx_count > 0) { - addressInfos.push({ - address, - n_tx: addressInfo.chain_stats.tx_count, - balance: addressInfo.chain_stats.funded_txo_sum - addressInfo.chain_stats.spent_txo_sum, - }); - } + + result.set(address, { + txs: Array.isArray(txs) ? txs : [], + utxos: Array.isArray(addressUtxos) ? addressUtxos : [], + info: addressInfo, + }); } catch { // Skip this address on failure + } finally { + processed++; + onAddressProcessed?.(processed, result); } } } - await Promise.all(Array.from({ length: Math.min(concurrency, usedAddresses.length) }, () => worker())); + await Promise.all( + Array.from({ length: Math.min(concurrency, addresses.length) }, () => worker()) + ); + + return result; +} + +async function fetchWalletSnapshot(xpub: string, currency: Currency): Promise { + const usedAddresses = await getCachedUsedAddresses(xpub); + + if (usedAddresses.length === 0) { + return createEmptySnapshot(xpub); + } + + // Fetch non-critical data, allowing for graceful failure. + const latestBlockHeight = await esploraGet(`/blocks/tip/height`, 60).catch(() => null); + + const allTxs = new Map(); + const utxos: UTXO[] = []; + const addressInfos: AddressInfo[] = []; + + // Fetch all data for used addresses with bounded concurrency to avoid provider throttling + const addressData = await fetchAddressDataConcurrent(usedAddresses); + for (const [address, data] of addressData) { + if (Array.isArray(data.txs)) { + data.txs.forEach((tx: any) => allTxs.set(tx.txid, tx)); + } + if (Array.isArray(data.utxos)) { + data.utxos.forEach((utxo: any) => { + utxos.push({ txid: utxo.txid, vout: utxo.vout, address, value: utxo.value }); + }); + } + if (data.info && data.info.chain_stats?.tx_count > 0) { + addressInfos.push({ + address, + n_tx: data.info.chain_stats.tx_count, + balance: data.info.chain_stats.funded_txo_sum - data.info.chain_stats.spent_txo_sum, + }); + } + } const transactionDates = Array.from(allTxs.values()) .map(tx => new Date(tx.status.confirmed ? tx.status.block_time * 1000 : Date.now())); @@ -785,40 +831,64 @@ export async function getWalletDataProgressive( } } - // Progressive address discovery with real-time updates + // Progressive address discovery with real-time updates. + // Discovery only COLLECTS addresses (cheap); the heavy /txs + /utxo + // fetching happens afterwards via a bounded worker pool. This avoids the + // previous behaviour where an un-awaited per-address fetch fired 3 calls + // per address with unbounded concurrency, flooding the provider (429/5xx + // → retry-backoff) and racing the snapshot build. const discoveredAddresses: string[] = []; - const addressDataMap = new Map(); - + const statsByAddress = new Map(); + const latestBlockHeight = await esploraGet(`/blocks/tip/height`, 60).catch(() => null); - - await discoverUsedAddressesProgressive(xpub, { - onAddressFound: async (address, txCount) => { - console.log(`[Progressive] Found address ${address} with ${txCount} transactions`); - discoveredAddresses.push(address); - - // Fetch data for this address immediately for final assembly - // Note: Not calling onProgress here to avoid server/client boundary issues - try { - const [txs, utxos, info] = await Promise.all([ - esploraGet(`/address/${address}/txs`).catch(() => []), - esploraGet(`/address/${address}/utxo`).catch(() => []), - esploraGet(`/address/${address}`).catch(() => null) - ]); - - addressDataMap.set(address, { txs, utxos, info }); - } catch (err) { - console.error(`[Progressive] Failed to fetch data for address ${address}:`, err); - } - }, - onBatchComplete: (progress) => { - console.log(`[Progressive] Batch complete - checked: ${progress.addressesChecked}, found: ${progress.addressesWithActivity}`); - } - }); - + + // Emit a lightweight discovery-phase progress update. It carries no wallet + // data yet (only the running counts), so the dashboard keeps its skeleton + // and shows a live "Found N addresses" counter while discovery runs. + const emitDiscoveryProgress = (progress: DiscoveryProgress) => { + if (!onProgress) return; + void buildPartialWalletData(xpub, [], btcPrices, currency, latestBlockHeight, progress) + .then((partial) => onProgress(partial)) + .catch(() => { /* progress emission is best-effort */ }); + }; + + // Wrap discovery in the same timeout the cached path uses so a stuck + // provider fails gracefully instead of hanging the connect flow. + let discoveryTimeoutId: ReturnType | undefined; + let discoveryReturned: string[] = []; + try { + discoveryReturned = await Promise.race([ + discoverUsedAddressesProgressive(xpub, { + onAddressFound: (address, _txCount, stats) => { + discoveredAddresses.push(address); + if (stats !== undefined) statsByAddress.set(address, stats); + }, + onBatchComplete: (progress) => { + console.log(`[Progressive] Batch complete - checked: ${progress.addressesChecked}, found: ${progress.addressesWithActivity}`); + emitDiscoveryProgress(progress); + }, + }), + new Promise((_, reject) => { + discoveryTimeoutId = setTimeout( + () => reject(new Error(`Address discovery timed out after ${DISCOVERY_TIMEOUT_MINUTES} minutes. The blockchain provider may be slow or unavailable. Please try again.`)), + DISCOVERY_TIMEOUT_MS + ); + }), + ]); + } finally { + if (discoveryTimeoutId) clearTimeout(discoveryTimeoutId); + } + + // Prefer the deduped list returned by discovery (it merges addresses found + // under multiple types during the ambiguous-xpub fallback). + const uniqueAddresses = discoveryReturned.length > 0 + ? discoveryReturned + : Array.from(new Set(discoveredAddresses)); + // Discovery complete - build final wallet data - console.log(`[Progressive] Discovery complete - ${discoveredAddresses.length} addresses found`); - - if (discoveredAddresses.length === 0) { + console.log(`[Progressive] Discovery complete - ${uniqueAddresses.length} addresses found`); + + if (uniqueAddresses.length === 0) { const emptySnapshot = createEmptySnapshot(xpub); setCachedSnapshot(emptySnapshot); const emptyWalletData = await assembleFinalWalletData(emptySnapshot, btcPrices, currency); @@ -841,6 +911,36 @@ export async function getWalletDataProgressive( return { data: emptyWalletData, error: null }; } + // Fetch per-address data with bounded concurrency, reusing the /address + // stats already gathered during discovery (so we don't re-fetch them). + // Real wallet data is streamed to the UI as addresses are processed. + const totalToFetch = uniqueAddresses.length; + const addressDataMap = await fetchAddressDataConcurrent( + uniqueAddresses, + statsByAddress, + ADDRESS_DATA_CONCURRENCY, + (processed, soFar) => { + if (!onProgress) return; + // Throttle re-renders: emit every few addresses (and never on the + // last one, which the final isComplete update below covers). + if (processed % 5 === 0 && processed < totalToFetch) { + void buildPartialWalletData( + xpub, + Array.from(soFar.entries()), + btcPrices, + currency, + latestBlockHeight, + { + addressesChecked: totalToFetch, + addressesWithActivity: totalToFetch, + currentBatch: Math.ceil(processed / GAP_LIMIT), + isComplete: false, + } + ).then((partial) => onProgress(partial)).catch(() => { /* best-effort */ }); + } + } + ); + // Build final snapshot and cache it const finalSnapshot = await buildSnapshotFromAddressData( xpub, @@ -848,22 +948,22 @@ export async function getWalletDataProgressive( currency, latestBlockHeight ); - + if (finalSnapshot) { setCachedSnapshot(finalSnapshot); } - + // Assemble final wallet data const finalWalletData = await assembleFinalWalletData(finalSnapshot!, btcPrices, currency); - + // Send final update with isComplete = true if (onProgress && finalWalletData) { const finalPartialData: PartialWalletData = { ...finalWalletData, discoveryProgress: { - addressesChecked: discoveredAddresses.length, - addressesWithActivity: discoveredAddresses.length, - currentBatch: Math.ceil(discoveredAddresses.length / 20), + addressesChecked: uniqueAddresses.length, + addressesWithActivity: uniqueAddresses.length, + currentBatch: Math.ceil(uniqueAddresses.length / GAP_LIMIT), isComplete: true, }, isComplete: true, diff --git a/tests/address-data-concurrency.test.ts b/tests/address-data-concurrency.test.ts new file mode 100644 index 00000000..138331b2 --- /dev/null +++ b/tests/address-data-concurrency.test.ts @@ -0,0 +1,134 @@ +/** + * Behavioral tests for the bounded per-address data fetcher used during wallet + * connect. These lock in the fix for the connect→dashboard performance + * regression where per-address fetches fired with unbounded concurrency + * (flooding the provider) and raced the snapshot build (dropping data for the + * last-discovered addresses). + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock the blockchain API layer so we can observe call patterns deterministically. +vi.mock('../src/lib/blockchain-api', () => { + return { + esploraGet: vi.fn(), + fetchJson: vi.fn(), + getHistoricalPriceRange: vi.fn(), + }; +}); + +import { fetchAddressDataConcurrent } from '../src/lib/blockchain'; +import { esploraGet } from '../src/lib/blockchain-api'; + +const mockedEsploraGet = esploraGet as unknown as ReturnType; + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +const makeAddresses = (n: number) => Array.from({ length: n }, (_, i) => `addr_${i}`); + +beforeEach(() => { + mockedEsploraGet.mockReset(); +}); + +describe('fetchAddressDataConcurrent', () => { + it('caps the number of addresses processed concurrently', async () => { + const addresses = makeAddresses(30); + let inFlightAddresses = 0; + let maxInFlightAddresses = 0; + // Track concurrency at the address level: each address issues its calls + // via Promise.all, so counting the /txs call (always first per address) + // approximates how many addresses are being worked on at once. + mockedEsploraGet.mockImplementation(async (path: string) => { + if (path.endsWith('/txs')) { + inFlightAddresses++; + maxInFlightAddresses = Math.max(maxInFlightAddresses, inFlightAddresses); + await sleep(10); + inFlightAddresses--; + return []; + } + if (path.endsWith('/utxo')) return []; + return { chain_stats: { tx_count: 1, funded_txo_sum: 0, spent_txo_sum: 0 }, mempool_stats: { tx_count: 0 } }; + }); + + const result = await fetchAddressDataConcurrent(addresses, undefined, 6); + + expect(result.size).toBe(30); + // Concurrency must be bounded by the worker pool (6), not the address count (30). + expect(maxInFlightAddresses).toBeLessThanOrEqual(6); + expect(maxInFlightAddresses).toBeGreaterThan(1); + }); + + it('awaits every address before returning (no dropped data even when the last is slow)', async () => { + const addresses = makeAddresses(8); + const slowAddress = addresses[addresses.length - 1]; + mockedEsploraGet.mockImplementation(async (path: string) => { + if (path.includes(slowAddress) && path.endsWith('/txs')) { + await sleep(50); // last-discovered address resolves slowly + return [{ txid: 'slow-tx', status: { confirmed: true } }]; + } + if (path.endsWith('/txs')) return [{ txid: `tx-${path}`, status: { confirmed: true } }]; + if (path.endsWith('/utxo')) return []; + return { chain_stats: { tx_count: 1, funded_txo_sum: 0, spent_txo_sum: 0 }, mempool_stats: { tx_count: 0 } }; + }); + + const result = await fetchAddressDataConcurrent(addresses, undefined, 6); + + expect(result.size).toBe(8); + expect(result.has(slowAddress)).toBe(true); + expect(result.get(slowAddress)?.txs[0].txid).toBe('slow-tx'); + }); + + it('reuses discovery stats and skips the redundant /address call', async () => { + const addresses = makeAddresses(5); + const statsByAddress = new Map(); + addresses.forEach((a, i) => + statsByAddress.set(a, { + chain_stats: { tx_count: i + 1, funded_txo_sum: 1000, spent_txo_sum: 0 }, + mempool_stats: { tx_count: 0 }, + }) + ); + + mockedEsploraGet.mockImplementation(async (path: string) => { + if (path.endsWith('/txs')) return []; + if (path.endsWith('/utxo')) return []; + return null; + }); + + const result = await fetchAddressDataConcurrent(addresses, statsByAddress, 6); + + // No bare /address/{addr} call should have been made (stats were supplied). + const bareAddressCalls = mockedEsploraGet.mock.calls.filter( + ([path]: [string]) => /^\/address\/[^/]+$/.test(path) + ); + expect(bareAddressCalls.length).toBe(0); + // The supplied stats are used as `info`. + expect(result.get('addr_0')?.info.chain_stats.tx_count).toBe(1); + }); + + it('retries the utxo call when balance is positive but the first utxo fetch is empty', async () => { + const addresses = ['addr_0']; + const statsByAddress = new Map([ + ['addr_0', { chain_stats: { tx_count: 1, funded_txo_sum: 5000, spent_txo_sum: 0 }, mempool_stats: { tx_count: 0 } }], + ]); + let utxoCalls = 0; + mockedEsploraGet.mockImplementation(async (path: string) => { + if (path.endsWith('/txs')) return []; + if (path.endsWith('/utxo')) { + utxoCalls++; + return utxoCalls === 1 ? [] : [{ txid: 't', vout: 0, value: 5000 }]; + } + return null; + }); + + const result = await fetchAddressDataConcurrent(addresses, statsByAddress, 6); + + expect(utxoCalls).toBe(2); // initial empty + balance-driven retry + expect(result.get('addr_0')?.utxos).toHaveLength(1); + }); + + it('returns an empty map for no addresses without calling the API', async () => { + const result = await fetchAddressDataConcurrent([], undefined, 6); + expect(result.size).toBe(0); + expect(mockedEsploraGet).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/address-discovery-unit.test.ts b/tests/address-discovery-unit.test.ts index 7f893837..d6b98581 100644 --- a/tests/address-discovery-unit.test.ts +++ b/tests/address-discovery-unit.test.ts @@ -143,3 +143,39 @@ describe('Address Discovery Optimization', () => { expect(content).toContain('Using cached snapshot'); }); }); + +describe('Progressive connect performance regression guards', () => { + const blockchainPath = path.join(__dirname, '../src/lib/blockchain.ts'); + const content = fs.readFileSync(blockchainPath, 'utf-8'); + + it('Both the progressive and snapshot paths use the shared bounded fetcher', () => { + // The bounded worker-pool helper exists and is reused, not duplicated. + expect(content).toContain('fetchAddressDataConcurrent'); + expect(content).toContain('ADDRESS_DATA_CONCURRENCY'); + const usages = content.match(/fetchAddressDataConcurrent\(/g) ?? []; + // One definition + at least two call sites (fetchWalletSnapshot + progressive). + expect(usages.length).toBeGreaterThanOrEqual(3); + }); + + it('getWalletDataProgressive does not fetch per-address data inside onAddressFound', () => { + // The discovery callback must only COLLECT addresses; fetching there is + // exactly the unbounded-flood regression we removed. + const start = content.indexOf('getWalletDataProgressive'); + const slice = content.slice(start); + const callbackStart = slice.indexOf('onAddressFound:'); + const callbackEnd = slice.indexOf('onBatchComplete:'); + expect(callbackStart).toBeGreaterThan(-1); + expect(callbackEnd).toBeGreaterThan(callbackStart); + const callbackBody = slice.slice(callbackStart, callbackEnd); + expect(callbackBody).not.toContain('esploraGet'); + }); + + it('Progressive discovery is wrapped in the discovery timeout for parity', () => { + expect(content).toContain('DISCOVERY_TIMEOUT_MS'); + // The progressive path should race discovery against the timeout. + const start = content.indexOf('getWalletDataProgressive'); + const slice = content.slice(start); + expect(slice).toContain('Promise.race'); + expect(slice).toContain('DISCOVERY_TIMEOUT_MS'); + }); +}); diff --git a/tests/validate-initial-check-limit.test.ts b/tests/validate-initial-check-limit.test.ts index 0de48fec..650c0cd1 100644 --- a/tests/validate-initial-check-limit.test.ts +++ b/tests/validate-initial-check-limit.test.ts @@ -99,10 +99,10 @@ describe('Performance Optimization Validation', () => { expect(match?.[1]).toBe('20'); }); - it('PARALLEL_BATCH_SIZE is reasonable (10)', () => { + it('PARALLEL_BATCH_SIZE matches the gap window so a full window resolves in one round-trip (20)', () => { const match = blockchainContent.match(/const\s+PARALLEL_BATCH_SIZE\s*=\s*(\d+)/); expect(match).toBeTruthy(); - expect(match?.[1]).toBe('10'); + expect(match?.[1]).toBe('20'); }); it('All optimization constants are defined', () => { From 5c0ba25de103e95088b4251573153483f2c1b542 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 08:43:18 +0000 Subject: [PATCH 2/3] Fix type error in address-data-concurrency test 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 --- tests/address-data-concurrency.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/address-data-concurrency.test.ts b/tests/address-data-concurrency.test.ts index 138331b2..26e3a009 100644 --- a/tests/address-data-concurrency.test.ts +++ b/tests/address-data-concurrency.test.ts @@ -98,7 +98,7 @@ describe('fetchAddressDataConcurrent', () => { // No bare /address/{addr} call should have been made (stats were supplied). const bareAddressCalls = mockedEsploraGet.mock.calls.filter( - ([path]: [string]) => /^\/address\/[^/]+$/.test(path) + (call: any[]) => /^\/address\/[^/]+$/.test(call[0]) ); expect(bareAddressCalls.length).toBe(0); // The supplied stats are used as `info`. From 53f0f4fa2e44c62a7edcb0ae4e7e72ab9d67e740 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 08:53:41 +0000 Subject: [PATCH 3/3] Address code review: fix premature-complete + harden progress emits 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 --- src/contexts/wallet-context.tsx | 8 ++++++++ src/lib/blockchain.ts | 13 ++++++++++--- tests/address-discovery-unit.test.ts | 13 +++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/contexts/wallet-context.tsx b/src/contexts/wallet-context.tsx index 3148147b..38df73ec 100644 --- a/src/contexts/wallet-context.tsx +++ b/src/contexts/wallet-context.tsx @@ -759,10 +759,17 @@ export const WalletProvider = ({ children, testXpub }: { children: ReactNode; te if (!hasCachedData) { console.log(`[WalletContext] Starting progressive discovery for wallet switch ${activeXpub.substring(0, 20)}...`); + let receivedComplete = false; const result = await getWalletDataProgressive(activeXpub, currency, (partialData: PartialWalletData) => { if (requestId !== activeRequestId.current) { return; } + // Once a completion update has been applied, ignore any late or + // out-of-order partial emit so it can't revert the finished dashboard + // back to a partial (lower-balance / fewer-tx) view. + if (receivedComplete) { + return; + } // Real-time UI updates as addresses are discovered! console.log(`[WalletContext] Progressive update - ${partialData.discoveryProgress.addressesWithActivity} addresses, ${partialData.transactions.length} txs, ${partialData.balanceBTC} BTC`); @@ -787,6 +794,7 @@ export const WalletProvider = ({ children, testXpub }: { children: ReactNode; te // If complete, mark as no longer discovering if (partialData.isComplete) { + receivedComplete = true; setIsDiscovering(false); setIsLoading(false); } diff --git a/src/lib/blockchain.ts b/src/lib/blockchain.ts index bf6bfd08..7d3b9cb4 100644 --- a/src/lib/blockchain.ts +++ b/src/lib/blockchain.ts @@ -847,7 +847,12 @@ export async function getWalletDataProgressive( // and shows a live "Found N addresses" counter while discovery runs. const emitDiscoveryProgress = (progress: DiscoveryProgress) => { if (!onProgress) return; - void buildPartialWalletData(xpub, [], btcPrices, currency, latestBlockHeight, progress) + // Discovery finishing is NOT the whole load finishing - the heavy + // per-address fetch still follows. Never forward isComplete:true here, + // or the dashboard would drop its loading state (and clobber any cached + // snapshot already shown) with an empty, zero-balance wallet. + const discoveryOnly: DiscoveryProgress = { ...progress, isComplete: false }; + void buildPartialWalletData(xpub, [], btcPrices, currency, latestBlockHeight, discoveryOnly) .then((partial) => onProgress(partial)) .catch(() => { /* progress emission is best-effort */ }); }; @@ -1014,10 +1019,12 @@ async function buildPartialWalletData( } // Build transactions (simplified for performance) + // Hoisted out of the map: this is rebuilt for every transaction otherwise, + // and buildPartialWalletData runs repeatedly as data streams in during connect. + const ourAddressesSet = new Set(usedAddresses); const transactions: Transaction[] = Array.from(allTxs.values()).map((tx: any): Transaction => { let netAmountSatoshis = 0; - const ourAddressesSet = new Set(usedAddresses); - + tx.vout.forEach((out: any) => { if (out.scriptpubkey_address && ourAddressesSet.has(out.scriptpubkey_address)) { netAmountSatoshis += out.value; diff --git a/tests/address-discovery-unit.test.ts b/tests/address-discovery-unit.test.ts index d6b98581..0a5fb8bc 100644 --- a/tests/address-discovery-unit.test.ts +++ b/tests/address-discovery-unit.test.ts @@ -178,4 +178,17 @@ describe('Progressive connect performance regression guards', () => { expect(slice).toContain('Promise.race'); expect(slice).toContain('DISCOVERY_TIMEOUT_MS'); }); + + it('Discovery-phase progress emits never report isComplete:true', () => { + // discoverUsedAddressesProgressive fires a final onBatchComplete with + // isComplete:true when DISCOVERY ends - but the heavy fetch still follows. + // emitDiscoveryProgress must override isComplete:false, otherwise the + // dashboard flips out of loading (and clobbers cached data) with an empty + // zero-balance wallet before any transaction data is fetched. + const start = content.indexOf('const emitDiscoveryProgress'); + const end = content.indexOf('};', start); + expect(start).toBeGreaterThan(-1); + const body = content.slice(start, end); + expect(body).toContain('isComplete: false'); + }); });