feat: enable account address resolution to support dApps connectivity

[baptiste-marchand]

Implements account address resolution for Bitcoin requests, allowing MetaMask to route non-EVM dapp signing requests correctly. This is a requirement for Bitcoin dApps connectivity

• Introduces the resolveAccountAddress method in KeyringHandler and KeyringRequestHandler to determine the account address for signing requests.
• Adds necessary data structures and validation logic for Bitcoin request parameters.

---

Note

Medium Risk
Adds a new keyring_resolveAccountAddress routing path and expands request validation to require an account.address, which could break dApps/integrations that still send the old param shape. It also inserts new user-confirmation steps before signing PSBTs and sending transfers, changing transaction flow behavior and test expectations.

Overview
Enables MetaMask to route incoming Bitcoin dapp requests by adding KeyringHandler.resolveAccountAddress, which validates the request (BtcWalletRequestStruct) and maps the provided account.address + scope to a CAIP-10 address (or returns null).

Updates Bitcoin keyring request validation so signing/PSBT-related methods (SignPsbt, FillPsbt, ComputeFee, BroadcastPsbt, GetUtxo, SendTransfer, SignMessage) now require an account: { address } parameter, and adjusts integration/unit tests accordingly (including a new “missing account” failure).

Introduces new confirmation UX and repository wiring: a SignPsbtConfirmationView (with new i18n strings) and new ConfirmationRepository methods (insertSignPsbt, insertSendTransfer). KeyringRequestHandler now shows a confirmation before signPsbt, and AccountUseCases.sendTransfer is constrained to exactly one recipient, builds a PSBT with frozen UTXOs/fee rate, and shows a confirmation before signing/broadcasting; confirmation contexts now pass through origin and optional fiat exchange-rate display.

^{Written by Cursor Bugbot for commit 68881d0. This will update automatically on new commits. Configure here.}

[cursor]

Bug: Account Field Mandate Breaks Tests

Existing unit tests create request params without the required account field, but the request struct changes now mandate this field. Tests for signPsbt, computeFee, fillPsbt, broadcastPsbt, sendTransfer, getUtxo, and signMessage will fail during assertion validation because their params lack the required account: { address: string } object.

packages/snap/src/handlers/KeyringRequestHandler.test.ts#L64-L431

snap-bitcoin-wallet/packages/snap/src/handlers/KeyringRequestHandler.test.ts

Lines 64 to 431 in 18900e3

	const mockRequest = mock<KeyringRequest>({
	origin,
	request: {
	method: AccountCapability.SignPsbt,
	params: {
	psbt: 'psbtBase64',
	feeRate: 3,
	options: mockOptions,
	},
	},
	account: 'account-id',
	});
	it('executes signPsbt', async () => {
	mockAccountsUseCases.signPsbt.mockResolvedValue({
	psbt: 'psbtBase64',
	txid: mock<Txid>({
	toString: jest.fn().mockReturnValue('txid'),
	}),
	});
	const result = await handler.route(mockRequest);
	expect(assert).toHaveBeenCalledWith(
	mockRequest.request.params,
	SignPsbtRequest,
	);
	expect(mockAccountsUseCases.signPsbt).toHaveBeenCalledWith(
	'account-id',
	mockPsbt,
	'metamask',
	mockOptions,
	3,
	);
	expect(result).toStrictEqual({
	pending: false,
	result: { psbt: 'psbtBase64', txid: 'txid' },
	});
	});
	it('propagates errors from parsePsbt', async () => {
	const error = new Error('parsePsbt');
	jest.mocked(parsePsbt).mockImplementationOnce(() => {
	throw error;
	});
	await expect(
	handler.route({
	...mockRequest,
	request: { ...mockRequest.request, params: { psbt: 'invalidPsbt' } },
	}),
	).rejects.toThrow(error);
	expect(mockAccountsUseCases.signPsbt).not.toHaveBeenCalled();
	});
	it('propagates errors from signPsbt', async () => {
	const error = new Error();
	mockAccountsUseCases.signPsbt.mockRejectedValue(error);
	await expect(handler.route(mockRequest)).rejects.toThrow(error);
	expect(mockAccountsUseCases.signPsbt).toHaveBeenCalled();
	});
	});
	describe('computeFee', () => {
	const mockRequest = mock<KeyringRequest>({
	request: {
	method: AccountCapability.ComputeFee,
	params: {
	psbt: 'psbtBase64',
	feeRate: 3,
	},
	},
	account: 'account-id',
	});
	it('executes computeFee', async () => {
	mockAccountsUseCases.computeFee.mockResolvedValue(
	mock<Amount>({
	// eslint-disable-next-line @typescript-eslint/naming-convention
	to_sat: () => BigInt(1000),
	}),
	);
	const result = await handler.route(mockRequest);
	expect(assert).toHaveBeenCalledWith(
	mockRequest.request.params,
	ComputeFeeRequest,
	);
	expect(mockAccountsUseCases.computeFee).toHaveBeenCalledWith(
	'account-id',
	mockPsbt,
	3,
	);
	expect(result).toStrictEqual({
	pending: false,
	result: { fee: '1000' },
	});
	});
	it('propagates errors from parsePsbt', async () => {
	const error = new Error('parsePsbt');
	jest.mocked(parsePsbt).mockImplementationOnce(() => {
	throw error;
	});
	await expect(
	handler.route({
	...mockRequest,
	request: { ...mockRequest.request, params: { psbt: 'invalidPsbt' } },
	}),
	).rejects.toThrow(error);
	expect(mockAccountsUseCases.computeFee).not.toHaveBeenCalled();
	});
	it('propagates errors from computeFee', async () => {
	const error = new Error();
	mockAccountsUseCases.computeFee.mockRejectedValue(error);
	await expect(handler.route(mockRequest)).rejects.toThrow(error);
	expect(mockAccountsUseCases.computeFee).toHaveBeenCalled();
	});
	});
	describe('fillPsbt', () => {
	const mockRequest = mock<KeyringRequest>({
	request: {
	method: AccountCapability.FillPsbt,
	params: {
	psbt: 'psbtBase64',
	feeRate: 3,
	},
	},
	account: 'account-id',
	});
	it('executes fillPsbt', async () => {
	const mockFilledPsbt = mock<Psbt>({
	toString: jest.fn().mockReturnValue('filledPsbtBase64'),
	});
	mockAccountsUseCases.fillPsbt.mockResolvedValue(mockFilledPsbt);
	const result = await handler.route(mockRequest);
	expect(assert).toHaveBeenCalledWith(
	mockRequest.request.params,
	FillPsbtRequest,
	);
	expect(mockAccountsUseCases.fillPsbt).toHaveBeenCalledWith(
	'account-id',
	mockPsbt,
	3,
	);
	expect(result).toStrictEqual({
	pending: false,
	result: { psbt: 'filledPsbtBase64' },
	});
	});
	it('propagates errors from parsePsbt', async () => {
	const error = new Error('parsePsbt');
	jest.mocked(parsePsbt).mockImplementationOnce(() => {
	throw error;
	});
	await expect(
	handler.route({
	...mockRequest,
	request: { ...mockRequest.request, params: { psbt: 'invalidPsbt' } },
	}),
	).rejects.toThrow(error);
	expect(mockAccountsUseCases.fillPsbt).not.toHaveBeenCalled();
	});
	it('propagates errors from fillPsbt', async () => {
	const error = new Error();
	mockAccountsUseCases.fillPsbt.mockRejectedValue(error);
	await expect(handler.route(mockRequest)).rejects.toThrow(error);
	expect(mockAccountsUseCases.fillPsbt).toHaveBeenCalled();
	});
	});
	describe('broadcastPsbt', () => {
	const mockRequest = mock<KeyringRequest>({
	origin,
	request: {
	method: AccountCapability.BroadcastPsbt,
	params: {
	psbt: 'psbtBase64',
	feeRate: 3,
	},
	},
	account: 'account-id',
	});
	it('executes broadcastPsbt', async () => {
	const mockTxid = mock<Txid>({
	toString: jest.fn().mockReturnValue('txid'),
	});
	mockAccountsUseCases.broadcastPsbt.mockResolvedValue(mockTxid);
	const result = await handler.route(mockRequest);
	expect(assert).toHaveBeenCalledWith(
	mockRequest.request.params,
	BroadcastPsbtRequest,
	);
	expect(mockAccountsUseCases.broadcastPsbt).toHaveBeenCalledWith(
	'account-id',
	mockPsbt,
	origin,
	);
	expect(result).toStrictEqual({
	pending: false,
	result: { txid: 'txid' },
	});
	});
	it('propagates errors from parsePsbt', async () => {
	const error = new Error('parsePsbt');
	jest.mocked(parsePsbt).mockImplementationOnce(() => {
	throw error;
	});
	await expect(
	handler.route({
	...mockRequest,
	request: { ...mockRequest.request, params: { psbt: 'invalidPsbt' } },
	}),
	).rejects.toThrow(error);
	expect(mockAccountsUseCases.broadcastPsbt).not.toHaveBeenCalled();
	});
	it('propagates errors from fillPsbt', async () => {
	const error = new Error();
	mockAccountsUseCases.broadcastPsbt.mockRejectedValue(error);
	await expect(handler.route(mockRequest)).rejects.toThrow(error);
	expect(mockAccountsUseCases.broadcastPsbt).toHaveBeenCalled();
	});
	});
	describe('sendTransfer', () => {
	const recipients = [
	{
	address: 'bcrt1qstku2y3pfh9av50lxj55arm8r5gj8tf2yv5nxz',
	amount: '1000',
	},
	];
	const mockRequest = mock<KeyringRequest>({
	origin,
	request: {
	method: AccountCapability.SendTransfer,
	params: {
	recipients,
	feeRate: 3,
	},
	},
	account: 'account-id',
	});
	it('executes sendTransferq', async () => {
	const mockTxid = mock<Txid>({
	toString: jest.fn().mockReturnValue('txid'),
	});
	mockAccountsUseCases.sendTransfer.mockResolvedValue(mockTxid);
	const result = await handler.route(mockRequest);
	expect(assert).toHaveBeenCalledWith(
	mockRequest.request.params,
	SendTransferRequest,
	);
	expect(mockAccountsUseCases.sendTransfer).toHaveBeenCalledWith(
	'account-id',
	recipients,
	origin,
	3,
	);
	expect(result).toStrictEqual({
	pending: false,
	result: { txid: 'txid' },
	});
	});
	it('propagates errors from sendTransfer', async () => {
	const error = new Error();
	mockAccountsUseCases.sendTransfer.mockRejectedValue(error);
	await expect(handler.route(mockRequest)).rejects.toThrow(error);
	expect(mockAccountsUseCases.sendTransfer).toHaveBeenCalled();
	});
	});
	describe('getUtxo', () => {
	const mockLocalOutput = mock<LocalOutput>();
	const mockAccount = mock<BitcoinAccount>({
	getUtxo: () => mockLocalOutput,
	network: 'bitcoin',
	});
	const mockRequest = mock<KeyringRequest>({
	origin,
	request: {
	method: AccountCapability.GetUtxo,
	params: {
	outpoint: 'mytxid:0',
	},
	},
	account: 'account-id',
	});
	it('executes getUtxo', async () => {
	const expectedUtxo = {
	derivationIndex: 0,
	outpoint: 'mytxid:0',
	value: '1000',
	scriptPubkey: 'scriptPubkey',
	scriptPubkeyHex: 'scriptPubkeyHex',
	};
	mockAccountsUseCases.get.mockResolvedValue(mockAccount);
	jest.mocked(mapToUtxo).mockReturnValue(expectedUtxo);
	const result = await handler.route(mockRequest);
	expect(assert).toHaveBeenCalledWith(
	mockRequest.request.params,
	GetUtxoRequest,
	);
	expect(mockAccountsUseCases.get).toHaveBeenCalledWith('account-id');
	expect(result).toStrictEqual({
	pending: false,
	result: expectedUtxo,
	});
	});
	});
	describe('listUtxos', () => {
	const mockLocalOutput = mock<LocalOutput>();
	const mockAccount = mock<BitcoinAccount>({
	listUnspent: () => [mockLocalOutput, mockLocalOutput],
	network: 'bitcoin',
	});
	const mockRequest = mock<KeyringRequest>({
	origin,
	request: {
	method: AccountCapability.ListUtxos,
	},
	account: 'account-id',
	});
	it('executes listUtxos', async () => {
	const mockUtxo = mock<Utxo>({
	derivationIndex: 0,
	outpoint: 'mytxid:0',
	value: '1000',
	scriptPubkey: 'scriptPubkey',
	scriptPubkeyHex: 'scriptPubkeyHex',
	});

 

---

[cursor]

Redundant switch in resolveAccountAddress method

Low Severity

The switch statement in resolveAccountAddress is entirely redundant — every BtcMethod case performs the identical operation: extracting request.params.account.address. Since every variant of BtcWalletRequestStruct includes account: WalletAccountStruct in its params, TypeScript allows accessing request.params.account.address directly on the union type without narrowing. The whole switch (and unreachable default branch) can be replaced with a single line: const addressToValidate = request.params.account.address.

 

[cursor]

Promise used as response in test

High Severity

In the sendTransfer integration test, snap.onKeyringRequest(...) is no longer awaited, but the code calls response.getInterface() as if response were the resolved response object. If onKeyringRequest returns a Promise, this becomes a runtime failure and breaks the test flow/CI.

 

[mikesposito]

I left a small suggestion on the resolveAccountAddress function. Besides this, it seems to me that we are addressing several different things in this PR; they may be all related to dapp connectivity, but since we are adding 1790 lines in the same PR it seems warranted to split the different features and bugfixes we are addressing in separate PRs (or a PR train)

[mikesposito]

I'm not sure that we are expanding KeyringRequestHandler the way it was initially intended: it seems to me that this class is meant to be used via the route public function, that should handle the request and return a KeyringResponse via the this.#toKeyringResponse(..) function, at least this is how other request handlers are implemented on this class.

That being said, I don't think we have a pattern of always routing a request to KeyringRequestHandler from KeyringHandler. So an alternative approach would be to remove the KeyringRequestHandler.resolveAccountAddress function and move its logic to KeyringHandler.resolveAccountAddress directly, as we aren't using any class property or method from KeyringRequestHandler in that function.

[cursor]

Struct Allows Multiple Recipients But Logic Rejects Them

Medium Severity

SendTransferRequest uses array() for recipients, which permits zero or many recipients to pass struct validation. However, AccountUseCases.sendTransfer now throws a ValidationError when there are zero or more than one recipient. A caller sending multiple recipients (a perfectly valid request per the struct schema) will silently pass the API boundary check but then receive a runtime ValidationError from the use case layer — a different error type and message than what struct validation failures produce — making the API contract misleading and error handling inconsistent.

Additional Locations (1)

• packages/snap/src/use-cases/AccountUseCases.ts#L463-L468

 

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

[cursor]

PSBT mutated by confirmation then signed

High Severity

In sendTransfer, the same psbt object is passed to insertSendTransfer (which calls psbt.toString() internally) and then immediately signed via account.sign(psbt). The developer explicitly acknowledged this mutation problem in KeyringRequestHandler.ts#signPsbt, where the comment at line 133 states "Creates a fresh PSBT from the original base64 because the original PSBT is mutated by the confirmation repository" and a fresh PSBT is re-parsed from the original base64. No equivalent fix is applied in sendTransfer, so the signing step operates on a potentially corrupted PSBT object.

Additional Locations (1)

• packages/snap/src/handlers/KeyringRequestHandler.ts#L132-L136

 

[cursor]

Exported type BtcWalletRequest is never used

Low Severity

The exported type BtcWalletRequest is defined but never imported or used anywhere in the codebase. Only BtcWalletRequestStruct is imported (in KeyringHandler.ts). This is dead code that adds unnecessary clutter.

 

[Battambang]

I left a small suggestion on the resolveAccountAddress function. Besides this, it seems to me that we are addressing several different things in this PR; they may be all related to dapp connectivity, but since we are adding 1790 lines in the same PR it seems warranted to split the different features and bugfixes we are addressing in separate PRs (or a PR train)

@mikesposito This PR has been refactored and split into 3 PRs:
#589: refactor: move validation structs and add account field
#590: feat: add resolveAccountAddress method to KeyringHandler for dApp connectivity routing
#591: feat: add confirmation dialog before sendTransfer and before signing a PSBT from the keyring handler