From b31971a349ad587fa9a64829d6b1c8ff4004d699 Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Thu, 21 Sep 2023 10:13:38 +0100 Subject: [PATCH 1/2] Add validation for EIP-1559 compatibility --- .../src/TransactionController.test.ts | 2 ++ .../src/TransactionController.ts | 28 ++++++++++++++++++- .../transaction-controller/src/utils.test.ts | 18 ++++++++++++ packages/transaction-controller/src/utils.ts | 17 +++++++++-- 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 60d1e15ae6..6203f8b020 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -466,6 +466,8 @@ describe('TransactionController', () => { { blockTracker: finalNetwork.blockTracker, getNetworkState: () => finalNetwork.state, + getCurrentAccountEIP1559Compatibility: () => true, + getCurrentNetworkEIP1559Compatibility: () => true, messenger, onNetworkStateChange: finalNetwork.subscribe, provider: finalNetwork.provider, diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 061877914b..02c2e63dec 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -184,6 +184,10 @@ export class TransactionController extends BaseController< private readonly getNetworkState: () => NetworkState; + private readonly getCurrentAccountEIP1559Compatibility: () => Promise; + + private readonly getCurrentNetworkEIP1559Compatibility: () => Promise; + private readonly messagingSystem: TransactionControllerMessenger; private readonly incomingTransactionHelper: IncomingTransactionHelper; @@ -232,6 +236,8 @@ export class TransactionController extends BaseController< * @param options.blockTracker - The block tracker used to poll for new blocks data. * @param options.disableHistory - Whether to disable storing history in transaction metadata. * @param options.disableSendFlowHistory - Explicitly disable transaction metadata history. + * @param options.getCurrentAccountEIP1559Compatibility - Whether or not the account supports EIP-1559. + * @param options.getCurrentNetworkEIP1559Compatibility - Whether or not the network supports EIP-1559. * @param options.getNetworkState - Gets the state of the network controller. * @param options.getSelectedAddress - Gets the address of the currently selected account. * @param options.incomingTransactions - Configuration options for incoming transaction support. @@ -250,6 +256,8 @@ export class TransactionController extends BaseController< blockTracker, disableHistory, disableSendFlowHistory, + getCurrentAccountEIP1559Compatibility, + getCurrentNetworkEIP1559Compatibility, getNetworkState, getSelectedAddress, incomingTransactions = {}, @@ -260,6 +268,8 @@ export class TransactionController extends BaseController< blockTracker: BlockTracker; disableHistory: boolean; disableSendFlowHistory: boolean; + getCurrentAccountEIP1559Compatibility: () => Promise; + getCurrentNetworkEIP1559Compatibility: () => Promise; getNetworkState: () => NetworkState; getSelectedAddress: () => string; incomingTransactions: { @@ -297,6 +307,10 @@ export class TransactionController extends BaseController< this.isSendFlowHistoryDisabled = disableSendFlowHistory ?? false; this.isHistoryDisabled = disableHistory ?? false; this.registry = new MethodRegistry({ provider }); + this.getCurrentAccountEIP1559Compatibility = + getCurrentAccountEIP1559Compatibility; + this.getCurrentNetworkEIP1559Compatibility = + getCurrentNetworkEIP1559Compatibility; this.nonceTracker = new NonceTracker({ provider, @@ -423,7 +437,8 @@ export class TransactionController extends BaseController< const { chainId, networkId } = this.getChainAndNetworkId(); const { transactions } = this.state; txParams = normalizeTxParams(txParams); - validateTxParams(txParams); + const isEIP1559Compatible = await this.getEIP1559Compatibility(); + validateTxParams(txParams, isEIP1559Compatible); const dappSuggestedGasFees = this.generateDappSuggestedGasFees( txParams, @@ -1811,6 +1826,17 @@ export class TransactionController extends BaseController< transactionMeta.v = addHexPrefix(signedTx.v.toString(16)); } } + + private async getEIP1559Compatibility() { + const currentNetworkIsEIP1559Compatible = + await this.getCurrentNetworkEIP1559Compatibility(); + const currentAccountIsEIP1559Compatible = + this.getCurrentAccountEIP1559Compatibility?.() ?? true; + + return ( + currentNetworkIsEIP1559Compatible && currentAccountIsEIP1559Compatible + ); + } } export default TransactionController; diff --git a/packages/transaction-controller/src/utils.test.ts b/packages/transaction-controller/src/utils.test.ts index c326af2532..42b49d4f97 100644 --- a/packages/transaction-controller/src/utils.test.ts +++ b/packages/transaction-controller/src/utils.test.ts @@ -1,3 +1,4 @@ +import { ethErrors } from 'eth-rpc-errors'; import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker'; import type { @@ -142,6 +143,23 @@ describe('utils', () => { } 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( + ethErrors.rpc.invalidParams( + 'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559', + ), + ); + }); }); describe('isEIP1559Transaction', () => { diff --git a/packages/transaction-controller/src/utils.ts b/packages/transaction-controller/src/utils.ts index e3856f454c..154e05b8b8 100644 --- a/packages/transaction-controller/src/utils.ts +++ b/packages/transaction-controller/src/utils.ts @@ -3,6 +3,7 @@ import { isValidHexAddress, } from '@metamask/controller-utils'; import type { Hex } from '@metamask/utils'; +import { ethErrors } from 'eth-rpc-errors'; import { addHexPrefix, isHexString } from 'ethereumjs-util'; import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker'; @@ -53,8 +54,12 @@ export function normalizeTxParams(txParams: TransactionParams) { * 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) { +export function validateTxParams( + txParams: TransactionParams, + isEIP1559Compatible = true, +) { if ( !txParams.from || typeof txParams.from !== 'string' || @@ -65,6 +70,12 @@ export function validateTxParams(txParams: TransactionParams) { ); } + if (isEIP1559Transaction(txParams) && !isEIP1559Compatible) { + throw ethErrors.rpc.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; @@ -111,14 +122,14 @@ export function validateTxParams(txParams: TransactionParams) { * @param txParams - Transaction params object to add. * @returns Boolean that is true if the transaction is EIP-1559 (has maxFeePerGas and maxPriorityFeePerGas), otherwise returns false. */ -export const isEIP1559Transaction = (txParams: TransactionParams): boolean => { +export function isEIP1559Transaction(txParams: TransactionParams): boolean { const hasOwnProp = (obj: TransactionParams, key: string) => Object.prototype.hasOwnProperty.call(obj, key); return ( hasOwnProp(txParams, 'maxFeePerGas') && hasOwnProp(txParams, 'maxPriorityFeePerGas') ); -}; +} export const validateGasValues = ( gasValues: GasPriceValue | FeeMarketEIP1559Values, From 666702a68f58913d9e03ce2c60fc58f3b2959ae8 Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Wed, 27 Sep 2023 08:53:42 +0100 Subject: [PATCH 2/2] use rpc-errors --- packages/transaction-controller/src/utils.test.ts | 2 +- packages/transaction-controller/src/utils.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/src/utils.test.ts b/packages/transaction-controller/src/utils.test.ts index 29709d89bf..bc3f08a553 100644 --- a/packages/transaction-controller/src/utils.test.ts +++ b/packages/transaction-controller/src/utils.test.ts @@ -212,7 +212,7 @@ describe('utils', () => { false, ), ).toThrow( - ethErrors.rpc.invalidParams( + rpcErrors.invalidParams( 'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559', ), ); diff --git a/packages/transaction-controller/src/utils.ts b/packages/transaction-controller/src/utils.ts index d44ce5947d..086ae9ea4d 100644 --- a/packages/transaction-controller/src/utils.ts +++ b/packages/transaction-controller/src/utils.ts @@ -71,7 +71,7 @@ export function validateTxParams( } if (isEIP1559Transaction(txParams) && !isEIP1559Compatible) { - throw ethErrors.rpc.invalidParams( + throw rpcErrors.invalidParams( 'Invalid transaction params: params specify an EIP-1559 transaction but the current network does not support EIP-1559', ); }