diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index e36b5a5e1d..7fe23531ae 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -64,9 +64,9 @@ import { validateGasValues, validateIfTransactionUnapproved, validateMinimumIncrease, - validateTxParams, ESTIMATE_GAS_ERROR, } from './utils'; +import { validateTxParams } from './validation'; export const HARDFORK = Hardfork.London; diff --git a/packages/transaction-controller/src/utils.test.ts b/packages/transaction-controller/src/utils.test.ts index bc3f08a553..93f9c7befa 100644 --- a/packages/transaction-controller/src/utils.test.ts +++ b/packages/transaction-controller/src/utils.test.ts @@ -1,4 +1,3 @@ -import { rpcErrors } from '@metamask/rpc-errors'; import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker'; import type { @@ -72,153 +71,6 @@ describe('utils', () => { }); }); - describe('validateTxParams', () => { - it('should throw if no from address', () => { - expect(() => util.validateTxParams({} as any)).toThrow( - rpcErrors.invalidParams( - 'Invalid "from" address: undefined must be a valid string.', - ), - ); - }); - - it('should throw if non-string from address', () => { - expect(() => util.validateTxParams({ from: 1337 } as any)).toThrow( - rpcErrors.invalidParams( - 'Invalid "from" address: 1337 must be a valid string.', - ), - ); - }); - - it('should throw if invalid from address', () => { - expect(() => util.validateTxParams({ from: '1337' } as any)).toThrow( - rpcErrors.invalidParams( - 'Invalid "from" address: 1337 must be a valid string.', - ), - ); - }); - - it('should throw if no data', () => { - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '0x', - } as any), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid "to" address: 0x must be a valid string.', - ), - ); - - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - } as any), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid "to" address: undefined must be a valid string.', - ), - ); - }); - - it('should delete data', () => { - const transaction = { - data: 'foo', - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '0x', - }; - util.validateTxParams(transaction); - expect(transaction.to).toBeUndefined(); - }); - - it('should throw if invalid to address', () => { - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '1337', - } as any), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid "to" address: 1337 must be a valid string.', - ), - ); - }); - - it('should throw if value is invalid', () => { - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '0x3244e191f1b4903970224322180f1fbbc415696b', - value: '133-7', - } as any), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid "value": 133-7 is not a positive number.', - ), - ); - - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '0x3244e191f1b4903970224322180f1fbbc415696b', - value: '133.7', - } as any), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid "value": 133.7 number must be denominated in wei.', - ), - ); - - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '0x3244e191f1b4903970224322180f1fbbc415696b', - value: 'hello', - } as any), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid "value": hello number must be a valid number.', - ), - ); - - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '0x3244e191f1b4903970224322180f1fbbc415696b', - value: 'one million dollar$', - } as any), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid "value": one million dollar$ number must be a valid number.', - ), - ); - - expect(() => - util.validateTxParams({ - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - to: '0x3244e191f1b4903970224322180f1fbbc415696b', - value: '1', - } as any), - ).not.toThrow(); - }); - - it('throws if params specifies an EIP-1559 transaction but the current network does not support EIP-1559', () => { - expect(() => - util.validateTxParams( - { - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - maxFeePerGas: '2', - maxPriorityFeePerGas: '3', - } as any, - false, - ), - ).toThrow( - rpcErrors.invalidParams( - 'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559', - ), - ); - }); - }); - describe('isEIP1559Transaction', () => { it('should detect EIP1559 transaction', () => { const tx: TransactionParams = { from: '' }; diff --git a/packages/transaction-controller/src/utils.ts b/packages/transaction-controller/src/utils.ts index 086ae9ea4d..bff356e479 100644 --- a/packages/transaction-controller/src/utils.ts +++ b/packages/transaction-controller/src/utils.ts @@ -1,8 +1,4 @@ -import { - convertHexToDecimal, - isValidHexAddress, -} from '@metamask/controller-utils'; -import { rpcErrors } from '@metamask/rpc-errors'; +import { convertHexToDecimal } from '@metamask/controller-utils'; import { addHexPrefix, isHexString } from 'ethereumjs-util'; import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker'; @@ -49,74 +45,6 @@ export function normalizeTxParams(txParams: TransactionParams) { return normalizedTxParams; } -/** - * Validates the transaction params for required properties and throws in - * the event of any validation error. - * - * @param txParams - Transaction params object to validate. - * @param isEIP1559Compatible - whether or not the current network supports EIP-1559 transactions. - */ -export function validateTxParams( - txParams: TransactionParams, - isEIP1559Compatible = true, -) { - if ( - !txParams.from || - typeof txParams.from !== 'string' || - !isValidHexAddress(txParams.from) - ) { - throw rpcErrors.invalidParams( - `Invalid "from" address: ${txParams.from} must be a valid string.`, - ); - } - - if (isEIP1559Transaction(txParams) && !isEIP1559Compatible) { - throw rpcErrors.invalidParams( - 'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559', - ); - } - - if (txParams.to === '0x' || txParams.to === undefined) { - if (txParams.data) { - delete txParams.to; - } else { - throw rpcErrors.invalidParams( - `Invalid "to" address: ${txParams.to} must be a valid string.`, - ); - } - } else if (txParams.to !== undefined && !isValidHexAddress(txParams.to)) { - throw rpcErrors.invalidParams( - `Invalid "to" address: ${txParams.to} must be a valid string.`, - ); - } - - if (txParams.value !== undefined) { - const value = txParams.value.toString(); - if (value.includes('-')) { - throw rpcErrors.invalidParams( - `Invalid "value": ${value} is not a positive number.`, - ); - } - - if (value.includes('.')) { - throw rpcErrors.invalidParams( - `Invalid "value": ${value} number must be denominated in wei.`, - ); - } - const intValue = parseInt(txParams.value, 10); - const isValid = - Number.isFinite(intValue) && - !Number.isNaN(intValue) && - !isNaN(Number(value)) && - Number.isSafeInteger(intValue); - if (!isValid) { - throw rpcErrors.invalidParams( - `Invalid "value": ${value} number must be a valid number.`, - ); - } - } -} - /** * Checks if a transaction is EIP-1559 by checking for the existence of * maxFeePerGas and maxPriorityFeePerGas within its parameters. diff --git a/packages/transaction-controller/src/validation.test.ts b/packages/transaction-controller/src/validation.test.ts new file mode 100644 index 0000000000..1e7597fbda --- /dev/null +++ b/packages/transaction-controller/src/validation.test.ts @@ -0,0 +1,178 @@ +import { rpcErrors } from '@metamask/rpc-errors'; + +import { validateTxParams } from './validation'; + +describe('validation', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('validateTxParams', () => { + it('should throw if no from address', () => { + expect(() => validateTxParams({} as any)).toThrow( + rpcErrors.invalidParams( + 'Invalid "from" address: undefined must be a valid string.', + ), + ); + }); + + it('should throw if non-string from address', () => { + expect(() => validateTxParams({ from: 1337 } as any)).toThrow( + rpcErrors.invalidParams( + 'Invalid "from" address: 1337 must be a valid string.', + ), + ); + }); + + it('should throw if invalid from address', () => { + expect(() => validateTxParams({ from: '1337' } as any)).toThrow( + rpcErrors.invalidParams( + 'Invalid "from" address: 1337 must be a valid string.', + ), + ); + }); + + it('should throw if no data', () => { + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '0x', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid "to" address: 0x must be a valid string.', + ), + ); + + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid "to" address: undefined must be a valid string.', + ), + ); + }); + + it('should delete data', () => { + const transaction = { + data: 'foo', + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '0x', + }; + validateTxParams(transaction); + expect(transaction.to).toBeUndefined(); + }); + + it('should throw if invalid to address', () => { + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '1337', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid "to" address: 1337 must be a valid string.', + ), + ); + }); + + it('should throw if value is invalid', () => { + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '0x3244e191f1b4903970224322180f1fbbc415696b', + value: '133-7', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid "value": 133-7 is not a positive number.', + ), + ); + + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '0x3244e191f1b4903970224322180f1fbbc415696b', + value: '133.7', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid "value": 133.7 number must be denominated in wei.', + ), + ); + + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '0x3244e191f1b4903970224322180f1fbbc415696b', + value: 'hello', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid "value": hello number must be a valid number.', + ), + ); + + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '0x3244e191f1b4903970224322180f1fbbc415696b', + value: 'one million dollar$', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid "value": one million dollar$ number must be a valid number.', + ), + ); + + expect(() => + validateTxParams({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + to: '0x3244e191f1b4903970224322180f1fbbc415696b', + value: '1', + } as any), + ).not.toThrow(); + }); + + it('throws if params specifies an EIP-1559 transaction but the current network does not support EIP-1559', () => { + expect(() => + validateTxParams( + { + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + maxFeePerGas: '2', + maxPriorityFeePerGas: '3', + } as any, + false, + ), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559', + ), + ); + }); + + it('throws if data is invalid', () => { + expect(() => + validateTxParams({ + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + to: '0xfbb5595c18ca76bab52d66188e4ca50c7d95f77a', + data: '0xa9059cbb00000000000000000000000011b6A5fE2906F3354145613DB0d99CEB51f604C90000000000000000000000000000000000000000000000004563918244F400', + } as any), + ).toThrow( + rpcErrors.invalidParams( + 'Invalid transaction params: data out-of-bounds, BUFFER_OVERRUN.', + ), + ); + + expect(() => + validateTxParams({ + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + value: '0x01', + data: 'INVALID_ARGUMENT', + } as any), + ).not.toThrow(); + }); + }); +}); diff --git a/packages/transaction-controller/src/validation.ts b/packages/transaction-controller/src/validation.ts new file mode 100644 index 0000000000..4a79ed55a1 --- /dev/null +++ b/packages/transaction-controller/src/validation.ts @@ -0,0 +1,142 @@ +import { Interface } from '@ethersproject/abi'; +import { isValidHexAddress } from '@metamask/controller-utils'; +import { abiERC20 } from '@metamask/metamask-eth-abis'; +import { rpcErrors } from '@metamask/rpc-errors'; + +import type { TransactionParams } from './types'; +import { isEIP1559Transaction } from './utils'; + +/** + * Validates the transaction params for required properties and throws in + * the event of any validation error. + * + * @param txParams - Transaction params object to validate. + * @param isEIP1559Compatible - whether or not the current network supports EIP-1559 transactions. + */ +export function validateTxParams( + txParams: TransactionParams, + isEIP1559Compatible = true, +) { + validateEIP1559Compatibility(txParams, isEIP1559Compatible); + validateParamFrom(txParams.from); + validateParamRecipient(txParams); + validateParamValue(txParams.value); + validateParamData(txParams.data); +} + +/** + * Validates EIP-1559 compatibility for transaction creation. + * + * @param txParams - The transaction parameters to validate. + * @param isEIP1559Compatible - Indicates if the current network supports EIP-1559. + * @throws Throws invalid params if the transaction specifies EIP-1559 but the network does not support it. + */ +function validateEIP1559Compatibility( + txParams: TransactionParams, + isEIP1559Compatible: boolean, +) { + if (isEIP1559Transaction(txParams) && !isEIP1559Compatible) { + throw rpcErrors.invalidParams( + 'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559', + ); + } +} + +/** + * Validates value property, ensuring it is a valid positive integer number + * denominated in wei. + * + * @param value - The value to validate, expressed as a string. + * @throws Throws an error if the value is not a valid positive integer + * number denominated in wei. + * - If the value contains a hyphen (-), it is considered invalid. + * - If the value contains a decimal point (.), it is considered invalid. + * - If the value is not a finite number, is NaN, or is not a safe integer, it is considered invalid. + */ +function validateParamValue(value?: string) { + if (value !== undefined) { + if (value.includes('-')) { + throw rpcErrors.invalidParams( + `Invalid "value": ${value} is not a positive number.`, + ); + } + + if (value.includes('.')) { + throw rpcErrors.invalidParams( + `Invalid "value": ${value} number must be denominated in wei.`, + ); + } + const intValue = parseInt(value, 10); + const isValid = + Number.isFinite(intValue) && + !Number.isNaN(intValue) && + !isNaN(Number(value)) && + Number.isSafeInteger(intValue); + if (!isValid) { + throw rpcErrors.invalidParams( + `Invalid "value": ${value} number must be a valid number.`, + ); + } + } +} + +/** + * Validates the recipient address in a transaction's parameters. + * + * @param txParams - The transaction parameters object to validate. + * @throws Throws an error if the recipient address is invalid: + * - If the recipient address is an empty string ('0x') or undefined and the transaction contains data, + * the "to" field is removed from the transaction parameters. + * - If the recipient address is not a valid hexadecimal Ethereum address, an error is thrown. + */ +function validateParamRecipient(txParams: TransactionParams) { + if (txParams.to === '0x' || txParams.to === undefined) { + if (txParams.data) { + delete txParams.to; + } else { + throw new Error( + `Invalid "to" address: ${txParams.to} must be a valid string.`, + ); + } + } else if (txParams.to !== undefined && !isValidHexAddress(txParams.to)) { + throw new Error( + `Invalid "to" address: ${txParams.to} must be a valid string.`, + ); + } +} + +/** + * Validates the recipient address in a transaction's parameters. + * + * @param from - The from property to validate. + * @throws Throws an error if the recipient address is invalid: + * - If the recipient address is an empty string ('0x') or undefined and the transaction contains data, + * the "to" field is removed from the transaction parameters. + * - If the recipient address is not a valid hexadecimal Ethereum address, an error is thrown. + */ +function validateParamFrom(from: string) { + if (!from || typeof from !== 'string' || !isValidHexAddress(from)) { + throw new Error(`Invalid "from" address: ${from} must be a valid string.`); + } +} + +/** + * Validates input data for transactions. + * + * @param value - The input data to validate. + * @throws Throws invalid params if the input data is invalid. + */ +function validateParamData(value?: string) { + if (value) { + const ERC20Interface = new Interface(abiERC20); + try { + ERC20Interface.parseTransaction({ data: value }); + } catch (error: any) { + if (error.message.match(/BUFFER_OVERRUN/u)) { + throw rpcErrors.invalidParams( + 'Invalid transaction params: data out-of-bounds, BUFFER_OVERRUN.', + ); + } + } + } +}