diff --git a/packages/snap/CHANGELOG.md b/packages/snap/CHANGELOG.md index bf02a33ae..aa75544ff 100644 --- a/packages/snap/CHANGELOG.md +++ b/packages/snap/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `createMany` use case ([#610](https://github.com/MetaMask/snap-bitcoin-wallet/pull/610)) + ### Changed - Show a confirmation dialog before signing a PSBT from KeyringHandler and sending a transfer ([#591](https://github.com/MetaMask/snap-bitcoin-wallet/pull/591)) 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/store/BdkAccountRepository.test.ts b/packages/snap/src/store/BdkAccountRepository.test.ts index a89e12411..da29f74ab 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..3e630dcc8 100644 --- a/packages/snap/src/store/BdkAccountRepository.ts +++ b/packages/snap/src/store/BdkAccountRepository.ts @@ -34,6 +34,30 @@ 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('/'); +} + +/** + * @param account - Account to persist. + * @param walletData - Serialized wallet data. + * @returns Account state. + */ +function getAccountState( + account: BitcoinAccount, + walletData: ChangeSet, +): AccountState { + return { + wallet: walletData.to_json(), + inscriptions: [], + derivationPath: account.derivationPath, + }; +} + export class BdkAccountRepository implements BitcoinAccountRepository { readonly #snapClient: SnapClient; @@ -49,11 +73,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 +84,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 +102,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}`, @@ -148,7 +221,6 @@ export class BdkAccountRepository implements BitcoinAccountRepository { async insert(account: BitcoinAccount): Promise { const { id, derivationPath } = account; - const walletData = account.takeStaged(); if (!walletData) { throw new StorageError( @@ -158,19 +230,66 @@ export class BdkAccountRepository implements BitcoinAccountRepository { await Promise.all([ this.#snapClient.setState( - `derivationPaths.${derivationPath.join('/')}`, + `derivationPaths.${getDerivationPathKey(derivationPath)}`, id, ), - this.#snapClient.setState(`accounts.${id}`, { - wallet: walletData.to_json(), - inscriptions: [], - derivationPath, - }), + this.#snapClient.setState( + `accounts.${id}`, + getAccountState(account, walletData), + ), ]); 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, getAccountState(account, walletData)]); + 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 +365,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 210f9f9c9..1a25b6c91 100644 --- a/packages/snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/snap/src/use-cases/AccountUseCases.test.ts @@ -291,6 +291,146 @@ 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).not.toHaveBeenCalled(); + 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).not.toHaveBeenCalled(); + 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).not.toHaveBeenCalled(); + expect(result).toStrictEqual([existingAccount]); + }); + + it('propagates insertMany errors without emitting account-created 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 345fe472a..871247cf4 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -48,6 +48,83 @@ export type CreateAccountParams = DiscoverAccountParams & { accountName?: string; }; +// Snap entropy derivation can become very spiky under wider parallelism. +const CREATE_ACCOUNTS_CONCURRENCY = 2; + +/** + * @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 +142,8 @@ export class AccountUseCases { readonly #targetBlocksConfirmation: number; + #accountMutationQueue: Promise = Promise.resolve(); + constructor( logger: Logger, snapClient: SnapClient, @@ -109,14 +188,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 +219,165 @@ 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()]; - newAccount.revealNextAddress(); + const existingAccounts = await this.#repository.getByDerivationPaths( + uniqueEntries.map(({ derivationPath }) => derivationPath), + ); + const existingAccountsByPath = new Map(); - await this.#repository.insert(newAccount); + uniqueEntries.forEach((entry, index) => { + const account = existingAccounts[index]; + if (account && account.network === entry.req.network) { + existingAccountsByPath.set(entry.pathKey, account); + } + }); - // First notify the event has been created, then schedule full scan. - await this.#snapClient.emitAccountCreatedEvent( - newAccount, - correlationId, - accountName, + 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; + }, + ); + + if (newAccounts.length > 0) { + await this.#repository.insertMany(newAccounts); + } + + 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); + + 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 }, - }); + 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 }, + }); + } } - this.#logger.info( - 'Bitcoin account created successfully: %s. Public address: %s, Request: %o', - newAccount.id, - newAccount.publicAddress, - req, - ); - return newAccount; + return accounts; } async synchronize( @@ -331,15 +502,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( @@ -680,6 +853,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,