From 775a386402b86822529f66298230a53cbfc1a030 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 17 Sep 2025 07:56:14 -0600 Subject: [PATCH 1/5] Remove fn references to TransactionController in favor of messenger Currently, the constructor takes `getNonceLock`, `confirmExternalTransaction`, `getTransactions`, and `updateTransaction` options. In practice these are references to methods within TransactionController. This commit removes these options and calls the TransactionController methods via the messenger. --- CHANGELOG.md | 11 ++ package.json | 2 +- src/SmartTransactionsController.test.ts | 170 ++++++++++++++---------- src/SmartTransactionsController.ts | 50 +++---- src/index.test.ts | 4 - src/types.ts | 8 -- src/utils.test.ts | 81 +++++++++-- src/utils.ts | 37 ++++-- yarn.lock | 67 +++++----- 9 files changed, 266 insertions(+), 164 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c918f0a..758c36b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Upgrade `@metamask/base-controller` from `^7.0.1` to `^8.3.0` ([#529](https://github.com/MetaMask/smart-transactions-controller/pull/529)) - Upgrade `@metamask/polling-controller` from `^12.0.0` to `^14.0.0` ([#529](https://github.com/MetaMask/smart-transactions-controller/pull/529)) +### Removed + +- Remove `getNonceLock` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) + - The messenger is now used to access TransactionController. Please add the `TransactionController:getNonce` action to the messenger's allowlist. +- Remove `confirmExternalTransaction` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) + - The messenger is now used to access TransactionController. Please add the `TransactionController:confirmExternalTransaction` action to the messenger's allowlist. +- Remove `getTransactions` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) + - The messenger is now used to access TransactionController. Please add the `TransactionController:getTransactions` action to the messenger's allowlist. +- Remove `updateTransaction` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) + - The messenger is now used to access TransactionController. Please add the `TransactionController:updateTransaction` action to the messenger's allowlist. + ## [18.1.0] ### Added diff --git a/package.json b/package.json index 53de3cdb..5fa6c665 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "@metamask/gas-fee-controller": "^22.0.0", "@metamask/json-rpc-engine": "^10.0.1", "@metamask/network-controller": "^24.0.0", - "@metamask/transaction-controller": "^60.3.0", + "@metamask/transaction-controller": "^60.4.0", "@ts-bridge/cli": "^0.6.3", "@types/jest": "^26.0.24", "@types/lodash": "^4.14.194", diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index c57f1750..5b340e12 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -12,6 +12,12 @@ import { RpcEndpointType, type NetworkState, } from '@metamask/network-controller'; +import type { + TransactionControllerConfirmExternalTransactionAction, + TransactionControllerGetNonceLockAction, + TransactionControllerGetTransactionsAction, + TransactionControllerUpdateTransactionAction, +} from '@metamask/transaction-controller'; import { type TransactionParams, TransactionStatus, @@ -732,9 +738,7 @@ describe('SmartTransactionsController', () => { await withController( { - options: { - getNonceLock: mockGetNonceLock, - }, + getNonceLock: mockGetNonceLock, }, async ({ controller }) => { const signedTransaction = createSignedTransaction(); @@ -801,9 +805,7 @@ describe('SmartTransactionsController', () => { await withController( { - options: { - getNonceLock: mockGetNonceLock, - }, + getNonceLock: mockGetNonceLock, }, async ({ controller }) => { const signedTransaction = createSignedTransaction(); @@ -858,9 +860,7 @@ describe('SmartTransactionsController', () => { await withController( { - options: { - getNonceLock: mockGetNonceLock, - }, + getNonceLock: mockGetNonceLock, }, async ({ controller }) => { const signedTransaction = createSignedTransaction(); @@ -1239,9 +1239,9 @@ describe('SmartTransactionsController', () => { }, }, }, - confirmExternalTransaction: confirmExternalTransactionSpy, - getTransactions: getRegularTransactionsSpy, }, + confirmExternalTransaction: confirmExternalTransactionSpy, + getTransactions: getRegularTransactionsSpy, }, async ({ controller }) => { const updateTransaction = { @@ -1298,9 +1298,9 @@ describe('SmartTransactionsController', () => { }, }, }, - confirmExternalTransaction: confirmExternalTransactionSpy, - getTransactions: getRegularTransactionsSpy, }, + confirmExternalTransaction: confirmExternalTransactionSpy, + getTransactions: getRegularTransactionsSpy, }, async ({ controller }) => { const updateTransaction = { @@ -1357,9 +1357,9 @@ describe('SmartTransactionsController', () => { }, }, }, - confirmExternalTransaction: confirmExternalTransactionSpy, - getTransactions: getRegularTransactionsSpy, }, + confirmExternalTransaction: confirmExternalTransactionSpy, + getTransactions: getRegularTransactionsSpy, }, async ({ controller }) => { const updateTransaction = { @@ -1416,9 +1416,9 @@ describe('SmartTransactionsController', () => { }, }, }, - confirmExternalTransaction: confirmExternalTransactionSpy, - getTransactions: getRegularTransactionsSpy, }, + confirmExternalTransaction: confirmExternalTransactionSpy, + getTransactions: getRegularTransactionsSpy, }, async ({ controller }) => { const updateTransaction = { @@ -1475,9 +1475,9 @@ describe('SmartTransactionsController', () => { }, }, }, - confirmExternalTransaction: confirmExternalTransactionSpy, - getTransactions: getRegularTransactionsSpy, }, + confirmExternalTransaction: confirmExternalTransactionSpy, + getTransactions: getRegularTransactionsSpy, }, async ({ controller }) => { const updateTransaction = { @@ -1519,24 +1519,11 @@ describe('SmartTransactionsController', () => { await withController( { options: { - updateTransaction: mockUpdateTransaction, getFeatureFlags: () => ({ smartTransactions: { mobileReturnTxHashAsap: true, }, }), - getTransactions: () => [ - { - id: 'test-tx-id', - status: TransactionStatus.submitted, - chainId: '0x1', - time: 123, - txParams: { - from: '0x123', - }, - networkClientId: NetworkType.mainnet, - }, - ], state: { smartTransactionsState: { ...defaultState.smartTransactionsState, @@ -1546,6 +1533,19 @@ describe('SmartTransactionsController', () => { }, }, }, + updateTransaction: mockUpdateTransaction, + getTransactions: () => [ + { + id: 'test-tx-id', + status: TransactionStatus.submitted, + chainId: '0x1', + time: 123, + txParams: { + from: '0x123', + }, + networkClientId: NetworkType.mainnet, + }, + ], }, async ({ controller }) => { const smartTransaction = { @@ -1582,25 +1582,25 @@ describe('SmartTransactionsController', () => { await withController( { options: { - updateTransaction: mockUpdateTransaction, getFeatureFlags: () => ({ smartTransactions: { mobileReturnTxHashAsap: false, }, }), - getTransactions: () => [ - { - id: 'test-tx-id', - status: TransactionStatus.submitted, - chainId: '0x1', - time: 123, - txParams: { - from: '0x123', - }, - networkClientId: NetworkType.mainnet, - }, - ], }, + updateTransaction: mockUpdateTransaction, + getTransactions: () => [ + { + id: 'test-tx-id', + status: TransactionStatus.submitted, + chainId: '0x1', + time: 123, + txParams: { + from: '0x123', + }, + networkClientId: NetworkType.mainnet, + }, + ], }, async ({ controller }) => { const smartTransaction = { @@ -1622,14 +1622,14 @@ describe('SmartTransactionsController', () => { await withController( { options: { - updateTransaction: mockUpdateTransaction, getFeatureFlags: () => ({ smartTransactions: { mobileReturnTxHashAsap: true, }, }), - getTransactions: () => [], }, + updateTransaction: mockUpdateTransaction, + getTransactions: () => [], }, async ({ controller }) => { const smartTransaction = { @@ -1650,25 +1650,25 @@ describe('SmartTransactionsController', () => { await withController( { options: { - updateTransaction: mockUpdateTransaction, getFeatureFlags: () => ({ smartTransactions: { mobileReturnTxHashAsap: true, }, }), - getTransactions: () => [ - { - id: 'test-tx-id', - status: TransactionStatus.submitted, - chainId: '0x1', - time: 123, - txParams: { - from: '0x123', - }, - networkClientId: NetworkType.mainnet, - }, - ], }, + updateTransaction: mockUpdateTransaction, + getTransactions: () => [ + { + id: 'test-tx-id', + status: TransactionStatus.submitted, + chainId: '0x1', + time: 123, + txParams: { + from: '0x123', + }, + networkClientId: NetworkType.mainnet, + }, + ], }, async ({ controller }) => { const smartTransaction = { @@ -2682,6 +2682,10 @@ type WithControllerOptions = { options?: Partial< ConstructorParameters[0] >; + getNonceLock?: TransactionControllerGetNonceLockAction['handler']; + confirmExternalTransaction?: TransactionControllerConfirmExternalTransactionAction['handler']; + getTransactions?: TransactionControllerGetTransactionsAction['handler']; + updateTransaction?: TransactionControllerUpdateTransactionAction['handler']; }; type WithControllerArgs = @@ -2701,11 +2705,25 @@ async function withController( ...args: WithControllerArgs ): Promise { const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]]; - const { options } = rest; + const { + options, + getNonceLock = jest.fn().mockResolvedValue({ + nextNonce: 42, + releaseLock: jest.fn(), + }), + confirmExternalTransaction = jest.fn(), + getTransactions = jest.fn(), + updateTransaction = jest.fn(), + } = rest; + const controllerMessenger = new Messenger< | SmartTransactionsControllerActions | NetworkControllerGetNetworkClientByIdAction - | NetworkControllerGetStateAction, + | NetworkControllerGetStateAction + | TransactionControllerGetNonceLockAction + | TransactionControllerConfirmExternalTransactionAction + | TransactionControllerGetTransactionsAction + | TransactionControllerUpdateTransactionAction, SmartTransactionsControllerEvents | NetworkControllerStateChangeEvent >(); controllerMessenger.registerActionHandler( @@ -2731,7 +2749,6 @@ async function withController( } }), ); - controllerMessenger.registerActionHandler( 'NetworkController:getState', jest.fn().mockReturnValue({ @@ -2770,12 +2787,32 @@ async function withController( }, }), ); + controllerMessenger.registerActionHandler( + 'TransactionController:getNonceLock', + getNonceLock, + ); + controllerMessenger.registerActionHandler( + 'TransactionController:confirmExternalTransaction', + confirmExternalTransaction, + ); + controllerMessenger.registerActionHandler( + 'TransactionController:getTransactions', + getTransactions, + ); + controllerMessenger.registerActionHandler( + 'TransactionController:updateTransaction', + updateTransaction, + ); const messenger = controllerMessenger.getRestricted({ name: 'SmartTransactionsController', allowedActions: [ 'NetworkController:getNetworkClientById', 'NetworkController:getState', + 'TransactionController:getNonceLock', + 'TransactionController:confirmExternalTransaction', + 'TransactionController:getTransactions', + 'TransactionController:updateTransaction', ], allowedEvents: ['NetworkController:stateChange'], }); @@ -2783,12 +2820,6 @@ async function withController( const controller = new SmartTransactionsController({ messenger, clientId: ClientId.Mobile, - getNonceLock: jest.fn().mockResolvedValue({ - nextNonce: 42, - releaseLock: jest.fn(), - }), - confirmExternalTransaction: jest.fn(), - getTransactions: jest.fn(), trackMetaMetricsEvent: trackMetaMetricsEventSpy, getMetaMetricsProps: jest.fn(async () => { return Promise.resolve({ @@ -2798,7 +2829,6 @@ async function withController( }); }), getFeatureFlags: jest.fn(), - updateTransaction: jest.fn(), ...options, }); diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 84ae520d..25a2bec2 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -21,7 +21,10 @@ import type { } from '@metamask/network-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { - TransactionController, + TransactionControllerConfirmExternalTransactionAction, + TransactionControllerGetNonceLockAction, + TransactionControllerGetTransactionsAction, + TransactionControllerUpdateTransactionAction, TransactionMeta, TransactionParams, } from '@metamask/transaction-controller'; @@ -43,7 +46,6 @@ import type { SmartTransaction, SmartTransactionsStatus, UnsignedTransaction, - GetTransactionsOptions, MetaMetricsProps, FeatureFlags, ClientId, @@ -150,7 +152,11 @@ export type SmartTransactionsControllerActions = type AllowedActions = | NetworkControllerGetNetworkClientByIdAction - | NetworkControllerGetStateAction; + | NetworkControllerGetStateAction + | TransactionControllerGetNonceLockAction + | TransactionControllerConfirmExternalTransactionAction + | TransactionControllerGetTransactionsAction + | TransactionControllerUpdateTransactionAction; export type SmartTransactionsControllerStateChangeEvent = ControllerStateChangeEvent< @@ -194,8 +200,6 @@ type SmartTransactionsControllerOptions = { clientId: ClientId; chainId?: Hex; supportedChainIds?: Hex[]; - getNonceLock: TransactionController['getNonceLock']; - confirmExternalTransaction: TransactionController['confirmExternalTransaction']; trackMetaMetricsEvent: ( event: { event: MetaMetricsEventName; @@ -209,10 +213,8 @@ type SmartTransactionsControllerOptions = { ) => void; state?: Partial; messenger: SmartTransactionsControllerMessenger; - getTransactions: (options?: GetTransactionsOptions) => TransactionMeta[]; getMetaMetricsProps: () => Promise; getFeatureFlags: () => FeatureFlags; - updateTransaction: (transaction: TransactionMeta, note: string) => void; trace?: TraceCallback; }; @@ -235,24 +237,14 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo timeoutHandle?: NodeJS.Timeout; - readonly #getNonceLock: SmartTransactionsControllerOptions['getNonceLock']; - #ethQuery: EthQuery | undefined; - #confirmExternalTransaction: SmartTransactionsControllerOptions['confirmExternalTransaction']; - - #getRegularTransactions: ( - options?: GetTransactionsOptions, - ) => TransactionMeta[]; - readonly #trackMetaMetricsEvent: SmartTransactionsControllerOptions['trackMetaMetricsEvent']; readonly #getMetaMetricsProps: () => Promise; #getFeatureFlags: SmartTransactionsControllerOptions['getFeatureFlags']; - #updateTransaction: SmartTransactionsControllerOptions['updateTransaction']; - #trace: TraceCallback; /* istanbul ignore next */ @@ -273,15 +265,11 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo clientId, chainId: InitialChainId = ChainId.mainnet, supportedChainIds = [ChainId.mainnet, ChainId.sepolia], - getNonceLock, - confirmExternalTransaction, trackMetaMetricsEvent, state = {}, messenger, - getTransactions, getMetaMetricsProps, getFeatureFlags, - updateTransaction, trace, }: SmartTransactionsControllerOptions) { super({ @@ -298,14 +286,10 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo this.#chainId = InitialChainId; this.#supportedChainIds = supportedChainIds; this.setIntervalLength(interval); - this.#getNonceLock = getNonceLock; this.#ethQuery = undefined; - this.#confirmExternalTransaction = confirmExternalTransaction; - this.#getRegularTransactions = getTransactions; this.#trackMetaMetricsEvent = trackMetaMetricsEvent; this.#getMetaMetricsProps = getMetaMetricsProps; this.#getFeatureFlags = getFeatureFlags; - this.#updateTransaction = updateTransaction; this.#trace = trace ?? (((_request, fn) => fn?.()) as TraceCallback); this.initializeSmartTransactionsForChainId(); @@ -594,9 +578,8 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo }) ) { markRegularTransactionAsFailed({ + messenger: this.messagingSystem, smartTransaction: nextSmartTransaction, - getRegularTransactions: this.#getRegularTransactions, - updateTransaction: this.#updateTransaction, }); } @@ -660,7 +643,9 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo if (!txHash) { return true; } - const transactions = this.#getRegularTransactions(); + const transactions = this.messagingSystem.call( + 'TransactionController:getTransactions', + ); const foundTransaction = transactions?.find((tx) => { return tx.hash?.toLowerCase() === txHash.toLowerCase(); }); @@ -741,7 +726,8 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo : originalTxMeta; if (this.#doesTransactionNeedConfirmation(txHash)) { - this.#confirmExternalTransaction( + this.messagingSystem.call( + 'TransactionController:confirmExternalTransaction', // TODO: Replace 'as' assertion with correct typing for `txMeta` txMeta as TransactionMeta, transactionReceipt, @@ -836,7 +822,8 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo transaction: UnsignedTransaction, networkClientId: NetworkClientId, ): Promise { - const nonceLock = await this.#getNonceLock( + const nonceLock = await this.messagingSystem.call( + 'TransactionController:getNonceLock', transaction.from, networkClientId, ); @@ -991,7 +978,8 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo // This should only happen for Swaps. Non-swaps transactions should already have a nonce if (requiresNonce) { try { - nonceLock = await this.#getNonceLock( + nonceLock = await this.messagingSystem.call( + 'TransactionController:getNonceLock', txParams.from, selectedNetworkClientId, ); diff --git a/src/index.test.ts b/src/index.test.ts index 6fe46f5e..28bf607d 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -28,15 +28,11 @@ describe('default export', () => { }); const controller = new DefaultExport({ messenger, - getNonceLock: jest.fn(), - confirmExternalTransaction: jest.fn(), - getTransactions: jest.fn(), trackMetaMetricsEvent: jest.fn(), getMetaMetricsProps: jest.fn(async () => { return Promise.resolve({}); }), getFeatureFlags: jest.fn(), - updateTransaction: jest.fn(), clientId: ClientId.Extension, }); expect(controller).toBeInstanceOf(SmartTransactionsController); diff --git a/src/types.ts b/src/types.ts index dceecf55..c3f1180f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,5 +1,4 @@ import type { NetworkClientId } from '@metamask/network-controller'; -import type { TransactionMeta } from '@metamask/transaction-controller'; /** API */ export enum APIType { @@ -129,13 +128,6 @@ export type SignedCanceledTransaction = any; export type Hex = `0x${string}`; -export type GetTransactionsOptions = { - searchCriteria?: any; - initialList?: TransactionMeta[]; - filterToCurrentNetwork?: boolean; - limit?: number; -}; - export type MetaMetricsProps = { accountHardwareType?: string; accountType?: string; diff --git a/src/utils.test.ts b/src/utils.test.ts index 42a8aa46..9ba8e132 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,6 +1,11 @@ import { arrayify, hexlify } from '@ethersproject/bytes'; import { keccak256 } from '@ethersproject/keccak256'; +import { Messenger } from '@metamask/base-controller'; import { ChainId, NetworkType } from '@metamask/controller-utils'; +import type { + TransactionControllerGetTransactionsAction, + TransactionControllerUpdateTransactionAction, +} from '@metamask/transaction-controller'; import { type TransactionMeta, TransactionStatus, @@ -560,13 +565,33 @@ describe('src/utils.js', () => { it('updates transaction with failed status and error message', () => { const updateTransactionMock = jest.fn(); + const messenger = new Messenger< + | TransactionControllerGetTransactionsAction + | TransactionControllerUpdateTransactionAction, + never + >(); + const restrictedMessenger = messenger.getRestricted({ + name: 'SmartTransactionController', + allowedActions: [ + 'TransactionController:getTransactions', + 'TransactionController:updateTransaction', + ], + allowedEvents: [], + }); + messenger.registerActionHandler( + 'TransactionController:getTransactions', + () => [mockTransaction], + ); + messenger.registerActionHandler( + 'TransactionController:updateTransaction', + updateTransactionMock, + ); utils.markRegularTransactionAsFailed({ + messenger: restrictedMessenger, smartTransaction: createSmartTransaction( SmartTransactionStatuses.CANCELLED, ), - getRegularTransactions: () => [mockTransaction], - updateTransaction: updateTransactionMock, }); expect(updateTransactionMock).toHaveBeenCalledWith( @@ -583,16 +608,36 @@ describe('src/utils.js', () => { }); it('throws error if original transaction cannot be found', () => { + const getTransactionsMock = jest.fn(() => []); const updateTransactionMock = jest.fn(); - const getRegularTransactionsMock = jest.fn(() => []); + const messenger = new Messenger< + | TransactionControllerGetTransactionsAction + | TransactionControllerUpdateTransactionAction, + never + >(); + const restrictedMessenger = messenger.getRestricted({ + name: 'SmartTransactionController', + allowedActions: [ + 'TransactionController:getTransactions', + 'TransactionController:updateTransaction', + ], + allowedEvents: [], + }); + messenger.registerActionHandler( + 'TransactionController:getTransactions', + getTransactionsMock, + ); + messenger.registerActionHandler( + 'TransactionController:updateTransaction', + updateTransactionMock, + ); expect(() => utils.markRegularTransactionAsFailed({ + messenger: restrictedMessenger, smartTransaction: createSmartTransaction( SmartTransactionStatuses.CANCELLED, ), - getRegularTransactions: getRegularTransactionsMock, - updateTransaction: updateTransactionMock, }), ).toThrow('Cannot find regular transaction to mark it as failed'); @@ -600,7 +645,6 @@ describe('src/utils.js', () => { }); it('does not update transaction if status is already failed', () => { - const updateTransactionMock = jest.fn(); const failedTransaction = { ...mockTransaction, status: TransactionStatus.failed, @@ -609,13 +653,34 @@ describe('src/utils.js', () => { message: 'Smart transaction failed', }, }; + const updateTransactionMock = jest.fn(); + const messenger = new Messenger< + | TransactionControllerGetTransactionsAction + | TransactionControllerUpdateTransactionAction, + never + >(); + const restrictedMessenger = messenger.getRestricted({ + name: 'SmartTransactionController', + allowedActions: [ + 'TransactionController:getTransactions', + 'TransactionController:updateTransaction', + ], + allowedEvents: [], + }); + messenger.registerActionHandler( + 'TransactionController:getTransactions', + () => [failedTransaction], + ); + messenger.registerActionHandler( + 'TransactionController:updateTransaction', + updateTransactionMock, + ); utils.markRegularTransactionAsFailed({ + messenger: restrictedMessenger, smartTransaction: createSmartTransaction( SmartTransactionStatuses.CANCELLED, ), - getRegularTransactions: () => [failedTransaction], - updateTransaction: updateTransactionMock, }); expect(updateTransactionMock).not.toHaveBeenCalled(); diff --git a/src/utils.ts b/src/utils.ts index cabdc178..7699eacb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,7 +1,12 @@ import { arrayify, hexlify } from '@ethersproject/bytes'; import { keccak256 } from '@ethersproject/keccak256'; import { parse } from '@ethersproject/transactions'; -import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { RestrictedMessenger } from '@metamask/base-controller'; +import type { + TransactionControllerGetTransactionsAction, + TransactionControllerUpdateTransactionAction, + TransactionMeta, +} from '@metamask/transaction-controller'; import { TransactionStatus } from '@metamask/transaction-controller'; import { BigNumber } from 'bignumber.js'; import jsonDiffer from 'fast-json-patch'; @@ -27,6 +32,18 @@ import { ClientId, } from './types'; +export type MarkRegularTransactionsAsFailedMessenger = RestrictedMessenger< + string, + | TransactionControllerGetTransactionsAction + | TransactionControllerUpdateTransactionAction, + never, + ( + | TransactionControllerGetTransactionsAction + | TransactionControllerUpdateTransactionAction + )['type'], + never +>; + export function isSmartTransactionPending(smartTransaction: SmartTransaction) { return smartTransaction.status === SmartTransactionStatuses.PENDING; } @@ -320,18 +337,16 @@ export const shouldMarkRegularTransactionAsFailed = ({ }; export const markRegularTransactionAsFailed = ({ + messenger, smartTransaction, - getRegularTransactions, - updateTransaction, }: { + messenger: MarkRegularTransactionsAsFailedMessenger; smartTransaction: SmartTransaction; - getRegularTransactions: () => TransactionMeta[]; - updateTransaction: (transaction: TransactionMeta, note: string) => void; }) => { const { transactionId, status } = smartTransaction; - const originalTransaction = getRegularTransactions().find( - (transaction) => transaction.id === transactionId, - ); + const originalTransaction = messenger + .call('TransactionController:getTransactions') + .find((transaction) => transaction.id === transactionId); if (!originalTransaction) { throw new Error('Cannot find regular transaction to mark it as failed'); } @@ -346,5 +361,9 @@ export const markRegularTransactionAsFailed = ({ message: `Smart transaction failed with status: ${status}`, }, }; - updateTransaction(updatedTransaction, `Smart transaction status: ${status}`); + messenger.call( + 'TransactionController:updateTransaction', + updatedTransaction, + `Smart transaction status: ${status}`, + ); }; diff --git a/yarn.lock b/yarn.lock index f880f2d6..642ebf2b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1424,24 +1424,24 @@ __metadata: languageName: node linkType: hard -"@metamask/base-controller@npm:^8.0.1, @metamask/base-controller@npm:^8.3.0": - version: 8.3.0 - resolution: "@metamask/base-controller@npm:8.3.0" +"@metamask/base-controller@npm:^8.0.1, @metamask/base-controller@npm:^8.3.0, @metamask/base-controller@npm:^8.4.0": + version: 8.4.0 + resolution: "@metamask/base-controller@npm:8.4.0" dependencies: - "@metamask/messenger": ^0.2.0 - "@metamask/utils": ^11.4.2 + "@metamask/messenger": ^0.3.0 + "@metamask/utils": ^11.8.0 immer: ^9.0.6 - checksum: 957528b7f52d16c401bc2c391b1723efdbeaa950c25c1cbcde7372309cdc1028a30efcef5c121cf1dc143f4da9539a4f5ec1470c6e5e0b4d7905ac6a9c780b71 + checksum: 170838507ce30cd1820fdcd0be0a11b489bd2421305b4a6a87bf04cbe6a754af9a65628522cb916ed049dd4306be79e5a33e6e1b671a2b5ad778a66f7aa20967 languageName: node linkType: hard -"@metamask/controller-utils@npm:^11.0.0, @metamask/controller-utils@npm:^11.10.0, @metamask/controller-utils@npm:^11.12.0, @metamask/controller-utils@npm:^11.4.2, @metamask/controller-utils@npm:^11.4.3": - version: 11.12.0 - resolution: "@metamask/controller-utils@npm:11.12.0" +"@metamask/controller-utils@npm:^11.0.0, @metamask/controller-utils@npm:^11.10.0, @metamask/controller-utils@npm:^11.14.0, @metamask/controller-utils@npm:^11.4.2, @metamask/controller-utils@npm:^11.4.3": + version: 11.14.0 + resolution: "@metamask/controller-utils@npm:11.14.0" dependencies: "@metamask/eth-query": ^4.0.0 "@metamask/ethjs-unit": ^0.3.0 - "@metamask/utils": ^11.4.2 + "@metamask/utils": ^11.8.0 "@spruceid/siwe-parser": 2.1.0 "@types/bn.js": ^5.1.5 bignumber.js: ^9.1.2 @@ -1452,7 +1452,7 @@ __metadata: lodash: ^4.17.21 peerDependencies: "@babel/runtime": ^7.0.0 - checksum: a96dea30d56676c3070d118d4e130f6f97c2ddeac1161d49a99f114af7ca6e9b87f955ea5a6a17c72410243cf9eafed5748474144013b548955b9edc7e97b409 + checksum: 727a627e96645a1b17f629bcb640ab5e26f656121d38e34e1ff9ac357f47f3b9deefb17f7e4a6a639594e5fa7f0d1ef3764fd54908f24c3d6e6b4a89758815e6 languageName: node linkType: hard @@ -1708,10 +1708,10 @@ __metadata: languageName: node linkType: hard -"@metamask/messenger@npm:^0.2.0": - version: 0.2.0 - resolution: "@metamask/messenger@npm:0.2.0" - checksum: fad113b3bdeda5c481c1bd121b9a6ca8d8f06dab3031550c5bc0da72844876dc3fe27241fe604c5b70e6b553be4bc6a63f07c48c6215ca60d554d9dafd7cc246 +"@metamask/messenger@npm:^0.3.0": + version: 0.3.0 + resolution: "@metamask/messenger@npm:0.3.0" + checksum: 72050d7ba672bc82319a6b6ff126c52d372418a9049555a1b94f520e664b6e8037e44203f2ecffb33f8de8e3b874174ad40da306fb8cb17decccaeb50f36f180 languageName: node linkType: hard @@ -1847,7 +1847,7 @@ __metadata: "@metamask/json-rpc-engine": ^10.0.1 "@metamask/network-controller": ^24.0.0 "@metamask/polling-controller": ^14.0.0 - "@metamask/transaction-controller": ^60.3.0 + "@metamask/transaction-controller": ^60.4.0 "@ts-bridge/cli": ^0.6.3 "@types/jest": ^26.0.24 "@types/lodash": ^4.14.194 @@ -1905,9 +1905,9 @@ __metadata: languageName: node linkType: hard -"@metamask/transaction-controller@npm:^60.3.0": - version: 60.3.0 - resolution: "@metamask/transaction-controller@npm:60.3.0" +"@metamask/transaction-controller@npm:^60.4.0": + version: 60.4.0 + resolution: "@metamask/transaction-controller@npm:60.4.0" dependencies: "@ethereumjs/common": ^4.4.0 "@ethereumjs/tx": ^5.4.0 @@ -1916,13 +1916,13 @@ __metadata: "@ethersproject/contracts": ^5.7.0 "@ethersproject/providers": ^5.7.0 "@ethersproject/wallet": ^5.7.0 - "@metamask/base-controller": ^8.3.0 - "@metamask/controller-utils": ^11.12.0 + "@metamask/base-controller": ^8.4.0 + "@metamask/controller-utils": ^11.14.0 "@metamask/eth-query": ^4.0.0 "@metamask/metamask-eth-abis": ^3.1.1 "@metamask/nonce-tracker": ^6.0.0 "@metamask/rpc-errors": ^7.0.2 - "@metamask/utils": ^11.4.2 + "@metamask/utils": ^11.8.0 async-mutex: ^0.5.0 bn.js: ^5.2.1 eth-method-registry: ^4.0.0 @@ -1937,7 +1937,7 @@ __metadata: "@metamask/gas-fee-controller": ^24.0.0 "@metamask/network-controller": ^24.0.0 "@metamask/remote-feature-flag-controller": ^1.5.0 - checksum: 910475ae2a20279d1743a6331b178a22a5416ecabcf4b9391defb2c8753b6963e1e34e55f6fd50bc2740f468ebc636e864722d8d7607d18b750bb1c2edb844a2 + checksum: 1e43062f640bd797971a141081117ac5e4044f9d3fd0adb6592b7dbe5679e79b7bee0064dc17865609d6f1652e6c49f82cc58b15af2498355ffcbfcff31154fc languageName: node linkType: hard @@ -1958,21 +1958,22 @@ __metadata: languageName: node linkType: hard -"@metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.1.0, @metamask/utils@npm:^11.2.0, @metamask/utils@npm:^11.4.2": - version: 11.4.2 - resolution: "@metamask/utils@npm:11.4.2" +"@metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.1.0, @metamask/utils@npm:^11.2.0, @metamask/utils@npm:^11.4.2, @metamask/utils@npm:^11.8.0": + version: 11.8.0 + resolution: "@metamask/utils@npm:11.8.0" dependencies: "@ethereumjs/tx": ^4.2.0 "@metamask/superstruct": ^3.1.0 "@noble/hashes": ^1.3.1 "@scure/base": ^1.1.3 "@types/debug": ^4.1.7 + "@types/lodash": ^4.17.20 debug: ^4.3.4 - lodash.memoize: ^4.1.2 + lodash: ^4.17.21 pony-cause: ^2.1.10 semver: ^7.5.4 uuid: ^9.0.1 - checksum: 11061a93f49684563a14caaaab2d8dbb969c907dbc24358cf188dd10ec00ac91e5d04369ef605e9d78e75f8ad53d9a0fbdb65f2325b12ef6c8db85bb46160dff + checksum: 61f0eb5f9066ea7f59637389910698d78e48fc810669d156dc1cfea879699cbc12644933ee04929dfc14bc100440a241003a1b68de9e0c41f292c4f290af1ae6 languageName: node linkType: hard @@ -2407,10 +2408,10 @@ __metadata: languageName: node linkType: hard -"@types/lodash@npm:^4.14.194": - version: 4.14.194 - resolution: "@types/lodash@npm:4.14.194" - checksum: 113f34831c461469d91feca2dde737f88487732898b4d25e9eb23b087bb193985f864d1e1e0f3b777edc5022e460443588b6000a3b2348c966f72d17eedc35ea +"@types/lodash@npm:^4.14.194, @types/lodash@npm:^4.17.20": + version: 4.17.20 + resolution: "@types/lodash@npm:4.17.20" + checksum: dc7bb4653514dd91117a4c4cec2c37e2b5a163d7643445e4757d76a360fabe064422ec7a42dde7450c5e7e0e7e678d5e6eae6d2a919abcddf581d81e63e63839 languageName: node linkType: hard @@ -6258,7 +6259,7 @@ __metadata: languageName: node linkType: hard -"lodash.memoize@npm:4.x, lodash.memoize@npm:^4.1.2": +"lodash.memoize@npm:4.x": version: 4.1.2 resolution: "lodash.memoize@npm:4.1.2" checksum: 9ff3942feeccffa4f1fafa88d32f0d24fdc62fd15ded5a74a5f950ff5f0c6f61916157246744c620173dddf38d37095a92327d5fd3861e2063e736a5c207d089 From 6e996522a8034dc9243ff11b55c562abcfb81d6e Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 17 Sep 2025 11:21:54 -0600 Subject: [PATCH 2/5] Move changelog entries to Changed; consolidate --- CHANGELOG.md | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 758c36b4..3c16ea6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,20 +16,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** Disallow subpath exports ([#469](https://github.com/MetaMask/smart-transactions-controller/pull/469)) - **BREAKING:** Upgrade peer dependency `@metamask/transaction-controller` from `^58.0.0` to `^60.3.0` +- **BREAKING:** Remove `getNonceLock`, `confirmExternalTransaction`, `getTransactions`, and `updateTransaction` constructor option in favor of messenger actions ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) + - The messenger is now used to access TransactionController; you must add these actions to the SmartTransactionController messenger allowlist: + - `TransactionController:getNonceLock` + - `TransactionController:confirmExternalTransaction` + - `TransactionController:getTransactions` + - `TransactionController:updateTransaction` - Upgrade `@metamask/base-controller` from `^7.0.1` to `^8.3.0` ([#529](https://github.com/MetaMask/smart-transactions-controller/pull/529)) - Upgrade `@metamask/polling-controller` from `^12.0.0` to `^14.0.0` ([#529](https://github.com/MetaMask/smart-transactions-controller/pull/529)) -### Removed - -- Remove `getNonceLock` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) - - The messenger is now used to access TransactionController. Please add the `TransactionController:getNonce` action to the messenger's allowlist. -- Remove `confirmExternalTransaction` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) - - The messenger is now used to access TransactionController. Please add the `TransactionController:confirmExternalTransaction` action to the messenger's allowlist. -- Remove `getTransactions` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) - - The messenger is now used to access TransactionController. Please add the `TransactionController:getTransactions` action to the messenger's allowlist. -- Remove `updateTransaction` constructor option ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) - - The messenger is now used to access TransactionController. Please add the `TransactionController:updateTransaction` action to the messenger's allowlist. - ## [18.1.0] ### Added From 6250974352fc7742212940a743f77a7280a1935b Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 17 Sep 2025 12:05:06 -0600 Subject: [PATCH 3/5] Don't have markRegularTransactionsAsFailed take a messenger --- src/SmartTransactionsController.ts | 9 +++- src/utils.test.ts | 81 +++--------------------------- src/utils.ts | 31 +++--------- 3 files changed, 24 insertions(+), 97 deletions(-) diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index 25a2bec2..336d6eb1 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -578,8 +578,15 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo }) ) { markRegularTransactionAsFailed({ - messenger: this.messagingSystem, smartTransaction: nextSmartTransaction, + getRegularTransactions: () => + this.messagingSystem.call('TransactionController:getTransactions'), + updateTransaction: (transactionMeta: TransactionMeta, note: string) => + this.messagingSystem.call( + 'TransactionController:updateTransaction', + transactionMeta, + note, + ), }); } diff --git a/src/utils.test.ts b/src/utils.test.ts index 9ba8e132..42a8aa46 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,11 +1,6 @@ import { arrayify, hexlify } from '@ethersproject/bytes'; import { keccak256 } from '@ethersproject/keccak256'; -import { Messenger } from '@metamask/base-controller'; import { ChainId, NetworkType } from '@metamask/controller-utils'; -import type { - TransactionControllerGetTransactionsAction, - TransactionControllerUpdateTransactionAction, -} from '@metamask/transaction-controller'; import { type TransactionMeta, TransactionStatus, @@ -565,33 +560,13 @@ describe('src/utils.js', () => { it('updates transaction with failed status and error message', () => { const updateTransactionMock = jest.fn(); - const messenger = new Messenger< - | TransactionControllerGetTransactionsAction - | TransactionControllerUpdateTransactionAction, - never - >(); - const restrictedMessenger = messenger.getRestricted({ - name: 'SmartTransactionController', - allowedActions: [ - 'TransactionController:getTransactions', - 'TransactionController:updateTransaction', - ], - allowedEvents: [], - }); - messenger.registerActionHandler( - 'TransactionController:getTransactions', - () => [mockTransaction], - ); - messenger.registerActionHandler( - 'TransactionController:updateTransaction', - updateTransactionMock, - ); utils.markRegularTransactionAsFailed({ - messenger: restrictedMessenger, smartTransaction: createSmartTransaction( SmartTransactionStatuses.CANCELLED, ), + getRegularTransactions: () => [mockTransaction], + updateTransaction: updateTransactionMock, }); expect(updateTransactionMock).toHaveBeenCalledWith( @@ -608,36 +583,16 @@ describe('src/utils.js', () => { }); it('throws error if original transaction cannot be found', () => { - const getTransactionsMock = jest.fn(() => []); const updateTransactionMock = jest.fn(); - const messenger = new Messenger< - | TransactionControllerGetTransactionsAction - | TransactionControllerUpdateTransactionAction, - never - >(); - const restrictedMessenger = messenger.getRestricted({ - name: 'SmartTransactionController', - allowedActions: [ - 'TransactionController:getTransactions', - 'TransactionController:updateTransaction', - ], - allowedEvents: [], - }); - messenger.registerActionHandler( - 'TransactionController:getTransactions', - getTransactionsMock, - ); - messenger.registerActionHandler( - 'TransactionController:updateTransaction', - updateTransactionMock, - ); + const getRegularTransactionsMock = jest.fn(() => []); expect(() => utils.markRegularTransactionAsFailed({ - messenger: restrictedMessenger, smartTransaction: createSmartTransaction( SmartTransactionStatuses.CANCELLED, ), + getRegularTransactions: getRegularTransactionsMock, + updateTransaction: updateTransactionMock, }), ).toThrow('Cannot find regular transaction to mark it as failed'); @@ -645,6 +600,7 @@ describe('src/utils.js', () => { }); it('does not update transaction if status is already failed', () => { + const updateTransactionMock = jest.fn(); const failedTransaction = { ...mockTransaction, status: TransactionStatus.failed, @@ -653,34 +609,13 @@ describe('src/utils.js', () => { message: 'Smart transaction failed', }, }; - const updateTransactionMock = jest.fn(); - const messenger = new Messenger< - | TransactionControllerGetTransactionsAction - | TransactionControllerUpdateTransactionAction, - never - >(); - const restrictedMessenger = messenger.getRestricted({ - name: 'SmartTransactionController', - allowedActions: [ - 'TransactionController:getTransactions', - 'TransactionController:updateTransaction', - ], - allowedEvents: [], - }); - messenger.registerActionHandler( - 'TransactionController:getTransactions', - () => [failedTransaction], - ); - messenger.registerActionHandler( - 'TransactionController:updateTransaction', - updateTransactionMock, - ); utils.markRegularTransactionAsFailed({ - messenger: restrictedMessenger, smartTransaction: createSmartTransaction( SmartTransactionStatuses.CANCELLED, ), + getRegularTransactions: () => [failedTransaction], + updateTransaction: updateTransactionMock, }); expect(updateTransactionMock).not.toHaveBeenCalled(); diff --git a/src/utils.ts b/src/utils.ts index 7699eacb..755a64eb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,7 +1,6 @@ import { arrayify, hexlify } from '@ethersproject/bytes'; import { keccak256 } from '@ethersproject/keccak256'; import { parse } from '@ethersproject/transactions'; -import type { RestrictedMessenger } from '@metamask/base-controller'; import type { TransactionControllerGetTransactionsAction, TransactionControllerUpdateTransactionAction, @@ -32,18 +31,6 @@ import { ClientId, } from './types'; -export type MarkRegularTransactionsAsFailedMessenger = RestrictedMessenger< - string, - | TransactionControllerGetTransactionsAction - | TransactionControllerUpdateTransactionAction, - never, - ( - | TransactionControllerGetTransactionsAction - | TransactionControllerUpdateTransactionAction - )['type'], - never ->; - export function isSmartTransactionPending(smartTransaction: SmartTransaction) { return smartTransaction.status === SmartTransactionStatuses.PENDING; } @@ -337,16 +324,18 @@ export const shouldMarkRegularTransactionAsFailed = ({ }; export const markRegularTransactionAsFailed = ({ - messenger, smartTransaction, + getRegularTransactions, + updateTransaction, }: { - messenger: MarkRegularTransactionsAsFailedMessenger; smartTransaction: SmartTransaction; + getRegularTransactions: TransactionControllerGetTransactionsAction['handler']; + updateTransaction: TransactionControllerUpdateTransactionAction['handler']; }) => { const { transactionId, status } = smartTransaction; - const originalTransaction = messenger - .call('TransactionController:getTransactions') - .find((transaction) => transaction.id === transactionId); + const originalTransaction = getRegularTransactions().find( + (transaction) => transaction.id === transactionId, + ); if (!originalTransaction) { throw new Error('Cannot find regular transaction to mark it as failed'); } @@ -361,9 +350,5 @@ export const markRegularTransactionAsFailed = ({ message: `Smart transaction failed with status: ${status}`, }, }; - messenger.call( - 'TransactionController:updateTransaction', - updatedTransaction, - `Smart transaction status: ${status}`, - ); + updateTransaction(updatedTransaction, `Smart transaction status: ${status}`); }; From d804f8bf63164f51d94d0d9d01a537dcf2238498 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 17 Sep 2025 12:07:22 -0600 Subject: [PATCH 4/5] Bump peer dep again --- CHANGELOG.md | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c16ea6f..a1f4177c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** Disallow subpath exports ([#469](https://github.com/MetaMask/smart-transactions-controller/pull/469)) -- **BREAKING:** Upgrade peer dependency `@metamask/transaction-controller` from `^58.0.0` to `^60.3.0` +- **BREAKING:** Upgrade peer dependency `@metamask/transaction-controller` from `^58.0.0` to `^60.4.0` ([#532](https://github.com/MetaMask/smart-transactions-controller/pull/532), [#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) - **BREAKING:** Remove `getNonceLock`, `confirmExternalTransaction`, `getTransactions`, and `updateTransaction` constructor option in favor of messenger actions ([#534](https://github.com/MetaMask/smart-transactions-controller/pull/534)) - The messenger is now used to access TransactionController; you must add these actions to the SmartTransactionController messenger allowlist: - `TransactionController:getNonceLock` diff --git a/package.json b/package.json index 5fa6c665..9ec9f86d 100644 --- a/package.json +++ b/package.json @@ -93,7 +93,7 @@ }, "peerDependencies": { "@metamask/network-controller": "^24.0.0", - "@metamask/transaction-controller": "^60.0.0" + "@metamask/transaction-controller": "^60.4.0" }, "peerDependenciesMeta": { "@metamask/accounts-controller": { From 55aee3ceef30fab717f3e2e205e58172891740e8 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 17 Sep 2025 12:09:56 -0600 Subject: [PATCH 5/5] Update lockfile --- yarn.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 642ebf2b..29ea3c88 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1876,7 +1876,7 @@ __metadata: typescript: ~4.8.4 peerDependencies: "@metamask/network-controller": ^24.0.0 - "@metamask/transaction-controller": ^60.0.0 + "@metamask/transaction-controller": ^60.4.0 peerDependenciesMeta: "@metamask/accounts-controller": optional: true