From 814bd26d509527564c1bd36b607dbc5c10032ecd Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Mon, 27 Apr 2026 22:27:45 -0400 Subject: [PATCH 01/26] feat: implement keyring_createAccounts for batch creation (MUL-1782) - Route KeyringRpcMethod.CreateAccounts and validate with CreateAccountsRequestStruct - Support Bip44DeriveIndex and Bip44DeriveIndexRange on mainnet P2WPKH via AccountUseCases.create - Bump @metamask/keyring-api to ^22.0.0 and @metamask/keyring-snap-sdk to ^8.0.0 - Add unit tests and changelog; release snap 1.11.0 Made-with: Cursor --- packages/snap/CHANGELOG.md | 9 ++ packages/snap/locales/en.json | 2 +- packages/snap/package.json | 6 +- packages/snap/snap.manifest.json | 4 +- .../snap/src/handlers/KeyringHandler.test.ts | 129 +++++++++++++++++- packages/snap/src/handlers/KeyringHandler.ts | 72 ++++++++++ yarn.lock | 90 +++++++++--- 7 files changed, 288 insertions(+), 24 deletions(-) diff --git a/packages/snap/CHANGELOG.md b/packages/snap/CHANGELOG.md index c255e4028..64d9740e9 100644 --- a/packages/snap/CHANGELOG.md +++ b/packages/snap/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Implement `keyring_createAccounts` method for batch account creation supporting `bip44:derive-index` and `bip44:derive-index-range` capabilities ([MUL-1782](https://consensyssoftware.atlassian.net/browse/MUL-1782)) + +### Changed + +- Bump `@metamask/keyring-api` from `^21.3.0` to `^22.0.0` +- Bump `@metamask/keyring-snap-sdk` from `^7.1.1` to `^8.0.0` + ## [1.10.1] ### Fixed diff --git a/packages/snap/locales/en.json b/packages/snap/locales/en.json index a4e7e54a4..e0443012c 100644 --- a/packages/snap/locales/en.json +++ b/packages/snap/locales/en.json @@ -215,4 +215,4 @@ "message": "Request from" } } -} +} \ No newline at end of file diff --git a/packages/snap/package.json b/packages/snap/package.json index 34c1132ac..35713e3e2 100644 --- a/packages/snap/package.json +++ b/packages/snap/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/bitcoin-wallet-snap", - "version": "1.10.1", + "version": "1.11.0", "description": "A Bitcoin wallet Snap.", "repository": { "type": "git", @@ -43,8 +43,8 @@ "@metamask/auto-changelog": "^3.4.4", "@metamask/bitcoindevkit": "^0.1.13", "@metamask/key-tree": "^10.1.1", - "@metamask/keyring-api": "^21.3.0", - "@metamask/keyring-snap-sdk": "^7.1.1", + "@metamask/keyring-api": "^22.0.0", + "@metamask/keyring-snap-sdk": "^8.0.0", "@metamask/slip44": "^4.2.0", "@metamask/snaps-cli": "^8.3.0", "@metamask/snaps-jest": "^9.8.0", diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index e0ad4da16..3624eea41 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -1,5 +1,5 @@ { - "version": "1.10.0", + "version": "1.10.1", "description": "Manage Bitcoin using MetaMask", "proposedName": "Bitcoin", "repository": { @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "M7LJk6fzMpw9tjV4ytZEJpcNaMfY6t4RUQacQCqfGz8=", + "shasum": "mAOxf/YQOAte5hVPSFWcfs089UL9SXOVMXHR6jwnD8U=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index b2f0047f5..70233efaf 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -15,7 +15,11 @@ import type { Transaction as KeyringTransaction, KeyringRequest, } from '@metamask/keyring-api'; -import { BtcAccountType, BtcScope } from '@metamask/keyring-api'; +import { + AccountCreationType, + BtcAccountType, + BtcScope, +} from '@metamask/keyring-api'; import { mock } from 'jest-mock-extended'; import { assert } from 'superstruct'; @@ -389,6 +393,129 @@ describe('KeyringHandler', () => { }); }); + describe('createAccounts', () => { + const entropySource = 'some-source'; + + const buildMockAccount = (index: number): BitcoinAccount => + mock({ + id: `id-${index}`, + addressType: 'p2wpkh', + balance: { trusted_spendable: { to_btc: () => 1 } }, + network: 'bitcoin', + derivationPath: ['myEntropy', "84'", "0'", `${index}'`], + entropySource: 'myEntropy', + accountIndex: index, + publicAddress: mockAddress, + capabilities: [ + AccountCapability.SignPsbt, + AccountCapability.ComputeFee, + ], + }); + + it('creates a single account for Bip44DeriveIndex', async () => { + const account = buildMockAccount(2); + mockAccounts.create.mockResolvedValue(account); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndex, + groupIndex: 2, + entropySource, + }); + + expect(mockAccounts.create).toHaveBeenCalledTimes(1); + expect(mockAccounts.create).toHaveBeenCalledWith({ + network: 'bitcoin', + entropySource, + index: 2, + addressType: 'p2wpkh', + synchronize: false, + }); + expect(result).toHaveLength(1); + }); + + it('creates an inclusive range of accounts for Bip44DeriveIndexRange', async () => { + mockAccounts.create.mockImplementation(async ({ index }) => + buildMockAccount(index), + ); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 0, to: 2 }, + entropySource, + }); + + expect(mockAccounts.create).toHaveBeenCalledTimes(3); + [0, 1, 2].forEach((index, callIdx) => { + expect(mockAccounts.create).toHaveBeenNthCalledWith(callIdx + 1, { + network: 'bitcoin', + entropySource, + index, + addressType: 'p2wpkh', + synchronize: false, + }); + }); + expect(result).toHaveLength(3); + }); + + it('rejects unsupported creation types', async () => { + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44DerivePath, + derivationPath: "m/84'/0'/0'", + entropySource, + }), + ).rejects.toThrow(/not supported|unsupported/iu); + expect(mockAccounts.create).not.toHaveBeenCalled(); + }); + + it('propagates errors from create', async () => { + const error = new Error('create error'); + mockAccounts.create.mockRejectedValue(error); + + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndex, + groupIndex: 0, + entropySource, + }), + ).rejects.toThrow(error); + }); + + describe('tracing', () => { + const options = { + type: AccountCreationType.Bip44DeriveIndex as const, + groupIndex: 0, + entropySource, + }; + + beforeEach(() => { + mockSnapClient.startTrace.mockResolvedValue(undefined); + mockSnapClient.endTrace.mockResolvedValue(undefined); + mockAccounts.create.mockResolvedValue(buildMockAccount(0)); + }); + + it('calls startTrace and endTrace with correct trace name', async () => { + await handler.createAccounts(options); + + expect(mockSnapClient.startTrace).toHaveBeenCalledWith( + 'Create Bitcoin Accounts Batch', + ); + expect(mockSnapClient.endTrace).toHaveBeenCalledWith( + 'Create Bitcoin Accounts Batch', + ); + }); + + it('calls endTrace even if create fails', async () => { + mockAccounts.create.mockRejectedValue(new Error('boom')); + + await expect(handler.createAccounts(options)).rejects.toThrow('boom'); + expect(mockSnapClient.endTrace).toHaveBeenCalledWith( + 'Create Bitcoin Accounts Batch', + ); + }); + }); + }); + describe('discoverAccounts', () => { const entropySource = 'some-source'; const groupIndex = 0; diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 14e2b2497..af0c7a083 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -1,8 +1,11 @@ import type { AddressType } from '@metamask/bitcoindevkit'; import { + AccountCreationType, + assertCreateAccountOptionIsSupported, BtcAccountType, BtcScope, CreateAccountRequestStruct, + CreateAccountsRequestStruct, DeleteAccountRequestStruct, DiscoverAccountsRequestStruct, FilterAccountChainsStruct, @@ -17,6 +20,7 @@ import { SubmitRequestRequestStruct, } from '@metamask/keyring-api'; import type { + CreateAccountOptions, Keyring, KeyringAccount, KeyringResponse, @@ -116,6 +120,10 @@ export class KeyringHandler implements Keyring { assert(request, CreateAccountRequestStruct); return this.createAccount(request.params.options); } + case `${KeyringRpcMethod.CreateAccounts}`: { + assert(request, CreateAccountsRequestStruct); + return this.createAccounts(request.params.options); + } case `${KeyringRpcMethod.DiscoverAccounts}`: { assert(request, DiscoverAccountsRequestStruct); return this.discoverAccounts( @@ -277,6 +285,70 @@ export class KeyringHandler implements Keyring { } } + async createAccounts( + options: CreateAccountOptions, + ): Promise { + assertCreateAccountOptionIsSupported(options, [ + `${AccountCreationType.Bip44DeriveIndex}`, + `${AccountCreationType.Bip44DeriveIndexRange}`, + ]); + + const traceName = 'Create Bitcoin Accounts Batch'; + let traceStarted = false; + + try { + await runSnapActionSafely( + async () => { + await this.#snapClient.startTrace(traceName); + traceStarted = true; + }, + this.#logger, + 'startTrace', + ); + + const { entropySource } = options; + + // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring + // the defaults used by `createAccount` when no scope is provided. + const network = scopeToNetwork[BtcScope.Mainnet]; + const addressType = this.#defaultAddressType; + if (addressType !== 'p2wpkh') { + throw new FormatError( + 'Only native segwit (P2WPKH) addresses are supported', + ); + } + + const range = + options.type === AccountCreationType.Bip44DeriveIndex + ? { from: options.groupIndex, to: options.groupIndex } + : options.range; + + const accounts: KeyringAccount[] = []; + for (let index = range.from; index <= range.to; index += 1) { + // `AccountUseCases.create` is idempotent: if an account already exists + // for the resolved derivation path, it will be returned as-is. + const account = await this.#accountsUseCases.create({ + network, + entropySource, + index, + addressType, + synchronize: false, + }); + accounts.push(mapToKeyringAccount(account)); + } + + return accounts; + } finally { + if (traceStarted) { + await runSnapActionSafely( + async () => this.#snapClient.endTrace(traceName), + this.#logger, + 'endTrace', + ); + } + } + } + async discoverAccounts( scopes: BtcScope[], entropySource: string, diff --git a/yarn.lock b/yarn.lock index 4b094eb72..f24e40da6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3018,6 +3018,16 @@ __metadata: languageName: node linkType: hard +"@metamask/abi-utils@npm:^3.0.0": + version: 3.0.0 + resolution: "@metamask/abi-utils@npm:3.0.0" + dependencies: + "@metamask/superstruct": "npm:^3.1.0" + "@metamask/utils": "npm:^11.0.1" + checksum: 10/068b98185148b9e185b4af4392c6a6f82f1d4b1ff60013c57679c618f37afe9030e3ccc940e1a8b690be6f62ea91115ab18b73f3c3c09f4eff1794e31ababb9b + languageName: node + linkType: hard + "@metamask/approval-controller@npm:^8.0.0": version: 8.0.0 resolution: "@metamask/approval-controller@npm:8.0.0" @@ -3076,8 +3086,8 @@ __metadata: "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/bitcoindevkit": "npm:^0.1.13" "@metamask/key-tree": "npm:^10.1.1" - "@metamask/keyring-api": "npm:^21.3.0" - "@metamask/keyring-snap-sdk": "npm:^7.1.1" + "@metamask/keyring-api": "npm:^22.0.0" + "@metamask/keyring-snap-sdk": "npm:^8.0.0" "@metamask/slip44": "npm:^4.2.0" "@metamask/snaps-cli": "npm:^8.3.0" "@metamask/snaps-jest": "npm:^9.8.0" @@ -3235,6 +3245,21 @@ __metadata: languageName: node linkType: hard +"@metamask/eth-sig-util@npm:^8.2.0": + version: 8.2.0 + resolution: "@metamask/eth-sig-util@npm:8.2.0" + dependencies: + "@ethereumjs/rlp": "npm:^4.0.1" + "@ethereumjs/util": "npm:^8.1.0" + "@metamask/abi-utils": "npm:^3.0.0" + "@metamask/utils": "npm:^11.0.1" + "@scure/base": "npm:~1.1.3" + ethereum-cryptography: "npm:^2.1.2" + tweetnacl: "npm:^1.0.3" + checksum: 10/385df1ec541116e1bd725a1df1a519996bad167f99d1b2677126e398cdfda6fc3f03d2ff8f1ca523966bc0aae3ea92a9050953a45d5a7711f4128aacf9242bfc + languageName: node + linkType: hard + "@metamask/ethjs-unit@npm:^0.3.0": version: 0.3.0 resolution: "@metamask/ethjs-unit@npm:0.3.0" @@ -3287,42 +3312,59 @@ __metadata: linkType: hard "@metamask/keyring-api@npm:^21.3.0": - version: 21.3.0 - resolution: "@metamask/keyring-api@npm:21.3.0" + version: 21.6.0 + resolution: "@metamask/keyring-api@npm:21.6.0" dependencies: - "@metamask/keyring-utils": "npm:^3.1.0" + "@ethereumjs/tx": "npm:^5.4.0" + "@metamask/eth-sig-util": "npm:^8.2.0" + "@metamask/keyring-utils": "npm:^3.2.0" "@metamask/superstruct": "npm:^3.1.0" "@metamask/utils": "npm:^11.1.0" + "@types/uuid": "npm:^9.0.8" + async-mutex: "npm:^0.5.0" bitcoin-address-validation: "npm:^2.2.3" - checksum: 10/5d1f2705d76d8cfc70378f740e054fca163735b7b77b86b092cfd311d1befb27fac16f81b210ef2c719ce32010e1db55916b7b78d1f7a84239e47b771e5d84cc + uuid: "npm:^9.0.1" + checksum: 10/ecd482ec83fbdb16da5f0c548db29931edd4718c57550547aed9f3532c8e60ec39a6894571c96a819e5f205e53ec149e6b52710194ac0610960aa51834f15dd8 languageName: node linkType: hard -"@metamask/keyring-snap-sdk@npm:^7.1.1": - version: 7.1.1 - resolution: "@metamask/keyring-snap-sdk@npm:7.1.1" +"@metamask/keyring-api@npm:^22.0.0": + version: 22.0.0 + resolution: "@metamask/keyring-api@npm:22.0.0" + dependencies: + "@metamask/keyring-utils": "npm:^3.2.0" + "@metamask/superstruct": "npm:^3.1.0" + "@metamask/utils": "npm:^11.1.0" + bitcoin-address-validation: "npm:^2.2.3" + checksum: 10/ff1e9537c7219fb906b61d6755de28890239ec44f634732bd8571801c662e8b801671e98961a1c5047e079cef314353297499a52f7091a2b5391325d575ec4f9 + languageName: node + linkType: hard + +"@metamask/keyring-snap-sdk@npm:^8.0.0": + version: 8.0.0 + resolution: "@metamask/keyring-snap-sdk@npm:8.0.0" dependencies: - "@metamask/keyring-utils": "npm:^3.1.0" - "@metamask/snaps-sdk": "npm:^9.0.0" + "@metamask/keyring-utils": "npm:^3.2.0" + "@metamask/snaps-sdk": "npm:^11.0.0" "@metamask/superstruct": "npm:^3.1.0" "@metamask/utils": "npm:^11.1.0" webextension-polyfill: "npm:^0.12.0" peerDependencies: - "@metamask/keyring-api": ^21.2.0 + "@metamask/keyring-api": ^22.0.0 "@metamask/providers": ^19.0.0 - checksum: 10/ac4ce050f4647096ef66ebd04d99d1423c002ca0fb05bd83e11caec59754b56d73bb8a95ac3a76f64472713256205e889d6785003dfe2c35f5f1d67c2f2efd12 + checksum: 10/271b4f84d214813c0312ffbbe3fb6201b66172210da4d06a155c3ddb359403ed1fa4c7afa424cd34fa9db04e3c876f30f69afb676bb7a905d392552d871ebd57 languageName: node linkType: hard -"@metamask/keyring-utils@npm:^3.1.0": - version: 3.1.0 - resolution: "@metamask/keyring-utils@npm:3.1.0" +"@metamask/keyring-utils@npm:^3.2.0": + version: 3.2.0 + resolution: "@metamask/keyring-utils@npm:3.2.0" dependencies: "@ethereumjs/tx": "npm:^5.4.0" "@metamask/superstruct": "npm:^3.1.0" "@metamask/utils": "npm:^11.1.0" bitcoin-address-validation: "npm:^2.2.3" - checksum: 10/d7325bb72e47bd3d81b1bce55203d8343408c0d37dd2862203c21bb68c6a1e32a1cfa7ca46a4f6fe1f14e757084bbc45db8db3eedbefc90ce81805ce22d335e8 + checksum: 10/e71aa1b9ec9a24c72ea6d4864a10f11e68e5b77789728067230ec40cee2e85ad69073404d2fa62c760f014fd910fb68b3305a08f906f2534b9119e2a26d06a2b languageName: node linkType: hard @@ -5762,6 +5804,13 @@ __metadata: languageName: node linkType: hard +"@types/uuid@npm:^9.0.8": + version: 9.0.8 + resolution: "@types/uuid@npm:9.0.8" + checksum: 10/b8c60b7ba8250356b5088302583d1704a4e1a13558d143c549c408bf8920535602ffc12394ede77f8a8083511b023704bc66d1345792714002bfa261b17c5275 + languageName: node + linkType: hard + "@types/yargs-parser@npm:*": version: 21.0.3 resolution: "@types/yargs-parser@npm:21.0.3" @@ -20381,6 +20430,13 @@ __metadata: languageName: node linkType: hard +"tweetnacl@npm:^1.0.3": + version: 1.0.3 + resolution: "tweetnacl@npm:1.0.3" + checksum: 10/ca122c2f86631f3c0f6d28efb44af2a301d4a557a62a3e2460286b08e97567b258c2212e4ad1cfa22bd6a57edcdc54ba76ebe946847450ab0999e6d48ccae332 + languageName: node + linkType: hard + "type-check@npm:^0.4.0, type-check@npm:~0.4.0": version: 0.4.0 resolution: "type-check@npm:0.4.0" From 40ffbc155edd404690c0de3cf66f2beb0d0908c1 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 28 Apr 2026 11:53:39 -0400 Subject: [PATCH 02/26] chore: update manifest --- packages/snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 3624eea41..e199404d7 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -1,5 +1,5 @@ { - "version": "1.10.1", + "version": "1.11.0", "description": "Manage Bitcoin using MetaMask", "proposedName": "Bitcoin", "repository": { From 0df7c66ebab657bcf66bee16848ecdf6ee3da897 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 28 Apr 2026 12:13:27 -0400 Subject: [PATCH 03/26] chore: update manifest --- packages/snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index e199404d7..90bdd1e3e 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "mAOxf/YQOAte5hVPSFWcfs089UL9SXOVMXHR6jwnD8U=", + "shasum": "NMd9t7NM97ji1KpzOF2Wamcc7/cIpYbExM91CuDYJG8=", "location": { "npm": { "filePath": "dist/bundle.js", From e3948cfeed9983f045664f0f65583ef977843fdb Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:11:18 -0400 Subject: [PATCH 04/26] chore: update shasum --- packages/snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 90bdd1e3e..6b8179630 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "NMd9t7NM97ji1KpzOF2Wamcc7/cIpYbExM91CuDYJG8=", + "shasum": "QWz1Y9iMOLbOyIQl8eG+EaIUqI863vf/LG4U1y69dyc=", "location": { "npm": { "filePath": "dist/bundle.js", From 94cfdd6373de3e4bff080975a5409ba508b6a6d1 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:13:03 -0400 Subject: [PATCH 05/26] chore: update CHANGELOG --- packages/snap/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/snap/CHANGELOG.md b/packages/snap/CHANGELOG.md index 64d9740e9..7d15fef47 100644 --- a/packages/snap/CHANGELOG.md +++ b/packages/snap/CHANGELOG.md @@ -9,12 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Implement `keyring_createAccounts` method for batch account creation supporting `bip44:derive-index` and `bip44:derive-index-range` capabilities ([MUL-1782](https://consensyssoftware.atlassian.net/browse/MUL-1782)) +- Implement `keyring_createAccounts` method for batch account creation supporting `bip44:derive-index` and `bip44:derive-index-range` capabilities ([#601](https://github.com/MetaMask/snap-bitcoin-wallet/pull/601)) ### Changed -- Bump `@metamask/keyring-api` from `^21.3.0` to `^22.0.0` -- Bump `@metamask/keyring-snap-sdk` from `^7.1.1` to `^8.0.0` +- Bump `@metamask/keyring-api` from `^21.3.0` to `^22.0.0` ([#601](https://github.com/MetaMask/snap-bitcoin-wallet/pull/601)) +- Bump `@metamask/keyring-snap-sdk` from `^7.1.1` to `^8.0.0` ([#601](https://github.com/MetaMask/snap-bitcoin-wallet/pull/601)) ## [1.10.1] From b2337decc56a917e41d4eb1107ede712d69a5e09 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:17:15 -0400 Subject: [PATCH 06/26] chore: update CHANGELOG --- packages/snap/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/CHANGELOG.md b/packages/snap/CHANGELOG.md index 7d15fef47..79f00cf52 100644 --- a/packages/snap/CHANGELOG.md +++ b/packages/snap/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Implement `keyring_createAccounts` method for batch account creation supporting `bip44:derive-index` and `bip44:derive-index-range` capabilities ([#601](https://github.com/MetaMask/snap-bitcoin-wallet/pull/601)) +- Add support for `keyring_createAccounts` method to enable batch account creation ([#601](https://github.com/MetaMask/snap-bitcoin-wallet/pull/601)) ### Changed From 6dac9724c249d1f71a5ae774ea89e5f2a45d6d50 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 28 Apr 2026 16:26:35 -0400 Subject: [PATCH 07/26] fix: lint --- packages/snap/locales/en.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/locales/en.json b/packages/snap/locales/en.json index e0443012c..a4e7e54a4 100644 --- a/packages/snap/locales/en.json +++ b/packages/snap/locales/en.json @@ -215,4 +215,4 @@ "message": "Request from" } } -} \ No newline at end of file +} From ba86c3ff06d6c217c56d55647ecaface6df2a9d2 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 28 Apr 2026 17:30:40 -0400 Subject: [PATCH 08/26] chore: update manifest --- packages/snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 6b8179630..07fae0bff 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "QWz1Y9iMOLbOyIQl8eG+EaIUqI863vf/LG4U1y69dyc=", + "shasum": "+Ms9MA2WKL/qj4K1b6sJqpBXoeU0rKWae6fc3wLejbg=", "location": { "npm": { "filePath": "dist/bundle.js", From 75cfc13e0c551abb8552ddebeffbdb7cd3d64b40 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:39:37 -0400 Subject: [PATCH 09/26] chore: update manifest --- packages/snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 07fae0bff..c0871ea1d 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "+Ms9MA2WKL/qj4K1b6sJqpBXoeU0rKWae6fc3wLejbg=", + "shasum": "Vja4On7BmX76wESqY5xyQCpq0yRY2K+WSh9QPbM3bqk=", "location": { "npm": { "filePath": "dist/bundle.js", From 6a72002d98abcb798b54e995fa5be7b37227b389 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 29 Apr 2026 12:04:12 -0400 Subject: [PATCH 10/26] fix: revert version change --- packages/snap/package.json | 2 +- packages/snap/snap.manifest.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/snap/package.json b/packages/snap/package.json index 35713e3e2..1d8f26ab6 100644 --- a/packages/snap/package.json +++ b/packages/snap/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/bitcoin-wallet-snap", - "version": "1.11.0", + "version": "1.10.0", "description": "A Bitcoin wallet Snap.", "repository": { "type": "git", diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index c0871ea1d..706ad823b 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -1,5 +1,5 @@ { - "version": "1.11.0", + "version": "1.10.0", "description": "Manage Bitcoin using MetaMask", "proposedName": "Bitcoin", "repository": { @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "Vja4On7BmX76wESqY5xyQCpq0yRY2K+WSh9QPbM3bqk=", + "shasum": "OybOeuTi9jpL6hYD5f+z3KwzuEtqAM+yJQIPHFRW3UI=", "location": { "npm": { "filePath": "dist/bundle.js", From 401d8228f784aad510cc11e8cd6d2765aa7b250b Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 29 Apr 2026 12:06:05 -0400 Subject: [PATCH 11/26] fix: revert manifest change --- packages/snap/package.json | 2 +- packages/snap/snap.manifest.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snap/package.json b/packages/snap/package.json index 1d8f26ab6..8a0e8bbbe 100644 --- a/packages/snap/package.json +++ b/packages/snap/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/bitcoin-wallet-snap", - "version": "1.10.0", + "version": "1.10.1", "description": "A Bitcoin wallet Snap.", "repository": { "type": "git", diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 706ad823b..e0ad4da16 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "OybOeuTi9jpL6hYD5f+z3KwzuEtqAM+yJQIPHFRW3UI=", + "shasum": "M7LJk6fzMpw9tjV4ytZEJpcNaMfY6t4RUQacQCqfGz8=", "location": { "npm": { "filePath": "dist/bundle.js", From fa702f325a29f67df64971d07e3c7c150923e11c Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 29 Apr 2026 13:03:32 -0400 Subject: [PATCH 12/26] chore: align keyring-api --- packages/site/package.json | 2 +- yarn.lock | 58 +------------------------------------- 2 files changed, 2 insertions(+), 58 deletions(-) diff --git a/packages/site/package.json b/packages/site/package.json index 2ddbd71af..89781150b 100644 --- a/packages/site/package.json +++ b/packages/site/package.json @@ -29,7 +29,7 @@ ] }, "dependencies": { - "@metamask/keyring-api": "^21.3.0", + "@metamask/keyring-api": "^22.0.0", "@metamask/providers": "^22.1.0", "react": "^18.2.0", "react-dom": "^18.2.0", diff --git a/yarn.lock b/yarn.lock index f24e40da6..e2304b2e6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3018,16 +3018,6 @@ __metadata: languageName: node linkType: hard -"@metamask/abi-utils@npm:^3.0.0": - version: 3.0.0 - resolution: "@metamask/abi-utils@npm:3.0.0" - dependencies: - "@metamask/superstruct": "npm:^3.1.0" - "@metamask/utils": "npm:^11.0.1" - checksum: 10/068b98185148b9e185b4af4392c6a6f82f1d4b1ff60013c57679c618f37afe9030e3ccc940e1a8b690be6f62ea91115ab18b73f3c3c09f4eff1794e31ababb9b - languageName: node - linkType: hard - "@metamask/approval-controller@npm:^8.0.0": version: 8.0.0 resolution: "@metamask/approval-controller@npm:8.0.0" @@ -3110,7 +3100,7 @@ __metadata: version: 0.0.0-use.local resolution: "@metamask/bitcoin-wallet-test-dapp@workspace:packages/site" dependencies: - "@metamask/keyring-api": "npm:^21.3.0" + "@metamask/keyring-api": "npm:^22.0.0" "@metamask/providers": "npm:^22.1.0" "@svgr/webpack": "npm:^8.1.0" "@testing-library/dom": "npm:^8.17.1" @@ -3245,21 +3235,6 @@ __metadata: languageName: node linkType: hard -"@metamask/eth-sig-util@npm:^8.2.0": - version: 8.2.0 - resolution: "@metamask/eth-sig-util@npm:8.2.0" - dependencies: - "@ethereumjs/rlp": "npm:^4.0.1" - "@ethereumjs/util": "npm:^8.1.0" - "@metamask/abi-utils": "npm:^3.0.0" - "@metamask/utils": "npm:^11.0.1" - "@scure/base": "npm:~1.1.3" - ethereum-cryptography: "npm:^2.1.2" - tweetnacl: "npm:^1.0.3" - checksum: 10/385df1ec541116e1bd725a1df1a519996bad167f99d1b2677126e398cdfda6fc3f03d2ff8f1ca523966bc0aae3ea92a9050953a45d5a7711f4128aacf9242bfc - languageName: node - linkType: hard - "@metamask/ethjs-unit@npm:^0.3.0": version: 0.3.0 resolution: "@metamask/ethjs-unit@npm:0.3.0" @@ -3311,23 +3286,6 @@ __metadata: languageName: node linkType: hard -"@metamask/keyring-api@npm:^21.3.0": - version: 21.6.0 - resolution: "@metamask/keyring-api@npm:21.6.0" - dependencies: - "@ethereumjs/tx": "npm:^5.4.0" - "@metamask/eth-sig-util": "npm:^8.2.0" - "@metamask/keyring-utils": "npm:^3.2.0" - "@metamask/superstruct": "npm:^3.1.0" - "@metamask/utils": "npm:^11.1.0" - "@types/uuid": "npm:^9.0.8" - async-mutex: "npm:^0.5.0" - bitcoin-address-validation: "npm:^2.2.3" - uuid: "npm:^9.0.1" - checksum: 10/ecd482ec83fbdb16da5f0c548db29931edd4718c57550547aed9f3532c8e60ec39a6894571c96a819e5f205e53ec149e6b52710194ac0610960aa51834f15dd8 - languageName: node - linkType: hard - "@metamask/keyring-api@npm:^22.0.0": version: 22.0.0 resolution: "@metamask/keyring-api@npm:22.0.0" @@ -5804,13 +5762,6 @@ __metadata: languageName: node linkType: hard -"@types/uuid@npm:^9.0.8": - version: 9.0.8 - resolution: "@types/uuid@npm:9.0.8" - checksum: 10/b8c60b7ba8250356b5088302583d1704a4e1a13558d143c549c408bf8920535602ffc12394ede77f8a8083511b023704bc66d1345792714002bfa261b17c5275 - languageName: node - linkType: hard - "@types/yargs-parser@npm:*": version: 21.0.3 resolution: "@types/yargs-parser@npm:21.0.3" @@ -20430,13 +20381,6 @@ __metadata: languageName: node linkType: hard -"tweetnacl@npm:^1.0.3": - version: 1.0.3 - resolution: "tweetnacl@npm:1.0.3" - checksum: 10/ca122c2f86631f3c0f6d28efb44af2a301d4a557a62a3e2460286b08e97567b258c2212e4ad1cfa22bd6a57edcdc54ba76ebe946847450ab0999e6d48ccae332 - languageName: node - linkType: hard - "type-check@npm:^0.4.0, type-check@npm:~0.4.0": version: 0.4.0 resolution: "type-check@npm:0.4.0" From deaea5af1eaa256fea646bb3518d8a7bd9f884a3 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 30 Apr 2026 14:29:21 -0400 Subject: [PATCH 13/26] feat: speed optimization --- .../snap/src/handlers/KeyringHandler.test.ts | 31 ++++- packages/snap/src/handlers/KeyringHandler.ts | 125 +++++++++++++----- 2 files changed, 121 insertions(+), 35 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 70233efaf..96f77785f 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -445,11 +445,14 @@ describe('KeyringHandler', () => { }); expect(mockAccounts.create).toHaveBeenCalledTimes(3); - [0, 1, 2].forEach((index, callIdx) => { - expect(mockAccounts.create).toHaveBeenNthCalledWith(callIdx + 1, { + const calledIndices = mockAccounts.create.mock.calls + .map((call) => (call[0] as { index: number }).index) + .sort((a, b) => a - b); + expect(calledIndices).toEqual([0, 1, 2]); + mockAccounts.create.mock.calls.forEach((call) => { + expect(call[0]).toMatchObject({ network: 'bitcoin', entropySource, - index, addressType: 'p2wpkh', synchronize: false, }); @@ -457,6 +460,28 @@ describe('KeyringHandler', () => { expect(result).toHaveLength(3); }); + it('rejects an invalid index range when from is greater than to', async () => { + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 2, to: 0 }, + entropySource, + }), + ).rejects.toThrow(/invalid.*range|from must be/iu); + expect(mockAccounts.create).not.toHaveBeenCalled(); + }); + + it('rejects batches larger than the per-RPC limit', async () => { + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 0, to: 100 }, + entropySource, + }), + ).rejects.toThrow(/more than 100 accounts/iu); + expect(mockAccounts.create).not.toHaveBeenCalled(); + }); + it('rejects unsupported creation types', async () => { await expect( handler.createAccounts({ diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index af0c7a083..7da5673ad 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -81,6 +81,48 @@ export const CreateAccountRequest = object({ ...MetaMaskOptionsStruct.schema, }); +/** Upper bound on inclusive index range width for a single createAccounts RPC. */ +const MAX_CREATE_ACCOUNTS_PER_BATCH = 100; + +/** Max concurrent AccountUseCases.create calls within one batch. */ +const CREATE_ACCOUNTS_CONCURRENCY = 4; + +/** + * Map items to results with at most `concurrency` in-flight async operations. + * Output order matches `items` order. + * + * @param items - Values to map in pool order. + * @param concurrency - Maximum number of concurrent mapper executions. + * @param mapper - Async function applied to each item. + * @returns Results in the same order as `items`. + */ +async function runWithConcurrencyLimit( + items: readonly Item[], + concurrency: number, + mapper: (item: Item, index: number) => Promise, +): Promise { + if (items.length === 0) { + return []; + } + const results: Result[] = new Array(items.length); + let next = 0; + + const worker = async (): Promise => { + while (true) { + const idx = next; + next += 1; + if (idx >= items.length) { + return; + } + results[idx] = await mapper(items[idx] as Item, idx); + } + }; + + const poolSize = Math.min(Math.max(1, concurrency), items.length); + await Promise.all(Array.from({ length: poolSize }, async () => worker())); + return results; +} + export class KeyringHandler implements Keyring { readonly #accountsUseCases: AccountUseCases; @@ -293,6 +335,41 @@ export class KeyringHandler implements Keyring { `${AccountCreationType.Bip44DeriveIndexRange}`, ]); + const { entropySource } = options; + + const range = + options.type === AccountCreationType.Bip44DeriveIndex + ? { from: options.groupIndex, to: options.groupIndex } + : options.range; + + if (range.from > range.to) { + throw new FormatError( + 'Account index range is invalid: from must be less than or equal to to', + ); + } + + const batchSize = range.to - range.from + 1; + if (batchSize > MAX_CREATE_ACCOUNTS_PER_BATCH) { + throw new FormatError( + `Cannot create more than ${MAX_CREATE_ACCOUNTS_PER_BATCH} accounts in one batch`, + ); + } + + // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring + // the defaults used by `createAccount` when no scope is provided. + const network = scopeToNetwork[BtcScope.Mainnet]; + const addressType = this.#defaultAddressType; + if (addressType !== 'p2wpkh') { + throw new FormatError( + 'Only native segwit (P2WPKH) addresses are supported', + ); + } + + const indices: number[] = []; + for (let index = range.from; index <= range.to; index += 1) { + indices.push(index); + } + const traceName = 'Create Bitcoin Accounts Batch'; let traceStarted = false; @@ -306,38 +383,22 @@ export class KeyringHandler implements Keyring { 'startTrace', ); - const { entropySource } = options; - - // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring - // the defaults used by `createAccount` when no scope is provided. - const network = scopeToNetwork[BtcScope.Mainnet]; - const addressType = this.#defaultAddressType; - if (addressType !== 'p2wpkh') { - throw new FormatError( - 'Only native segwit (P2WPKH) addresses are supported', - ); - } - - const range = - options.type === AccountCreationType.Bip44DeriveIndex - ? { from: options.groupIndex, to: options.groupIndex } - : options.range; - - const accounts: KeyringAccount[] = []; - for (let index = range.from; index <= range.to; index += 1) { - // `AccountUseCases.create` is idempotent: if an account already exists - // for the resolved derivation path, it will be returned as-is. - const account = await this.#accountsUseCases.create({ - network, - entropySource, - index, - addressType, - synchronize: false, - }); - accounts.push(mapToKeyringAccount(account)); - } - - return accounts; + // `AccountUseCases.create` is idempotent: if an account already exists + // for the resolved derivation path, it will be returned as-is. + return await runWithConcurrencyLimit( + indices, + CREATE_ACCOUNTS_CONCURRENCY, + async (index) => { + const account = await this.#accountsUseCases.create({ + network, + entropySource, + index, + addressType, + synchronize: false, + }); + return mapToKeyringAccount(account); + }, + ); } finally { if (traceStarted) { await runSnapActionSafely( From 112cb8e726647b3836c9c0ba36bd2aa744ced82b Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:07:29 -0400 Subject: [PATCH 14/26] feat: speed optimization --- packages/snap/src/handlers/KeyringHandler.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 96f77785f..4b6ad0fa6 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -96,6 +96,7 @@ describe('KeyringHandler', () => { const correlationId = 'correlation-id'; // non-P2WPKH address types as we are not supporting them for v1 + // eslint-disable-next-line jest/no-disabled-tests it.skip('respects provided params', async () => { const options = { scope: BtcScope.Signet, @@ -126,6 +127,7 @@ describe('KeyringHandler', () => { }); // only P2WPKH (BIP-84) derivation paths are now supported for v1 + // eslint-disable-next-line jest/no-disabled-tests it.skip('extracts index from derivationPath', async () => { const options = { scope: BtcScope.Signet, @@ -225,6 +227,7 @@ describe('KeyringHandler', () => { ); // skip non-P2WPKH address types as they are not supported on v1 + // eslint-disable-next-line jest/no-disabled-tests it.skip.each([ { purpose: Purpose.Legacy, addressType: 'p2pkh' }, { purpose: Purpose.Segwit, addressType: 'p2sh' }, @@ -448,7 +451,7 @@ describe('KeyringHandler', () => { const calledIndices = mockAccounts.create.mock.calls .map((call) => (call[0] as { index: number }).index) .sort((a, b) => a - b); - expect(calledIndices).toEqual([0, 1, 2]); + expect(calledIndices).toStrictEqual([0, 1, 2]); mockAccounts.create.mock.calls.forEach((call) => { expect(call[0]).toMatchObject({ network: 'bitcoin', From 38cc8b52785e7d4715b89838df1a451048945c57 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 30 Apr 2026 17:12:06 -0400 Subject: [PATCH 15/26] chore: add logs --- packages/snap/src/handlers/KeyringHandler.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 7da5673ad..8dc54a365 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -330,6 +330,11 @@ export class KeyringHandler implements Keyring { async createAccounts( options: CreateAccountOptions, ): Promise { + console.log( + '[PERFORMANCE LOGS][BITCOIN SNAP] execute createAccounts method', + options, + ); + assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, From 15eea66c1b6b4986a3c9e5ef125fdbc323da3a62 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 6 May 2026 15:48:58 -0400 Subject: [PATCH 16/26] refactor: batch account creation --- packages/snap/src/entities/account.ts | 17 + packages/snap/src/entities/snap.ts | 2 +- .../snap/src/handlers/KeyringHandler.test.ts | 183 ++++++++-- packages/snap/src/handlers/KeyringHandler.ts | 79 ++--- .../src/store/BdkAccountRepository.test.ts | 167 +++++++++ .../snap/src/store/BdkAccountRepository.ts | 152 ++++++++- .../src/use-cases/AccountUseCases.test.ts | 155 +++++++++ .../snap/src/use-cases/AccountUseCases.ts | 323 ++++++++++++++---- 8 files changed, 913 insertions(+), 165 deletions(-) diff --git a/packages/snap/src/entities/account.ts b/packages/snap/src/entities/account.ts index f7f30af65..dc17ef789 100644 --- a/packages/snap/src/entities/account.ts +++ b/packages/snap/src/entities/account.ts @@ -262,6 +262,16 @@ export type BitcoinAccountRepository = { */ getByDerivationPath(derivationPath: string[]): Promise; + /** + * Get accounts by derivation path. + * + * @param derivationPaths - derivation paths. + * @returns the accounts or null if they do not exist, in input order + */ + getByDerivationPaths( + derivationPaths: string[][], + ): Promise<(BitcoinAccount | null)[]>; + /** * Create a new account, without persisting it. * @@ -283,6 +293,13 @@ export type BitcoinAccountRepository = { */ insert(account: BitcoinAccount): Promise; + /** + * Insert accounts. + * + * @param accounts - Bitcoin accounts. + */ + insertMany(accounts: BitcoinAccount[]): Promise; + /** * Update an account. * diff --git a/packages/snap/src/entities/snap.ts b/packages/snap/src/entities/snap.ts index 9d8e9f42d..8d150ce32 100644 --- a/packages/snap/src/entities/snap.ts +++ b/packages/snap/src/entities/snap.ts @@ -13,7 +13,7 @@ import type { Inscription } from './meta-protocols'; export type SnapState = { // accountId -> account state. This is the main state of the snap. - accounts: Record; + accounts: Record; // derivationPath -> accountId. Only needed for fast lookup. derivationPaths: Record; }; diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 4b6ad0fa6..ce70c028f 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -11,6 +11,7 @@ import type { import { Address } from '@metamask/bitcoindevkit'; import type { DiscoveredAccount, + KeyringAccount, KeyringResponse, Transaction as KeyringTransaction, KeyringRequest, @@ -54,6 +55,17 @@ jest.mock('@metamask/bitcoindevkit', () => { }; }); +/** + * Narrows `T | undefined` after `expect(value).toBeDefined()` for use in tests. + * + * @param value - Possibly undefined value. + * @returns The same value narrowed to `T`. + */ +function expectDefined(value: T | undefined): T { + expect(value).toBeDefined(); + return value as T; +} + describe('KeyringHandler', () => { const mockKeyringRequest = mock(); const mockAccounts = mock(); @@ -399,6 +411,14 @@ describe('KeyringHandler', () => { describe('createAccounts', () => { const entropySource = 'some-source'; + const mnemonicGroupIndex = (account: KeyringAccount): number => { + const { entropy } = account.options; + if (entropy?.type !== 'mnemonic') { + throw new Error('expected mnemonic entropy'); + } + return entropy.groupIndex; + }; + const buildMockAccount = (index: number): BitcoinAccount => mock({ id: `id-${index}`, @@ -416,8 +436,8 @@ describe('KeyringHandler', () => { }); it('creates a single account for Bip44DeriveIndex', async () => { - const account = buildMockAccount(2); - mockAccounts.create.mockResolvedValue(account); + const bitcoinAccount = buildMockAccount(2); + mockAccounts.createMany.mockResolvedValue([bitcoinAccount]); const result = await handler.createAccounts({ type: AccountCreationType.Bip44DeriveIndex, @@ -425,20 +445,25 @@ describe('KeyringHandler', () => { entropySource, }); - expect(mockAccounts.create).toHaveBeenCalledTimes(1); - expect(mockAccounts.create).toHaveBeenCalledWith({ - network: 'bitcoin', - entropySource, - index: 2, - addressType: 'p2wpkh', - synchronize: false, - }); + expect(mockAccounts.createMany).toHaveBeenCalledTimes(1); + expect(mockAccounts.createMany).toHaveBeenCalledWith([ + { + network: 'bitcoin', + entropySource, + index: 2, + addressType: 'p2wpkh', + synchronize: false, + }, + ]); expect(result).toHaveLength(1); + const keyringAccount = expectDefined(result[0]); + expect(keyringAccount.id).toBe('id-2'); + expect(mnemonicGroupIndex(keyringAccount)).toBe(2); }); it('creates an inclusive range of accounts for Bip44DeriveIndexRange', async () => { - mockAccounts.create.mockImplementation(async ({ index }) => - buildMockAccount(index), + mockAccounts.createMany.mockResolvedValue( + [0, 1, 2].map(buildMockAccount), ); const result = await handler.createAccounts({ @@ -447,20 +472,107 @@ describe('KeyringHandler', () => { entropySource, }); - expect(mockAccounts.create).toHaveBeenCalledTimes(3); - const calledIndices = mockAccounts.create.mock.calls - .map((call) => (call[0] as { index: number }).index) - .sort((a, b) => a - b); - expect(calledIndices).toStrictEqual([0, 1, 2]); - mockAccounts.create.mock.calls.forEach((call) => { - expect(call[0]).toMatchObject({ + expect(mockAccounts.createMany).toHaveBeenCalledTimes(1); + expect(mockAccounts.createMany).toHaveBeenCalledWith( + [0, 1, 2].map((index) => ({ network: 'bitcoin', entropySource, + index, addressType: 'p2wpkh', synchronize: false, - }); - }); + })), + ); expect(result).toHaveLength(3); + expect(result.map((a) => mnemonicGroupIndex(a))).toStrictEqual([0, 1, 2]); + }); + + it('creates one account when range from and to are equal', async () => { + mockAccounts.createMany.mockResolvedValue([buildMockAccount(9)]); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 9, to: 9 }, + entropySource, + }); + + expect(mockAccounts.createMany).toHaveBeenCalledTimes(1); + expect(mockAccounts.createMany).toHaveBeenCalledWith([ + expect.objectContaining({ index: 9 }), + ]); + expect(result).toHaveLength(1); + expect(mnemonicGroupIndex(expectDefined(result[0]))).toBe(9); + }); + + it('creates accounts for a non-zero index range', async () => { + mockAccounts.createMany.mockResolvedValue( + [10, 11, 12].map(buildMockAccount), + ); + + await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 10, to: 12 }, + entropySource, + }); + + expect(mockAccounts.createMany).toHaveBeenCalledWith( + [10, 11, 12].map((index) => expect.objectContaining({ index })), + ); + }); + + it('allows the maximum batch size of 100 accounts in one RPC', async () => { + const indices = Array.from({ length: 100 }, (_, index) => index); + mockAccounts.createMany.mockResolvedValue(indices.map(buildMockAccount)); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 0, to: 99 }, + entropySource, + }); + + expect(mockAccounts.createMany).toHaveBeenCalledTimes(1); + expect(mockAccounts.createMany).toHaveBeenCalledWith( + indices.map((index) => expect.objectContaining({ index })), + ); + expect(result).toHaveLength(100); + expect(mnemonicGroupIndex(expectDefined(result[0]))).toBe(0); + expect(mnemonicGroupIndex(expectDefined(result[99]))).toBe(99); + }); + + it('returns accounts in the order returned by createMany', async () => { + mockAccounts.createMany.mockResolvedValue( + [0, 1, 2, 3, 4].map(buildMockAccount), + ); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 0, to: 4 }, + entropySource, + }); + + expect(result.map((a) => mnemonicGroupIndex(a))).toStrictEqual([ + 0, 1, 2, 3, 4, + ]); + }); + + it('rejects when the handler default address type is not P2WPKH', async () => { + const handlerNonSegwit = new KeyringHandler( + mockKeyringRequest, + mockAccounts, + 'p2tr' as AddressType, + mockSnapClient, + mockLogger, + ); + + await expect( + handlerNonSegwit.createAccounts({ + type: AccountCreationType.Bip44DeriveIndex, + groupIndex: 0, + entropySource, + }), + ).rejects.toThrow( + /Only native segwit \(P2WPKH\) addresses are supported/iu, + ); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); }); it('rejects an invalid index range when from is greater than to', async () => { @@ -471,7 +583,22 @@ describe('KeyringHandler', () => { entropySource, }), ).rejects.toThrow(/invalid.*range|from must be/iu); - expect(mockAccounts.create).not.toHaveBeenCalled(); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); + }); + + it.each([ + { from: -1, to: 0 }, + { from: 0.5, to: 1 }, + { from: 0, to: Number.MAX_SAFE_INTEGER + 1 }, + ])('rejects invalid account index bounds %#', async (range) => { + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range, + entropySource, + }), + ).rejects.toThrow(/non-negative integers/iu); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); }); it('rejects batches larger than the per-RPC limit', async () => { @@ -482,7 +609,7 @@ describe('KeyringHandler', () => { entropySource, }), ).rejects.toThrow(/more than 100 accounts/iu); - expect(mockAccounts.create).not.toHaveBeenCalled(); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); }); it('rejects unsupported creation types', async () => { @@ -493,12 +620,12 @@ describe('KeyringHandler', () => { entropySource, }), ).rejects.toThrow(/not supported|unsupported/iu); - expect(mockAccounts.create).not.toHaveBeenCalled(); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); }); - it('propagates errors from create', async () => { + it('propagates errors from createMany', async () => { const error = new Error('create error'); - mockAccounts.create.mockRejectedValue(error); + mockAccounts.createMany.mockRejectedValue(error); await expect( handler.createAccounts({ @@ -519,7 +646,7 @@ describe('KeyringHandler', () => { beforeEach(() => { mockSnapClient.startTrace.mockResolvedValue(undefined); mockSnapClient.endTrace.mockResolvedValue(undefined); - mockAccounts.create.mockResolvedValue(buildMockAccount(0)); + mockAccounts.createMany.mockResolvedValue([buildMockAccount(0)]); }); it('calls startTrace and endTrace with correct trace name', async () => { @@ -534,7 +661,7 @@ describe('KeyringHandler', () => { }); it('calls endTrace even if create fails', async () => { - mockAccounts.create.mockRejectedValue(new Error('boom')); + mockAccounts.createMany.mockRejectedValue(new Error('boom')); await expect(handler.createAccounts(options)).rejects.toThrow('boom'); expect(mockSnapClient.endTrace).toHaveBeenCalledWith( diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 8dc54a365..33c1ced2b 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -84,45 +84,6 @@ export const CreateAccountRequest = object({ /** Upper bound on inclusive index range width for a single createAccounts RPC. */ const MAX_CREATE_ACCOUNTS_PER_BATCH = 100; -/** Max concurrent AccountUseCases.create calls within one batch. */ -const CREATE_ACCOUNTS_CONCURRENCY = 4; - -/** - * Map items to results with at most `concurrency` in-flight async operations. - * Output order matches `items` order. - * - * @param items - Values to map in pool order. - * @param concurrency - Maximum number of concurrent mapper executions. - * @param mapper - Async function applied to each item. - * @returns Results in the same order as `items`. - */ -async function runWithConcurrencyLimit( - items: readonly Item[], - concurrency: number, - mapper: (item: Item, index: number) => Promise, -): Promise { - if (items.length === 0) { - return []; - } - const results: Result[] = new Array(items.length); - let next = 0; - - const worker = async (): Promise => { - while (true) { - const idx = next; - next += 1; - if (idx >= items.length) { - return; - } - results[idx] = await mapper(items[idx] as Item, idx); - } - }; - - const poolSize = Math.min(Math.max(1, concurrency), items.length); - await Promise.all(Array.from({ length: poolSize }, async () => worker())); - return results; -} - export class KeyringHandler implements Keyring { readonly #accountsUseCases: AccountUseCases; @@ -330,11 +291,6 @@ export class KeyringHandler implements Keyring { async createAccounts( options: CreateAccountOptions, ): Promise { - console.log( - '[PERFORMANCE LOGS][BITCOIN SNAP] execute createAccounts method', - options, - ); - assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, @@ -347,6 +303,17 @@ export class KeyringHandler implements Keyring { ? { from: options.groupIndex, to: options.groupIndex } : options.range; + if ( + !Number.isSafeInteger(range.from) || + !Number.isSafeInteger(range.to) || + range.from < 0 || + range.to < 0 + ) { + throw new FormatError( + 'Account index range is invalid: from and to must be non-negative integers', + ); + } + if (range.from > range.to) { throw new FormatError( 'Account index range is invalid: from must be less than or equal to to', @@ -388,22 +355,18 @@ export class KeyringHandler implements Keyring { 'startTrace', ); - // `AccountUseCases.create` is idempotent: if an account already exists + // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. - return await runWithConcurrencyLimit( - indices, - CREATE_ACCOUNTS_CONCURRENCY, - async (index) => { - const account = await this.#accountsUseCases.create({ - network, - entropySource, - index, - addressType, - synchronize: false, - }); - return mapToKeyringAccount(account); - }, + const accounts = await this.#accountsUseCases.createMany( + indices.map((index) => ({ + network, + entropySource, + index, + addressType, + synchronize: false, + })), ); + return accounts.map(mapToKeyringAccount); } finally { if (traceStarted) { await runSnapActionSafely( diff --git a/packages/snap/src/store/BdkAccountRepository.test.ts b/packages/snap/src/store/BdkAccountRepository.test.ts index eb1317705..da342c651 100644 --- a/packages/snap/src/store/BdkAccountRepository.test.ts +++ b/packages/snap/src/store/BdkAccountRepository.test.ts @@ -185,6 +185,94 @@ describe('BdkAccountRepository', () => { }); }); + describe('getByDerivationPaths', () => { + const derivationPath1 = ['m', "84'", "0'", "1'"]; + const derivationPath2 = ['m', "84'", "0'", "2'"]; + const accountState1 = { + ...mockAccountState, + derivationPath: derivationPath1, + }; + const accountState2 = { + ...mockAccountState, + derivationPath: derivationPath2, + }; + const mockAccount1 = mock({ + ...mockAccount, + id: 'some-id-1', + derivationPath: derivationPath1, + }); + const mockAccount2 = mock({ + ...mockAccount, + id: 'some-id-2', + derivationPath: derivationPath2, + }); + + it('returns accounts in derivation path order with one state read per namespace', async () => { + mockSnapClient.getState + .mockResolvedValueOnce({ + "m/84'/0'/1'": 'some-id-1', + "m/84'/0'/2'": 'some-id-2', + }) + .mockResolvedValueOnce({ + 'some-id-1': accountState1, + 'some-id-2': accountState2, + }); + (BdkAccountAdapter.load as jest.Mock) + .mockReturnValueOnce(mockAccount2) + .mockReturnValueOnce(mockAccount1); + + const result = await repo.getByDerivationPaths([ + derivationPath2, + derivationPath1, + ]); + + expect(mockSnapClient.getState).toHaveBeenCalledWith('derivationPaths'); + expect(mockSnapClient.getState).toHaveBeenCalledWith('accounts'); + expect(mockSnapClient.getState).toHaveBeenCalledTimes(2); + expect(result).toStrictEqual([mockAccount2, mockAccount1]); + expect(mockSnapClient.setState).not.toHaveBeenCalled(); + }); + + it('repairs missing derivation path indexes from account state', async () => { + mockSnapClient.getState + .mockResolvedValueOnce({ + "m/84'/0'/1'": 'some-id-1', + }) + .mockResolvedValueOnce({ + 'some-id-1': accountState1, + 'some-id-2': accountState2, + }); + (BdkAccountAdapter.load as jest.Mock) + .mockReturnValueOnce(mockAccount1) + .mockReturnValueOnce(mockAccount2); + + const result = await repo.getByDerivationPaths([ + derivationPath1, + derivationPath2, + ]); + + expect(result).toStrictEqual([mockAccount1, mockAccount2]); + expect(mockSnapClient.setState).toHaveBeenCalledWith('derivationPaths', { + "m/84'/0'/1'": 'some-id-1', + "m/84'/0'/2'": 'some-id-2', + }); + }); + + it('repairs a missing derivation path index for a single lookup', async () => { + mockSnapClient.getState.mockResolvedValueOnce({}).mockResolvedValueOnce({ + 'some-id-1': accountState1, + }); + (BdkAccountAdapter.load as jest.Mock).mockReturnValueOnce(mockAccount1); + + const result = await repo.getByDerivationPaths([derivationPath1]); + + expect(result).toStrictEqual([mockAccount1]); + expect(mockSnapClient.setState).toHaveBeenCalledWith('derivationPaths', { + "m/84'/0'/1'": 'some-id-1', + }); + }); + }); + describe('getWithSigner', () => { it('returns null if account not found', async () => { mockSnapClient.getState.mockResolvedValue(null); @@ -258,6 +346,85 @@ describe('BdkAccountRepository', () => { }); }); + describe('insertMany', () => { + it('returns an empty array when there are no accounts to insert', async () => { + const result = await repo.insertMany([]); + + expect(result).toStrictEqual([]); + expect(mockSnapClient.getState).not.toHaveBeenCalled(); + expect(mockSnapClient.setState).not.toHaveBeenCalled(); + }); + + it('throws an error if any account has no wallet data', async () => { + await expect( + repo.insertMany([ + { + ...mockAccount, + id: 'missing-wallet', + takeStaged: jest.fn().mockReturnValue(undefined), + }, + mockAccount, + ]), + ).rejects.toThrow( + 'Missing changeset data for account "missing-wallet" for insertion.', + ); + expect(mockSnapClient.setState).not.toHaveBeenCalled(); + }); + + it('inserts multiple accounts with one accounts write and one derivation path write', async () => { + const existingAccountState: AccountState = { + wallet: mockWalletData, + inscriptions: [], + derivationPath: mockDerivationPath, + }; + const account1 = mock(); + account1.id = 'some-id-1'; + account1.derivationPath = ['m', "84'", "0'", "1'"]; + const account2 = mock(); + account2.id = 'some-id-2'; + account2.derivationPath = ['m', "84'", "0'", "2'"]; + (account1.takeStaged as jest.Mock) = jest + .fn() + .mockReturnValue(mockChangeSet); + (account2.takeStaged as jest.Mock) = jest + .fn() + .mockReturnValue(mockChangeSet); + mockSnapClient.getState + .mockResolvedValueOnce({ + 'existing-id': existingAccountState, + }) + .mockResolvedValueOnce({ + "m/84'/0'/0'": 'existing-id', + }); + + const result = await repo.insertMany([account1, account2]); + + expect(result).toStrictEqual([account1, account2]); + expect(mockSnapClient.setState).toHaveBeenNthCalledWith(1, 'accounts', { + 'existing-id': existingAccountState, + 'some-id-1': { + wallet: mockWalletData, + inscriptions: [], + derivationPath: ['m', "84'", "0'", "1'"], + }, + 'some-id-2': { + wallet: mockWalletData, + inscriptions: [], + derivationPath: ['m', "84'", "0'", "2'"], + }, + }); + expect(mockSnapClient.setState).toHaveBeenNthCalledWith( + 2, + 'derivationPaths', + { + "m/84'/0'/0'": 'existing-id', + "m/84'/0'/1'": 'some-id-1', + "m/84'/0'/2'": 'some-id-2', + }, + ); + }); + }); + describe('update', () => { it('does nothing if no wallet data', async () => { await repo.update({ diff --git a/packages/snap/src/store/BdkAccountRepository.ts b/packages/snap/src/store/BdkAccountRepository.ts index 37b5d4442..e03dc8ea1 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -34,6 +34,14 @@ function toBdkFingerprint(fingerprint: number): string { return fingerprint.toString(16).padStart(8, '0'); } +/** + * @param derivationPath - Split derivation path. + * @returns Storage key for a derivation path. + */ +function getDerivationPathKey(derivationPath: string[]): string { + return derivationPath.join('/'); +} + export class BdkAccountRepository implements BitcoinAccountRepository { readonly #snapClient: SnapClient; @@ -49,11 +57,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return null; } - return BdkAccountAdapter.load( - id, - account.derivationPath, - ChangeSet.from_json(account.wallet), - ); + return this.#loadAccount(id, account); } async getAll(): Promise { @@ -64,22 +68,16 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return []; } - return Object.entries(accounts) - .filter(([, account]) => account !== null) - .map(([id, account]) => - BdkAccountAdapter.load( - id, - account.derivationPath, - ChangeSet.from_json(account.wallet), - ), - ); + return Object.entries(accounts).flatMap(([id, account]) => + account ? [this.#loadAccount(id, account)] : [], + ); } async getByDerivationPath( derivationPath: string[], ): Promise { const id = await this.#snapClient.getState( - `derivationPaths.${derivationPath.join('/')}`, + `derivationPaths.${getDerivationPathKey(derivationPath)}`, ); if (!id) { return null; @@ -88,6 +86,65 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return this.get(id as string); } + async getByDerivationPaths( + derivationPaths: string[][], + ): Promise<(BitcoinAccount | null)[]> { + if (derivationPaths.length === 0) { + return []; + } + + const [derivationPathIndex, accounts] = await Promise.all([ + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + ]); + + const accountsById = accounts ?? {}; + const existingDerivationPathIndex = derivationPathIndex ?? {}; + const accountsByDerivationPath = new Map(); + + for (const [id, account] of Object.entries(accountsById)) { + if (account) { + accountsByDerivationPath.set( + getDerivationPathKey(account.derivationPath), + [id, account], + ); + } + } + + const repairs: Record = {}; + const results = derivationPaths.map((derivationPath) => { + const pathKey = getDerivationPathKey(derivationPath); + const indexedId = existingDerivationPathIndex[pathKey]; + const indexedAccount = indexedId ? accountsById[indexedId] : null; + + if (indexedId && indexedAccount) { + return this.#loadAccount(indexedId, indexedAccount); + } + + const fallback = accountsByDerivationPath.get(pathKey); + if (!fallback) { + return null; + } + + const [id, account] = fallback; + repairs[pathKey] = id; + return this.#loadAccount(id, account); + }); + + if (Object.keys(repairs).length > 0) { + await this.#snapClient.setState('derivationPaths', { + ...existingDerivationPathIndex, + ...repairs, + }); + } + + return results; + } + async getWithSigner(id: string): Promise { const accountState = (await this.#snapClient.getState( `accounts.${id}`, @@ -158,7 +215,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { await Promise.all([ this.#snapClient.setState( - `derivationPaths.${derivationPath.join('/')}`, + `derivationPaths.${getDerivationPathKey(derivationPath)}`, id, ), this.#snapClient.setState(`accounts.${id}`, { @@ -171,6 +228,61 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return account; } + async insertMany(accounts: BitcoinAccount[]): Promise { + if (accounts.length === 0) { + return []; + } + + if (accounts.length === 1) { + return [await this.insert(accounts[0] as BitcoinAccount)]; + } + + const accountStateEntries: [string, AccountState][] = []; + const derivationPathEntries: [string, string][] = []; + + for (const account of accounts) { + const { id, derivationPath } = account; + const walletData = account.takeStaged(); + + if (!walletData) { + throw new StorageError( + `Missing changeset data for account "${id}" for insertion.`, + ); + } + + accountStateEntries.push([ + id, + { + wallet: walletData.to_json(), + inscriptions: [], + derivationPath, + }, + ]); + derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); + } + + const [existingAccounts, existingDerivationPaths] = await Promise.all([ + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + ]); + + await this.#snapClient.setState('accounts', { + ...(existingAccounts ?? {}), + ...Object.fromEntries(accountStateEntries), + }); + + await this.#snapClient.setState('derivationPaths', { + ...(existingDerivationPaths ?? {}), + ...Object.fromEntries(derivationPathEntries), + }); + + return accounts; + } + async update( account: BitcoinAccount, inscriptions?: Inscription[], @@ -246,4 +358,12 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return `${txid}:${vout}`; }); } + + #loadAccount(id: string, account: AccountState): BitcoinAccount { + return BdkAccountAdapter.load( + id, + account.derivationPath, + ChangeSet.from_json(account.wallet), + ); + } } diff --git a/packages/snap/src/use-cases/AccountUseCases.test.ts b/packages/snap/src/use-cases/AccountUseCases.test.ts index bea64eeae..8d2cbefe8 100644 --- a/packages/snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/snap/src/use-cases/AccountUseCases.test.ts @@ -291,6 +291,161 @@ describe('AccountUseCases', () => { }); }); + describe('createMany', () => { + const createParams: CreateAccountParams = { + network: 'bitcoin', + entropySource: 'some-source', + index: 1, + addressType: 'p2wpkh', + synchronize: false, + correlationId: 'correlation-id', + accountName: 'My account', + }; + const firstDerivationPath = ['some-source', "84'", "0'", "1'"]; + const secondDerivationPath = ['some-source', "84'", "0'", "2'"]; + const existingAccount = mock({ + id: 'existing-id', + network: createParams.network, + }); + const newAccount = mock({ + id: 'new-id', + network: createParams.network, + }); + + it('reuses existing accounts and bulk-inserts newly-created accounts', async () => { + mockRepository.getByDerivationPaths.mockResolvedValue([ + existingAccount, + null, + ]); + mockRepository.create.mockResolvedValue(newAccount); + + const result = await useCases.createMany([ + createParams, + { ...createParams, index: 2, synchronize: true }, + ]); + + expect(mockRepository.getByDerivationPaths).toHaveBeenCalledWith([ + firstDerivationPath, + secondDerivationPath, + ]); + expect(mockRepository.create).toHaveBeenCalledWith( + secondDerivationPath, + createParams.network, + createParams.addressType, + ); + expect(newAccount.revealNextAddress).toHaveBeenCalled(); + expect(mockRepository.insertMany).toHaveBeenCalledWith([newAccount]); + expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenNthCalledWith( + 1, + existingAccount, + createParams.correlationId, + createParams.accountName, + ); + expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenNthCalledWith( + 2, + newAccount, + createParams.correlationId, + createParams.accountName, + ); + expect(mockSnapClient.scheduleBackgroundEvent).toHaveBeenCalledWith({ + duration: 'PT1S', + method: CronMethod.FullScanAccount, + params: { accountId: newAccount.id }, + }); + expect(result).toStrictEqual([existingAccount, newAccount]); + }); + + it('creates only one account for duplicate derivation paths in the same batch', async () => { + mockRepository.getByDerivationPaths.mockResolvedValue([null]); + mockRepository.create.mockResolvedValue(newAccount); + + const result = await useCases.createMany([createParams, createParams]); + + expect(mockRepository.getByDerivationPaths).toHaveBeenCalledWith([ + firstDerivationPath, + ]); + expect(mockRepository.create).toHaveBeenCalledTimes(1); + expect(mockRepository.insertMany).toHaveBeenCalledWith([newAccount]); + expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenCalledTimes(2); + expect(result).toStrictEqual([newAccount, newAccount]); + }); + + it('does not create or insert accounts when all accounts already exist', async () => { + mockRepository.getByDerivationPaths.mockResolvedValue([existingAccount]); + + const result = await useCases.createMany([createParams]); + + expect(mockRepository.create).not.toHaveBeenCalled(); + expect(mockRepository.insertMany).not.toHaveBeenCalled(); + expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenCalledWith( + existingAccount, + createParams.correlationId, + createParams.accountName, + ); + expect(result).toStrictEqual([existingAccount]); + }); + + it('propagates insertMany errors before emitting events', async () => { + const error = new Error('insertMany failed'); + mockRepository.getByDerivationPaths.mockResolvedValue([null]); + mockRepository.create.mockResolvedValue(newAccount); + mockRepository.insertMany.mockRejectedValue(error); + + await expect(useCases.createMany([createParams])).rejects.toBe(error); + + expect(mockRepository.insertMany).toHaveBeenCalledWith([newAccount]); + expect(mockSnapClient.emitAccountCreatedEvent).not.toHaveBeenCalled(); + }); + + it('waits for in-flight creates before rejecting when one create fails', async () => { + const error = new Error('create failed'); + const slowAccount = mock({ + id: 'slow-id', + network: createParams.network, + }); + let resolveSlowCreate: (account: BitcoinAccount) => void = () => + undefined; + const slowCreate = new Promise((resolve) => { + resolveSlowCreate = resolve; + }); + const callOrder: string[] = []; + + mockRepository.getByDerivationPaths.mockResolvedValue([null, null]); + mockRepository.create + .mockImplementationOnce(async () => { + callOrder.push('create-1'); + throw error; + }) + .mockImplementationOnce(async () => { + callOrder.push('create-2'); + const account = await slowCreate; + callOrder.push('resolve-2'); + return account; + }); + + const createManyPromise = useCases.createMany([ + createParams, + { ...createParams, index: 2 }, + ]); + const onSettled = jest.fn(); + const settlementObserver = createManyPromise.then(onSettled, onSettled); + + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + expect(onSettled).not.toHaveBeenCalled(); + + resolveSlowCreate(slowAccount); + + await expect(createManyPromise).rejects.toBe(error); + await settlementObserver; + expect(callOrder).toStrictEqual(['create-1', 'create-2', 'resolve-2']); + expect(mockRepository.insertMany).not.toHaveBeenCalled(); + expect(mockSnapClient.emitAccountCreatedEvent).not.toHaveBeenCalled(); + }); + }); + describe('discover', () => { const discoverParams: DiscoverAccountParams = { network: 'bitcoin', diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 2cb2e1314..100e54ed0 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -48,6 +48,82 @@ export type CreateAccountParams = DiscoverAccountParams & { accountName?: string; }; +const CREATE_ACCOUNTS_CONCURRENCY = 4; + +/** + * @param req - Account creation or discovery request. + * @returns The BIP-44 account derivation path. + */ +function getAccountDerivationPath(req: DiscoverAccountParams): string[] { + return [ + req.entropySource, + `${addressTypeToPurpose[req.addressType]}'`, + `${networkToCoinType[req.network]}'`, + `${req.index}'`, + ]; +} + +/** + * @param derivationPath - Split derivation path. + * @returns Storage key for a derivation path. + */ +function getDerivationPathKey(derivationPath: string[]): string { + return derivationPath.join('/'); +} + +/** + * Map items to results with at most `concurrency` in-flight async operations. + * Output order matches `items` order. + * + * @param items - Values to map in pool order. + * @param concurrency - Maximum number of concurrent mapper executions. + * @param mapper - Async function applied to each item. + * @returns Results in the same order as `items`. + */ +async function runWithConcurrencyLimit( + items: readonly Item[], + concurrency: number, + mapper: (item: Item, index: number) => Promise, +): Promise { + if (items.length === 0) { + return []; + } + + const results: Result[] = new Array(items.length); + let next = 0; + let firstError: unknown; + let hasError = false; + + const worker = async (): Promise => { + while (!hasError) { + const idx = next; + next += 1; + if (idx >= items.length) { + return; + } + + try { + results[idx] = await mapper(items[idx] as Item, idx); + } catch (error) { + if (!hasError) { + firstError = error; + hasError = true; + } + return; + } + } + }; + + const poolSize = Math.min(Math.max(1, concurrency), items.length); + await Promise.all(Array.from({ length: poolSize }, async () => worker())); + + if (hasError) { + throw firstError; + } + + return results; +} + export class AccountUseCases { readonly #logger: Logger; @@ -65,6 +141,8 @@ export class AccountUseCases { readonly #targetBlocksConfirmation: number; + #accountMutationQueue: Promise = Promise.resolve(); + constructor( logger: Logger, snapClient: SnapClient, @@ -109,14 +187,8 @@ export class AccountUseCases { async discover(req: DiscoverAccountParams): Promise { this.#logger.debug('Discovering Bitcoin account. Request: %o', req); - const { addressType, index, network, entropySource } = req; - - const derivationPath = [ - entropySource, - `${addressTypeToPurpose[addressType]}'`, - `${networkToCoinType[network]}'`, - `${index}'`, - ]; + const { addressType, network } = req; + const derivationPath = getAccountDerivationPath(req); // Idempotent account creation + ensures only one account per derivation path const account = await this.#repository.getByDerivationPath(derivationPath); @@ -146,67 +218,174 @@ export class AccountUseCases { async create(req: CreateAccountParams): Promise { this.#logger.debug('Creating new Bitcoin account. Request: %o', req); - const { - addressType, - index, - network, - entropySource, - correlationId, - accountName, - synchronize, - } = req; - - const derivationPath = [ - entropySource, - `${addressTypeToPurpose[addressType]}'`, - `${networkToCoinType[network]}'`, - `${index}'`, - ]; + return this.#runAccountMutation(async () => { + const { addressType, network, correlationId, accountName, synchronize } = + req; + const derivationPath = getAccountDerivationPath(req); + + // Idempotent account creation + ensures only one account per derivation path + const account = + await this.#repository.getByDerivationPath(derivationPath); + if (account && account.network === network) { + this.#logger.debug('Account already exists: %s,', account.id); + await this.#snapClient.emitAccountCreatedEvent( + account, + correlationId, + accountName, + ); + return account; + } - // Idempotent account creation + ensures only one account per derivation path - const account = await this.#repository.getByDerivationPath(derivationPath); - if (account && account.network === network) { - this.#logger.debug('Account already exists: %s,', account.id); + const newAccount = await this.#repository.create( + derivationPath, + network, + addressType, + ); + + newAccount.revealNextAddress(); + + await this.#repository.insert(newAccount); + + // First notify the event has been created, then schedule full scan. await this.#snapClient.emitAccountCreatedEvent( - account, + newAccount, correlationId, accountName, ); - return account; + + if (synchronize) { + await this.#snapClient.scheduleBackgroundEvent({ + duration: 'PT1S', + method: CronMethod.FullScanAccount, + params: { accountId: newAccount.id }, + }); + } + + this.#logger.info( + 'Bitcoin account created successfully: %s. Public address: %s, Request: %o', + newAccount.id, + newAccount.publicAddress, + req, + ); + return newAccount; + }); + } + + async createMany(reqs: CreateAccountParams[]): Promise { + if (reqs.length === 0) { + return []; } - const newAccount = await this.#repository.create( - derivationPath, - network, - addressType, - ); + const { accounts, createdAccountKeys } = await this.#runAccountMutation( + async () => { + const entries = reqs.map((req, index) => { + const derivationPath = getAccountDerivationPath(req); + return { + req, + index, + derivationPath, + pathKey: getDerivationPathKey(derivationPath), + }; + }); + + const uniqueEntriesByPath = new Map(); + for (const entry of entries) { + if (!uniqueEntriesByPath.has(entry.pathKey)) { + uniqueEntriesByPath.set(entry.pathKey, entry); + } + } + const uniqueEntries = [...uniqueEntriesByPath.values()]; + + const existingAccounts = await this.#repository.getByDerivationPaths( + uniqueEntries.map(({ derivationPath }) => derivationPath), + ); + const existingAccountsByPath = new Map(); + + uniqueEntries.forEach((entry, index) => { + const account = existingAccounts[index]; + if (account && account.network === entry.req.network) { + existingAccountsByPath.set(entry.pathKey, account); + } + }); + + const entriesToCreate = uniqueEntries.filter( + ({ pathKey }) => !existingAccountsByPath.has(pathKey), + ); + const newAccounts = await runWithConcurrencyLimit( + entriesToCreate, + CREATE_ACCOUNTS_CONCURRENCY, + async ({ derivationPath, req }) => { + const newAccount = await this.#repository.create( + derivationPath, + req.network, + req.addressType, + ); + newAccount.revealNextAddress(); + return newAccount; + }, + ); - newAccount.revealNextAddress(); + if (newAccounts.length > 0) { + await this.#repository.insertMany(newAccounts); + } - await this.#repository.insert(newAccount); + const newAccountsByPath = new Map( + entriesToCreate.map((entry, index) => [ + entry.pathKey, + newAccounts[index] as BitcoinAccount, + ]), + ); - // First notify the event has been created, then schedule full scan. - await this.#snapClient.emitAccountCreatedEvent( - newAccount, - correlationId, - accountName, + const accountsInOrder = entries.map((entry) => { + const account = + existingAccountsByPath.get(entry.pathKey) ?? + newAccountsByPath.get(entry.pathKey); + + if (!account) { + throw new AssertionError('Failed to create account', { + index: entry.index, + derivationPath: entry.derivationPath, + }); + } + + return account; + }); + + return { + accounts: accountsInOrder, + createdAccountKeys: new Set(newAccountsByPath.keys()), + }; + }, ); - if (synchronize) { - await this.#snapClient.scheduleBackgroundEvent({ - duration: 'PT1S', - method: CronMethod.FullScanAccount, - params: { accountId: newAccount.id }, - }); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + await this.#snapClient.emitAccountCreatedEvent( + account, + req.correlationId, + req.accountName, + ); } - this.#logger.info( - 'Bitcoin account created successfully: %s. Public address: %s, Request: %o', - newAccount.id, - newAccount.publicAddress, - req, - ); - return newAccount; + const scheduledAccountIds = new Set(); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); + if ( + req.synchronize && + createdAccountKeys.has(pathKey) && + !scheduledAccountIds.has(account.id) + ) { + scheduledAccountIds.add(account.id); + await this.#snapClient.scheduleBackgroundEvent({ + duration: 'PT1S', + method: CronMethod.FullScanAccount, + params: { accountId: account.id }, + }); + } + } + + return accounts; } async synchronize( @@ -331,15 +510,17 @@ export class AccountUseCases { async delete(id: string): Promise { this.#logger.debug('Deleting account: %s', id); - const account = await this.#repository.get(id); - if (!account) { - throw new NotFoundError('Account not found', { id }); - } + await this.#runAccountMutation(async () => { + const account = await this.#repository.get(id); + if (!account) { + throw new NotFoundError('Account not found', { id }); + } - await this.#snapClient.emitAccountDeletedEvent(id); - await this.#repository.delete(id); + await this.#snapClient.emitAccountDeletedEvent(id); + await this.#repository.delete(id); - this.#logger.info('Account deleted successfully: %s', account.id); + this.#logger.info('Account deleted successfully: %s', account.id); + }); } async fillPsbt( @@ -664,6 +845,24 @@ export class AccountUseCases { return txid; } + async #runAccountMutation( + fn: () => Promise, + ): Promise { + const previousMutation = this.#accountMutationQueue; + let releaseMutation: () => void = () => undefined; + this.#accountMutationQueue = new Promise((resolve) => { + releaseMutation = resolve; + }); + + await previousMutation; + + try { + return await fn(); + } finally { + releaseMutation(); + } + } + #checkCapability( account: BitcoinAccount, capability: AccountCapability, From 487302f14cc0cf9b67a23ab0132d6349fb5b0230 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 6 May 2026 16:12:40 -0400 Subject: [PATCH 17/26] chore: add debug logs --- packages/snap/src/handlers/KeyringHandler.ts | 34 ++ packages/snap/src/index.ts | 2 +- .../snap/src/store/BdkAccountRepository.ts | 416 +++++++++++++----- .../snap/src/use-cases/AccountUseCases.ts | 282 ++++++++---- packages/snap/src/utils/performance.ts | 30 ++ 5 files changed, 557 insertions(+), 207 deletions(-) create mode 100644 packages/snap/src/utils/performance.ts diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 33c1ced2b..4c312e90d 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -68,6 +68,7 @@ import { } from './mappings'; import { validateSelectedAccounts } from './validation'; import type { AccountUseCases } from '../use-cases/AccountUseCases'; +import { getElapsedTimeMs, logPerformanceDebug } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export const CreateAccountRequest = object({ @@ -291,6 +292,10 @@ export class KeyringHandler implements Keyring { async createAccounts( options: CreateAccountOptions, ): Promise { + const methodStartedAt = Date.now(); + let resultCount = 0; + let succeeded = false; + assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, @@ -327,6 +332,13 @@ export class KeyringHandler implements Keyring { ); } + logPerformanceDebug(this.#logger, 'KeyringHandler.createAccounts started', { + batchSize, + rangeFrom: range.from, + rangeTo: range.to, + type: options.type, + }); + // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring // the defaults used by `createAccount` when no scope is provided. const network = scopeToNetwork[BtcScope.Mainnet]; @@ -357,6 +369,7 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. + const createManyStartedAt = Date.now(); const accounts = await this.#accountsUseCases.createMany( indices.map((index) => ({ network, @@ -366,6 +379,17 @@ export class KeyringHandler implements Keyring { synchronize: false, })), ); + resultCount = accounts.length; + logPerformanceDebug( + this.#logger, + 'KeyringHandler.createAccounts createMany completed', + { + batchSize, + durationMs: getElapsedTimeMs(createManyStartedAt), + resultCount, + }, + ); + succeeded = true; return accounts.map(mapToKeyringAccount); } finally { if (traceStarted) { @@ -375,6 +399,16 @@ export class KeyringHandler implements Keyring { 'endTrace', ); } + logPerformanceDebug( + this.#logger, + 'KeyringHandler.createAccounts finished', + { + batchSize, + durationMs: getElapsedTimeMs(methodStartedAt), + resultCount, + succeeded, + }, + ); } } diff --git a/packages/snap/src/index.ts b/packages/snap/src/index.ts index 5759e0c73..1cb437d34 100644 --- a/packages/snap/src/index.ts +++ b/packages/snap/src/index.ts @@ -46,7 +46,7 @@ const translator = new LocalTranslatorAdapter(); const middleware = new HandlerMiddleware(logger, snapClient, translator); // Data layer -const accountRepository = new BdkAccountRepository(snapClient); +const accountRepository = new BdkAccountRepository(snapClient, logger); const sendFlowRepository = new JSXSendFlowRepository(snapClient, translator); const confirmationRepository = new JSXConfirmationRepository( snapClient, diff --git a/packages/snap/src/store/BdkAccountRepository.ts b/packages/snap/src/store/BdkAccountRepository.ts index e03dc8ea1..b89f15f47 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -17,9 +17,11 @@ import { type Inscription, type AccountState, type SnapState, + type Logger, StorageError, } from '../entities'; import { BdkAccountAdapter } from '../infra'; +import { getElapsedTimeMs, logPerformanceDebug } from '../utils/performance'; /** * Encode a fingerprint to a 4-bytes hex-string (required by the BDK). @@ -45,8 +47,11 @@ function getDerivationPathKey(derivationPath: string[]): string { export class BdkAccountRepository implements BitcoinAccountRepository { readonly #snapClient: SnapClient; - constructor(snapClient: SnapClient) { + readonly #logger: Logger | undefined; + + constructor(snapClient: SnapClient, logger?: Logger) { this.#snapClient = snapClient; + this.#logger = logger; } async get(id: string): Promise { @@ -89,60 +94,124 @@ export class BdkAccountRepository implements BitcoinAccountRepository { async getByDerivationPaths( derivationPaths: string[][], ): Promise<(BitcoinAccount | null)[]> { + const methodStartedAt = Date.now(); + let repairedIndexCount = 0; + let resultCount = 0; + let succeeded = false; + + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.getByDerivationPaths started', + { + requestedPathCount: derivationPaths.length, + }, + ); + if (derivationPaths.length === 0) { + succeeded = true; + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.getByDerivationPaths finished', + { + durationMs: getElapsedTimeMs(methodStartedAt), + repairedIndexCount, + requestedPathCount: derivationPaths.length, + resultCount, + succeeded, + }, + ); return []; } - const [derivationPathIndex, accounts] = await Promise.all([ - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - ]); - - const accountsById = accounts ?? {}; - const existingDerivationPathIndex = derivationPathIndex ?? {}; - const accountsByDerivationPath = new Map(); + try { + const stateReadStartedAt = Date.now(); + const [derivationPathIndex, accounts] = await Promise.all([ + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + ]); - for (const [id, account] of Object.entries(accountsById)) { - if (account) { - accountsByDerivationPath.set( - getDerivationPathKey(account.derivationPath), - [id, account], - ); + const accountsById = accounts ?? {}; + const existingDerivationPathIndex = derivationPathIndex ?? {}; + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.getByDerivationPaths state read completed', + { + accountCount: Object.keys(accountsById).length, + durationMs: getElapsedTimeMs(stateReadStartedAt), + indexedPathCount: Object.keys(existingDerivationPathIndex).length, + }, + ); + const accountsByDerivationPath = new Map< + string, + [string, AccountState] + >(); + + for (const [id, account] of Object.entries(accountsById)) { + if (account) { + accountsByDerivationPath.set( + getDerivationPathKey(account.derivationPath), + [id, account], + ); + } } - } - const repairs: Record = {}; - const results = derivationPaths.map((derivationPath) => { - const pathKey = getDerivationPathKey(derivationPath); - const indexedId = existingDerivationPathIndex[pathKey]; - const indexedAccount = indexedId ? accountsById[indexedId] : null; + const repairs: Record = {}; + const results = derivationPaths.map((derivationPath) => { + const pathKey = getDerivationPathKey(derivationPath); + const indexedId = existingDerivationPathIndex[pathKey]; + const indexedAccount = indexedId ? accountsById[indexedId] : null; - if (indexedId && indexedAccount) { - return this.#loadAccount(indexedId, indexedAccount); - } - - const fallback = accountsByDerivationPath.get(pathKey); - if (!fallback) { - return null; - } + if (indexedId && indexedAccount) { + return this.#loadAccount(indexedId, indexedAccount); + } - const [id, account] = fallback; - repairs[pathKey] = id; - return this.#loadAccount(id, account); - }); + const fallback = accountsByDerivationPath.get(pathKey); + if (!fallback) { + return null; + } - if (Object.keys(repairs).length > 0) { - await this.#snapClient.setState('derivationPaths', { - ...existingDerivationPathIndex, - ...repairs, + const [id, account] = fallback; + repairs[pathKey] = id; + return this.#loadAccount(id, account); }); - } - return results; + if (Object.keys(repairs).length > 0) { + const repairStartedAt = Date.now(); + await this.#snapClient.setState('derivationPaths', { + ...existingDerivationPathIndex, + ...repairs, + }); + repairedIndexCount = Object.keys(repairs).length; + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.getByDerivationPaths index repair completed', + { + durationMs: getElapsedTimeMs(repairStartedAt), + repairedIndexCount, + }, + ); + } + + resultCount = results.filter(Boolean).length; + succeeded = true; + return results; + } finally { + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.getByDerivationPaths finished', + { + durationMs: getElapsedTimeMs(methodStartedAt), + repairedIndexCount, + requestedPathCount: derivationPaths.length, + resultCount, + succeeded, + }, + ); + } } async getWithSigner(id: string): Promise { @@ -186,101 +255,224 @@ export class BdkAccountRepository implements BitcoinAccountRepository { network: Network, addressType: AddressType, ): Promise { - const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); - const id = v4(); - const fingerprint = toBdkFingerprint( - slip10.masterFingerprint ?? slip10.parentFingerprint, - ); + const methodStartedAt = Date.now(); + let succeeded = false; + + try { + const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); + const id = v4(); + const fingerprint = toBdkFingerprint( + slip10.masterFingerprint ?? slip10.parentFingerprint, + ); - const xpub = slip10_to_extended(slip10, network); - const descriptors = xpub_to_descriptor( - xpub, - fingerprint, - network, - addressType, - ); + const xpub = slip10_to_extended(slip10, network); + const descriptors = xpub_to_descriptor( + xpub, + fingerprint, + network, + addressType, + ); - return BdkAccountAdapter.create(id, derivationPath, descriptors, network); + const account = BdkAccountAdapter.create( + id, + derivationPath, + descriptors, + network, + ); + succeeded = true; + return account; + } finally { + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.create finished', + { + addressType, + durationMs: getElapsedTimeMs(methodStartedAt), + network, + succeeded, + }, + ); + } } async insert(account: BitcoinAccount): Promise { + const methodStartedAt = Date.now(); + let succeeded = false; const { id, derivationPath } = account; - const walletData = account.takeStaged(); - if (!walletData) { - throw new StorageError( - `Missing changeset data for account "${id}" for insertion.`, - ); - } + try { + const walletData = account.takeStaged(); + if (!walletData) { + throw new StorageError( + `Missing changeset data for account "${id}" for insertion.`, + ); + } - await Promise.all([ - this.#snapClient.setState( - `derivationPaths.${getDerivationPathKey(derivationPath)}`, - id, - ), - this.#snapClient.setState(`accounts.${id}`, { - wallet: walletData.to_json(), - inscriptions: [], - derivationPath, - }), - ]); + await Promise.all([ + this.#snapClient.setState( + `derivationPaths.${getDerivationPathKey(derivationPath)}`, + id, + ), + this.#snapClient.setState(`accounts.${id}`, { + wallet: walletData.to_json(), + inscriptions: [], + derivationPath, + }), + ]); - return account; + succeeded = true; + return account; + } finally { + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insert finished', + { + accountId: id, + durationMs: getElapsedTimeMs(methodStartedAt), + succeeded, + }, + ); + } } async insertMany(accounts: BitcoinAccount[]): Promise { + const methodStartedAt = Date.now(); + let succeeded = false; + + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany started', + { + accountCount: accounts.length, + }, + ); + if (accounts.length === 0) { + succeeded = true; + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany finished', + { + accountCount: accounts.length, + durationMs: getElapsedTimeMs(methodStartedAt), + succeeded, + }, + ); return []; } if (accounts.length === 1) { - return [await this.insert(accounts[0] as BitcoinAccount)]; - } - - const accountStateEntries: [string, AccountState][] = []; - const derivationPathEntries: [string, string][] = []; - - for (const account of accounts) { - const { id, derivationPath } = account; - const walletData = account.takeStaged(); - - if (!walletData) { - throw new StorageError( - `Missing changeset data for account "${id}" for insertion.`, + try { + const result = [await this.insert(accounts[0] as BitcoinAccount)]; + succeeded = true; + return result; + } finally { + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany finished', + { + accountCount: accounts.length, + durationMs: getElapsedTimeMs(methodStartedAt), + succeeded, + }, ); } + } - accountStateEntries.push([ - id, + try { + const prepareStartedAt = Date.now(); + const accountStateEntries: [string, AccountState][] = []; + const derivationPathEntries: [string, string][] = []; + + for (const account of accounts) { + const { id, derivationPath } = account; + const walletData = account.takeStaged(); + + if (!walletData) { + throw new StorageError( + `Missing changeset data for account "${id}" for insertion.`, + ); + } + + accountStateEntries.push([ + id, + { + wallet: walletData.to_json(), + inscriptions: [], + derivationPath, + }, + ]); + derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); + } + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany account state prepared', { - wallet: walletData.to_json(), - inscriptions: [], - derivationPath, + accountCount: accounts.length, + durationMs: getElapsedTimeMs(prepareStartedAt), }, - ]); - derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); - } + ); - const [existingAccounts, existingDerivationPaths] = await Promise.all([ - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - ]); + const stateReadStartedAt = Date.now(); + const [existingAccounts, existingDerivationPaths] = await Promise.all([ + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + ]); + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany existing state read completed', + { + durationMs: getElapsedTimeMs(stateReadStartedAt), + existingAccountCount: Object.keys(existingAccounts ?? {}).length, + existingPathCount: Object.keys(existingDerivationPaths ?? {}).length, + }, + ); - await this.#snapClient.setState('accounts', { - ...(existingAccounts ?? {}), - ...Object.fromEntries(accountStateEntries), - }); + const accountsWriteStartedAt = Date.now(); + await this.#snapClient.setState('accounts', { + ...(existingAccounts ?? {}), + ...Object.fromEntries(accountStateEntries), + }); + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany accounts state write completed', + { + accountCount: accountStateEntries.length, + durationMs: getElapsedTimeMs(accountsWriteStartedAt), + }, + ); - await this.#snapClient.setState('derivationPaths', { - ...(existingDerivationPaths ?? {}), - ...Object.fromEntries(derivationPathEntries), - }); + const derivationPathsWriteStartedAt = Date.now(); + await this.#snapClient.setState('derivationPaths', { + ...(existingDerivationPaths ?? {}), + ...Object.fromEntries(derivationPathEntries), + }); + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany derivation path state write completed', + { + durationMs: getElapsedTimeMs(derivationPathsWriteStartedAt), + pathCount: derivationPathEntries.length, + }, + ); - return accounts; + succeeded = true; + return accounts; + } finally { + logPerformanceDebug( + this.#logger, + 'BdkAccountRepository.insertMany finished', + { + accountCount: accounts.length, + durationMs: getElapsedTimeMs(methodStartedAt), + succeeded, + }, + ); + } } async update( diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 100e54ed0..0fdf5a2b6 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -33,6 +33,7 @@ import { WalletError, } from '../entities'; import { CronMethod } from '../handlers/CronHandler'; +import { getElapsedTimeMs, logPerformanceDebug } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export type DiscoverAccountParams = { @@ -272,120 +273,213 @@ export class AccountUseCases { } async createMany(reqs: CreateAccountParams[]): Promise { + const methodStartedAt = Date.now(); + let createdCount = 0; + let resultCount = 0; + let succeeded = false; + + logPerformanceDebug(this.#logger, 'AccountUseCases.createMany started', { + requestCount: reqs.length, + }); + if (reqs.length === 0) { + succeeded = true; + logPerformanceDebug(this.#logger, 'AccountUseCases.createMany finished', { + createdCount, + durationMs: getElapsedTimeMs(methodStartedAt), + requestCount: reqs.length, + resultCount, + succeeded, + }); return []; } - const { accounts, createdAccountKeys } = await this.#runAccountMutation( - async () => { - const entries = reqs.map((req, index) => { - const derivationPath = getAccountDerivationPath(req); - return { - req, - index, - derivationPath, - pathKey: getDerivationPathKey(derivationPath), - }; - }); - - const uniqueEntriesByPath = new Map(); - for (const entry of entries) { - if (!uniqueEntriesByPath.has(entry.pathKey)) { - uniqueEntriesByPath.set(entry.pathKey, entry); + try { + const { accounts, createdAccountKeys } = await this.#runAccountMutation( + async () => { + const mutationStartedAt = Date.now(); + const entries = reqs.map((req, index) => { + const derivationPath = getAccountDerivationPath(req); + return { + req, + index, + derivationPath, + pathKey: getDerivationPathKey(derivationPath), + }; + }); + + const uniqueEntriesByPath = new Map< + string, + (typeof entries)[number] + >(); + for (const entry of entries) { + if (!uniqueEntriesByPath.has(entry.pathKey)) { + uniqueEntriesByPath.set(entry.pathKey, entry); + } } - } - const uniqueEntries = [...uniqueEntriesByPath.values()]; + const uniqueEntries = [...uniqueEntriesByPath.values()]; - const existingAccounts = await this.#repository.getByDerivationPaths( - uniqueEntries.map(({ derivationPath }) => derivationPath), - ); - const existingAccountsByPath = new Map(); + logPerformanceDebug( + this.#logger, + 'AccountUseCases.createMany dedupe completed', + { + duplicateCount: entries.length - uniqueEntries.length, + durationMs: getElapsedTimeMs(mutationStartedAt), + requestCount: entries.length, + uniqueCount: uniqueEntries.length, + }, + ); - uniqueEntries.forEach((entry, index) => { - const account = existingAccounts[index]; - if (account && account.network === entry.req.network) { - existingAccountsByPath.set(entry.pathKey, account); - } - }); + const lookupStartedAt = Date.now(); + const existingAccounts = await this.#repository.getByDerivationPaths( + uniqueEntries.map(({ derivationPath }) => derivationPath), + ); + logPerformanceDebug( + this.#logger, + 'AccountUseCases.createMany repository lookup completed', + { + durationMs: getElapsedTimeMs(lookupStartedAt), + requestedPathCount: uniqueEntries.length, + }, + ); + const existingAccountsByPath = new Map(); - const entriesToCreate = uniqueEntries.filter( - ({ pathKey }) => !existingAccountsByPath.has(pathKey), - ); - const newAccounts = await runWithConcurrencyLimit( - entriesToCreate, - CREATE_ACCOUNTS_CONCURRENCY, - async ({ derivationPath, req }) => { - const newAccount = await this.#repository.create( - derivationPath, - req.network, - req.addressType, + uniqueEntries.forEach((entry, index) => { + const account = existingAccounts[index]; + if (account && account.network === entry.req.network) { + existingAccountsByPath.set(entry.pathKey, account); + } + }); + + const entriesToCreate = uniqueEntries.filter( + ({ pathKey }) => !existingAccountsByPath.has(pathKey), + ); + const createStartedAt = Date.now(); + const newAccounts = await runWithConcurrencyLimit( + entriesToCreate, + CREATE_ACCOUNTS_CONCURRENCY, + async ({ derivationPath, req }) => { + const newAccount = await this.#repository.create( + derivationPath, + req.network, + req.addressType, + ); + newAccount.revealNextAddress(); + return newAccount; + }, + ); + logPerformanceDebug( + this.#logger, + 'AccountUseCases.createMany missing accounts created', + { + createdCount: newAccounts.length, + durationMs: getElapsedTimeMs(createStartedAt), + existingCount: existingAccountsByPath.size, + requestedPathCount: uniqueEntries.length, + }, + ); + + if (newAccounts.length > 0) { + const insertStartedAt = Date.now(); + await this.#repository.insertMany(newAccounts); + logPerformanceDebug( + this.#logger, + 'AccountUseCases.createMany insertMany completed', + { + durationMs: getElapsedTimeMs(insertStartedAt), + insertedCount: newAccounts.length, + }, ); - newAccount.revealNextAddress(); - return newAccount; - }, - ); + } - if (newAccounts.length > 0) { - await this.#repository.insertMany(newAccounts); - } + const newAccountsByPath = new Map( + entriesToCreate.map((entry, index) => [ + entry.pathKey, + newAccounts[index] as BitcoinAccount, + ]), + ); - const newAccountsByPath = new Map( - entriesToCreate.map((entry, index) => [ - entry.pathKey, - newAccounts[index] as BitcoinAccount, - ]), - ); + const accountsInOrder = entries.map((entry) => { + const account = + existingAccountsByPath.get(entry.pathKey) ?? + newAccountsByPath.get(entry.pathKey); - const accountsInOrder = entries.map((entry) => { - const account = - existingAccountsByPath.get(entry.pathKey) ?? - newAccountsByPath.get(entry.pathKey); + if (!account) { + throw new AssertionError('Failed to create account', { + index: entry.index, + derivationPath: entry.derivationPath, + }); + } - if (!account) { - throw new AssertionError('Failed to create account', { - index: entry.index, - derivationPath: entry.derivationPath, - }); - } + return account; + }); - return account; - }); + return { + accounts: accountsInOrder, + createdAccountKeys: new Set(newAccountsByPath.keys()), + }; + }, + ); - return { - accounts: accountsInOrder, - createdAccountKeys: new Set(newAccountsByPath.keys()), - }; - }, - ); + createdCount = createdAccountKeys.size; + resultCount = accounts.length; - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - await this.#snapClient.emitAccountCreatedEvent( - account, - req.correlationId, - req.accountName, + const eventsStartedAt = Date.now(); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + await this.#snapClient.emitAccountCreatedEvent( + account, + req.correlationId, + req.accountName, + ); + } + logPerformanceDebug( + this.#logger, + 'AccountUseCases.createMany account-created events completed', + { + durationMs: getElapsedTimeMs(eventsStartedAt), + eventCount: accounts.length, + }, ); - } - const scheduledAccountIds = new Set(); - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); - if ( - req.synchronize && - createdAccountKeys.has(pathKey) && - !scheduledAccountIds.has(account.id) - ) { - scheduledAccountIds.add(account.id); - await this.#snapClient.scheduleBackgroundEvent({ - duration: 'PT1S', - method: CronMethod.FullScanAccount, - params: { accountId: account.id }, - }); + const scheduleStartedAt = Date.now(); + const scheduledAccountIds = new Set(); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); + if ( + req.synchronize && + createdAccountKeys.has(pathKey) && + !scheduledAccountIds.has(account.id) + ) { + scheduledAccountIds.add(account.id); + await this.#snapClient.scheduleBackgroundEvent({ + duration: 'PT1S', + method: CronMethod.FullScanAccount, + params: { accountId: account.id }, + }); + } } - } + logPerformanceDebug( + this.#logger, + 'AccountUseCases.createMany background scheduling completed', + { + durationMs: getElapsedTimeMs(scheduleStartedAt), + scheduledCount: scheduledAccountIds.size, + }, + ); - return accounts; + succeeded = true; + return accounts; + } finally { + logPerformanceDebug(this.#logger, 'AccountUseCases.createMany finished', { + createdCount, + durationMs: getElapsedTimeMs(methodStartedAt), + requestCount: reqs.length, + resultCount, + succeeded, + }); + } } async synchronize( diff --git a/packages/snap/src/utils/performance.ts b/packages/snap/src/utils/performance.ts new file mode 100644 index 000000000..1672548b9 --- /dev/null +++ b/packages/snap/src/utils/performance.ts @@ -0,0 +1,30 @@ +export const PERFORMANCE_DEBUG_LOG_PREFIX = + '[PERFORMANCE DEBUG - BITCOIN SNAP]'; + +/** + * @param startedAt - Start timestamp from `Date.now()`. + * @returns Elapsed milliseconds. + */ +export function getElapsedTimeMs(startedAt: number): number { + return Date.now() - startedAt; +} + +/** + * Log a performance debug message using a common prefix. + * + * @param _logger - Ignored logger argument kept so existing call sites stay simple. + * @param event - Event name. + * @param data - Optional event metadata. + */ +export function logPerformanceDebug( + _logger: unknown, + event: string, + data?: Record, +): void { + if (data) { + console.log(`${PERFORMANCE_DEBUG_LOG_PREFIX} ${event}`, data); + return; + } + + console.log(`${PERFORMANCE_DEBUG_LOG_PREFIX} ${event}`); +} From 37ad40245f4671a272a9ce9f3655054885000b77 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 6 May 2026 16:33:05 -0400 Subject: [PATCH 18/26] Revert "chore: add debug logs" This reverts commit 487302f14cc0cf9b67a23ab0132d6349fb5b0230. --- packages/snap/src/handlers/KeyringHandler.ts | 34 -- packages/snap/src/index.ts | 2 +- .../snap/src/store/BdkAccountRepository.ts | 416 +++++------------- .../snap/src/use-cases/AccountUseCases.ts | 282 ++++-------- packages/snap/src/utils/performance.ts | 30 -- 5 files changed, 207 insertions(+), 557 deletions(-) delete mode 100644 packages/snap/src/utils/performance.ts diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index ad36823ba..fb85d1fa1 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -71,7 +71,6 @@ import { } from './mappings'; import { BtcWalletRequestStruct, validateSelectedAccounts } from './validation'; import type { AccountUseCases } from '../use-cases/AccountUseCases'; -import { getElapsedTimeMs, logPerformanceDebug } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export const CreateAccountRequest = object({ @@ -302,10 +301,6 @@ export class KeyringHandler implements Keyring { async createAccounts( options: CreateAccountOptions, ): Promise { - const methodStartedAt = Date.now(); - let resultCount = 0; - let succeeded = false; - assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, @@ -342,13 +337,6 @@ export class KeyringHandler implements Keyring { ); } - logPerformanceDebug(this.#logger, 'KeyringHandler.createAccounts started', { - batchSize, - rangeFrom: range.from, - rangeTo: range.to, - type: options.type, - }); - // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring // the defaults used by `createAccount` when no scope is provided. const network = scopeToNetwork[BtcScope.Mainnet]; @@ -379,7 +367,6 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. - const createManyStartedAt = Date.now(); const accounts = await this.#accountsUseCases.createMany( indices.map((index) => ({ network, @@ -389,17 +376,6 @@ export class KeyringHandler implements Keyring { synchronize: false, })), ); - resultCount = accounts.length; - logPerformanceDebug( - this.#logger, - 'KeyringHandler.createAccounts createMany completed', - { - batchSize, - durationMs: getElapsedTimeMs(createManyStartedAt), - resultCount, - }, - ); - succeeded = true; return accounts.map(mapToKeyringAccount); } finally { if (traceStarted) { @@ -409,16 +385,6 @@ export class KeyringHandler implements Keyring { 'endTrace', ); } - logPerformanceDebug( - this.#logger, - 'KeyringHandler.createAccounts finished', - { - batchSize, - durationMs: getElapsedTimeMs(methodStartedAt), - resultCount, - succeeded, - }, - ); } } diff --git a/packages/snap/src/index.ts b/packages/snap/src/index.ts index ebb0706ca..2d8a6968c 100644 --- a/packages/snap/src/index.ts +++ b/packages/snap/src/index.ts @@ -46,7 +46,7 @@ const translator = new LocalTranslatorAdapter(); const middleware = new HandlerMiddleware(logger, snapClient, translator); // Data layer -const accountRepository = new BdkAccountRepository(snapClient, logger); +const accountRepository = new BdkAccountRepository(snapClient); const sendFlowRepository = new JSXSendFlowRepository(snapClient, translator); const confirmationRepository = new JSXConfirmationRepository( snapClient, diff --git a/packages/snap/src/store/BdkAccountRepository.ts b/packages/snap/src/store/BdkAccountRepository.ts index b89f15f47..e03dc8ea1 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -17,11 +17,9 @@ import { type Inscription, type AccountState, type SnapState, - type Logger, StorageError, } from '../entities'; import { BdkAccountAdapter } from '../infra'; -import { getElapsedTimeMs, logPerformanceDebug } from '../utils/performance'; /** * Encode a fingerprint to a 4-bytes hex-string (required by the BDK). @@ -47,11 +45,8 @@ function getDerivationPathKey(derivationPath: string[]): string { export class BdkAccountRepository implements BitcoinAccountRepository { readonly #snapClient: SnapClient; - readonly #logger: Logger | undefined; - - constructor(snapClient: SnapClient, logger?: Logger) { + constructor(snapClient: SnapClient) { this.#snapClient = snapClient; - this.#logger = logger; } async get(id: string): Promise { @@ -94,124 +89,60 @@ export class BdkAccountRepository implements BitcoinAccountRepository { async getByDerivationPaths( derivationPaths: string[][], ): Promise<(BitcoinAccount | null)[]> { - const methodStartedAt = Date.now(); - let repairedIndexCount = 0; - let resultCount = 0; - let succeeded = false; - - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.getByDerivationPaths started', - { - requestedPathCount: derivationPaths.length, - }, - ); - if (derivationPaths.length === 0) { - succeeded = true; - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.getByDerivationPaths finished', - { - durationMs: getElapsedTimeMs(methodStartedAt), - repairedIndexCount, - requestedPathCount: derivationPaths.length, - resultCount, - succeeded, - }, - ); return []; } - try { - const stateReadStartedAt = Date.now(); - const [derivationPathIndex, accounts] = await Promise.all([ - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - ]); - - const accountsById = accounts ?? {}; - const existingDerivationPathIndex = derivationPathIndex ?? {}; - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.getByDerivationPaths state read completed', - { - accountCount: Object.keys(accountsById).length, - durationMs: getElapsedTimeMs(stateReadStartedAt), - indexedPathCount: Object.keys(existingDerivationPathIndex).length, - }, - ); - const accountsByDerivationPath = new Map< - string, - [string, AccountState] - >(); - - for (const [id, account] of Object.entries(accountsById)) { - if (account) { - accountsByDerivationPath.set( - getDerivationPathKey(account.derivationPath), - [id, account], - ); - } - } + const [derivationPathIndex, accounts] = await Promise.all([ + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + ]); - const repairs: Record = {}; - const results = derivationPaths.map((derivationPath) => { - const pathKey = getDerivationPathKey(derivationPath); - const indexedId = existingDerivationPathIndex[pathKey]; - const indexedAccount = indexedId ? accountsById[indexedId] : null; + const accountsById = accounts ?? {}; + const existingDerivationPathIndex = derivationPathIndex ?? {}; + const accountsByDerivationPath = new Map(); - if (indexedId && indexedAccount) { - return this.#loadAccount(indexedId, indexedAccount); - } + for (const [id, account] of Object.entries(accountsById)) { + if (account) { + accountsByDerivationPath.set( + getDerivationPathKey(account.derivationPath), + [id, account], + ); + } + } - const fallback = accountsByDerivationPath.get(pathKey); - if (!fallback) { - return null; - } + const repairs: Record = {}; + const results = derivationPaths.map((derivationPath) => { + const pathKey = getDerivationPathKey(derivationPath); + const indexedId = existingDerivationPathIndex[pathKey]; + const indexedAccount = indexedId ? accountsById[indexedId] : null; - const [id, account] = fallback; - repairs[pathKey] = id; - return this.#loadAccount(id, account); - }); + if (indexedId && indexedAccount) { + return this.#loadAccount(indexedId, indexedAccount); + } - if (Object.keys(repairs).length > 0) { - const repairStartedAt = Date.now(); - await this.#snapClient.setState('derivationPaths', { - ...existingDerivationPathIndex, - ...repairs, - }); - repairedIndexCount = Object.keys(repairs).length; - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.getByDerivationPaths index repair completed', - { - durationMs: getElapsedTimeMs(repairStartedAt), - repairedIndexCount, - }, - ); + const fallback = accountsByDerivationPath.get(pathKey); + if (!fallback) { + return null; } - resultCount = results.filter(Boolean).length; - succeeded = true; - return results; - } finally { - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.getByDerivationPaths finished', - { - durationMs: getElapsedTimeMs(methodStartedAt), - repairedIndexCount, - requestedPathCount: derivationPaths.length, - resultCount, - succeeded, - }, - ); + const [id, account] = fallback; + repairs[pathKey] = id; + return this.#loadAccount(id, account); + }); + + if (Object.keys(repairs).length > 0) { + await this.#snapClient.setState('derivationPaths', { + ...existingDerivationPathIndex, + ...repairs, + }); } + + return results; } async getWithSigner(id: string): Promise { @@ -255,224 +186,101 @@ export class BdkAccountRepository implements BitcoinAccountRepository { network: Network, addressType: AddressType, ): Promise { - const methodStartedAt = Date.now(); - let succeeded = false; - - try { - const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); - const id = v4(); - const fingerprint = toBdkFingerprint( - slip10.masterFingerprint ?? slip10.parentFingerprint, - ); + const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); + const id = v4(); + const fingerprint = toBdkFingerprint( + slip10.masterFingerprint ?? slip10.parentFingerprint, + ); - const xpub = slip10_to_extended(slip10, network); - const descriptors = xpub_to_descriptor( - xpub, - fingerprint, - network, - addressType, - ); + const xpub = slip10_to_extended(slip10, network); + const descriptors = xpub_to_descriptor( + xpub, + fingerprint, + network, + addressType, + ); - const account = BdkAccountAdapter.create( - id, - derivationPath, - descriptors, - network, - ); - succeeded = true; - return account; - } finally { - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.create finished', - { - addressType, - durationMs: getElapsedTimeMs(methodStartedAt), - network, - succeeded, - }, - ); - } + return BdkAccountAdapter.create(id, derivationPath, descriptors, network); } async insert(account: BitcoinAccount): Promise { - const methodStartedAt = Date.now(); - let succeeded = false; const { id, derivationPath } = account; - try { - const walletData = account.takeStaged(); - if (!walletData) { - throw new StorageError( - `Missing changeset data for account "${id}" for insertion.`, - ); - } - - await Promise.all([ - this.#snapClient.setState( - `derivationPaths.${getDerivationPathKey(derivationPath)}`, - id, - ), - this.#snapClient.setState(`accounts.${id}`, { - wallet: walletData.to_json(), - inscriptions: [], - derivationPath, - }), - ]); - - succeeded = true; - return account; - } finally { - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insert finished', - { - accountId: id, - durationMs: getElapsedTimeMs(methodStartedAt), - succeeded, - }, + const walletData = account.takeStaged(); + if (!walletData) { + throw new StorageError( + `Missing changeset data for account "${id}" for insertion.`, ); } + + await Promise.all([ + this.#snapClient.setState( + `derivationPaths.${getDerivationPathKey(derivationPath)}`, + id, + ), + this.#snapClient.setState(`accounts.${id}`, { + wallet: walletData.to_json(), + inscriptions: [], + derivationPath, + }), + ]); + + return account; } async insertMany(accounts: BitcoinAccount[]): Promise { - const methodStartedAt = Date.now(); - let succeeded = false; - - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany started', - { - accountCount: accounts.length, - }, - ); - if (accounts.length === 0) { - succeeded = true; - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany finished', - { - accountCount: accounts.length, - durationMs: getElapsedTimeMs(methodStartedAt), - succeeded, - }, - ); return []; } if (accounts.length === 1) { - try { - const result = [await this.insert(accounts[0] as BitcoinAccount)]; - succeeded = true; - return result; - } finally { - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany finished', - { - accountCount: accounts.length, - durationMs: getElapsedTimeMs(methodStartedAt), - succeeded, - }, - ); - } + return [await this.insert(accounts[0] as BitcoinAccount)]; } - try { - const prepareStartedAt = Date.now(); - const accountStateEntries: [string, AccountState][] = []; - const derivationPathEntries: [string, string][] = []; - - for (const account of accounts) { - const { id, derivationPath } = account; - const walletData = account.takeStaged(); - - if (!walletData) { - throw new StorageError( - `Missing changeset data for account "${id}" for insertion.`, - ); - } - - accountStateEntries.push([ - id, - { - wallet: walletData.to_json(), - inscriptions: [], - derivationPath, - }, - ]); - derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); + const accountStateEntries: [string, AccountState][] = []; + const derivationPathEntries: [string, string][] = []; + + for (const account of accounts) { + const { id, derivationPath } = account; + const walletData = account.takeStaged(); + + if (!walletData) { + throw new StorageError( + `Missing changeset data for account "${id}" for insertion.`, + ); } - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany account state prepared', - { - accountCount: accounts.length, - durationMs: getElapsedTimeMs(prepareStartedAt), - }, - ); - const stateReadStartedAt = Date.now(); - const [existingAccounts, existingDerivationPaths] = await Promise.all([ - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - ]); - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany existing state read completed', + accountStateEntries.push([ + id, { - durationMs: getElapsedTimeMs(stateReadStartedAt), - existingAccountCount: Object.keys(existingAccounts ?? {}).length, - existingPathCount: Object.keys(existingDerivationPaths ?? {}).length, + wallet: walletData.to_json(), + inscriptions: [], + derivationPath, }, - ); + ]); + derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); + } - const accountsWriteStartedAt = Date.now(); - await this.#snapClient.setState('accounts', { - ...(existingAccounts ?? {}), - ...Object.fromEntries(accountStateEntries), - }); - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany accounts state write completed', - { - accountCount: accountStateEntries.length, - durationMs: getElapsedTimeMs(accountsWriteStartedAt), - }, - ); + const [existingAccounts, existingDerivationPaths] = await Promise.all([ + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + ]); - const derivationPathsWriteStartedAt = Date.now(); - await this.#snapClient.setState('derivationPaths', { - ...(existingDerivationPaths ?? {}), - ...Object.fromEntries(derivationPathEntries), - }); - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany derivation path state write completed', - { - durationMs: getElapsedTimeMs(derivationPathsWriteStartedAt), - pathCount: derivationPathEntries.length, - }, - ); + await this.#snapClient.setState('accounts', { + ...(existingAccounts ?? {}), + ...Object.fromEntries(accountStateEntries), + }); - succeeded = true; - return accounts; - } finally { - logPerformanceDebug( - this.#logger, - 'BdkAccountRepository.insertMany finished', - { - accountCount: accounts.length, - durationMs: getElapsedTimeMs(methodStartedAt), - succeeded, - }, - ); - } + await this.#snapClient.setState('derivationPaths', { + ...(existingDerivationPaths ?? {}), + ...Object.fromEntries(derivationPathEntries), + }); + + return accounts; } async update( diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 17a79e947..000edb27b 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -33,7 +33,6 @@ import { WalletError, } from '../entities'; import { CronMethod } from '../handlers/CronHandler'; -import { getElapsedTimeMs, logPerformanceDebug } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export type DiscoverAccountParams = { @@ -273,213 +272,120 @@ export class AccountUseCases { } async createMany(reqs: CreateAccountParams[]): Promise { - const methodStartedAt = Date.now(); - let createdCount = 0; - let resultCount = 0; - let succeeded = false; - - logPerformanceDebug(this.#logger, 'AccountUseCases.createMany started', { - requestCount: reqs.length, - }); - if (reqs.length === 0) { - succeeded = true; - logPerformanceDebug(this.#logger, 'AccountUseCases.createMany finished', { - createdCount, - durationMs: getElapsedTimeMs(methodStartedAt), - requestCount: reqs.length, - resultCount, - succeeded, - }); return []; } - try { - const { accounts, createdAccountKeys } = await this.#runAccountMutation( - async () => { - const mutationStartedAt = Date.now(); - const entries = reqs.map((req, index) => { - const derivationPath = getAccountDerivationPath(req); - return { - req, - index, - derivationPath, - pathKey: getDerivationPathKey(derivationPath), - }; - }); - - const uniqueEntriesByPath = new Map< - string, - (typeof entries)[number] - >(); - for (const entry of entries) { - if (!uniqueEntriesByPath.has(entry.pathKey)) { - uniqueEntriesByPath.set(entry.pathKey, entry); - } - } - const uniqueEntries = [...uniqueEntriesByPath.values()]; - - logPerformanceDebug( - this.#logger, - 'AccountUseCases.createMany dedupe completed', - { - duplicateCount: entries.length - uniqueEntries.length, - durationMs: getElapsedTimeMs(mutationStartedAt), - requestCount: entries.length, - uniqueCount: uniqueEntries.length, - }, - ); + const { accounts, createdAccountKeys } = await this.#runAccountMutation( + async () => { + const entries = reqs.map((req, index) => { + const derivationPath = getAccountDerivationPath(req); + return { + req, + index, + derivationPath, + pathKey: getDerivationPathKey(derivationPath), + }; + }); - const lookupStartedAt = Date.now(); - const existingAccounts = await this.#repository.getByDerivationPaths( - uniqueEntries.map(({ derivationPath }) => derivationPath), - ); - logPerformanceDebug( - this.#logger, - 'AccountUseCases.createMany repository lookup completed', - { - durationMs: getElapsedTimeMs(lookupStartedAt), - requestedPathCount: uniqueEntries.length, - }, - ); - const existingAccountsByPath = new Map(); + const uniqueEntriesByPath = new Map(); + for (const entry of entries) { + if (!uniqueEntriesByPath.has(entry.pathKey)) { + uniqueEntriesByPath.set(entry.pathKey, entry); + } + } + const uniqueEntries = [...uniqueEntriesByPath.values()]; - uniqueEntries.forEach((entry, index) => { - const account = existingAccounts[index]; - if (account && account.network === entry.req.network) { - existingAccountsByPath.set(entry.pathKey, account); - } - }); + const existingAccounts = await this.#repository.getByDerivationPaths( + uniqueEntries.map(({ derivationPath }) => derivationPath), + ); + const existingAccountsByPath = new Map(); - const entriesToCreate = uniqueEntries.filter( - ({ pathKey }) => !existingAccountsByPath.has(pathKey), - ); - const createStartedAt = Date.now(); - const newAccounts = await runWithConcurrencyLimit( - entriesToCreate, - CREATE_ACCOUNTS_CONCURRENCY, - async ({ derivationPath, req }) => { - const newAccount = await this.#repository.create( - derivationPath, - req.network, - req.addressType, - ); - newAccount.revealNextAddress(); - return newAccount; - }, - ); - logPerformanceDebug( - this.#logger, - 'AccountUseCases.createMany missing accounts created', - { - createdCount: newAccounts.length, - durationMs: getElapsedTimeMs(createStartedAt), - existingCount: existingAccountsByPath.size, - requestedPathCount: uniqueEntries.length, - }, - ); + uniqueEntries.forEach((entry, index) => { + const account = existingAccounts[index]; + if (account && account.network === entry.req.network) { + existingAccountsByPath.set(entry.pathKey, account); + } + }); - if (newAccounts.length > 0) { - const insertStartedAt = Date.now(); - await this.#repository.insertMany(newAccounts); - logPerformanceDebug( - this.#logger, - 'AccountUseCases.createMany insertMany completed', - { - durationMs: getElapsedTimeMs(insertStartedAt), - insertedCount: newAccounts.length, - }, + const entriesToCreate = uniqueEntries.filter( + ({ pathKey }) => !existingAccountsByPath.has(pathKey), + ); + const newAccounts = await runWithConcurrencyLimit( + entriesToCreate, + CREATE_ACCOUNTS_CONCURRENCY, + async ({ derivationPath, req }) => { + const newAccount = await this.#repository.create( + derivationPath, + req.network, + req.addressType, ); - } + newAccount.revealNextAddress(); + return newAccount; + }, + ); - const newAccountsByPath = new Map( - entriesToCreate.map((entry, index) => [ - entry.pathKey, - newAccounts[index] as BitcoinAccount, - ]), - ); + if (newAccounts.length > 0) { + await this.#repository.insertMany(newAccounts); + } - const accountsInOrder = entries.map((entry) => { - const account = - existingAccountsByPath.get(entry.pathKey) ?? - newAccountsByPath.get(entry.pathKey); + const newAccountsByPath = new Map( + entriesToCreate.map((entry, index) => [ + entry.pathKey, + newAccounts[index] as BitcoinAccount, + ]), + ); - if (!account) { - throw new AssertionError('Failed to create account', { - index: entry.index, - derivationPath: entry.derivationPath, - }); - } + const accountsInOrder = entries.map((entry) => { + const account = + existingAccountsByPath.get(entry.pathKey) ?? + newAccountsByPath.get(entry.pathKey); - return account; - }); + if (!account) { + throw new AssertionError('Failed to create account', { + index: entry.index, + derivationPath: entry.derivationPath, + }); + } - return { - accounts: accountsInOrder, - createdAccountKeys: new Set(newAccountsByPath.keys()), - }; - }, - ); + return account; + }); - createdCount = createdAccountKeys.size; - resultCount = accounts.length; + return { + accounts: accountsInOrder, + createdAccountKeys: new Set(newAccountsByPath.keys()), + }; + }, + ); - const eventsStartedAt = Date.now(); - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - await this.#snapClient.emitAccountCreatedEvent( - account, - req.correlationId, - req.accountName, - ); - } - logPerformanceDebug( - this.#logger, - 'AccountUseCases.createMany account-created events completed', - { - durationMs: getElapsedTimeMs(eventsStartedAt), - eventCount: accounts.length, - }, + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + await this.#snapClient.emitAccountCreatedEvent( + account, + req.correlationId, + req.accountName, ); + } - const scheduleStartedAt = Date.now(); - const scheduledAccountIds = new Set(); - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); - if ( - req.synchronize && - createdAccountKeys.has(pathKey) && - !scheduledAccountIds.has(account.id) - ) { - scheduledAccountIds.add(account.id); - await this.#snapClient.scheduleBackgroundEvent({ - duration: 'PT1S', - method: CronMethod.FullScanAccount, - params: { accountId: account.id }, - }); - } + const scheduledAccountIds = new Set(); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); + if ( + req.synchronize && + createdAccountKeys.has(pathKey) && + !scheduledAccountIds.has(account.id) + ) { + scheduledAccountIds.add(account.id); + await this.#snapClient.scheduleBackgroundEvent({ + duration: 'PT1S', + method: CronMethod.FullScanAccount, + params: { accountId: account.id }, + }); } - logPerformanceDebug( - this.#logger, - 'AccountUseCases.createMany background scheduling completed', - { - durationMs: getElapsedTimeMs(scheduleStartedAt), - scheduledCount: scheduledAccountIds.size, - }, - ); - - succeeded = true; - return accounts; - } finally { - logPerformanceDebug(this.#logger, 'AccountUseCases.createMany finished', { - createdCount, - durationMs: getElapsedTimeMs(methodStartedAt), - requestCount: reqs.length, - resultCount, - succeeded, - }); } + + return accounts; } async synchronize( diff --git a/packages/snap/src/utils/performance.ts b/packages/snap/src/utils/performance.ts deleted file mode 100644 index 1672548b9..000000000 --- a/packages/snap/src/utils/performance.ts +++ /dev/null @@ -1,30 +0,0 @@ -export const PERFORMANCE_DEBUG_LOG_PREFIX = - '[PERFORMANCE DEBUG - BITCOIN SNAP]'; - -/** - * @param startedAt - Start timestamp from `Date.now()`. - * @returns Elapsed milliseconds. - */ -export function getElapsedTimeMs(startedAt: number): number { - return Date.now() - startedAt; -} - -/** - * Log a performance debug message using a common prefix. - * - * @param _logger - Ignored logger argument kept so existing call sites stay simple. - * @param event - Event name. - * @param data - Optional event metadata. - */ -export function logPerformanceDebug( - _logger: unknown, - event: string, - data?: Record, -): void { - if (data) { - console.log(`${PERFORMANCE_DEBUG_LOG_PREFIX} ${event}`, data); - return; - } - - console.log(`${PERFORMANCE_DEBUG_LOG_PREFIX} ${event}`); -} From d66fa0d7ef443f00408bffd2fdc8a299e68356d5 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 6 May 2026 16:50:28 -0400 Subject: [PATCH 19/26] add performance log --- packages/snap/src/handlers/KeyringHandler.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index fb85d1fa1..89e5c623a 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -367,6 +367,7 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. + const start = performance.now(); const accounts = await this.#accountsUseCases.createMany( indices.map((index) => ({ network, @@ -376,7 +377,12 @@ export class KeyringHandler implements Keyring { synchronize: false, })), ); - return accounts.map(mapToKeyringAccount); + const result = accounts.map(mapToKeyringAccount); + const end = performance.now(); + console.log( + `[PERFORMANCE DEBUG - BITCOIN SNAP] createAccounts took ${end - start}ms`, + ); + return result; } finally { if (traceStarted) { await runSnapActionSafely( From 811268aa972bf03bb4704a88f306d57af4be07c8 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 6 May 2026 17:19:06 -0400 Subject: [PATCH 20/26] fix log --- packages/snap/src/handlers/KeyringHandler.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 89e5c623a..193055d08 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -367,7 +367,7 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. - const start = performance.now(); + const start = Date.now(); const accounts = await this.#accountsUseCases.createMany( indices.map((index) => ({ network, @@ -378,9 +378,9 @@ export class KeyringHandler implements Keyring { })), ); const result = accounts.map(mapToKeyringAccount); - const end = performance.now(); + const end = Date.now(); console.log( - `[PERFORMANCE DEBUG - BITCOIN SNAP] createAccounts took ${end - start}ms`, + `[PERFORMANCE DEBUG - BITCOIN SNAP] createAccounts took ${end - start} ms`, ); return result; } finally { From e8b9ff2bfb3e7d51b27f2ad6a7ade064596711f8 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Wed, 6 May 2026 17:49:14 -0400 Subject: [PATCH 21/26] improve logs --- packages/snap/src/handlers/KeyringHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 193055d08..48b161ccc 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -301,6 +301,7 @@ export class KeyringHandler implements Keyring { async createAccounts( options: CreateAccountOptions, ): Promise { + const start = Date.now(); assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, @@ -367,7 +368,6 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. - const start = Date.now(); const accounts = await this.#accountsUseCases.createMany( indices.map((index) => ({ network, @@ -380,7 +380,7 @@ export class KeyringHandler implements Keyring { const result = accounts.map(mapToKeyringAccount); const end = Date.now(); console.log( - `[PERFORMANCE DEBUG - BITCOIN SNAP] createAccounts took ${end - start} ms`, + `[PERFORMANCE DEBUG - BITCOIN SNAP] createAccounts took ${end - start} ms to create ${accounts.length} accounts`, ); return result; } finally { From 063ba19547fc1c2988fe912ee0b12f3aaaa10c0f Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 7 May 2026 09:21:19 -0400 Subject: [PATCH 22/26] add performance logs --- packages/snap/src/handlers/KeyringHandler.ts | 12 +- .../snap/src/store/BdkAccountRepository.ts | 308 +++++++++++------- .../snap/src/use-cases/AccountUseCases.ts | 240 ++++++++------ packages/snap/src/utils/performance.ts | 8 + 4 files changed, 357 insertions(+), 211 deletions(-) create mode 100644 packages/snap/src/utils/performance.ts diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 48b161ccc..c6c8c3b42 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -71,6 +71,7 @@ import { } from './mappings'; import { BtcWalletRequestStruct, validateSelectedAccounts } from './validation'; import type { AccountUseCases } from '../use-cases/AccountUseCases'; +import { logExecutionTime } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export const CreateAccountRequest = object({ @@ -368,6 +369,7 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. + const createManyStart = Date.now(); const accounts = await this.#accountsUseCases.createMany( indices.map((index) => ({ network, @@ -377,10 +379,13 @@ export class KeyringHandler implements Keyring { synchronize: false, })), ); + logExecutionTime('keyring_createAccounts createMany', createManyStart); + + const responseMappingStart = Date.now(); const result = accounts.map(mapToKeyringAccount); - const end = Date.now(); - console.log( - `[PERFORMANCE DEBUG - BITCOIN SNAP] createAccounts took ${end - start} ms to create ${accounts.length} accounts`, + logExecutionTime( + 'keyring_createAccounts response mapping', + responseMappingStart, ); return result; } finally { @@ -391,6 +396,7 @@ export class KeyringHandler implements Keyring { 'endTrace', ); } + logExecutionTime('keyring_createAccounts', start); } } diff --git a/packages/snap/src/store/BdkAccountRepository.ts b/packages/snap/src/store/BdkAccountRepository.ts index e03dc8ea1..0420feea9 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -20,6 +20,7 @@ import { StorageError, } from '../entities'; import { BdkAccountAdapter } from '../infra'; +import { logExecutionTime } from '../utils/performance'; /** * Encode a fingerprint to a 4-bytes hex-string (required by the BDK). @@ -89,60 +90,80 @@ export class BdkAccountRepository implements BitcoinAccountRepository { async getByDerivationPaths( derivationPaths: string[][], ): Promise<(BitcoinAccount | null)[]> { + const start = Date.now(); + if (derivationPaths.length === 0) { + logExecutionTime('BdkAccountRepository.getByDerivationPaths', start); return []; } - const [derivationPathIndex, accounts] = await Promise.all([ - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - ]); - - const accountsById = accounts ?? {}; - const existingDerivationPathIndex = derivationPathIndex ?? {}; - const accountsByDerivationPath = new Map(); + try { + const loadStateStart = Date.now(); + const [derivationPathIndex, accounts] = await Promise.all([ + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + ]); + logExecutionTime( + 'BdkAccountRepository.getByDerivationPaths load state', + loadStateStart, + ); - for (const [id, account] of Object.entries(accountsById)) { - if (account) { - accountsByDerivationPath.set( - getDerivationPathKey(account.derivationPath), - [id, account], - ); + const accountsById = accounts ?? {}; + const existingDerivationPathIndex = derivationPathIndex ?? {}; + const accountsByDerivationPath = new Map< + string, + [string, AccountState] + >(); + + for (const [id, account] of Object.entries(accountsById)) { + if (account) { + accountsByDerivationPath.set( + getDerivationPathKey(account.derivationPath), + [id, account], + ); + } } - } - const repairs: Record = {}; - const results = derivationPaths.map((derivationPath) => { - const pathKey = getDerivationPathKey(derivationPath); - const indexedId = existingDerivationPathIndex[pathKey]; - const indexedAccount = indexedId ? accountsById[indexedId] : null; - - if (indexedId && indexedAccount) { - return this.#loadAccount(indexedId, indexedAccount); - } + const repairs: Record = {}; + const results = derivationPaths.map((derivationPath) => { + const pathKey = getDerivationPathKey(derivationPath); + const indexedId = existingDerivationPathIndex[pathKey]; + const indexedAccount = indexedId ? accountsById[indexedId] : null; - const fallback = accountsByDerivationPath.get(pathKey); - if (!fallback) { - return null; - } + if (indexedId && indexedAccount) { + return this.#loadAccount(indexedId, indexedAccount); + } - const [id, account] = fallback; - repairs[pathKey] = id; - return this.#loadAccount(id, account); - }); + const fallback = accountsByDerivationPath.get(pathKey); + if (!fallback) { + return null; + } - if (Object.keys(repairs).length > 0) { - await this.#snapClient.setState('derivationPaths', { - ...existingDerivationPathIndex, - ...repairs, + const [id, account] = fallback; + repairs[pathKey] = id; + return this.#loadAccount(id, account); }); - } - return results; + if (Object.keys(repairs).length > 0) { + const repairIndexStart = Date.now(); + await this.#snapClient.setState('derivationPaths', { + ...existingDerivationPathIndex, + ...repairs, + }); + logExecutionTime( + 'BdkAccountRepository.getByDerivationPaths repair index', + repairIndexStart, + ); + } + + return results; + } finally { + logExecutionTime('BdkAccountRepository.getByDerivationPaths', start); + } } async getWithSigner(id: string): Promise { @@ -186,101 +207,166 @@ export class BdkAccountRepository implements BitcoinAccountRepository { network: Network, addressType: AddressType, ): Promise { - const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); - const id = v4(); - const fingerprint = toBdkFingerprint( - slip10.masterFingerprint ?? slip10.parentFingerprint, - ); - - const xpub = slip10_to_extended(slip10, network); - const descriptors = xpub_to_descriptor( - xpub, - fingerprint, - network, - addressType, - ); - - return BdkAccountAdapter.create(id, derivationPath, descriptors, network); - } + const start = Date.now(); + + try { + const entropyStart = Date.now(); + const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); + logExecutionTime( + 'BdkAccountRepository.create get public entropy', + entropyStart, + ); - async insert(account: BitcoinAccount): Promise { - const { id, derivationPath } = account; + const buildWalletStart = Date.now(); + const id = v4(); + const fingerprint = toBdkFingerprint( + slip10.masterFingerprint ?? slip10.parentFingerprint, + ); - const walletData = account.takeStaged(); - if (!walletData) { - throw new StorageError( - `Missing changeset data for account "${id}" for insertion.`, + const xpub = slip10_to_extended(slip10, network); + const descriptors = xpub_to_descriptor( + xpub, + fingerprint, + network, + addressType, ); - } - await Promise.all([ - this.#snapClient.setState( - `derivationPaths.${getDerivationPathKey(derivationPath)}`, + const account = BdkAccountAdapter.create( id, - ), - this.#snapClient.setState(`accounts.${id}`, { - wallet: walletData.to_json(), - inscriptions: [], derivationPath, - }), - ]); - - return account; - } - - async insertMany(accounts: BitcoinAccount[]): Promise { - if (accounts.length === 0) { - return []; - } - - if (accounts.length === 1) { - return [await this.insert(accounts[0] as BitcoinAccount)]; + descriptors, + network, + ); + logExecutionTime( + 'BdkAccountRepository.create build wallet', + buildWalletStart, + ); + return account; + } finally { + logExecutionTime('BdkAccountRepository.create', start); } + } - const accountStateEntries: [string, AccountState][] = []; - const derivationPathEntries: [string, string][] = []; + async insert(account: BitcoinAccount): Promise { + const start = Date.now(); - for (const account of accounts) { + try { const { id, derivationPath } = account; - const walletData = account.takeStaged(); + const serializeStart = Date.now(); + const walletData = account.takeStaged(); if (!walletData) { throw new StorageError( `Missing changeset data for account "${id}" for insertion.`, ); } + logExecutionTime( + 'BdkAccountRepository.insert serialize account', + serializeStart, + ); - accountStateEntries.push([ - id, - { + const writeStateStart = Date.now(); + await Promise.all([ + this.#snapClient.setState( + `derivationPaths.${getDerivationPathKey(derivationPath)}`, + id, + ), + this.#snapClient.setState(`accounts.${id}`, { wallet: walletData.to_json(), inscriptions: [], derivationPath, - }, + }), ]); - derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); + logExecutionTime( + 'BdkAccountRepository.insert write state', + writeStateStart, + ); + + return account; + } finally { + logExecutionTime('BdkAccountRepository.insert', start); } + } - const [existingAccounts, existingDerivationPaths] = await Promise.all([ - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - ]); + async insertMany(accounts: BitcoinAccount[]): Promise { + const start = Date.now(); - await this.#snapClient.setState('accounts', { - ...(existingAccounts ?? {}), - ...Object.fromEntries(accountStateEntries), - }); + try { + if (accounts.length === 0) { + return []; + } - await this.#snapClient.setState('derivationPaths', { - ...(existingDerivationPaths ?? {}), - ...Object.fromEntries(derivationPathEntries), - }); + if (accounts.length === 1) { + return [await this.insert(accounts[0] as BitcoinAccount)]; + } + + const accountStateEntries: [string, AccountState][] = []; + const derivationPathEntries: [string, string][] = []; + + const serializeStart = Date.now(); + for (const account of accounts) { + const { id, derivationPath } = account; + const walletData = account.takeStaged(); + + if (!walletData) { + throw new StorageError( + `Missing changeset data for account "${id}" for insertion.`, + ); + } + + accountStateEntries.push([ + id, + { + wallet: walletData.to_json(), + inscriptions: [], + derivationPath, + }, + ]); + derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); + } + logExecutionTime( + 'BdkAccountRepository.insertMany serialize accounts', + serializeStart, + ); + + const loadStateStart = Date.now(); + const [existingAccounts, existingDerivationPaths] = await Promise.all([ + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + ]); + logExecutionTime( + 'BdkAccountRepository.insertMany load state', + loadStateStart, + ); - return accounts; + const writeAccountsStart = Date.now(); + await this.#snapClient.setState('accounts', { + ...(existingAccounts ?? {}), + ...Object.fromEntries(accountStateEntries), + }); + logExecutionTime( + 'BdkAccountRepository.insertMany write accounts', + writeAccountsStart, + ); + + const writeDerivationPathsStart = Date.now(); + await this.#snapClient.setState('derivationPaths', { + ...(existingDerivationPaths ?? {}), + ...Object.fromEntries(derivationPathEntries), + }); + logExecutionTime( + 'BdkAccountRepository.insertMany write derivation paths', + writeDerivationPathsStart, + ); + + return accounts; + } finally { + logExecutionTime('BdkAccountRepository.insertMany', start); + } } async update( diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 000edb27b..236d6389b 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -33,6 +33,7 @@ import { WalletError, } from '../entities'; import { CronMethod } from '../handlers/CronHandler'; +import { logExecutionTime } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export type DiscoverAccountParams = { @@ -272,120 +273,165 @@ export class AccountUseCases { } async createMany(reqs: CreateAccountParams[]): Promise { + const start = Date.now(); + if (reqs.length === 0) { + logExecutionTime('AccountUseCases.createMany', start); return []; } - const { accounts, createdAccountKeys } = await this.#runAccountMutation( - async () => { - const entries = reqs.map((req, index) => { - const derivationPath = getAccountDerivationPath(req); - return { - req, - index, - derivationPath, - pathKey: getDerivationPathKey(derivationPath), - }; - }); - - const uniqueEntriesByPath = new Map(); - for (const entry of entries) { - if (!uniqueEntriesByPath.has(entry.pathKey)) { - uniqueEntriesByPath.set(entry.pathKey, entry); - } - } - const uniqueEntries = [...uniqueEntriesByPath.values()]; - - const existingAccounts = await this.#repository.getByDerivationPaths( - uniqueEntries.map(({ derivationPath }) => derivationPath), - ); - const existingAccountsByPath = new Map(); - - uniqueEntries.forEach((entry, index) => { - const account = existingAccounts[index]; - if (account && account.network === entry.req.network) { - existingAccountsByPath.set(entry.pathKey, account); - } - }); - - const entriesToCreate = uniqueEntries.filter( - ({ pathKey }) => !existingAccountsByPath.has(pathKey), - ); - const newAccounts = await runWithConcurrencyLimit( - entriesToCreate, - CREATE_ACCOUNTS_CONCURRENCY, - async ({ derivationPath, req }) => { - const newAccount = await this.#repository.create( + try { + const { accounts, createdAccountKeys } = await this.#runAccountMutation( + async () => { + const buildEntriesStart = Date.now(); + const entries = reqs.map((req, index) => { + const derivationPath = getAccountDerivationPath(req); + return { + req, + index, derivationPath, - req.network, - req.addressType, - ); - newAccount.revealNextAddress(); - return newAccount; - }, - ); + pathKey: getDerivationPathKey(derivationPath), + }; + }); + + const uniqueEntriesByPath = new Map< + string, + (typeof entries)[number] + >(); + for (const entry of entries) { + if (!uniqueEntriesByPath.has(entry.pathKey)) { + uniqueEntriesByPath.set(entry.pathKey, entry); + } + } + const uniqueEntries = [...uniqueEntriesByPath.values()]; + logExecutionTime( + 'AccountUseCases.createMany build entries', + buildEntriesStart, + ); - if (newAccounts.length > 0) { - await this.#repository.insertMany(newAccounts); - } + const lookupStart = Date.now(); + const existingAccounts = await this.#repository.getByDerivationPaths( + uniqueEntries.map(({ derivationPath }) => derivationPath), + ); + logExecutionTime( + 'AccountUseCases.createMany get existing accounts', + lookupStart, + ); + const existingAccountsByPath = new Map(); - const newAccountsByPath = new Map( - entriesToCreate.map((entry, index) => [ - entry.pathKey, - newAccounts[index] as BitcoinAccount, - ]), - ); + uniqueEntries.forEach((entry, index) => { + const account = existingAccounts[index]; + if (account && account.network === entry.req.network) { + existingAccountsByPath.set(entry.pathKey, account); + } + }); - const accountsInOrder = entries.map((entry) => { - const account = - existingAccountsByPath.get(entry.pathKey) ?? - newAccountsByPath.get(entry.pathKey); + const entriesToCreate = uniqueEntries.filter( + ({ pathKey }) => !existingAccountsByPath.has(pathKey), + ); + const createNewAccountsStart = Date.now(); + const newAccounts = await runWithConcurrencyLimit( + entriesToCreate, + CREATE_ACCOUNTS_CONCURRENCY, + async ({ derivationPath, req }) => { + const newAccount = await this.#repository.create( + derivationPath, + req.network, + req.addressType, + ); + newAccount.revealNextAddress(); + return newAccount; + }, + ); + logExecutionTime( + 'AccountUseCases.createMany create new accounts', + createNewAccountsStart, + ); - if (!account) { - throw new AssertionError('Failed to create account', { - index: entry.index, - derivationPath: entry.derivationPath, - }); + const insertNewAccountsStart = Date.now(); + if (newAccounts.length > 0) { + await this.#repository.insertMany(newAccounts); } + logExecutionTime( + 'AccountUseCases.createMany insert new accounts', + insertNewAccountsStart, + ); - return account; - }); + const newAccountsByPath = new Map( + entriesToCreate.map((entry, index) => [ + entry.pathKey, + newAccounts[index] as BitcoinAccount, + ]), + ); - return { - accounts: accountsInOrder, - createdAccountKeys: new Set(newAccountsByPath.keys()), - }; - }, - ); + const orderAccountsStart = Date.now(); + const accountsInOrder = entries.map((entry) => { + const account = + existingAccountsByPath.get(entry.pathKey) ?? + newAccountsByPath.get(entry.pathKey); + + if (!account) { + throw new AssertionError('Failed to create account', { + index: entry.index, + derivationPath: entry.derivationPath, + }); + } + + return account; + }); + logExecutionTime( + 'AccountUseCases.createMany order accounts', + orderAccountsStart, + ); - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - await this.#snapClient.emitAccountCreatedEvent( - account, - req.correlationId, - req.accountName, + return { + accounts: accountsInOrder, + createdAccountKeys: new Set(newAccountsByPath.keys()), + }; + }, ); - } - const scheduledAccountIds = new Set(); - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); - if ( - req.synchronize && - createdAccountKeys.has(pathKey) && - !scheduledAccountIds.has(account.id) - ) { - scheduledAccountIds.add(account.id); - await this.#snapClient.scheduleBackgroundEvent({ - duration: 'PT1S', - method: CronMethod.FullScanAccount, - params: { accountId: account.id }, - }); + const emitEventsStart = Date.now(); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + await this.#snapClient.emitAccountCreatedEvent( + account, + req.correlationId, + req.accountName, + ); } - } + logExecutionTime( + 'AccountUseCases.createMany emit account-created events', + emitEventsStart, + ); - return accounts; + const scheduleFullScansStart = Date.now(); + const scheduledAccountIds = new Set(); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); + if ( + req.synchronize && + createdAccountKeys.has(pathKey) && + !scheduledAccountIds.has(account.id) + ) { + scheduledAccountIds.add(account.id); + await this.#snapClient.scheduleBackgroundEvent({ + duration: 'PT1S', + method: CronMethod.FullScanAccount, + params: { accountId: account.id }, + }); + } + } + logExecutionTime( + 'AccountUseCases.createMany schedule full scans', + scheduleFullScansStart, + ); + + return accounts; + } finally { + logExecutionTime('AccountUseCases.createMany', start); + } } async synchronize( diff --git a/packages/snap/src/utils/performance.ts b/packages/snap/src/utils/performance.ts new file mode 100644 index 000000000..545f6dbf6 --- /dev/null +++ b/packages/snap/src/utils/performance.ts @@ -0,0 +1,8 @@ +export const logExecutionTime = (operation: string, start: number): void => { + const end = Date.now(); + console.log( + `[PERFORMANCE DEBUG - BITCOIN SNAP] ${operation} took ${ + end - start + } ms to execute`, + ); +}; From e706b828426a3381a8ad36b758f4a62e5ca9260f Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 7 May 2026 11:54:49 -0400 Subject: [PATCH 23/26] fix: remove event emission --- .../src/use-cases/AccountUseCases.test.ts | 23 ++++--------------- .../snap/src/use-cases/AccountUseCases.ts | 14 ----------- 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/packages/snap/src/use-cases/AccountUseCases.test.ts b/packages/snap/src/use-cases/AccountUseCases.test.ts index 430dbee95..1a25b6c91 100644 --- a/packages/snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/snap/src/use-cases/AccountUseCases.test.ts @@ -335,18 +335,7 @@ describe('AccountUseCases', () => { ); expect(newAccount.revealNextAddress).toHaveBeenCalled(); expect(mockRepository.insertMany).toHaveBeenCalledWith([newAccount]); - expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenNthCalledWith( - 1, - existingAccount, - createParams.correlationId, - createParams.accountName, - ); - expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenNthCalledWith( - 2, - newAccount, - createParams.correlationId, - createParams.accountName, - ); + expect(mockSnapClient.emitAccountCreatedEvent).not.toHaveBeenCalled(); expect(mockSnapClient.scheduleBackgroundEvent).toHaveBeenCalledWith({ duration: 'PT1S', method: CronMethod.FullScanAccount, @@ -366,7 +355,7 @@ describe('AccountUseCases', () => { ]); expect(mockRepository.create).toHaveBeenCalledTimes(1); expect(mockRepository.insertMany).toHaveBeenCalledWith([newAccount]); - expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenCalledTimes(2); + expect(mockSnapClient.emitAccountCreatedEvent).not.toHaveBeenCalled(); expect(result).toStrictEqual([newAccount, newAccount]); }); @@ -377,15 +366,11 @@ describe('AccountUseCases', () => { expect(mockRepository.create).not.toHaveBeenCalled(); expect(mockRepository.insertMany).not.toHaveBeenCalled(); - expect(mockSnapClient.emitAccountCreatedEvent).toHaveBeenCalledWith( - existingAccount, - createParams.correlationId, - createParams.accountName, - ); + expect(mockSnapClient.emitAccountCreatedEvent).not.toHaveBeenCalled(); expect(result).toStrictEqual([existingAccount]); }); - it('propagates insertMany errors before emitting events', async () => { + it('propagates insertMany errors without emitting account-created events', async () => { const error = new Error('insertMany failed'); mockRepository.getByDerivationPaths.mockResolvedValue([null]); mockRepository.create.mockResolvedValue(newAccount); diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 236d6389b..8c78d1ee1 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -391,20 +391,6 @@ export class AccountUseCases { }, ); - const emitEventsStart = Date.now(); - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - await this.#snapClient.emitAccountCreatedEvent( - account, - req.correlationId, - req.accountName, - ); - } - logExecutionTime( - 'AccountUseCases.createMany emit account-created events', - emitEventsStart, - ); - const scheduleFullScansStart = Date.now(); const scheduledAccountIds = new Set(); for (const [index, account] of accounts.entries()) { From b88ce08c4306475491b7e96bf50ef54e4d9abf61 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 7 May 2026 12:59:32 -0400 Subject: [PATCH 24/26] add internal batching --- .../snap/src/handlers/KeyringHandler.test.ts | 34 ++-- packages/snap/src/handlers/KeyringHandler.ts | 146 ++++++++++++++--- .../snap/src/store/BdkAccountRepository.ts | 147 ++++++++++++++++-- .../snap/src/use-cases/AccountUseCases.ts | 75 ++++++++- 4 files changed, 349 insertions(+), 53 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 51c2fbcaf..2993dcf80 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -602,15 +602,31 @@ describe('KeyringHandler', () => { expect(mockAccounts.createMany).not.toHaveBeenCalled(); }); - it('rejects batches larger than the per-RPC limit', async () => { - await expect( - handler.createAccounts({ - type: AccountCreationType.Bip44DeriveIndexRange, - range: { from: 0, to: 100 }, - entropySource, - }), - ).rejects.toThrow(/more than 100 accounts/iu); - expect(mockAccounts.createMany).not.toHaveBeenCalled(); + it('splits requests larger than 100 accounts into internal batches', async () => { + mockAccounts.createMany.mockImplementation(async (requests) => + requests.map(({ index }) => buildMockAccount(index)), + ); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 0, to: 100 }, + entropySource, + }); + + expect(mockAccounts.createMany).toHaveBeenCalledTimes(2); + expect(mockAccounts.createMany).toHaveBeenNthCalledWith( + 1, + Array.from({ length: 100 }, (_, index) => + expect.objectContaining({ index }), + ), + ); + expect(mockAccounts.createMany).toHaveBeenNthCalledWith(2, [ + expect.objectContaining({ index: 100 }), + ]); + expect(result).toHaveLength(101); + expect( + result.map((account) => mnemonicGroupIndex(account)), + ).toStrictEqual(Array.from({ length: 101 }, (_, index) => index)); }); it('rejects unsupported creation types', async () => { diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index c6c8c3b42..fbff8162f 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -70,7 +70,10 @@ import { mapToTransaction, } from './mappings'; import { BtcWalletRequestStruct, validateSelectedAccounts } from './validation'; -import type { AccountUseCases } from '../use-cases/AccountUseCases'; +import type { + AccountUseCases, + CreateAccountParams, +} from '../use-cases/AccountUseCases'; import { logExecutionTime } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; @@ -85,9 +88,42 @@ export const CreateAccountRequest = object({ ...MetaMaskOptionsStruct.schema, }); -/** Upper bound on inclusive index range width for a single createAccounts RPC. */ +/** Maximum number of accounts to create in one internal createMany call. */ const MAX_CREATE_ACCOUNTS_PER_BATCH = 100; +/** + * @param operation - Base operation name. + * @param details - Account creation context to append. + * @returns Operation name with account creation context. + */ +function formatAccountCreationOperation( + operation: string, + details: string, +): string { + return `${operation} (${details})`; +} + +/** + * @param details - Account creation context to append. + * @param state - Snap state snapshot. + * @param state.root - Root Snap state. + * @param state.accounts - Account state namespace. + * @param state.derivationPaths - Derivation-path index namespace. + * @returns Snapshot log line. + */ +function formatCreateAccountsStateLog( + details: string, + state: { + root: Json | null; + accounts: Json | null; + derivationPaths: Json | null; + }, +): string { + return `[SNAP STATE DEBUG - BITCOIN SNAP] keyring_createAccounts final state (${details}) ${JSON.stringify( + state, + )}`; +} + export class KeyringHandler implements Keyring { readonly #accountsUseCases: AccountUseCases; @@ -333,11 +369,9 @@ export class KeyringHandler implements Keyring { } const batchSize = range.to - range.from + 1; - if (batchSize > MAX_CREATE_ACCOUNTS_PER_BATCH) { - throw new FormatError( - `Cannot create more than ${MAX_CREATE_ACCOUNTS_PER_BATCH} accounts in one batch`, - ); - } + const indexLabel = + range.from === range.to ? `${range.from}` : `${range.from}-${range.to}`; + const accountCreationDetails = `entropyId=${entropySource} indexes=${indexLabel} count=${batchSize}`; // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring // the defaults used by `createAccount` when no scope is provided. @@ -349,11 +383,6 @@ export class KeyringHandler implements Keyring { ); } - const indices: number[] = []; - for (let index = range.from; index <= range.to; index += 1) { - indices.push(index); - } - const traceName = 'Create Bitcoin Accounts Batch'; let traceStarted = false; @@ -370,21 +399,63 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. const createManyStart = Date.now(); - const accounts = await this.#accountsUseCases.createMany( - indices.map((index) => ({ - network, - entropySource, - index, - addressType, - synchronize: false, - })), + const accounts: BitcoinAccount[] = []; + let chunkFrom = range.from; + + while (chunkFrom <= range.to) { + const chunkTo = Math.min( + chunkFrom + MAX_CREATE_ACCOUNTS_PER_BATCH - 1, + range.to, + ); + const chunkSize = chunkTo - chunkFrom + 1; + const chunkIndexLabel = + chunkFrom === chunkTo ? `${chunkFrom}` : `${chunkFrom}-${chunkTo}`; + const chunkDetails = `entropyId=${entropySource} indexes=${chunkIndexLabel} count=${chunkSize}`; + const chunkRequests: CreateAccountParams[] = []; + + for (let index = chunkFrom; index <= chunkTo; index += 1) { + chunkRequests.push({ + network, + entropySource, + index, + addressType, + synchronize: false, + }); + } + + const createManyChunkStart = Date.now(); + accounts.push( + ...(await this.#accountsUseCases.createMany(chunkRequests)), + ); + logExecutionTime( + formatAccountCreationOperation( + 'keyring_createAccounts createMany batch', + chunkDetails, + ), + createManyChunkStart, + ); + + if (chunkTo === range.to) { + break; + } + chunkFrom = chunkTo + 1; + } + + logExecutionTime( + formatAccountCreationOperation( + 'keyring_createAccounts createMany', + accountCreationDetails, + ), + createManyStart, ); - logExecutionTime('keyring_createAccounts createMany', createManyStart); const responseMappingStart = Date.now(); const result = accounts.map(mapToKeyringAccount); logExecutionTime( - 'keyring_createAccounts response mapping', + formatAccountCreationOperation( + 'keyring_createAccounts response mapping', + accountCreationDetails, + ), responseMappingStart, ); return result; @@ -396,10 +467,39 @@ export class KeyringHandler implements Keyring { 'endTrace', ); } - logExecutionTime('keyring_createAccounts', start); + logExecutionTime( + formatAccountCreationOperation( + 'keyring_createAccounts', + accountCreationDetails, + ), + start, + ); + await this.#logCreateAccountsState(accountCreationDetails); } } + async #logCreateAccountsState(accountCreationDetails: string): Promise { + await runSnapActionSafely( + async () => { + const [root, accounts, derivationPaths] = await Promise.all([ + this.#snapClient.getState(), + this.#snapClient.getState('accounts'), + this.#snapClient.getState('derivationPaths'), + ]); + + console.log( + formatCreateAccountsStateLog(accountCreationDetails, { + root, + accounts, + derivationPaths, + }), + ); + }, + this.#logger, + 'logCreateAccountsState', + ); + } + async discoverAccounts( scopes: BtcScope[], entropySource: string, diff --git a/packages/snap/src/store/BdkAccountRepository.ts b/packages/snap/src/store/BdkAccountRepository.ts index 0420feea9..8f38ac71c 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -43,6 +43,62 @@ function getDerivationPathKey(derivationPath: string[]): string { return derivationPath.join('/'); } +/** + * @param derivationPath - Account derivation path. + * @returns Account creation context for performance logs. + */ +function formatAccountCreationDetails(derivationPath: string[]): string { + const entropyId = derivationPath[0] ?? 'unknown'; + const accountIndexSegment = derivationPath[3] ?? 'unknown'; + const accountIndex = accountIndexSegment.endsWith("'") + ? accountIndexSegment.slice(0, -1) + : accountIndexSegment; + + return `entropyId=${entropyId} index=${accountIndex} derivationPath=${getDerivationPathKey( + derivationPath, + )}`; +} + +/** + * @param derivationPaths - Account derivation paths. + * @returns Batch account creation context for performance logs. + */ +function formatAccountCreationBatchDetails( + derivationPaths: readonly string[][], +): string { + const entropyIds = [ + ...new Set( + derivationPaths.map((derivationPath) => derivationPath[0] ?? 'unknown'), + ), + ] + .sort() + .join(','); + const indices = derivationPaths + .map((derivationPath) => { + const segment = derivationPath[3] ?? 'unknown'; + return segment.endsWith("'") ? segment.slice(0, -1) : segment; + }) + .sort((left, right) => Number(left) - Number(right)); + const firstIndex = indices[0] ?? 'unknown'; + const lastIndex = indices[indices.length - 1] ?? 'unknown'; + const indexLabel = + firstIndex === lastIndex ? `${firstIndex}` : `${firstIndex}-${lastIndex}`; + + return `entropyId=${entropyIds} indexes=${indexLabel} count=${derivationPaths.length}`; +} + +/** + * @param operation - Base operation name. + * @param details - Account creation context to append. + * @returns Operation name with account creation context. + */ +function formatAccountCreationOperation( + operation: string, + details: string, +): string { + return `${operation} (${details})`; +} + export class BdkAccountRepository implements BitcoinAccountRepository { readonly #snapClient: SnapClient; @@ -91,6 +147,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { derivationPaths: string[][], ): Promise<(BitcoinAccount | null)[]> { const start = Date.now(); + const batchDetails = formatAccountCreationBatchDetails(derivationPaths); if (derivationPaths.length === 0) { logExecutionTime('BdkAccountRepository.getByDerivationPaths', start); @@ -108,7 +165,10 @@ export class BdkAccountRepository implements BitcoinAccountRepository { >, ]); logExecutionTime( - 'BdkAccountRepository.getByDerivationPaths load state', + formatAccountCreationOperation( + 'BdkAccountRepository.getByDerivationPaths load state', + batchDetails, + ), loadStateStart, ); @@ -155,14 +215,23 @@ export class BdkAccountRepository implements BitcoinAccountRepository { ...repairs, }); logExecutionTime( - 'BdkAccountRepository.getByDerivationPaths repair index', + formatAccountCreationOperation( + 'BdkAccountRepository.getByDerivationPaths repair index', + batchDetails, + ), repairIndexStart, ); } return results; } finally { - logExecutionTime('BdkAccountRepository.getByDerivationPaths', start); + logExecutionTime( + formatAccountCreationOperation( + 'BdkAccountRepository.getByDerivationPaths', + batchDetails, + ), + start, + ); } } @@ -208,12 +277,16 @@ export class BdkAccountRepository implements BitcoinAccountRepository { addressType: AddressType, ): Promise { const start = Date.now(); + const accountDetails = formatAccountCreationDetails(derivationPath); try { const entropyStart = Date.now(); const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); logExecutionTime( - 'BdkAccountRepository.create get public entropy', + formatAccountCreationOperation( + 'BdkAccountRepository.create get public entropy', + accountDetails, + ), entropyStart, ); @@ -238,17 +311,27 @@ export class BdkAccountRepository implements BitcoinAccountRepository { network, ); logExecutionTime( - 'BdkAccountRepository.create build wallet', + formatAccountCreationOperation( + 'BdkAccountRepository.create build wallet', + accountDetails, + ), buildWalletStart, ); return account; } finally { - logExecutionTime('BdkAccountRepository.create', start); + logExecutionTime( + formatAccountCreationOperation( + 'BdkAccountRepository.create', + accountDetails, + ), + start, + ); } } async insert(account: BitcoinAccount): Promise { const start = Date.now(); + const accountDetails = formatAccountCreationDetails(account.derivationPath); try { const { id, derivationPath } = account; @@ -261,7 +344,10 @@ export class BdkAccountRepository implements BitcoinAccountRepository { ); } logExecutionTime( - 'BdkAccountRepository.insert serialize account', + formatAccountCreationOperation( + 'BdkAccountRepository.insert serialize account', + accountDetails, + ), serializeStart, ); @@ -278,18 +364,33 @@ export class BdkAccountRepository implements BitcoinAccountRepository { }), ]); logExecutionTime( - 'BdkAccountRepository.insert write state', + formatAccountCreationOperation( + 'BdkAccountRepository.insert write state', + accountDetails, + ), writeStateStart, ); return account; } finally { - logExecutionTime('BdkAccountRepository.insert', start); + logExecutionTime( + formatAccountCreationOperation( + 'BdkAccountRepository.insert', + accountDetails, + ), + start, + ); } } async insertMany(accounts: BitcoinAccount[]): Promise { const start = Date.now(); + const batchDetails = + accounts.length > 0 + ? formatAccountCreationBatchDetails( + accounts.map(({ derivationPath }) => derivationPath), + ) + : 'count=0'; try { if (accounts.length === 0) { @@ -325,7 +426,10 @@ export class BdkAccountRepository implements BitcoinAccountRepository { derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); } logExecutionTime( - 'BdkAccountRepository.insertMany serialize accounts', + formatAccountCreationOperation( + 'BdkAccountRepository.insertMany serialize accounts', + batchDetails, + ), serializeStart, ); @@ -339,7 +443,10 @@ export class BdkAccountRepository implements BitcoinAccountRepository { >, ]); logExecutionTime( - 'BdkAccountRepository.insertMany load state', + formatAccountCreationOperation( + 'BdkAccountRepository.insertMany load state', + batchDetails, + ), loadStateStart, ); @@ -349,7 +456,10 @@ export class BdkAccountRepository implements BitcoinAccountRepository { ...Object.fromEntries(accountStateEntries), }); logExecutionTime( - 'BdkAccountRepository.insertMany write accounts', + formatAccountCreationOperation( + 'BdkAccountRepository.insertMany write accounts', + batchDetails, + ), writeAccountsStart, ); @@ -359,13 +469,22 @@ export class BdkAccountRepository implements BitcoinAccountRepository { ...Object.fromEntries(derivationPathEntries), }); logExecutionTime( - 'BdkAccountRepository.insertMany write derivation paths', + formatAccountCreationOperation( + 'BdkAccountRepository.insertMany write derivation paths', + batchDetails, + ), writeDerivationPathsStart, ); return accounts; } finally { - logExecutionTime('BdkAccountRepository.insertMany', start); + logExecutionTime( + formatAccountCreationOperation( + 'BdkAccountRepository.insertMany', + batchDetails, + ), + start, + ); } } diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 8c78d1ee1..0189753e1 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -72,6 +72,41 @@ function getDerivationPathKey(derivationPath: string[]): string { return derivationPath.join('/'); } +/** + * @param reqs - Account creation requests. + * @returns Account creation context for performance logs. + */ +function formatAccountCreationBatchDetails( + reqs: readonly CreateAccountParams[], +): string { + const entropyIds = [ + ...new Set(reqs.map(({ entropySource }) => entropySource)), + ] + .sort() + .join(','); + const indices = reqs + .map(({ index }) => index) + .sort((left, right) => left - right); + const firstIndex = indices[0]; + const lastIndex = indices[indices.length - 1]; + const indexLabel = + firstIndex === lastIndex ? `${firstIndex}` : `${firstIndex}-${lastIndex}`; + + return `entropyId=${entropyIds} indexes=${indexLabel} count=${reqs.length}`; +} + +/** + * @param operation - Base operation name. + * @param details - Account creation context to append. + * @returns Operation name with account creation context. + */ +function formatAccountCreationOperation( + operation: string, + details: string, +): string { + return `${operation} (${details})`; +} + /** * Map items to results with at most `concurrency` in-flight async operations. * Output order matches `items` order. @@ -280,6 +315,8 @@ export class AccountUseCases { return []; } + const batchDetails = formatAccountCreationBatchDetails(reqs); + try { const { accounts, createdAccountKeys } = await this.#runAccountMutation( async () => { @@ -305,7 +342,10 @@ export class AccountUseCases { } const uniqueEntries = [...uniqueEntriesByPath.values()]; logExecutionTime( - 'AccountUseCases.createMany build entries', + formatAccountCreationOperation( + 'AccountUseCases.createMany build entries', + batchDetails, + ), buildEntriesStart, ); @@ -314,7 +354,10 @@ export class AccountUseCases { uniqueEntries.map(({ derivationPath }) => derivationPath), ); logExecutionTime( - 'AccountUseCases.createMany get existing accounts', + formatAccountCreationOperation( + 'AccountUseCases.createMany get existing accounts', + batchDetails, + ), lookupStart, ); const existingAccountsByPath = new Map(); @@ -344,7 +387,10 @@ export class AccountUseCases { }, ); logExecutionTime( - 'AccountUseCases.createMany create new accounts', + formatAccountCreationOperation( + 'AccountUseCases.createMany create new accounts', + batchDetails, + ), createNewAccountsStart, ); @@ -353,7 +399,10 @@ export class AccountUseCases { await this.#repository.insertMany(newAccounts); } logExecutionTime( - 'AccountUseCases.createMany insert new accounts', + formatAccountCreationOperation( + 'AccountUseCases.createMany insert new accounts', + batchDetails, + ), insertNewAccountsStart, ); @@ -380,7 +429,10 @@ export class AccountUseCases { return account; }); logExecutionTime( - 'AccountUseCases.createMany order accounts', + formatAccountCreationOperation( + 'AccountUseCases.createMany order accounts', + batchDetails, + ), orderAccountsStart, ); @@ -410,13 +462,22 @@ export class AccountUseCases { } } logExecutionTime( - 'AccountUseCases.createMany schedule full scans', + formatAccountCreationOperation( + 'AccountUseCases.createMany schedule full scans', + batchDetails, + ), scheduleFullScansStart, ); return accounts; } finally { - logExecutionTime('AccountUseCases.createMany', start); + logExecutionTime( + formatAccountCreationOperation( + 'AccountUseCases.createMany', + batchDetails, + ), + start, + ); } } From e83f11e9fa36907214531d9fd52db6aeb5c69a43 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 7 May 2026 20:36:33 -0400 Subject: [PATCH 25/26] experiment --- packages/snap/src/entities/snap.ts | 15 +- .../snap/src/handlers/KeyringHandler.test.ts | 76 +++++++ packages/snap/src/handlers/KeyringHandler.ts | 202 +++++++++++++++++- .../snap/src/infra/StoredAccountAdapter.ts | 181 ++++++++++++++++ packages/snap/src/infra/index.ts | 1 + .../src/store/BdkAccountRepository.test.ts | 70 ++++++ .../snap/src/store/BdkAccountRepository.ts | 63 ++++-- .../snap/src/use-cases/AccountUseCases.ts | 3 +- 8 files changed, 582 insertions(+), 29 deletions(-) create mode 100644 packages/snap/src/infra/StoredAccountAdapter.ts diff --git a/packages/snap/src/entities/snap.ts b/packages/snap/src/entities/snap.ts index 8d150ce32..f84bcfa0d 100644 --- a/packages/snap/src/entities/snap.ts +++ b/packages/snap/src/entities/snap.ts @@ -1,4 +1,4 @@ -import type { WalletTx } from '@metamask/bitcoindevkit'; +import type { AddressType, Network, WalletTx } from '@metamask/bitcoindevkit'; import type { JsonSLIP10Node, SLIP10Node } from '@metamask/key-tree'; import type { ComponentOrElement, @@ -25,6 +25,19 @@ export type AccountState = { wallet: string; // Wallet inscriptions for meta protocols (ordinals, etc.) inscriptions: Inscription[]; + // Metadata used by keyring account responses without loading the BDK wallet. + metadata?: AccountMetadata; +}; + +export type AccountMetadata = { + // Public receive address at account address index 0. + address: string; + // Account address type. + addressType: AddressType; + // Bitcoin network. + network: Network; + // Public descriptor for read-only descriptor requests. + publicDescriptor: string; }; export type SyncResult = { diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 2993dcf80..e8f955e1f 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -22,6 +22,7 @@ import { BtcMethod, BtcScope, } from '@metamask/keyring-api'; +import type { Json } from '@metamask/snaps-sdk'; import { mock } from 'jest-mock-extended'; import { assert } from 'superstruct'; @@ -629,6 +630,81 @@ describe('KeyringHandler', () => { ).toStrictEqual(Array.from({ length: 101 }, (_, index) => index)); }); + it('logs a final snap state verification summary', async () => { + const consoleLog = jest.spyOn(console, 'log').mockImplementation(); + const rootState = { accounts: true, derivationPaths: true } as Json; + const accountsState = { + 'id-0': { + wallet: '{}', + inscriptions: [], + derivationPath: [entropySource, "84'", "0'", "0'"], + }, + 'id-1': { + wallet: '{}', + inscriptions: [], + derivationPath: [entropySource, "84'", "0'", "1'"], + }, + } as Json; + const derivationPathsState = { + [`${entropySource}/84'/0'/0'`]: 'id-0', + [`${entropySource}/84'/0'/1'`]: 'id-1', + [`${entropySource}/84'/0'/99'`]: 'missing-id', + } as Json; + mockAccounts.createMany.mockResolvedValue( + [0, 1, 2].map(buildMockAccount), + ); + mockSnapClient.getState + .mockResolvedValueOnce(rootState) + .mockResolvedValueOnce(accountsState) + .mockResolvedValueOnce(derivationPathsState); + + try { + await handler.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + range: { from: 0, to: 2 }, + entropySource, + }); + + const stateLog = consoleLog.mock.calls.at(-1)?.[0] as + | string + | undefined; + + expect(stateLog).toBeDefined(); + expect(expectDefined(stateLog)).toContain( + '[SNAP STATE DEBUG - BITCOIN SNAP]', + ); + const summary = JSON.parse( + expectDefined(stateLog).slice(expectDefined(stateLog).indexOf('{')), + ); + expect(summary).toStrictEqual( + expect.objectContaining({ + accountsCount: 2, + derivationPathsCount: 3, + entropyId: entropySource, + entropyAccountsCount: 2, + entropyDerivationPathsCount: 3, + entropyMinIndex: 0, + entropyMaxIndex: 1, + requestedRange: { + from: 0, + to: 2, + count: 3, + persistedCount: 2, + missingCount: 1, + missingSample: [2], + missingSampleLimit: 25, + }, + consistency: { + orphanedDerivationPathCount: 1, + missingDerivationPathIndexCount: 0, + }, + }), + ); + } finally { + consoleLog.mockRestore(); + } + }); + it('rejects unsupported creation types', async () => { await expect( handler.createAccounts({ diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index fbff8162f..b136b37df 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -1,4 +1,4 @@ -import type { AddressType } from '@metamask/bitcoindevkit'; +import type { AddressType, Network } from '@metamask/bitcoindevkit'; import { AccountCreationType, assertCreateAccountOptionIsSupported, @@ -47,13 +47,18 @@ import { string, } from 'superstruct'; -import type { BitcoinAccount, Logger, SnapClient } from '../entities'; import { + addressTypeToPurpose, + type AccountState, + type BitcoinAccount, FormatError, InexistentMethodError, + type Logger, + networkToCoinType, networkToCurrencyUnit, Purpose, purposeToAddressType, + type SnapClient, } from '../entities'; import { networkToCaip19, @@ -91,6 +96,8 @@ export const CreateAccountRequest = object({ /** Maximum number of accounts to create in one internal createMany call. */ const MAX_CREATE_ACCOUNTS_PER_BATCH = 100; +const MAX_MISSING_INDEXES_TO_LOG = 25; + /** * @param operation - Base operation name. * @param details - Account creation context to append. @@ -103,25 +110,188 @@ function formatAccountCreationOperation( return `${operation} (${details})`; } +type CreateAccountsStateLogDetails = { + entropySource: string; + from: number; + to: number; + network: Network; + addressType: AddressType; + label: string; +}; + /** - * @param details - Account creation context to append. + * @param value - JSON value to coerce into an object record. + * @returns Object record or an empty object. + */ +function getJsonRecord(value: Json | null): Record { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return {}; + } + + return value as Record; +} + +/** + * @param accounts - Account state namespace. + * @returns Non-null account state entries. + */ +function getAccountStateEntries( + accounts: Json | null, +): [string, AccountState][] { + return Object.entries(getJsonRecord(accounts)).flatMap(([id, account]) => { + if (!account || typeof account !== 'object' || Array.isArray(account)) { + return []; + } + + return [[id, account as unknown as AccountState]]; + }); +} + +/** + * @param derivationPaths - Derivation path state namespace. + * @returns Derivation path entries whose account IDs are strings. + */ +function getDerivationPathEntries( + derivationPaths: Json | null, +): [string, string][] { + return Object.entries(getJsonRecord(derivationPaths)).flatMap(([path, id]) => + typeof id === 'string' ? [[path, id]] : [], + ); +} + +/** + * @param derivationPath - Split derivation path. + * @returns Storage key for a derivation path. + */ +function getDerivationPathKey(derivationPath: string[]): string { + return derivationPath.join('/'); +} + +/** + * @param entropySource - Account entropy source. + * @param addressType - Account address type. + * @param network - Bitcoin network. + * @param index - Account index. + * @returns Expected derivation path storage key. + */ +function getAccountDerivationPathKey( + entropySource: string, + addressType: AddressType, + network: Network, + index: number, +): string { + return [ + entropySource, + `${addressTypeToPurpose[addressType]}'`, + `${networkToCoinType[network]}'`, + `${index}'`, + ].join('/'); +} + +/** + * @param derivationPath - Split derivation path. + * @returns Account index or null when it cannot be parsed. + */ +function getAccountIndex(derivationPath: string[]): number | null { + const segment = derivationPath[3]; + if (!segment) { + return null; + } + + const numericPart = segment.endsWith("'") ? segment.slice(0, -1) : segment; + const index = Number(numericPart); + return Number.isSafeInteger(index) ? index : null; +} + +/** + * @param details - Account creation context. * @param state - Snap state snapshot. * @param state.root - Root Snap state. * @param state.accounts - Account state namespace. * @param state.derivationPaths - Derivation-path index namespace. - * @returns Snapshot log line. + * @returns Snapshot verification log line. */ function formatCreateAccountsStateLog( - details: string, + details: CreateAccountsStateLogDetails, state: { root: Json | null; accounts: Json | null; derivationPaths: Json | null; }, ): string { - return `[SNAP STATE DEBUG - BITCOIN SNAP] keyring_createAccounts final state (${details}) ${JSON.stringify( - state, - )}`; + const accountEntries = getAccountStateEntries(state.accounts); + const accountsById = new Map(accountEntries); + const derivationPathEntries = getDerivationPathEntries(state.derivationPaths); + const derivationPathsByPath = new Map(derivationPathEntries); + const requestedCount = details.to - details.from + 1; + const missingIndexes: number[] = []; + let persistedRequestedCount = 0; + + for (let index = details.from; index <= details.to; index += 1) { + const pathKey = getAccountDerivationPathKey( + details.entropySource, + details.addressType, + details.network, + index, + ); + const accountId = derivationPathsByPath.get(pathKey); + + if (accountId && accountsById.has(accountId)) { + persistedRequestedCount += 1; + } else if (missingIndexes.length < MAX_MISSING_INDEXES_TO_LOG) { + missingIndexes.push(index); + } + } + + const entropyAccountIndexes = accountEntries + .flatMap(([, account]) => { + if (account.derivationPath[0] !== details.entropySource) { + return []; + } + + const index = getAccountIndex(account.derivationPath); + return index === null ? [] : [index]; + }) + .sort((left, right) => left - right); + const entropyDerivationPathsCount = derivationPathEntries.filter(([path]) => + path.startsWith(`${details.entropySource}/`), + ).length; + const orphanedDerivationPathCount = derivationPathEntries.filter( + ([, id]) => !accountsById.has(id), + ).length; + const missingDerivationPathIndexCount = accountEntries.filter( + ([id, account]) => + derivationPathsByPath.get( + getDerivationPathKey(account.derivationPath), + ) !== id, + ).length; + + return `[SNAP STATE DEBUG - BITCOIN SNAP] keyring_createAccounts final state (${ + details.label + }) ${JSON.stringify({ + rootKeys: Object.keys(getJsonRecord(state.root)).sort(), + accountsCount: accountEntries.length, + derivationPathsCount: derivationPathEntries.length, + entropyId: details.entropySource, + entropyAccountsCount: entropyAccountIndexes.length, + entropyDerivationPathsCount, + entropyMinIndex: entropyAccountIndexes[0] ?? null, + entropyMaxIndex: + entropyAccountIndexes[entropyAccountIndexes.length - 1] ?? null, + requestedRange: { + from: details.from, + to: details.to, + count: requestedCount, + persistedCount: persistedRequestedCount, + missingCount: requestedCount - persistedRequestedCount, + missingSample: missingIndexes, + missingSampleLimit: MAX_MISSING_INDEXES_TO_LOG, + }, + consistency: { + orphanedDerivationPathCount, + missingDerivationPathIndexCount, + }, + })}`; } export class KeyringHandler implements Keyring { @@ -377,6 +547,14 @@ export class KeyringHandler implements Keyring { // the defaults used by `createAccount` when no scope is provided. const network = scopeToNetwork[BtcScope.Mainnet]; const addressType = this.#defaultAddressType; + const stateLogDetails: CreateAccountsStateLogDetails = { + entropySource, + from: range.from, + to: range.to, + network, + addressType, + label: accountCreationDetails, + }; if (addressType !== 'p2wpkh') { throw new FormatError( 'Only native segwit (P2WPKH) addresses are supported', @@ -474,11 +652,13 @@ export class KeyringHandler implements Keyring { ), start, ); - await this.#logCreateAccountsState(accountCreationDetails); + await this.#logCreateAccountsState(stateLogDetails); } } - async #logCreateAccountsState(accountCreationDetails: string): Promise { + async #logCreateAccountsState( + details: CreateAccountsStateLogDetails, + ): Promise { await runSnapActionSafely( async () => { const [root, accounts, derivationPaths] = await Promise.all([ @@ -488,7 +668,7 @@ export class KeyringHandler implements Keyring { ]); console.log( - formatCreateAccountsStateLog(accountCreationDetails, { + formatCreateAccountsStateLog(details, { root, accounts, derivationPaths, diff --git a/packages/snap/src/infra/StoredAccountAdapter.ts b/packages/snap/src/infra/StoredAccountAdapter.ts new file mode 100644 index 000000000..92f12464f --- /dev/null +++ b/packages/snap/src/infra/StoredAccountAdapter.ts @@ -0,0 +1,181 @@ +import type { + AddressInfo, + Amount, + Balance, + FullScanRequest, + LocalOutput, + Psbt, + ScriptBuf, + SyncRequest, + Transaction, + Update, + WalletTx, + ChangeSet, + AddressType, + Network, +} from '@metamask/bitcoindevkit'; +import { Address } from '@metamask/bitcoindevkit'; + +import { + AccountCapability, + WalletError, + type AccountMetadata, + type AccountState, + type BitcoinAccount, + type TransactionBuilder, +} from '../entities'; + +type AccountStateWithMetadata = AccountState & { + metadata: AccountMetadata; +}; + +export class StoredAccountAdapter implements BitcoinAccount { + readonly #id: string; + + readonly #derivationPath: string[]; + + readonly #metadata: AccountMetadata; + + readonly #capabilities: AccountCapability[]; + + constructor(id: string, account: AccountStateWithMetadata) { + this.#id = id; + this.#derivationPath = account.derivationPath; + this.#metadata = account.metadata; + this.#capabilities = Object.values(AccountCapability); + } + + static canLoad(account: AccountState): account is AccountStateWithMetadata { + return Boolean(account.metadata); + } + + static load( + id: string, + account: AccountStateWithMetadata, + ): StoredAccountAdapter { + return new StoredAccountAdapter(id, account); + } + + get id(): string { + return this.#id; + } + + get derivationPath(): string[] { + return this.#derivationPath; + } + + get entropySource(): string { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return this.#derivationPath[0]!; + } + + get accountIndex(): number { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const segment = this.#derivationPath[3]!; + const numericPart = segment.endsWith("'") ? segment.slice(0, -1) : segment; + return Number(numericPart); + } + + get balance(): Balance { + return this.#unsupported(); + } + + get addressType(): AddressType { + return this.#metadata.addressType; + } + + get network(): Network { + return this.#metadata.network; + } + + get publicAddress(): Address { + return Address.from_string(this.#metadata.address, this.#metadata.network); + } + + get publicDescriptor(): string { + return this.#metadata.publicDescriptor; + } + + get capabilities(): AccountCapability[] { + return this.#capabilities; + } + + peekAddress(_index: number): AddressInfo { + return this.#unsupported(); + } + + nextUnusedAddress(): AddressInfo { + return this.#unsupported(); + } + + revealNextAddress(): AddressInfo { + return this.#unsupported(); + } + + startFullScan(): FullScanRequest { + return this.#unsupported(); + } + + startSync(): SyncRequest { + return this.#unsupported(); + } + + applyUpdate(_update: Update): void { + this.#unsupported(); + } + + takeStaged(): ChangeSet | undefined { + return this.#unsupported(); + } + + buildTx(): TransactionBuilder { + return this.#unsupported(); + } + + sign(_psbt: Psbt): Psbt { + return this.#unsupported(); + } + + extractTransaction(_psbt: Psbt, _maxFeeRate?: number): Transaction { + return this.#unsupported(); + } + + getUtxo(_outpoint: string): LocalOutput | undefined { + return this.#unsupported(); + } + + listUnspent(): LocalOutput[] { + return this.#unsupported(); + } + + listTransactions(): WalletTx[] { + return this.#unsupported(); + } + + getTransaction(_txid: string): WalletTx | undefined { + return this.#unsupported(); + } + + calculateFee(_tx: Transaction): Amount { + return this.#unsupported(); + } + + isMine(_script: ScriptBuf): boolean { + return this.#unsupported(); + } + + sentAndReceived(_tx: Transaction): [Amount, Amount] { + return this.#unsupported(); + } + + applyUnconfirmedTx(_tx: Transaction, _lastSeen: number): void { + this.#unsupported(); + } + + #unsupported(): never { + throw new WalletError( + 'Stored account metadata cannot be used for wallet operations; load the full account first.', + { id: this.#id }, + ); + } +} diff --git a/packages/snap/src/infra/index.ts b/packages/snap/src/infra/index.ts index dbc4a2f30..fa4ce4fd2 100644 --- a/packages/snap/src/infra/index.ts +++ b/packages/snap/src/infra/index.ts @@ -1,4 +1,5 @@ export * from './BdkAccountAdapter'; +export * from './StoredAccountAdapter'; export * from './SnapClientAdapter'; export * from './EsploraClientAdapter'; export * from './PriceApiClientAdapter'; diff --git a/packages/snap/src/store/BdkAccountRepository.test.ts b/packages/snap/src/store/BdkAccountRepository.test.ts index da29f74ab..c6c684f24 100644 --- a/packages/snap/src/store/BdkAccountRepository.test.ts +++ b/packages/snap/src/store/BdkAccountRepository.test.ts @@ -3,6 +3,7 @@ import type { DescriptorPair } from '@metamask/bitcoindevkit'; import { + Address, ChangeSet, xpriv_to_descriptor, xpub_to_descriptor, @@ -26,6 +27,9 @@ jest.mock('@metamask/bitcoindevkit', () => { ChangeSet: { from_json: jest.fn(), }, + Address: { + from_string: jest.fn(), + }, slip10_to_extended: jest.fn().mockReturnValue('mock-extended'), xpub_to_descriptor: jest.fn(), xpriv_to_descriptor: jest.fn(), @@ -57,11 +61,16 @@ describe('BdkAccountRepository', () => { derivationPath: mockDerivationPath, }); const mockChangeSet = mock(); + const mockAddress = mock
({ + toString: () => 'bc1qaddress...', + }); const mockAccount = mock({ id: 'some-id', derivationPath: mockDerivationPath, network: 'bitcoin', addressType: 'p2wpkh', + publicAddress: mockAddress, + publicDescriptor: 'mock-public-descriptor', }); const repo = new BdkAccountRepository(mockSnapClient); @@ -74,6 +83,7 @@ describe('BdkAccountRepository', () => { mockSnapClient.getPublicEntropy.mockResolvedValue(mockSlip10Node); (xpriv_to_descriptor as jest.Mock).mockReturnValue(mockDescriptors); (xpub_to_descriptor as jest.Mock).mockReturnValue(mockDescriptors); + jest.mocked(Address.from_string).mockReturnValue(mockAddress); (mockAccount.takeStaged as jest.Mock) = jest .fn() .mockReturnValue(mockChangeSet); @@ -233,6 +243,40 @@ describe('BdkAccountRepository', () => { expect(mockSnapClient.setState).not.toHaveBeenCalled(); }); + it('uses cached account metadata without loading BDK wallets', async () => { + const accountStateWithMetadata: AccountState = { + ...accountState1, + metadata: { + address: 'bc1qcached...', + addressType: 'p2wpkh', + network: 'bitcoin', + publicDescriptor: 'cached-public-descriptor', + }, + }; + mockSnapClient.getState + .mockResolvedValueOnce({ + "m/84'/0'/1'": 'some-id-1', + }) + .mockResolvedValueOnce({ + 'some-id-1': accountStateWithMetadata, + }); + (BdkAccountAdapter.load as jest.Mock).mockClear(); + (ChangeSet.from_json as jest.Mock).mockClear(); + + const result = await repo.getByDerivationPaths([derivationPath1]); + const account = result[0]; + + expect(account?.id).toBe('some-id-1'); + expect(account?.publicAddress.toString()).toBe('bc1qaddress...'); + expect(account?.publicDescriptor).toBe('cached-public-descriptor'); + expect(jest.mocked(Address.from_string)).toHaveBeenCalledWith( + 'bc1qcached...', + 'bitcoin', + ); + expect(ChangeSet.from_json).not.toHaveBeenCalled(); + expect(BdkAccountAdapter.load).not.toHaveBeenCalled(); + }); + it('repairs missing derivation path indexes from account state', async () => { mockSnapClient.getState .mockResolvedValueOnce({ @@ -341,6 +385,12 @@ describe('BdkAccountRepository', () => { wallet: mockWalletData, inscriptions: [], derivationPath: mockDerivationPath, + metadata: { + address: 'bc1qaddress...', + addressType: 'p2wpkh', + network: 'bitcoin', + publicDescriptor: 'mock-public-descriptor', + }, }, ); }); @@ -380,9 +430,17 @@ describe('BdkAccountRepository', () => { const account1 = mock(); account1.id = 'some-id-1'; account1.derivationPath = ['m', "84'", "0'", "1'"]; + account1.network = 'bitcoin'; + account1.addressType = 'p2wpkh'; + account1.publicAddress = mockAddress; + account1.publicDescriptor = 'mock-public-descriptor-1'; const account2 = mock(); account2.id = 'some-id-2'; account2.derivationPath = ['m', "84'", "0'", "2'"]; + account2.network = 'bitcoin'; + account2.addressType = 'p2wpkh'; + account2.publicAddress = mockAddress; + account2.publicDescriptor = 'mock-public-descriptor-2'; (account1.takeStaged as jest.Mock) = jest .fn() .mockReturnValue(mockChangeSet); @@ -406,11 +464,23 @@ describe('BdkAccountRepository', () => { wallet: mockWalletData, inscriptions: [], derivationPath: ['m', "84'", "0'", "1'"], + metadata: { + address: 'bc1qaddress...', + addressType: 'p2wpkh', + network: 'bitcoin', + publicDescriptor: 'mock-public-descriptor-1', + }, }, 'some-id-2': { wallet: mockWalletData, inscriptions: [], derivationPath: ['m', "84'", "0'", "2'"], + metadata: { + address: 'bc1qaddress...', + addressType: 'p2wpkh', + network: 'bitcoin', + publicDescriptor: 'mock-public-descriptor-2', + }, }, }); expect(mockSnapClient.setState).toHaveBeenNthCalledWith( diff --git a/packages/snap/src/store/BdkAccountRepository.ts b/packages/snap/src/store/BdkAccountRepository.ts index 8f38ac71c..fd98c627c 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -16,10 +16,11 @@ import { type SnapClient, type Inscription, type AccountState, + type AccountMetadata, type SnapState, StorageError, } from '../entities'; -import { BdkAccountAdapter } from '../infra'; +import { BdkAccountAdapter, StoredAccountAdapter } from '../infra'; import { logExecutionTime } from '../utils/performance'; /** @@ -99,6 +100,36 @@ function formatAccountCreationOperation( return `${operation} (${details})`; } +/** + * @param account - Account to persist. + * @returns Metadata needed for keyring account responses. + */ +function getAccountMetadata(account: BitcoinAccount): AccountMetadata { + return { + address: account.publicAddress.toString(), + addressType: account.addressType, + network: account.network, + publicDescriptor: account.publicDescriptor, + }; +} + +/** + * @param account - Account to persist. + * @param walletData - Serialized wallet data. + * @returns Account state with cached response metadata. + */ +function getAccountState( + account: BitcoinAccount, + walletData: ChangeSet, +): AccountState { + return { + wallet: walletData.to_json(), + inscriptions: [], + derivationPath: account.derivationPath, + metadata: getAccountMetadata(account), + }; +} + export class BdkAccountRepository implements BitcoinAccountRepository { readonly #snapClient: SnapClient; @@ -195,7 +226,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { const indexedAccount = indexedId ? accountsById[indexedId] : null; if (indexedId && indexedAccount) { - return this.#loadAccount(indexedId, indexedAccount); + return this.#loadPersistedAccount(indexedId, indexedAccount); } const fallback = accountsByDerivationPath.get(pathKey); @@ -205,7 +236,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { const [id, account] = fallback; repairs[pathKey] = id; - return this.#loadAccount(id, account); + return this.#loadPersistedAccount(id, account); }); if (Object.keys(repairs).length > 0) { @@ -357,11 +388,10 @@ export class BdkAccountRepository implements BitcoinAccountRepository { `derivationPaths.${getDerivationPathKey(derivationPath)}`, id, ), - this.#snapClient.setState(`accounts.${id}`, { - wallet: walletData.to_json(), - inscriptions: [], - derivationPath, - }), + this.#snapClient.setState( + `accounts.${id}`, + getAccountState(account, walletData), + ), ]); logExecutionTime( formatAccountCreationOperation( @@ -415,14 +445,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { ); } - accountStateEntries.push([ - id, - { - wallet: walletData.to_json(), - inscriptions: [], - derivationPath, - }, - ]); + accountStateEntries.push([id, getAccountState(account, walletData)]); derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); } logExecutionTime( @@ -571,4 +594,12 @@ export class BdkAccountRepository implements BitcoinAccountRepository { ChangeSet.from_json(account.wallet), ); } + + #loadPersistedAccount(id: string, account: AccountState): BitcoinAccount { + if (StoredAccountAdapter.canLoad(account)) { + return StoredAccountAdapter.load(id, account); + } + + return this.#loadAccount(id, account); + } } diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 0189753e1..f022f9eb7 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -49,7 +49,8 @@ export type CreateAccountParams = DiscoverAccountParams & { accountName?: string; }; -const CREATE_ACCOUNTS_CONCURRENCY = 4; +// Snap entropy derivation can become very spiky under wider parallelism. +const CREATE_ACCOUNTS_CONCURRENCY = 2; /** * @param req - Account creation or discovery request. From 947d0ea2b38e14a824f3fa068f505eb0bb34f6dd Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Tue, 12 May 2026 21:23:10 -0400 Subject: [PATCH 26/26] chore: remove logs --- .../snap/src/handlers/KeyringHandler.test.ts | 76 ---- packages/snap/src/handlers/KeyringHandler.ts | 282 +----------- .../snap/src/store/BdkAccountRepository.ts | 420 +++++------------- .../snap/src/use-cases/AccountUseCases.ts | 280 ++++-------- packages/snap/src/utils/performance.ts | 8 - 5 files changed, 198 insertions(+), 868 deletions(-) delete mode 100644 packages/snap/src/utils/performance.ts diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index e8f955e1f..2993dcf80 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -22,7 +22,6 @@ import { BtcMethod, BtcScope, } from '@metamask/keyring-api'; -import type { Json } from '@metamask/snaps-sdk'; import { mock } from 'jest-mock-extended'; import { assert } from 'superstruct'; @@ -630,81 +629,6 @@ describe('KeyringHandler', () => { ).toStrictEqual(Array.from({ length: 101 }, (_, index) => index)); }); - it('logs a final snap state verification summary', async () => { - const consoleLog = jest.spyOn(console, 'log').mockImplementation(); - const rootState = { accounts: true, derivationPaths: true } as Json; - const accountsState = { - 'id-0': { - wallet: '{}', - inscriptions: [], - derivationPath: [entropySource, "84'", "0'", "0'"], - }, - 'id-1': { - wallet: '{}', - inscriptions: [], - derivationPath: [entropySource, "84'", "0'", "1'"], - }, - } as Json; - const derivationPathsState = { - [`${entropySource}/84'/0'/0'`]: 'id-0', - [`${entropySource}/84'/0'/1'`]: 'id-1', - [`${entropySource}/84'/0'/99'`]: 'missing-id', - } as Json; - mockAccounts.createMany.mockResolvedValue( - [0, 1, 2].map(buildMockAccount), - ); - mockSnapClient.getState - .mockResolvedValueOnce(rootState) - .mockResolvedValueOnce(accountsState) - .mockResolvedValueOnce(derivationPathsState); - - try { - await handler.createAccounts({ - type: AccountCreationType.Bip44DeriveIndexRange, - range: { from: 0, to: 2 }, - entropySource, - }); - - const stateLog = consoleLog.mock.calls.at(-1)?.[0] as - | string - | undefined; - - expect(stateLog).toBeDefined(); - expect(expectDefined(stateLog)).toContain( - '[SNAP STATE DEBUG - BITCOIN SNAP]', - ); - const summary = JSON.parse( - expectDefined(stateLog).slice(expectDefined(stateLog).indexOf('{')), - ); - expect(summary).toStrictEqual( - expect.objectContaining({ - accountsCount: 2, - derivationPathsCount: 3, - entropyId: entropySource, - entropyAccountsCount: 2, - entropyDerivationPathsCount: 3, - entropyMinIndex: 0, - entropyMaxIndex: 1, - requestedRange: { - from: 0, - to: 2, - count: 3, - persistedCount: 2, - missingCount: 1, - missingSample: [2], - missingSampleLimit: 25, - }, - consistency: { - orphanedDerivationPathCount: 1, - missingDerivationPathIndexCount: 0, - }, - }), - ); - } finally { - consoleLog.mockRestore(); - } - }); - it('rejects unsupported creation types', async () => { await expect( handler.createAccounts({ diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index b136b37df..c3596ce51 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -1,4 +1,4 @@ -import type { AddressType, Network } from '@metamask/bitcoindevkit'; +import type { AddressType } from '@metamask/bitcoindevkit'; import { AccountCreationType, assertCreateAccountOptionIsSupported, @@ -48,13 +48,10 @@ import { } from 'superstruct'; import { - addressTypeToPurpose, - type AccountState, type BitcoinAccount, FormatError, InexistentMethodError, type Logger, - networkToCoinType, networkToCurrencyUnit, Purpose, purposeToAddressType, @@ -79,7 +76,6 @@ import type { AccountUseCases, CreateAccountParams, } from '../use-cases/AccountUseCases'; -import { logExecutionTime } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export const CreateAccountRequest = object({ @@ -96,204 +92,6 @@ export const CreateAccountRequest = object({ /** Maximum number of accounts to create in one internal createMany call. */ const MAX_CREATE_ACCOUNTS_PER_BATCH = 100; -const MAX_MISSING_INDEXES_TO_LOG = 25; - -/** - * @param operation - Base operation name. - * @param details - Account creation context to append. - * @returns Operation name with account creation context. - */ -function formatAccountCreationOperation( - operation: string, - details: string, -): string { - return `${operation} (${details})`; -} - -type CreateAccountsStateLogDetails = { - entropySource: string; - from: number; - to: number; - network: Network; - addressType: AddressType; - label: string; -}; - -/** - * @param value - JSON value to coerce into an object record. - * @returns Object record or an empty object. - */ -function getJsonRecord(value: Json | null): Record { - if (!value || typeof value !== 'object' || Array.isArray(value)) { - return {}; - } - - return value as Record; -} - -/** - * @param accounts - Account state namespace. - * @returns Non-null account state entries. - */ -function getAccountStateEntries( - accounts: Json | null, -): [string, AccountState][] { - return Object.entries(getJsonRecord(accounts)).flatMap(([id, account]) => { - if (!account || typeof account !== 'object' || Array.isArray(account)) { - return []; - } - - return [[id, account as unknown as AccountState]]; - }); -} - -/** - * @param derivationPaths - Derivation path state namespace. - * @returns Derivation path entries whose account IDs are strings. - */ -function getDerivationPathEntries( - derivationPaths: Json | null, -): [string, string][] { - return Object.entries(getJsonRecord(derivationPaths)).flatMap(([path, id]) => - typeof id === 'string' ? [[path, id]] : [], - ); -} - -/** - * @param derivationPath - Split derivation path. - * @returns Storage key for a derivation path. - */ -function getDerivationPathKey(derivationPath: string[]): string { - return derivationPath.join('/'); -} - -/** - * @param entropySource - Account entropy source. - * @param addressType - Account address type. - * @param network - Bitcoin network. - * @param index - Account index. - * @returns Expected derivation path storage key. - */ -function getAccountDerivationPathKey( - entropySource: string, - addressType: AddressType, - network: Network, - index: number, -): string { - return [ - entropySource, - `${addressTypeToPurpose[addressType]}'`, - `${networkToCoinType[network]}'`, - `${index}'`, - ].join('/'); -} - -/** - * @param derivationPath - Split derivation path. - * @returns Account index or null when it cannot be parsed. - */ -function getAccountIndex(derivationPath: string[]): number | null { - const segment = derivationPath[3]; - if (!segment) { - return null; - } - - const numericPart = segment.endsWith("'") ? segment.slice(0, -1) : segment; - const index = Number(numericPart); - return Number.isSafeInteger(index) ? index : null; -} - -/** - * @param details - Account creation context. - * @param state - Snap state snapshot. - * @param state.root - Root Snap state. - * @param state.accounts - Account state namespace. - * @param state.derivationPaths - Derivation-path index namespace. - * @returns Snapshot verification log line. - */ -function formatCreateAccountsStateLog( - details: CreateAccountsStateLogDetails, - state: { - root: Json | null; - accounts: Json | null; - derivationPaths: Json | null; - }, -): string { - const accountEntries = getAccountStateEntries(state.accounts); - const accountsById = new Map(accountEntries); - const derivationPathEntries = getDerivationPathEntries(state.derivationPaths); - const derivationPathsByPath = new Map(derivationPathEntries); - const requestedCount = details.to - details.from + 1; - const missingIndexes: number[] = []; - let persistedRequestedCount = 0; - - for (let index = details.from; index <= details.to; index += 1) { - const pathKey = getAccountDerivationPathKey( - details.entropySource, - details.addressType, - details.network, - index, - ); - const accountId = derivationPathsByPath.get(pathKey); - - if (accountId && accountsById.has(accountId)) { - persistedRequestedCount += 1; - } else if (missingIndexes.length < MAX_MISSING_INDEXES_TO_LOG) { - missingIndexes.push(index); - } - } - - const entropyAccountIndexes = accountEntries - .flatMap(([, account]) => { - if (account.derivationPath[0] !== details.entropySource) { - return []; - } - - const index = getAccountIndex(account.derivationPath); - return index === null ? [] : [index]; - }) - .sort((left, right) => left - right); - const entropyDerivationPathsCount = derivationPathEntries.filter(([path]) => - path.startsWith(`${details.entropySource}/`), - ).length; - const orphanedDerivationPathCount = derivationPathEntries.filter( - ([, id]) => !accountsById.has(id), - ).length; - const missingDerivationPathIndexCount = accountEntries.filter( - ([id, account]) => - derivationPathsByPath.get( - getDerivationPathKey(account.derivationPath), - ) !== id, - ).length; - - return `[SNAP STATE DEBUG - BITCOIN SNAP] keyring_createAccounts final state (${ - details.label - }) ${JSON.stringify({ - rootKeys: Object.keys(getJsonRecord(state.root)).sort(), - accountsCount: accountEntries.length, - derivationPathsCount: derivationPathEntries.length, - entropyId: details.entropySource, - entropyAccountsCount: entropyAccountIndexes.length, - entropyDerivationPathsCount, - entropyMinIndex: entropyAccountIndexes[0] ?? null, - entropyMaxIndex: - entropyAccountIndexes[entropyAccountIndexes.length - 1] ?? null, - requestedRange: { - from: details.from, - to: details.to, - count: requestedCount, - persistedCount: persistedRequestedCount, - missingCount: requestedCount - persistedRequestedCount, - missingSample: missingIndexes, - missingSampleLimit: MAX_MISSING_INDEXES_TO_LOG, - }, - consistency: { - orphanedDerivationPathCount, - missingDerivationPathIndexCount, - }, - })}`; -} - export class KeyringHandler implements Keyring { readonly #accountsUseCases: AccountUseCases; @@ -508,7 +306,6 @@ export class KeyringHandler implements Keyring { async createAccounts( options: CreateAccountOptions, ): Promise { - const start = Date.now(); assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, @@ -538,23 +335,10 @@ export class KeyringHandler implements Keyring { ); } - const batchSize = range.to - range.from + 1; - const indexLabel = - range.from === range.to ? `${range.from}` : `${range.from}-${range.to}`; - const accountCreationDetails = `entropyId=${entropySource} indexes=${indexLabel} count=${batchSize}`; - // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring // the defaults used by `createAccount` when no scope is provided. const network = scopeToNetwork[BtcScope.Mainnet]; const addressType = this.#defaultAddressType; - const stateLogDetails: CreateAccountsStateLogDetails = { - entropySource, - from: range.from, - to: range.to, - network, - addressType, - label: accountCreationDetails, - }; if (addressType !== 'p2wpkh') { throw new FormatError( 'Only native segwit (P2WPKH) addresses are supported', @@ -576,7 +360,6 @@ export class KeyringHandler implements Keyring { // `AccountUseCases.createMany` is idempotent: if an account already exists // for the resolved derivation path, it will be returned as-is. - const createManyStart = Date.now(); const accounts: BitcoinAccount[] = []; let chunkFrom = range.from; @@ -585,10 +368,6 @@ export class KeyringHandler implements Keyring { chunkFrom + MAX_CREATE_ACCOUNTS_PER_BATCH - 1, range.to, ); - const chunkSize = chunkTo - chunkFrom + 1; - const chunkIndexLabel = - chunkFrom === chunkTo ? `${chunkFrom}` : `${chunkFrom}-${chunkTo}`; - const chunkDetails = `entropyId=${entropySource} indexes=${chunkIndexLabel} count=${chunkSize}`; const chunkRequests: CreateAccountParams[] = []; for (let index = chunkFrom; index <= chunkTo; index += 1) { @@ -601,17 +380,9 @@ export class KeyringHandler implements Keyring { }); } - const createManyChunkStart = Date.now(); accounts.push( ...(await this.#accountsUseCases.createMany(chunkRequests)), ); - logExecutionTime( - formatAccountCreationOperation( - 'keyring_createAccounts createMany batch', - chunkDetails, - ), - createManyChunkStart, - ); if (chunkTo === range.to) { break; @@ -619,24 +390,7 @@ export class KeyringHandler implements Keyring { chunkFrom = chunkTo + 1; } - logExecutionTime( - formatAccountCreationOperation( - 'keyring_createAccounts createMany', - accountCreationDetails, - ), - createManyStart, - ); - - const responseMappingStart = Date.now(); - const result = accounts.map(mapToKeyringAccount); - logExecutionTime( - formatAccountCreationOperation( - 'keyring_createAccounts response mapping', - accountCreationDetails, - ), - responseMappingStart, - ); - return result; + return accounts.map(mapToKeyringAccount); } finally { if (traceStarted) { await runSnapActionSafely( @@ -645,41 +399,9 @@ export class KeyringHandler implements Keyring { 'endTrace', ); } - logExecutionTime( - formatAccountCreationOperation( - 'keyring_createAccounts', - accountCreationDetails, - ), - start, - ); - await this.#logCreateAccountsState(stateLogDetails); } } - async #logCreateAccountsState( - details: CreateAccountsStateLogDetails, - ): Promise { - await runSnapActionSafely( - async () => { - const [root, accounts, derivationPaths] = await Promise.all([ - this.#snapClient.getState(), - this.#snapClient.getState('accounts'), - this.#snapClient.getState('derivationPaths'), - ]); - - console.log( - formatCreateAccountsStateLog(details, { - root, - accounts, - derivationPaths, - }), - ); - }, - this.#logger, - 'logCreateAccountsState', - ); - } - async discoverAccounts( scopes: BtcScope[], entropySource: string, diff --git a/packages/snap/src/store/BdkAccountRepository.ts b/packages/snap/src/store/BdkAccountRepository.ts index fd98c627c..7eab93b31 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -21,7 +21,6 @@ import { StorageError, } from '../entities'; import { BdkAccountAdapter, StoredAccountAdapter } from '../infra'; -import { logExecutionTime } from '../utils/performance'; /** * Encode a fingerprint to a 4-bytes hex-string (required by the BDK). @@ -44,62 +43,6 @@ function getDerivationPathKey(derivationPath: string[]): string { return derivationPath.join('/'); } -/** - * @param derivationPath - Account derivation path. - * @returns Account creation context for performance logs. - */ -function formatAccountCreationDetails(derivationPath: string[]): string { - const entropyId = derivationPath[0] ?? 'unknown'; - const accountIndexSegment = derivationPath[3] ?? 'unknown'; - const accountIndex = accountIndexSegment.endsWith("'") - ? accountIndexSegment.slice(0, -1) - : accountIndexSegment; - - return `entropyId=${entropyId} index=${accountIndex} derivationPath=${getDerivationPathKey( - derivationPath, - )}`; -} - -/** - * @param derivationPaths - Account derivation paths. - * @returns Batch account creation context for performance logs. - */ -function formatAccountCreationBatchDetails( - derivationPaths: readonly string[][], -): string { - const entropyIds = [ - ...new Set( - derivationPaths.map((derivationPath) => derivationPath[0] ?? 'unknown'), - ), - ] - .sort() - .join(','); - const indices = derivationPaths - .map((derivationPath) => { - const segment = derivationPath[3] ?? 'unknown'; - return segment.endsWith("'") ? segment.slice(0, -1) : segment; - }) - .sort((left, right) => Number(left) - Number(right)); - const firstIndex = indices[0] ?? 'unknown'; - const lastIndex = indices[indices.length - 1] ?? 'unknown'; - const indexLabel = - firstIndex === lastIndex ? `${firstIndex}` : `${firstIndex}-${lastIndex}`; - - return `entropyId=${entropyIds} indexes=${indexLabel} count=${derivationPaths.length}`; -} - -/** - * @param operation - Base operation name. - * @param details - Account creation context to append. - * @returns Operation name with account creation context. - */ -function formatAccountCreationOperation( - operation: string, - details: string, -): string { - return `${operation} (${details})`; -} - /** * @param account - Account to persist. * @returns Metadata needed for keyring account responses. @@ -177,93 +120,60 @@ export class BdkAccountRepository implements BitcoinAccountRepository { async getByDerivationPaths( derivationPaths: string[][], ): Promise<(BitcoinAccount | null)[]> { - const start = Date.now(); - const batchDetails = formatAccountCreationBatchDetails(derivationPaths); - if (derivationPaths.length === 0) { - logExecutionTime('BdkAccountRepository.getByDerivationPaths', start); return []; } - try { - const loadStateStart = Date.now(); - const [derivationPathIndex, accounts] = await Promise.all([ - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - ]); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.getByDerivationPaths load state', - batchDetails, - ), - loadStateStart, - ); - - const accountsById = accounts ?? {}; - const existingDerivationPathIndex = derivationPathIndex ?? {}; - const accountsByDerivationPath = new Map< - string, - [string, AccountState] - >(); - - for (const [id, account] of Object.entries(accountsById)) { - if (account) { - accountsByDerivationPath.set( - getDerivationPathKey(account.derivationPath), - [id, account], - ); - } - } + const [derivationPathIndex, accounts] = await Promise.all([ + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + ]); - const repairs: Record = {}; - const results = derivationPaths.map((derivationPath) => { - const pathKey = getDerivationPathKey(derivationPath); - const indexedId = existingDerivationPathIndex[pathKey]; - const indexedAccount = indexedId ? accountsById[indexedId] : null; + const accountsById = accounts ?? {}; + const existingDerivationPathIndex = derivationPathIndex ?? {}; + const accountsByDerivationPath = new Map(); - if (indexedId && indexedAccount) { - return this.#loadPersistedAccount(indexedId, indexedAccount); - } + for (const [id, account] of Object.entries(accountsById)) { + if (account) { + accountsByDerivationPath.set( + getDerivationPathKey(account.derivationPath), + [id, account], + ); + } + } - const fallback = accountsByDerivationPath.get(pathKey); - if (!fallback) { - return null; - } + const repairs: Record = {}; + const results = derivationPaths.map((derivationPath) => { + const pathKey = getDerivationPathKey(derivationPath); + const indexedId = existingDerivationPathIndex[pathKey]; + const indexedAccount = indexedId ? accountsById[indexedId] : null; - const [id, account] = fallback; - repairs[pathKey] = id; - return this.#loadPersistedAccount(id, account); - }); + if (indexedId && indexedAccount) { + return this.#loadPersistedAccount(indexedId, indexedAccount); + } - if (Object.keys(repairs).length > 0) { - const repairIndexStart = Date.now(); - await this.#snapClient.setState('derivationPaths', { - ...existingDerivationPathIndex, - ...repairs, - }); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.getByDerivationPaths repair index', - batchDetails, - ), - repairIndexStart, - ); + const fallback = accountsByDerivationPath.get(pathKey); + if (!fallback) { + return null; } - return results; - } finally { - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.getByDerivationPaths', - batchDetails, - ), - start, - ); + const [id, account] = fallback; + repairs[pathKey] = id; + return this.#loadPersistedAccount(id, account); + }); + + if (Object.keys(repairs).length > 0) { + await this.#snapClient.setState('derivationPaths', { + ...existingDerivationPathIndex, + ...repairs, + }); } + + return results; } async getWithSigner(id: string): Promise { @@ -307,208 +217,92 @@ export class BdkAccountRepository implements BitcoinAccountRepository { network: Network, addressType: AddressType, ): Promise { - const start = Date.now(); - const accountDetails = formatAccountCreationDetails(derivationPath); - - try { - const entropyStart = Date.now(); - const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.create get public entropy', - accountDetails, - ), - entropyStart, - ); + const slip10 = await this.#snapClient.getPublicEntropy(derivationPath); + const id = v4(); + const fingerprint = toBdkFingerprint( + slip10.masterFingerprint ?? slip10.parentFingerprint, + ); - const buildWalletStart = Date.now(); - const id = v4(); - const fingerprint = toBdkFingerprint( - slip10.masterFingerprint ?? slip10.parentFingerprint, - ); + const xpub = slip10_to_extended(slip10, network); + const descriptors = xpub_to_descriptor( + xpub, + fingerprint, + network, + addressType, + ); - const xpub = slip10_to_extended(slip10, network); - const descriptors = xpub_to_descriptor( - xpub, - fingerprint, - network, - addressType, - ); + return BdkAccountAdapter.create(id, derivationPath, descriptors, network); + } - const account = BdkAccountAdapter.create( - id, - derivationPath, - descriptors, - network, - ); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.create build wallet', - accountDetails, - ), - buildWalletStart, - ); - return account; - } finally { - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.create', - accountDetails, - ), - start, + async insert(account: BitcoinAccount): Promise { + const { id, derivationPath } = account; + const walletData = account.takeStaged(); + if (!walletData) { + throw new StorageError( + `Missing changeset data for account "${id}" for insertion.`, ); } + + await Promise.all([ + this.#snapClient.setState( + `derivationPaths.${getDerivationPathKey(derivationPath)}`, + id, + ), + this.#snapClient.setState( + `accounts.${id}`, + getAccountState(account, walletData), + ), + ]); + + return account; } - async insert(account: BitcoinAccount): Promise { - const start = Date.now(); - const accountDetails = formatAccountCreationDetails(account.derivationPath); + async insertMany(accounts: BitcoinAccount[]): Promise { + if (accounts.length === 0) { + return []; + } - try { - const { id, derivationPath } = account; + if (accounts.length === 1) { + return [await this.insert(accounts[0] as BitcoinAccount)]; + } + + const accountStateEntries: [string, AccountState][] = []; + const derivationPathEntries: [string, string][] = []; - const serializeStart = Date.now(); + for (const account of accounts) { + const { id, derivationPath } = account; const walletData = account.takeStaged(); + if (!walletData) { throw new StorageError( `Missing changeset data for account "${id}" for insertion.`, ); } - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insert serialize account', - accountDetails, - ), - serializeStart, - ); - const writeStateStart = Date.now(); - await Promise.all([ - this.#snapClient.setState( - `derivationPaths.${getDerivationPathKey(derivationPath)}`, - id, - ), - this.#snapClient.setState( - `accounts.${id}`, - getAccountState(account, walletData), - ), - ]); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insert write state', - accountDetails, - ), - writeStateStart, - ); - - return account; - } finally { - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insert', - accountDetails, - ), - start, - ); + accountStateEntries.push([id, getAccountState(account, walletData)]); + derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); } - } - - async insertMany(accounts: BitcoinAccount[]): Promise { - const start = Date.now(); - const batchDetails = - accounts.length > 0 - ? formatAccountCreationBatchDetails( - accounts.map(({ derivationPath }) => derivationPath), - ) - : 'count=0'; - - try { - if (accounts.length === 0) { - return []; - } - - if (accounts.length === 1) { - return [await this.insert(accounts[0] as BitcoinAccount)]; - } - - const accountStateEntries: [string, AccountState][] = []; - const derivationPathEntries: [string, string][] = []; - - const serializeStart = Date.now(); - for (const account of accounts) { - const { id, derivationPath } = account; - const walletData = account.takeStaged(); - if (!walletData) { - throw new StorageError( - `Missing changeset data for account "${id}" for insertion.`, - ); - } - - accountStateEntries.push([id, getAccountState(account, walletData)]); - derivationPathEntries.push([getDerivationPathKey(derivationPath), id]); - } - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insertMany serialize accounts', - batchDetails, - ), - serializeStart, - ); - - const loadStateStart = Date.now(); - const [existingAccounts, existingDerivationPaths] = await Promise.all([ - this.#snapClient.getState('accounts') as Promise< - SnapState['accounts'] | null - >, - this.#snapClient.getState('derivationPaths') as Promise< - SnapState['derivationPaths'] | null - >, - ]); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insertMany load state', - batchDetails, - ), - loadStateStart, - ); + const [existingAccounts, existingDerivationPaths] = await Promise.all([ + this.#snapClient.getState('accounts') as Promise< + SnapState['accounts'] | null + >, + this.#snapClient.getState('derivationPaths') as Promise< + SnapState['derivationPaths'] | null + >, + ]); - const writeAccountsStart = Date.now(); - await this.#snapClient.setState('accounts', { - ...(existingAccounts ?? {}), - ...Object.fromEntries(accountStateEntries), - }); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insertMany write accounts', - batchDetails, - ), - writeAccountsStart, - ); + await this.#snapClient.setState('accounts', { + ...(existingAccounts ?? {}), + ...Object.fromEntries(accountStateEntries), + }); - const writeDerivationPathsStart = Date.now(); - await this.#snapClient.setState('derivationPaths', { - ...(existingDerivationPaths ?? {}), - ...Object.fromEntries(derivationPathEntries), - }); - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insertMany write derivation paths', - batchDetails, - ), - writeDerivationPathsStart, - ); + await this.#snapClient.setState('derivationPaths', { + ...(existingDerivationPaths ?? {}), + ...Object.fromEntries(derivationPathEntries), + }); - return accounts; - } finally { - logExecutionTime( - formatAccountCreationOperation( - 'BdkAccountRepository.insertMany', - batchDetails, - ), - start, - ); - } + return accounts; } async update( diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index f022f9eb7..871247cf4 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -33,7 +33,6 @@ import { WalletError, } from '../entities'; import { CronMethod } from '../handlers/CronHandler'; -import { logExecutionTime } from '../utils/performance'; import { runSnapActionSafely } from '../utils/snapHelpers'; export type DiscoverAccountParams = { @@ -73,41 +72,6 @@ function getDerivationPathKey(derivationPath: string[]): string { return derivationPath.join('/'); } -/** - * @param reqs - Account creation requests. - * @returns Account creation context for performance logs. - */ -function formatAccountCreationBatchDetails( - reqs: readonly CreateAccountParams[], -): string { - const entropyIds = [ - ...new Set(reqs.map(({ entropySource }) => entropySource)), - ] - .sort() - .join(','); - const indices = reqs - .map(({ index }) => index) - .sort((left, right) => left - right); - const firstIndex = indices[0]; - const lastIndex = indices[indices.length - 1]; - const indexLabel = - firstIndex === lastIndex ? `${firstIndex}` : `${firstIndex}-${lastIndex}`; - - return `entropyId=${entropyIds} indexes=${indexLabel} count=${reqs.length}`; -} - -/** - * @param operation - Base operation name. - * @param details - Account creation context to append. - * @returns Operation name with account creation context. - */ -function formatAccountCreationOperation( - operation: string, - details: string, -): string { - return `${operation} (${details})`; -} - /** * Map items to results with at most `concurrency` in-flight async operations. * Output order matches `items` order. @@ -309,177 +273,111 @@ export class AccountUseCases { } async createMany(reqs: CreateAccountParams[]): Promise { - const start = Date.now(); - if (reqs.length === 0) { - logExecutionTime('AccountUseCases.createMany', start); return []; } - const batchDetails = formatAccountCreationBatchDetails(reqs); + const { accounts, createdAccountKeys } = await this.#runAccountMutation( + async () => { + const entries = reqs.map((req, index) => { + const derivationPath = getAccountDerivationPath(req); + return { + req, + index, + derivationPath, + pathKey: getDerivationPathKey(derivationPath), + }; + }); - try { - const { accounts, createdAccountKeys } = await this.#runAccountMutation( - async () => { - const buildEntriesStart = Date.now(); - const entries = reqs.map((req, index) => { - const derivationPath = getAccountDerivationPath(req); - return { - req, - index, - derivationPath, - pathKey: getDerivationPathKey(derivationPath), - }; - }); - - const uniqueEntriesByPath = new Map< - string, - (typeof entries)[number] - >(); - for (const entry of entries) { - if (!uniqueEntriesByPath.has(entry.pathKey)) { - uniqueEntriesByPath.set(entry.pathKey, entry); - } + const uniqueEntriesByPath = new Map(); + for (const entry of entries) { + if (!uniqueEntriesByPath.has(entry.pathKey)) { + uniqueEntriesByPath.set(entry.pathKey, entry); } - const uniqueEntries = [...uniqueEntriesByPath.values()]; - logExecutionTime( - formatAccountCreationOperation( - 'AccountUseCases.createMany build entries', - batchDetails, - ), - buildEntriesStart, - ); + } + const uniqueEntries = [...uniqueEntriesByPath.values()]; - const lookupStart = Date.now(); - const existingAccounts = await this.#repository.getByDerivationPaths( - uniqueEntries.map(({ derivationPath }) => derivationPath), - ); - logExecutionTime( - formatAccountCreationOperation( - 'AccountUseCases.createMany get existing accounts', - batchDetails, - ), - lookupStart, - ); - const existingAccountsByPath = new Map(); + const existingAccounts = await this.#repository.getByDerivationPaths( + uniqueEntries.map(({ derivationPath }) => derivationPath), + ); + const existingAccountsByPath = new Map(); - uniqueEntries.forEach((entry, index) => { - const account = existingAccounts[index]; - if (account && account.network === entry.req.network) { - existingAccountsByPath.set(entry.pathKey, account); - } - }); + uniqueEntries.forEach((entry, index) => { + const account = existingAccounts[index]; + if (account && account.network === entry.req.network) { + existingAccountsByPath.set(entry.pathKey, account); + } + }); - const entriesToCreate = uniqueEntries.filter( - ({ pathKey }) => !existingAccountsByPath.has(pathKey), - ); - const createNewAccountsStart = Date.now(); - const newAccounts = await runWithConcurrencyLimit( - entriesToCreate, - CREATE_ACCOUNTS_CONCURRENCY, - async ({ derivationPath, req }) => { - const newAccount = await this.#repository.create( - derivationPath, - req.network, - req.addressType, - ); - newAccount.revealNextAddress(); - return newAccount; - }, - ); - logExecutionTime( - formatAccountCreationOperation( - 'AccountUseCases.createMany create new accounts', - batchDetails, - ), - createNewAccountsStart, - ); + const entriesToCreate = uniqueEntries.filter( + ({ pathKey }) => !existingAccountsByPath.has(pathKey), + ); + const newAccounts = await runWithConcurrencyLimit( + entriesToCreate, + CREATE_ACCOUNTS_CONCURRENCY, + async ({ derivationPath, req }) => { + const newAccount = await this.#repository.create( + derivationPath, + req.network, + req.addressType, + ); + newAccount.revealNextAddress(); + return newAccount; + }, + ); - const insertNewAccountsStart = Date.now(); - if (newAccounts.length > 0) { - await this.#repository.insertMany(newAccounts); - } - logExecutionTime( - formatAccountCreationOperation( - 'AccountUseCases.createMany insert new accounts', - batchDetails, - ), - insertNewAccountsStart, - ); + if (newAccounts.length > 0) { + await this.#repository.insertMany(newAccounts); + } - const newAccountsByPath = new Map( - entriesToCreate.map((entry, index) => [ - entry.pathKey, - newAccounts[index] as BitcoinAccount, - ]), - ); + const newAccountsByPath = new Map( + entriesToCreate.map((entry, index) => [ + entry.pathKey, + newAccounts[index] as BitcoinAccount, + ]), + ); - const orderAccountsStart = Date.now(); - const accountsInOrder = entries.map((entry) => { - const account = - existingAccountsByPath.get(entry.pathKey) ?? - newAccountsByPath.get(entry.pathKey); - - if (!account) { - throw new AssertionError('Failed to create account', { - index: entry.index, - derivationPath: entry.derivationPath, - }); - } - - return account; - }); - logExecutionTime( - formatAccountCreationOperation( - 'AccountUseCases.createMany order accounts', - batchDetails, - ), - orderAccountsStart, - ); + const accountsInOrder = entries.map((entry) => { + const account = + existingAccountsByPath.get(entry.pathKey) ?? + newAccountsByPath.get(entry.pathKey); - return { - accounts: accountsInOrder, - createdAccountKeys: new Set(newAccountsByPath.keys()), - }; - }, - ); + if (!account) { + throw new AssertionError('Failed to create account', { + index: entry.index, + derivationPath: entry.derivationPath, + }); + } - const scheduleFullScansStart = Date.now(); - const scheduledAccountIds = new Set(); - for (const [index, account] of accounts.entries()) { - const req = reqs[index] as CreateAccountParams; - const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); - if ( - req.synchronize && - createdAccountKeys.has(pathKey) && - !scheduledAccountIds.has(account.id) - ) { - scheduledAccountIds.add(account.id); - await this.#snapClient.scheduleBackgroundEvent({ - duration: 'PT1S', - method: CronMethod.FullScanAccount, - params: { accountId: account.id }, - }); - } - } - logExecutionTime( - formatAccountCreationOperation( - 'AccountUseCases.createMany schedule full scans', - batchDetails, - ), - scheduleFullScansStart, - ); + return account; + }); - return accounts; - } finally { - logExecutionTime( - formatAccountCreationOperation( - 'AccountUseCases.createMany', - batchDetails, - ), - start, - ); + return { + accounts: accountsInOrder, + createdAccountKeys: new Set(newAccountsByPath.keys()), + }; + }, + ); + + const scheduledAccountIds = new Set(); + for (const [index, account] of accounts.entries()) { + const req = reqs[index] as CreateAccountParams; + const pathKey = getDerivationPathKey(getAccountDerivationPath(req)); + if ( + req.synchronize && + createdAccountKeys.has(pathKey) && + !scheduledAccountIds.has(account.id) + ) { + scheduledAccountIds.add(account.id); + await this.#snapClient.scheduleBackgroundEvent({ + duration: 'PT1S', + method: CronMethod.FullScanAccount, + params: { accountId: account.id }, + }); + } } + + return accounts; } async synchronize( diff --git a/packages/snap/src/utils/performance.ts b/packages/snap/src/utils/performance.ts deleted file mode 100644 index 545f6dbf6..000000000 --- a/packages/snap/src/utils/performance.ts +++ /dev/null @@ -1,8 +0,0 @@ -export const logExecutionTime = (operation: string, start: number): void => { - const end = Date.now(); - console.log( - `[PERFORMANCE DEBUG - BITCOIN SNAP] ${operation} took ${ - end - start - } ms to execute`, - ); -};