diff --git a/packages/bitcoin-wallet-snap/CHANGELOG.md b/packages/bitcoin-wallet-snap/CHANGELOG.md index 8bb5167a..0767b979 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] +### Fixed + +- Fix `onKeyringRequest` responses to return `Json` directly (v2 protocol) instead of the v1 `{ pending: false, result }` envelope ([#100](https://github.com/MetaMask/internal-snaps/pull/100)) + ## [2.0.0] ### Added diff --git a/packages/bitcoin-wallet-snap/snap.manifest.json b/packages/bitcoin-wallet-snap/snap.manifest.json index 9d682594..ae4143e3 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": "ye8FAG8Punj6snX/zr6AwKJCidDHdim1FUHqKwjTQog=", + "shasum": "nmBUkNRCyH60zI0wMFb9HzlW6ZVjCRxnLZdWOKcp88s=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts index a94d33f5..3dc0a342 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts @@ -11,7 +11,6 @@ import type { import { Address } from '@metamask/bitcoindevkit'; import type { KeyringAccount, - KeyringResponse, Transaction as KeyringTransaction, KeyringRequest, } from '@metamask/keyring-api'; @@ -962,7 +961,7 @@ describe('KeyringHandler', () => { describe('submitRequest', () => { it('calls KeyringRequestHandler', async () => { const mockRequest = mock(); - const expectedResponse = mock(); + const expectedResponse = { signature: 'mockSig' }; mockKeyringRequest.route.mockResolvedValue(expectedResponse); const result = await handler.submitRequest(mockRequest); diff --git a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts index 903427ef..600cd7e4 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts @@ -12,7 +12,6 @@ import type { CreateAccountOptions, KeyringAccount, KeyringRequest, - KeyringResponse, Paginated, Pagination, ResolvedAccountAddress, @@ -24,7 +23,7 @@ import type { KeyringSnapRpc, } from '@metamask/keyring-api/v2'; import { SnapError } from '@metamask/snaps-sdk'; -import type { CaipChainId, JsonRpcRequest } from '@metamask/snaps-sdk'; +import type { CaipChainId, Json, JsonRpcRequest } from '@metamask/snaps-sdk'; import { sensitive } from '@metamask/superstruct'; import { assert, is, string } from 'superstruct'; import { encode } from 'wif'; @@ -337,7 +336,7 @@ export class KeyringHandler implements KeyringSnapRpc { }; } - async submitRequest(request: KeyringRequest): Promise { + async submitRequest(request: KeyringRequest): Promise { return this.#keyringRequest.route(request); } diff --git a/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.test.ts b/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.test.ts index f396be20..26962c49 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.test.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.test.ts @@ -155,12 +155,9 @@ describe('KeyringRequestHandler', () => { 3, ); expect(result).toStrictEqual({ - pending: false, - result: { - psbt: 'psbtBase64', - txid: 'txid', - canBeMalleable: false, - }, + psbt: 'psbtBase64', + txid: 'txid', + canBeMalleable: false, }); }); @@ -183,10 +180,7 @@ describe('KeyringRequestHandler', () => { const result = await handler.route(noBroadcastRequest); - expect(result).toStrictEqual({ - pending: false, - result: { psbt: 'psbtBase64', txid: null }, - }); + expect(result).toStrictEqual({ psbt: 'psbtBase64', txid: null }); }); it('throws AssertionError if usecase returns txid without canBeMalleable', async () => { @@ -215,12 +209,9 @@ describe('KeyringRequestHandler', () => { const result = await handler.route(mockRequest); expect(result).toStrictEqual({ - pending: false, - result: { - psbt: 'psbtBase64', - txid: 'txid', - canBeMalleable: true, - }, + psbt: 'psbtBase64', + txid: 'txid', + canBeMalleable: true, }); }); @@ -303,10 +294,7 @@ describe('KeyringRequestHandler', () => { const result = await handler.route(mockRequest); - expect(result).toStrictEqual({ - pending: false, - result: { psbt: 'psbtBase64', txid: null }, - }); + expect(result).toStrictEqual({ psbt: 'psbtBase64', txid: null }); }); it('does not sign if user cancels confirmation', async () => { @@ -409,10 +397,7 @@ describe('KeyringRequestHandler', () => { mockPsbt, 3, ); - expect(result).toStrictEqual({ - pending: false, - result: { fee: '1000' }, - }); + expect(result).toStrictEqual({ fee: '1000' }); }); it('propagates errors from parsePsbt', async () => { @@ -474,10 +459,7 @@ describe('KeyringRequestHandler', () => { mockPsbt, 3, ); - expect(result).toStrictEqual({ - pending: false, - result: { psbt: 'filledPsbtBase64' }, - }); + expect(result).toStrictEqual({ psbt: 'filledPsbtBase64' }); }); it('propagates errors from parsePsbt', async () => { @@ -543,10 +525,7 @@ describe('KeyringRequestHandler', () => { mockPsbt, origin, ); - expect(result).toStrictEqual({ - pending: false, - result: { txid: 'txid', canBeMalleable: false }, - }); + expect(result).toStrictEqual({ txid: 'txid', canBeMalleable: false }); }); it('passes canBeMalleable=true through for legacy P2PKH accounts', async () => { @@ -560,10 +539,7 @@ describe('KeyringRequestHandler', () => { const result = await handler.route(mockRequest); - expect(result).toStrictEqual({ - pending: false, - result: { txid: 'txid', canBeMalleable: true }, - }); + expect(result).toStrictEqual({ txid: 'txid', canBeMalleable: true }); }); it('propagates errors from parsePsbt', async () => { @@ -636,10 +612,7 @@ describe('KeyringRequestHandler', () => { origin, 3, ); - expect(result).toStrictEqual({ - pending: false, - result: { txid: 'txid', canBeMalleable: false }, - }); + expect(result).toStrictEqual({ txid: 'txid', canBeMalleable: false }); }); it('passes canBeMalleable=true through for legacy P2PKH accounts', async () => { @@ -653,10 +626,7 @@ describe('KeyringRequestHandler', () => { const result = await handler.route(mockRequest); - expect(result).toStrictEqual({ - pending: false, - result: { txid: 'txid', canBeMalleable: true }, - }); + expect(result).toStrictEqual({ txid: 'txid', canBeMalleable: true }); }); it('propagates errors from sendTransfer', async () => { @@ -704,10 +674,7 @@ describe('KeyringRequestHandler', () => { GetUtxoRequest, ); expect(mockAccountsUseCases.get).toHaveBeenCalledWith('account-id'); - expect(result).toStrictEqual({ - pending: false, - result: expectedUtxo, - }); + expect(result).toStrictEqual(expectedUtxo); }); it('throws NotFoundError when UTXO does not exist', async () => { @@ -750,10 +717,7 @@ describe('KeyringRequestHandler', () => { const result = await handler.route(mockRequest); expect(mockAccountsUseCases.get).toHaveBeenCalledWith('account-id'); - expect(result).toStrictEqual({ - pending: false, - result: [mockUtxo, mockUtxo], - }); + expect(result).toStrictEqual([mockUtxo, mockUtxo]); }); }); @@ -774,10 +738,7 @@ describe('KeyringRequestHandler', () => { const result = await handler.route(mockRequest); expect(mockAccountsUseCases.get).toHaveBeenCalledWith('account-id'); - expect(result).toStrictEqual({ - pending: false, - result: 'publicDescriptor', - }); + expect(result).toBe('publicDescriptor'); }); }); @@ -803,10 +764,7 @@ describe('KeyringRequestHandler', () => { 'message', 'metamask', ); - expect(result).toStrictEqual({ - pending: false, - result: { signature: 'signature' }, - }); + expect(result).toStrictEqual({ signature: 'signature' }); }); }); }); diff --git a/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.ts b/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.ts index 360151b1..8bec111c 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/KeyringRequestHandler.ts @@ -1,4 +1,4 @@ -import type { KeyringRequest, KeyringResponse } from '@metamask/keyring-api'; +import type { KeyringRequest } from '@metamask/keyring-api'; import type { Json } from '@metamask/snaps-sdk'; import { assert } from 'superstruct'; @@ -11,6 +11,7 @@ import { } from '../entities'; import type { AccountUseCases } from '../use-cases/AccountUseCases'; import { mapToUtxo } from './mappings'; +import type { Utxo } from './mappings'; import { parsePsbt } from './parsers'; import { BroadcastPsbtRequest, @@ -64,7 +65,7 @@ export class KeyringRequestHandler { this.#confirmationRepository = confirmationRepository; } - async route(request: KeyringRequest): Promise { + async route(request: KeyringRequest): Promise { const { account, request: requestData, origin } = request; const { method, params } = requestData; @@ -127,7 +128,7 @@ export class KeyringRequestHandler { origin: string, options: { fill: boolean; broadcast: boolean }, feeRate?: number, - ): Promise { + ): Promise { const account = await this.#accountsUseCases.get(id); const psbtBase64ToSign = options.fill @@ -173,53 +174,46 @@ export class KeyringRequestHandler { if (canBeMalleable !== undefined) { response.canBeMalleable = canBeMalleable; } - return this.#toKeyringResponse(response); + return response; } async #fillPsbt( id: string, psbtBase64: string, feeRate?: number, - ): Promise { + ): Promise { const psbt = await this.#accountsUseCases.fillPsbt( id, parsePsbt(psbtBase64), feeRate, ); - return this.#toKeyringResponse({ - psbt: psbt.toString(), - } as FillPsbtResponse); + return { psbt: psbt.toString() }; } async #computeFee( id: string, psbtBase64: string, feeRate?: number, - ): Promise { + ): Promise { const fee = await this.#accountsUseCases.computeFee( id, parsePsbt(psbtBase64), feeRate, ); - return this.#toKeyringResponse({ - fee: fee.to_sat().toString(), - } as ComputeFeeResponse); + return { fee: fee.to_sat().toString() }; } async #broadcastPsbt( id: string, psbtBase64: string, origin: string, - ): Promise { + ): Promise { const { txid, canBeMalleable } = await this.#accountsUseCases.broadcastPsbt( id, parsePsbt(psbtBase64), origin, ); - return this.#toKeyringResponse({ - txid: txid.toString(), - canBeMalleable, - } as BroadcastPsbtResponse); + return { txid: txid.toString(), canBeMalleable }; } async #sendTransfer( @@ -227,59 +221,47 @@ export class KeyringRequestHandler { recipients: { address: string; amount: string }[], origin: string, feeRate?: number, - ): Promise { + ): Promise { const { txid, canBeMalleable } = await this.#accountsUseCases.sendTransfer( id, recipients, origin, feeRate, ); - return this.#toKeyringResponse({ - txid: txid.toString(), - canBeMalleable, - } as BroadcastPsbtResponse); + return { txid: txid.toString(), canBeMalleable }; } - async #getUtxo(id: string, outpoint: string): Promise { + async #getUtxo(id: string, outpoint: string): Promise { const account = await this.#accountsUseCases.get(id); const utxo = account.getUtxo(outpoint); if (!utxo) { throw new NotFoundError('UTXO not found', { id }); } - return this.#toKeyringResponse(mapToUtxo(utxo, account.network)); + return mapToUtxo(utxo, account.network); } - async #listUtxos(id: string): Promise { + async #listUtxos(id: string): Promise { const account = await this.#accountsUseCases.get(id); - return this.#toKeyringResponse( - account.listUnspent().map((utxo) => mapToUtxo(utxo, account.network)), - ); + return account + .listUnspent() + .map((utxo) => mapToUtxo(utxo, account.network)); } - async #publicDescriptor(id: string): Promise { + async #publicDescriptor(id: string): Promise { const account = await this.#accountsUseCases.get(id); - return this.#toKeyringResponse(account.publicDescriptor); + return account.publicDescriptor; } async #signMessage( id: string, message: string, origin: string, - ): Promise { + ): Promise { const signature = await this.#accountsUseCases.signMessage( id, message, origin, ); - return this.#toKeyringResponse({ - signature, - } as SignMessageResponse); - } - - #toKeyringResponse(result: Json): KeyringResponse { - return { - pending: false, - result, - }; + return { signature }; } }