From 724b846c635ead5479bdf06c080f6c1bde7ff288 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 10:29:00 +0100 Subject: [PATCH 1/8] Remove deprecated TransactionController constructor options - Remove disableHistory and disableSendFlowHistory options - Replace getCurrentAccountEIP1559Compatibility with constant true - Replace getCurrentNetworkEIP1559Compatibility with NetworkController:getEIP1559Compatibility messenger call - Remove getExternalPendingTransactions (STX now routes through TransactionController) - Replace getGasFeeEstimates with GasFeeController:fetchGasFeeEstimates messenger call - Remove getNetworkClientRegistry option; inline messenger call in MultichainTrackingHelper - Replace getNetworkState with NetworkController:getState messenger call - Replace sign option with KeyringController:signTransaction messenger call; add txDataToTransaction helper - Remove transactionHistoryLimit option (now a feature flag) - Remove securityProviderRequest option and SecurityProviderRequest type export - Remove afterSimulate hook option and AfterSimulateHook type (unused in both clients) - Remove isResubmitEnabled option and all resubmit logic from PendingTransactionTracker --- .../src/TransactionController.ts | 276 ++++-------------- .../src/helpers/PendingTransactionTracker.ts | 124 -------- packages/transaction-controller/src/index.ts | 2 - packages/transaction-controller/src/types.ts | 23 -- .../src/utils/prepare.ts | 10 + 5 files changed, 64 insertions(+), 371 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index d75a550ae7..5c2563ab94 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1,4 +1,4 @@ -import type { TypedTransaction } from '@ethereumjs/tx'; +import type { TypedTransaction, TypedTxData } from '@ethereumjs/tx'; import type { AccountsController, AccountsControllerGetSelectedAccountAction, @@ -27,13 +27,11 @@ import type { AccountActivityServiceTransactionUpdatedEvent, BackendWebSocketServiceConnectionStateChangedEvent, } from '@metamask/core-backend'; -import type { - FetchGasFeeEstimateOptions, - GasFeeState, -} from '@metamask/gas-fee-controller'; +import type { GasFeeControllerFetchGasFeeEstimatesAction } from '@metamask/gas-fee-controller'; import type { KeyringControllerGetStateAction, KeyringControllerSignEip7702AuthorizationAction, + KeyringControllerSignTransactionAction, } from '@metamask/keyring-controller'; import type { Messenger } from '@metamask/messenger'; import type { @@ -41,10 +39,12 @@ import type { NetworkClientId, NetworkController, NetworkControllerStateChangeEvent, - NetworkState, Provider, NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetNetworkClientByIdAction, + NetworkControllerGetNetworkClientRegistryAction, + NetworkControllerGetStateAction, + NetworkControllerGetEIP1559CompatibilityAction, } from '@metamask/network-controller'; import { NetworkClientType } from '@metamask/network-controller'; import type { @@ -99,7 +99,6 @@ import type { Layer1GasFeeFlow, Revert, SavedGasFees, - SecurityProviderRequest, SendFlowHistoryEntry, TransactionParams, TransactionMeta, @@ -124,7 +123,6 @@ import type { AfterAddHook, GasFeeEstimateLevel as GasFeeEstimateLevelType, TransactionBatchMeta, - AfterSimulateHook, BeforeSignHook, GetSimulationConfig, AddTransactionOptions, @@ -174,7 +172,11 @@ import { getAndFormatTransactionsForNonceTracker, getNextNonce, } from './utils/nonce'; -import { prepareTransaction, serializeTransaction } from './utils/prepare'; +import { + prepareTransaction, + serializeTransaction, + txDataToTransaction, +} from './utils/prepare'; import { getChainId, getNetworkClientId, rpcRequest } from './utils/provider'; import { getTransactionParamsWithIncreasedGasFee } from './utils/retry'; import { @@ -320,48 +322,11 @@ export type TransactionControllerActions = | TransactionControllerGetStateAction | TransactionControllerMethodActions; -/** - * Configuration options for the PendingTransactionTracker - */ -export type PendingTransactionOptions = { - /** Whether transaction publishing is automatically retried. */ - isResubmitEnabled?: () => boolean; -}; - /** TransactionController constructor options. */ export type TransactionControllerOptions = { - /** @deprecated No longer used — kept only for backward compatibility. */ - disableHistory: boolean; - - /** @deprecated No longer used — kept only for backward compatibility. */ - disableSendFlowHistory: boolean; - /** Whether to disable additional processing on swaps transactions. */ disableSwaps: boolean; - /** Whether or not the account supports EIP-1559. */ - getCurrentAccountEIP1559Compatibility?: () => Promise; - - /** Whether or not the network supports EIP-1559. */ - getCurrentNetworkEIP1559Compatibility: () => Promise; - - /** Callback to retrieve pending transactions from external sources. */ - getExternalPendingTransactions?: ( - address: string, - chainId?: string, - ) => NonceTrackerTransaction[]; - - /** Callback to retrieve gas fee estimates. */ - getGasFeeEstimates?: ( - options: FetchGasFeeEstimateOptions, - ) => Promise; - - /** Gets the network client registry. */ - getNetworkClientRegistry: NetworkController['getNetworkClientRegistry']; - - /** Gets the state of the network controller. */ - getNetworkState: () => NetworkState; - /** Get accounts that a given origin has permissions for. */ getPermittedAccounts?: (origin?: string) => Promise; @@ -401,37 +366,15 @@ export type TransactionControllerOptions = { /** The controller messenger. */ messenger: TransactionControllerMessenger; - /** Configuration options for pending transaction support. */ - pendingTransactions?: PendingTransactionOptions; - /** Public key used to validate EIP-7702 contract signatures in feature flags. */ publicKeyEIP7702?: Hex; - /** A function for verifying a transaction, whether it is malicious or not. */ - securityProviderRequest?: SecurityProviderRequest; - - /** Function used to sign transactions. */ - sign?: ( - transaction: TypedTransaction, - from: string, - transactionMeta?: TransactionMeta, - ) => Promise; - /** Initial state to set on this controller. */ state?: Partial; testGasFeeFlows?: boolean; trace?: TraceCallback; - /** - * Transaction history limit. - * - * @deprecated Use the `transactionHistoryLimit` feature flag in - * `RemoteFeatureFlagController` instead. This option will be removed - * in a future version. - */ - transactionHistoryLimit?: number; - /** The controller hooks. */ hooks: { /** Additional logic to execute after adding a transaction. */ @@ -443,9 +386,6 @@ export type TransactionControllerOptions = { signedTx: TypedTransaction, ) => boolean; - /** Additional logic to execute after simulating a transaction. */ - afterSimulate?: AfterSimulateHook; - /** * Additional logic to execute before checking pending transactions. * Return false to prevent the broadcast of the transaction. @@ -496,10 +436,15 @@ export type AllowedActions = | AccountsControllerGetSelectedAccountAction | AccountsControllerGetStateAction | ApprovalControllerAddRequestAction + | GasFeeControllerFetchGasFeeEstimatesAction | KeyringControllerGetStateAction | KeyringControllerSignEip7702AuthorizationAction + | KeyringControllerSignTransactionAction | NetworkControllerFindNetworkClientIdByChainIdAction + | NetworkControllerGetEIP1559CompatibilityAction | NetworkControllerGetNetworkClientByIdAction + | NetworkControllerGetNetworkClientRegistryAction + | NetworkControllerGetStateAction | RemoteFeatureFlagControllerGetStateAction; /** @@ -787,8 +732,6 @@ export class TransactionController extends BaseController< signedTx: TypedTransaction, ) => boolean; - readonly #afterSimulate: AfterSimulateHook; - readonly #approvingTransactionIds: Set = new Set(); readonly #beforeCheckPendingTransaction: ( @@ -807,23 +750,6 @@ export class TransactionController extends BaseController< transactionMeta: TransactionMeta, ) => (TransactionMeta | undefined)[]; - readonly #getCurrentAccountEIP1559Compatibility: () => Promise; - - readonly #getCurrentNetworkEIP1559Compatibility: ( - networkClientId?: NetworkClientId, - ) => Promise; - - readonly #getExternalPendingTransactions: ( - address: string, - chainId?: string, - ) => NonceTrackerTransaction[]; - - readonly #getGasFeeEstimates: ( - options: FetchGasFeeEstimateOptions, - ) => Promise; - - readonly #getNetworkState: () => NetworkState; - readonly #getPermittedAccounts?: (origin?: string) => Promise; readonly #getSavedGasFees: (chainId: Hex) => SavedGasFees | undefined; @@ -860,8 +786,6 @@ export class TransactionController extends BaseController< readonly #multichainTrackingHelper: MultichainTrackingHelper; - readonly #pendingTransactionOptions: PendingTransactionOptions; - readonly #publicKeyEIP7702?: Hex; readonly #publish: ( @@ -871,14 +795,6 @@ export class TransactionController extends BaseController< readonly #publishBatchHook?: PublishBatchHook; - readonly #securityProviderRequest?: SecurityProviderRequest; - - readonly #sign?: ( - transaction: TypedTransaction, - from: string, - transactionMeta?: TransactionMeta, - ) => Promise; - readonly #signAbortCallbacks: Map void> = new Map(); readonly #skipSimulationTransactionIds: Set = new Set(); @@ -895,12 +811,6 @@ export class TransactionController extends BaseController< constructor(options: TransactionControllerOptions) { const { disableSwaps, - getCurrentAccountEIP1559Compatibility, - getCurrentNetworkEIP1559Compatibility, - getExternalPendingTransactions, - getGasFeeEstimates, - getNetworkClientRegistry, - getNetworkState, getPermittedAccounts, getSavedGasFees, getSimulationConfig, @@ -911,10 +821,7 @@ export class TransactionController extends BaseController< isFirstTimeInteractionEnabled, isSimulationEnabled, messenger, - pendingTransactions = {}, publicKeyEIP7702, - securityProviderRequest, - sign, state, testGasFeeFlows, trace, @@ -935,9 +842,6 @@ export class TransactionController extends BaseController< this.#afterAdd = hooks?.afterAdd ?? ((): ReturnType => Promise.resolve({})); this.#afterSign = hooks?.afterSign ?? ((): boolean => true); - this.#afterSimulate = - hooks?.afterSimulate ?? - ((): ReturnType => Promise.resolve({})); this.#beforeCheckPendingTransaction = /* istanbul ignore next */ hooks?.beforeCheckPendingTransaction ?? @@ -950,17 +854,6 @@ export class TransactionController extends BaseController< this.#getAdditionalSignArguments = hooks?.getAdditionalSignArguments ?? ((): (TransactionMeta | undefined)[] => []); - this.#getCurrentAccountEIP1559Compatibility = - getCurrentAccountEIP1559Compatibility ?? - ((): Promise => Promise.resolve(true)); - this.#getCurrentNetworkEIP1559Compatibility = - getCurrentNetworkEIP1559Compatibility; - this.#getExternalPendingTransactions = - getExternalPendingTransactions ?? ((): NonceTrackerTransaction[] => []); - this.#getGasFeeEstimates = - getGasFeeEstimates ?? - ((): Promise => Promise.resolve({} as GasFeeState)); - this.#getNetworkState = getNetworkState; this.#getPermittedAccounts = getPermittedAccounts; this.#getSavedGasFees = getSavedGasFees ?? ((_chainId): SavedGasFees | undefined => undefined); @@ -979,15 +872,12 @@ export class TransactionController extends BaseController< this.#isSimulationEnabled = isSimulationEnabled ?? ((): boolean => true); this.#isSwapsDisabled = disableSwaps ?? false; this.#isTimeoutEnabled = hooks?.isTimeoutEnabled ?? ((): boolean => true); - this.#pendingTransactionOptions = pendingTransactions; this.#publicKeyEIP7702 = publicKeyEIP7702; this.#publish = hooks?.publish ?? ((): Promise<{ transactionHash?: string }> => Promise.resolve({ transactionHash: undefined })); this.#publishBatchHook = hooks?.publishBatch; - this.#securityProviderRequest = securityProviderRequest; - this.#sign = sign; this.#testGasFeeFlows = testGasFeeFlows === true; this.#trace = trace ?? (((_request, fn) => fn?.()) as TraceCallback); @@ -1006,7 +896,8 @@ export class TransactionController extends BaseController< networkClientId, ); }) as NetworkController['getNetworkClientById'], - getNetworkClientRegistry, + getNetworkClientRegistry: () => + this.messenger.call('NetworkController:getNetworkClientRegistry'), removePendingTransactionTrackerListeners: this.#removePendingTransactionTrackerListeners.bind(this), createNonceTracker: this.#createNonceTracker.bind(this), @@ -1023,7 +914,8 @@ export class TransactionController extends BaseController< const gasFeePoller = new GasFeePoller({ gasFeeFlows: this.#gasFeeFlows, - getGasFeeControllerEstimates: this.#getGasFeeEstimates, + getGasFeeControllerEstimates: (options) => + this.messenger.call('GasFeeController:fetchGasFeeEstimates', options), getTransactions: (): TransactionMeta[] => this.state.transactions, getTransactionBatches: (): TransactionBatchMeta[] => this.state.transactionBatches, @@ -1140,7 +1032,8 @@ export class TransactionController extends BaseController< return await addTransactionBatch({ addTransaction: this.addTransaction.bind(this), estimateGas: this.estimateGas.bind(this), - getGasFeeEstimates: this.#getGasFeeEstimates, + getGasFeeEstimates: (options) => + this.messenger.call('GasFeeController:fetchGasFeeEstimates', options), getInternalAccounts: this.#getInternalAccounts.bind(this), getSimulationConfig: this.#getSimulationConfig.bind(this), getPendingTransactionTracker: (networkClientId: NetworkClientId) => @@ -1382,16 +1275,6 @@ export class TransactionController extends BaseController< .catch(noop); } - // Set security provider response - if (method && this.#securityProviderRequest) { - const securityProviderResponse = await this.#securityProviderRequest( - addedTransactionMeta, - method, - ); - // eslint-disable-next-line require-atomic-updates - addedTransactionMeta.securityProviderResponse = securityProviderResponse; - } - addedTransactionMeta = updateSwapsTransaction( addedTransactionMeta, transactionType, @@ -1605,11 +1488,6 @@ export class TransactionController extends BaseController< return; } - /* istanbul ignore next */ - if (!this.#sign) { - throw new Error('No sign method defined.'); - } - const newTxParams: TransactionParams = getTransactionParamsWithIncreasedGasFee( transactionMeta.txParams, @@ -1624,10 +1502,12 @@ export class TransactionController extends BaseController< newTxParams, ); - const signedTx = await this.#sign( + const signedTxData = await this.messenger.call( + 'KeyringController:signTransaction', unsignedEthTx, transactionMeta.txParams.from, ); + const signedTx = txDataToTransaction(transactionMeta.chainId, signedTxData); const transactionMetaWithRsv = this.#updateTransactionMetaRSV( transactionMeta, @@ -2589,9 +2469,10 @@ export class TransactionController extends BaseController< this.messenger, ) as GasFeeFlow; - const gasFeeControllerData = await this.#getGasFeeEstimates({ - networkClientId, - }); + const gasFeeControllerData = await this.messenger.call( + 'GasFeeController:fetchGasFeeEstimates', + { networkClientId }, + ); return gasFeeFlow.getGasFees({ gasFeeControllerData, @@ -2639,10 +2520,6 @@ export class TransactionController extends BaseController< chainId: Hex, transactionParams: TransactionParams, ): Promise { - if (!this.#sign) { - throw new Error('No sign method defined.'); - } - const normalizedTransactionParams = normalizeTransactionParams(transactionParams); const type = isEIP1559Transaction(normalizedTransactionParams) @@ -2662,7 +2539,12 @@ export class TransactionController extends BaseController< updatedTransactionParams, ); - const signedTransaction = await this.#sign(unsignedTransaction, from); + const signedTxData = await this.messenger.call( + 'KeyringController:signTransaction', + unsignedTransaction, + from, + ); + const signedTransaction = txDataToTransaction(chainId, signedTxData); const rawTransaction = serializeTransaction(signedTransaction); return rawTransaction; @@ -2961,7 +2843,11 @@ export class TransactionController extends BaseController< await updateGasFees({ eip1559: isEIP1559Compatible, gasFeeFlows: this.#gasFeeFlows, - getGasFeeEstimates: this.#getGasFeeEstimates, + getGasFeeEstimates: (options) => + this.messenger.call( + 'GasFeeController:fetchGasFeeEstimates', + options, + ), getSavedGasFees: this.#getSavedGasFees.bind(this), messenger: this.messenger, txMeta: transactionMeta, @@ -3194,13 +3080,7 @@ export class TransactionController extends BaseController< log('Approving transaction', transactionMeta); try { - if (!this.#sign) { - this.#failTransaction( - transactionMeta, - new Error('No sign method defined.'), - ); - return ApprovalState.NotApproved; - } else if (!transactionMeta.chainId) { + if (!transactionMeta.chainId) { this.#failTransaction( transactionMeta, new Error('No chainId defined.'), @@ -3825,14 +3705,11 @@ export class TransactionController extends BaseController< async #getEIP1559Compatibility( networkClientId?: NetworkClientId, ): Promise { - const currentNetworkIsEIP1559Compatible = - await this.#getCurrentNetworkEIP1559Compatibility(networkClientId); - - const currentAccountIsEIP1559Compatible = - await this.#getCurrentAccountEIP1559Compatibility(); - return ( - currentNetworkIsEIP1559Compatible && currentAccountIsEIP1559Compatible + (await this.messenger.call( + 'NetworkController:getEIP1559Compatibility', + networkClientId, + )) ?? false ); } @@ -3902,12 +3779,10 @@ export class TransactionController extends BaseController< log('Signing transaction', finalTxParams); - const signedTx = await new Promise((resolve, reject) => { - this.#sign?.( - unsignedEthTx, - from, - ...this.#getAdditionalSignArguments(finalTransactionMeta), - ).then(resolve, reject); + const signedTxData = await new Promise((resolve, reject) => { + this.messenger + .call('KeyringController:signTransaction', unsignedEthTx, from) + .then(resolve, reject); this.#signAbortCallbacks.set(transactionId, () => reject(new Error('Signing aborted by user')), @@ -3916,10 +3791,7 @@ export class TransactionController extends BaseController< this.#signAbortCallbacks.delete(transactionId); - if (!signedTx) { - log('Skipping signed status as no signed transaction'); - return undefined; - } + const signedTx = txDataToTransaction(chainId, signedTxData); const transactionMetaFromHook = cloneDeep(finalTransactionMeta); @@ -4075,7 +3947,6 @@ export class TransactionController extends BaseController< beforeCheckPendingTransaction: this.#beforeCheckPendingTransaction.bind(this), }, - isResubmitEnabled: this.#pendingTransactionOptions.isResubmitEnabled, isTimeoutEnabled: this.#isTimeoutEnabled, messenger: this.messenger, networkClientId, @@ -4154,11 +4025,7 @@ export class TransactionController extends BaseController< chainId, ); - const externalPendingTransactions = this.#getExternalPendingTransactions( - address, - chainId, - ); - return [...standardPendingTransactions, ...externalPendingTransactions]; + return standardPendingTransactions; } async #publishTransactionForRetry( @@ -4395,8 +4262,6 @@ export class TransactionController extends BaseController< ); log('Updated simulation data', transactionId, updatedTransactionMeta); - - await this.#runAfterSimulateHook(updatedTransactionMeta); } #onGasFeePollerTransactionUpdate({ @@ -4467,7 +4332,9 @@ export class TransactionController extends BaseController< const { chainId, networkClientId, origin, rawTx, txParams } = transactionMeta; - const { networkConfigurationsByChainId } = this.#getNetworkState(); + const { networkConfigurationsByChainId } = this.messenger.call( + 'NetworkController:getState', + ); const networkConfiguration = networkConfigurationsByChainId[chainId]; const endpoint = networkConfiguration?.rpcEndpoints.find( @@ -4630,41 +4497,6 @@ export class TransactionController extends BaseController< ); } - async #runAfterSimulateHook(transactionMeta: TransactionMeta): Promise { - log('Calling afterSimulate hook', transactionMeta); - - const { id: transactionId } = transactionMeta; - - const result = await this.#afterSimulate({ - transactionMeta, - }); - - const { skipSimulation, updateTransaction } = result ?? {}; - - if (skipSimulation) { - this.#skipSimulationTransactionIds.add(transactionId); - } else if (skipSimulation === false) { - this.#skipSimulationTransactionIds.delete(transactionId); - } - - if (!updateTransaction) { - return; - } - - const updatedTransactionMeta = this.#updateTransactionInternal( - { - transactionId, - skipResimulateCheck: true, - }, - (txMeta) => { - txMeta.txParamsOriginal = cloneDeep(txMeta.txParams); - updateTransaction(txMeta); - }, - ); - - log('Updated transaction with afterSimulate data', updatedTransactionMeta); - } - async #defaultPublishHook( { networkClientId, diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index 683ebe57a3..8b2288398d 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -28,16 +28,6 @@ const DROPPED_BLOCK_COUNT = 3; const RECEIPT_STATUS_SUCCESS = '0x1'; const RECEIPT_STATUS_FAILURE = '0x0'; -const MAX_RETRY_BLOCK_DISTANCE = 50; - -const KNOWN_TRANSACTION_ERRORS = [ - 'replacement transaction underpriced', - 'known transaction', - 'gas price too low to replace', - 'transaction with the same hash was already imported', - 'gateway timeout', - 'nonce too low', -]; const log = createModuleLogger(projectLogger, 'pending-transactions'); @@ -87,8 +77,6 @@ export class PendingTransactionTracker { readonly #getTransactions: () => TransactionMeta[]; - readonly #isResubmitEnabled: () => boolean; - readonly #lastSeenTimestampByHash: Map; // TODO: Replace `any` with type @@ -115,7 +103,6 @@ export class PendingTransactionTracker { getTransactions, isTimeoutEnabled, hooks, - isResubmitEnabled, messenger, networkClientId, publishTransaction, @@ -128,7 +115,6 @@ export class PendingTransactionTracker { transactionMeta: TransactionMeta, ) => Promise; }; - isResubmitEnabled?: () => boolean; isTimeoutEnabled: (transactionMeta: TransactionMeta) => boolean; messenger: TransactionControllerMessenger; networkClientId: NetworkClientId; @@ -143,7 +129,6 @@ export class PendingTransactionTracker { this.#getGlobalLock = getGlobalLock; this.#networkClientId = networkClientId; this.#getTransactions = getTransactions; - this.#isResubmitEnabled = isResubmitEnabled ?? ((): boolean => true); this.#lastSeenTimestampByHash = new Map(); this.#listener = this.#onLatestBlock.bind(this); this.#messenger = messenger; @@ -247,12 +232,6 @@ export class PendingTransactionTracker { releaseLock(); } - try { - await this.#resubmitTransactions(latestBlockNumber); - } catch (error) { - /* istanbul ignore next */ - this.#log('Failed to resubmit transactions', error); - } } async #checkTransactions(): Promise { @@ -278,109 +257,6 @@ export class PendingTransactionTracker { ); } - async #resubmitTransactions(latestBlockNumber: string): Promise { - if (!this.#isResubmitEnabled() || !this.#running) { - return; - } - - this.#log('Resubmitting transactions'); - - const pendingTransactions = this.#getPendingTransactions(); - - if (!pendingTransactions.length) { - this.#log('No pending transactions to resubmit'); - return; - } - - this.#log('Found pending transactions to resubmit', { - count: pendingTransactions.length, - ids: pendingTransactions.map((tx) => tx.id), - }); - - for (const txMeta of pendingTransactions) { - try { - await this.#resubmitTransaction(txMeta, latestBlockNumber); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { - /* istanbul ignore next */ - const errorMessage = - error.value?.message?.toLowerCase() ?? - error.message?.toLowerCase() ?? - String(error); - - if (this.#isKnownTransactionError(errorMessage)) { - this.#log('Ignoring known transaction error', errorMessage); - continue; - } - - this.#warnTransaction( - txMeta, - error.message, - 'There was an error when resubmitting this transaction.', - ); - } - } - } - - #isKnownTransactionError(errorMessage: string): boolean { - return KNOWN_TRANSACTION_ERRORS.some((knownError) => - errorMessage.includes(knownError), - ); - } - - async #resubmitTransaction( - txMeta: TransactionMeta, - latestBlockNumber: string, - ): Promise { - if (!this.#isResubmitDue(txMeta, latestBlockNumber)) { - return; - } - - if (!(await this.#beforeCheckPendingTransaction(txMeta))) { - return; - } - - await this.#publishTransaction(txMeta); - - const retryCount = (txMeta.retryCount ?? 0) + 1; - - this.#updateTransaction( - merge({}, txMeta, { retryCount }), - 'PendingTransactionTracker:transaction-retry - Retry count increased', - ); - } - - #isResubmitDue(txMeta: TransactionMeta, latestBlockNumber: string): boolean { - const txMetaWithFirstRetryBlockNumber = cloneDeep(txMeta); - - if (!txMetaWithFirstRetryBlockNumber.firstRetryBlockNumber) { - txMetaWithFirstRetryBlockNumber.firstRetryBlockNumber = latestBlockNumber; - - this.#updateTransaction( - txMetaWithFirstRetryBlockNumber, - 'PendingTransactionTracker:#isResubmitDue - First retry block number set', - ); - } - - const { firstRetryBlockNumber } = txMetaWithFirstRetryBlockNumber; - - const blocksSinceFirstRetry = - Number.parseInt(latestBlockNumber, 16) - - Number.parseInt(firstRetryBlockNumber, 16); - - const retryCount = txMeta.retryCount ?? 0; - - // Exponential backoff to limit retries at publishing - // Capped at ~15 minutes between retries - const requiredBlocksSinceFirstRetry = Math.min( - MAX_RETRY_BLOCK_DISTANCE, - Math.pow(2, retryCount), - ); - - return blocksSinceFirstRetry >= requiredBlocksSinceFirstRetry; - } - #cleanTransaction(txMeta: TransactionMeta): void { const { hash, id } = txMeta; diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index bccdc5b33a..3d78cc8a5c 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -68,7 +68,6 @@ export { export type { AddTransactionOptions, AfterAddHook, - AfterSimulateHook, Authorization, AuthorizationList, BatchTransaction, @@ -102,7 +101,6 @@ export type { RequiredAsset, SavedGasFees, SecurityAlertResponse, - SecurityProviderRequest, SendFlowHistoryEntry, SimulationBalanceChange, SimulationData, diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 6694b67dba..c9f8a8c9c8 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -1308,16 +1308,6 @@ export type InferTransactionTypeResult = { type: TransactionType; }; -/** - * A function for verifying a transaction, whether it is malicious or not. - */ -export type SecurityProviderRequest = ( - requestData: TransactionMeta, - messageType: string, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any -) => Promise; - /** * Specifies the shape of the base transaction parameters. * Added in EIP-2718. @@ -2129,19 +2119,6 @@ export type AfterAddHook = (request: { updateTransaction?: (transaction: TransactionMeta) => void; }>; -/** - * Custom logic to be executed after a transaction is simulated. - * Can optionally update the transaction by returning the `updateTransaction` callback. - */ -export type AfterSimulateHook = (request: { - transactionMeta: TransactionMeta; -}) => Promise< - | { - skipSimulation?: boolean; - updateTransaction?: (transaction: TransactionMeta) => void; - } - | undefined ->; /** * Custom logic to be executed before a transaction is signed. diff --git a/packages/transaction-controller/src/utils/prepare.ts b/packages/transaction-controller/src/utils/prepare.ts index 3c44534d4f..ed80f3bb7a 100644 --- a/packages/transaction-controller/src/utils/prepare.ts +++ b/packages/transaction-controller/src/utils/prepare.ts @@ -42,6 +42,16 @@ export function serializeTransaction(transaction: TypedTransaction): Hex { return bytesToHex(transaction.serialize()); } +export function txDataToTransaction( + chainId: Hex, + txData: TypedTxData, +): TypedTransaction { + return TransactionFactory.fromTxData(txData, { + freeze: false, + common: getCommonConfiguration(chainId), + }); +} + /** * Generates the configuration used to prepare transactions. * From 6ee1c74c84ad8a0677ccd1edb500acde4153f5ac Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 11:21:36 +0100 Subject: [PATCH 2/8] Remove unused hooks and simplify signing types - Remove afterSign hook (unused in both clients) - Remove getAdditionalSignArguments hook (unused in both clients, also never called internally) - Remove afterSimulate hook (unused in both clients) - Move isTimeoutEnabled from hooks to root constructor options - Change serializeTransaction to accept TypedTxData directly via TransactionFactory.fromTxData - Remove txDataToTransaction helper; #updateTransactionMetaRSV now accepts TypedTxData --- .../src/TransactionController.ts | 72 ++++--------------- .../src/utils/prepare.ts | 18 ++--- 2 files changed, 17 insertions(+), 73 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 5c2563ab94..7f976b31e7 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -172,11 +172,7 @@ import { getAndFormatTransactionsForNonceTracker, getNextNonce, } from './utils/nonce'; -import { - prepareTransaction, - serializeTransaction, - txDataToTransaction, -} from './utils/prepare'; +import { prepareTransaction, serializeTransaction } from './utils/prepare'; import { getChainId, getNetworkClientId, rpcRequest } from './utils/provider'; import { getTransactionParamsWithIncreasedGasFee } from './utils/retry'; import { @@ -363,6 +359,9 @@ export type TransactionControllerOptions = { /** Whether new transactions will be automatically simulated. */ isSimulationEnabled?: () => boolean; + /** Whether timeout checking is enabled for a transaction. */ + isTimeoutEnabled?: (transactionMeta: TransactionMeta) => boolean; + /** The controller messenger. */ messenger: TransactionControllerMessenger; @@ -380,12 +379,6 @@ export type TransactionControllerOptions = { /** Additional logic to execute after adding a transaction. */ afterAdd?: AfterAddHook; - /** Additional logic to execute after signing a transaction. Return false to not change the status to signed. */ - afterSign?: ( - transactionMeta: TransactionMeta, - signedTx: TypedTransaction, - ) => boolean; - /** * Additional logic to execute before checking pending transactions. * Return false to prevent the broadcast of the transaction. @@ -405,17 +398,6 @@ export type TransactionControllerOptions = { */ beforeSign?: BeforeSignHook; - /** Returns additional arguments required to sign a transaction. */ - getAdditionalSignArguments?: ( - transactionMeta: TransactionMeta, - ) => (TransactionMeta | undefined)[]; - - /** - * Callback to determine whether timeout checking should be enabled for a transaction. - * Return false to disable timeout for the transaction. - */ - isTimeoutEnabled?: (transactionMeta: TransactionMeta) => boolean; - /** Alternate logic to publish a transaction. */ publish?: ( transactionMeta: TransactionMeta, @@ -727,11 +709,6 @@ export class TransactionController extends BaseController< > { readonly #afterAdd: AfterAddHook; - readonly #afterSign: ( - transactionMeta: TransactionMeta, - signedTx: TypedTransaction, - ) => boolean; - readonly #approvingTransactionIds: Set = new Set(); readonly #beforeCheckPendingTransaction: ( @@ -746,10 +723,6 @@ export class TransactionController extends BaseController< readonly #gasFeeFlows: GasFeeFlow[]; - readonly #getAdditionalSignArguments: ( - transactionMeta: TransactionMeta, - ) => (TransactionMeta | undefined)[]; - readonly #getPermittedAccounts?: (origin?: string) => Promise; readonly #getSavedGasFees: (chainId: Hex) => SavedGasFees | undefined; @@ -820,6 +793,7 @@ export class TransactionController extends BaseController< isEIP7702GasFeeTokensEnabled, isFirstTimeInteractionEnabled, isSimulationEnabled, + isTimeoutEnabled, messenger, publicKeyEIP7702, state, @@ -841,7 +815,6 @@ export class TransactionController extends BaseController< this.#afterAdd = hooks?.afterAdd ?? ((): ReturnType => Promise.resolve({})); - this.#afterSign = hooks?.afterSign ?? ((): boolean => true); this.#beforeCheckPendingTransaction = /* istanbul ignore next */ hooks?.beforeCheckPendingTransaction ?? @@ -851,9 +824,6 @@ export class TransactionController extends BaseController< this.#beforeSign = hooks?.beforeSign ?? ((): ReturnType => Promise.resolve({})); - this.#getAdditionalSignArguments = - hooks?.getAdditionalSignArguments ?? - ((): (TransactionMeta | undefined)[] => []); this.#getPermittedAccounts = getPermittedAccounts; this.#getSavedGasFees = getSavedGasFees ?? ((_chainId): SavedGasFees | undefined => undefined); @@ -871,7 +841,7 @@ export class TransactionController extends BaseController< isFirstTimeInteractionEnabled ?? ((): boolean => true); this.#isSimulationEnabled = isSimulationEnabled ?? ((): boolean => true); this.#isSwapsDisabled = disableSwaps ?? false; - this.#isTimeoutEnabled = hooks?.isTimeoutEnabled ?? ((): boolean => true); + this.#isTimeoutEnabled = isTimeoutEnabled ?? ((): boolean => true); this.#publicKeyEIP7702 = publicKeyEIP7702; this.#publish = hooks?.publish ?? @@ -1507,14 +1477,12 @@ export class TransactionController extends BaseController< unsignedEthTx, transactionMeta.txParams.from, ); - const signedTx = txDataToTransaction(transactionMeta.chainId, signedTxData); - const transactionMetaWithRsv = this.#updateTransactionMetaRSV( transactionMeta, - signedTx, + signedTxData, ); - const rawTx = serializeTransaction(signedTx); + const rawTx = serializeTransaction(signedTxData); const newFee = newTxParams.maxFeePerGas ?? newTxParams.gasPrice; const oldFee = newTxParams.maxFeePerGas @@ -2544,8 +2512,7 @@ export class TransactionController extends BaseController< unsignedTransaction, from, ); - const signedTransaction = txDataToTransaction(chainId, signedTxData); - const rawTransaction = serializeTransaction(signedTransaction); + const rawTransaction = serializeTransaction(signedTxData); return rawTransaction; } @@ -3685,7 +3652,7 @@ export class TransactionController extends BaseController< */ #updateTransactionMetaRSV( transactionMeta: TransactionMeta, - signedTx: TypedTransaction, + signedTx: TypedTxData, ): TransactionMeta { const transactionMetaWithRsv = cloneDeep(transactionMeta); @@ -3696,7 +3663,7 @@ export class TransactionController extends BaseController< continue; } - transactionMetaWithRsv[key] = add0x(value.toString(16)); + transactionMetaWithRsv[key] = add0x(BigInt(value as bigint).toString(16)); } return transactionMetaWithRsv; @@ -3791,23 +3758,10 @@ export class TransactionController extends BaseController< this.#signAbortCallbacks.delete(transactionId); - const signedTx = txDataToTransaction(chainId, signedTxData); - const transactionMetaFromHook = cloneDeep(finalTransactionMeta); - if (!this.#afterSign(transactionMetaFromHook, signedTx)) { - this.updateTransaction( - transactionMetaFromHook, - 'TransactionController#signTransaction - Update after sign', - ); - - log('Skipping signed status based on hook'); - - return undefined; - } - const transactionMetaWithRsv = { - ...this.#updateTransactionMetaRSV(transactionMetaFromHook, signedTx), + ...this.#updateTransactionMetaRSV(transactionMetaFromHook, signedTxData), status: TransactionStatus.signed as const, txParams: finalTxParams, }; @@ -3819,7 +3773,7 @@ export class TransactionController extends BaseController< this.#onTransactionStatusChange(transactionMetaWithRsv); - const rawTx = serializeTransaction(signedTx); + const rawTx = serializeTransaction(signedTxData); const transactionMetaWithRawTx = merge({}, transactionMetaWithRsv, { rawTx, diff --git a/packages/transaction-controller/src/utils/prepare.ts b/packages/transaction-controller/src/utils/prepare.ts index ed80f3bb7a..cba926a97f 100644 --- a/packages/transaction-controller/src/utils/prepare.ts +++ b/packages/transaction-controller/src/utils/prepare.ts @@ -33,23 +33,13 @@ export function prepareTransaction( } /** - * Serializes a transaction object into a hex string. + * Serializes transaction data into a hex string. * - * @param transaction - The transaction object. + * @param txData - The signed transaction data. * @returns The prefixed hex string. */ -export function serializeTransaction(transaction: TypedTransaction): Hex { - return bytesToHex(transaction.serialize()); -} - -export function txDataToTransaction( - chainId: Hex, - txData: TypedTxData, -): TypedTransaction { - return TransactionFactory.fromTxData(txData, { - freeze: false, - common: getCommonConfiguration(chainId), - }); +export function serializeTransaction(txData: TypedTxData): Hex { + return bytesToHex(TransactionFactory.fromTxData(txData).serialize()); } /** From 05f2384e6fa2958f07020ae500092456f0dba914 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 11:22:30 +0100 Subject: [PATCH 3/8] Add changelog entry for deprecated option removals --- packages/transaction-controller/CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 979a313dff..7b89a33c9d 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Remove deprecated `TransactionController` constructor options and unused hooks, and replace them with direct messenger calls ([#8983](https://github.com/MetaMask/core/pull/8983)) + - Removed options: `disableHistory`, `disableSendFlowHistory`, `getCurrentAccountEIP1559Compatibility`, `getCurrentNetworkEIP1559Compatibility`, `getExternalPendingTransactions`, `getGasFeeEstimates`, `getNetworkClientRegistry`, `getNetworkState`, `pendingTransactions`, `securityProviderRequest`, `sign`, `transactionHistoryLimit` + - Removed hooks: `afterSign`, `afterSimulate`, `getAdditionalSignArguments` + - Moved hook: `isTimeoutEnabled` is now a root constructor option instead of a hook + - Removed types: `AfterSimulateHook`, `PendingTransactionOptions`, `SecurityProviderRequest`; `AfterAddHook` parameter `skipSimulation` removed + - `serializeTransaction` now accepts `TypedTxData` instead of `TypedTransaction` + - Added required `AllowedActions`: `GasFeeController:fetchGasFeeEstimates`, `KeyringController:signTransaction`, `NetworkController:getEIP1559Compatibility`, `NetworkController:getNetworkClientRegistry`, `NetworkController:getState` + - Removed resubmit logic from `PendingTransactionTracker` - Bump `@metamask/accounts-controller` from `^38.1.1` to `^38.1.2` ([#8912](https://github.com/MetaMask/core/pull/8912)) - Bump `@metamask/core-backend` from `^6.3.0` to `^6.3.1` ([#8912](https://github.com/MetaMask/core/pull/8912)) From 33f848efe16196374bb02735b39cb17ddf3033b1 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 11:26:01 +0100 Subject: [PATCH 4/8] Remove cast and changelog line for serializeTransaction --- packages/transaction-controller/CHANGELOG.md | 1 - packages/transaction-controller/src/TransactionController.ts | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 7b89a33c9d..3e4176c55c 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -14,7 +14,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed hooks: `afterSign`, `afterSimulate`, `getAdditionalSignArguments` - Moved hook: `isTimeoutEnabled` is now a root constructor option instead of a hook - Removed types: `AfterSimulateHook`, `PendingTransactionOptions`, `SecurityProviderRequest`; `AfterAddHook` parameter `skipSimulation` removed - - `serializeTransaction` now accepts `TypedTxData` instead of `TypedTransaction` - Added required `AllowedActions`: `GasFeeController:fetchGasFeeEstimates`, `KeyringController:signTransaction`, `NetworkController:getEIP1559Compatibility`, `NetworkController:getNetworkClientRegistry`, `NetworkController:getState` - Removed resubmit logic from `PendingTransactionTracker` - Bump `@metamask/accounts-controller` from `^38.1.1` to `^38.1.2` ([#8912](https://github.com/MetaMask/core/pull/8912)) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 7f976b31e7..e02e3f0774 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -3663,7 +3663,9 @@ export class TransactionController extends BaseController< continue; } - transactionMetaWithRsv[key] = add0x(BigInt(value as bigint).toString(16)); + transactionMetaWithRsv[key] = add0x( + BigInt(value as bigint | number | string).toString(16), + ); } return transactionMetaWithRsv; From add46ea7add70ffd147da13f261f59fa32e820b8 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 12:01:24 +0100 Subject: [PATCH 5/8] refactor: update tests and fix formatting for deprecated option removals --- .../src/TransactionController.test.ts | 628 ++++++------------ .../TransactionControllerIntegration.test.ts | 75 +-- .../helpers/PendingTransactionTracker.test.ts | 396 +---------- .../src/helpers/PendingTransactionTracker.ts | 1 - packages/transaction-controller/src/types.ts | 1 - 5 files changed, 219 insertions(+), 882 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 12d4f2b8a2..3a824c862e 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -607,17 +607,10 @@ describe('TransactionController', () => { ...otherOptions }: Partial = { disableSwaps: false, - getCurrentNetworkEIP1559Compatibility: async () => false, - getNetworkState: () => networkState, - getNetworkClientRegistry: () => - // eslint-disable-next-line @typescript-eslint/no-explicit-any - mockNetworkClientConfigurationsByNetworkClientId as any, getPermittedAccounts: async () => [ACCOUNT_MOCK], hooks: {}, isEIP7702GasFeeTokensEnabled: isEIP7702GasFeeTokensEnabledMock, publicKeyEIP7702: '0x1234', - sign: signMock, - transactionHistoryLimit: 40, ...givenOptions, }; @@ -633,8 +626,13 @@ describe('TransactionController', () => { 'AccountsController:getSelectedAccount', 'AccountsController:getState', 'ApprovalController:addRequest', - 'NetworkController:getNetworkClientById', + 'GasFeeController:fetchGasFeeEstimates', + 'KeyringController:signTransaction', 'NetworkController:findNetworkClientIdByChainId', + 'NetworkController:getEIP1559Compatibility', + 'NetworkController:getNetworkClientById', + 'NetworkController:getNetworkClientRegistry', + 'NetworkController:getState', 'RemoteFeatureFlagController:getState', ], }); @@ -659,6 +657,36 @@ describe('TransactionController', () => { remoteFeatureFlagControllerGetStateMock, ); + rootMessenger.registerActionHandler( + 'NetworkController:getEIP1559Compatibility', + async () => false, + ); + + rootMessenger.registerActionHandler( + 'NetworkController:getState', + () => + ({ + ...getDefaultNetworkControllerState(), + selectedNetworkClientId: MOCK_NETWORK.state.selectedNetworkClientId, + ...network.state, + }) as NetworkState, + ); + + rootMessenger.registerActionHandler( + 'NetworkController:getNetworkClientRegistry', + () => ({}) as never, + ); + + rootMessenger.registerActionHandler( + 'GasFeeController:fetchGasFeeEstimates', + async () => ({}) as never, + ); + + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + signMock, + ); + const controller = new TransactionController({ ...otherOptions, messenger: transactionControllerMessenger, @@ -923,7 +951,9 @@ describe('TransactionController', () => { (transactionMeta) => transactionMeta, ); - signMock = jest.fn().mockImplementation(async (transaction) => transaction); + signMock = jest + .fn() + .mockImplementation(async (transaction) => transaction.toJSON()); isEIP7702GasFeeTokensEnabledMock = jest.fn().mockResolvedValue(false); getBalanceChangesMock.mockResolvedValue({ simulationData: SIMULATION_DATA_RESULT_MOCK, @@ -1628,19 +1658,6 @@ describe('TransactionController', () => { it.each([ [TransactionEnvelopeType.legacy], - [ - TransactionEnvelopeType.feeMarket, - { - maxFeePerGas: '0x1', - maxPriorityFeePerGas: '0x1', - }, - { - getCurrentNetworkEIP1559Compatibility: async (): Promise => - true, - getCurrentAccountEIP1559Compatibility: async (): Promise => - true, - }, - ], [TransactionEnvelopeType.accessList, { accessList: [] }], [TransactionEnvelopeType.setCode, { authorizationList: [] }], ])( @@ -1648,11 +1665,8 @@ describe('TransactionController', () => { async ( type: TransactionEnvelopeType, extraTxParamsToSet: Partial = {}, - options: Partial< - ConstructorParameters[0] - > = {}, ) => { - const { controller } = setupController({ options }); + const { controller } = setupController(); await controller.addTransaction( { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, ...extraTxParamsToSet }, @@ -1663,6 +1677,32 @@ describe('TransactionController', () => { }, ); + it('sets txParams.type to 0x2 if not defined in given txParams', async () => { + const { controller, rootMessenger } = setupController(); + + rootMessenger.unregisterActionHandler( + 'NetworkController:getEIP1559Compatibility', + ); + rootMessenger.registerActionHandler( + 'NetworkController:getEIP1559Compatibility', + async () => true, + ); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + maxFeePerGas: '0x1', + maxPriorityFeePerGas: '0x1', + }, + { isInternal: true, networkClientId: NETWORK_CLIENT_ID_MOCK }, + ); + + expect(controller.state.transactions[0].txParams.type).toBe( + TransactionEnvelopeType.feeMarket, + ); + }); + describe('networkClientId exists in the MultichainTrackingHelper', () => { it('adds unapproved transaction to state when using networkClientId', async () => { const { controller } = setupController(); @@ -1971,47 +2011,6 @@ describe('TransactionController', () => { ); }); - it('calls security provider with transaction meta and sets response in to securityProviderResponse', async () => { - const mockRPCMethodName = 'MOCK_RPC_METHOD_NAME'; - const mockSecurityProviderResponse = { - flagAsDangerous: 1, - info: 'Mock info', - }; - const securityProviderRequestMock = jest - .fn() - .mockResolvedValue(mockSecurityProviderResponse); - - const { controller } = setupController({ - options: { - securityProviderRequest: securityProviderRequestMock, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - method: mockRPCMethodName, - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - expect(securityProviderRequestMock).toHaveBeenCalledTimes(1); - expect(securityProviderRequestMock).toHaveBeenCalledWith( - expect.objectContaining({ - id: MOCK_V1_UUID, - }), - mockRPCMethodName, - ); - - const { securityProviderResponse } = controller.state.transactions[0]; - expect(securityProviderResponse).toStrictEqual( - mockSecurityProviderResponse, - ); - }); - it('updates gas properties', async () => { const { controller } = setupController(); @@ -2036,12 +2035,15 @@ describe('TransactionController', () => { }); it('updates gas fee properties', async () => { - const { controller } = setupController({ - options: { - getCurrentNetworkEIP1559Compatibility: async () => true, - getCurrentAccountEIP1559Compatibility: async () => true, - }, - }); + const { controller, rootMessenger } = setupController(); + + rootMessenger.unregisterActionHandler( + 'NetworkController:getEIP1559Compatibility', + ); + rootMessenger.registerActionHandler( + 'NetworkController:getEIP1559Compatibility', + async () => true, + ); await controller.addTransaction( { @@ -2180,171 +2182,6 @@ describe('TransactionController', () => { }); }); - describe('with afterSimulate hook', () => { - it('calls afterSimulate hook', async () => { - const afterSimulateHook = jest.fn().mockResolvedValueOnce({}); - - const { controller } = setupController({ - options: { - hooks: { - afterSimulate: afterSimulateHook, - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(afterSimulateHook).toHaveBeenCalledTimes(1); - }); - - it('updates transaction if update callback returned', async () => { - const updateTransactionMock = jest.fn(); - - const afterSimulateHook = jest - .fn() - .mockResolvedValueOnce({ updateTransaction: updateTransactionMock }); - - const { controller } = setupController({ - options: { - hooks: { - afterSimulate: afterSimulateHook, - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(updateTransactionMock).toHaveBeenCalledTimes(1); - }); - - it('saves original transaction params if update callback returned', async () => { - const updateTransactionMock = jest.fn(); - - const afterSimulateHook = jest - .fn() - .mockResolvedValueOnce({ updateTransaction: updateTransactionMock }); - - const { controller } = setupController({ - options: { - hooks: { - afterSimulate: afterSimulateHook, - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(controller.state.transactions[0].txParamsOriginal).toStrictEqual( - expect.objectContaining({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }), - ); - }); - - it('will re-simulate balance changes if hook returns skipSimulation as false', async () => { - const afterSimulateHook = jest - .fn() - .mockResolvedValue({ skipSimulation: false }); - - const { controller } = setupController({ - options: { - hooks: { - afterSimulate: afterSimulateHook, - }, - }, - }); - - const { transactionMeta } = await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - shouldResimulateMock.mockReturnValue({ - blockTime: 123, - resimulate: true, - }); - - await controller.updateEditableParams(transactionMeta.id, {}); - - expect(getBalanceChangesMock).toHaveBeenCalledTimes(2); - }); - - it('will not re-simulate balance changes if hook returns skipSimulation as true', async () => { - const afterSimulateHook = jest - .fn() - .mockResolvedValue({ skipSimulation: true }); - - const { controller } = setupController({ - options: { - hooks: { - afterSimulate: afterSimulateHook, - }, - }, - }); - - const { transactionMeta } = await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - shouldResimulateMock.mockReturnValue({ - blockTime: 123, - resimulate: true, - }); - - await controller.updateEditableParams(transactionMeta.id, {}); - - await flushPromises(); - - expect(getBalanceChangesMock).toHaveBeenCalledTimes(1); - }); - }); - it('skips simulation when containerTypes includes EnforcedSimulations', async () => { const { controller } = setupController(); @@ -3093,8 +2930,7 @@ describe('TransactionController', () => { it('reports error to approval acceptor on error', async () => { const errorCallback = jest.fn(); - const { controller } = setupController({ - options: { sign: undefined }, + const { controller, rootMessenger } = setupController({ messengerOptions: { addTransactionApprovalRequest: { state: 'approved', @@ -3110,6 +2946,10 @@ describe('TransactionController', () => { }, }); + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + const { result } = await controller.addTransaction( { from: ACCOUNT_MOCK, @@ -3277,12 +3117,7 @@ describe('TransactionController', () => { } it('if signing error', async () => { - const { controller } = setupController({ - options: { - sign: () => { - throw new Error('foo'); - }, - }, + const { controller, rootMessenger } = setupController({ messengerOptions: { addTransactionApprovalRequest: { state: 'approved', @@ -3290,14 +3125,21 @@ describe('TransactionController', () => { }, }); + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + () => { + throw new Error('foo'); + }, + ); + await expectTransactionToFail(controller, 'foo'); }); it('if no sign method defined', async () => { - const { controller } = setupController({ - options: { - sign: undefined, - }, + const { controller, rootMessenger } = setupController({ messengerOptions: { addTransactionApprovalRequest: { state: 'approved', @@ -3305,7 +3147,14 @@ describe('TransactionController', () => { }, }); - await expectTransactionToFail(controller, 'No sign method defined'); + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + + await expectTransactionToFail( + controller, + 'A handler for KeyringController:signTransaction has not been registered', + ); }); it('if unexpected status', async () => { @@ -4079,16 +3928,27 @@ describe('TransactionController', () => { }); it('throws if no sign method', async () => { - const { controller } = setupController({ options: { sign: undefined } }); + const { controller, rootMessenger } = setupController(); + + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); await controller.addTransaction( - { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }, + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + gasPrice: '0x1', + gas: '0x5208', + }, { networkClientId: NETWORK_CLIENT_ID_MOCK }, ); await expect( controller.stopTransaction(controller.state.transactions[0].id), - ).rejects.toThrow('No sign method defined'); + ).rejects.toThrow( + 'A handler for KeyringController:signTransaction has not been registered', + ); }); it('publishes transaction events', async () => { @@ -4222,9 +4082,6 @@ describe('TransactionController', () => { it('creates additional transaction', async () => { const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, - options: { - getCurrentNetworkEIP1559Compatibility: async () => false, - }, }); const { transactionMeta } = await controller.addTransaction( @@ -4356,9 +4213,6 @@ describe('TransactionController', () => { it('creates additional transaction with increased gas', async () => { const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, - options: { - getCurrentNetworkEIP1559Compatibility: async () => false, - }, }); const { transactionMeta } = await controller.addTransaction( @@ -4378,25 +4232,27 @@ describe('TransactionController', () => { const { transactions } = controller.state; expect(transactions).toHaveLength(2); - expect(transactions[1].txParams.gasPrice).toBe( - '0x5916a6d6', // 1.1 * 0x50fd51da - ); + expect(transactions[1].txParams.gasPrice).toBe('0x5916a6d6'); }); it('verifies s,r and v values are correctly populated', async () => { - const { controller } = setupController({ + const { controller, rootMessenger } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, - options: { - sign: async (transaction) => { - return Object.assign(transaction, { - r: 128n, - s: 256n, - v: 512n, - }); - }, - }, }); + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + async (transaction) => ({ + ...transaction.toJSON(), + r: 128n, + s: 256n, + v: 512n, + }), + ); + const { transactionMeta } = await controller.addTransaction( { from: ACCOUNT_MOCK, @@ -4423,21 +4279,23 @@ describe('TransactionController', () => { }); it('verifies s,r and v values are correctly populated if values are zero', async () => { - const { controller } = setupController({ + const { controller, rootMessenger } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, - options: { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sign: async (transaction: any) => { - return Object.assign(transaction, { - r: 0n, - s: 0n, - v: 0n, - }); - }, - }, }); + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + async (transaction) => ({ + ...transaction.toJSON(), + r: 0n, + s: 0n, + v: 27n, + }), + ); + const { transactionMeta } = await controller.addTransaction( { from: ACCOUNT_MOCK, @@ -4458,15 +4316,12 @@ describe('TransactionController', () => { const speedUpTransaction = transactions[1]; expect(speedUpTransaction.r).toBe('0x0'); expect(speedUpTransaction.s).toBe('0x0'); - expect(speedUpTransaction.v).toBe('0x0'); + expect(speedUpTransaction.v).toBe('0x1b'); }); it('creates additional transaction specifying the gasPrice', async () => { const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, - options: { - getCurrentNetworkEIP1559Compatibility: async () => false, - }, }); const { transactionMeta } = await controller.addTransaction( @@ -5046,9 +4901,7 @@ describe('TransactionController', () => { it('limits max transactions when adding to state', async () => { getTransactionHistoryLimitMock.mockReturnValue(1); - const { controller } = setupController({ - options: { transactionHistoryLimit: 1 }, - }); + const { controller } = setupController(); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -5927,11 +5780,12 @@ describe('TransactionController', () => { describe('approveTransactionsWithSameNonce', () => { it('throws error if no sign method', async () => { - const { controller } = setupController({ - options: { - sign: undefined, - }, - }); + const { controller, rootMessenger } = setupController(); + + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + const mockTransactionParam2 = { from: ACCOUNT_MOCK, nonce: '0x1', @@ -5943,7 +5797,9 @@ describe('TransactionController', () => { await expect( controller.approveTransactionsWithSameNonce([mockTransactionParam2]), - ).rejects.toThrow('No sign method defined.'); + ).rejects.toThrow( + 'A handler for KeyringController:signTransaction has not been registered', + ); }); it('returns empty string if no transactions are provided', async () => { @@ -5953,15 +5809,19 @@ describe('TransactionController', () => { }); it('return empty string if transaction is already being signed', async () => { - const { controller } = setupController({ - options: { - // We never resolve this promise, so the transaction is always in the process of being signed - sign: async () => - new Promise(() => { - /* noop */ - }), - }, - }); + const { controller, rootMessenger } = setupController(); + + // We never resolve this promise, so the transaction is always in the process of being signed + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + async () => + new Promise(() => { + /* noop */ + }), + ); const mockTransactionParam = { from: ACCOUNT_MOCK, nonce: '0x1', @@ -5984,8 +5844,8 @@ describe('TransactionController', () => { }); it('signs transactions and return raw transactions', async () => { - signMock.mockImplementation(async (transactionParams) => - Promise.resolve(TransactionFactory.fromTxData(transactionParams)), + signMock.mockImplementation(async (transaction) => + Promise.resolve(transaction.toJSON()), ); const { controller } = setupController(); @@ -6023,11 +5883,7 @@ describe('TransactionController', () => { Promise.reject(new Error(mockSignError)), ); - const { controller } = setupController({ - options: { - sign: signMock, - }, - }); + const { controller } = setupController(); const mockTransactionParam = { from: ACCOUNT_MOCK, nonce: '0x1', @@ -6134,113 +5990,6 @@ describe('TransactionController', () => { status: TransactionStatus.approved, }; - it('adds a transaction, signs and update status to `approved`', async () => { - const { controller, mockTransactionApprovalRequest } = setupController({ - options: { - hooks: { - afterSign: () => false, - beforePublish: () => Promise.resolve(false), - getAdditionalSignArguments: () => [metadataMock], - }, - }, - }); - - const updateTransactionSpy = jest.spyOn(controller, 'updateTransaction'); - - await controller.addTransaction(paramsMock, { - origin: 'origin', - actionId: ACTION_ID_MOCK, - networkClientId: NETWORK_CLIENT_ID_MOCK, - }); - - mockTransactionApprovalRequest.approve({ - value: TRANSACTION_META_MOCK, - }); - await wait(0); - - const transactionMeta = controller.state.transactions[0]; - - expect(signMock).toHaveBeenCalledTimes(1); - - expect(transactionMeta.txParams).toStrictEqual( - expect.objectContaining(paramsMock), - ); - expect(updateTransactionSpy).toHaveBeenCalledTimes(1); - expect(updateTransactionSpy).toHaveBeenCalledWith( - expect.objectContaining({ - txParams: expect.objectContaining(paramsMock), - }), - 'TransactionController#signTransaction - Update after sign', - ); - - expect(transactionMeta.status).toBe(TransactionStatus.approved); - }); - - it('adds a transaction and signing returns undefined', async () => { - signMock.mockResolvedValue(undefined); - - const { controller, mockTransactionApprovalRequest } = setupController({ - options: { - hooks: { - afterSign: () => false, - beforePublish: () => Promise.resolve(false), - getAdditionalSignArguments: () => [metadataMock], - }, - }, - }); - - await controller.addTransaction(paramsMock, { - origin: 'origin', - actionId: ACTION_ID_MOCK, - networkClientId: NETWORK_CLIENT_ID_MOCK, - }); - - mockTransactionApprovalRequest.approve({ - value: TRANSACTION_META_MOCK, - }); - await wait(0); - - expect(signMock).toHaveBeenCalledTimes(1); - }); - - it('adds a transaction, signs and skips publish the transaction', async () => { - const { controller, mockTransactionApprovalRequest } = setupController({ - options: { - hooks: { - beforePublish: undefined, - afterSign: () => false, - getAdditionalSignArguments: () => [metadataMock], - }, - }, - }); - - const updateTransactionSpy = jest.spyOn(controller, 'updateTransaction'); - - await controller.addTransaction(paramsMock, { - origin: 'origin', - actionId: ACTION_ID_MOCK, - networkClientId: NETWORK_CLIENT_ID_MOCK, - }); - - mockTransactionApprovalRequest.approve(); - await wait(0); - - const transactionMeta = controller.state.transactions[0]; - - expect(transactionMeta.txParams).toStrictEqual( - expect.objectContaining(paramsMock), - ); - - expect(signMock).toHaveBeenCalledTimes(1); - expect(updateTransactionSpy).toHaveBeenCalledTimes(1); - expect(updateTransactionSpy).toHaveBeenCalledWith( - expect.objectContaining({ - txParams: expect.objectContaining(paramsMock), - }), - 'TransactionController#signTransaction - Update after sign', - ); - }); - it('gets transaction hash from publish hook and does not submit to provider', async () => { const { controller } = setupController({ options: { @@ -7616,10 +7365,7 @@ describe('TransactionController', () => { }); it('sets status to failed if transaction being signed', async () => { - const { controller } = setupController({ - options: { - sign: jest.fn().mockReturnValue(createDeferredPromise().promise), - }, + const { controller, rootMessenger } = setupController({ messengerOptions: { addTransactionApprovalRequest: { state: 'approved', @@ -7627,6 +7373,14 @@ describe('TransactionController', () => { }, }); + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + () => createDeferredPromise().promise, + ); + const { transactionMeta, result } = await controller.addTransaction( { from: ACCOUNT_MOCK, @@ -8654,14 +8408,21 @@ describe('TransactionController', () => { describe('TransactionController:transactionApproved event', () => { it('does not publish transactionApproved event when keyring throws during signing', async () => { const signingError = new Error('Keyring signing failed'); - const mockSignWithError = jest.fn().mockRejectedValue(signingError); - const { controller, messenger, mockTransactionApprovalRequest } = - setupController({ - options: { - sign: mockSignWithError, - }, - }); + const { + controller, + messenger, + rootMessenger, + mockTransactionApprovalRequest, + } = setupController(); + + rootMessenger.unregisterActionHandler( + 'KeyringController:signTransaction', + ); + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + jest.fn().mockRejectedValue(signingError), + ); const approvedEventListener = jest.fn(); const mockActionId = 'test-action-id-456'; @@ -8692,9 +8453,6 @@ describe('TransactionController', () => { const transaction = controller.state.transactions[0]; expect(transaction.status).toBe(TransactionStatus.failed); - expect(mockSignWithError).toHaveBeenCalledTimes(1); - - // Verify the transactionApproved event was NOT published despite approval expect(approvedEventListener).not.toHaveBeenCalled(); }); }); diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 1406e1d60d..ea877e4908 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -1,4 +1,4 @@ -import type { TypedTransaction } from '@ethereumjs/tx'; +import type { TypedTransaction, TypedTxData } from '@ethereumjs/tx'; import type { AccountsControllerActions } from '@metamask/accounts-controller'; import type { ApprovalControllerActions, @@ -14,6 +14,8 @@ import { InfuraNetworkType, NetworkType, } from '@metamask/controller-utils'; +import type { GasFeeControllerFetchGasFeeEstimatesAction } from '@metamask/gas-fee-controller'; +import type { KeyringControllerSignTransactionAction } from '@metamask/keyring-controller'; import { MOCK_ANY_NAMESPACE, Messenger } from '@metamask/messenger'; import type { MockAnyNamespace, @@ -83,7 +85,9 @@ type AllActions = | ConnectivityControllerGetStateAction | ApprovalControllerActions | AccountsControllerActions - | RemoteFeatureFlagControllerGetStateAction; + | RemoteFeatureFlagControllerGetStateAction + | GasFeeControllerFetchGasFeeEstimatesAction + | KeyringControllerSignTransactionAction; type AllEvents = | AllTransactionControllerEvents @@ -251,8 +255,13 @@ const setupController = async ( 'AccountsController:getSelectedAccount', 'AccountsController:getState', 'ApprovalController:addRequest', - 'NetworkController:getNetworkClientById', + 'GasFeeController:fetchGasFeeEstimates', + 'KeyringController:signTransaction', 'NetworkController:findNetworkClientIdByChainId', + 'NetworkController:getEIP1559Compatibility', + 'NetworkController:getNetworkClientById', + 'NetworkController:getNetworkClientRegistry', + 'NetworkController:getState', 'RemoteFeatureFlagController:getState', ], events: ['NetworkController:stateChange'], @@ -295,30 +304,26 @@ const setupController = async ( () => getDefaultRemoteFeatureFlagControllerState(), ); + rootMessenger.registerActionHandler( + 'GasFeeController:fetchGasFeeEstimates', + async () => ({}) as never, + ); + + rootMessenger.registerActionHandler( + 'KeyringController:signTransaction', + async (transaction: TypedTransaction) => + transaction.toJSON() as TypedTxData, + ); + const options: TransactionControllerOptions = { - disableHistory: false, - disableSendFlowHistory: false, disableSwaps: false, isAutomaticGasFeeUpdateEnabled: () => true, - getCurrentNetworkEIP1559Compatibility: async ( - networkClientId?: NetworkClientId, - ) => { - return ( - (await networkController.getEIP1559Compatibility(networkClientId)) ?? - false - ); - }, - getNetworkState: () => networkController.state, - getNetworkClientRegistry: () => - networkController.getNetworkClientRegistry(), getPermittedAccounts: async () => [ACCOUNT_MOCK], hooks: {}, messenger, pendingTransactions: { isResubmitEnabled: () => false, }, - sign: async (transaction: TypedTransaction) => transaction, - transactionHistoryLimit: 40, ...givenOptions, }; @@ -1223,40 +1228,6 @@ describe('TransactionController Integration', () => { transactionController.destroy(); }); - describe('feature flag', () => { - it('should call getNetworkClientRegistry on networkController:stateChange when feature flag is enabled', async () => { - uuidV4Mock.mockReturnValue('AAAA-AAAA-AAAA-AAAA'); - - const { networkController, transactionController } = - await setupController(); - const getNetworkClientRegistrySpy = jest.spyOn( - networkController, - 'getNetworkClientRegistry', - ); - - networkController.addNetwork(buildAddNetworkFields()); - - expect(getNetworkClientRegistrySpy).toHaveBeenCalled(); - transactionController.destroy(); - }); - - it('should call getNetworkClientRegistry on construction when feature flag is enabled', async () => { - const getNetworkClientRegistrySpy = jest.fn().mockImplementation(() => { - return { - [NetworkType.sepolia]: { - configuration: BUILT_IN_NETWORKS[NetworkType.sepolia], - }, - }; - }); - - await setupController({ - getNetworkClientRegistry: getNetworkClientRegistrySpy, - }); - - expect(getNetworkClientRegistrySpy).toHaveBeenCalled(); - }); - }); - describe('getNonceLock', () => { it('should get the nonce lock from the nonceTracker for the given networkClientId', async () => { const { networkController, transactionController } = diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts index 8ad6708e49..4744e5e003 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts @@ -826,7 +826,7 @@ describe('PendingTransactionTracker', () => { await onPoll(); - expect(listener).toHaveBeenCalledTimes(2); + expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith( expect.objectContaining({ ...TRANSACTION_SUBMITTED_MOCK, @@ -878,9 +878,8 @@ describe('PendingTransactionTracker', () => { ), ); - expect(listener).toHaveBeenCalledTimes(2); - expect(listener).toHaveBeenNthCalledWith( - 1, + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith( expect.objectContaining({ ...TRANSACTION_SUBMITTED_MOCK, warning: { @@ -893,395 +892,6 @@ describe('PendingTransactionTracker', () => { }); }); }); - - describe('resubmits', () => { - describe('does nothing', () => { - it('if no pending transactions', async () => { - pendingTransactionTracker = new PendingTransactionTracker(options); - - await onPoll(undefined, []); - - expect(options.publishTransaction).toHaveBeenCalledTimes(0); - }); - }); - - describe('fires updated event', () => { - it('if first retry check', async () => { - const listener = jest.fn(); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions: (): TransactionMeta[] => - freeze([{ ...TRANSACTION_SUBMITTED_MOCK }], true), - }); - - pendingTransactionTracker.hub.addListener( - 'transaction-updated', - listener, - ); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - await onPoll(BLOCK_NUMBER_MOCK); - - expect(listener).toHaveBeenCalledTimes(1); - expect(listener).toHaveBeenCalledWith( - { - ...TRANSACTION_SUBMITTED_MOCK, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - 'PendingTransactionTracker:#isResubmitDue - First retry block number set', - ); - }); - - it('if published', async () => { - const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; - const getTransactions = jest - .fn() - .mockReturnValue(freeze([transaction], true)); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions, - }); - - const listener = jest.fn(); - pendingTransactionTracker.hub.addListener( - 'transaction-updated', - listener, - ); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - await onPoll(BLOCK_NUMBER_MOCK); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - ], - true, - ), - ); - await onPoll('0x124'); - - expect(listener).toHaveBeenCalledTimes(2); - expect(listener).toHaveBeenCalledWith( - { - ...TRANSACTION_SUBMITTED_MOCK, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - retryCount: 1, - }, - 'PendingTransactionTracker:transaction-retry - Retry count increased', - ); - }); - - it('if beforeCheckPendingTransaction returns false, does not resubmit the transaction', async () => { - const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; - const getTransactions = jest - .fn() - .mockReturnValue(freeze([transaction], true)); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions, - hooks: { - beforeCheckPendingTransaction: (): Promise => - Promise.resolve(false), - }, - }); - - const listener = jest.fn(); - pendingTransactionTracker.hub.addListener( - 'transaction-updated', - listener, - ); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - await onPoll(BLOCK_NUMBER_MOCK); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - ], - true, - ), - ); - await onPoll('0x124'); - - expect(listener).toHaveBeenCalledTimes(1); - expect(listener).toHaveBeenCalledWith( - { - ...TRANSACTION_SUBMITTED_MOCK, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - 'PendingTransactionTracker:#isResubmitDue - First retry block number set', - ); - expect(options.publishTransaction).toHaveBeenCalledTimes(0); - }); - - it('if publishing fails', async () => { - const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; - const getTransactions = jest - .fn() - .mockReturnValue(freeze([transaction], true)); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions, - }); - - const listener = jest.fn(); - pendingTransactionTracker.hub.addListener( - 'transaction-updated', - listener, - ); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - options.publishTransaction.mockRejectedValueOnce( - new Error('TestError'), - ); - - await onPoll(BLOCK_NUMBER_MOCK); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - ], - true, - ), - ); - await onPoll('0x124'); - - expect(listener).toHaveBeenCalledTimes(2); - expect(listener).toHaveBeenCalledWith( - { - ...TRANSACTION_SUBMITTED_MOCK, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - warning: { - error: 'TestError', - message: - 'There was an error when resubmitting this transaction.', - }, - }, - 'PendingTransactionTracker:#warnTransaction - Warning added', - ); - }); - - it('unless publishing fails and known error', async () => { - const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; - const getTransactions = jest - .fn() - .mockReturnValue(freeze([transaction], true)); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions, - }); - - const listener = jest.fn(); - pendingTransactionTracker.hub.addListener( - 'transaction-updated', - listener, - ); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - options.publishTransaction.mockRejectedValueOnce( - new Error('test gas price too low to replace test'), - ); - - await onPoll(BLOCK_NUMBER_MOCK); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - ], - true, - ), - ); - await onPoll('0x124'); - - expect(listener).toHaveBeenCalledTimes(1); - expect(listener).not.toHaveBeenCalledWith( - expect.any(Object), - 'PendingTransactionTracker:#warnTransaction - Warning added', - ); - }); - }); - - describe('publishes transaction', () => { - it('if latest block number increased', async () => { - const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; - const getTransactions = jest - .fn() - .mockReturnValue(freeze([transaction], true)); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions, - }); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - await onPoll(BLOCK_NUMBER_MOCK); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - ], - true, - ), - ); - await onPoll('0x124'); - - expect(options.publishTransaction).toHaveBeenCalledTimes(1); - expect(options.publishTransaction).toHaveBeenCalledWith({ - ...TRANSACTION_SUBMITTED_MOCK, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }); - }); - - it('if latest block number matches retry count exponential delay', async () => { - const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; - const getTransactions = jest - .fn() - .mockReturnValue(freeze([transaction], true)); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions, - }); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - await onPoll(BLOCK_NUMBER_MOCK); - expect(options.publishTransaction).toHaveBeenCalledTimes(0); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - ], - true, - ), - ); - - await onPoll('0x124'); - expect(options.publishTransaction).toHaveBeenCalledTimes(1); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - retryCount: 1, - }, - ], - true, - ), - ); - - await onPoll('0x125'); - expect(options.publishTransaction).toHaveBeenCalledTimes(2); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - retryCount: 2, - }, - ], - true, - ), - ); - - await onPoll('0x126'); - expect(options.publishTransaction).toHaveBeenCalledTimes(2); - - await onPoll('0x127'); - expect(options.publishTransaction).toHaveBeenCalledTimes(3); - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - retryCount: 3, - }, - ], - true, - ), - ); - - await onPoll('0x12A'); - expect(options.publishTransaction).toHaveBeenCalledTimes(3); - - await onPoll('0x12B'); - expect(options.publishTransaction).toHaveBeenCalledTimes(4); - }); - - it('unless resubmit disabled', async () => { - const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; - const getTransactions = jest - .fn() - .mockReturnValueOnce(freeze([transaction], true)); - - pendingTransactionTracker = new PendingTransactionTracker({ - ...options, - getTransactions, - isResubmitEnabled: (): boolean => false, - }); - - getTransactionReceiptMock.mockResolvedValueOnce(undefined); - getTransactionCountMock.mockResolvedValueOnce('0x1'); - - await onPoll(BLOCK_NUMBER_MOCK); - - getTransactions.mockReturnValue( - freeze( - [ - { - ...transaction, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, - }, - ], - true, - ), - ); - - await onPoll('0x124'); - - expect(options.publishTransaction).toHaveBeenCalledTimes(0); - }); - }); - }); }); describe('forceCheckTransaction', () => { diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index 8b2288398d..c0b8dfd155 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -231,7 +231,6 @@ export class PendingTransactionTracker { } finally { releaseLock(); } - } async #checkTransactions(): Promise { diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index c9f8a8c9c8..48cbcd8d05 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -2119,7 +2119,6 @@ export type AfterAddHook = (request: { updateTransaction?: (transaction: TransactionMeta) => void; }>; - /** * Custom logic to be executed before a transaction is signed. * Can optionally update the transaction by returning the `updateTransaction` callback. From bb41091482e6eca5364f92389c689c6d107dca83 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 12:49:15 +0100 Subject: [PATCH 6/8] refactor: fix lint errors in transaction controller and related files --- .../src/TransactionController.test.ts | 20 +----------- .../src/TransactionController.ts | 32 +++++++++++++------ .../TransactionControllerIntegration.test.ts | 2 -- .../src/helpers/PendingTransactionTracker.ts | 11 ++----- 4 files changed, 25 insertions(+), 40 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 3a824c862e..b8c4560efc 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -3,7 +3,7 @@ /* eslint-disable camelcase */ /* eslint-disable jest/expect-expect */ -import { TransactionFactory } from '@ethereumjs/tx'; + import type { ApprovalControllerAddRequestAction, AddResult, @@ -816,15 +816,6 @@ describe('TransactionController', () => { }; } - /** - * Wait for a specified number of milliseconds. - * - * @param ms - The number of milliseconds to wait. - */ - async function wait(ms: number): Promise { - await new Promise((resolve) => setTimeout(resolve, ms)); - } - beforeEach(() => { jest.resetAllMocks(); @@ -5981,15 +5972,6 @@ describe('TransactionController', () => { to: ACCOUNT_MOCK, }; - const metadataMock: TransactionMeta = { - txParams: paramsMock, - chainId: '0x1' as const, - networkClientId: NETWORK_CLIENT_ID_MOCK, - id: '1', - time: 0, - status: TransactionStatus.approved, - }; - it('gets transaction hash from publish hook and does not submit to provider', async () => { const { controller } = setupController({ options: { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index e02e3f0774..db798075ca 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1,4 +1,5 @@ -import type { TypedTransaction, TypedTxData } from '@ethereumjs/tx'; +/* eslint-disable no-restricted-syntax */ +import type { TypedTxData } from '@ethereumjs/tx'; import type { AccountsController, AccountsControllerGetSelectedAccountAction, @@ -27,7 +28,11 @@ import type { AccountActivityServiceTransactionUpdatedEvent, BackendWebSocketServiceConnectionStateChangedEvent, } from '@metamask/core-backend'; -import type { GasFeeControllerFetchGasFeeEstimatesAction } from '@metamask/gas-fee-controller'; +import type { + FetchGasFeeEstimateOptions, + GasFeeControllerFetchGasFeeEstimatesAction, + GasFeeState, +} from '@metamask/gas-fee-controller'; import type { KeyringControllerGetStateAction, KeyringControllerSignEip7702AuthorizationAction, @@ -866,8 +871,10 @@ export class TransactionController extends BaseController< networkClientId, ); }) as NetworkController['getNetworkClientById'], - getNetworkClientRegistry: () => - this.messenger.call('NetworkController:getNetworkClientRegistry'), + getNetworkClientRegistry: (() => + this.messenger.call( + 'NetworkController:getNetworkClientRegistry', + )) as NetworkController['getNetworkClientRegistry'], removePendingTransactionTrackerListeners: this.#removePendingTransactionTrackerListeners.bind(this), createNonceTracker: this.#createNonceTracker.bind(this), @@ -884,15 +891,20 @@ export class TransactionController extends BaseController< const gasFeePoller = new GasFeePoller({ gasFeeFlows: this.#gasFeeFlows, - getGasFeeControllerEstimates: (options) => - this.messenger.call('GasFeeController:fetchGasFeeEstimates', options), + getGasFeeControllerEstimates: ( + gasFeeOpts: FetchGasFeeEstimateOptions, + ): Promise => + this.messenger.call( + 'GasFeeController:fetchGasFeeEstimates', + gasFeeOpts, + ), getTransactions: (): TransactionMeta[] => this.state.transactions, getTransactionBatches: (): TransactionBatchMeta[] => this.state.transactionBatches, layer1GasFeeFlows: this.#layer1GasFeeFlows, messenger: this.messenger, onStateChange: (listener): void => { - this.messenger.subscribe('TransactionController:stateChange', listener); + this.messenger.subscribe('TransactionController:stateChanged', listener); }, }); @@ -942,7 +954,7 @@ export class TransactionController extends BaseController< // when transactionsController state changes // check for pending transactions and start polling if there are any this.messenger.subscribe( - 'TransactionController:stateChange', + 'TransactionController:stateChanged', this.#checkForPendingTransactionAndStartPolling, ); @@ -951,7 +963,7 @@ export class TransactionController extends BaseController< simulateTransaction: this.#updateSimulationData.bind(this), onTransactionsUpdate: (listener): void => { this.messenger.subscribe( - 'TransactionController:stateChange', + 'TransactionController:stateChanged', listener, (controllerState) => controllerState.transactions, ); @@ -1069,7 +1081,6 @@ export class TransactionController extends BaseController< isGasFeeSponsored, isInternal = false, isStateOnly, - method, nestedTransactions, networkClientId, origin, @@ -3749,6 +3760,7 @@ export class TransactionController extends BaseController< log('Signing transaction', finalTxParams); const signedTxData = await new Promise((resolve, reject) => { + // eslint-disable-next-line promise/catch-or-return this.messenger .call('KeyringController:signTransaction', unsignedEthTx, from) .then(resolve, reject); diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index ea877e4908..6c1e5c8106 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -12,7 +12,6 @@ import { BUILT_IN_NETWORKS, ChainId, InfuraNetworkType, - NetworkType, } from '@metamask/controller-utils'; import type { GasFeeControllerFetchGasFeeEstimatesAction } from '@metamask/gas-fee-controller'; import type { KeyringControllerSignTransactionAction } from '@metamask/keyring-controller'; @@ -30,7 +29,6 @@ import type { NetworkClientConfiguration, NetworkControllerActions, NetworkControllerEvents, - NetworkClientId, NetworkControllerOptions, } from '@metamask/network-controller'; import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index c0b8dfd155..691f229033 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -6,7 +6,7 @@ import type { Json } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. // eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; -import { cloneDeep, merge } from 'lodash'; +import { cloneDeep } from 'lodash'; import { createModuleLogger, projectLogger } from '../logger'; import type { TransactionControllerMessenger } from '../TransactionController'; @@ -87,10 +87,6 @@ export class PendingTransactionTracker { readonly #messenger: TransactionControllerMessenger; - readonly #publishTransaction: ( - transactionMeta: TransactionMeta, - ) => Promise; - #running: boolean; readonly #transactionPoller: TransactionPoller; @@ -105,7 +101,6 @@ export class PendingTransactionTracker { hooks, messenger, networkClientId, - publishTransaction, }: { blockTracker: BlockTracker; getGlobalLock: () => Promise<() => void>; @@ -118,7 +113,6 @@ export class PendingTransactionTracker { isTimeoutEnabled: (transactionMeta: TransactionMeta) => boolean; messenger: TransactionControllerMessenger; networkClientId: NetworkClientId; - publishTransaction: (transactionMeta: TransactionMeta) => Promise; }) { this.hub = new EventEmitter() as PendingTransactionTrackerEventEmitter; @@ -132,7 +126,6 @@ export class PendingTransactionTracker { this.#lastSeenTimestampByHash = new Map(); this.#listener = this.#onLatestBlock.bind(this); this.#messenger = messenger; - this.#publishTransaction = publishTransaction; this.#running = false; this.#transactionToForcePoll = undefined; @@ -220,7 +213,7 @@ export class PendingTransactionTracker { this.#log('Stopped polling'); } - async #onLatestBlock(latestBlockNumber: string): Promise { + async #onLatestBlock(_latestBlockNumber: string): Promise { const releaseLock = await this.#getGlobalLock(); try { From e7e5e115255fa1becef5e3c42a966858cfa60d21 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 12:52:07 +0100 Subject: [PATCH 7/8] refactor: fix prettier formatting in transaction controller files --- .../transaction-controller/src/TransactionController.test.ts | 1 - packages/transaction-controller/src/TransactionController.ts | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index b8c4560efc..203912721d 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -3,7 +3,6 @@ /* eslint-disable camelcase */ /* eslint-disable jest/expect-expect */ - import type { ApprovalControllerAddRequestAction, AddResult, diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index db798075ca..23f050504d 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -904,7 +904,10 @@ export class TransactionController extends BaseController< layer1GasFeeFlows: this.#layer1GasFeeFlows, messenger: this.messenger, onStateChange: (listener): void => { - this.messenger.subscribe('TransactionController:stateChanged', listener); + this.messenger.subscribe( + 'TransactionController:stateChanged', + listener, + ); }, }); From a836b7b169dc9d6fb6d58dd1bd1f86cdbf63fca3 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 3 Jun 2026 13:06:58 +0100 Subject: [PATCH 8/8] refactor: fix stateChange event names, remove publishTransaction, prune ESLint suppressions --- eslint-suppressions.json | 5 ----- .../src/TransactionController.ts | 13 +++---------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index eedb60a53b..4b9a2704a4 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -2166,11 +2166,6 @@ "count": 2 } }, - "packages/transaction-controller/src/TransactionController.ts": { - "no-restricted-syntax": { - "count": 7 - } - }, "packages/transaction-controller/src/TransactionControllerIntegration.test.ts": { "no-restricted-syntax": { "count": 1 diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 23f050504d..bf5a2a710f 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -904,10 +904,7 @@ export class TransactionController extends BaseController< layer1GasFeeFlows: this.#layer1GasFeeFlows, messenger: this.messenger, onStateChange: (listener): void => { - this.messenger.subscribe( - 'TransactionController:stateChanged', - listener, - ); + this.messenger.subscribe('TransactionController:stateChange', listener); }, }); @@ -957,7 +954,7 @@ export class TransactionController extends BaseController< // when transactionsController state changes // check for pending transactions and start polling if there are any this.messenger.subscribe( - 'TransactionController:stateChanged', + 'TransactionController:stateChange', this.#checkForPendingTransactionAndStartPolling, ); @@ -966,7 +963,7 @@ export class TransactionController extends BaseController< simulateTransaction: this.#updateSimulationData.bind(this), onTransactionsUpdate: (listener): void => { this.messenger.subscribe( - 'TransactionController:stateChanged', + 'TransactionController:stateChange', listener, (controllerState) => controllerState.transactions, ); @@ -3921,10 +3918,6 @@ export class TransactionController extends BaseController< isTimeoutEnabled: this.#isTimeoutEnabled, messenger: this.messenger, networkClientId, - publishTransaction: (transactionMeta): Promise => - this.#publishTransaction(transactionMeta, { - skipSubmitHistory: true, - }), }); this.#addPendingTransactionTrackerListeners(pendingTransactionTracker);