From f2ec677ecb14415d4e8cef3eff221ac08c59ca58 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 3 Sep 2026 16:37:12 +0200 Subject: [PATCH 1/9] feat(bitcoin-wallet-snap): add batch proof-of-ownership signing --- packages/bitcoin-wallet-snap/CHANGELOG.md | 4 + .../bitcoin-wallet-snap/snap.manifest.json | 2 +- .../src/entities/account.ts | 8 + .../src/handlers/RpcHandler.test.ts | 126 ++++++++++ .../src/handlers/RpcHandler.ts | 155 +++++++++++++ .../src/handlers/validation.ts | 5 + .../src/store/BdkAccountRepository.test.ts | 40 ++++ .../src/store/BdkAccountRepository.ts | 18 ++ .../src/use-cases/AccountUseCases.test.ts | 108 ++++++++- .../src/use-cases/AccountUseCases.ts | 215 ++++++++++++++++++ 10 files changed, 679 insertions(+), 2 deletions(-) diff --git a/packages/bitcoin-wallet-snap/CHANGELOG.md b/packages/bitcoin-wallet-snap/CHANGELOG.md index 0d8a862e0..684f91f98 100644 --- a/packages/bitcoin-wallet-snap/CHANGELOG.md +++ b/packages/bitcoin-wallet-snap/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `signProofOfOwnershipBatch` for signing multiple proof-of-ownership messages in one request. ([#XXX](https://github.com/MetaMask/internal-snaps/pull/XXX)) + ### Changed - **BREAKING** Bump `@metamask/keyring-api` from `^23.7.0` to `^24.1.0` ([#214](https://github.com/MetaMask/internal-snaps/pull/214)) diff --git a/packages/bitcoin-wallet-snap/snap.manifest.json b/packages/bitcoin-wallet-snap/snap.manifest.json index 3fd7c0f9d..dcc7a6165 100644 --- a/packages/bitcoin-wallet-snap/snap.manifest.json +++ b/packages/bitcoin-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "N40X9yuzuO3nWtRwrVRhAMQC1+XOcB3Ay3sxYrg2YFA=", + "shasum": "3Msbb6pMl2lUUJnwKtW0ONeFu5n1WTvLuNW2Vc2HW0c=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/bitcoin-wallet-snap/src/entities/account.ts b/packages/bitcoin-wallet-snap/src/entities/account.ts index 8788fb759..c305a590d 100644 --- a/packages/bitcoin-wallet-snap/src/entities/account.ts +++ b/packages/bitcoin-wallet-snap/src/entities/account.ts @@ -265,6 +265,14 @@ export type BitcoinAccountRepository = { */ getAll(): Promise; + /** + * Get accounts by their ids. + * + * @param ids - Account IDs. + * @returns the accounts that exist, in requested order + */ + getByIds(ids: string[]): Promise; + /** * Get an account by its derivation path. * diff --git a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts index 46af2613c..160cd5642 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts @@ -1252,5 +1252,131 @@ describe('RpcHandler', () => { handler.route(origin, buildRequest(message)), ).rejects.toThrow('signer unavailable'); }); + + describe('signProofOfOwnershipBatch', () => { + const secondAccountId = '8eb1f949-c0cc-4f7d-b0ca-880b17f442b3'; + const secondAccountAddress = 'bc1qux9xtsj6mr4un7yg9kgd7tv8kndvlhv2gv5yc8'; + const secondBitcoinAccount = mock({ + id: secondAccountId, + publicAddress: { + toString: () => secondAccountAddress, + } as never, + network: 'bitcoin', + }); + + const buildBatchRequest = ( + items: { accountId: string; message: string }[], + ): JsonRpcRequest => ({ + jsonrpc: '2.0', + id: '1', + method: RpcMethod.SignProofOfOwnershipBatch, + params: { items }, + }); + + it('signs a batch and returns signatures in input order', async () => { + const message1 = `metamask:proof-of-ownership:${nonce}:${accountAddress}`; + const message2 = `metamask:proof-of-ownership:${nonce}:${secondAccountAddress}`; + mockAccountsUseCases.getByIds.mockResolvedValue([ + mockBitcoinAccount, + secondBitcoinAccount, + ]); + mockAccountsUseCases.signProofOfOwnershipMessages.mockResolvedValue([ + { signature: 'mock-bip322-signature-1' }, + { signature: 'mock-bip322-signature-2' }, + ]); + + const result = await handler.route( + origin, + buildBatchRequest([ + { accountId: validAccountId, message: message1 }, + { accountId: secondAccountId, message: message2 }, + ]), + ); + + expect(mockAccountsUseCases.getByIds).toHaveBeenCalledWith([ + validAccountId, + secondAccountId, + ]); + expect( + mockAccountsUseCases.signProofOfOwnershipMessages, + ).toHaveBeenCalledWith([ + { account: mockBitcoinAccount, message: message1 }, + { account: secondBitcoinAccount, message: message2 }, + ]); + expect(result).toStrictEqual({ + results: [ + { + accountId: validAccountId, + signature: 'mock-bip322-signature-1', + }, + { + accountId: secondAccountId, + signature: 'mock-bip322-signature-2', + }, + ], + }); + }); + + it('returns item-level errors for missing accounts and address mismatches', async () => { + const missingAccountId = '6b3df9d2-07fc-4e08-baf9-769254ab3fc8'; + const validMessage = `metamask:proof-of-ownership:${nonce}:${accountAddress}`; + const mismatchedMessage = `metamask:proof-of-ownership:${nonce}:${secondAccountAddress}`; + mockAccountsUseCases.getByIds.mockResolvedValue([mockBitcoinAccount]); + mockAccountsUseCases.signProofOfOwnershipMessages.mockResolvedValue([ + { signature: 'mock-bip322-signature' }, + ]); + + const result = await handler.route( + origin, + buildBatchRequest([ + { accountId: validAccountId, message: validMessage }, + { accountId: missingAccountId, message: validMessage }, + { accountId: validAccountId, message: mismatchedMessage }, + ]), + ); + + expect( + mockAccountsUseCases.signProofOfOwnershipMessages, + ).toHaveBeenCalledTimes(1); + expect(result).toStrictEqual({ + results: [ + { + accountId: validAccountId, + signature: 'mock-bip322-signature', + }, + { + accountId: missingAccountId, + error: `Account not found: ${missingAccountId}`, + }, + { + accountId: validAccountId, + error: `Address in proof-of-ownership message (${secondAccountAddress}) does not match signing account address (${accountAddress})`, + }, + ], + }); + }); + + it('returns item-level errors from batch signing', async () => { + const message = `metamask:proof-of-ownership:${nonce}:${accountAddress}`; + mockAccountsUseCases.getByIds.mockResolvedValue([mockBitcoinAccount]); + mockAccountsUseCases.signProofOfOwnershipMessages.mockResolvedValue([ + { error: 'Failed to get private entropy' }, + ]); + + const result = await handler.route( + origin, + buildBatchRequest([{ accountId: validAccountId, message }]), + ); + + expect(result).toStrictEqual({ + results: [ + { + accountId: validAccountId, + error: 'Failed to get private entropy', + }, + ], + }); + }); + }); }); }); diff --git a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts index dd545ad1d..971702cd4 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts @@ -3,6 +3,7 @@ import type { Json, JsonRpcRequest } from '@metamask/snaps-sdk'; import { Verifier } from 'bip322-js'; import { assert, + array, enums, object, optional, @@ -88,6 +89,44 @@ export const SignProofOfOwnershipRequest = object({ message: string(), }); +/** + * Validates one proof-of-ownership batch request item. + * + * Batch items intentionally validate messages as plain strings so invalid + * proof messages can be reported per item instead of failing the whole batch. + */ +export const SignProofOfOwnershipBatchRequestItem = object({ + accountId: string(), + message: string(), +}); + +/** + * Validates `signProofOfOwnershipBatch` request params. + */ +export const SignProofOfOwnershipBatchRequest = object({ + items: array(SignProofOfOwnershipBatchRequestItem), +}); + +export type SignProofOfOwnershipBatchResponse = { + results: ( + | { accountId: string; signature: string } + | { + accountId: string; + error: string; + } + )[]; +}; + +/** + * Converts an unknown thrown value into a JSON-serializable error message. + * + * @param error - The thrown value. + * @returns A string error message. + */ +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + export class RpcHandler { readonly #logger: Logger; @@ -156,6 +195,10 @@ export class RpcHandler { assert(params, SignProofOfOwnershipRequest); return this.#signProofOfOwnership(params.accountId, params.message); } + case RpcMethod.SignProofOfOwnershipBatch: { + assert(params, SignProofOfOwnershipBatchRequest); + return this.#signProofOfOwnershipBatch(params.items); + } default: throw new InexistentMethodError(`Method not found: ${method}`); @@ -454,4 +497,116 @@ export class RpcHandler { return { signature }; } + + /** + * Handles batch signing of proof-of-ownership messages. + * + * Valid items are signed together so key derivation can be grouped by parent + * path. Invalid items return per-item errors instead of failing the whole + * batch. + * + * @param items - Batch request items. + * @returns One result per item, in input order. + */ + async #signProofOfOwnershipBatch( + items: { accountId: string; message: string }[], + ): Promise { + const uniqueAccountIds = [ + ...new Set(items.map(({ accountId }) => accountId)), + ]; + const allAccounts = await this.#accountUseCases.getByIds(uniqueAccountIds); + const accountsById = new Map( + allAccounts.map((account) => [account.id, account]), + ); + const results: SignProofOfOwnershipBatchResponse['results'] = new Array( + items.length, + ); + const signingRequests: { + index: number; + accountId: string; + account: (typeof allAccounts)[number]; + message: string; + }[] = []; + + items.forEach(({ accountId, message }, index) => { + const account = accountsById.get(accountId); + if (!account) { + results[index] = { + accountId, + error: `Account not found: ${accountId}`, + }; + return; + } + + try { + const { address: messageAddress } = + parseProofOfOwnershipMessage(message); + + const canonicalMessageAddress = + canonicalizeBitcoinAddress(messageAddress); + const canonicalAccountAddress = canonicalizeBitcoinAddress( + account.publicAddress.toString(), + ); + + const addressValidation = validateAddress( + canonicalMessageAddress, + account.network, + this.#logger, + ); + if (!addressValidation.valid) { + results[index] = { + accountId, + error: `Invalid Bitcoin address in proof-of-ownership message for network ${account.network}`, + }; + return; + } + + if (canonicalMessageAddress !== canonicalAccountAddress) { + results[index] = { + accountId, + error: `Address in proof-of-ownership message (${messageAddress}) does not match signing account address (${canonicalAccountAddress})`, + }; + return; + } + + signingRequests.push({ + index, + accountId, + account, + message, + }); + } catch (error) { + results[index] = { + accountId, + error: getErrorMessage(error), + }; + } + }); + + if (signingRequests.length === 0) { + return { results }; + } + + const signedMessages = + await this.#accountUseCases.signProofOfOwnershipMessages( + signingRequests.map(({ account, message }) => ({ account, message })), + ); + + signedMessages.forEach((signedMessage, signingRequestIndex) => { + const { index, accountId } = signingRequests[ + signingRequestIndex + ] as (typeof signingRequests)[number]; + const { error } = signedMessage as { error?: string }; + + if (error !== undefined) { + results[index] = { accountId, error }; + return; + } + + const { signature } = signedMessage as { signature: string }; + results[index] = { accountId, signature }; + }); + + return { results }; + } } diff --git a/packages/bitcoin-wallet-snap/src/handlers/validation.ts b/packages/bitcoin-wallet-snap/src/handlers/validation.ts index 481636377..60ebdf655 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/validation.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/validation.ts @@ -33,6 +33,11 @@ export const RpcMethod = { ConfirmSend: 'confirmSend', SignRewardsMessage: 'signRewardsMessage', SignProofOfOwnership: 'signProofOfOwnership', + /** + * Sign multiple proof-of-ownership messages for MetaMask identity + * authentication. + */ + SignProofOfOwnershipBatch: 'signProofOfOwnershipBatch', } as const; export type RpcMethod = (typeof RpcMethod)[keyof typeof RpcMethod]; diff --git a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts index aeaa8b7a6..e2c1289ea 100644 --- a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts +++ b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts @@ -179,6 +179,46 @@ describe('BdkAccountRepository', () => { }); }); + describe('getByIds', () => { + it('returns empty array if no account IDs are provided', async () => { + const result = await repo.getByIds([]); + + expect(mockSnapClient.getState).not.toHaveBeenCalled(); + expect(result).toStrictEqual([]); + }); + + it('returns requested accounts in requested order', async () => { + const id1 = 'some-id-1'; + const id2 = 'some-id-2'; + const state = { + [id1]: { ...mockAccountState, id: id1 }, + [id2]: { ...mockAccountState, id: id2 }, + }; + const mockAccount1 = { ...mockAccount, id: id1 }; + const mockAccount2 = { ...mockAccount, id: id2 }; + + mockSnapClient.getState.mockResolvedValue(state); + (BdkAccountAdapter.load as jest.Mock) + .mockReturnValueOnce(mockAccount2) + .mockReturnValueOnce(mockAccount1); + + const result = await repo.getByIds([id2, 'missing-id', id1]); + + expect(mockSnapClient.getState).toHaveBeenCalledWith('accounts'); + expect(BdkAccountAdapter.load).toHaveBeenCalledTimes(2); + expect(result).toStrictEqual([mockAccount2, mockAccount1]); + }); + + it('returns empty array if no accounts are found', async () => { + mockSnapClient.getState.mockResolvedValue(null); + + const result = await repo.getByIds(['some-id']); + + expect(mockSnapClient.getState).toHaveBeenCalledWith('accounts'); + expect(result).toStrictEqual([]); + }); + }); + describe('getByDerivationPath', () => { it('returns null if account not found', async () => { mockSnapClient.getState.mockResolvedValue(null); diff --git a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts index efae5c57b..364ebdb63 100644 --- a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts +++ b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts @@ -107,6 +107,24 @@ export class BdkAccountRepository implements BitcoinAccountRepository { ); } + async getByIds(ids: string[]): Promise { + if (ids.length === 0) { + return []; + } + + const accounts = (await this.#snapClient.getState('accounts')) as + | SnapState['accounts'] + | null; + if (!accounts) { + return []; + } + + return ids.flatMap((id) => { + const account = accounts[id]; + return account ? [this.#loadAccount(id, account)] : []; + }); + } + async getByDerivationPath( derivationPath: string[], ): Promise { diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts index 2a94fa259..6469854e9 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts @@ -11,7 +11,12 @@ import type { Psbt, Address, } from '@metamask/bitcoindevkit'; -import type { JsonSLIP10Node } from '@metamask/key-tree'; +import type { BIP32Node, BIP39Node, JsonSLIP10Node } from '@metamask/key-tree'; +import { + mnemonicPhraseToBytes, + SLIP10Node as RealSlip10Node, +} from '@metamask/key-tree'; +import { Signer } from 'bip322-js'; import { mock } from 'jest-mock-extended'; import type { @@ -116,6 +121,28 @@ describe('AccountUseCases', () => { }); }); + describe('getByIds', () => { + it('returns accounts by id', async () => { + const mockAccount = mock(); + + mockRepository.getByIds.mockResolvedValue([mockAccount]); + + const result = await useCases.getByIds(['some-id']); + + expect(mockRepository.getByIds).toHaveBeenCalledWith(['some-id']); + expect(result).toStrictEqual([mockAccount]); + }); + + it('propagates an error if the repository getByIds fails', async () => { + const error = new Error('Get failed'); + mockRepository.getByIds.mockRejectedValue(error); + + await expect(useCases.getByIds(['some-id'])).rejects.toBe(error); + + expect(mockRepository.getByIds).toHaveBeenCalledWith(['some-id']); + }); + }); + describe('createMany', () => { const createParams: CreateAccountParams = { network: 'bitcoin', @@ -1888,4 +1915,83 @@ describe('AccountUseCases', () => { ).not.toHaveBeenCalled(); }); }); + + describe('signProofOfOwnershipMessages', () => { + const mnemonic = + 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'; + const parentPath = ['entropy-1', "84'", "1'"]; + const mockMessage = 'metamask:proof-of-ownership:nonce:bcrt1qaddress'; + + /** + * Derives the real SLIP-10 node for a path from the fixture mnemonic. + * + * @param segments - Hardened path segments below the master node. + * @returns The derived node. + */ + async function deriveFixtureNode( + segments: string[], + ): Promise { + const derivationPath: [BIP39Node, ...BIP32Node[]] = [ + mnemonicPhraseToBytes(mnemonic) as BIP39Node, + ...segments.map((segment) => `bip32:${segment}` as BIP32Node), + ]; + + return RealSlip10Node.fromDerivationPath({ + derivationPath, + curve: 'secp256k1', + }); + } + + const createAccount = (index: number): BitcoinAccount => + mock({ + id: `account-${index}`, + publicAddress: mock
({ + toString: () => 'bcrt1qs2fj7czz0amfm74j73yujx6dn6223md56gkkuy', + }), + capabilities: [AccountCapability.SignMessage], + derivationPath: [...parentPath, `${index}'`], + network: 'regtest', + }); + + beforeEach(async () => { + const parentNode = await deriveFixtureNode(["84'", "1'"]); + mockSnapClient.getPrivateEntropy.mockResolvedValue(parentNode.toJSON()); + }); + + it('signs messages with one private entropy fetch for accounts sharing a parent path', async () => { + jest + .spyOn(Signer, 'sign') + .mockReturnValueOnce('mock-bip322-signature-0') + .mockReturnValueOnce('mock-bip322-signature-1'); + + const result = await useCases.signProofOfOwnershipMessages([ + { account: createAccount(0), message: mockMessage }, + { account: createAccount(1), message: mockMessage }, + ]); + + expect(mockSnapClient.getPrivateEntropy).toHaveBeenCalledTimes(1); + expect(mockSnapClient.getPrivateEntropy).toHaveBeenCalledWith(parentPath); + expect( + mockConfirmationRepository.insertSignMessage, + ).not.toHaveBeenCalled(); + expect(result).toStrictEqual([ + { signature: 'mock-bip322-signature-0' }, + { signature: 'mock-bip322-signature-1' }, + ]); + }); + + it('returns an item-level error when an account cannot sign messages', async () => { + const account = createAccount(0); + account.capabilities = []; + + const result = await useCases.signProofOfOwnershipMessages([ + { account, message: mockMessage }, + ]); + + expect(mockSnapClient.getPrivateEntropy).not.toHaveBeenCalled(); + expect(result).toStrictEqual([ + { error: 'Account missing given capability' }, + ]); + }); + }); }); diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts index e77521f51..b91bc8e92 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts @@ -7,6 +7,8 @@ import type { Txid, WalletTx, } from '@metamask/bitcoindevkit'; +import type { BIP32Node } from '@metamask/key-tree'; +import { SLIP10Node } from '@metamask/key-tree'; import { getCurrentUnixTimestamp } from '@metamask/keyring-snap-sdk'; import { Signer } from 'bip322-js'; import { encode } from 'wif'; @@ -48,6 +50,27 @@ export type CreateAccountParams = DiscoverAccountParams & { accountName?: string; }; +/** + * One proof-of-ownership message signing request. + */ +export type SignProofOfOwnershipMessageBatchRequest = { + /** + * Account whose address should own the BIP-322 signature. + */ + account: BitcoinAccount; + /** + * Plaintext proof-of-ownership message to sign. + */ + message: string; +}; + +/** + * Result for one proof-of-ownership batch signing request. + */ +export type SignProofOfOwnershipMessageBatchResult = + | { signature: string } + | { error: string }; + /** * @param req - Account creation or discovery request. * @returns The BIP-44 account derivation path. @@ -69,6 +92,58 @@ function getDerivationPathKey(derivationPath: string[]): string { return derivationPath.join('/'); } +/** + * Converts an unknown thrown value into a JSON-serializable error message. + * + * @param error - The thrown value. + * @returns A string error message. + */ +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** + * Converts split derivation path segments into key-tree BIP-32 path nodes. + * + * @param segments - Split derivation path segments below a parent node. + * @returns BIP-32 path nodes accepted by key-tree. + */ +function toBip32Path(segments: string[]): BIP32Node[] { + return segments.map((segment) => `bip32:${segment}` as BIP32Node); +} + +/** + * Returns the parent derivation path used for grouped proof signing. + * + * @param account - The account whose account-level derivation path is used. + * @returns The parent path one level above the account index. + */ +function getProofSigningParentPath(account: BitcoinAccount): string[] { + if (account.derivationPath.length === 0) { + throw new Error('Missing account derivation path'); + } + + return account.derivationPath.slice(0, -1); +} + +/** + * Returns the child path from the grouped parent node to the receive address at + * index 0, which is the account public address used for BIP-322 signing. + * + * @param account - The account whose address-0 signing path should be built. + * @returns The child path from parent node to receive address 0. + */ +function getProofSigningChildPath(account: BitcoinAccount): string[] { + const accountIndexSegment = + account.derivationPath[account.derivationPath.length - 1]; + + if (!accountIndexSegment) { + throw new Error('Missing account derivation path'); + } + + return [accountIndexSegment, '0', '0']; +} + /** * Result of broadcasting a Bitcoin transaction. * @@ -140,6 +215,24 @@ export class AccountUseCases { return accounts; } + /** + * Gets accounts by id using one account-map read. + * + * @param ids - Account IDs. + * @returns Existing accounts in requested order. + */ + async getByIds(ids: string[]): Promise { + this.#logger.debug('Fetching accounts: %o', ids); + + const accounts = await this.#repository.getByIds(ids); + + this.#logger.debug( + 'Accounts found: %o', + accounts.map(({ id }) => id), + ); + return accounts; + } + async get(id: string): Promise { this.#logger.debug('Fetching account: %s', id); @@ -680,6 +773,128 @@ export class AccountUseCases { } } + /** + * Signs multiple proof-of-ownership messages without user confirmation. + * + * Requests are grouped by the account-level parent path so the private parent + * node is fetched once per distinct parent and address-0 keys are derived + * locally. Results are returned in input order, with per-item errors for + * accounts that cannot sign or fail derivation/signing. + * + * @param requests - Proof-of-ownership message signing requests. + * @returns One signing result per request, in input order. + */ + async signProofOfOwnershipMessages( + requests: SignProofOfOwnershipMessageBatchRequest[], + ): Promise { + const results: SignProofOfOwnershipMessageBatchResult[] = new Array( + requests.length, + ); + const requestsByParentPath = new Map< + string, + { + index: number; + request: SignProofOfOwnershipMessageBatchRequest; + parentPath: string[]; + }[] + >(); + + requests.forEach((request, index) => { + try { + this.#checkCapability(request.account, AccountCapability.SignMessage); + + const parentPath = getProofSigningParentPath(request.account); + const parentKey = getDerivationPathKey(parentPath); + const parentRequests = requestsByParentPath.get(parentKey) ?? []; + parentRequests.push({ index, request, parentPath }); + requestsByParentPath.set(parentKey, parentRequests); + } catch (error) { + results[index] = { error: getErrorMessage(error) }; + } + }); + + await Promise.all( + [...requestsByParentPath.values()].map(async (parentRequests) => { + const { parentPath } = + parentRequests[0] as (typeof parentRequests)[number]; + + try { + const parentJson = + await this.#snapClient.getPrivateEntropy(parentPath); + const parentNode = await SLIP10Node.fromJSON(parentJson); + + for (const { index, request } of parentRequests) { + try { + const entropy = await parentNode.derive( + toBip32Path(getProofSigningChildPath(request.account)), + ); + + if (!entropy.privateKey) { + throw new AssertionError('Failed to get private entropy', { + id: request.account.id, + }); + } + + results[index] = { + signature: this.#signProofOfOwnershipMessage( + request.account, + request.message, + entropy.privateKey, + ), + }; + } catch (error) { + results[index] = { error: getErrorMessage(error) }; + } + } + } catch (error) { + for (const { index } of parentRequests) { + results[index] = { error: getErrorMessage(error) }; + } + } + }), + ); + + return results; + } + + /** + * Signs one proof-of-ownership message using private key entropy. + * + * @param account - Account whose public address should own the signature. + * @param message - Plaintext proof-of-ownership message. + * @param privateKey - 0x-prefixed private key hex string. + * @returns The BIP-322 signature. + */ + #signProofOfOwnershipMessage( + account: BitcoinAccount, + message: string, + privateKey: string, + ): string { + try { + const wifPrivateKey = encode({ + version: account.network === 'bitcoin' ? 128 : 239, + // eslint-disable-next-line no-restricted-globals + privateKey: Buffer.from(privateKey.slice(2), 'hex'), + compressed: true, + }); + + return Signer.sign( + wifPrivateKey, + account.publicAddress.toString(), + message, + ); + } catch (error) { + throw new WalletError( + 'Failed to sign message', + { + id: account.id, + message, + }, + error, + ); + } + } + async getFrozenUTXOs(accountId: string): Promise { return this.#repository.getFrozenUTXOs(accountId); } From fe3749da54b5b435ef3880fa197dc497fd16750c Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 3 Sep 2026 23:17:45 +0200 Subject: [PATCH 2/9] chore(bitcoin-wallet-snap): add PR number to changelog entry --- packages/bitcoin-wallet-snap/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/bitcoin-wallet-snap/CHANGELOG.md b/packages/bitcoin-wallet-snap/CHANGELOG.md index 684f91f98..9010fbab4 100644 --- a/packages/bitcoin-wallet-snap/CHANGELOG.md +++ b/packages/bitcoin-wallet-snap/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `signProofOfOwnershipBatch` for signing multiple proof-of-ownership messages in one request. ([#XXX](https://github.com/MetaMask/internal-snaps/pull/XXX)) +- Add `signProofOfOwnershipBatch` for signing multiple proof-of-ownership messages in one request. ([#266](https://github.com/MetaMask/internal-snaps/pull/266)) ### Changed From 791919ba23b195bc9468a6cd8114f415fa12ee6d Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 4 Sep 2026 13:59:22 +0200 Subject: [PATCH 3/9] perf(bitcoin-wallet-snap): improve account loading --- .../src/store/BdkAccountRepository.test.ts | 50 +++++++++++++++++++ .../src/store/BdkAccountRepository.ts | 2 +- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts index e2c1289ea..7e79ca843 100644 --- a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts +++ b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts @@ -209,6 +209,56 @@ describe('BdkAccountRepository', () => { expect(result).toStrictEqual([mockAccount2, mockAccount1]); }); + it('uses cached account metadata without loading BDK wallets', async () => { + const id1 = 'some-id-1'; + const id2 = 'some-id-2'; + const accountState1: AccountState = { + ...mockAccountState, + metadata: { + address: 'bc1qcached1...', + addressType: 'p2wpkh', + network: 'bitcoin', + publicDescriptor: 'cached-public-descriptor-1', + }, + }; + const accountState2: AccountState = { + ...mockAccountState, + metadata: { + address: 'bc1qcached2...', + addressType: 'p2wpkh', + network: 'bitcoin', + publicDescriptor: 'cached-public-descriptor-2', + }, + }; + mockSnapClient.getState.mockResolvedValue({ + [id1]: accountState1, + [id2]: accountState2, + }); + (BdkAccountAdapter.load as jest.Mock).mockClear(); + (ChangeSet.from_json as jest.Mock).mockClear(); + + const result = await repo.getByIds([id2, 'missing-id', id1]); + + expect(mockSnapClient.getState).toHaveBeenCalledWith('accounts'); + expect(result).toHaveLength(2); + expect(result[0]?.id).toBe(id2); + expect(result[0]?.publicAddress.toString()).toBe('bc1qaddress...'); + expect(result[0]?.publicDescriptor).toBe('cached-public-descriptor-2'); + expect(result[1]?.id).toBe(id1); + expect(result[1]?.publicAddress.toString()).toBe('bc1qaddress...'); + expect(result[1]?.publicDescriptor).toBe('cached-public-descriptor-1'); + expect(jest.mocked(Address.from_string)).toHaveBeenCalledWith( + 'bc1qcached2...', + 'bitcoin', + ); + expect(jest.mocked(Address.from_string)).toHaveBeenCalledWith( + 'bc1qcached1...', + 'bitcoin', + ); + expect(ChangeSet.from_json).not.toHaveBeenCalled(); + expect(BdkAccountAdapter.load).not.toHaveBeenCalled(); + }); + it('returns empty array if no accounts are found', async () => { mockSnapClient.getState.mockResolvedValue(null); diff --git a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts index 364ebdb63..8ce9be24c 100644 --- a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts +++ b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts @@ -121,7 +121,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return ids.flatMap((id) => { const account = accounts[id]; - return account ? [this.#loadAccount(id, account)] : []; + return account ? [this.#loadPersistedAccount(id, account)] : []; }); } From 27f987cedb1a1859126beae5aa85ebc5d0359f45 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 4 Sep 2026 15:30:08 +0200 Subject: [PATCH 4/9] refactor(bitcoin-wallet-snap): use poo utils --- .../src/handlers/RpcHandler.ts | 16 +++---- .../src/handlers/validation.ts | 46 ++++--------------- 2 files changed, 15 insertions(+), 47 deletions(-) diff --git a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts index 971702cd4..cd22ea340 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts @@ -1,4 +1,8 @@ import { BtcScope } from '@metamask/keyring-api'; +import type { + ProofOfOwnershipBatchRequestItem, + ProofOfOwnershipBatchResponse, +} from '@metamask/snap-networks-utils'; import type { Json, JsonRpcRequest } from '@metamask/snaps-sdk'; import { Verifier } from 'bip322-js'; import { @@ -107,15 +111,7 @@ export const SignProofOfOwnershipBatchRequest = object({ items: array(SignProofOfOwnershipBatchRequestItem), }); -export type SignProofOfOwnershipBatchResponse = { - results: ( - | { accountId: string; signature: string } - | { - accountId: string; - error: string; - } - )[]; -}; +export type SignProofOfOwnershipBatchResponse = ProofOfOwnershipBatchResponse; /** * Converts an unknown thrown value into a JSON-serializable error message. @@ -509,7 +505,7 @@ export class RpcHandler { * @returns One result per item, in input order. */ async #signProofOfOwnershipBatch( - items: { accountId: string; message: string }[], + items: ProofOfOwnershipBatchRequestItem[], ): Promise { const uniqueAccountIds = [ ...new Set(items.map(({ accountId }) => accountId)), diff --git a/packages/bitcoin-wallet-snap/src/handlers/validation.ts b/packages/bitcoin-wallet-snap/src/handlers/validation.ts index 60ebdf655..1f5bf9e1f 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/validation.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/validation.ts @@ -1,7 +1,11 @@ import type { Network, AddressType } from '@metamask/bitcoindevkit'; import { Address, Amount } from '@metamask/bitcoindevkit'; import { BtcMethod } from '@metamask/keyring-api'; -import { UuidStruct } from '@metamask/snap-networks-utils'; +import { + parseProofOfOwnershipMessage as parseSharedProofOfOwnershipMessage, + UuidStruct, +} from '@metamask/snap-networks-utils'; +import type { ProofOfOwnershipMessage } from '@metamask/snap-networks-utils'; import { CaipAssetTypeStruct } from '@metamask/utils'; import type { Infer } from 'superstruct'; import { @@ -406,8 +410,6 @@ export function parseRewardsMessage(base64Message: string): { }; } -export const PROOF_OF_OWNERSHIP_MESSAGE_PREFIX = 'metamask:proof-of-ownership:'; - // bech32/bech32m HRPs for Bitcoin mainnet, testnet, and regtest. Addresses // starting with one of these are case-insensitive but only canonical in // lowercase. @@ -438,38 +440,8 @@ export function canonicalizeBitcoinAddress(address: string): string { * @returns Object containing the parsed nonce and address * @throws Error if the message format is invalid */ -export function parseProofOfOwnershipMessage(message: string): { - nonce: string; - address: string; -} { - if (!message.startsWith(PROOF_OF_OWNERSHIP_MESSAGE_PREFIX)) { - throw new Error( - `Message must start with "${PROOF_OF_OWNERSHIP_MESSAGE_PREFIX}"`, - ); - } - - const remainder = message.slice(PROOF_OF_OWNERSHIP_MESSAGE_PREFIX.length); - const separatorIdx = remainder.lastIndexOf(':'); - if (separatorIdx === -1) { - throw new Error( - 'Message must follow the format "metamask:proof-of-ownership:{nonce}:{address}"', - ); - } - - const nonce = remainder.slice(0, separatorIdx); - const address = remainder.slice(separatorIdx + 1); - - if (nonce === '') { - throw new Error( - 'Proof-of-ownership message must contain a non-empty nonce', - ); - } - - if (address === '') { - throw new Error( - 'Proof-of-ownership message must contain a non-empty address', - ); - } - - return { nonce, address }; +export function parseProofOfOwnershipMessage( + message: string, +): ProofOfOwnershipMessage { + return parseSharedProofOfOwnershipMessage(message); } From badfb34fa498aca820eea258a86c129deda4f731 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 4 Sep 2026 16:00:14 +0200 Subject: [PATCH 5/9] refactor(bitcoin-wallet-snap): use shared error normalization --- .../src/handlers/RpcHandler.ts | 13 ++----------- .../src/use-cases/AccountUseCases.ts | 17 ++++------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts index cd22ea340..355f5853b 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts @@ -1,4 +1,5 @@ import { BtcScope } from '@metamask/keyring-api'; +import { normalizeError } from '@metamask/snap-networks-utils'; import type { ProofOfOwnershipBatchRequestItem, ProofOfOwnershipBatchResponse, @@ -113,16 +114,6 @@ export const SignProofOfOwnershipBatchRequest = object({ export type SignProofOfOwnershipBatchResponse = ProofOfOwnershipBatchResponse; -/** - * Converts an unknown thrown value into a JSON-serializable error message. - * - * @param error - The thrown value. - * @returns A string error message. - */ -function getErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error); -} - export class RpcHandler { readonly #logger: Logger; @@ -574,7 +565,7 @@ export class RpcHandler { } catch (error) { results[index] = { accountId, - error: getErrorMessage(error), + error: normalizeError(error).message, }; } }); diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts index b91bc8e92..e66858589 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts @@ -10,6 +10,7 @@ import type { import type { BIP32Node } from '@metamask/key-tree'; import { SLIP10Node } from '@metamask/key-tree'; import { getCurrentUnixTimestamp } from '@metamask/keyring-snap-sdk'; +import { normalizeError } from '@metamask/snap-networks-utils'; import { Signer } from 'bip322-js'; import { encode } from 'wif'; @@ -92,16 +93,6 @@ function getDerivationPathKey(derivationPath: string[]): string { return derivationPath.join('/'); } -/** - * Converts an unknown thrown value into a JSON-serializable error message. - * - * @param error - The thrown value. - * @returns A string error message. - */ -function getErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error); -} - /** * Converts split derivation path segments into key-tree BIP-32 path nodes. * @@ -809,7 +800,7 @@ export class AccountUseCases { parentRequests.push({ index, request, parentPath }); requestsByParentPath.set(parentKey, parentRequests); } catch (error) { - results[index] = { error: getErrorMessage(error) }; + results[index] = { error: normalizeError(error).message }; } }); @@ -843,12 +834,12 @@ export class AccountUseCases { ), }; } catch (error) { - results[index] = { error: getErrorMessage(error) }; + results[index] = { error: normalizeError(error).message }; } } } catch (error) { for (const { index } of parentRequests) { - results[index] = { error: getErrorMessage(error) }; + results[index] = { error: normalizeError(error).message }; } } }), From 17d43a241b9628604ce680d54ff01fee4a7f9006 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 8 Sep 2026 18:07:01 +0200 Subject: [PATCH 6/9] fix(bitcoin-wallet-snap): fix sonarcloud issue --- packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts index 12d314ac5..3a26f26f7 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts @@ -125,8 +125,7 @@ function getProofSigningParentPath(account: BitcoinAccount): string[] { * @returns The child path from parent node to receive address 0. */ function getProofSigningChildPath(account: BitcoinAccount): string[] { - const accountIndexSegment = - account.derivationPath[account.derivationPath.length - 1]; + const accountIndexSegment = account.derivationPath.at(-1); if (!accountIndexSegment) { throw new Error('Missing account derivation path'); From 1ce5e2118c87a43d3427eb97c17ac021270482bb Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 11 Sep 2026 12:51:34 -0400 Subject: [PATCH 7/9] refactor(bitcoin-wallet-snap): address PR comments --- .../bitcoin-wallet-snap/snap.manifest.json | 2 +- .../src/handlers/RpcHandler.test.ts | 29 +++++ .../src/handlers/RpcHandler.ts | 87 +++++++------ .../src/store/BdkAccountRepository.test.ts | 17 +++ .../src/store/BdkAccountRepository.ts | 18 ++- .../src/use-cases/AccountUseCases.test.ts | 34 +++++ .../src/use-cases/AccountUseCases.ts | 117 ++++++++++-------- 7 files changed, 212 insertions(+), 92 deletions(-) diff --git a/packages/bitcoin-wallet-snap/snap.manifest.json b/packages/bitcoin-wallet-snap/snap.manifest.json index e2e913fa6..d1ae0daa8 100644 --- a/packages/bitcoin-wallet-snap/snap.manifest.json +++ b/packages/bitcoin-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "3Msbb6pMl2lUUJnwKtW0ONeFu5n1WTvLuNW2Vc2HW0c=", + "shasum": "ym8AM1UALP/J8Al0hkltshJPyaZppBN8f0WpLj0JIJc=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts index 160cd5642..43157791c 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.test.ts @@ -1317,6 +1317,35 @@ describe('RpcHandler', () => { }); }); + it('matches account IDs case-insensitively while preserving the requested account ID in the response', async () => { + const uppercaseAccountId = validAccountId.toUpperCase(); + const message = `metamask:proof-of-ownership:${nonce}:${accountAddress}`; + mockAccountsUseCases.getByIds.mockResolvedValue([mockBitcoinAccount]); + mockAccountsUseCases.signProofOfOwnershipMessages.mockResolvedValue([ + { signature: 'mock-bip322-signature' }, + ]); + + const result = await handler.route( + origin, + buildBatchRequest([{ accountId: uppercaseAccountId, message }]), + ); + + expect(mockAccountsUseCases.getByIds).toHaveBeenCalledWith([ + uppercaseAccountId, + ]); + expect( + mockAccountsUseCases.signProofOfOwnershipMessages, + ).toHaveBeenCalledWith([{ account: mockBitcoinAccount, message }]); + expect(result).toStrictEqual({ + results: [ + { + accountId: uppercaseAccountId, + signature: 'mock-bip322-signature', + }, + ], + }); + }); + it('returns item-level errors for missing accounts and address mismatches', async () => { const missingAccountId = '6b3df9d2-07fc-4e08-baf9-769254ab3fc8'; const validMessage = `metamask:proof-of-ownership:${nonce}:${accountAddress}`; diff --git a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts index 355f5853b..fa236c802 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/RpcHandler.ts @@ -24,7 +24,12 @@ import { ValidationError, } from '../entities'; import type { CodifiedError, Logger } from '../entities'; -import type { AccountUseCases, SendFlowUseCases } from '../use-cases'; +import type { + AccountUseCases, + SendFlowUseCases, + SignProofOfOwnershipMessageBatchRequest, + SignProofOfOwnershipMessageBatchResult, +} from '../use-cases'; import { scopeToNetwork } from './caip'; import type { TransactionFee } from './mappings'; import { mapPsbtToTransaction, mapToTransactionFees } from './mappings'; @@ -114,6 +119,21 @@ export const SignProofOfOwnershipBatchRequest = object({ export type SignProofOfOwnershipBatchResponse = ProofOfOwnershipBatchResponse; +/** + * Checks whether a batch proof-signing result is an item-level error. + * + * @param signedMessage - The result returned by batch proof signing. + * @returns Whether the result is an error response. + */ +function isSignProofOfOwnershipMessageBatchError( + signedMessage: SignProofOfOwnershipMessageBatchResult, +): signedMessage is Extract< + SignProofOfOwnershipMessageBatchResult, + { error: string } +> { + return Object.hasOwn(signedMessage, 'error'); +} + export class RpcHandler { readonly #logger: Logger; @@ -453,6 +473,16 @@ export class RpcHandler { account.publicAddress.toString(), ); + if (canonicalMessageAddress !== canonicalAccountAddress) { + throw new ValidationError( + `Address in proof-of-ownership message (${messageAddress}) does not match signing account address (${canonicalAccountAddress})`, + { + messageAddress, + accountAddress: canonicalAccountAddress, + }, + ); + } + const addressValidation = validateAddress( canonicalMessageAddress, account.network, @@ -465,16 +495,6 @@ export class RpcHandler { ); } - if (canonicalMessageAddress !== canonicalAccountAddress) { - throw new ValidationError( - `Address in proof-of-ownership message (${messageAddress}) does not match signing account address (${canonicalAccountAddress})`, - { - messageAddress, - accountAddress: canonicalAccountAddress, - }, - ); - } - const signature = await this.#accountUseCases.signMessage( accountId, message, @@ -503,20 +523,19 @@ export class RpcHandler { ]; const allAccounts = await this.#accountUseCases.getByIds(uniqueAccountIds); const accountsById = new Map( - allAccounts.map((account) => [account.id, account]), + allAccounts.map((account) => [account.id.toLowerCase(), account]), ); const results: SignProofOfOwnershipBatchResponse['results'] = new Array( items.length, ); - const signingRequests: { + const signingRequestMetadata: { index: number; accountId: string; - account: (typeof allAccounts)[number]; - message: string; }[] = []; + const signingRequests: SignProofOfOwnershipMessageBatchRequest[] = []; items.forEach(({ accountId, message }, index) => { - const account = accountsById.get(accountId); + const account = accountsById.get(accountId.toLowerCase()); if (!account) { results[index] = { accountId, @@ -535,6 +554,14 @@ export class RpcHandler { account.publicAddress.toString(), ); + if (canonicalMessageAddress !== canonicalAccountAddress) { + results[index] = { + accountId, + error: `Address in proof-of-ownership message (${messageAddress}) does not match signing account address (${canonicalAccountAddress})`, + }; + return; + } + const addressValidation = validateAddress( canonicalMessageAddress, account.network, @@ -548,17 +575,11 @@ export class RpcHandler { return; } - if (canonicalMessageAddress !== canonicalAccountAddress) { - results[index] = { - accountId, - error: `Address in proof-of-ownership message (${messageAddress}) does not match signing account address (${canonicalAccountAddress})`, - }; - return; - } - - signingRequests.push({ + signingRequestMetadata.push({ index, accountId, + }); + signingRequests.push({ account, message, }); @@ -575,23 +596,19 @@ export class RpcHandler { } const signedMessages = - await this.#accountUseCases.signProofOfOwnershipMessages( - signingRequests.map(({ account, message }) => ({ account, message })), - ); + await this.#accountUseCases.signProofOfOwnershipMessages(signingRequests); signedMessages.forEach((signedMessage, signingRequestIndex) => { - const { index, accountId } = signingRequests[ + const { index, accountId } = signingRequestMetadata[ signingRequestIndex - ] as (typeof signingRequests)[number]; - const { error } = signedMessage as { error?: string }; + ] as (typeof signingRequestMetadata)[number]; - if (error !== undefined) { - results[index] = { accountId, error }; + if (isSignProofOfOwnershipMessageBatchError(signedMessage)) { + results[index] = { accountId, error: signedMessage.error }; return; } - const { signature } = signedMessage as { signature: string }; - results[index] = { accountId, signature }; + results[index] = { accountId, signature: signedMessage.signature }; }); return { results }; diff --git a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts index 7e79ca843..9d3162fbd 100644 --- a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts +++ b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.test.ts @@ -209,6 +209,23 @@ describe('BdkAccountRepository', () => { expect(result).toStrictEqual([mockAccount2, mockAccount1]); }); + it('matches account IDs case-insensitively', async () => { + const id = '724ac464-6572-4d9c-a8e2-4075c8846d65'; + const state = { + [id]: { ...mockAccountState, id }, + }; + const loadedAccount = { ...mockAccount, id }; + + mockSnapClient.getState.mockResolvedValue(state); + (BdkAccountAdapter.load as jest.Mock).mockReturnValueOnce(loadedAccount); + + const result = await repo.getByIds([id.toUpperCase()]); + + expect(mockSnapClient.getState).toHaveBeenCalledWith('accounts'); + expect(BdkAccountAdapter.load).toHaveBeenCalledTimes(1); + expect(result).toStrictEqual([loadedAccount]); + }); + it('uses cached account metadata without loading BDK wallets', async () => { const id1 = 'some-id-1'; const id2 = 'some-id-2'; diff --git a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts index 8ce9be24c..96ca68878 100644 --- a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts +++ b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts @@ -119,9 +119,23 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return []; } + const accountsByLowercaseId = new Map( + Object.entries(accounts).map(([id, account]) => [ + id.toLowerCase(), + { id, account }, + ]), + ); + return ids.flatMap((id) => { - const account = accounts[id]; - return account ? [this.#loadPersistedAccount(id, account)] : []; + const storedAccount = accountsByLowercaseId.get(id.toLowerCase()); + return storedAccount?.account + ? [ + this.#loadPersistedAccount( + storedAccount.id, + storedAccount.account, + ), + ] + : []; }); } diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts index af892083e..48a618029 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts @@ -2101,6 +2101,7 @@ describe('AccountUseCases', () => { describe('signMessage', () => { const mockAccount = mock({ + id: 'account-id', publicAddress: mock
({ toString: () => 'bcrt1qs2fj7czz0amfm74j73yujx6dn6223md56gkkuy', }), @@ -2162,6 +2163,25 @@ describe('AccountUseCases', () => { ).rejects.toThrow('Failed to sign message'); }); + it('does not include the signed message in WalletError metadata', async () => { + mockSnapClient.getPrivateEntropy.mockResolvedValue({ + privateKey: '0x1234567890abcdef', // wrong private key returned + } as JsonSLIP10Node); + + try { + await useCases.signMessage('account-id', mockMessage, mockOrigin); + throw new Error('Expected signMessage to throw'); + } catch (error) { + expect(error).toMatchObject({ + message: 'Failed to sign message', + data: { id: 'account-id' }, + }); + expect((error as { data?: Record }).data).not.toHaveProperty( + 'message', + ); + } + }); + it('throws AssertionError if entropy has no privateKey', async () => { mockSnapClient.getPrivateEntropy.mockResolvedValue( mock({ privateKey: undefined }), @@ -2278,5 +2298,19 @@ describe('AccountUseCases', () => { { error: 'Account missing given capability' }, ]); }); + + it('does not expose derivation error details in batch signing results', async () => { + const sensitiveError = 'derived private key bytes: secret'; + jest.spyOn(RealSlip10Node, 'fromJSON').mockResolvedValueOnce({ + derive: jest.fn().mockRejectedValue(new Error(sensitiveError)), + } as never); + + const result = await useCases.signProofOfOwnershipMessages([ + { account: createAccount(0), message: mockMessage }, + ]); + + expect(result).toStrictEqual([{ error: 'Unable to derive private key' }]); + expect(JSON.stringify(result)).not.toContain(sensitiveError); + }); }); }); diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts index 3a26f26f7..56d077c9a 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.ts @@ -740,37 +740,18 @@ export class AccountUseCases { }); } - try { - // Private key is returned in "0x..." format, transform into WIF: - const wifPrivateKey = encode({ - version: account.network === 'bitcoin' ? 128 : 239, // 128 for mainnet, 239 for testnets - // eslint-disable-next-line no-restricted-globals - privateKey: Buffer.from(entropy.privateKey.slice(2), 'hex'), - compressed: true, - }); - const signature = Signer.sign( - wifPrivateKey, - account.publicAddress.toString(), - message, - ); + const signature = this.#signMessageWithPrivateKey( + account, + message, + entropy.privateKey, + ); - this.#logger.info( - 'Message signed successfully: %s. Message: %s, Signature: %s.', - id, - message, - signature, - ); - return signature; - } catch (error) { - throw new WalletError( - 'Failed to sign message', - { - id, - message, - }, - error, - ); - } + this.#logger.info( + 'Message signed successfully: %s. Signature: %s.', + id, + signature, + ); + return signature; } /** @@ -793,10 +774,12 @@ export class AccountUseCases { const requestsByParentPath = new Map< string, { - index: number; - request: SignProofOfOwnershipMessageBatchRequest; parentPath: string[]; - }[] + requests: { + index: number; + request: SignProofOfOwnershipMessageBatchRequest; + }[]; + } >(); requests.forEach((request, index) => { @@ -805,19 +788,20 @@ export class AccountUseCases { const parentPath = getProofSigningParentPath(request.account); const parentKey = getDerivationPathKey(parentPath); - const parentRequests = requestsByParentPath.get(parentKey) ?? []; - parentRequests.push({ index, request, parentPath }); - requestsByParentPath.set(parentKey, parentRequests); + const parentRequestGroup = requestsByParentPath.get(parentKey) ?? { + parentPath, + requests: [], + }; + parentRequestGroup.requests.push({ index, request }); + requestsByParentPath.set(parentKey, parentRequestGroup); } catch (error) { results[index] = { error: normalizeError(error).message }; } }); await Promise.all( - [...requestsByParentPath.values()].map(async (parentRequests) => { - const { parentPath } = - parentRequests[0] as (typeof parentRequests)[number]; - + [...requestsByParentPath.values()].map(async (parentRequestGroup) => { + const { parentPath, requests: parentRequests } = parentRequestGroup; try { const parentJson = await this.#snapClient.getPrivateEntropy(parentPath); @@ -825,21 +809,16 @@ export class AccountUseCases { for (const { index, request } of parentRequests) { try { - const entropy = await parentNode.derive( - toBip32Path(getProofSigningChildPath(request.account)), + const privateKey = await this.#deriveProofSigningPrivateKey( + parentNode, + request.account, ); - if (!entropy.privateKey) { - throw new AssertionError('Failed to get private entropy', { - id: request.account.id, - }); - } - results[index] = { - signature: this.#signProofOfOwnershipMessage( + signature: this.#signMessageWithPrivateKey( request.account, request.message, - entropy.privateKey, + privateKey, ), }; } catch (error) { @@ -858,14 +837,45 @@ export class AccountUseCases { } /** - * Signs one proof-of-ownership message using private key entropy. + * Derives private key entropy for one proof-signing account from its parent + * node. + * + * @param parentNode - Parent node for the account's derivation path. + * @param account - Account to derive private key entropy for. + * @returns The account private key as a 0x-prefixed hex string. + */ + async #deriveProofSigningPrivateKey( + parentNode: SLIP10Node, + account: BitcoinAccount, + ): Promise { + try { + const entropy = await parentNode.derive( + toBip32Path(getProofSigningChildPath(account)), + ); + + if (!entropy.privateKey) { + throw new AssertionError('Failed to get private entropy', { + id: account.id, + }); + } + + return entropy.privateKey; + } catch { + throw new WalletError('Unable to derive private key', { + id: account.id, + }); + } + } + + /** + * Signs one message using private key entropy. * * @param account - Account whose public address should own the signature. - * @param message - Plaintext proof-of-ownership message. + * @param message - Plaintext message. * @param privateKey - 0x-prefixed private key hex string. * @returns The BIP-322 signature. */ - #signProofOfOwnershipMessage( + #signMessageWithPrivateKey( account: BitcoinAccount, message: string, privateKey: string, @@ -888,7 +898,6 @@ export class AccountUseCases { 'Failed to sign message', { id: account.id, - message, }, error, ); From 3de774ea62995eb695f9525633c7ed37a4eb36ba Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 11 Sep 2026 13:08:37 -0400 Subject: [PATCH 8/9] fix(bitcoin-wallet-snap): lint fixes --- .../bitcoin-wallet-snap/src/store/BdkAccountRepository.ts | 7 +------ .../src/use-cases/AccountUseCases.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts index 96ca68878..1488546bd 100644 --- a/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts +++ b/packages/bitcoin-wallet-snap/src/store/BdkAccountRepository.ts @@ -129,12 +129,7 @@ export class BdkAccountRepository implements BitcoinAccountRepository { return ids.flatMap((id) => { const storedAccount = accountsByLowercaseId.get(id.toLowerCase()); return storedAccount?.account - ? [ - this.#loadPersistedAccount( - storedAccount.id, - storedAccount.account, - ), - ] + ? [this.#loadPersistedAccount(storedAccount.id, storedAccount.account)] : []; }); } diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts index 48a618029..07bbba684 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts @@ -2176,9 +2176,9 @@ describe('AccountUseCases', () => { message: 'Failed to sign message', data: { id: 'account-id' }, }); - expect((error as { data?: Record }).data).not.toHaveProperty( - 'message', - ); + expect( + (error as { data?: Record }).data, + ).not.toHaveProperty('message'); } }); From 3a4c28c08de6b3cb0c9d21e262e8ee3325955c17 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 11 Sep 2026 13:44:03 -0400 Subject: [PATCH 9/9] fix(bitcoin-wallet-snap): fix lint issue in test --- .../src/use-cases/AccountUseCases.test.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts index 07bbba684..22c77adf0 100644 --- a/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts +++ b/packages/bitcoin-wallet-snap/src/use-cases/AccountUseCases.test.ts @@ -2168,18 +2168,20 @@ describe('AccountUseCases', () => { privateKey: '0x1234567890abcdef', // wrong private key returned } as JsonSLIP10Node); + let thrownError: unknown; try { await useCases.signMessage('account-id', mockMessage, mockOrigin); - throw new Error('Expected signMessage to throw'); } catch (error) { - expect(error).toMatchObject({ - message: 'Failed to sign message', - data: { id: 'account-id' }, - }); - expect( - (error as { data?: Record }).data, - ).not.toHaveProperty('message'); + thrownError = error; } + + expect(thrownError).toMatchObject({ + message: 'Failed to sign message', + data: { id: 'account-id' }, + }); + expect( + (thrownError as { data?: Record }).data, + ).not.toHaveProperty('message'); }); it('throws AssertionError if entropy has no privateKey', async () => {