From 394f41fa9106ed7a342027c7afe3b88b6b570ac8 Mon Sep 17 00:00:00 2001 From: jeremytsng Date: Mon, 31 Aug 2026 14:55:28 +0800 Subject: [PATCH] fix(bitcoin-wallet-snap): use BIP44 gap limit for full account scans Split the chain stopGap config into { discovery: 5, scan: 20 }. Account discovery keeps the cheap 5-address probe; a full scan of an account the user owns walks until 20 consecutive empty addresses, the BIP44 gap limit, so it cannot stop before funds parked deeper in the address chain. --- packages/bitcoin-wallet-snap/CHANGELOG.md | 4 ++ packages/bitcoin-wallet-snap/src/config.ts | 2 +- .../bitcoin-wallet-snap/src/entities/chain.ts | 4 +- .../src/entities/config.ts | 2 +- .../src/infra/EsploraClientAdapter.test.ts | 68 +++++++++++++++++++ .../src/infra/EsploraClientAdapter.ts | 11 ++- .../src/use-cases/AccountUseCases.test.ts | 10 ++- .../src/use-cases/AccountUseCases.ts | 2 +- 8 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts diff --git a/packages/bitcoin-wallet-snap/CHANGELOG.md b/packages/bitcoin-wallet-snap/CHANGELOG.md index 5908ee56b..053d97aa4 100644 --- a/packages/bitcoin-wallet-snap/CHANGELOG.md +++ b/packages/bitcoin-wallet-snap/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Split the chain `stopGap` configuration into `{ discovery: 5, scan: 20 }` so account discovery keeps the cheap probe while full account scans use the BIP44 gap limit ([#224](https://github.com/MetaMask/internal-snaps/pull/224)) + ### Fixed - Ensure certain errors are stringified correctly ([#179](https://github.com/MetaMask/internal-snaps/pull/179)) diff --git a/packages/bitcoin-wallet-snap/src/config.ts b/packages/bitcoin-wallet-snap/src/config.ts index 21d35aa56..614013b24 100644 --- a/packages/bitcoin-wallet-snap/src/config.ts +++ b/packages/bitcoin-wallet-snap/src/config.ts @@ -31,7 +31,7 @@ export const Config: SnapConfig = { encrypt: false, chain: { parallelRequests: 5, - stopGap: 5, + stopGap: { discovery: 5, scan: 20 }, maxRetries: 3, url: { bitcoin: fromEnv('ESPLORA_BITCOIN', 'https://blockstream.info/api'), diff --git a/packages/bitcoin-wallet-snap/src/entities/chain.ts b/packages/bitcoin-wallet-snap/src/entities/chain.ts index 112f916eb..77c29eb93 100644 --- a/packages/bitcoin-wallet-snap/src/entities/chain.ts +++ b/packages/bitcoin-wallet-snap/src/entities/chain.ts @@ -20,8 +20,10 @@ export type BlockchainClient = { * Note that this operation modifies the account in place. * * @param account - the account to full scan. + * @param mode - 'discovery' uses the short discovery stop gap for probing + * candidate accounts; the default 'scan' uses the full BIP44-sized gap. */ - fullScan(account: BitcoinAccount): Promise; + fullScan(account: BitcoinAccount, mode?: 'discovery' | 'scan'): Promise; /** * Perform a sync operation on the account. diff --git a/packages/bitcoin-wallet-snap/src/entities/config.ts b/packages/bitcoin-wallet-snap/src/entities/config.ts index 4675c6e20..ad2c50b68 100644 --- a/packages/bitcoin-wallet-snap/src/entities/config.ts +++ b/packages/bitcoin-wallet-snap/src/entities/config.ts @@ -15,7 +15,7 @@ export type SnapConfig = { export type ChainConfig = { parallelRequests: number; - stopGap: number; + stopGap: { discovery: number; scan: number }; maxRetries: number; url: { [network in Network]: string; diff --git a/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts new file mode 100644 index 000000000..8c73769ea --- /dev/null +++ b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.test.ts @@ -0,0 +1,68 @@ +import type { FullScanRequest } from '@metamask/bitcoindevkit'; +import { EsploraClient } from '@metamask/bitcoindevkit'; +import { mock } from 'jest-mock-extended'; + +import type { BitcoinAccount, ChainConfig } from '../entities'; +import { EsploraClientAdapter } from './EsploraClientAdapter'; + +jest.mock('@metamask/bitcoindevkit', () => ({ + EsploraClient: jest.fn(), +})); + +const setupTest = (): { + adapter: EsploraClientAdapter; + mockEsploraClient: ReturnType>; + account: BitcoinAccount; + mockRequest: FullScanRequest; +} => { + const mockEsploraClient = mock(); + jest.mocked(EsploraClient).mockReturnValue(mockEsploraClient); + + const config = mock({ + parallelRequests: 5, + maxRetries: 3, + stopGap: { discovery: 5, scan: 20 }, + url: { + bitcoin: 'https://bitcoin.example', + testnet: 'https://testnet.example', + testnet4: 'https://testnet4.example', + signet: 'https://signet.example', + regtest: 'https://regtest.example', + }, + }); + + const adapter = new EsploraClientAdapter(config); + const mockRequest = mock(); + const account = mock({ network: 'bitcoin' }); + account.startFullScan.mockReturnValue(mockRequest); + + return { adapter, mockEsploraClient, account, mockRequest }; +}; + +describe('EsploraClientAdapter', () => { + describe('fullScan', () => { + it('uses the scan stop gap by default', async () => { + const { adapter, mockEsploraClient, account, mockRequest } = setupTest(); + + await adapter.fullScan(account); + + expect(mockEsploraClient.full_scan).toHaveBeenCalledWith( + mockRequest, + 20, + 5, + ); + }); + + it("uses the discovery stop gap in 'discovery' mode", async () => { + const { adapter, mockEsploraClient, account, mockRequest } = setupTest(); + + await adapter.fullScan(account, 'discovery'); + + expect(mockEsploraClient.full_scan).toHaveBeenCalledWith( + mockRequest, + 5, + 5, + ); + }); + }); +}); diff --git a/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts index 65734c189..8710ecdc7 100644 --- a/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts +++ b/packages/bitcoin-wallet-snap/src/infra/EsploraClientAdapter.ts @@ -30,12 +30,19 @@ export class EsploraClientAdapter implements BlockchainClient { this.#config = config; } - async fullScan(account: BitcoinAccount): Promise { + async fullScan( + account: BitcoinAccount, + mode: 'discovery' | 'scan' = 'scan', + ): Promise { try { + const stopGap = + mode === 'discovery' + ? this.#config.stopGap.discovery + : this.#config.stopGap.scan; const request = account.startFullScan(); const update = await this.#clients[account.network].full_scan( request, - this.#config.stopGap, + stopGap, this.#config.parallelRequests, ); account.applyUpdate(update); 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 694d55691..d4e4972c5 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts @@ -475,7 +475,10 @@ describe('AccountUseCases', () => { discoverParams.network, tAddressType, ); - expect(mockChain.fullScan).toHaveBeenCalledWith(mockAccount); + expect(mockChain.fullScan).toHaveBeenCalledWith( + mockAccount, + 'discovery', + ); }, ); @@ -508,7 +511,10 @@ describe('AccountUseCases', () => { tNetwork, discoverParams.addressType, ); - expect(mockChain.fullScan).toHaveBeenCalledWith(mockAccount); + expect(mockChain.fullScan).toHaveBeenCalledWith( + mockAccount, + 'discovery', + ); }, ); diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts index 85c8b34d9..57a9e00b1 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts @@ -230,7 +230,7 @@ export class AccountUseCases { // We need to do a full scan here to know if the account // has any previous activity since later on we filter out // accounts with no tx history - await this.#chain.fullScan(newAccount); + await this.#chain.fullScan(newAccount, 'discovery'); this.#logger.info( 'Bitcoin account discovered successfully. Request: %o',