diff --git a/README.md b/README.md index d3bba33aa..759c4b6bc 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,7 @@ linkStyle default opacity:0.5 eth_qr_keyring --> account_api; eth_simple_keyring --> keyring_api; eth_simple_keyring --> keyring_utils; + eth_trezor_keyring --> hw_wallet_sdk; eth_trezor_keyring --> keyring_api; eth_trezor_keyring --> keyring_utils; eth_trezor_keyring --> account_api; diff --git a/packages/hw-wallet-sdk/CHANGELOG.md b/packages/hw-wallet-sdk/CHANGELOG.md index 983e1ceba..f5a6da8ca 100644 --- a/packages/hw-wallet-sdk/CHANGELOG.md +++ b/packages/hw-wallet-sdk/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `TREZOR_ERROR_MAPPINGS` static error data for Trezor hardware wallets ([#471](https://github.com/MetaMask/accounts/pull/471)) + ## [0.5.0] ### Added diff --git a/packages/hw-wallet-sdk/src/hardware-error-mappings.test.ts b/packages/hw-wallet-sdk/src/hardware-error-mappings.test.ts index 9431b68ab..43db9df60 100644 --- a/packages/hw-wallet-sdk/src/hardware-error-mappings.test.ts +++ b/packages/hw-wallet-sdk/src/hardware-error-mappings.test.ts @@ -2,6 +2,7 @@ import { LEDGER_ERROR_MAPPINGS, BLE_ERROR_MAPPINGS, MOBILE_ERROR_MAPPINGS, + TREZOR_ERROR_MAPPINGS, } from './hardware-error-mappings'; import type { ErrorMapping } from './hardware-error-mappings'; import { ErrorCode, Severity, Category } from './hardware-errors-enums'; @@ -244,4 +245,44 @@ describe('HARDWARE_ERROR_MAPPINGS', () => { }); }); }); + + describe('Trezor mappings', () => { + it('has TREZOR_ERROR_MAPPINGS object', () => { + expect(TREZOR_ERROR_MAPPINGS).toBeDefined(); + expect(typeof TREZOR_ERROR_MAPPINGS).toBe('object'); + }); + + it('has valid structure for all mappings', () => { + Object.values(TREZOR_ERROR_MAPPINGS).forEach((mapping) => { + expect(mapping).toHaveProperty('code'); + expect(mapping).toHaveProperty('message'); + expect(mapping).toHaveProperty('severity'); + expect(mapping).toHaveProperty('category'); + + const numericErrorCodes = Object.values(ErrorCode).filter( + (value): value is number => typeof value === 'number', + ); + expect(numericErrorCodes).toContain(mapping.code); + expect(Object.values(Severity)).toContain(mapping.severity); + expect(Object.values(Category)).toContain(mapping.category); + expect(typeof mapping.message).toBe('string'); + }); + }); + + it('maps Init_IframeTimeout to ConnectionTimeout', () => { + expect(TREZOR_ERROR_MAPPINGS.Init_IframeTimeout).toMatchObject({ + code: ErrorCode.ConnectionTimeout, + severity: Severity.Err, + category: Category.Connection, + }); + }); + + it('maps Transport_Missing to ConnectionTransportMissing', () => { + expect(TREZOR_ERROR_MAPPINGS.Transport_Missing).toMatchObject({ + code: ErrorCode.ConnectionTransportMissing, + severity: Severity.Err, + category: Category.Connection, + }); + }); + }); }); diff --git a/packages/hw-wallet-sdk/src/hardware-error-mappings.ts b/packages/hw-wallet-sdk/src/hardware-error-mappings.ts index 07dc73781..1b576c133 100644 --- a/packages/hw-wallet-sdk/src/hardware-error-mappings.ts +++ b/packages/hw-wallet-sdk/src/hardware-error-mappings.ts @@ -220,3 +220,219 @@ export const MOBILE_ERROR_MAPPINGS = { userMessage: 'This operation is not supported on mobile devices.', }, }; + +/* eslint-disable @typescript-eslint/naming-convention */ +/** + * Trezor error mappings - static error data for Trezor hardware wallets. + * These mappings provide consistent error classification across Trezor integrations. + */ +export const TREZOR_ERROR_MAPPINGS: Record = { + Transport_Missing: { + code: ErrorCode.ConnectionTransportMissing, + message: 'Trezor transport is unavailable', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'Unable to connect to your Trezor device. Please reconnect and try again.', + }, + Device_Disconnected: { + code: ErrorCode.DeviceDisconnected, + message: 'Trezor device disconnected', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'Your Trezor device was disconnected. Please reconnect and try again.', + }, + Popup_ConnectionMissing: { + code: ErrorCode.ConnectionClosed, + message: 'Trezor popup connection unavailable', + severity: Severity.Err, + category: Category.Connection, + userMessage: 'Connection to your Trezor device popup failed. Please retry.', + }, + Desktop_ConnectionMissing: { + code: ErrorCode.ConnectionClosed, + message: 'Trezor desktop connection unavailable', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'Connection to Trezor Suite failed. Please retry with your device connected.', + }, + Method_Interrupted: { + code: ErrorCode.ConnectionClosed, + message: 'Trezor action was interrupted', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'Connection to your Trezor device was closed. Please reconnect and try again.', + }, + Method_Cancel: { + code: ErrorCode.UserCancelled, + message: 'User cancelled action on Trezor device', + severity: Severity.Warning, + category: Category.UserAction, + userMessage: 'Action was cancelled on your Trezor device.', + }, + Method_PermissionsNotGranted: { + code: ErrorCode.UserRejected, + message: 'Permission not granted on Trezor device', + severity: Severity.Warning, + category: Category.UserAction, + userMessage: 'Permission was rejected on your Trezor device.', + }, + Failure_ActionCancelled: { + code: ErrorCode.UserCancelled, + message: 'User cancelled action on Trezor device', + severity: Severity.Warning, + category: Category.UserAction, + userMessage: 'Action was cancelled on your Trezor device.', + }, + Device_InvalidState: { + code: ErrorCode.AuthenticationFailed, + message: 'Trezor device authentication failed', + severity: Severity.Err, + category: Category.Authentication, + userMessage: + 'Authentication failed on your Trezor device. Check your passphrase and retry.', + }, + Device_CallInProgress: { + code: ErrorCode.DeviceCallInProgress, + message: 'Trezor device call already in progress', + severity: Severity.Err, + category: Category.DeviceState, + userMessage: + 'Your Trezor device is busy. Finish the current action and retry.', + }, + Init_IframeTimeout: { + code: ErrorCode.ConnectionTimeout, + message: 'Trezor connection timed out', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'Connection to your Trezor device timed out. Please try again.', + }, + Init_IframeBlocked: { + code: ErrorCode.ConnectionBlocked, + message: 'Trezor iframe blocked', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'Trezor connection popup was blocked. Please allow popups and try again.', + }, + Init_ManifestMissing: { + code: ErrorCode.Unknown, + message: 'Trezor manifest is missing', + severity: Severity.Err, + category: Category.Configuration, + userMessage: + 'Trezor integration is not configured correctly. Please retry later.', + }, + Device_NotFound: { + code: ErrorCode.DeviceNotFound, + message: 'Trezor device not found', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'No Trezor device found. Please connect your device and try again.', + }, + Device_UsedElsewhere: { + code: ErrorCode.DeviceUsedElsewhere, + message: 'Trezor device is used elsewhere', + severity: Severity.Err, + category: Category.DeviceState, + userMessage: + 'Your Trezor device is busy in another window. Close the other flow and try again.', + }, + Device_MultipleNotSupported: { + code: ErrorCode.DeviceMultipleConnected, + message: 'Multiple Trezor devices are not supported', + severity: Severity.Err, + category: Category.DeviceState, + userMessage: + 'Multiple Trezor devices are connected. Keep one connected and retry.', + }, + Device_MissingCapability: { + code: ErrorCode.DeviceMissingCapability, + message: 'Trezor device is missing capability', + severity: Severity.Err, + category: Category.DeviceState, + userMessage: + 'Your Trezor firmware does not support this action. Please update and retry.', + }, + Device_MissingCapabilityBtcOnly: { + code: ErrorCode.DeviceBtcOnlyFirmware, + message: 'Trezor device firmware only supports BTC', + severity: Severity.Err, + category: Category.DeviceState, + userMessage: + 'Your Trezor firmware currently supports BTC only. Update firmware and retry.', + }, + Failure_PinCancelled: { + code: ErrorCode.AuthenticationPinCancelled, + message: 'Trezor PIN entry cancelled', + severity: Severity.Warning, + category: Category.Authentication, + userMessage: 'PIN entry was cancelled on your Trezor device.', + }, + Failure_PinInvalid: { + code: ErrorCode.AuthenticationIncorrectPin, + message: 'Trezor PIN is invalid', + severity: Severity.Err, + category: Category.Authentication, + userMessage: 'The PIN is incorrect. Please try again.', + }, + Failure_PinMismatch: { + code: ErrorCode.AuthenticationIncorrectPin, + message: 'Trezor PIN mismatch', + severity: Severity.Err, + category: Category.Authentication, + userMessage: 'The PIN does not match. Please try again.', + }, + Failure_WipeCodeMismatch: { + code: ErrorCode.AuthenticationWipeCodeMismatch, + message: 'Trezor wipe code mismatch', + severity: Severity.Err, + category: Category.Authentication, + userMessage: 'The wipe code does not match. Please verify and try again.', + }, + Device_ModeException: { + code: ErrorCode.DeviceIncompatibleMode, + message: 'Trezor device mode is incompatible', + severity: Severity.Err, + category: Category.DeviceState, + userMessage: + 'Your Trezor is in an incompatible mode for this action. Check the device and retry.', + }, + Device_ThpPairingTagInvalid: { + code: ErrorCode.AuthenticationSecurityCondition, + message: 'Trezor pairing security check failed', + severity: Severity.Err, + category: Category.Authentication, + userMessage: + 'A security check failed on your Trezor device. Reconnect and try again.', + }, + Backend_Disconnected: { + code: ErrorCode.ConnectionClosed, + message: 'Trezor backend disconnected', + severity: Severity.Err, + category: Category.Connection, + userMessage: 'Trezor backend disconnected. Please retry.', + }, + Method_NoResponse: { + code: ErrorCode.ConnectionClosed, + message: 'Trezor call returned no response', + severity: Severity.Err, + category: Category.Connection, + userMessage: + 'Trezor did not return a response. Reconnect your device and try again.', + }, + Device_InitializeFailed: { + code: ErrorCode.AuthenticationDeviceLocked, + message: 'Trezor device initialization failed', + severity: Severity.Err, + category: Category.Authentication, + userMessage: + 'Your Trezor device failed to initialize. Please unlock it and try again.', + }, +}; +/* eslint-enable @typescript-eslint/naming-convention */ diff --git a/packages/keyring-eth-trezor/CHANGELOG.md b/packages/keyring-eth-trezor/CHANGELOG.md index 21974f882..08b7fb037 100644 --- a/packages/keyring-eth-trezor/CHANGELOG.md +++ b/packages/keyring-eth-trezor/CHANGELOG.md @@ -13,6 +13,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Wraps legacy `TrezorKeyring` and `OneKeyKeyring` to expose accounts via the unified `KeyringV2` API and the `KeyringAccount` type. - Extends `EthKeyringWrapper` for common Ethereum logic. +### Changed + +- Integrate `@metamask/hw-wallet-sdk` for standardized Trezor error handling ([#471](https://github.com/MetaMask/accounts/pull/471)) + - Replace custom transport and user-action error handling with typed `HardwareWalletError` instances. + - Move Trezor error mappings and utilities to `@metamask/hw-wallet-sdk` for reuse across packages. + - Import `createTrezorError` and `getTrezorErrorIdentifier` from `@metamask/hw-wallet-sdk`. + ## [9.0.0] ### Changed diff --git a/packages/keyring-eth-trezor/jest.config.js b/packages/keyring-eth-trezor/jest.config.js index aaf749f04..09d8ee193 100644 --- a/packages/keyring-eth-trezor/jest.config.js +++ b/packages/keyring-eth-trezor/jest.config.js @@ -23,10 +23,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 62.65, - functions: 93.15, - lines: 93.57, - statements: 93.66, + branches: 86.5, + functions: 97.14, + lines: 98.42, + statements: 98.44, }, }, }); diff --git a/packages/keyring-eth-trezor/package.json b/packages/keyring-eth-trezor/package.json index 60544d846..bc4c360a5 100644 --- a/packages/keyring-eth-trezor/package.json +++ b/packages/keyring-eth-trezor/package.json @@ -49,6 +49,7 @@ "@ethereumjs/tx": "^5.4.0", "@ethereumjs/util": "^9.1.0", "@metamask/eth-sig-util": "^8.2.0", + "@metamask/hw-wallet-sdk": "workspace:^", "@metamask/keyring-api": "workspace:^", "@metamask/keyring-utils": "workspace:^", "@metamask/utils": "^11.1.0", diff --git a/packages/keyring-eth-trezor/src/index.ts b/packages/keyring-eth-trezor/src/index.ts index 2f22f6bb8..5423a3f80 100644 --- a/packages/keyring-eth-trezor/src/index.ts +++ b/packages/keyring-eth-trezor/src/index.ts @@ -2,5 +2,7 @@ export * from './trezor-keyring'; export * from './trezor-keyring-v2'; export * from './onekey-keyring'; export * from './onekey-keyring-v2'; +export * from './trezor-error-handler'; +export * from './trezor-errors'; export type * from './trezor-bridge'; export * from './trezor-connect-bridge'; diff --git a/packages/keyring-eth-trezor/src/trezor-error-handler.test.ts b/packages/keyring-eth-trezor/src/trezor-error-handler.test.ts new file mode 100644 index 000000000..bc98bf167 --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-error-handler.test.ts @@ -0,0 +1,157 @@ +import { + HardwareWalletError, + ErrorCode, + Severity, + Category, +} from '@metamask/hw-wallet-sdk'; + +import { handleTrezorTransportError } from './trezor-error-handler'; + +describe('handleTrezorTransportError', () => { + const fallbackMessage = 'Default Trezor error'; + + it.each([ + { + tc: 'transport missing', + input: Object.assign(new Error('error'), { + code: 'Transport_Missing', + }), + code: ErrorCode.ConnectionTransportMissing, + }, + { + tc: 'disconnected device', + input: Object.assign(new Error('error'), { + code: 'Device_Disconnected', + }), + code: ErrorCode.DeviceDisconnected, + }, + { + tc: 'closed popup/session', + input: Object.assign(new Error('error'), { + code: 'Method_Interrupted', + }), + code: ErrorCode.ConnectionClosed, + }, + { + tc: 'cancelled action', + input: Object.assign(new Error('error'), { code: 'Method_Cancel' }), + code: ErrorCode.UserCancelled, + }, + { + tc: 'rejected action', + input: Object.assign(new Error('error'), { + code: 'Method_PermissionsNotGranted', + }), + code: ErrorCode.UserRejected, + }, + { + tc: 'timeout', + input: Object.assign(new Error('error'), { + code: 'Init_IframeTimeout', + }), + code: ErrorCode.ConnectionTimeout, + }, + ])('maps $tc to HardwareWalletError', ({ input, code }) => { + let thrownError: unknown; + try { + handleTrezorTransportError(input, fallbackMessage); + } catch (error) { + thrownError = error; + } + + expect(thrownError).toBeInstanceOf(HardwareWalletError); + expect((thrownError as HardwareWalletError).code).toBe(code); + }); + + it('prioritizes machine-readable code when present', () => { + const error = new Error('error') as Error & { code: string }; + error.code = 'Method_PermissionsNotGranted'; + + let thrownError: unknown; + try { + handleTrezorTransportError(error, fallbackMessage); + } catch (error_) { + thrownError = error_; + } + + expect(thrownError).toBeInstanceOf(HardwareWalletError); + expect((thrownError as HardwareWalletError).code).toBe( + ErrorCode.UserRejected, + ); + }); + + it('uses error name as fallback identifier when code is absent', () => { + const error = new Error('error'); + error.name = 'Device_Disconnected'; + + let thrownError: unknown; + try { + handleTrezorTransportError(error, fallbackMessage); + } catch (error_) { + thrownError = error_; + } + + expect(thrownError).toBeInstanceOf(HardwareWalletError); + expect((thrownError as HardwareWalletError).code).toBe( + ErrorCode.DeviceDisconnected, + ); + }); + + it('passes through HardwareWalletError instances unchanged', () => { + const originalError = new HardwareWalletError('original', { + code: ErrorCode.UserRejected, + severity: Severity.Warning, + category: Category.UserAction, + userMessage: 'original', + }); + + let thrownError: unknown; + try { + handleTrezorTransportError(originalError, fallbackMessage); + } catch (error) { + thrownError = error; + } + + expect(thrownError).toBe(originalError); + }); + + it('wraps unknown Error instances as ErrorCode.Unknown', () => { + const originalError = new Error('Unexpected Trezor failure'); + + let thrownError: unknown; + try { + handleTrezorTransportError(originalError, fallbackMessage); + } catch (error) { + thrownError = error; + } + + expect(thrownError).toBeInstanceOf(HardwareWalletError); + expect((thrownError as HardwareWalletError).code).toBe(ErrorCode.Unknown); + expect((thrownError as HardwareWalletError).cause).toBe(originalError); + expect((thrownError as HardwareWalletError).message).toBe( + 'Unexpected Trezor failure', + ); + }); + + it.each([null, undefined, 'string error', { message: 'not an error' }])( + 'uses fallback for non-Error input: %p', + (value) => { + const throwingFunction = (): never => + handleTrezorTransportError(value, fallbackMessage); + + expect(throwingFunction).toThrow(HardwareWalletError); + expect(throwingFunction).toThrow(fallbackMessage); + }, + ); + + it('has never return type', () => { + type ReturnTypeIsNever = ReturnType< + typeof handleTrezorTransportError + > extends never + ? true + : false; + + const isNever: ReturnTypeIsNever = true; + expect(isNever).toBe(true); + }); +}); diff --git a/packages/keyring-eth-trezor/src/trezor-error-handler.ts b/packages/keyring-eth-trezor/src/trezor-error-handler.ts new file mode 100644 index 000000000..eae0e09c3 --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-error-handler.ts @@ -0,0 +1,73 @@ +import { + ErrorCode, + Severity, + Category, + HardwareWalletError, +} from '@metamask/hw-wallet-sdk'; + +import { createTrezorError, isKnownTrezorError } from './trezor-errors'; + +type ErrorDetails = { + message?: string; + code?: string; + name?: string; +}; + +function getErrorDetails(error: Error): ErrorDetails { + const details: ErrorDetails = { + message: error.message, + name: error.name, + }; + + if ('code' in error) { + const { code } = error as Error & { code?: unknown }; + if (typeof code === 'string') { + details.code = code; + } + } + + return details; +} + +/** + * Converts unknown Trezor errors into typed HardwareWalletError instances. + * + * @param error - Error thrown from Trezor bridge or keyring flow. + * @param fallbackMessage - Default message for unknown non-Error inputs. + * @throws HardwareWalletError Always throws typed errors. + */ +export function handleTrezorTransportError( + error: unknown, + fallbackMessage: string, +): never { + if (error instanceof HardwareWalletError) { + throw error; + } + + if (error instanceof Error) { + const details = getErrorDetails(error); + const identifier = [details.code, details.name, details.message].find( + (value): value is string => + value !== undefined && isKnownTrezorError(value), + ); + + if (identifier) { + throw createTrezorError(identifier, details.message); + } + + throw new HardwareWalletError(details.message ?? fallbackMessage, { + code: ErrorCode.Unknown, + severity: Severity.Err, + category: Category.Unknown, + userMessage: details.message ?? fallbackMessage, + cause: error, + }); + } + + throw new HardwareWalletError(fallbackMessage, { + code: ErrorCode.Unknown, + severity: Severity.Err, + category: Category.Unknown, + userMessage: fallbackMessage, + }); +} diff --git a/packages/keyring-eth-trezor/src/trezor-errors.test.ts b/packages/keyring-eth-trezor/src/trezor-errors.test.ts new file mode 100644 index 000000000..70f161be2 --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-errors.test.ts @@ -0,0 +1,72 @@ +import { + Category, + ErrorCode, + HardwareWalletError, + Severity, + TREZOR_ERROR_MAPPINGS, +} from '@metamask/hw-wallet-sdk'; + +import { + createTrezorError, + getTrezorErrorMapping, + isKnownTrezorError, +} from './trezor-errors'; + +describe('trezor-errors', () => { + describe('isKnownTrezorError', () => { + it('returns true for known identifiers', () => { + expect(isKnownTrezorError('Device_Disconnected')).toBe(true); + expect(isKnownTrezorError('Method_Cancel')).toBe(true); + }); + + it('returns false for unknown identifiers', () => { + expect(isKnownTrezorError('unknownIdentifier')).toBe(false); + }); + }); + + describe('getTrezorErrorMapping', () => { + it('maps all TREZOR_ERROR_MAPPINGS error codes', () => { + for (const identifier of Object.keys(TREZOR_ERROR_MAPPINGS)) { + expect(getTrezorErrorMapping(identifier)).toBeDefined(); + } + }); + + it('returns mapping for known identifiers', () => { + expect(getTrezorErrorMapping('Init_IframeTimeout')).toMatchObject({ + code: ErrorCode.ConnectionTimeout, + severity: Severity.Err, + category: Category.Connection, + }); + }); + + it('returns undefined for unknown identifiers', () => { + expect(getTrezorErrorMapping('not-real')).toBeUndefined(); + }); + }); + + describe('createTrezorError', () => { + it('creates typed errors for known identifiers', () => { + const error = createTrezorError('Transport_Missing'); + + expect(error).toBeInstanceOf(HardwareWalletError); + expect(error.code).toBe(ErrorCode.ConnectionTransportMissing); + expect(error.severity).toBe(Severity.Err); + expect(error.category).toBe(Category.Connection); + }); + + it('appends context to the error message', () => { + const error = createTrezorError('Method_Cancel', 'during sign operation'); + expect(error.message).toContain('(during sign operation)'); + }); + + it('falls back to ErrorCode.Unknown for unknown identifiers', () => { + const error = createTrezorError('not-real', 'while testing'); + expect(error).toBeInstanceOf(HardwareWalletError); + expect(error.code).toBe(ErrorCode.Unknown); + expect(error.category).toBe(Category.Unknown); + expect(error.userMessage).toBe( + 'Unknown Trezor error: not-real (while testing)', + ); + }); + }); +}); diff --git a/packages/keyring-eth-trezor/src/trezor-errors.ts b/packages/keyring-eth-trezor/src/trezor-errors.ts new file mode 100644 index 000000000..c6107e9bf --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-errors.ts @@ -0,0 +1,69 @@ +import { + type ErrorMapping, + TREZOR_ERROR_MAPPINGS, + ErrorCode, + Severity, + Category, + HardwareWalletError, +} from '@metamask/hw-wallet-sdk'; + +/** + * Factory function to create a HardwareWalletError from a Trezor error identifier. + * + * @param trezorErrorIdentifier - The Trezor error identifier (e.g., 'Device_Disconnected', 'Method_Cancel') + * @param context - Optional additional context to append to the error message + * @returns A HardwareWalletError instance with mapped error details + */ +export function createTrezorError( + trezorErrorIdentifier: string, + context?: string, +): HardwareWalletError { + const errorMapping = getTrezorErrorMapping(trezorErrorIdentifier); + + if (errorMapping) { + const message = context + ? `${errorMapping.message} (${context})` + : errorMapping.message; + + return new HardwareWalletError(message, { + code: errorMapping.code, + severity: errorMapping.severity, + category: errorMapping.category, + userMessage: errorMapping.userMessage ?? message, + }); + } + + // Fallback for unknown error codes + const fallbackMessage = context + ? `Unknown Trezor error: ${trezorErrorIdentifier} (${context})` + : `Unknown Trezor error: ${trezorErrorIdentifier}`; + + return new HardwareWalletError(fallbackMessage, { + code: ErrorCode.Unknown, + severity: Severity.Err, + category: Category.Unknown, + userMessage: fallbackMessage, + }); +} + +/** + * Checks if a Trezor error identifier exists in the error mappings. + * + * @param trezorErrorIdentifier - The Trezor error identifier to check + * @returns True if the error identifier is mapped, false otherwise + */ +export function isKnownTrezorError(trezorErrorIdentifier: string): boolean { + return trezorErrorIdentifier in TREZOR_ERROR_MAPPINGS; +} + +/** + * Gets the error mapping details for a Trezor error identifier without creating an error instance. + * + * @param trezorErrorIdentifier - The Trezor error identifier to look up + * @returns The error mapping details or undefined if not found + */ +export function getTrezorErrorMapping( + trezorErrorIdentifier: string, +): ErrorMapping | undefined { + return TREZOR_ERROR_MAPPINGS[trezorErrorIdentifier]; +} diff --git a/packages/keyring-eth-trezor/src/trezor-keyring.test.ts b/packages/keyring-eth-trezor/src/trezor-keyring.test.ts index 2747b5079..2991ca81a 100644 --- a/packages/keyring-eth-trezor/src/trezor-keyring.test.ts +++ b/packages/keyring-eth-trezor/src/trezor-keyring.test.ts @@ -6,6 +6,7 @@ import { } from '@ethereumjs/tx'; import { Address } from '@ethereumjs/util'; import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { ErrorCode, HardwareWalletError } from '@metamask/hw-wallet-sdk'; import EthereumTx from 'ethereumjs-tx'; import HDKey from 'hdkey'; import * as sinon from 'sinon'; @@ -556,6 +557,23 @@ describe('TrezorKeyring', function () { ...expectedRSV, }); }); + + it('converts message-only failures to ErrorCode.Unknown', async function () { + const ethereumSignTransactionStub = sinon.stub().resolves({ + success: false, + payload: { error: 'Trezor device disconnected' }, + }); + bridge.ethereumSignTransaction = ethereumSignTransactionStub; + + await expect( + keyring.signTransaction(fakeAccounts[0], fakeTx), + ).rejects.toThrow(HardwareWalletError); + await expect( + keyring.signTransaction(fakeAccounts[0], fakeTx), + ).rejects.toMatchObject({ + code: ErrorCode.Unknown, + }); + }); }); describe('signMessage', function () { @@ -588,6 +606,23 @@ describe('TrezorKeyring', function () { expect(ethereumSignMessageStub.calledOnce).toBe(true); }); + + it('converts message-only failures to ErrorCode.Unknown', async function () { + const ethereumSignMessageStub = sinon.stub().resolves({ + success: false, + payload: { error: 'User cancelled action' }, + }); + bridge.ethereumSignMessage = ethereumSignMessageStub; + + await expect( + keyring.signPersonalMessage(fakeAccounts[0], 'some msg'), + ).rejects.toThrow(HardwareWalletError); + await expect( + keyring.signPersonalMessage(fakeAccounts[0], 'some msg'), + ).rejects.toMatchObject({ + code: ErrorCode.Unknown, + }); + }); }); describe('signTypedData', function () { @@ -659,6 +694,42 @@ describe('TrezorKeyring', function () { 'c9e71eb57cf9fa86ec670283b58cb15326bb6933c8d8e2ecb2c0849021b3ef42', }); }); + + it('converts unknown typed-data signing failures to ErrorCode.Unknown', async function () { + const ethereumSignTypedDataStub = sinon.stub().resolves({ + success: false, + payload: { error: 'Unexpected bridge failure' }, + }); + bridge.ethereumSignTypedData = ethereumSignTypedDataStub; + + await expect( + keyring.signTypedData( + fakeAccounts[0], + { + types: { EIP712Domain: [], EmptyMessage: [] }, + primaryType: 'EmptyMessage', + domain: {}, + message: {}, + }, + { version: SignTypedDataVersion.V4 }, + ), + ).rejects.toThrow(HardwareWalletError); + + await expect( + keyring.signTypedData( + fakeAccounts[0], + { + types: { EIP712Domain: [], EmptyMessage: [] }, + primaryType: 'EmptyMessage', + domain: {}, + message: {}, + }, + { version: SignTypedDataVersion.V4 }, + ), + ).rejects.toMatchObject({ + code: ErrorCode.Unknown, + }); + }); }); describe('forgetDevice', function () { @@ -732,4 +803,221 @@ describe('TrezorKeyring', function () { ); }); }); + + describe('getModel', function () { + it('returns the model from the bridge', function () { + bridge.model = 'Trezor Model T'; + expect(keyring.getModel()).toBe('Trezor Model T'); + }); + + it('returns undefined if model is not set', function () { + bridge.model = undefined; + expect(keyring.getModel()).toBeUndefined(); + }); + }); + + describe('forgetDevice state reset', function () { + it('resets all keyring state', async function () { + // Add an account + keyring.setAccountToUnlock(0); + await keyring.addAccounts(1); + + // Set some state + // eslint-disable-next-line require-atomic-updates + keyring.page = 5; + // eslint-disable-next-line require-atomic-updates + keyring.unlockedAccount = 3; + // eslint-disable-next-line require-atomic-updates + keyring.paths = { [fakeAccounts[0]]: 0 }; + + // Forget the device + keyring.forgetDevice(); + + expect(keyring.accounts).toHaveLength(0); + expect(keyring.hdk.publicKey).toBeNull(); + expect(keyring.page).toBe(0); + expect(keyring.unlockedAccount).toBe(0); + expect(keyring.paths).toStrictEqual({}); + }); + }); + + describe('signMessage delegation', function () { + it('delegates to signPersonalMessage', async function () { + const ethereumSignMessageStub = sinon.stub().resolves({ + success: true, + payload: { signature: 'signature', address: fakeAccounts[0] }, + }); + bridge.ethereumSignMessage = ethereumSignMessageStub; + + const result = await keyring.signMessage(fakeAccounts[0], '0xmessage'); + + expect(ethereumSignMessageStub.calledOnce).toBe(true); + expect(result).toBe('0xsignature'); + }); + }); + + describe('signPersonalMessage error handling', function () { + it('signs a personal message successfully', async function () { + const ethereumSignMessageStub = sinon.stub().resolves({ + success: true, + payload: { signature: 'signature', address: fakeAccounts[0] }, + }); + bridge.ethereumSignMessage = ethereumSignMessageStub; + + const result = await keyring.signPersonalMessage( + fakeAccounts[0], + '0xmessage', + ); + + expect(ethereumSignMessageStub.calledOnce).toBe(true); + expect(result).toBe('0xsignature'); + }); + + it('throws error when signature address does not match', async function () { + const ethereumSignMessageStub = sinon.stub().resolves({ + success: true, + payload: { signature: 'signature', address: fakeAccounts[1] }, + }); + bridge.ethereumSignMessage = ethereumSignMessageStub; + + await expect( + keyring.signPersonalMessage(fakeAccounts[0], '0xmessage'), + ).rejects.toThrow('signature doesnt match the right address'); + }); + + it('converts non-address errors to HardwareWalletError', async function () { + const ethereumSignMessageStub = sinon.stub().resolves({ + success: false, + payload: { error: 'Device disconnected' }, + }); + bridge.ethereumSignMessage = ethereumSignMessageStub; + + await expect( + keyring.signPersonalMessage(fakeAccounts[0], '0xmessage'), + ).rejects.toThrow(HardwareWalletError); + }); + }); + + describe('signTypedData error handling', function () { + it('signs typed data successfully', async function () { + const ethereumSignTypedDataStub = sinon.stub().resolves({ + success: true, + payload: { signature: '0xsignature', address: fakeAccounts[0] }, + }); + bridge.ethereumSignTypedData = ethereumSignTypedDataStub; + + const result = await keyring.signTypedData( + fakeAccounts[0], + { + types: { EIP712Domain: [], EmptyMessage: [] }, + primaryType: 'EmptyMessage', + domain: {}, + message: {}, + }, + { version: SignTypedDataVersion.V4 }, + ); + + expect(ethereumSignTypedDataStub.calledOnce).toBe(true); + expect(result).toBe('0xsignature'); + }); + + it('throws error when signature address does not match', async function () { + const ethereumSignTypedDataStub = sinon.stub().resolves({ + success: true, + payload: { signature: '0xsignature', address: fakeAccounts[1] }, + }); + bridge.ethereumSignTypedData = ethereumSignTypedDataStub; + + await expect( + keyring.signTypedData( + fakeAccounts[0], + { + types: { EIP712Domain: [], EmptyMessage: [] }, + primaryType: 'EmptyMessage', + domain: {}, + message: {}, + }, + { version: SignTypedDataVersion.V4 }, + ), + ).rejects.toThrow('signature doesnt match the right address'); + }); + + it('converts non-address errors to HardwareWalletError', async function () { + const ethereumSignTypedDataStub = sinon.stub().resolves({ + success: false, + payload: { error: 'Device disconnected' }, + }); + bridge.ethereumSignTypedData = ethereumSignTypedDataStub; + + await expect( + keyring.signTypedData( + fakeAccounts[0], + { + types: { EIP712Domain: [], EmptyMessage: [] }, + primaryType: 'EmptyMessage', + domain: {}, + message: {}, + }, + { version: SignTypedDataVersion.V4 }, + ), + ).rejects.toThrow(HardwareWalletError); + }); + }); + + describe('unlock error handling', function () { + it('handles unsuccessful response from getPublicKey', async function () { + const getPublicKeyStub = sinon.stub().resolves({ + success: false, + payload: { error: 'Device not connected' }, + }); + bridge.getPublicKey = getPublicKeyStub; + + keyring.hdk = new HDKey(); + + await expect(keyring.unlock()).rejects.toThrow('Device not connected'); + }); + + it('converts unlock errors to HardwareWalletError', async function () { + const getPublicKeyStub = sinon + .stub() + .rejects(new Error('Transport error')); + bridge.getPublicKey = getPublicKeyStub; + + keyring.hdk = new HDKey(); + + await expect(keyring.unlock()).rejects.toThrow(HardwareWalletError); + }); + }); + + describe('signTransaction error handling', function () { + it('throws HardwareWalletError on bridge failure', async function () { + const ethereumSignTransactionStub = sinon.stub().resolves({ + success: false, + payload: { error: 'Device disconnected' }, + }); + bridge.ethereumSignTransaction = ethereumSignTransactionStub; + + await expect( + keyring.signTransaction(fakeAccounts[0], fakeTx), + ).rejects.toThrow(HardwareWalletError); + }); + + it('throws error when signature address does not match', async function () { + const ethereumSignTransactionStub = sinon.stub().resolves({ + success: true, + payload: { v: '0x1', r: '0x0', s: '0x0' }, + }); + bridge.ethereumSignTransaction = ethereumSignTransactionStub; + + sinon + .stub(fakeTx, 'getSenderAddress') + .callsFake(() => + Buffer.from(Address.fromString(fakeAccounts[1]).bytes), + ); + + await expect( + keyring.signTransaction(fakeAccounts[0], fakeTx), + ).rejects.toThrow("signature doesn't match the right address"); + }); + }); }); diff --git a/packages/keyring-eth-trezor/src/trezor-keyring.ts b/packages/keyring-eth-trezor/src/trezor-keyring.ts index 5706dc3c5..15b151b14 100644 --- a/packages/keyring-eth-trezor/src/trezor-keyring.ts +++ b/packages/keyring-eth-trezor/src/trezor-keyring.ts @@ -24,6 +24,7 @@ import type OldEthJsTransaction from 'ethereumjs-tx'; import HDKey from 'hdkey'; import { TrezorBridge } from './trezor-bridge'; +import { handleTrezorTransportError } from './trezor-error-handler'; const hdPathString = `m/44'/60'/0'/0`; const SLIP0044TestnetPath = `m/44'/1'/0'/0`; @@ -169,25 +170,25 @@ export class TrezorKeyring implements Keyring { if (this.isUnlocked()) { return Promise.resolve('already unlocked'); } - return new Promise((resolve, reject) => { - this.bridge - .getPublicKey({ - path: this.hdPath, - coin: 'ETH', - }) - .then((response) => { - if (response.success) { - this.hdk.publicKey = Buffer.from(response.payload.publicKey, 'hex'); - this.hdk.chainCode = Buffer.from(response.payload.chainCode, 'hex'); - resolve('just unlocked'); - } else { - reject(new Error(response.payload?.error || 'Unknown error')); - } - }) - .catch((e) => { - reject(new Error(e?.toString() || 'Unknown error')); - }); - }); + + try { + const response = await this.bridge.getPublicKey({ + path: this.hdPath, + coin: 'ETH', + }); + if (!response.success) { + throw new Error(response.payload?.error || 'Unknown error'); + } + + this.hdk.publicKey = Buffer.from(response.payload.publicKey, 'hex'); + this.hdk.chainCode = Buffer.from(response.payload.chainCode, 'hex'); + return 'just unlocked'; + } catch (error) { + return handleTrezorTransportError( + error, + 'Failed to unlock Trezor device', + ); + } } setAccountToUnlock(index: number | string): void { @@ -401,8 +402,18 @@ export class TrezorKeyring implements Keyring { return newOrMutatedTx; } throw new Error(response.payload?.error || 'Unknown error'); - } catch (e) { - throw new Error(e?.toString() ?? 'Unknown error'); + } catch (error) { + // Re-throw address validation errors as plain Errors, not hardware errors + if ( + error instanceof Error && + error.message === "signature doesn't match the right address" + ) { + throw error; + } + return handleTrezorTransportError( + error, + 'Failed to sign transaction with Trezor device', + ); } } @@ -415,48 +426,39 @@ export class TrezorKeyring implements Keyring { withAccount: Hex, message: string, ): Promise { - return new Promise((resolve, reject) => { - this.unlock() - .then((status) => { - setTimeout( - () => { - this.bridge - .ethereumSignMessage({ - path: this.#pathFromAddress(withAccount), - message: remove0x(message), - hex: true, - }) - .then((response) => { - if (response.success) { - if ( - response.payload.address !== - getChecksumAddress(withAccount) - ) { - reject( - new Error('signature doesnt match the right address'), - ); - } - const signature = `0x${response.payload.signature}`; - resolve(signature); - } else { - reject( - new Error(response.payload?.error || 'Unknown error'), - ); - } - }) - .catch((e) => { - reject(new Error(e?.toString() || 'Unknown error')); - }); - // This is necessary to avoid popup collision - // between the unlock & sign trezor popups - }, - status === 'just unlocked' ? DELAY_BETWEEN_POPUPS : 0, - ); - }) - .catch((e) => { - reject(new Error(e?.toString() || 'Unknown error')); - }); - }); + try { + const status = await this.unlock(); + // This is necessary to avoid popup collision + // between the unlock & sign trezor popups + await wait(status === 'just unlocked' ? DELAY_BETWEEN_POPUPS : 0); + const response = await this.bridge.ethereumSignMessage({ + path: this.#pathFromAddress(withAccount), + message: remove0x(message), + hex: true, + }); + + if (!response.success) { + throw new Error(response.payload?.error || 'Unknown error'); + } + + if (response.payload.address !== getChecksumAddress(withAccount)) { + throw new Error('signature doesnt match the right address'); + } + + return `0x${response.payload.signature}`; + } catch (error) { + // Re-throw address validation errors as plain Errors, not hardware errors + if ( + error instanceof Error && + error.message === 'signature doesnt match the right address' + ) { + throw error; + } + return handleTrezorTransportError( + error, + 'Failed to sign personal message with Trezor device', + ); + } } // EIP-712 Sign Typed Data @@ -469,52 +471,66 @@ export class TrezorKeyring implements Keyring { data: TypedMessage, options?: Options, ): Promise { - const { version } = options ?? { version: SignTypedDataVersion.V4 }; + try { + const { version } = options ?? { version: SignTypedDataVersion.V4 }; - const dataWithHashes = transformTypedData( - data, - version === SignTypedDataVersion.V4, - ); + const dataWithHashes = transformTypedData( + data, + version === SignTypedDataVersion.V4, + ); - // set default values for signTypedData - // Trezor is stricter than @metamask/eth-sig-util in what it accepts - const { - types, - message = {}, - domain = {}, - primaryType, - // snake_case since Trezor uses Protobuf naming conventions here - domain_separator_hash, // eslint-disable-line camelcase - message_hash, // eslint-disable-line camelcase - } = dataWithHashes; - - // This is necessary to avoid popup collision - // between the unlock & sign trezor popups - const status = await this.unlock(); - await wait(status === 'just unlocked' ? DELAY_BETWEEN_POPUPS : 0); - - const response = await this.bridge.ethereumSignTypedData({ - path: this.#pathFromAddress(address), - data: { - types: { ...types, EIP712Domain: types.EIP712Domain ?? [] }, - message, - domain, + // set default values for signTypedData + // Trezor is stricter than @metamask/eth-sig-util in what it accepts + const { + types, + message = {}, + domain = {}, primaryType, - }, - metamask_v4_compat: true, // eslint-disable-line camelcase - // Trezor 1 only supports blindly signing hashes - domain_separator_hash, // eslint-disable-line camelcase - message_hash: message_hash ?? '', // eslint-disable-line camelcase - }); + // snake_case since Trezor uses Protobuf naming conventions here + domain_separator_hash, // eslint-disable-line camelcase + message_hash, // eslint-disable-line camelcase + } = dataWithHashes; + + // This is necessary to avoid popup collision + // between the unlock & sign trezor popups + const status = await this.unlock(); + await wait(status === 'just unlocked' ? DELAY_BETWEEN_POPUPS : 0); + + const response = await this.bridge.ethereumSignTypedData({ + path: this.#pathFromAddress(address), + data: { + types: { ...types, EIP712Domain: types.EIP712Domain ?? [] }, + message, + domain, + primaryType, + }, + metamask_v4_compat: true, // eslint-disable-line camelcase + // Trezor 1 only supports blindly signing hashes + domain_separator_hash, // eslint-disable-line camelcase + message_hash: message_hash ?? '', // eslint-disable-line camelcase + }); + + if (!response.success) { + throw new Error(response.payload?.error ?? 'Unknown error'); + } - if (response.success) { if (getChecksumAddress(address) !== response.payload.address) { throw new Error('signature doesnt match the right address'); } return response.payload.signature; + } catch (error) { + // Re-throw address validation errors as plain Errors, not hardware errors + if ( + error instanceof Error && + error.message === 'signature doesnt match the right address' + ) { + throw error; + } + return handleTrezorTransportError( + error, + 'Failed to sign typed data with Trezor device', + ); } - - throw new Error(response.payload?.error || 'Unknown error'); } forgetDevice(): void { diff --git a/packages/keyring-eth-trezor/tsconfig.build.json b/packages/keyring-eth-trezor/tsconfig.build.json index 1d0fc0990..b0dd80e16 100644 --- a/packages/keyring-eth-trezor/tsconfig.build.json +++ b/packages/keyring-eth-trezor/tsconfig.build.json @@ -11,6 +11,7 @@ "target": "es2017" }, "references": [ + { "path": "../hw-wallet-sdk/tsconfig.build.json" }, { "path": "../keyring-utils/tsconfig.build.json" }, { "path": "../keyring-api/tsconfig.build.json" }, { diff --git a/packages/keyring-eth-trezor/tsconfig.json b/packages/keyring-eth-trezor/tsconfig.json index a9d6ac716..9b537c978 100644 --- a/packages/keyring-eth-trezor/tsconfig.json +++ b/packages/keyring-eth-trezor/tsconfig.json @@ -9,6 +9,7 @@ "target": "es2017" }, "references": [ + { "path": "../hw-wallet-sdk" }, { "path": "../keyring-utils" }, { "path": "../keyring-api" }, { "path": "../account-api" } diff --git a/yarn.lock b/yarn.lock index 0ddb96ba1..03e394573 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1900,6 +1900,7 @@ __metadata: "@metamask/account-api": "workspace:^" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/eth-sig-util": "npm:^8.2.0" + "@metamask/hw-wallet-sdk": "workspace:^" "@metamask/keyring-api": "workspace:^" "@metamask/keyring-utils": "workspace:^" "@metamask/utils": "npm:^11.1.0"