diff --git a/packages/keyring-controller/package.json b/packages/keyring-controller/package.json index 5d46d10393a..e7daaa0d307 100644 --- a/packages/keyring-controller/package.json +++ b/packages/keyring-controller/package.json @@ -62,6 +62,7 @@ "@metamask/keyring-api": "^21.6.0", "@metamask/keyring-internal-api": "^10.0.0", "@metamask/messenger": "^1.1.1", + "@metamask/rpc-errors": "^7.0.2", "@metamask/utils": "^11.9.0", "async-mutex": "^0.5.0", "ethereumjs-wallet": "^1.0.1", diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index e52a9fcacd5..0f50d36b0bf 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -21,6 +21,7 @@ import type { MessengerEvents, MockAnyNamespace, } from '@metamask/messenger'; +import { errorCodes } from '@metamask/rpc-errors'; import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english'; import { bytesToHex, isValidHexAddress } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; @@ -32,10 +33,7 @@ import MockEncryptor, { SALT, } from '../tests/mocks/mockEncryptor'; import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring'; -import { - HardwareWalletError, - MockHardwareKeyring, -} from '../tests/mocks/mockHardwareKeyring'; +import { MockHardwareKeyring } from '../tests/mocks/mockHardwareKeyring'; import { MockKeyring } from '../tests/mocks/mockKeyring'; import MockShallowKeyring from '../tests/mocks/mockShallowKeyring'; import { buildMockTransaction } from '../tests/mocks/mockTransaction'; @@ -1797,6 +1795,29 @@ describe('KeyringController', () => { ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound); }); }); + + it('normalizes cancellation-like signMessage errors to 4001', async () => { + await withController(async ({ controller, initialState }) => { + const account = initialState.keyrings[0].accounts[0]; + const keyring = (await controller.getKeyringForAccount( + account, + )) as EthKeyring; + jest + .spyOn(keyring, 'signMessage') + .mockRejectedValue(new Error('Action canceled by user')); + + await expect( + controller.signMessage({ + data: '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0', + from: account, + }), + ).rejects.toMatchObject({ + code: errorCodes.provider.userRejectedRequest, + message: + 'MetaMask Tx Signature: User denied transaction signature.', + }); + }); + }); }); describe('when the keyring for the given address does not support signMessage', () => { @@ -2224,6 +2245,38 @@ describe('KeyringController', () => { ).rejects.toThrow(/^Keyring Controller signTypedMessage:/u); }); }); + + it('normalizes cancellation-like signTypedMessage errors to 4001', async () => { + await withController(async ({ controller, initialState }) => { + const account = initialState.keyrings[0].accounts[0]; + const keyring = (await controller.getKeyringForAccount( + account, + )) as EthKeyring; + jest + .spyOn(keyring, 'signTypedData') + .mockRejectedValue(new Error('failure_actioncancelled')); + + await expect( + controller.signTypedMessage( + { + data: [ + { + name: 'Message', + type: 'string', + value: 'Hi, Alice!', + }, + ], + from: account, + }, + SignTypedDataVersion.V1, + ), + ).rejects.toMatchObject({ + code: errorCodes.provider.userRejectedRequest, + message: + 'MetaMask Tx Signature: User denied transaction signature.', + }); + }); + }); }); describe('when the keyring for the given address does not support signTypedMessage', () => { @@ -2351,6 +2404,26 @@ describe('KeyringController', () => { }).rejects.toThrow('tx.sign is not a function'); }); }); + + it('normalizes cancellation-like signTransaction errors to 4001', async () => { + await withController(async ({ controller, initialState }) => { + const account = initialState.keyrings[0].accounts[0]; + const keyring = (await controller.getKeyringForAccount( + account, + )) as EthKeyring; + jest + .spyOn(keyring, 'signTransaction') + .mockRejectedValue(new Error('Action cancelled by user')); + + await expect( + controller.signTransaction(buildMockTransaction(), account), + ).rejects.toMatchObject({ + code: errorCodes.provider.userRejectedRequest, + message: + 'MetaMask Tx Signature: User denied transaction signature.', + }); + }); + }); }); describe('when the keyring for the given address does not support signTransaction', () => { @@ -5344,7 +5417,7 @@ describe('KeyringController', () => { describe('error handling', () => { describe('when hardware wallet throws custom error', () => { - it('should preserve hardware wallet error in originalError property', async () => { + it('normalizes hardware user-rejection errors to 4001', async () => { const mockHardwareKeyringBuilder = keyringBuilderFactory( MockHardwareKeyring as unknown as KeyringClass, ); @@ -5384,7 +5457,11 @@ describe('KeyringController', () => { { data: JSON.stringify(typedData), from: hardwareAddress }, SignTypedDataVersion.V4, ), - ).rejects.toThrow(KeyringControllerError); + ).rejects.toMatchObject({ + code: errorCodes.provider.userRejectedRequest, + message: + 'MetaMask Tx Signature: User denied transaction signature.', + }); // Verify the error details by catching it explicitly let caughtError: unknown; @@ -5397,29 +5474,11 @@ describe('KeyringController', () => { caughtError = error; } - // Verify the error is a KeyringControllerError (wrapped by signTypedMessage) - expect(caughtError).toBeInstanceOf(KeyringControllerError); - - const keyringError = caughtError as KeyringControllerError; - - // Verify the error message contains information about the hardware wallet error - expect(keyringError.message).toContain( - 'Keyring Controller signTypedMessage', - ); - expect(keyringError.message).toContain('HardwareWalletError'); - expect(keyringError.message).toContain( - 'User rejected the request on hardware device', - ); - - // Verify the original hardware wallet error is preserved in originalError - expect(keyringError.cause).toBeInstanceOf(HardwareWalletError); - expect(keyringError.cause?.message).toBe( - 'User rejected the request on hardware device', - ); - expect(keyringError.cause?.name).toBe('HardwareWalletError'); - expect((keyringError.cause as HardwareWalletError).code).toBe( - 'USER_REJECTED', - ); + expect(caughtError).toMatchObject({ + code: errorCodes.provider.userRejectedRequest, + message: + 'MetaMask Tx Signature: User denied transaction signature.', + }); }, ); }); diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index d3d7def12b7..5a7ef854b18 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -17,6 +17,7 @@ import type { import type { EthKeyring } from '@metamask/keyring-internal-api'; import type { Keyring, KeyringClass } from '@metamask/keyring-utils'; import type { Messenger } from '@metamask/messenger'; +import { providerErrors } from '@metamask/rpc-errors'; import type { Eip1024EncryptedData, Hex, Json } from '@metamask/utils'; import { add0x, @@ -1367,7 +1368,11 @@ export class KeyringController< ); } - return await keyring.signMessage(address, messageParams.data); + try { + return await keyring.signMessage(address, messageParams.data); + } catch (error) { + throw this.#normalizeSigningRejectionError(error); + } } /** @@ -1428,7 +1433,11 @@ export class KeyringController< const normalizedData = normalize(messageParams.data) as Hex; - return await keyring.signPersonalMessage(address, normalizedData); + try { + return await keyring.signPersonalMessage(address, normalizedData); + } catch (error) { + throw this.#normalizeSigningRejectionError(error); + } } /** @@ -1477,6 +1486,11 @@ export class KeyringController< { version }, ); } catch (error) { + const normalizedError = this.#normalizeSigningRejectionError(error); + if (normalizedError !== error) { + throw normalizedError; + } + const errorMessage = error instanceof Error ? `${error.name}: ${error.message}` @@ -1510,7 +1524,66 @@ export class KeyringController< ); } - return await keyring.signTransaction(address, transaction, opts); + try { + return await keyring.signTransaction(address, transaction, opts); + } catch (error) { + throw this.#normalizeSigningRejectionError(error); + } + } + + #normalizeSigningRejectionError(error: unknown): unknown { + if (!this.#isSigningUserRejectedError(error)) { + return error; + } + + const errorData = isObject(error) + ? (error.data as Json | undefined) + : undefined; + + return providerErrors.userRejectedRequest({ + message: 'MetaMask Tx Signature: User denied transaction signature.', + data: errorData, + }); + } + + #isSigningUserRejectedError( + error: unknown, + visited = new Set(), + ): boolean { + if (!error || visited.has(error)) { + return false; + } + visited.add(error); + + if (typeof error === 'string') { + return /(?:\buser rejected\b|\baction cancelled\b|\bcancelled\b|\bcanceled\b|failure_actioncancelled)/iu.test( + error, + ); + } + + if (typeof error !== 'object') { + return false; + } + + const errorObject = error as { + code?: unknown; + message?: unknown; + stack?: unknown; + cause?: unknown; + originalError?: unknown; + }; + + if (errorObject.code === 4001) { + return true; + } + + return ( + this.#isSigningUserRejectedError(errorObject.code, visited) || + this.#isSigningUserRejectedError(errorObject.message, visited) || + this.#isSigningUserRejectedError(errorObject.stack, visited) || + this.#isSigningUserRejectedError(errorObject.cause, visited) || + this.#isSigningUserRejectedError(errorObject.originalError, visited) + ); } /** diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ea4c95a0dea..95a55b38e3d 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -3253,10 +3253,12 @@ describe('TransactionController', () => { * * @param controller - The controller instance. * @param expectedError - The expected error message. + * @param expectedErrorCode - The expected persisted error code. */ async function expectTransactionToFail( controller: TransactionController, expectedError: string, + expectedErrorCode?: number, ): Promise { const { result } = await controller.addTransaction( { @@ -3274,6 +3276,11 @@ describe('TransactionController', () => { expect(txParams.from).toBe(ACCOUNT_MOCK); expect(txParams.to).toBe(ACCOUNT_MOCK); expect(status).toBe(TransactionStatus.failed); + if (expectedErrorCode !== undefined) { + expect(controller.state.transactions[0].error).toMatchObject({ + code: expectedErrorCode, + }); + } } it('if signing error', async () => { @@ -3308,6 +3315,48 @@ describe('TransactionController', () => { await expectTransactionToFail(controller, 'No sign method defined'); }); + it('normalizes "cancelled" signing errors to userRejectedRequest', async () => { + const { controller } = setupController({ + options: { + sign: () => { + throw new Error('Action cancelled by user'); + }, + }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + }); + + await expectTransactionToFail( + controller, + 'MetaMask Tx Signature: User denied transaction signature.', + errorCodes.provider.userRejectedRequest, + ); + }); + + it('normalizes "canceled" signing errors to userRejectedRequest', async () => { + const { controller } = setupController({ + options: { + sign: () => { + throw new Error('Action canceled by user'); + }, + }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + }); + + await expectTransactionToFail( + controller, + 'MetaMask Tx Signature: User denied transaction signature.', + errorCodes.provider.userRejectedRequest, + ); + }); + it('if unexpected status', async () => { const { controller } = setupController({ messengerOptions: { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index b78422231e7..688371c601b 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -4500,6 +4500,59 @@ export class TransactionController extends BaseController< ].includes(error.code as number); } + #hasUserRejectedMessage( + error: unknown, + visited = new Set(), + ): boolean { + if (!error || visited.has(error)) { + return false; + } + visited.add(error); + + if (typeof error === 'string') { + const normalizedError = error.toLowerCase(); + + if ( + normalizedError.includes('trezorkeyring') && + normalizedError.includes('unknown error') + ) { + return true; + } + + return /(?:\buser rejected\b|\baction cancelled\b|\bcancelled\b|\bcanceled\b|failure_actioncancelled)/iu.test( + error, + ); + } + + if (error instanceof Error) { + return ( + this.#hasUserRejectedMessage(error.message, visited) || + this.#hasUserRejectedMessage(error.stack, visited) || + this.#hasUserRejectedMessage( + error as Error & { cause?: unknown; originalError?: unknown }, + visited, + ) + ); + } + + if (typeof error === 'object') { + const objectError = error as { + message?: unknown; + stack?: unknown; + cause?: unknown; + originalError?: unknown; + }; + return ( + this.#hasUserRejectedMessage(objectError.message, visited) || + this.#hasUserRejectedMessage(objectError.stack, visited) || + this.#hasUserRejectedMessage(objectError.cause, visited) || + this.#hasUserRejectedMessage(objectError.originalError, visited) + ); + } + + return false; + } + #rejectTransactionAndThrow( transactionId: string, actionId: string | undefined, @@ -4522,6 +4575,13 @@ export class TransactionController extends BaseController< error: Error, actionId?: string, ): void { + const errorToPersist = this.#hasUserRejectedMessage(error) + ? providerErrors.userRejectedRequest({ + message: 'MetaMask Tx Signature: User denied transaction signature.', + data: (error as Error & { data?: Json })?.data, + }) + : error; + let newTransactionMeta: TransactionMeta; try { @@ -4537,7 +4597,7 @@ export class TransactionController extends BaseController< draftTransactionMeta as TransactionMeta & { status: TransactionStatus.failed; } - ).error = normalizeTxError(error); + ).error = normalizeTxError(errorToPersist); }, ); } catch (caughtError: unknown) { @@ -4546,13 +4606,13 @@ export class TransactionController extends BaseController< newTransactionMeta = { ...transactionMeta, status: TransactionStatus.failed, - error: normalizeTxError(error), + error: normalizeTxError(errorToPersist), }; } this.messenger.publish(`${controllerName}:transactionFailed`, { actionId, - error: error.message, + error: errorToPersist.message, transactionMeta: newTransactionMeta, });