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/keyring-eth-trezor/CHANGELOG.md b/packages/keyring-eth-trezor/CHANGELOG.md index 21974f882..46b798487 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. + - Add Trezor-specific error mappings for consistent `ErrorCode`, `Severity`, and `Category` classification. + - Export Trezor error helpers for creating and normalizing typed hardware wallet errors. + ## [9.0.0] ### Changed diff --git a/packages/keyring-eth-trezor/jest.config.js b/packages/keyring-eth-trezor/jest.config.js index aaf749f04..392cfda7c 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: 83.05, + functions: 95.89, + lines: 96.43, + statements: 96.48, }, }, }); 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..4384c20de --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-error-handler.test.ts @@ -0,0 +1,158 @@ +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); + expect((thrownError as HardwareWalletError).cause).toBe(input); + }); + + 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..ad1ea7850 --- /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, getTrezorErrorIdentifier } 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 = + getTrezorErrorIdentifier(details.code) ?? + getTrezorErrorIdentifier(details.name) ?? + getTrezorErrorIdentifier(details.message); + + if (identifier) { + throw createTrezorError(identifier, details.message, error); + } + + 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..38e8d10e9 --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-errors.test.ts @@ -0,0 +1,113 @@ +import { + Category, + ErrorCode, + HardwareWalletError, + Severity, +} from '@metamask/hw-wallet-sdk'; +import { ERRORS } from '@trezor/connect-web'; + +import { + createTrezorError, + getTrezorErrorIdentifier, + 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); + expect(isKnownTrezorError('')).toBe(false); + }); + }); + + describe('getTrezorErrorMapping', () => { + it('maps all current TrezorConnect error codes', () => { + for (const identifier of Object.keys(ERRORS.ERROR_CODES)) { + 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('getTrezorErrorIdentifier', () => { + it('returns undefined for empty values', () => { + expect(getTrezorErrorIdentifier(undefined)).toBeUndefined(); + expect(getTrezorErrorIdentifier('')).toBeUndefined(); + }); + + it('matches known identifiers case-insensitively', () => { + expect(getTrezorErrorIdentifier('Device_Disconnected')).toBe( + 'Device_Disconnected', + ); + expect(getTrezorErrorIdentifier('DEVice_disconnected')).toBe( + 'Device_Disconnected', + ); + }); + + it('maps sdk messages to identifiers', () => { + expect(getTrezorErrorIdentifier('Device disconnected')).toBe( + 'Device_Disconnected', + ); + }); + + it('does not resolve removed legacy identifiers', () => { + expect(getTrezorErrorIdentifier('deviceDisconnected')).toBeUndefined(); + expect(getTrezorErrorIdentifier('connectionTimeout')).toBeUndefined(); + }); + }); + + describe('createTrezorError', () => { + it('creates typed errors for known identifiers', () => { + const cause = new Error('underlying'); + const error = createTrezorError('Transport_Missing', undefined, cause); + + expect(error).toBeInstanceOf(HardwareWalletError); + expect(error.code).toBe(ErrorCode.ConnectionTransportMissing); + expect(error.severity).toBe(Severity.Err); + expect(error.category).toBe(Category.Connection); + expect(error.cause).toBe(cause); + }); + + it('appends context when it differs from mapped message', () => { + const error = createTrezorError('Method_Cancel', 'during sign operation'); + expect(error.message).toContain('(during sign operation)'); + }); + + it('does not append context when it only repeats mapped message casing/spacing', () => { + const error = createTrezorError( + 'Method_Cancel', + ' USER CANCELLED ACTION ON TREZOR DEVICE ', + ); + expect(error.message).toBe('User cancelled action on Trezor device'); + }); + + it('falls back to ErrorCode.Unknown for unknown identifiers', () => { + const cause = new Error('unknown cause'); + const error = createTrezorError('not-real', 'while testing', cause); + 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)', + ); + expect(error.cause).toBe(cause); + }); + }); +}); 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..c6e7e8de1 --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-errors.ts @@ -0,0 +1,335 @@ +import { + type ErrorMapping, + ErrorCode, + Severity, + Category, + HardwareWalletError, +} from '@metamask/hw-wallet-sdk'; +import { ERRORS } from '@trezor/connect-web'; + +const TREZOR_ERROR_OVERRIDES: Partial> = { + 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.', + }, +}; + +const TREZOR_ERROR_CODES = ERRORS.ERROR_CODES as Record; +const TREZOR_ERROR_MAPPINGS: Record = Object.fromEntries( + Object.entries(TREZOR_ERROR_CODES).map(([identifier, sdkMessage]) => [ + identifier, + TREZOR_ERROR_OVERRIDES[identifier] ?? { + code: ErrorCode.Unknown, + message: sdkMessage || `Trezor error (${identifier})`, + severity: Severity.Err, + category: Category.Unknown, + userMessage: + sdkMessage || + `A Trezor error occurred (${identifier}). Please try again.`, + }, + ]), +); + +const NORMALIZED_IDENTIFIER_MAP = new Map(); + +function normalizeValue(value: string): string { + return value.trim().toLowerCase(); +} + +function registerAlias(alias: string, identifier: string): void { + const normalizedAlias = normalizeValue(alias); + if (!normalizedAlias) { + return; + } + NORMALIZED_IDENTIFIER_MAP.set(normalizedAlias, identifier); +} + +for (const identifier of Object.keys(TREZOR_ERROR_MAPPINGS)) { + registerAlias(identifier, identifier); +} + +for (const [identifier, message] of Object.entries(TREZOR_ERROR_CODES)) { + registerAlias(message, identifier); +} + +registerAlias(ERRORS.LIBUSB_ERROR_MESSAGE, 'Transport_Missing'); + +/** + * Checks if a Trezor error identifier has a known mapping. + * + * @param identifier - The identifier to check. + * @returns True if identifier is mapped, false otherwise. + */ +export function isKnownTrezorError(identifier: string): boolean { + return NORMALIZED_IDENTIFIER_MAP.has(normalizeValue(identifier)); +} + +/** + * Gets mapped error details for a Trezor identifier. + * + * @param identifier - The identifier to look up. + * @returns The mapped error details, if available. + */ +export function getTrezorErrorMapping( + identifier: string, +): ErrorMapping | undefined { + const normalizedIdentifier = normalizeValue(identifier); + const mappedIdentifier = NORMALIZED_IDENTIFIER_MAP.get(normalizedIdentifier); + if (!mappedIdentifier) { + return undefined; + } + return TREZOR_ERROR_MAPPINGS[mappedIdentifier]; +} + +/** + * Resolves a deterministic Trezor error identifier from raw text. + * + * @param rawValue - A code/name string. + * @returns The mapped identifier if matched, otherwise undefined. + */ +export function getTrezorErrorIdentifier( + rawValue: string | undefined, +): string | undefined { + if (!rawValue) { + return undefined; + } + return NORMALIZED_IDENTIFIER_MAP.get(normalizeValue(rawValue)); +} + +/** + * Factory to create a typed HardwareWalletError for Trezor errors. + * + * @param identifier - Mapped Trezor identifier. + * @param context - Optional extra context appended to the message. + * @param cause - Optional original cause error. + * @returns A typed HardwareWalletError. + */ +export function createTrezorError( + identifier: string, + context?: string, + cause?: Error, +): HardwareWalletError { + const errorMapping = getTrezorErrorMapping(identifier); + + if (errorMapping) { + const normalizedContext = context?.trim().toLowerCase(); + const normalizedMessage = errorMapping.message.toLowerCase(); + const message = + normalizedContext && normalizedContext !== normalizedMessage + ? `${errorMapping.message} (${context})` + : errorMapping.message; + return new HardwareWalletError(message, { + code: errorMapping.code, + severity: errorMapping.severity, + category: errorMapping.category, + userMessage: errorMapping.userMessage ?? message, + cause, + }); + } + + const fallbackMessage = context + ? `Unknown Trezor error: ${identifier} (${context})` + : `Unknown Trezor error: ${identifier}`; + return new HardwareWalletError(fallbackMessage, { + code: ErrorCode.Unknown, + severity: Severity.Err, + category: Category.Unknown, + userMessage: fallbackMessage, + cause, + }); +} diff --git a/packages/keyring-eth-trezor/src/trezor-keyring.test.ts b/packages/keyring-eth-trezor/src/trezor-keyring.test.ts index 2747b5079..85b5730b3 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 () { diff --git a/packages/keyring-eth-trezor/src/trezor-keyring.ts b/packages/keyring-eth-trezor/src/trezor-keyring.ts index 5706dc3c5..2d31a773b 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,11 @@ 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) { + return handleTrezorTransportError( + error, + 'Failed to sign transaction with Trezor device', + ); } } @@ -415,48 +419,32 @@ 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) { + return handleTrezorTransportError( + error, + 'Failed to sign personal message with Trezor device', + ); + } } // EIP-712 Sign Typed Data @@ -469,52 +457,59 @@ 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) { + 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"