From c87265257159fcee97b3aadbbfcefac0d20a1969 Mon Sep 17 00:00:00 2001 From: local Date: Mon, 26 Oct 2026 10:23:46 +0100 Subject: [PATCH] perf(bitcoin): scan new accounts with a gap of 5 and deepen only if used The first full scan of a new account uses the discovery stop gap (5) instead of the standard gap (20). When that scan finds activity, extendDiscoveryScan checks only the addresses the standard gap adds (one Esplora history request each, through the network's shared fetch policy) and runs the gap-20 full scan only if one of them has history or the check fails, so coverage is unchanged. BitcoinAccount.hasSynchronized() tells a first scan (genesis checkpoint) from a rescan. In the A/B runs this cut the first-2-minute bitcoin requests from 257 to 137. --- .../src/entities/account.ts | 38 ++++ .../bitcoin-wallet-snap/src/entities/chain.ts | 12 ++ .../src/infra/BdkAccountAdapter.test.ts | 36 ++++ .../src/infra/BdkAccountAdapter.ts | 29 ++- .../src/infra/EsploraClientAdapter.test.ts | 192 +++++++++++++++++- .../src/infra/EsploraClientAdapter.ts | 111 ++++++++++ .../src/infra/StoredAccountAdapter.test.ts | 12 ++ .../src/infra/StoredAccountAdapter.ts | 20 ++ .../src/use-cases/AccountUseCases.test.ts | 63 +++++- .../src/use-cases/AccountUseCases.ts | 16 +- 10 files changed, 524 insertions(+), 5 deletions(-) diff --git a/packages/bitcoin-wallet-snap/src/entities/account.ts b/packages/bitcoin-wallet-snap/src/entities/account.ts index 067a3be3b..9ee4104e2 100644 --- a/packages/bitcoin-wallet-snap/src/entities/account.ts +++ b/packages/bitcoin-wallet-snap/src/entities/account.ts @@ -20,6 +20,9 @@ import type { Inscription } from './meta-protocols'; import type { AccountStateSnapshot } from './snap'; import type { TransactionBuilder } from './transaction'; +/** A BIP-32 keychain of an account: receive or change addresses. */ +export type Keychain = 'external' | 'internal'; + /** * A Bitcoin account. */ @@ -82,6 +85,32 @@ export type BitcoinAccount = { */ peekAddress(index: number): AddressInfo; + /** + * Get the address at a given index of a keychain, without revealing it. + * + * @param keychain - 'external' (receive) or 'internal' (change). + * @param index - derivation index. + * @returns the address + */ + peekKeychainAddress(keychain: Keychain, index: number): AddressInfo; + + /** + * Highest derivation index of the keychain that received an output the + * wallet knows of (the keychain's last active index for a gap scan). + * + * @param keychain - 'external' (receive) or 'internal' (change). + * @returns the index, or undefined if the keychain has no known output + */ + lastUsedIndex(keychain: Keychain): number | undefined; + + /** + * Highest revealed derivation index of the keychain. + * + * @param keychain - 'external' (receive) or 'internal' (change). + * @returns the index, or undefined if no address is revealed + */ + lastRevealedIndex(keychain: Keychain): number | undefined; + /** * Get the next unused address. This will reveal a new address if there is no unused address. * Note that the account needs to be persisted for this operation to be idempotent. @@ -126,6 +155,15 @@ export type BitcoinAccount = { */ applyUpdate(update: Update): void; + /** + * Whether the account's wallet has applied at least one chain update (sync + * or full scan). A wallet that never synced still sits on the genesis + * checkpoint (height 0), and its zero balance means "unknown", not "empty". + * + * @returns true if the wallet has synchronized with the chain at least once + */ + hasSynchronized(): boolean; + /** * Extract the change set if it exists. * diff --git a/packages/bitcoin-wallet-snap/src/entities/chain.ts b/packages/bitcoin-wallet-snap/src/entities/chain.ts index c6a9e63ca..aac520a6d 100644 --- a/packages/bitcoin-wallet-snap/src/entities/chain.ts +++ b/packages/bitcoin-wallet-snap/src/entities/chain.ts @@ -25,6 +25,18 @@ export type BlockchainClient = { */ fullScan(account: BitcoinAccount, mode?: 'discovery' | 'scan'): Promise; + /** + * Complete a discovery-gap full scan that found activity to the standard + * gap. Only the addresses the standard gap adds beyond what the discovery + * scan already fetched are checked; the standard-gap full scan runs only if + * one of them has history or the check fails, so a used account keeps its + * full address coverage without refetching the addresses already scanned. + * Note that this operation may modify the account in place. + * + * @param account - the account the discovery-gap scan was applied to. + */ + extendDiscoveryScan(account: BitcoinAccount): Promise; + /** * Perform a sync operation on the account. * Note that this operation modifies the account in place. diff --git a/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.test.ts b/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.test.ts index a582bb799..25d5aaa74 100644 --- a/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.test.ts +++ b/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.test.ts @@ -3,6 +3,7 @@ import type { ChangeSet, DescriptorPair, KeychainKind, + LocalOutput, Network, ScriptBuf, SpkIndexed, @@ -205,6 +206,41 @@ describe('BdkAccountAdapter', () => { mockNetwork, ); + describe('gap scan indexes (v5)', () => { + const output = (keychain: KeychainKind, index: number): LocalOutput => + ({ keychain, derivation_index: index }) as unknown as LocalOutput; + + it('returns the highest index of a keychain that received an output', () => { + mockWallet.list_output.mockReturnValue([ + output('external', 2), + output('internal', 7), + output('external', 0), + output('external', 4), + ]); + + expect(adapter().lastUsedIndex('external')).toBe(4); + expect(adapter().lastUsedIndex('internal')).toBe(7); + }); + + it('returns undefined for a keychain without outputs', () => { + mockWallet.list_output.mockReturnValue([output('external', 0)]); + + expect(adapter().lastUsedIndex('internal')).toBeUndefined(); + }); + + it('returns the last revealed index and peeks keychain addresses without revealing them', () => { + const info = mock(); + mockWallet.derivation_index.mockReturnValue(3); + mockWallet.peek_address.mockReturnValue(info); + + expect(adapter().lastRevealedIndex('internal')).toBe(3); + expect(mockWallet.derivation_index).toHaveBeenCalledWith('internal'); + expect(adapter().peekKeychainAddress('internal', 9)).toBe(info); + expect(mockWallet.peek_address).toHaveBeenCalledWith('internal', 9); + expect(mockWallet.reveal_addresses_to).not.toHaveBeenCalled(); + }); + }); + describe('isChange', () => { it('returns true when the script belongs to the internal keychain', () => { mockWallet.derivation_of_spk.mockReturnValue(indexed('internal', 3)); diff --git a/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.ts b/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.ts index 2171e1b94..59c434851 100644 --- a/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.ts +++ b/packages/bitcoin-wallet-snap/src/infra/BdkAccountAdapter.ts @@ -26,7 +26,7 @@ import { } from '@metamask/bitcoindevkit'; import { AccountCapability, ValidationError, WalletError } from '../entities'; -import type { BitcoinAccount, TransactionBuilder } from '../entities'; +import type { BitcoinAccount, Keychain, TransactionBuilder } from '../entities'; import { toFeeRateSatsPerVb } from '../utils/fee-rate'; import { BdkTxBuilderAdapter } from './BdkTxBuilderAdapter'; @@ -149,6 +149,27 @@ export class BdkAccountAdapter implements BitcoinAccount { return this.#wallet.peek_address('external', index); } + peekKeychainAddress(keychain: Keychain, index: number): AddressInfo { + return this.#wallet.peek_address(keychain, index); + } + + lastUsedIndex(keychain: Keychain): number | undefined { + let last: number | undefined; + for (const output of this.#wallet.list_output()) { + if ( + output.keychain === keychain && + (last === undefined || output.derivation_index > last) + ) { + last = output.derivation_index; + } + } + return last; + } + + lastRevealedIndex(keychain: Keychain): number | undefined { + return this.#wallet.derivation_index(keychain); + } + nextUnusedAddress(): AddressInfo { return this.#wallet.next_unused_address('external'); } @@ -183,6 +204,12 @@ export class BdkAccountAdapter implements BitcoinAccount { return this.#wallet.apply_update(update); } + hasSynchronized(): boolean { + // A freshly created wallet only holds the genesis checkpoint (height 0); + // every successful sync or full scan applies a chain update to the tip. + return this.#wallet.latest_checkpoint.height > 0; + } + takeStaged(): ChangeSet | undefined { const staged = this.#collectStaged(); this.#staged = undefined; diff --git a/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts index d0f94f650..3c9b387a3 100644 --- a/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts +++ b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts @@ -1,4 +1,5 @@ import type { + AddressInfo, FeeEstimates, FullScanRequest, Psbt as BdkPsbt, @@ -16,8 +17,8 @@ import { readFile } from 'fs/promises'; import { mock } from 'jest-mock-extended'; import { dirname, join } from 'path'; -import type { BitcoinAccount, ChainConfig } from '../entities'; -import { EsploraClientAdapter } from './EsploraClientAdapter'; +import type { BitcoinAccount, ChainConfig, Keychain } from '../entities'; +import { EsploraClientAdapter, toScripthash } from './EsploraClientAdapter'; jest.mock('@metamask/bitcoindevkit', () => ({ EsploraClient: jest.fn(), @@ -455,6 +456,193 @@ describe('EsploraClientAdapter', () => { }); }); + describe('extendDiscoveryScan (v5: complete a discovery-gap scan without refetching it)', () => { + const mockFetch = jest.fn(); + + beforeEach(() => { + global.fetch = mockFetch; + }); + + afterEach(() => { + mockFetch.mockReset(); + }); + + const okResponse = ( + body: unknown, + ): { ok: boolean; text: () => Promise } => ({ + ok: true, + text: async (): Promise => JSON.stringify(body), + }); + + // Script bytes encode the keychain and index, so each address has a + // distinct script hash that the tests can map back. + const scriptBytes = (keychain: Keychain, index: number): Uint8Array => + new Uint8Array([keychain === 'external' ? 0 : 1, index]); + + const setupAccount = ( + account: BitcoinAccount, + indexes: Record, + ): void => { + jest + .mocked(account.lastUsedIndex) + .mockImplementation((keychain) => indexes[keychain].used); + jest + .mocked(account.lastRevealedIndex) + .mockImplementation((keychain) => indexes[keychain].revealed); + jest.mocked(account.peekKeychainAddress).mockImplementation( + (keychain, index) => + ({ + address: { + // eslint-disable-next-line @typescript-eslint/naming-convention + script_pubkey: { as_bytes: () => scriptBytes(keychain, index) }, + }, + }) as unknown as AddressInfo, + ); + }; + + const checkedIndexes = (account: BitcoinAccount): [Keychain, number][] => + jest + .mocked(account.peekKeychainAddress) + .mock.calls.map(([keychain, index]) => [keychain, index]); + + const range = ( + keychain: Keychain, + first: number, + last: number, + ): [Keychain, number][] => + Array.from({ length: last - first + 1 }, (_unused, offset) => [ + keychain, + first + offset, + ]); + + it('computes the Esplora script hash (reversed SHA-256)', async () => { + expect(await toScripthash(new Uint8Array())).toBe( + '55b852781b9995a44c939b64e441ae2724b96f99c8f4fb9a141cfc9842c4b0e3', + ); + }); + + it('checks only the addresses the standard gap adds and skips the full scan when they are unused (run B-v4 account 0)', async () => { + const { adapter, mockEsploraClient, account } = setupTest(); + // Account 0 of run B-v4: activity at index 0 of both keychains (the + // receive keychain has one more revealed address). The discovery scan + // fetched up to index 5 of each keychain (last active + 5). + setupAccount(account, { + external: { used: 0, revealed: 1 }, + internal: { used: 0, revealed: 0 }, + }); + mockFetch.mockResolvedValue(okResponse([])); + + await adapter.extendDiscoveryScan(account); + + expect(checkedIndexes(account)).toStrictEqual([ + ...range('external', 6, 21), + ...range('internal', 6, 20), + ]); + expect(mockFetch).toHaveBeenCalledTimes(31); + // The standard-gap full scan refetched both keychains from index 0 + // (50 requests in run B-v4). + expect(mockEsploraClient.full_scan).not.toHaveBeenCalled(); + const hash = await toScripthash(scriptBytes('external', 6)); + expect(mockFetch).toHaveBeenCalledWith( + `https://bitcoin.example/scripthash/${hash}/txs`, + { signal: expect.any(AbortSignal) }, + ); + expect(account.applyUpdate).not.toHaveBeenCalled(); + }); + + it('covers indexes discovery..scan-1 of a keychain without activity', async () => { + const { adapter, account } = setupTest(); + setupAccount(account, { + external: { used: 3 }, + internal: {}, + }); + mockFetch.mockResolvedValue(okResponse([])); + + await adapter.extendDiscoveryScan(account); + + expect(checkedIndexes(account)).toStrictEqual([ + ...range('external', 9, 23), + ...range('internal', 5, 19), + ]); + }); + + it('runs the standard-gap full scan as soon as a checked address has history', async () => { + const { adapter, mockEsploraClient, account, mockRequest } = setupTest(); + setupAccount(account, { + external: { used: 0, revealed: 0 }, + internal: { used: 0, revealed: 0 }, + }); + const usedHash = await toScripthash(scriptBytes('external', 8)); + mockFetch.mockImplementation(async (url: string) => + okResponse(url.includes(usedHash) ? [{ txid: 'tx' }] : []), + ); + + await adapter.extendDiscoveryScan(account); + + // Batches of `parallelRequests`: 6..10 holds the used index 8, so the + // following batches are not requested. + expect(mockFetch).toHaveBeenCalledTimes(5); + expect(mockEsploraClient.full_scan).toHaveBeenCalledTimes(1); + expect(mockEsploraClient.full_scan).toHaveBeenCalledWith( + mockRequest, + 20, + 5, + ); + }); + + it('falls back to the standard-gap full scan when the check fails', async () => { + const { adapter, mockEsploraClient, account } = setupTest(); + setupAccount(account, { + external: { used: 0 }, + internal: {}, + }); + mockFetch.mockResolvedValue({ ok: false, status: 404 }); + + await adapter.extendDiscoveryScan(account); + + expect(mockEsploraClient.full_scan).toHaveBeenCalledTimes(1); + }); + + it('falls back to the standard-gap full scan when the addresses cannot be derived', async () => { + const { adapter, mockEsploraClient, account } = setupTest(); + jest.mocked(account.lastUsedIndex).mockImplementation(() => { + throw new Error('unsupported'); + }); + + await adapter.extendDiscoveryScan(account); + + expect(mockFetch).not.toHaveBeenCalled(); + expect(mockEsploraClient.full_scan).toHaveBeenCalledTimes(1); + }); + + it('treats an unexpected response body as history', async () => { + const { adapter, mockEsploraClient, account } = setupTest(); + setupAccount(account, { + external: { used: 0 }, + internal: {}, + }); + mockFetch.mockResolvedValue(okResponse({ error: 'unexpected' })); + + await adapter.extendDiscoveryScan(account); + + expect(mockEsploraClient.full_scan).toHaveBeenCalledTimes(1); + }); + + it('propagates a failure of the fallback full scan', async () => { + const { adapter, mockEsploraClient, account } = setupTest(); + setupAccount(account, { + external: { used: 0 }, + internal: {}, + }); + mockFetch.mockResolvedValue(okResponse([{ txid: 'tx' }])); + mockEsploraClient.full_scan.mockRejectedValue(new Error('boom')); + + await expect(adapter.extendDiscoveryScan(account)).rejects.toThrow( + 'Failed to perform initial full scan', + ); + }); + }); + describe('with the real BDK WASM client', () => { const SAMPLE_PSBT = 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgu3FEiFNy9ZR/zSpTo9nHREjrSoAAAAAAAAAAAA='; diff --git a/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts index 045b77bdd..7aa43493d 100644 --- a/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts +++ b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts @@ -1,6 +1,7 @@ import type { FeeEstimates, Network, + ScriptBuf, Transaction, } from '@metamask/bitcoindevkit'; import { EsploraClient } from '@metamask/bitcoindevkit'; @@ -22,6 +23,7 @@ import type { BitcoinAccount, ChainConfig, BlockchainClient, + Keychain, } from '../entities'; /** @@ -55,6 +57,24 @@ function toEsploraRestUrl(url: string): string { return url.replace(/\/+$/u, '').replace(/\/v1$/u, ''); } +const KEYCHAINS: Keychain[] = ['external', 'internal']; + +/** + * Computes the Electrum/Esplora script hash of an output script: the SHA-256 + * of the script, byte-reversed, hex-encoded (the key of `/scripthash/:hash`). + * + * @param script - The output script bytes. + * @returns The script hash. + */ +export async function toScripthash(script: Uint8Array): Promise { + const digest = new Uint8Array( + await globalThis.crypto.subtle.digest('SHA-256', script), + ); + return Array.from(digest.reverse(), (byte) => + byte.toString(16).padStart(2, '0'), + ).join(''); +} + const ALREADY_BROADCAST_PATTERN = /already in block\s*chain|already in mempool|txn-already-(?:in-mempool|known)/iu; @@ -119,6 +139,8 @@ export class EsploraClientAdapter implements BlockchainClient { readonly #policies: Record; + readonly #logger: Logger; + // Funding addresses are resolved while mapping transactions, which runs on // every sync/event emission. Cache by network + txid so a receive is only // looked up once instead of on each notification. @@ -150,6 +172,7 @@ export class EsploraClientAdapter implements BlockchainClient { }; this.#config = config; + this.#logger = logger; } async fullScan( @@ -181,6 +204,94 @@ export class EsploraClientAdapter implements BlockchainClient { } } + async extendDiscoveryScan(account: BitcoinAccount): Promise { + let hasHistory: boolean; + try { + hasHistory = await this.#hasHistoryInGapExtension(account); + } catch (error) { + // Never leave the account with the discovery gap only: fall back to the + // standard-gap full scan (which surfaces its own failure). + this.#logger.warn( + `[esplora ${account.network}] standard-gap check failed, running the standard-gap scan`, + error, + ); + hasHistory = true; + } + if (hasHistory) { + await this.fullScan(account, 'scan'); + } + } + + /** + * Lists, per keychain, the scripts a standard-gap full scan covers beyond + * the discovery-gap scan just applied to the account. A gap scan covers up + * to `last active index + stop gap` (indexes `0..stopGap-1` when nothing is + * active), so the discovery scan already fetched up to + * `lastUsed + discovery`. The upper bound also accounts for revealed + * addresses, so the check never covers less than the standard-gap scan. + * + * @param account - The account. + * @returns The scripts to check. + */ + #gapExtensionScripts(account: BitcoinAccount): ScriptBuf[] { + const { discovery, scan } = this.#config.stopGap; + const scripts: ScriptBuf[] = []; + for (const keychain of KEYCHAINS) { + const lastUsed = account.lastUsedIndex(keychain) ?? -1; + const lastRevealed = account.lastRevealedIndex(keychain) ?? -1; + const first = lastUsed + discovery + 1; + const last = Math.max(lastUsed, lastRevealed) + scan; + for (let index = first; index <= last; index++) { + scripts.push( + account.peekKeychainAddress(keychain, index).address.script_pubkey, + ); + } + } + return scripts; + } + + async #hasHistoryInGapExtension(account: BitcoinAccount): Promise { + const scripts = this.#gapExtensionScripts(account); + const batchSize = Math.max(1, this.#config.parallelRequests); + for (let start = 0; start < scripts.length; start += batchSize) { + const results = await Promise.all( + scripts + .slice(start, start + batchSize) + .map(async (script) => + this.#scriptHasHistory(account.network, script), + ), + ); + if (results.some(Boolean)) { + return true; + } + } + return false; + } + + /** + * Whether an address has any transaction history, from the Esplora REST + * API. The request goes through the network's shared fetch policy (retries, + * circuit breaker, and per-attempt timeout). + * + * @param network - Network of the address. + * @param script - Output script of the address. + * @returns `true` unless the indexer reports an empty history. + */ + async #scriptHasHistory( + network: Network, + script: ScriptBuf, + ): Promise { + const scripthash = await toScripthash(script.as_bytes()); + const body = await fetchWithPolicy( + this.#policies[network], + `${this.#restUrls[network]}/scripthash/${scripthash}/txs`, + ); + const history = JSON.parse(body) as unknown; + // Anything but an empty list counts as history: the caller then runs the + // standard-gap full scan. + return !Array.isArray(history) || history.length > 0; + } + async sync(account: BitcoinAccount): Promise { try { const update = await executeWithFetchPolicy( diff --git a/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.test.ts b/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.test.ts index bf6343c2b..9b7701c65 100644 --- a/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.test.ts +++ b/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.test.ts @@ -69,4 +69,16 @@ describe('StoredAccountAdapter', () => { expect(() => adapter().isMine(mockScript)).toThrow(WalletError); }); }); + + describe('gap scan indexes', () => { + it('throws a WalletError since stored metadata cannot derive addresses', () => { + expect(() => adapter().peekKeychainAddress('internal', 0)).toThrow( + WalletError, + ); + expect(() => adapter().lastUsedIndex('external')).toThrow(WalletError); + expect(() => adapter().lastRevealedIndex('external')).toThrow( + WalletError, + ); + }); + }); }); diff --git a/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.ts b/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.ts index edb5164c6..d2982b27d 100644 --- a/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.ts +++ b/packages/bitcoin-wallet-snap/src/infra/StoredAccountAdapter.ts @@ -21,6 +21,7 @@ import type { AccountMetadata, AccountState, BitcoinAccount, + Keychain, TransactionBuilder, } from '../entities'; @@ -103,6 +104,18 @@ export class StoredAccountAdapter implements BitcoinAccount { return this.#unsupported(); } + peekKeychainAddress(_keychain: Keychain, _index: number): AddressInfo { + return this.#unsupported(); + } + + lastUsedIndex(_keychain: Keychain): number | undefined { + return this.#unsupported(); + } + + lastRevealedIndex(_keychain: Keychain): number | undefined { + return this.#unsupported(); + } + nextUnusedAddress(): AddressInfo { return this.#unsupported(); } @@ -123,6 +136,13 @@ export class StoredAccountAdapter implements BitcoinAccount { this.#unsupported(); } + hasSynchronized(): boolean { + // Metadata-only view: the wallet (and its checkpoint) is not loaded. + // Callers that need the sync state load the full account; never report + // this view as unsynchronized, which would hide a known balance. + return true; + } + takeStaged(): ChangeSet | undefined { return this.#unsupported(); } diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts index 24911957a..7b16aa584 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts @@ -823,6 +823,8 @@ describe('AccountUseCases', () => { receivedAmount, ]); mockAccount.listTransactions.mockReturnValue([]); + // A rescan of an account that already synchronized once. + mockAccount.hasSynchronized.mockReturnValue(true); }); it('performs a full scan', async () => { @@ -832,7 +834,8 @@ describe('AccountUseCases', () => { const result = await useCases.fullScan(mockAccount); - expect(mockChain.fullScan).toHaveBeenCalledWith(mockAccount); + expect(mockChain.fullScan).toHaveBeenCalledTimes(1); + expect(mockChain.fullScan).toHaveBeenCalledWith(mockAccount, 'scan'); expect(mockMetaProtocols.fetchInscriptions).toHaveBeenCalledWith( mockAccount, ); @@ -847,6 +850,64 @@ describe('AccountUseCases', () => { }); }); + it('scans an account that never synchronized with the discovery gap only, when it finds no activity', async () => { + mockAccount.hasSynchronized.mockReturnValue(false); + mockChain.fullScan.mockResolvedValue(); + + await useCases.fullScan(mockAccount); + + expect(mockChain.fullScan).toHaveBeenCalledTimes(1); + expect(mockChain.fullScan).toHaveBeenCalledWith(mockAccount, 'discovery'); + expect(mockRepository.update).toHaveBeenCalledTimes(1); + }); + + it('completes the initial scan to the standard gap when the discovery gap finds activity', async () => { + mockAccount.hasSynchronized.mockReturnValue(false); + mockChain.fullScan.mockImplementationOnce(async () => { + mockAccount.listTransactions.mockReturnValue(mockTransactions); + }); + mockChain.extendDiscoveryScan.mockResolvedValueOnce(); + mockChain.getTransactionSenders.mockResolvedValue([]); + + const result = await useCases.fullScan(mockAccount); + + // The discovery-gap scan is not repeated with the standard gap: only + // the addresses the standard gap adds are checked. + expect(mockChain.fullScan.mock.calls).toStrictEqual([ + [mockAccount, 'discovery'], + ]); + expect(mockChain.extendDiscoveryScan).toHaveBeenCalledTimes(1); + expect(mockChain.extendDiscoveryScan).toHaveBeenCalledWith(mockAccount); + expect(result.transactionsToNotify).toStrictEqual(mockTransactions); + }); + + it('does not extend an initial scan that found no activity, nor a rescan', async () => { + mockAccount.hasSynchronized.mockReturnValue(false); + await useCases.fullScan(mockAccount); + mockAccount.hasSynchronized.mockReturnValue(true); + mockAccount.listTransactions.mockReturnValue(mockTransactions); + mockChain.getTransactionSenders.mockResolvedValue([]); + await useCases.fullScan(mockAccount); + + expect(mockChain.extendDiscoveryScan).not.toHaveBeenCalled(); + }); + + it('fails the initial scan (to be retried) if the standard-gap completion fails', async () => { + mockAccount.hasSynchronized.mockReturnValue(false); + const error = new Error('Failed to fetch'); + mockChain.fullScan.mockImplementationOnce(async () => { + mockAccount.listTransactions.mockReturnValue(mockTransactions); + }); + mockChain.extendDiscoveryScan.mockRejectedValueOnce(error); + mockRepository.update.mockClear(); + + await expect(useCases.fullScan(mockAccount)).rejects.toBe(error); + + expect(mockChain.fullScan).toHaveBeenCalledTimes(1); + expect(mockChain.extendDiscoveryScan).toHaveBeenCalledTimes(1); + expect(mockRepository.update).not.toHaveBeenCalled(); + }); + it('propagates an error if the chain full scan fails', async () => { const error = new Error('Full scan failed'); mockChain.fullScan.mockRejectedValue(error); diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts index b9771a4ab..83c38e700 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts @@ -571,7 +571,21 @@ export class AccountUseCases { async fullScan(account: BitcoinAccount): Promise { this.#logger.debug('Performing initial full scan: %s', account.id); - await this.#chain.fullScan(account); + // The first scan of an account uses the discovery gap: most new accounts + // are unused, and the standard gap costs four times the requests per + // empty account (onboarding scanned five accounts this way). Only an + // account where it finds activity is completed to the standard gap right + // after (checking just the addresses the standard gap adds), so used + // accounts keep their full address coverage. + const initialScan = !account.hasSynchronized(); + await this.#chain.fullScan(account, initialScan ? 'discovery' : 'scan'); + if (initialScan && account.listTransactions().length > 0) { + this.#logger.debug( + 'Activity found by the initial scan, completing it to the standard gap: %s', + account.id, + ); + await this.#chain.extendDiscoveryScan(account); + } const inscriptions = this.#metaProtocols ? await this.#metaProtocols.fetchInscriptions(account)