diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index 1a339c1e35..7b44033691 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 91.89, - functions: 98.86, - lines: 98.96, - statements: 98.97, + branches: 91.79, + functions: 98.37, + lines: 98.83, + statements: 98.84, }, }, diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index 1078b6c197..599ceb50d0 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -60,6 +60,7 @@ "@types/jest": "^27.4.1", "@types/node": "^16.18.54", "deepmerge": "^4.2.2", + "immer": "^9.0.6", "jest": "^27.5.1", "nock": "^13.3.1", "sinon": "^9.2.4", diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 681dfe467b..7461bef811 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1,9 +1,11 @@ /* eslint-disable jest/expect-expect */ +import type { TypedTransaction } from '@ethereumjs/tx'; import { TransactionFactory } from '@ethereumjs/tx'; import type { - AcceptResultCallbacks, + AddApprovalRequest, AddResult, } from '@metamask/approval-controller'; +import { ControllerMessenger } from '@metamask/base-controller'; import { ChainId, NetworkType, @@ -16,15 +18,16 @@ import EthQuery from '@metamask/eth-query'; import HttpProvider from '@metamask/ethjs-provider-http'; import type { BlockTracker, - NetworkControllerFindNetworkClientIdByChainIdAction, - NetworkControllerGetNetworkClientByIdAction, + NetworkController, NetworkState, Provider, } from '@metamask/network-controller'; import { NetworkClientType, NetworkStatus } from '@metamask/network-controller'; import { errorCodes, providerErrors, rpcErrors } from '@metamask/rpc-errors'; import { createDeferredPromise } from '@metamask/utils'; +import assert from 'assert'; import * as NonceTrackerPackage from 'nonce-tracker'; +import * as uuidModule from 'uuid'; import { FakeBlockTracker } from '../../../tests/fake-block-tracker'; import { flushPromises } from '../../../tests/helpers'; @@ -36,9 +39,10 @@ import { IncomingTransactionHelper } from './helpers/IncomingTransactionHelper'; import { MultichainTrackingHelper } from './helpers/MultichainTrackingHelper'; import { PendingTransactionTracker } from './helpers/PendingTransactionTracker'; import type { - TransactionControllerMessenger, - TransactionConfig, - TransactionState, + AllowedActions, + AllowedEvents, + TransactionControllerActions, + TransactionControllerEvents, } from './TransactionController'; import { TransactionController } from './TransactionController'; import type { @@ -56,9 +60,21 @@ import { updateSwapsTransaction, } from './utils/swaps'; +type UnrestrictedControllerMessenger = ControllerMessenger< + TransactionControllerActions | AllowedActions, + TransactionControllerEvents | AllowedEvents +>; + +type NetworkClient = ReturnType; + +type NetworkClientConfiguration = Pick< + NetworkClient['configuration'], + 'chainId' +>; + const MOCK_V1_UUID = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'; -const v1Stub = jest.fn().mockImplementation(() => MOCK_V1_UUID); +jest.mock('@metamask/eth-query'); jest.mock('./gas-flows/DefaultGasFeeFlow'); jest.mock('./gas-flows/LineaGasFeeFlow'); jest.mock('./helpers/GasFeePoller'); @@ -69,12 +85,7 @@ jest.mock('./utils/gas'); jest.mock('./utils/gas-fees'); jest.mock('./utils/swaps'); -jest.mock('uuid', () => { - return { - ...jest.requireActual('uuid'), - v1: () => v1Stub(), - }; -}); +jest.mock('uuid'); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -84,129 +95,130 @@ const mockFlags: { [key: string]: any } = { getBlockByNumberValue: null, }; -const ethQueryMockResults = { - sendRawTransaction: 'mockSendRawTransactionResult', -}; -const mockSendRawTransaction = jest - .fn() - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .mockImplementation((_transaction: any, callback: any) => { - callback(undefined, ethQueryMockResults.sendRawTransaction); - }); -jest.mock('@metamask/eth-query', () => - jest.fn().mockImplementation(() => { - return { +/** + * Constructs an EthQuery for use in tests, with various methods replaced with + * fake implementations. + * + * @returns The mock EthQuery instance. + */ +function buildMockEthQuery(): EthQuery { + return { + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + estimateGas: (_transaction: any, callback: any) => { + if (mockFlags.estimateGasError) { + callback(new Error(mockFlags.estimateGasError)); + return; + } + + if (mockFlags.estimateGasValue) { + callback(undefined, mockFlags.estimateGasValue); + return; + } + callback(undefined, '0x0'); + }, + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + gasPrice: (callback: any) => { + callback(undefined, '0x0'); + }, + getBlockByNumber: ( // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - estimateGas: (_transaction: any, callback: any) => { - if (mockFlags.estimateGasError) { - callback(new Error(mockFlags.estimateGasError)); - return; - } - - if (mockFlags.estimateGasValue) { - callback(undefined, mockFlags.estimateGasValue); - return; - } - callback(undefined, '0x0'); - }, + _blocknumber: any, + _fetchTxs: boolean, // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - gasPrice: (callback: any) => { - callback(undefined, '0x0'); - }, - getBlockByNumber: ( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - _blocknumber: any, - _fetchTxs: boolean, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - callback: any, - ) => { - if (mockFlags.getBlockByNumberValue) { - callback(undefined, { gasLimit: '0x12a05f200' }); - return; - } - callback(undefined, { gasLimit: '0x0' }); - }, + callback: any, + ) => { + if (mockFlags.getBlockByNumberValue) { + callback(undefined, { gasLimit: '0x12a05f200' }); + return; + } + callback(undefined, { gasLimit: '0x0' }); + }, + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getCode: (_to: any, callback: any) => { + callback(undefined, '0x0'); + }, + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getTransactionByHash: (_hash: string, callback: any) => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode: (_to: any, callback: any) => { - callback(undefined, '0x0'); - }, + const txs: any = [ + { blockNumber: '0x1', hash: '1337' }, + { blockNumber: null, hash: '1338' }, + ]; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - getTransactionByHash: (_hash: string, callback: any) => { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const txs: any = [ - { blockNumber: '0x1', hash: '1337' }, - { blockNumber: null, hash: '1338' }, - ]; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const tx: any = txs.find((element: any) => element.hash === _hash); - callback(undefined, tx); - }, + const tx: any = txs.find((element: any) => element.hash === _hash); + callback(undefined, tx); + }, + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getTransactionCount: (_from: any, _to: any, callback: any) => { + callback(undefined, '0x0'); + }, + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sendRawTransaction: (_transaction: unknown, callback: any) => { + callback(undefined, 'somehash'); + }, + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getTransactionReceipt: (_hash: any, callback: any) => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - getTransactionCount: (_from: any, _to: any, callback: any) => { - callback(undefined, '0x0'); - }, - sendRawTransaction: mockSendRawTransaction, + const txs: any = [ + { + blockHash: '1337', + gasUsed: '0x5208', + hash: '1337', + status: '0x1', + transactionIndex: 1337, + }, + { + gasUsed: '0x1108', + hash: '1111', + status: '0x0', + transactionIndex: 1111, + }, + ]; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - getTransactionReceipt: (_hash: any, callback: any) => { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const txs: any = [ - { - blockHash: '1337', - gasUsed: '0x5208', - hash: '1337', - status: '0x1', - transactionIndex: 1337, - }, - { - gasUsed: '0x1108', - hash: '1111', - status: '0x0', - transactionIndex: 1111, - }, - ]; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const tx: any = txs.find((element: any) => element.hash === _hash); - callback(undefined, tx); - }, + const tx: any = txs.find((element: any) => element.hash === _hash); + callback(undefined, tx); + }, + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getBlockByHash: (_blockHash: any, callback: any) => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - getBlockByHash: (_blockHash: any, callback: any) => { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const blocks: any = [ - { - baseFeePerGas: '0x14', - hash: '1337', - number: '0x1', - timestamp: '628dc0c8', - }, - { hash: '1338', number: '0x2' }, - ]; + const blocks: any = [ + { + baseFeePerGas: '0x14', + hash: '1337', + number: '0x1', + timestamp: '628dc0c8', + }, + { hash: '1338', number: '0x2' }, + ]; + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const block: any = blocks.find( // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - const block: any = blocks.find( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (element: any) => element.hash === _blockHash, - ); - callback(undefined, block); - }, - }; - }), -); + (element: any) => element.hash === _blockHash, + ); + callback(undefined, block); + }, + sendAsync: () => { + // do nothing + }, + }; +} /** * Builds a mock block tracker with a canned block number that can be used in @@ -222,140 +234,30 @@ function buildMockBlockTracker(latestBlockNumber: string): BlockTracker { return fakeBlockTracker; } -/** - * Create an object containing mock result callbacks to be used when testing the approval process. - * - * @returns The mock result callbacks. - */ -function buildMockResultCallbacks(): AcceptResultCallbacks { - return { - success: jest.fn(), - error: jest.fn(), - }; -} - -/** - * @type AddRequestOptions - * @property approved - Whether transactions should immediately be approved or rejected. - * @property delay - Whether to delay approval or rejection until the returned functions are called. - * @property resultCallbacks - The result callbacks to return when a request is approved. - */ -type AddRequestOptions = { - approved?: boolean; - delay?: boolean; - resultCallbacks?: AcceptResultCallbacks; -}; - -/** - * Create a mock controller messenger. - * - * @param opts - Options to customize the mock messenger. - * @param opts.addRequest - Options for ApprovalController.addRequest mock. - * @param opts.getNetworkClientById - The function to use as the NetworkController:getNetworkClientById mock. - * @param opts.findNetworkClientIdByChainId - The function to use as the NetworkController:findNetworkClientIdByChainId mock. - * @returns The mock controller messenger. - */ -// -function buildMockMessenger({ - addRequest: { approved, delay, resultCallbacks }, - getNetworkClientById, - findNetworkClientIdByChainId, -}: { - addRequest: AddRequestOptions; - getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; - findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; -}): { - messenger: TransactionControllerMessenger; - approve: () => void; - reject: (reason: unknown) => void; -} { - let approve, reject; - let promise: Promise; - - if (delay) { - promise = new Promise((res, rej) => { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - approve = (value?: any) => res({ resultCallbacks, value }); - reject = rej; - }); - } - - const mockSubscribe = jest.fn(); - mockSubscribe.mockImplementation((_type, handler) => { - setTimeout(() => { - handler({}, [ - { - op: 'add', - path: ['networkConfigurations', 'foo'], - value: 'foo', - }, - ]); - }, 0); - }); - - const messenger = { - subscribe: mockSubscribe, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - call: jest.fn().mockImplementation((actionType: string, ...args: any[]) => { - switch (actionType) { - case 'ApprovalController:addRequest': - if (approved) { - return Promise.resolve({ resultCallbacks }); - } - - if (delay) { - return promise; - } - - // eslint-disable-next-line prefer-promise-reject-errors - return Promise.reject({ - code: errorCodes.provider.userRejectedRequest, - }); - case 'NetworkController:getNetworkClientById': - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return (getNetworkClientById as any)(...args); - case 'NetworkController:findNetworkClientIdByChainId': - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return (findNetworkClientIdByChainId as any)(...args); - default: - throw new Error( - `A handler for ${actionType} has not been registered`, - ); - } - }), - } as unknown as TransactionControllerMessenger; - - return { - messenger, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - approve: approve as unknown as (value?: any) => void, - reject: reject as unknown as (reason: unknown) => void, - }; -} - /** * Wait for the controller to emit a transaction finished event. * - * @param controller - The transaction controller to monitor. + * @param messenger - The messenger to monitor. * @param options - Options to customize the wait. * @param options.confirmed - Whether to wait for the transaction to be confirmed or just finished. * @returns A promise that resolves with the transaction meta when the transaction is finished. */ function waitForTransactionFinished( - controller: TransactionController, + messenger: ControllerMessenger< + TransactionControllerActions | AllowedActions, + TransactionControllerEvents | AllowedEvents + >, { confirmed = false } = {}, ): Promise { + const eventName = confirmed + ? 'TransactionController:transactionConfirmed' + : 'TransactionController:transactionFinished'; return new Promise((resolve) => { - controller.hub.once( - `${controller.state.transactions[0].id}:${ - confirmed ? 'confirmed' : 'finished' - }`, - (txMeta) => { - resolve(txMeta); - }, - ); + const subscriber = (transactionMeta: TransactionMeta) => { + resolve(transactionMeta); + messenger.unsubscribe(eventName, subscriber); + }; + messenger.subscribe(eventName, subscriber); }); } @@ -507,11 +409,13 @@ const ACTION_ID_MOCK = '123456'; const TRANSACTION_META_MOCK = { hash: '0x1', + id: '1', status: TransactionStatus.confirmed as const, time: 123456789, txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, + value: '0x42', }, } as TransactionMeta; @@ -525,6 +429,8 @@ const TRANSACTION_META_2_MOCK = { } as TransactionMeta; describe('TransactionController', () => { + const uuidModuleMock = jest.mocked(uuidModule); + const EthQueryMock = jest.mocked(EthQuery); const updateGasMock = jest.mocked(updateGas); const updateGasFeesMock = jest.mocked(updateGasFees); const estimateGasMock = jest.mocked(estimateGas); @@ -537,11 +443,7 @@ describe('TransactionController', () => { const lineaGasFeeFlowClassMock = jest.mocked(LineaGasFeeFlow); const gasFeePollerClassMock = jest.mocked(GasFeePoller); - let resultCallbacksMock: AcceptResultCallbacks; - let messengerMock: TransactionControllerMessenger; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let approveTransaction: (value?: any) => void; + let mockEthQuery: EthQuery; let getNonceLockSpy: jest.Mock; let incomingTransactionHelperMock: jest.Mocked; let pendingTransactionTrackerMock: jest.Mocked; @@ -567,143 +469,190 @@ describe('TransactionController', () => { >; /** - * Create a new instance of the TransactionController. + * Constructs an instance of the TransactionController and supporting data for + * use in tests. * - * @param opts - Options to use when creating the controller. - * @param opts.options - Any controller options to override the test defaults. - * @param opts.config - Any configuration to override the test defaults. - * @param opts.network - The mock network to use with the controller. - * @param opts.approve - Whether transactions should be immediately approved. - * @param opts.reject - Whether transactions should be immediately rejected. - * @param opts.state - The initial state to use for the controller. + * @param args - The arguments to this function. + * @param args.options - TransactionController options. + * @param args.network - The mock network to use with the controller. + * @param args.messengerOptions - Options to build the mock unrestricted + * messenger. + * @param args.messengerOptions.addTransactionApprovalRequest - Options to mock + * the `ApprovalController:addRequest` action call for transactions. * @returns The new TransactionController instance. */ - function newController({ - options, - config, - network, - approve, - reject, - state, + function setupController({ + options: givenOptions = {}, + network = MOCK_NETWORK, + messengerOptions = {}, }: { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - options?: any; - config?: Partial; + options?: Partial[0]>; network?: MockNetwork; - approve?: boolean; - reject?: boolean; - state?: Partial; - } = {}): TransactionController { - const finalNetwork = network ?? MOCK_NETWORK; - - resultCallbacksMock = buildMockResultCallbacks(); - let addRequestMockOptions: AddRequestOptions; - if (approve) { - addRequestMockOptions = { - approved: true, - resultCallbacks: resultCallbacksMock, - }; - } else if (reject) { - addRequestMockOptions = { - approved: false, - resultCallbacks: resultCallbacksMock, - }; - } else { - addRequestMockOptions = { - delay: true, - resultCallbacks: resultCallbacksMock, - }; - } + messengerOptions?: { + addTransactionApprovalRequest?: Parameters< + typeof mockAddTransactionApprovalRequest + >[1]; + }; + } = {}) { + const unrestrictedMessenger: UnrestrictedControllerMessenger = + new ControllerMessenger(); + + const { addTransactionApprovalRequest = { state: 'pending' } } = + messengerOptions; + const mockTransactionApprovalRequest = mockAddTransactionApprovalRequest( + unrestrictedMessenger, + addTransactionApprovalRequest, + ); - const mockGetNetworkClientById = jest - .fn() - .mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - configuration: { - chainId: toHex(1), - }, - blockTracker: finalNetwork.blockTracker, - provider: finalNetwork.provider, - }; - case 'sepolia': - return { - configuration: { - chainId: ChainId.sepolia, - }, - blockTracker: buildMockBlockTracker('0x1'), - provider: MAINNET_PROVIDER, - }; - case 'goerli': - return { - configuration: { - chainId: ChainId.goerli, - }, - blockTracker: buildMockBlockTracker('0x1'), - provider: MAINNET_PROVIDER, - }; - case 'customNetworkClientId-1': - return { - configuration: { - chainId: '0xa', - }, - blockTracker: buildMockBlockTracker('0x1'), - provider: MAINNET_PROVIDER, - }; - default: - throw new Error(`Invalid network client id ${networkClientId}`); - } + const { messenger: givenRestrictedMessenger, ...otherOptions } = { + blockTracker: network.blockTracker, + disableHistory: false, + disableSendFlowHistory: false, + disableSwaps: false, + getCurrentNetworkEIP1559Compatibility: async () => false, + getNetworkState: () => network.state, + // TODO: Replace with a real type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getNetworkClientRegistry: () => ({} as any), + getPermittedAccounts: async () => [ACCOUNT_MOCK], + getSelectedAddress: () => ACCOUNT_MOCK, + isMultichainEnabled: false, + hooks: {}, + onNetworkStateChange: network.subscribe, + provider: network.provider, + sign: async (transaction: TypedTransaction) => transaction, + transactionHistoryLimit: 40, + ...givenOptions, + }; + + const restrictedMessenger = + givenRestrictedMessenger ?? + unrestrictedMessenger.getRestricted({ + name: 'TransactionController', + allowedActions: [ + 'ApprovalController:addRequest', + 'NetworkController:getNetworkClientById', + 'NetworkController:findNetworkClientIdByChainId', + ], }); - const mockFindNetworkClientIdByChainId = jest - .fn() - .mockImplementation((chainId) => { - switch (chainId) { - case '0x1': - return 'mainnet'; - case ChainId.sepolia: - return 'sepolia'; - case ChainId.goerli: - return 'goerli'; - case '0xa': - return 'customNetworkClientId-1'; - default: - throw new Error("Couldn't find networkClientId for chainId"); + const controller = new TransactionController({ + ...otherOptions, + messenger: restrictedMessenger, + }); + + return { + controller, + messenger: unrestrictedMessenger, + mockTransactionApprovalRequest, + }; + } + + /** + * Mocks the `ApprovalController:addRequest` action that the + * TransactionController calls as it creates transactions. + * + * This helper allows the `addRequest` action to be in one of three states: + * approved, rejected, or pending. In the approved state, the promise which the + * action returns is resolved ahead of time, and in the rejected state, the + * promise is rejected ahead of time. Otherwise, in the pending state, the + * promise is unresolved and it is assumed that the test will resolve or reject + * the promise. + * + * @param messenger - The unrestricted messenger. + * @param options - Options for the mock. `state` controls the state of the + * promise as outlined above. Note, if the `state` is approved, then its + * `result` may be specified; if the `state` is rejected, then its `error` may + * be specified. + * @returns An object which contains the aforementioned promise, functions to + * manually approve or reject the approval (and therefore the promise), and + * finally the mocked version of the action handler itself. + */ + function mockAddTransactionApprovalRequest( + messenger: UnrestrictedControllerMessenger, + options: + | { + state: 'approved'; + result?: Partial; } + | { + state: 'rejected'; + error?: unknown; + } + | { + state: 'pending'; + }, + ): { + promise: Promise; + approve: (approvalResult?: Partial) => void; + reject: (rejectionError: unknown) => void; + actionHandlerMock: jest.Mock< + ReturnType, + Parameters + >; + } { + const { promise, resolve, reject } = createDeferredPromise(); + + const approveTransaction = (approvalResult?: Partial) => { + resolve({ + resultCallbacks: { + success() { + // do nothing + }, + error() { + // do nothing + }, + }, + ...approvalResult, }); + }; - ({ messenger: messengerMock, approve: approveTransaction } = - buildMockMessenger({ - addRequest: addRequestMockOptions, - getNetworkClientById: mockGetNetworkClientById, - findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, - })); - - return new TransactionController( - { - blockTracker: finalNetwork.blockTracker, - getNetworkState: () => finalNetwork.state, - getCurrentNetworkEIP1559Compatibility: () => true, - getSavedGasFees: () => undefined, - getGasFeeEstimates: () => Promise.resolve({}), - getPermittedAccounts: () => [ACCOUNT_MOCK], - getSelectedAddress: () => ACCOUNT_MOCK, - getNetworkClientRegistry: jest.fn(), - messenger: messengerMock, - onNetworkStateChange: finalNetwork.subscribe, - provider: finalNetwork.provider, - ...options, + const rejectTransaction = ( + rejectionError: unknown = { + code: errorCodes.provider.userRejectedRequest, }, - { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sign: async (transaction: any) => transaction, - ...config, - }, - state ?? undefined, + ) => { + reject(rejectionError); + }; + + const actionHandlerMock: jest.Mock< + ReturnType, + Parameters + > = jest.fn().mockReturnValue(promise); + + if (options.state === 'approved') { + approveTransaction(options.result); + } else if (options.state === 'rejected') { + rejectTransaction(options.error); + } + + messenger.registerActionHandler( + 'ApprovalController:addRequest', + actionHandlerMock, ); + + return { + promise, + approve: approveTransaction, + reject: rejectTransaction, + actionHandlerMock, + }; + } + + /** + * Builds a network client that is only used in tests to get a chain ID. + * + * @param networkClient - The properties in the desired network client. + * Only needs to contain `configuration`. + * @param networkClient.configuration - The desired network client + * configuration. Only needs to contain `chainId`> + * @returns The network client. + */ + function buildMockNetworkClient(networkClient: { + configuration: NetworkClientConfiguration; + }): NetworkClient { + // Type assertion: We don't expect anything but the configuration to get used. + return networkClient as unknown as NetworkClient; } /** @@ -725,6 +674,11 @@ describe('TransactionController', () => { mockFlags[key] = null; } + uuidModuleMock.v1.mockReturnValue(MOCK_V1_UUID); + + mockEthQuery = buildMockEthQuery(); + EthQueryMock.mockImplementation(() => mockEthQuery); + getNonceLockSpy = jest.fn().mockResolvedValue({ nextNonce: NONCE_MOCK, releaseLock: () => Promise.resolve(), @@ -793,15 +747,19 @@ describe('TransactionController', () => { } as unknown as jest.Mocked; return gasFeePollerMock; }); + + updateSwapsTransactionMock.mockImplementation( + (transactionMeta) => transactionMeta, + ); }); afterEach(() => { - jest.clearAllMocks(); + jest.resetAllMocks(); }); describe('constructor', () => { it('sets default state', () => { - const controller = newController(); + const { controller } = setupController(); expect(controller.state).toStrictEqual({ methodData: {}, transactions: [], @@ -809,16 +767,8 @@ describe('TransactionController', () => { }); }); - it('sets default config', () => { - const controller = newController(); - expect(controller.config).toStrictEqual({ - txHistoryLimit: 40, - sign: expect.any(Function), - }); - }); - it('provides gas fee flows to GasFeePoller in correct order', () => { - newController(); + setupController(); expect(gasFeePollerClassMock).toHaveBeenCalledTimes(1); expect(gasFeePollerClassMock).toHaveBeenCalledWith( @@ -845,17 +795,20 @@ describe('TransactionController', () => { .fn() .mockReturnValueOnce(externalPendingTransactions); - const controller = newController({ - options: { getExternalPendingTransactions }, - }); - - controller.state.transactions = [ - { - ...TRANSACTION_META_MOCK, - chainId: MOCK_NETWORK.state.providerConfig.chainId, - status: TransactionStatus.submitted, + setupController({ + options: { + getExternalPendingTransactions, + state: { + transactions: [ + { + ...TRANSACTION_META_MOCK, + chainId: MOCK_NETWORK.state.providerConfig.chainId, + status: TransactionStatus.submitted, + }, + ], + }, }, - ]; + }); const pendingTransactions = nonceTrackerMock.mock.calls[0][0].getPendingTransactions( @@ -918,10 +871,12 @@ describe('TransactionController', () => { lastFetchedBlockNumbers: {}, }; - const controller = newController({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - state: mockedControllerState as any, + const { controller } = setupController({ + options: { + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + state: mockedControllerState as any, + }, }); await flushPromises(); @@ -943,7 +898,7 @@ describe('TransactionController', () => { errorKey: 'testKey', }; - const controller = newController(); + const { controller } = setupController(); estimateGasMock.mockResolvedValue({ estimatedGas: gasMock, @@ -982,7 +937,7 @@ describe('TransactionController', () => { }, }; - const controller = newController(); + const { controller } = setupController(); estimateGasMock.mockResolvedValue({ estimatedGas: gasMock, @@ -1017,7 +972,7 @@ describe('TransactionController', () => { describe('with actionId', () => { it('adds single unapproved transaction when called twice with same actionId', async () => { - const controller = newController(); + const { controller, mockTransactionApprovalRequest } = setupController(); const mockOrigin = 'origin'; @@ -1047,9 +1002,12 @@ describe('TransactionController', () => { const secondTransactionCount = controller.state.transactions.length; expect(firstTransactionCount).toStrictEqual(secondTransactionCount); - expect(messengerMock.call).toHaveBeenCalledTimes(1); - expect(messengerMock.call).toHaveBeenCalledWith( - 'ApprovalController:addRequest', + expect( + mockTransactionApprovalRequest.actionHandlerMock, + ).toHaveBeenCalledTimes(1); + expect( + mockTransactionApprovalRequest.actionHandlerMock, + ).toHaveBeenCalledWith( { id: expect.any(String), origin: mockOrigin, @@ -1062,7 +1020,7 @@ describe('TransactionController', () => { }); it('adds multiple transactions with same actionId and ensures second transaction result does not resolves before the first transaction result', async () => { - const controller = newController(); + const { controller, mockTransactionApprovalRequest } = setupController(); const mockOrigin = 'origin'; let firstTransactionCompleted = false; @@ -1106,7 +1064,7 @@ describe('TransactionController', () => { expect(firstTransactionCompleted).toBe(false); expect(secondTransactionCompleted).toBe(false); - approveTransaction(); + mockTransactionApprovalRequest.approve(); await firstResult; await secondResult; @@ -1130,7 +1088,8 @@ describe('TransactionController', () => { ])( '%s', async (_, firstActionId, secondActionId, expectedTransactionCount) => { - const controller = newController(); + const { controller, mockTransactionApprovalRequest } = + setupController(); const expectedRequestApprovalCalledTimes = expectedTransactionCount; const mockOrigin = 'origin'; @@ -1159,9 +1118,9 @@ describe('TransactionController', () => { const { transactions } = controller.state; expect(transactions).toHaveLength(expectedTransactionCount); - expect(messengerMock.call).toHaveBeenCalledTimes( - expectedRequestApprovalCalledTimes, - ); + expect( + mockTransactionApprovalRequest.actionHandlerMock, + ).toHaveBeenCalledTimes(expectedRequestApprovalCalledTimes); }, ); @@ -1186,7 +1145,7 @@ describe('TransactionController', () => { expectedTransactionCount, expectedSignCalledTimes, ) => { - const controller = newController(); + const { controller } = setupController(); const signSpy = jest.spyOn(controller, 'sign'); const { transactionMeta } = await controller.addTransaction({ @@ -1213,7 +1172,7 @@ describe('TransactionController', () => { describe('addTransaction', () => { it('adds unapproved transaction to state', async () => { - const controller = newController(); + const { controller } = setupController(); const mockDeviceConfirmedOn = WalletDevice.OTHER; const mockOrigin = 'origin'; @@ -1257,7 +1216,7 @@ describe('TransactionController', () => { expect(transactionMeta.deviceConfirmedOn).toBe(mockDeviceConfirmedOn); expect(transactionMeta.origin).toBe(mockOrigin); expect(transactionMeta.status).toBe(TransactionStatus.unapproved); - expect(transactionMeta.securityAlertResponse).toBe( + expect(transactionMeta.securityAlertResponse).toStrictEqual( mockSecurityAlertResponse, ); expect(controller.state.transactions[0].sendFlowHistory).toStrictEqual( @@ -1267,9 +1226,26 @@ describe('TransactionController', () => { describe('networkClientId exists in the MultichainTrackingHelper', () => { it('adds unapproved transaction to state when using networkClientId', async () => { - const controller = newController({ + const { controller, messenger } = setupController({ options: { isMultichainEnabled: true }, }); + messenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + (networkClientId) => { + switch (networkClientId) { + case 'sepolia': + return buildMockNetworkClient({ + configuration: { + chainId: ChainId.sepolia, + }, + }); + default: + throw new Error( + `Unknown network client ID: ${networkClientId}`, + ); + } + }, + ); const sepoliaTxParams: TransactionParams = { chainId: ChainId.sepolia, from: ACCOUNT_MOCK, @@ -1295,15 +1271,38 @@ describe('TransactionController', () => { }); it('adds unapproved transaction with networkClientId and can be updated to submitted', async () => { - const controller = newController({ - approve: true, + const { controller, messenger } = setupController({ options: { isMultichainEnabled: true }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); - + messenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + (networkClientId) => { + switch (networkClientId) { + case 'sepolia': + return buildMockNetworkClient({ + configuration: { + chainId: ChainId.sepolia, + }, + }); + default: + throw new Error( + `Unknown network client ID: ${networkClientId}`, + ); + } + }, + ); multichainTrackingHelperMock.has.mockReturnValue(true); const submittedEventListener = jest.fn(); - controller.hub.on('transaction-submitted', submittedEventListener); + messenger.subscribe( + 'TransactionController:transactionSubmitted', + submittedEventListener, + ); const sepoliaTxParams: TransactionParams = { chainId: ChainId.sepolia, @@ -1330,7 +1329,7 @@ describe('TransactionController', () => { }); it('generates initial history', async () => { - const controller = newController(); + const { controller } = setupController(); await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1356,14 +1355,14 @@ describe('TransactionController', () => { }; // Expect initial snapshot to be in place - expect(controller.state.transactions[0]?.history).toStrictEqual([ + expect(controller.state.transactions[0].history).toStrictEqual([ expectedInitialSnapshot, ]); }); it('only reads the current chain id to filter to initially populate the metadata', async () => { const getNetworkStateMock = jest.fn().mockReturnValue(MOCK_NETWORK.state); - const controller = newController({ + const { controller } = setupController({ options: { getNetworkState: getNetworkStateMock }, }); @@ -1383,7 +1382,7 @@ describe('TransactionController', () => { ['origin is not defined', undefined], ['no fee information is given', 'MockDappOrigin'], ])('as undefined if %s', async (_testName, origin) => { - const controller = newController(); + const { controller } = setupController(); await controller.addTransaction( { from: ACCOUNT_MOCK, @@ -1406,7 +1405,7 @@ describe('TransactionController', () => { ])( 'if %s is defined', async (gasPropName: keyof DappSuggestedGasFees) => { - const controller = newController(); + const { controller } = setupController(); const mockDappOrigin = 'MockDappOrigin'; const mockGasValue = '0x1'; await controller.addTransaction( @@ -1445,7 +1444,7 @@ describe('TransactionController', () => { networkStateChangeListener = listener; }; - const controller = newController({ + const { controller } = setupController({ options: { getNetworkState, onNetworkStateChange }, }); @@ -1473,18 +1472,24 @@ describe('TransactionController', () => { ); it('throws if address invalid', async () => { - const controller = newController(); + const { controller } = setupController(); await expect(controller.addTransaction({ from: 'foo' })).rejects.toThrow( 'Invalid "from" address', ); }); it('increments nonce when adding a new non-cancel non-speedup transaction', async () => { - v1Stub + uuidModuleMock.v1 .mockImplementationOnce(() => 'aaaab1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d') .mockImplementationOnce(() => 'bbbb1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'); - const controller = newController({ approve: true }); + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + }); const { result: firstResult } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1527,15 +1532,16 @@ describe('TransactionController', () => { }); it('requests approval using the approval controller', async () => { - const controller = newController(); + const { controller, messenger } = setupController(); + jest.spyOn(messenger, 'call'); await controller.addTransaction({ from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }); - expect(messengerMock.call).toHaveBeenCalledTimes(1); - expect(messengerMock.call).toHaveBeenCalledWith( + expect(messenger.call).toHaveBeenCalledTimes(1); + expect(messenger.call).toHaveBeenCalledWith( 'ApprovalController:addRequest', { id: expect.any(String), @@ -1549,7 +1555,8 @@ describe('TransactionController', () => { }); it('skips approval if option explicitly false', async () => { - const controller = newController(); + const { controller, messenger } = setupController(); + jest.spyOn(messenger, 'call'); await controller.addTransaction( { @@ -1561,7 +1568,7 @@ describe('TransactionController', () => { }, ); - expect(messengerMock.call).toHaveBeenCalledTimes(0); + expect(messenger.call).toHaveBeenCalledTimes(0); }); it('calls security provider with transaction meta and sets response in to securityProviderResponse', async () => { @@ -1574,7 +1581,7 @@ describe('TransactionController', () => { .fn() .mockResolvedValue(mockSecurityProviderResponse); - const controller = newController({ + const { controller } = setupController({ options: { securityProviderRequest: securityProviderRequestMock, }, @@ -1599,11 +1606,13 @@ describe('TransactionController', () => { ); const { securityProviderResponse } = controller.state.transactions[0]; - expect(securityProviderResponse).toBe(mockSecurityProviderResponse); + expect(securityProviderResponse).toStrictEqual( + mockSecurityProviderResponse, + ); }); it('updates gas properties', async () => { - const controller = newController(); + const { controller } = setupController(); await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1621,7 +1630,12 @@ describe('TransactionController', () => { }); it('updates gas fee properties', async () => { - const controller = newController(); + const { controller } = setupController({ + options: { + getCurrentNetworkEIP1559Compatibility: async () => true, + getCurrentAccountEIP1559Compatibility: async () => true, + }, + }); await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1641,9 +1655,18 @@ describe('TransactionController', () => { describe('on approve', () => { it('submits transaction', async () => { - const controller = newController({ approve: true }); + const { controller, messenger } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + }); const submittedEventListener = jest.fn(); - controller.hub.on('transaction-submitted', submittedEventListener); + messenger.subscribe( + 'TransactionController:transactionSubmitted', + submittedEventListener, + ); const { result } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1655,21 +1678,47 @@ describe('TransactionController', () => { await result; - const { txParams, status, submittedTime } = - controller.state.transactions[0]; - expect(txParams.from).toBe(ACCOUNT_MOCK); - expect(txParams.nonce).toBe(`0x${NONCE_MOCK.toString(16)}`); - expect(status).toBe(TransactionStatus.submitted); - expect(submittedTime).toStrictEqual(expect.any(Number)); + expect(controller.state.transactions).toMatchObject([ + expect.objectContaining({ + txParams: expect.objectContaining({ + from: ACCOUNT_MOCK, + nonce: toHex(NONCE_MOCK), + }), + status: TransactionStatus.submitted, + submittedTime: expect.any(Number), + }), + ]); expect(submittedEventListener).toHaveBeenCalledTimes(1); expect(submittedEventListener).toHaveBeenCalledWith({ - transactionMeta: controller.state.transactions[0], + transactionMeta: expect.objectContaining({ + txParams: expect.objectContaining({ + from: ACCOUNT_MOCK, + nonce: toHex(NONCE_MOCK), + }), + status: TransactionStatus.submitted, + submittedTime: expect.any(Number), + }), }); }); it('reports success to approval acceptor', async () => { - const controller = newController({ approve: true }); + const successCallback = jest.fn(); + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + result: { + resultCallbacks: { + success: successCallback, + error: () => { + // do nothing + }, + }, + }, + }, + }, + }); const { result } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1678,13 +1727,26 @@ describe('TransactionController', () => { await result; - expect(resultCallbacksMock.success).toHaveBeenCalledTimes(1); + expect(successCallback).toHaveBeenCalledTimes(1); }); it('reports error to approval acceptor on error', async () => { - const controller = newController({ - approve: true, - config: { sign: undefined }, + const errorCallback = jest.fn(); + const { controller } = setupController({ + options: { sign: undefined }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + result: { + resultCallbacks: { + success: () => { + // do nothing + }, + error: errorCallback, + }, + }, + }, + }, }); const { result } = await controller.addTransaction({ @@ -1698,11 +1760,12 @@ describe('TransactionController', () => { // Expected error } - expect(resultCallbacksMock.error).toHaveBeenCalledTimes(1); + expect(errorCallback).toHaveBeenCalledTimes(1); }); it('updates transaction if approval result includes updated metadata', async () => { - const controller = newController(); + const { controller, mockTransactionApprovalRequest } = + setupController(); const { result } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1711,8 +1774,10 @@ describe('TransactionController', () => { const transaction = controller.state.transactions[0]; - approveTransaction({ - txMeta: { ...transaction, customNonceValue: '123' }, + mockTransactionApprovalRequest.approve({ + value: { + txMeta: { ...transaction, customNonceValue: '123' }, + }, }); await result; @@ -1749,45 +1814,54 @@ describe('TransactionController', () => { } it('if signing error', async () => { - const controller = newController({ - approve: true, - config: { + const { controller } = setupController({ + options: { sign: () => { throw new Error('foo'); }, }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); await expectTransactionToFail(controller, 'foo'); }); it('if no sign method defined', async () => { - const controller = newController({ - approve: true, - config: { + const { controller } = setupController({ + options: { sign: undefined, }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); await expectTransactionToFail(controller, 'No sign method defined'); }); it('if no chainId defined', async () => { - const controller = newController({ - approve: true, + const { controller } = setupController({ network: MOCK_NETWORK_WITHOUT_CHAIN_ID, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); await expectTransactionToFail(controller, 'No chainId defined'); }); it('if unexpected status', async () => { - const controller = newController(); + const { controller, messenger } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const callMock = messengerMock.call as jest.MockedFunction; - callMock.mockImplementationOnce(() => { + jest.spyOn(messenger, 'call').mockImplementationOnce(() => { throw new Error('Unknown problem'); }); @@ -1803,12 +1877,9 @@ describe('TransactionController', () => { }); it('if unrecognised error', async () => { - const controller = newController(); + const { controller, messenger } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const callMock = messengerMock.call as jest.MockedFunction; - callMock.mockImplementationOnce(() => { + jest.spyOn(messenger, 'call').mockImplementationOnce(() => { throw new Error('TestError'); }); @@ -1824,13 +1895,10 @@ describe('TransactionController', () => { }); it('if transaction removed', async () => { - const controller = newController(); + const { controller, messenger } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const callMock = messengerMock.call as jest.MockedFunction; - callMock.mockImplementationOnce(() => { - controller.state.transactions = []; + jest.spyOn(messenger, 'call').mockImplementationOnce(() => { + controller.clearUnapprovedTransactions(); throw new Error('Unknown problem'); }); @@ -1849,14 +1917,20 @@ describe('TransactionController', () => { describe('on reject', () => { it('cancels transaction', async () => { - const controller = newController({ reject: true }); + const { controller, messenger } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'rejected', + }, + }, + }); const { result } = await controller.addTransaction({ from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }); - const finishedPromise = waitForTransactionFinished(controller); + const finishedPromise = waitForTransactionFinished(messenger); await expect(result).rejects.toThrow( 'MetaMask Tx Signature: User denied transaction signature.', @@ -1867,12 +1941,20 @@ describe('TransactionController', () => { expect(status).toBe(TransactionStatus.rejected); }); - it('emits rejected and finished event', async () => { - const controller = newController({ reject: true }); + it('publishes TransactionController:transactionRejected and TransactionController:transactionFinished', async () => { + const { controller, messenger } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'rejected', + }, + }, + }); const rejectedEventListener = jest.fn(); - const finishedEventListener = jest.fn(); - controller.hub.on('transaction-rejected', rejectedEventListener); + messenger.subscribe( + 'TransactionController:transactionRejected', + rejectedEventListener, + ); const mockActionId = 'mockActionId'; @@ -1886,12 +1968,7 @@ describe('TransactionController', () => { }, ); - controller.hub.on( - `${transactionMeta.id}:finished`, - finishedEventListener, - ); - - const finishedPromise = waitForTransactionFinished(controller); + const finishedPromise = waitForTransactionFinished(messenger); try { await result; @@ -1902,18 +1979,15 @@ describe('TransactionController', () => { expect(rejectedEventListener).toHaveBeenCalledTimes(1); expect(rejectedEventListener).toHaveBeenCalledWith({ - transactionMeta, + transactionMeta: { ...transactionMeta, status: 'rejected' }, actionId: mockActionId, }); - - expect(finishedEventListener).toHaveBeenCalledTimes(1); - expect(finishedEventListener).toHaveBeenCalledWith(transactionMeta); }); }); describe('checks from address origin', () => { it('throws if `from` address is different from current selected address', async () => { - const controller = newController(); + const { controller } = setupController(); const origin = ORIGIN_METAMASK; const notSelectedFromAddress = ACCOUNT_2_MOCK; await expect( @@ -1937,7 +2011,7 @@ describe('TransactionController', () => { }); it('throws if the origin does not have permissions to initiate transactions from the specified address', async () => { - const controller = newController(); + const { controller } = setupController(); const expectedOrigin = 'originMocked'; await expect( controller.addTransaction( @@ -1953,7 +2027,7 @@ describe('TransactionController', () => { describe('wipeTransactions', () => { it('removes all transactions on current network', async () => { - const controller = newController(); + const { controller } = setupController(); controller.wipeTransactions(); @@ -1968,31 +2042,33 @@ describe('TransactionController', () => { }); it('removes only txs with given address', async () => { - const controller = newController(); - - controller.wipeTransactions(); - const mockFromAccount1 = '0x1bf137f335ea1b8f193b8f6ea92561a60d23a207'; const mockFromAccount2 = '0x2bf137f335ea1b8f193b8f6ea92561a60d23a207'; const mockCurrentChainId = toHex(5); - - controller.state.transactions.push({ - id: '1', - chainId: mockCurrentChainId, - status: TransactionStatus.confirmed as const, - time: 123456789, - txParams: { - from: mockFromAccount1, - }, - }); - - controller.state.transactions.push({ - id: '2', - chainId: mockCurrentChainId, - status: TransactionStatus.confirmed as const, - time: 987654321, - txParams: { - from: mockFromAccount2, + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + id: '1', + chainId: mockCurrentChainId, + status: TransactionStatus.confirmed as const, + time: 123456789, + txParams: { + from: mockFromAccount1, + }, + }, + { + id: '2', + chainId: mockCurrentChainId, + status: TransactionStatus.confirmed as const, + time: 987654321, + txParams: { + from: mockFromAccount2, + }, + }, + ], + }, }, }); @@ -2003,32 +2079,34 @@ describe('TransactionController', () => { }); it('removes only txs with given address only on current network', async () => { - const controller = newController(); - - controller.wipeTransactions(); - const mockFromAccount1 = '0x1bf137f335ea1b8f193b8f6ea92561a60d23a207'; const mockDifferentChainId = toHex(1); const mockCurrentChainId = toHex(5); - - controller.state.transactions.push({ - id: '1', - chainId: mockCurrentChainId, - txParams: { - from: mockFromAccount1, - }, - status: TransactionStatus.confirmed as const, - time: 123456789, - }); - - controller.state.transactions.push({ - id: '4', - chainId: mockDifferentChainId, - txParams: { - from: mockFromAccount1, + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + id: '1', + chainId: mockCurrentChainId, + txParams: { + from: mockFromAccount1, + }, + status: TransactionStatus.confirmed as const, + time: 123456789, + }, + { + id: '4', + chainId: mockDifferentChainId, + txParams: { + from: mockFromAccount1, + }, + status: TransactionStatus.confirmed as const, + time: 987654321, + }, + ], + }, }, - status: TransactionStatus.confirmed as const, - time: 987654321, }); controller.wipeTransactions(false, mockFromAccount1); @@ -2040,7 +2118,7 @@ describe('TransactionController', () => { describe('handleMethodData', () => { it('loads method data from registry', async () => { - const controller = newController({ network: MOCK_MAINNET_NETWORK }); + const { controller } = setupController({ network: MOCK_MAINNET_NETWORK }); mockNetwork({ networkClientConfiguration: { chainId: BUILT_IN_NETWORKS.mainnet.chainId, @@ -2080,7 +2158,7 @@ describe('TransactionController', () => { }); it('skips reading registry if already cached in state', async () => { - const controller = newController({ network: MOCK_MAINNET_NETWORK }); + const { controller } = setupController({ network: MOCK_MAINNET_NETWORK }); mockNetwork({ networkClientConfiguration: { ticker: BUILT_IN_NETWORKS.mainnet.ticker, @@ -2125,17 +2203,23 @@ describe('TransactionController', () => { describe('stopTransaction', () => { it('should avoid creating cancel transaction if actionId already exist', async () => { const mockActionId = 'mockActionId'; - const controller = newController(); - - controller.state.transactions.push({ - actionId: mockActionId, - id: '2', - chainId: toHex(5), - status: TransactionStatus.submitted, - type: TransactionType.cancel, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + actionId: mockActionId, + id: '2', + chainId: toHex(5), + status: TransactionStatus.submitted, + type: TransactionType.cancel, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + }, + }, + ], + }, }, }); @@ -2147,69 +2231,71 @@ describe('TransactionController', () => { }); it('should throw error if transaction already confirmed', async () => { - const controller = newController(); - - controller.state.transactions.push({ - id: '2', - chainId: toHex(5), - status: TransactionStatus.submitted, - type: TransactionType.cancel, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + id: '2', + chainId: toHex(5), + status: TransactionStatus.submitted, + type: TransactionType.cancel, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + }, + }, + ], + }, }, }); - mockSendRawTransaction.mockImplementationOnce( - (_transaction, callback) => { - callback( - undefined, - // eslint-disable-next-line prefer-promise-reject-errors - Promise.reject({ - message: 'nonce too low', - }), - ); - }, - ); + jest + .spyOn(mockEthQuery, 'sendRawTransaction') + .mockImplementation((_transaction, callback) => { + callback(new Error('nonce too low')); + }); await expect(controller.stopTransaction('2')).rejects.toThrow( 'Previous transaction is already confirmed', ); // Expect cancel transaction to be submitted - it will fail - expect(mockSendRawTransaction).toHaveBeenCalledTimes(1); + expect(mockEthQuery.sendRawTransaction).toHaveBeenCalledTimes(1); expect(controller.state.transactions).toHaveLength(1); }); it('should throw error if publish transaction fails', async () => { - const errorMock = new Error('Another reason'); - const controller = newController(); - - controller.state.transactions.push({ - id: '2', - chainId: toHex(5), - status: TransactionStatus.submitted, - type: TransactionType.cancel, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, + const error = new Error('Another reason'); + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + id: '2', + chainId: toHex(5), + status: TransactionStatus.submitted, + type: TransactionType.cancel, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + }, + }, + ], + }, }, }); - mockSendRawTransaction.mockImplementationOnce( - (_transaction, callback) => { - callback( - undefined, - // eslint-disable-next-line prefer-promise-reject-errors - Promise.reject(errorMock), - ); - }, - ); + jest + .spyOn(mockEthQuery, 'sendRawTransaction') + .mockImplementation((_transaction, callback) => { + callback(error); + }); - await expect(controller.stopTransaction('2')).rejects.toThrow(errorMock); + await expect(controller.stopTransaction('2')).rejects.toThrow(error); // Expect cancel transaction to be submitted - it will fail - expect(mockSendRawTransaction).toHaveBeenCalledTimes(1); + expect(mockEthQuery.sendRawTransaction).toHaveBeenCalledTimes(1); expect(controller.state.transactions).toHaveLength(1); }); @@ -2218,20 +2304,31 @@ describe('TransactionController', () => { 'simpleeb1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'; const cancelTransactionId = 'cancel1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'; const mockNonce = '0x9'; - v1Stub.mockImplementationOnce(() => cancelTransactionId); - - const controller = newController(); + uuidModuleMock.v1.mockImplementationOnce(() => cancelTransactionId); + jest + .spyOn(mockEthQuery, 'sendRawTransaction') + .mockImplementation((_transaction, callback) => { + callback(undefined, 'transaction-hash'); + }); - // Assume we have a submitted transaction in the state - controller.state.transactions.push({ - id: simpleSendTransactionId, - chainId: toHex(5), - status: TransactionStatus.submitted, - type: TransactionType.simpleSend, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, - nonce: mockNonce, + const { controller } = setupController({ + options: { + state: { + transactions: [ + // Assume we have a submitted transaction in the state + { + id: simpleSendTransactionId, + chainId: toHex(5), + status: TransactionStatus.submitted, + type: TransactionType.simpleSend, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + nonce: mockNonce, + }, + }, + ], + }, }, }); @@ -2246,10 +2343,8 @@ describe('TransactionController', () => { ); // Expect cancel transaction to be submitted - expect(mockSendRawTransaction).toHaveBeenCalledTimes(1); - expect(cancelTransaction?.hash).toBe( - ethQueryMockResults.sendRawTransaction, - ); + expect(mockEthQuery.sendRawTransaction).toHaveBeenCalledTimes(1); + expect(cancelTransaction?.hash).toBe('transaction-hash'); }); it('adds cancel transaction to state', async () => { @@ -2257,9 +2352,9 @@ describe('TransactionController', () => { 'simpleeb1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'; const cancelTransactionId = 'cancel1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'; const mockNonce = '0x9'; - v1Stub.mockImplementationOnce(() => cancelTransactionId); + uuidModuleMock.v1.mockImplementationOnce(() => cancelTransactionId); - const controller = newController(); + const { controller } = setupController(); // Assume we have a submitted transaction in the state controller.state.transactions.push({ @@ -2299,7 +2394,7 @@ describe('TransactionController', () => { }); it('rejects unknown transaction', async () => { - const controller = newController({ + const { controller } = setupController({ network: MOCK_LINEA_GOERLI_NETWORK, }); @@ -2313,7 +2408,7 @@ describe('TransactionController', () => { }); it('throws if no sign method', async () => { - const controller = newController({ config: { sign: undefined } }); + const { controller } = setupController({ options: { sign: undefined } }); await controller.addTransaction({ from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }); @@ -2322,10 +2417,9 @@ describe('TransactionController', () => { ).rejects.toThrow('No sign method defined'); }); - it('emits transaction events', async () => { - const controller = newController({ - network: MOCK_LINEA_GOERLI_NETWORK, - }); + it('publishes transaction events', async () => { + const { controller, messenger, mockTransactionApprovalRequest } = + setupController({ network: MOCK_LINEA_GOERLI_NETWORK }); const approvedEventListener = jest.fn(); const submittedEventListener = jest.fn(); @@ -2333,8 +2427,14 @@ describe('TransactionController', () => { const mockActionId = 'mockActionId'; - controller.hub.on('transaction-approved', approvedEventListener); - controller.hub.on('transaction-approved', submittedEventListener); + messenger.subscribe( + 'TransactionController:transactionApproved', + approvedEventListener, + ); + messenger.subscribe( + 'TransactionController:transactionSubmitted', + submittedEventListener, + ); const { transactionMeta } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -2344,14 +2444,14 @@ describe('TransactionController', () => { value: '0x0', }); - controller.hub.on( - `${transactionMeta.id}:finished`, + messenger.subscribe( + 'TransactionController:transactionFinished', finishedEventListener, ); - approveTransaction(); + mockTransactionApprovalRequest.approve(); - // Release for add transaction transaction submission + // Release for add transaction submission await flushPromises(); await controller.stopTransaction(transactionMeta.id, undefined, { @@ -2386,10 +2486,10 @@ describe('TransactionController', () => { describe('speedUpTransaction', () => { it('creates additional transaction', async () => { - const controller = newController({ + const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, options: { - getCurrentNetworkEIP1559Compatibility: () => false, + getCurrentNetworkEIP1559Compatibility: async () => false, }, }); @@ -2412,7 +2512,7 @@ describe('TransactionController', () => { it('should avoid creating speedup transaction if actionId already exist', async () => { const mockActionId = 'mockActionId'; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ actionId: mockActionId, @@ -2434,79 +2534,79 @@ describe('TransactionController', () => { }); it('should throw error if transaction already confirmed', async () => { - const controller = newController(); - - controller.state.transactions.push({ - id: '2', - chainId: toHex(5), - status: TransactionStatus.submitted, - type: TransactionType.retry, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + id: '2', + chainId: toHex(5), + status: TransactionStatus.submitted, + type: TransactionType.retry, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + }, + }, + ], + }, }, }); - mockSendRawTransaction.mockImplementationOnce( - (_transaction, callback) => { - callback( - undefined, - // eslint-disable-next-line prefer-promise-reject-errors - Promise.reject({ - message: 'nonce too low', - }), - ); - }, - ); + jest + .spyOn(mockEthQuery, 'sendRawTransaction') + .mockImplementation((_transaction, callback) => { + callback(new Error('nonce too low')); + }); await expect(controller.speedUpTransaction('2')).rejects.toThrow( 'Previous transaction is already confirmed', - ); - - // Expect speedup transaction to be submitted - it will fail - expect(mockSendRawTransaction).toHaveBeenCalledTimes(1); - expect(controller.state.transactions).toHaveLength(1); - }); - - it('should throw error if publish transaction fails', async () => { - const controller = newController(); - const errorMock = new Error('Another reason'); - - controller.state.transactions.push({ - id: '2', - chainId: toHex(5), - status: TransactionStatus.submitted, - type: TransactionType.retry, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, + ); + + // Expect speedup transaction to be submitted - it will fail + expect(mockEthQuery.sendRawTransaction).toHaveBeenCalledTimes(1); + expect(controller.state.transactions).toHaveLength(1); + }); + + it('should throw error if publish transaction fails', async () => { + const error = new Error('Another reason'); + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + id: '2', + chainId: toHex(5), + status: TransactionStatus.submitted, + type: TransactionType.retry, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + }, + }, + ], + }, }, }); - mockSendRawTransaction.mockImplementationOnce( - (_transaction, callback) => { - callback( - undefined, - // eslint-disable-next-line prefer-promise-reject-errors - Promise.reject(errorMock), - ); - }, - ); + jest + .spyOn(mockEthQuery, 'sendRawTransaction') + .mockImplementation((_transaction, callback) => { + callback(error); + }); - await expect(controller.speedUpTransaction('2')).rejects.toThrow( - errorMock, - ); + await expect(controller.speedUpTransaction('2')).rejects.toThrow(error); // Expect speedup transaction to be submitted - it will fail - expect(mockSendRawTransaction).toHaveBeenCalledTimes(1); + expect(mockEthQuery.sendRawTransaction).toHaveBeenCalledTimes(1); expect(controller.state.transactions).toHaveLength(1); }); it('creates additional transaction with increased gas', async () => { - const controller = newController({ + const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, options: { - getCurrentNetworkEIP1559Compatibility: () => false, + getCurrentNetworkEIP1559Compatibility: async () => false, }, }); @@ -2528,16 +2628,15 @@ describe('TransactionController', () => { }); it('verifies s,r and v values are correctly populated', async () => { - const controller = newController({ + const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, - config: { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sign: async (transaction: any) => { - transaction.r = '1b'; - transaction.s = 'abc'; - transaction.v = '123'; - return transaction; + options: { + sign: async (transaction) => { + return Object.assign(transaction, { + r: 128n, + s: 256n, + v: 512n, + }); }, }, }); @@ -2555,22 +2654,25 @@ describe('TransactionController', () => { const { transactions } = controller.state; expect(transactions).toHaveLength(2); const speedUpTransaction = transactions[1]; - expect(speedUpTransaction.r).toBe('0x1b'); - expect(speedUpTransaction.s).toBe('0xabc'); - expect(speedUpTransaction.v).toBe('0x123'); + expect(speedUpTransaction).toMatchObject({ + r: '0x80', + s: '0x100', + v: '0x200', + }); }); it('verifies s,r and v values are correctly populated if values are zero', async () => { - const controller = newController({ + const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, - config: { + options: { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any sign: async (transaction: any) => { - transaction.r = 0; - transaction.s = 0; - transaction.v = 0; - return transaction; + return Object.assign(transaction, { + r: 0n, + s: 0n, + v: 0n, + }); }, }, }); @@ -2594,10 +2696,10 @@ describe('TransactionController', () => { }); it('creates additional transaction specifying the gasPrice', async () => { - const controller = newController({ + const { controller } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, options: { - getCurrentNetworkEIP1559Compatibility: () => false, + getCurrentNetworkEIP1559Compatibility: async () => false, }, }); @@ -2619,7 +2721,13 @@ describe('TransactionController', () => { }); it('uses the same nonce', async () => { - const controller = newController({ approve: true }); + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + }); const { transactionMeta, result } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -2646,10 +2754,14 @@ describe('TransactionController', () => { }); it('allows transaction count to exceed txHistorylimit', async () => { - const controller = newController({ - approve: true, - config: { - txHistoryLimit: 1, + const { controller } = setupController({ + options: { + transactionHistoryLimit: 1, + }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, }, }); @@ -2668,19 +2780,25 @@ describe('TransactionController', () => { expect(controller.state.transactions).toHaveLength(2); }); - it('emits transaction events', async () => { - const controller = newController({ + it('publishes transaction events', async () => { + const { controller, messenger } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, }); const approvedEventListener = jest.fn(); const submittedEventListener = jest.fn(); - const finishedEventListener = jest.fn(); + const speedupEventListener = jest.fn(); const mockActionId = 'mockActionId'; - controller.hub.on('transaction-approved', approvedEventListener); - controller.hub.on('transaction-approved', submittedEventListener); + messenger.subscribe( + 'TransactionController:transactionApproved', + approvedEventListener, + ); + messenger.subscribe( + 'TransactionController:transactionSubmitted', + submittedEventListener, + ); const { transactionMeta: firstTransactionMeta } = await controller.addTransaction({ @@ -2691,9 +2809,9 @@ describe('TransactionController', () => { value: '0x0', }); - controller.hub.on( - `${firstTransactionMeta.id}:speedup`, - finishedEventListener, + messenger.subscribe( + 'TransactionController:speedupTransactionAdded', + speedupEventListener, ); await controller.speedUpTransaction(firstTransactionMeta.id, undefined, { @@ -2715,14 +2833,14 @@ describe('TransactionController', () => { transactionMeta: speedUpTransaction, }); - expect(finishedEventListener).toHaveBeenCalledTimes(1); - expect(finishedEventListener).toHaveBeenCalledWith(speedUpTransaction); + expect(speedupEventListener).toHaveBeenCalledTimes(1); + expect(speedupEventListener).toHaveBeenCalledWith(speedUpTransaction); }); }); describe('confirmExternalTransaction', () => { it('adds external transaction to the state as confirmed', async () => { - const controller = newController(); + const { controller } = setupController(); const externalTransactionToConfirm = { id: '1', @@ -2758,7 +2876,7 @@ describe('TransactionController', () => { }); it('generates initial history', async () => { - const controller = newController(); + const { controller } = setupController(); const externalTransactionToConfirm = { from: ACCOUNT_MOCK, @@ -2831,19 +2949,6 @@ describe('TransactionController', () => { }); it('marks local transactions with the same nonce and chainId as status dropped and defines replacedBy properties', async () => { - const droppedEventListener = jest.fn(); - const changedStatusEventListener = jest.fn(); - const controller = newController({ - options: { - disableHistory: true, - }, - }); - controller.hub.on('transaction-dropped', droppedEventListener); - controller.hub.on( - 'transaction-status-update', - changedStatusEventListener, - ); - const externalTransactionId = '1'; const externalTransactionHash = '0x1'; const externalTransactionToConfirm = { @@ -2857,7 +2962,8 @@ describe('TransactionController', () => { txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, - nonce: String(NONCE_MOCK), + nonce: toHex(NONCE_MOCK), + value: '0x42', }, }; const externalTransactionReceipt = { @@ -2865,19 +2971,41 @@ describe('TransactionController', () => { }; const externalBaseFeePerGas = '0x14'; - // Local unapproved transaction with the same chainId and nonce const localTransactionIdWithSameNonce = '9'; - controller.state.transactions.push({ - id: localTransactionIdWithSameNonce, - chainId: toHex(5), - status: TransactionStatus.unapproved as const, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - nonce: String(NONCE_MOCK), + + const droppedEventListener = jest.fn(); + const statusUpdatedEventListener = jest.fn(); + + const { controller, messenger } = setupController({ + options: { + disableHistory: true, + state: { + transactions: [ + // Local unapproved transaction with the same chainId and nonce + { + id: localTransactionIdWithSameNonce, + chainId: toHex(5), + status: TransactionStatus.unapproved as const, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + to: ACCOUNT_2_MOCK, + nonce: toHex(NONCE_MOCK), + value: '0x42', + }, + }, + ], + }, }, }); + messenger.subscribe( + 'TransactionController:transactionDropped', + droppedEventListener, + ); + messenger.subscribe( + 'TransactionController:transactionStatusUpdated', + statusUpdatedEventListener, + ); await controller.confirmExternalTransaction( externalTransactionToConfirm, @@ -2888,32 +3016,30 @@ describe('TransactionController', () => { const droppedTx = controller.state.transactions.find( (transaction) => transaction.id === localTransactionIdWithSameNonce, ); - + assert(droppedTx, 'Could not find dropped transaction'); const externalTx = controller.state.transactions.find( (transaction) => transaction.id === externalTransactionId, ); - expect(droppedTx?.status).toBe(TransactionStatus.dropped); - - expect(droppedTx?.replacedById).toBe(externalTransactionId); + expect(droppedTx.status).toBe(TransactionStatus.dropped); + expect(droppedTx.replacedById).toBe(externalTransactionId); + expect(droppedTx.replacedBy).toBe(externalTransactionHash); - expect(droppedTx?.replacedBy).toBe(externalTransactionHash); expect(droppedEventListener).toHaveBeenCalledTimes(1); expect(droppedEventListener).toHaveBeenCalledWith({ transactionMeta: droppedTx, }); - expect(changedStatusEventListener).toHaveBeenCalledTimes(2); - expect(changedStatusEventListener.mock.calls[0][0]).toStrictEqual({ + expect(statusUpdatedEventListener).toHaveBeenCalledTimes(2); + expect(statusUpdatedEventListener.mock.calls[0][0]).toStrictEqual({ transactionMeta: droppedTx, }); - expect(changedStatusEventListener.mock.calls[1][0]).toStrictEqual({ + expect(statusUpdatedEventListener.mock.calls[1][0]).toStrictEqual({ transactionMeta: externalTx, }); }); it('doesnt mark transaction as dropped if local transaction with same nonce and chainId has status of failed', async () => { - const controller = newController(); const externalTransactionId = '1'; const externalTransactionHash = '0x1'; const externalTransactionToConfirm = { @@ -2927,7 +3053,7 @@ describe('TransactionController', () => { txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, - nonce: String(NONCE_MOCK), + nonce: toHex(NONCE_MOCK), }, }; const externalTransactionReceipt = { @@ -2935,18 +3061,26 @@ describe('TransactionController', () => { }; const externalBaseFeePerGas = '0x14'; - // Off-chain failed local transaction with the same chainId and nonce const localTransactionIdWithSameNonce = '9'; - controller.state.transactions.push({ - id: localTransactionIdWithSameNonce, - chainId: toHex(5), - status: TransactionStatus.failed as const, - error: new Error('mock error'), - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - nonce: String(NONCE_MOCK), + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + // Off-chain failed local transaction with the same chainId and nonce + id: localTransactionIdWithSameNonce, + chainId: toHex(5), + status: TransactionStatus.failed as const, + error: new Error('mock error'), + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + to: ACCOUNT_2_MOCK, + nonce: toHex(NONCE_MOCK), + }, + }, + ], + }, }, }); @@ -2966,6 +3100,7 @@ describe('TransactionController', () => { expect(failedTx?.replacedBy).toBe(externalTransactionHash); }); + it('updates post transaction balance if type is swap', async () => { const mockPostTxBalance = '7a00'; const mockApprovalTransactionMeta = { @@ -2985,11 +3120,11 @@ describe('TransactionController', () => { }); }, ); - const mockPostTransactionBalanceUpdatedListener = jest.fn(); - const controller = newController(); - controller.hub.on( - 'post-transaction-balance-updated', - mockPostTransactionBalanceUpdatedListener, + const postTransactionBalanceUpdatedListener = jest.fn(); + const { controller, messenger } = setupController(); + messenger.subscribe( + 'TransactionController:postTransactionBalanceUpdated', + postTransactionBalanceUpdatedListener, ); const externalTransactionToConfirm = { @@ -3024,10 +3159,8 @@ describe('TransactionController', () => { await flushPromises(); - expect(mockPostTransactionBalanceUpdatedListener).toHaveBeenCalledTimes( - 1, - ); - expect(mockPostTransactionBalanceUpdatedListener).toHaveBeenCalledWith( + expect(postTransactionBalanceUpdatedListener).toHaveBeenCalledTimes(1); + expect(postTransactionBalanceUpdatedListener).toHaveBeenCalledWith( expect.objectContaining({ transactionMeta: expect.objectContaining({ postTxBalance: mockPostTxBalance, @@ -3039,12 +3172,15 @@ describe('TransactionController', () => { ); }); - it('emits confirmed event', async () => { - const controller = newController(); + it('publishes TransactionController:transactionConfirmed', async () => { + const { controller, messenger } = setupController(); const confirmedEventListener = jest.fn(); - controller.hub.on('transaction-confirmed', confirmedEventListener); + messenger.subscribe( + 'TransactionController:transactionConfirmed', + confirmedEventListener, + ); const externalTransactionToConfirm = { from: ACCOUNT_MOCK, @@ -3071,15 +3207,13 @@ describe('TransactionController', () => { externalBaseFeePerGas, ); - const externalTransaction = controller.state.transactions[0]; - expect(confirmedEventListener).toHaveBeenCalledTimes(1); - expect(confirmedEventListener).toHaveBeenCalledWith({ - transactionMeta: externalTransaction, - }); + expect(confirmedEventListener).toHaveBeenCalledWith( + expect.objectContaining(externalTransactionToConfirm), + ); }); - it('emits confirmed event with transaction chainId regardless of whether it matches globally selected chainId', async () => { + it('publishes TransactionController:transactionConfirmed with transaction chainId regardless of whether it matches globally selected chainId', async () => { const mockGloballySelectedNetwork = { ...MOCK_NETWORK, state: { @@ -3091,13 +3225,15 @@ describe('TransactionController', () => { }, }, }; - const controller = newController({ + const { controller, messenger } = setupController({ network: mockGloballySelectedNetwork, }); const confirmedEventListener = jest.fn(); - - controller.hub.on('transaction-confirmed', confirmedEventListener); + messenger.subscribe( + 'TransactionController:transactionConfirmed', + confirmedEventListener, + ); const externalTransactionToConfirm = { from: ACCOUNT_MOCK, @@ -3124,14 +3260,15 @@ describe('TransactionController', () => { externalBaseFeePerGas, ); - const [[{ transactionMeta }]] = confirmedEventListener.mock.calls; - expect(transactionMeta.chainId).toBe(ChainId.goerli); + expect(confirmedEventListener).toHaveBeenCalledWith( + expect.objectContaining(externalTransactionToConfirm), + ); }); }); describe('updateTransactionSendFlowHistory', () => { it('appends sendFlowHistory entries to transaction meta', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3156,7 +3293,7 @@ describe('TransactionController', () => { }); it('appends sendFlowHistory entries to existing entries in transaction meta', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3193,7 +3330,7 @@ describe('TransactionController', () => { }); it('doesnt append if current sendFlowHistory lengths doesnt match', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3218,7 +3355,7 @@ describe('TransactionController', () => { }); it('throws if sendFlowHistory persistence is disabled', async () => { - const controller = newController({ + const { controller } = setupController({ options: { disableSendFlowHistory: true }, }); const mockSendFlowHistory = [ @@ -3245,7 +3382,7 @@ describe('TransactionController', () => { }); it('throws if transactionMeta is not found', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3265,7 +3402,7 @@ describe('TransactionController', () => { }); it('throws if the transaction is not unapproved status', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3297,7 +3434,7 @@ describe('TransactionController', () => { describe('clearUnapprovedTransactions', () => { it('clears unapproved transactions', async () => { - const controller = newController(); + const { controller } = setupController(); const firstUnapprovedTxId = '1'; const secondUnapprovedTxId = '2'; @@ -3358,7 +3495,7 @@ describe('TransactionController', () => { describe('on incoming transaction helper transactions event', () => { it('adds new transactions to state', async () => { - const controller = newController(); + const { controller } = setupController(); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -3374,12 +3511,13 @@ describe('TransactionController', () => { }); it('updates existing transactions in state', async () => { - const controller = newController(); - - controller.state.transactions = [ - TRANSACTION_META_MOCK, - TRANSACTION_META_2_MOCK, - ]; + const { controller } = setupController({ + options: { + state: { + transactions: [TRANSACTION_META_MOCK, TRANSACTION_META_2_MOCK], + }, + }, + }); const updatedTransaction = { ...TRANSACTION_META_MOCK, @@ -3400,7 +3538,9 @@ describe('TransactionController', () => { }); it('limits max transactions when adding to state', async () => { - const controller = newController({ config: { txHistoryLimit: 1 } }); + const { controller } = setupController({ + options: { transactionHistoryLimit: 1 }, + }); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -3417,7 +3557,7 @@ describe('TransactionController', () => { describe('on incoming transaction helper lastFetchedBlockNumbers event', () => { it('updates state', async () => { - const controller = newController(); + const { controller } = setupController(); const lastFetchedBlockNumbers = { key: 234, @@ -3435,12 +3575,15 @@ describe('TransactionController', () => { ); }); - it('emits incomingTransactionBlock event', async () => { + it('publishes TransactionController:incomingTransactionBlockReceived', async () => { const blockNumber = 123; const listener = jest.fn(); - const controller = newController(); - controller.hub.on('incomingTransactionBlock', listener); + const { messenger } = setupController(); + messenger.subscribe( + 'TransactionController:incomingTransactionBlockReceived', + listener, + ); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -3458,7 +3601,7 @@ describe('TransactionController', () => { describe('updateTransactionGasFees', () => { it('throws if transaction does not exist', async () => { - const controller = newController(); + const { controller } = setupController(); expect(() => controller.updateTransactionGasFees('123', { gasPrice: '0x1', @@ -3470,7 +3613,7 @@ describe('TransactionController', () => { const transactionId = '123'; const fnName = 'updateTransactionGasFees'; const status = TransactionStatus.failed; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ id: transactionId, status, @@ -3490,7 +3633,7 @@ describe('TransactionController', () => { it('updates provided legacy gas values', async () => { const transactionId = '123'; - const controller = newController(); + const { controller } = setupController(); const gas = '0xgas'; const gasLimit = '0xgasLimit'; @@ -3549,7 +3692,7 @@ describe('TransactionController', () => { const maxFeePerGas = '0xmaxFeePerGas'; const transactionId = '123'; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ id: transactionId, @@ -3586,7 +3729,7 @@ describe('TransactionController', () => { describe('updatePreviousGasParams', () => { it('throws if transaction does not exist', async () => { - const controller = newController(); + const { controller } = setupController(); expect(() => controller.updatePreviousGasParams('123', { maxFeePerGas: '0x1', @@ -3598,7 +3741,7 @@ describe('TransactionController', () => { const transactionId = '123'; const fnName = 'updatePreviousGasParams'; const status = TransactionStatus.failed; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ id: transactionId, status, @@ -3616,7 +3759,7 @@ describe('TransactionController', () => { it('updates previous gas values', async () => { const transactionId = '123'; - const controller = newController(); + const { controller } = setupController(); const gasLimit = '0xgasLimit'; const maxFeePerGas = '0xmaxFeePerGas'; @@ -3672,10 +3815,16 @@ describe('TransactionController', () => { it('bubbles event', async () => { const listener = jest.fn(); const statusUpdateListener = jest.fn(); - const controller = newController(); + const { messenger } = setupController(); - controller.hub.on(`${TRANSACTION_META_MOCK.id}:confirmed`, listener); - controller.hub.on(`transaction-status-update`, statusUpdateListener); + messenger.subscribe( + 'TransactionController:transactionConfirmed', + listener, + ); + messenger.subscribe( + 'TransactionController:transactionStatusUpdated', + statusUpdateListener, + ); firePendingTransactionTrackerEvent( 'transaction-confirmed', @@ -3692,8 +3841,6 @@ describe('TransactionController', () => { }); it('marks duplicate nonce transactions as dropped', async () => { - const controller = newController(); - const confirmed = { ...TRANSACTION_META_MOCK, id: 'testId1', @@ -3747,16 +3894,22 @@ describe('TransactionController', () => { type: TransactionType.incoming, }; - controller.state.transactions = [ - confirmed, - wrongChain, - wrongNonce, - wrongFrom, - wrongType, - duplicate_1, - duplicate_2, - duplicate_3, - ] as TransactionMeta[]; + const { controller } = setupController({ + options: { + state: { + transactions: [ + confirmed, + wrongChain, + wrongNonce, + wrongFrom, + wrongType, + duplicate_1, + duplicate_2, + duplicate_3, + ] as TransactionMeta[], + }, + }, + }); firePendingTransactionTrackerEvent('transaction-confirmed', confirmed); @@ -3802,12 +3955,15 @@ describe('TransactionController', () => { }); it('sets status to dropped on transaction-dropped event', async () => { - const controller = newController({ - options: { disableHistory: true }, + const { controller } = setupController({ + options: { + disableHistory: true, + state: { + transactions: [{ ...TRANSACTION_META_MOCK }], + }, + }, }); - controller.state.transactions = [{ ...TRANSACTION_META_MOCK }]; - firePendingTransactionTrackerEvent( 'transaction-dropped', TRANSACTION_META_MOCK, @@ -3819,13 +3975,18 @@ describe('TransactionController', () => { }); it('sets status to failed on transaction-failed event', async () => { - const changedStatusEventListener = jest.fn(); - const controller = newController({ - options: { disableHistory: true }, + const statusUpdatedEventListener = jest.fn(); + const { controller, messenger } = setupController({ + options: { + disableHistory: true, + state: { + transactions: [{ ...TRANSACTION_META_MOCK }], + }, + }, }); - controller.hub.on( - 'transaction-status-update', - changedStatusEventListener, + messenger.subscribe( + 'TransactionController:transactionStatusUpdated', + statusUpdatedEventListener, ); const errorMock = new Error('TestError'); @@ -3837,8 +3998,6 @@ describe('TransactionController', () => { rpc: undefined, }; - controller.state.transactions = [{ ...TRANSACTION_META_MOCK }]; - firePendingTransactionTrackerEvent( 'transaction-failed', TRANSACTION_META_MOCK, @@ -3853,19 +4012,22 @@ describe('TransactionController', () => { expect(controller.state.transactions[0]).toStrictEqual(failedTx); - expect(changedStatusEventListener).toHaveBeenCalledTimes(1); - expect(changedStatusEventListener).toHaveBeenCalledWith({ + expect(statusUpdatedEventListener).toHaveBeenCalledTimes(1); + expect(statusUpdatedEventListener).toHaveBeenCalledWith({ transactionMeta: failedTx, }); }); it('updates transaction on transaction-updated event', async () => { - const controller = newController({ - options: { disableHistory: true }, + const { controller } = setupController({ + options: { + state: { + transactions: [TRANSACTION_META_MOCK], + }, + disableHistory: true, + }, }); - controller.state.transactions = [{ ...TRANSACTION_META_MOCK }]; - firePendingTransactionTrackerEvent( 'transaction-updated', { ...TRANSACTION_META_MOCK, retryCount: 123 }, @@ -3883,8 +4045,8 @@ describe('TransactionController', () => { describe('approveTransactionsWithSameNonce', () => { it('throws error if no sign method', async () => { - const controller = newController({ - config: { + const { controller } = setupController({ + options: { sign: undefined, }, }); @@ -3903,14 +4065,14 @@ describe('TransactionController', () => { }); it('returns empty string if no transactions are provided', async () => { - const controller = newController(); + const { controller } = setupController(); const result = await controller.approveTransactionsWithSameNonce([]); expect(result).toBe(''); }); it('return empty string if transaction is already being signed', async () => { - const controller = newController({ - config: { + const { controller } = setupController({ + options: { // We never resolve this promise, so the transaction is always in the process of being signed sign: async () => new Promise(() => { @@ -3944,8 +4106,8 @@ describe('TransactionController', () => { .mockImplementation(async (transactionParams) => Promise.resolve(TransactionFactory.fromTxData(transactionParams)), ); - const controller = newController({ - config: { + const { controller } = setupController({ + options: { sign: signMock, }, }); @@ -3983,8 +4145,8 @@ describe('TransactionController', () => { .mockImplementation(async () => Promise.reject(new Error(mockSignError)), ); - const controller = newController({ - config: { + const { controller } = setupController({ + options: { sign: signMock, }, }); @@ -4014,7 +4176,7 @@ describe('TransactionController', () => { }); it('does not create nonce lock if hasNonce set', async () => { - const controller = newController(); + const { controller } = setupController(); const mockTransactionParam = { from: ACCOUNT_MOCK, @@ -4043,7 +4205,11 @@ describe('TransactionController', () => { }); it('uses the nonceTracker for the networkClientId matching the chainId', async () => { - const controller = newController(); + const { controller, messenger } = setupController(); + messenger.registerActionHandler( + 'NetworkController:findNetworkClientIdByChainId', + () => 'goerli', + ); const mockTransactionParam = { from: ACCOUNT_MOCK, @@ -4073,17 +4239,21 @@ describe('TransactionController', () => { }); describe('with hooks', () => { - const paramsMock = { + const paramsMock: TransactionParams = { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }; - const metadataMock = { + const metadataMock: TransactionMeta = { txParams: paramsMock, + chainId: '0x1' as const, + id: '1', + time: 0, + status: TransactionStatus.approved, }; it('adds a transaction, signs and update status to `approved`', async () => { - const controller = newController({ + const { controller, mockTransactionApprovalRequest } = setupController({ options: { hooks: { afterSign: () => false, @@ -4093,9 +4263,7 @@ describe('TransactionController', () => { }, }, }); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const signSpy = jest.spyOn(controller, 'sign' as any); + const signSpy = jest.spyOn(controller, 'sign'); const updateTransactionSpy = jest.spyOn(controller, 'updateTransaction'); await controller.addTransaction(paramsMock, { @@ -4103,7 +4271,9 @@ describe('TransactionController', () => { actionId: ACTION_ID_MOCK, }); - approveTransaction(); + mockTransactionApprovalRequest.approve({ + value: TRANSACTION_META_MOCK, + }); await wait(0); const transactionMeta = controller.state.transactions[0]; @@ -4128,7 +4298,7 @@ describe('TransactionController', () => { }); it('adds a transaction and signing returns undefined', async () => { - const controller = newController({ + const { controller, mockTransactionApprovalRequest } = setupController({ options: { hooks: { afterSign: () => false, @@ -4136,12 +4306,11 @@ describe('TransactionController', () => { beforePublish: () => false, getAdditionalSignArguments: () => [metadataMock], }, + // @ts-expect-error sign intentionally returns undefined + sign: async () => undefined, }, - config: { sign: async () => undefined }, }); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const signSpy = jest.spyOn(controller, 'sign' as any); + const signSpy = jest.spyOn(controller, 'sign'); const updateTransactionSpy = jest.spyOn(controller, 'updateTransaction'); await controller.addTransaction(paramsMock, { @@ -4149,7 +4318,9 @@ describe('TransactionController', () => { actionId: ACTION_ID_MOCK, }); - approveTransaction(); + mockTransactionApprovalRequest.approve({ + value: TRANSACTION_META_MOCK, + }); await wait(0); expect(signSpy).toHaveBeenCalledTimes(1); @@ -4157,7 +4328,7 @@ describe('TransactionController', () => { }); it('adds a transaction, signs and skips publish the transaction', async () => { - const controller = newController({ + const { controller, mockTransactionApprovalRequest } = setupController({ options: { hooks: { beforePublish: undefined, @@ -4166,9 +4337,7 @@ describe('TransactionController', () => { }, }, }); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const signSpy = jest.spyOn(controller, 'sign' as any); + const signSpy = jest.spyOn(controller, 'sign'); const updateTransactionSpy = jest.spyOn(controller, 'updateTransaction'); await controller.addTransaction(paramsMock, { @@ -4176,7 +4345,7 @@ describe('TransactionController', () => { actionId: ACTION_ID_MOCK, }); - approveTransaction(); + mockTransactionApprovalRequest.approve(); await wait(0); expect(signSpy).toHaveBeenCalledTimes(1); @@ -4197,7 +4366,7 @@ describe('TransactionController', () => { }); it('gets transaction hash from publish hook and does not submit to provider', async () => { - const controller = newController({ + const { controller } = setupController({ options: { hooks: { publish: async () => ({ @@ -4205,25 +4374,41 @@ describe('TransactionController', () => { }), }, }, - approve: true, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); + jest.spyOn(mockEthQuery, 'sendRawTransaction'); const { result } = await controller.addTransaction(paramsMock); await result; expect(controller.state.transactions[0].hash).toBe('0x123'); - expect(mockSendRawTransaction).not.toHaveBeenCalled(); + expect(mockEthQuery.sendRawTransaction).not.toHaveBeenCalled(); }); it('submits to provider if publish hook returns no transaction hash', async () => { - const controller = newController({ + jest + .spyOn(mockEthQuery, 'sendRawTransaction') + .mockImplementation((_transaction, callback) => { + callback(undefined, 'some-transaction-hash'); + }); + const { controller } = setupController({ options: { hooks: { + // @ts-expect-error We are intentionally having this hook return no + // transaction hash publish: async () => ({}), }, }, - approve: true, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); const { result } = await controller.addTransaction(paramsMock); @@ -4231,10 +4416,10 @@ describe('TransactionController', () => { await result; expect(controller.state.transactions[0].hash).toBe( - ethQueryMockResults.sendRawTransaction, + 'some-transaction-hash', ); - expect(mockSendRawTransaction).toHaveBeenCalledTimes(1); + expect(mockEthQuery.sendRawTransaction).toHaveBeenCalledTimes(1); }); }); @@ -4248,22 +4433,16 @@ describe('TransactionController', () => { ]; it('add securityAlertResponse to transaction meta', async () => { - const transactionMetaId = '123'; - const status = TransactionStatus.submitted; - const controller = newController(); - controller.state.transactions.push({ - id: transactionMetaId, - status, - txParams: { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, + const transactionMeta = TRANSACTION_META_MOCK; + const { controller } = setupController({ + options: { + state: { + transactions: [transactionMeta], + }, }, - history: mockSendFlowHistory, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); - expect(controller.state.transactions[0]).toBeDefined(); - controller.updateSecurityAlertResponse(transactionMetaId, { + }); + + controller.updateSecurityAlertResponse(transactionMeta.id, { reason: 'NA', result_type: 'Benign', }); @@ -4274,21 +4453,17 @@ describe('TransactionController', () => { }); it('should throw error if transactionMetaId is not defined', async () => { - const transactionMetaId = '123'; - const status = TransactionStatus.submitted; - const controller = newController(); - controller.state.transactions.push({ - id: transactionMetaId, - status, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); - expect(controller.state.transactions[0]).toBeDefined(); + const { controller } = setupController({ + options: { + state: { + transactions: [TRANSACTION_META_MOCK], + }, + }, + }); expect(() => - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - controller.updateSecurityAlertResponse(undefined as any, { + // @ts-expect-error Intentionally passing invalid input + controller.updateSecurityAlertResponse(undefined, { reason: 'NA', result_type: 'Benign', }), @@ -4300,7 +4475,7 @@ describe('TransactionController', () => { it('should throw error if securityAlertResponse is not defined', async () => { const transactionMetaId = '123'; const status = TransactionStatus.submitted; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ id: transactionMetaId, status, @@ -4324,7 +4499,7 @@ describe('TransactionController', () => { it('should throw error if transaction with given id does not exist', async () => { const transactionMetaId = '123'; const status = TransactionStatus.submitted; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ id: transactionMetaId, status, @@ -4349,23 +4524,30 @@ describe('TransactionController', () => { }); describe('updateCustodialTransaction', () => { - const transactionId = '1'; - const statusMock = TransactionStatus.unapproved as const; - const baseTransaction = { - id: transactionId, - chainId: toHex(5), - status: statusMock, - time: 123456789, - txParams: { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - }, - }; - const transactionMeta: TransactionMeta = { - ...baseTransaction, - custodyId: '123', - history: [{ ...baseTransaction }], - }; + let transactionId: string; + let statusMock: TransactionStatus; + let baseTransaction: TransactionMeta; + let transactionMeta: TransactionMeta; + + beforeEach(() => { + transactionId = '1'; + statusMock = TransactionStatus.unapproved as const; + baseTransaction = { + id: transactionId, + chainId: toHex(5), + status: statusMock, + time: 123456789, + txParams: { + from: ACCOUNT_MOCK, + to: ACCOUNT_2_MOCK, + }, + }; + transactionMeta = { + ...baseTransaction, + custodyId: '123', + history: [{ ...baseTransaction }], + }; + }); it.each([ { @@ -4381,8 +4563,13 @@ describe('TransactionController', () => { ])( 'updates transaction status to $newStatus', async ({ newStatus, errorMessage }) => { - const controller = newController(); - controller.state.transactions.push(transactionMeta); + const { controller } = setupController({ + options: { + state: { + transactions: [transactionMeta], + }, + }, + }); controller.updateCustodialTransaction(transactionId, { status: newStatus, @@ -4404,13 +4591,20 @@ describe('TransactionController', () => { errorMessage: 'Error mock', }, ])( - 'emits txId:finished when update transaction status to $newStatus', + 'publishes TransactionController:transactionFinished when update transaction status to $newStatus', async ({ newStatus, errorMessage }) => { - const controller = newController(); - const hubEmitSpy = jest - .spyOn(controller.hub, 'emit') - .mockImplementation(); - controller.state.transactions.push(transactionMeta); + const finishedEventListener = jest.fn(); + const { controller, messenger } = setupController({ + options: { + state: { + transactions: [transactionMeta], + }, + }, + }); + messenger.subscribe( + 'TransactionController:transactionFinished', + finishedEventListener, + ); controller.updateCustodialTransaction(transactionId, { status: newStatus, @@ -4419,22 +4613,26 @@ describe('TransactionController', () => { const updatedTransaction = controller.state.transactions[0]; - expect(hubEmitSpy).toHaveBeenCalledTimes(1); - expect(hubEmitSpy).toHaveBeenCalledWith( - `${transactionId}:finished`, - updatedTransaction, + expect(finishedEventListener).toHaveBeenCalledTimes(1); + expect(finishedEventListener).toHaveBeenCalledWith( + expect.objectContaining({ + ...transactionMeta, + status: newStatus, + }), ); - expect(updatedTransaction?.status).toStrictEqual(newStatus); + expect(updatedTransaction.status).toStrictEqual(newStatus); }, ); it('updates transaction hash', async () => { const newHash = '1234'; - const controller = newController(); - const hubEmitSpy = jest - .spyOn(controller.hub, 'emit') - .mockImplementation(); - controller.state.transactions.push(transactionMeta); + const { controller } = setupController({ + options: { + state: { + transactions: [transactionMeta], + }, + }, + }); controller.updateCustodialTransaction(transactionId, { hash: newHash, @@ -4442,14 +4640,13 @@ describe('TransactionController', () => { const updatedTransaction = controller.state.transactions[0]; - expect(hubEmitSpy).toHaveBeenCalledTimes(0); - expect(updatedTransaction?.hash).toStrictEqual(newHash); + expect(updatedTransaction.hash).toStrictEqual(newHash); }); it('throws if custodial transaction does not exists', async () => { const nonExistentId = 'nonExistentId'; const newStatus = TransactionStatus.approved as const; - const controller = newController(); + const { controller } = setupController(); expect(() => controller.updateCustodialTransaction(nonExistentId, { @@ -4466,7 +4663,7 @@ describe('TransactionController', () => { history: [{ ...baseTransaction }], }; const newStatus = TransactionStatus.approved as const; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push(nonCustodialTransaction); expect(() => @@ -4478,7 +4675,7 @@ describe('TransactionController', () => { it('throws if status is invalid', async () => { const newStatus = TransactionStatus.approved as const; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push(transactionMeta); expect(() => @@ -4491,17 +4688,20 @@ describe('TransactionController', () => { }); it('no property was updated', async () => { - const controller = newController(); - controller.state.transactions.push(transactionMeta); + const { controller } = setupController({ + options: { + state: { + transactions: [transactionMeta], + }, + }, + }); controller.updateCustodialTransaction(transactionId, {}); const updatedTransaction = controller.state.transactions[0]; - expect(updatedTransaction?.status).toStrictEqual( - transactionMeta.status, - ); - expect(updatedTransaction?.hash).toStrictEqual(transactionMeta.hash); + expect(updatedTransaction.status).toStrictEqual(transactionMeta.status); + expect(updatedTransaction.hash).toStrictEqual(transactionMeta.hash); }); }); @@ -4542,17 +4742,20 @@ describe('TransactionController', () => { lastFetchedBlockNumbers: {}, }; - const controller = newController({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - state: mockedControllerState as any, + const { controller, messenger } = setupController({ + options: { + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + state: mockedControllerState as any, + }, }); + jest.spyOn(messenger, 'call'); controller.initApprovals(); await flushPromises(); - expect(messengerMock.call).toHaveBeenCalledTimes(2); - expect(messengerMock.call).toHaveBeenCalledWith( + expect(messenger.call).toHaveBeenCalledTimes(2); + expect(messenger.call).toHaveBeenCalledWith( 'ApprovalController:addRequest', { expectsResult: true, @@ -4563,7 +4766,7 @@ describe('TransactionController', () => { }, false, ); - expect(messengerMock.call).toHaveBeenCalledWith( + expect(messenger.call).toHaveBeenCalledWith( 'ApprovalController:addRequest', { expectsResult: true, @@ -4616,11 +4819,13 @@ describe('TransactionController', () => { .fn() .mockReturnValue(MOCK_NETWORK.state); - const controller = newController({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - state: mockedControllerState as any, - options: { getNetworkState: getNetworkStateMock }, + const { controller } = setupController({ + options: { + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + state: mockedControllerState as any, + getNetworkState: getNetworkStateMock, + }, }); controller.initApprovals(); @@ -4659,18 +4864,18 @@ describe('TransactionController', () => { lastFetchedBlockNumbers: {}, }; - const controller = newController({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - state: mockedControllerState as any, - }); - const mockedErrorMessage = 'mocked error'; + const { controller, messenger } = setupController({ + options: { + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + state: mockedControllerState as any, + }, + }); // Expect both calls to throw error, one with code property to check if it is handled - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (messengerMock.call as jest.MockedFunction) + jest + .spyOn(messenger, 'call') .mockImplementationOnce(() => { // eslint-disable-next-line @typescript-eslint/no-throw-literal throw { message: mockedErrorMessage }; @@ -4693,21 +4898,20 @@ describe('TransactionController', () => { 'Error during persisted transaction approval', new Error(mockedErrorMessage), ); - expect(messengerMock.call).toHaveBeenCalledTimes(2); + expect(messenger.call).toHaveBeenCalledTimes(2); }); it('does not create any approval when there is no unapproved transaction', async () => { - const controller = newController(); + const { controller, messenger } = setupController(); + jest.spyOn(messenger, 'call'); controller.initApprovals(); await flushPromises(); - expect(messengerMock.call).not.toHaveBeenCalled(); + expect(messenger.call).not.toHaveBeenCalled(); }); }); describe('getTransactions', () => { it('returns transactions matching values in search criteria', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4732,7 +4936,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4743,8 +4951,6 @@ describe('TransactionController', () => { }); it('returns transactions matching param values in search criteria', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4769,7 +4975,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4780,8 +4990,6 @@ describe('TransactionController', () => { }); it('returns transactions matching multiple values in search criteria', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4806,7 +5014,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4817,8 +5029,6 @@ describe('TransactionController', () => { }); it('returns transactions matching function in search criteria', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4843,7 +5053,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4856,8 +5070,6 @@ describe('TransactionController', () => { }); it('returns transactions matching current network', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: MOCK_NETWORK.state.providerConfig.chainId, @@ -4882,7 +5094,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4892,7 +5108,7 @@ describe('TransactionController', () => { }); it('returns transactions from specified list', () => { - const controller = newController(); + const { controller } = setupController(); const transactions: TransactionMeta[] = [ { @@ -4928,8 +5144,6 @@ describe('TransactionController', () => { }); it('returns limited number of transactions sorted by ascending time', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4961,7 +5175,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4973,8 +5191,6 @@ describe('TransactionController', () => { }); it('returns limited number of transactions except for duplicate nonces', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -5007,7 +5223,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -5051,7 +5271,7 @@ describe('TransactionController', () => { }; it('updates editable params and returns updated transaction metadata', async () => { - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push(transactionMeta); const updatedTransaction = await controller.updateEditableParams( @@ -5063,7 +5283,7 @@ describe('TransactionController', () => { }); it('throws an error if no transaction metadata is found', async () => { - const controller = newController(); + const { controller } = setupController(); await expect( controller.updateEditableParams(transactionId, params), ).rejects.toThrow( @@ -5072,7 +5292,7 @@ describe('TransactionController', () => { }); it('throws an error if the transaction is not unapproved', async () => { - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ ...transactionMeta, status: TransactionStatus.submitted as const, @@ -5086,7 +5306,7 @@ describe('TransactionController', () => { describe('abortTransactionSigning', () => { it('throws if transaction does not exist', () => { - const controller = newController(); + const { controller } = setupController(); expect(() => controller.abortTransactionSigning(TRANSACTION_META_MOCK.id), @@ -5094,9 +5314,13 @@ describe('TransactionController', () => { }); it('throws if transaction not being signed', () => { - const controller = newController(); - - controller.state.transactions = [TRANSACTION_META_MOCK]; + const { controller } = setupController({ + options: { + state: { + transactions: [TRANSACTION_META_MOCK], + }, + }, + }); expect(() => controller.abortTransactionSigning(TRANSACTION_META_MOCK.id), @@ -5106,11 +5330,15 @@ describe('TransactionController', () => { }); it('sets status to failed if transaction being signed', async () => { - const controller = newController({ - approve: true, - config: { + const { controller } = setupController({ + options: { sign: jest.fn().mockReturnValue(createDeferredPromise().promise), }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); const { transactionMeta, result } = await controller.addTransaction({ diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index b4fb6573f4..8634d24f26 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -8,11 +8,11 @@ import type { AddResult, } from '@metamask/approval-controller'; import type { - BaseConfig, - BaseState, + ControllerGetStateAction, + ControllerStateChangeEvent, RestrictedControllerMessenger, } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; import { query, NetworkType, @@ -39,7 +39,7 @@ import { add0x } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { MethodRegistry } from 'eth-method-registry'; import { EventEmitter } from 'events'; -import { mapValues, merge, pickBy, sortBy } from 'lodash'; +import { cloneDeep, mapValues, merge, pickBy, sortBy } from 'lodash'; import { NonceTracker } from 'nonce-tracker'; import type { NonceLock, @@ -57,7 +57,6 @@ import { MultichainTrackingHelper } from './helpers/MultichainTrackingHelper'; import { PendingTransactionTracker } from './helpers/PendingTransactionTracker'; import { projectLogger as log } from './logger'; import type { - Events, DappSuggestedGasFees, SavedGasFees, SecurityProviderRequest, @@ -107,6 +106,25 @@ import { validateTxParams, } from './utils/validation'; +/** + * Metadata for the TransactionController state, describing how to "anonymize" + * the state and which parts should be persisted. + */ +const metadata = { + transactions: { + persist: true, + anonymous: false, + }, + methodData: { + persist: true, + anonymous: false, + }, + lastFetchedBlockNumbers: { + persist: true, + anonymous: false, + }, +}; + export const HARDFORK = Hardfork.London; /** @@ -139,50 +157,43 @@ export interface FeeMarketEIP1559Values { maxPriorityFeePerGas: string; } -/** - * Transaction controller configuration - * - * @property provider - Provider used to create a new underlying EthQuery instance - * @property sign - Method used to sign transactions - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TransactionConfig extends BaseConfig { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sign?: (txParams: TransactionParams, from: string) => Promise; - txHistoryLimit: number; -} - /** * Method data registry object * * @property registryMethod - Registry method raw string * @property parsedRegistryMethod - Registry method object, containing name and method arguments */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface MethodData { +export type MethodData = { registryMethod: string; - parsedRegistryMethod: Record; -} + parsedRegistryMethod: + | { + name: string; + args: { type: string }[]; + } + | { + // We're using `any` instead of `undefined` for compatibility with `Json` + // TODO: Correct this type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + name?: any; + // We're using `any` instead of `undefined` for compatibility with `Json` + // TODO: Correct this type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + args?: any; + }; +}; /** * Transaction controller state * * @property transactions - A list of TransactionMeta objects * @property methodData - Object containing all known method data information + * @property lastFetchedBlockNumbers - Last fetched block numbers. */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TransactionState extends BaseState { +export type TransactionControllerState = { transactions: TransactionMeta[]; - methodData: { [key: string]: MethodData }; + methodData: Record; lastFetchedBlockNumbers: { [key: string]: number }; -} +}; /** * Multiplier used to determine a transaction's increased gas fee during cancellation @@ -194,6 +205,19 @@ export const CANCEL_RATE = 1.1; */ export const SPEED_UP_RATE = 1.1; +/** + * Represents the `TransactionController:getState` action. + */ +export type TransactionControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + TransactionControllerState +>; + +/** + * The internal actions available to the TransactionController. + */ +export type TransactionControllerActions = TransactionControllerGetStateAction; + /** * Configuration options for the PendingTransactionTracker * @@ -210,7 +234,6 @@ export type PendingTransactionOptions = { * @property disableHistory - Whether to disable storing history in transaction metadata. * @property disableSendFlowHistory - Explicitly disable transaction metadata history. * @property disableSwaps - Whether to disable additional processing on swaps transactions. - * @property isMultichainEnabled - Enable multichain support. * @property getCurrentAccountEIP1559Compatibility - Whether or not the account supports EIP-1559. * @property getCurrentNetworkEIP1559Compatibility - Whether or not the network supports EIP-1559. * @property getExternalPendingTransactions - Callback to retrieve pending transactions from external sources. @@ -221,11 +244,15 @@ export type PendingTransactionOptions = { * @property getSavedGasFees - Gets the saved gas fee config. * @property getSelectedAddress - Gets the address of the currently selected account. * @property incomingTransactions - Configuration options for incoming transaction support. + * @property isMultichainEnabled - Enable multichain support. * @property messenger - The controller messenger. * @property onNetworkStateChange - Allows subscribing to network controller state changes. * @property pendingTransactions - Configuration options for pending transaction support. * @property provider - The provider used to create the underlying EthQuery instance. * @property securityProviderRequest - A function for verifying a transaction, whether it is malicious or not. + * @property sign - Function used to sign transactions. + * @property state - Initial state to set on this controller. + * @property transactionHistoryLimit - Transaction history limit. * @property hooks - The controller hooks. * @property hooks.afterSign - Additional logic to execute after signing a transaction. Return false to not change the status to signed. * @property hooks.beforeApproveOnInit - Additional logic to execute before starting an approval flow for a transaction during initialization. Return false to skip the transaction. @@ -246,18 +273,25 @@ export type TransactionControllerOptions = { chainId?: string, ) => NonceTrackerTransaction[]; getGasFeeEstimates?: () => Promise; + getNetworkClientRegistry: NetworkController['getNetworkClientRegistry']; getNetworkState: () => NetworkState; getPermittedAccounts: (origin?: string) => Promise; getSavedGasFees?: (chainId: Hex) => SavedGasFees | undefined; getSelectedAddress: () => string; incomingTransactions?: IncomingTransactionOptions; + isMultichainEnabled: boolean; messenger: TransactionControllerMessenger; onNetworkStateChange: (listener: (state: NetworkState) => void) => void; pendingTransactions?: PendingTransactionOptions; provider: Provider; securityProviderRequest?: SecurityProviderRequest; - getNetworkClientRegistry: NetworkController['getNetworkClientRegistry']; - isMultichainEnabled: boolean; + sign?: ( + transaction: TypedTransaction, + from: string, + transactionMeta?: TransactionMeta, + ) => Promise; + state?: Partial; + transactionHistoryLimit: number; hooks: { afterSign?: ( transactionMeta: TransactionMeta, @@ -285,43 +319,238 @@ const controllerName = 'TransactionController'; /** * The external actions available to the {@link TransactionController}. */ -type AllowedActions = +export type AllowedActions = | AddApprovalRequest | NetworkControllerFindNetworkClientIdByChainIdAction | NetworkControllerGetNetworkClientByIdAction; -type AllowedEvents = NetworkControllerStateChangeEvent; +/** + * The external events available to the {@link TransactionController}. + */ +export type AllowedEvents = NetworkControllerStateChangeEvent; + +/** + * Represents the `TransactionController:stateChange` event. + */ +export type TransactionControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + TransactionControllerState +>; + +/** + * Represents the `TransactionController:incomingTransactionBlockReceived` event. + */ +export type TransactionControllerIncomingTransactionBlockReceivedEvent = { + type: `${typeof controllerName}:incomingTransactionBlockReceived`; + payload: [blockNumber: number]; +}; + +/** + * Represents the `TransactionController:postTransactionBalanceUpdated` event. + */ +export type TransactionControllerPostTransactionBalanceUpdatedEvent = { + type: `${typeof controllerName}:postTransactionBalanceUpdated`; + payload: [ + { + transactionMeta: TransactionMeta; + approvalTransactionMeta?: TransactionMeta; + }, + ]; +}; + +/** + * Represents the `TransactionController:speedUpTransactionAdded` event. + */ +export type TransactionControllerSpeedupTransactionAddedEvent = { + type: `${typeof controllerName}:speedupTransactionAdded`; + payload: [transactionMeta: TransactionMeta]; +}; + +/** + * Represents the `TransactionController:transactionApproved` event. + */ +export type TransactionControllerTransactionApprovedEvent = { + type: `${typeof controllerName}:transactionApproved`; + payload: [ + { + transactionMeta: TransactionMeta; + actionId?: string; + }, + ]; +}; + +/** + * Represents the `TransactionController:transactionConfirmed` event. + */ +export type TransactionControllerTransactionConfirmedEvent = { + type: `${typeof controllerName}:transactionConfirmed`; + payload: [transactionMeta: TransactionMeta]; +}; + +/** + * Represents the `TransactionController:transactionDropped` event. + */ +export type TransactionControllerTransactionDroppedEvent = { + type: `${typeof controllerName}:transactionDropped`; + payload: [{ transactionMeta: TransactionMeta }]; +}; + +/** + * Represents the `TransactionController:transactionFailed` event. + */ +export type TransactionControllerTransactionFailedEvent = { + type: `${typeof controllerName}:transactionFailed`; + payload: [ + { + actionId?: string; + error: string; + transactionMeta: TransactionMeta; + }, + ]; +}; + +/** + * Represents the `TransactionController:transactionFinished` event. + */ +export type TransactionControllerTransactionFinishedEvent = { + type: `${typeof controllerName}:transactionFinished`; + payload: [transactionMeta: TransactionMeta]; +}; + +/** + * Represents the `TransactionController:transactionNewSwapApproval` event. + */ +export type TransactionControllerTransactionNewSwapApprovalEvent = { + type: `${typeof controllerName}:transactionNewSwapApproval`; + payload: [{ transactionMeta: TransactionMeta }]; +}; + +/** + * Represents the `TransactionController:transactionNewSwap` event. + */ +export type TransactionControllerTransactionNewSwapEvent = { + type: `${typeof controllerName}:transactionNewSwap`; + payload: [{ transactionMeta: TransactionMeta }]; +}; + +/** + * Represents the `TransactionController:transactionPublishingSkipped` event. + */ +export type TransactionControllerTransactionPublishingSkipped = { + type: `${typeof controllerName}:transactionPublishingSkipped`; + payload: [transactionMeta: TransactionMeta]; +}; + +/** + * Represents the `TransactionController:transactionRejected` event. + */ +export type TransactionControllerTransactionRejectedEvent = { + type: `${typeof controllerName}:transactionRejected`; + payload: [ + { + transactionMeta: TransactionMeta; + actionId?: string; + }, + ]; +}; + +/** + * Represents the `TransactionController:transactionStatusUpdated` event. + */ +export type TransactionControllerTransactionStatusUpdatedEvent = { + type: `${typeof controllerName}:transactionStatusUpdated`; + payload: [ + { + transactionMeta: TransactionMeta; + }, + ]; +}; + +/** + * Represents the `TransactionController:transactionSubmitted` event. + */ +export type TransactionControllerTransactionSubmittedEvent = { + type: `${typeof controllerName}:transactionSubmitted`; + payload: [ + { + transactionMeta: TransactionMeta; + actionId?: string; + }, + ]; +}; + +/** + * Represents the `TransactionController:unapprovedTransactionAdded` event. + */ +export type TransactionControllerUnapprovedTransactionAddedEvent = { + type: `${typeof controllerName}:unapprovedTransactionAdded`; + payload: [transactionMeta: TransactionMeta]; +}; + +/** + * The internal events available to the {@link TransactionController}. + */ +export type TransactionControllerEvents = + | TransactionControllerIncomingTransactionBlockReceivedEvent + | TransactionControllerPostTransactionBalanceUpdatedEvent + | TransactionControllerSpeedupTransactionAddedEvent + | TransactionControllerStateChangeEvent + | TransactionControllerTransactionApprovedEvent + | TransactionControllerTransactionConfirmedEvent + | TransactionControllerTransactionDroppedEvent + | TransactionControllerTransactionFailedEvent + | TransactionControllerTransactionFinishedEvent + | TransactionControllerTransactionNewSwapApprovalEvent + | TransactionControllerTransactionNewSwapEvent + | TransactionControllerTransactionPublishingSkipped + | TransactionControllerTransactionRejectedEvent + | TransactionControllerTransactionStatusUpdatedEvent + | TransactionControllerTransactionSubmittedEvent + | TransactionControllerUnapprovedTransactionAddedEvent; /** * The messenger of the {@link TransactionController}. */ export type TransactionControllerMessenger = RestrictedControllerMessenger< typeof controllerName, - AllowedActions, - AllowedEvents, + TransactionControllerActions | AllowedActions, + TransactionControllerEvents | AllowedEvents, AllowedActions['type'], AllowedEvents['type'] >; -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TransactionControllerEventEmitter extends EventEmitter { - on( - eventName: T, - listener: (...args: Events[T]) => void, - ): this; +/** + * Possible states of the approve transaction step. + */ +export enum ApprovalState { + Approved = 'approved', + NotApproved = 'not-approved', + SkippedViaBeforePublishHook = 'skipped-via-before-publish-hook', +} - emit(eventName: T, ...args: Events[T]): boolean; +/** + * Get the default TransactionsController state. + * + * @returns The default TransactionsController state. + */ +function getDefaultTransactionControllerState(): TransactionControllerState { + return { + methodData: {}, + transactions: [], + lastFetchedBlockNumbers: {}, + }; } /** * Controller responsible for submitting and managing transactions. */ -export class TransactionController extends BaseControllerV1< - TransactionConfig, - TransactionState +export class TransactionController extends BaseController< + typeof controllerName, + TransactionControllerState, + TransactionControllerMessenger > { + #internalEvents = new EventEmitter(); + private readonly isHistoryDisabled: boolean; private readonly isSwapsDisabled: boolean; @@ -332,9 +561,7 @@ export class TransactionController extends BaseControllerV1< private readonly nonceTracker: NonceTracker; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private readonly registry: any; + private readonly registry: MethodRegistry; private readonly mutex = new Mutex(); @@ -361,8 +588,6 @@ export class TransactionController extends BaseControllerV1< chainId?: string, ) => NonceTrackerTransaction[]; - private readonly messagingSystem: TransactionControllerMessenger; - readonly #incomingTransactionOptions: IncomingTransactionOptions; private readonly incomingTransactionHelper: IncomingTransactionHelper; @@ -375,6 +600,8 @@ export class TransactionController extends BaseControllerV1< private readonly signAbortCallbacks: Map void> = new Map(); + #transactionHistoryLimit: number; + private readonly afterSign: ( transactionMeta: TransactionMeta, signedTx: TypedTransaction, @@ -404,12 +631,11 @@ export class TransactionController extends BaseControllerV1< error: Error, actionId?: string, ) { - const newTransactionMeta = { - ...transactionMeta, + const newTransactionMeta = merge({}, transactionMeta, { error: normalizeTxError(error), - status: TransactionStatus.failed, - }; - this.hub.emit('transaction-failed', { + status: TransactionStatus.failed as const, + }); + this.messagingSystem.publish(`${controllerName}:transactionFailed`, { actionId, error: error.message, transactionMeta: newTransactionMeta, @@ -419,27 +645,30 @@ export class TransactionController extends BaseControllerV1< 'TransactionController#failTransaction - Add error message and set status to failed', ); this.onTransactionStatusChange(newTransactionMeta); - this.hub.emit(`${transactionMeta.id}:finished`, newTransactionMeta); + this.messagingSystem.publish( + `${controllerName}:transactionFinished`, + newTransactionMeta, + ); + this.#internalEvents.emit( + `${transactionMeta.id}:finished`, + newTransactionMeta, + ); } private async registryLookup(fourBytePrefix: string): Promise { const registryMethod = await this.registry.lookup(fourBytePrefix); + if (!registryMethod) { + return { + registryMethod: '', + parsedRegistryMethod: { name: undefined, args: undefined }, + }; + } const parsedRegistryMethod = this.registry.parse(registryMethod); return { registryMethod, parsedRegistryMethod }; } #multichainTrackingHelper: MultichainTrackingHelper; - /** - * EventEmitter instance used to listen to specific transactional events - */ - hub = new EventEmitter() as TransactionControllerEventEmitter; - - /** - * Name of this controller used during composition - */ - override name = 'TransactionController'; - /** * Method used to sign transactions */ @@ -449,45 +678,71 @@ export class TransactionController extends BaseControllerV1< transactionMeta?: TransactionMeta, ) => Promise; - constructor( - { - blockTracker, - disableHistory, - disableSendFlowHistory, - disableSwaps, - getCurrentAccountEIP1559Compatibility, - getCurrentNetworkEIP1559Compatibility, - getExternalPendingTransactions, - getGasFeeEstimates, - getNetworkState, - getPermittedAccounts, - getSavedGasFees, - getSelectedAddress, - incomingTransactions = {}, + /** + * Constructs a TransactionController. + * + * @param options - The controller options. + * @param options.blockTracker - The block tracker used to poll for new blocks data. + * @param options.disableHistory - Whether to disable storing history in transaction metadata. + * @param options.disableSendFlowHistory - Explicitly disable transaction metadata history. + * @param options.disableSwaps - Whether to disable additional processing on swaps transactions. + * @param options.getCurrentAccountEIP1559Compatibility - Whether or not the account supports EIP-1559. + * @param options.getCurrentNetworkEIP1559Compatibility - Whether or not the network supports EIP-1559. + * @param options.getExternalPendingTransactions - Callback to retrieve pending transactions from external sources. + * @param options.getGasFeeEstimates - Callback to retrieve gas fee estimates. + * @param options.getNetworkClientRegistry - Gets the network client registry. + * @param options.getNetworkState - Gets the state of the network controller. + * @param options.getPermittedAccounts - Get accounts that a given origin has permissions for. + * @param options.getSavedGasFees - Gets the saved gas fee config. + * @param options.getSelectedAddress - Gets the address of the currently selected account. + * @param options.incomingTransactions - Configuration options for incoming transaction support. + * @param options.isMultichainEnabled - Enable multichain support. + * @param options.messenger - The controller messenger. + * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. + * @param options.pendingTransactions - Configuration options for pending transaction support. + * @param options.provider - The provider used to create the underlying EthQuery instance. + * @param options.securityProviderRequest - A function for verifying a transaction, whether it is malicious or not. + * @param options.sign - Function used to sign transactions. + * @param options.state - Initial state to set on this controller. + * @param options.transactionHistoryLimit - Transaction history limit. + * @param options.hooks - The controller hooks. + */ + constructor({ + blockTracker, + disableHistory, + disableSendFlowHistory, + disableSwaps, + getCurrentAccountEIP1559Compatibility, + getCurrentNetworkEIP1559Compatibility, + getExternalPendingTransactions, + getGasFeeEstimates, + getNetworkClientRegistry, + getNetworkState, + getPermittedAccounts, + getSavedGasFees, + getSelectedAddress, + incomingTransactions = {}, + isMultichainEnabled = false, + messenger, + onNetworkStateChange, + pendingTransactions = {}, + provider, + securityProviderRequest, + sign, + state, + transactionHistoryLimit = 40, + hooks, + }: TransactionControllerOptions) { + super({ + name: controllerName, + metadata, messenger, - onNetworkStateChange, - pendingTransactions = {}, - provider, - securityProviderRequest, - getNetworkClientRegistry, - isMultichainEnabled = false, - hooks, - }: TransactionControllerOptions, - config?: Partial, - state?: Partial, - ) { - super(config, state); - - this.defaultConfig = { - txHistoryLimit: 40, - }; + state: { + ...getDefaultTransactionControllerState(), + ...state, + }, + }); - this.defaultState = { - methodData: {}, - transactions: [], - lastFetchedBlockNumbers: {}, - }; - this.initialize(); this.messagingSystem = messenger; this.getNetworkState = getNetworkState; this.isSendFlowHistoryDisabled = disableSendFlowHistory ?? false; @@ -509,6 +764,8 @@ export class TransactionController extends BaseControllerV1< this.securityProviderRequest = securityProviderRequest; this.#incomingTransactionOptions = incomingTransactions; this.#pendingTransactionOptions = pendingTransactions; + this.#transactionHistoryLimit = transactionHistoryLimit; + this.sign = sign; this.afterSign = hooks?.afterSign ?? (() => true); this.beforeApproveOnInit = hooks?.beforeApproveOnInit ?? (() => true); @@ -591,7 +848,10 @@ export class TransactionController extends BaseControllerV1< getGasFeeControllerEstimates: this.getGasFeeEstimates, getTransactions: () => this.state.transactions, onStateChange: (listener) => { - this.subscribe(listener); + this.messagingSystem.subscribe( + 'TransactionController:stateChange', + listener, + ); }, }); @@ -601,7 +861,10 @@ export class TransactionController extends BaseControllerV1< // when transactionsController state changes // check for pending transactions and start polling if there are any - this.subscribe(this.#checkForPendingTransactionAndStartPolling); + this.messagingSystem.subscribe( + 'TransactionController:stateChange', + this.#checkForPendingTransactionAndStartPolling, + ); // TODO once v2 is merged make sure this only runs when // selectedNetworkClientId changes @@ -638,8 +901,8 @@ export class TransactionController extends BaseControllerV1< return methodData[fourBytePrefix]; } const registry = await this.registryLookup(fourBytePrefix); - this.update({ - methodData: { ...methodData, ...{ [fourBytePrefix]: registry } }, + this.update((state) => { + state.methodData[fourBytePrefix] = registry; }); return registry; } finally { @@ -741,64 +1004,73 @@ export class TransactionController extends BaseControllerV1< const existingTransactionMeta = this.getTransactionWithActionId(actionId); // If a request to add a transaction with the same actionId is submitted again, a new transaction will not be created for it. - const transactionMeta: TransactionMeta = existingTransactionMeta || { - // Add actionId to txMeta to check if same actionId is seen again - actionId, - chainId, - dappSuggestedGasFees, - deviceConfirmedOn, - id: random(), - origin, - securityAlertResponse, - status: TransactionStatus.unapproved as TransactionStatus.unapproved, - time: Date.now(), - txParams, - userEditedGasLimit: false, - verifiedOnBlockchain: false, - type: transactionType, - networkClientId, - }; + let addedTransactionMeta = existingTransactionMeta + ? cloneDeep(existingTransactionMeta) + : { + // Add actionId to txMeta to check if same actionId is seen again + actionId, + chainId, + dappSuggestedGasFees, + deviceConfirmedOn, + id: random(), + origin, + securityAlertResponse, + status: TransactionStatus.unapproved as const, + time: Date.now(), + txParams, + userEditedGasLimit: false, + verifiedOnBlockchain: false, + type: transactionType, + networkClientId, + }; - await this.updateGasProperties(transactionMeta); + await this.updateGasProperties(addedTransactionMeta); // Checks if a transaction already exists with a given actionId if (!existingTransactionMeta) { // Set security provider response if (method && this.securityProviderRequest) { const securityProviderResponse = await this.securityProviderRequest( - transactionMeta, + addedTransactionMeta, method, ); - transactionMeta.securityProviderResponse = securityProviderResponse; + addedTransactionMeta.securityProviderResponse = + securityProviderResponse; } if (!this.isSendFlowHistoryDisabled) { - transactionMeta.sendFlowHistory = sendFlowHistory ?? []; + addedTransactionMeta.sendFlowHistory = sendFlowHistory ?? []; } // Initial history push if (!this.isHistoryDisabled) { - addInitialHistorySnapshot(transactionMeta); + addedTransactionMeta = addInitialHistorySnapshot(addedTransactionMeta); } - await updateSwapsTransaction(transactionMeta, transactionType, swaps, { - isSwapsDisabled: this.isSwapsDisabled, - cancelTransaction: this.cancelTransaction.bind(this), - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - controllerHubEmitter: this.hub.emit.bind(this.hub) as any, - }); + addedTransactionMeta = updateSwapsTransaction( + addedTransactionMeta, + transactionType, + swaps, + { + isSwapsDisabled: this.isSwapsDisabled, + cancelTransaction: this.cancelTransaction.bind(this), + messenger: this.messagingSystem, + }, + ); - this.addMetadata(transactionMeta); - this.hub.emit(`unapprovedTransaction`, transactionMeta); + this.addMetadata(addedTransactionMeta); + this.messagingSystem.publish( + `${controllerName}:unapprovedTransactionAdded`, + addedTransactionMeta, + ); } return { - result: this.processApproval(transactionMeta, { + result: this.processApproval(addedTransactionMeta, { isExisting: Boolean(existingTransactionMeta), requireApproval, actionId, }), - transactionMeta, + transactionMeta: addedTransactionMeta, }; } @@ -975,7 +1247,7 @@ export class TransactionController extends BaseControllerV1< transactionMeta, ); - const cancelTransactionMeta: TransactionMeta = { + const cancelTransactionMeta = { actionId, chainId: transactionMeta.chainId, networkClientId: transactionMeta.networkClientId, @@ -983,26 +1255,30 @@ export class TransactionController extends BaseControllerV1< hash, id: random(), originalGasEstimate: transactionMeta.txParams.gas, - status: TransactionStatus.submitted, + status: TransactionStatus.submitted as const, time: Date.now(), - type: TransactionType.cancel, + type: TransactionType.cancel as const, txParams: newTxParams, }; this.addMetadata(cancelTransactionMeta); // stopTransaction has no approval request, so we assume the user has already approved the transaction - this.hub.emit('transaction-approved', { + this.messagingSystem.publish(`${controllerName}:transactionApproved`, { transactionMeta: cancelTransactionMeta, actionId, }); - this.hub.emit('transaction-submitted', { + this.messagingSystem.publish(`${controllerName}:transactionSubmitted`, { transactionMeta: cancelTransactionMeta, actionId, }); - this.hub.emit( - `${cancelTransactionMeta.id}:finished`, + this.messagingSystem.publish( + `${controllerName}:transactionFinished`, + cancelTransactionMeta, + ); + this.#internalEvents.emit( + `${transactionMeta.id}:finished`, cancelTransactionMeta, ); } @@ -1118,14 +1394,17 @@ export class TransactionController extends BaseControllerV1< transactionMeta.txParams.from, ); - await this.updateTransactionMetaRSV(transactionMeta, signedTx); + const transactionMetaWithRsv = await this.updateTransactionMetaRSV( + transactionMeta, + signedTx, + ); const rawTx = bufferToHex(signedTx.serialize()); const newFee = txParams.maxFeePerGas ?? txParams.gasPrice; const oldFee = txParams.maxFeePerGas - ? transactionMeta.txParams.maxFeePerGas - : transactionMeta.txParams.gasPrice; + ? transactionMetaWithRsv.txParams.maxFeePerGas + : transactionMetaWithRsv.txParams.gasPrice; log('Submitting speed up transaction', { oldFee, newFee, txParams }); @@ -1139,15 +1418,15 @@ export class TransactionController extends BaseControllerV1< transactionMeta, ); - const baseTransactionMeta: TransactionMeta = { - ...transactionMeta, + const baseTransactionMeta = { + ...transactionMetaWithRsv, estimatedBaseFee, id: random(), time: Date.now(), hash, actionId, originalGasEstimate: transactionMeta.txParams.gas, - type: TransactionType.retry, + type: TransactionType.retry as const, originalType: transactionMeta.type, }; @@ -1172,17 +1451,20 @@ export class TransactionController extends BaseControllerV1< this.addMetadata(newTransactionMeta); // speedUpTransaction has no approval request, so we assume the user has already approved the transaction - this.hub.emit('transaction-approved', { + this.messagingSystem.publish(`${controllerName}:transactionApproved`, { transactionMeta: newTransactionMeta, actionId, }); - this.hub.emit('transaction-submitted', { + this.messagingSystem.publish(`${controllerName}:transactionSubmitted`, { transactionMeta: newTransactionMeta, actionId, }); - this.hub.emit(`${transactionMeta.id}:speedup`, newTransactionMeta); + this.messagingSystem.publish( + `${controllerName}:speedupTransactionAdded`, + newTransactionMeta, + ); } /** @@ -1269,10 +1551,13 @@ export class TransactionController extends BaseControllerV1< `Cannot update security alert response as no transaction metadata found`, ); } - const updatedMeta = merge(transactionMeta, { securityAlertResponse }); + const updatedTransactionMeta = { + ...transactionMeta, + securityAlertResponse, + }; this.updateTransaction( - updatedMeta, - 'TransactionController:updatesecurityAlertResponse - securityAlertResponse updated', + updatedTransactionMeta, + `${controllerName}:updatesecurityAlertResponse - securityAlertResponse updated`, ); } @@ -1287,7 +1572,9 @@ export class TransactionController extends BaseControllerV1< wipeTransactions(ignoreNetwork?: boolean, address?: string) { /* istanbul ignore next */ if (ignoreNetwork && !address) { - this.update({ transactions: [] }); + this.update((state) => { + state.transactions = []; + }); return; } const currentChainId = this.getChainId(); @@ -1306,8 +1593,8 @@ export class TransactionController extends BaseControllerV1< }, ); - this.update({ - transactions: this.trimTransactionsForState(newTransactions), + this.update((state) => { + state.transactions = this.trimTransactionsForState(newTransactions); }); } @@ -1324,16 +1611,19 @@ export class TransactionController extends BaseControllerV1< baseFeePerGas: Hex, ) { // Run validation and add external transaction to state. - this.addExternalTransaction(transactionMeta); + const newTransactionMeta = this.addExternalTransaction(transactionMeta); try { - const transactionId = transactionMeta.id; + const transactionId = newTransactionMeta.id; // Make sure status is confirmed and define gasUsed as in receipt. - transactionMeta.status = TransactionStatus.confirmed; - transactionMeta.txReceipt = transactionReceipt; + const updatedTransactionMeta = { + ...newTransactionMeta, + status: TransactionStatus.confirmed as const, + txReceipt: transactionReceipt, + }; if (baseFeePerGas) { - transactionMeta.baseFeePerGas = baseFeePerGas; + updatedTransactionMeta.baseFeePerGas = baseFeePerGas; } // Update same nonce local transactions as dropped and define replacedBy properties. @@ -1341,18 +1631,19 @@ export class TransactionController extends BaseControllerV1< // Update external provided transaction with updated gas values and confirmed status. this.updateTransaction( - transactionMeta, - 'TransactionController:confirmExternalTransaction - Add external transaction', + updatedTransactionMeta, + `${controllerName}:confirmExternalTransaction - Add external transaction`, ); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(updatedTransactionMeta); // Intentional given potential duration of process. // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.updatePostBalance(transactionMeta); + this.updatePostBalance(updatedTransactionMeta); - this.hub.emit('transaction-confirmed', { - transactionMeta, - }); + this.messagingSystem.publish( + `${controllerName}:transactionConfirmed`, + updatedTransactionMeta, + ); } catch (error) { console.error('Failed to confirm external transaction', error); } @@ -1390,17 +1681,15 @@ export class TransactionController extends BaseControllerV1< 'updateTransactionSendFlowHistory', ); - if ( - currentSendFlowHistoryLength === - (transactionMeta?.sendFlowHistory?.length || 0) - ) { - transactionMeta.sendFlowHistory = [ - ...(transactionMeta?.sendFlowHistory ?? []), - ...sendFlowHistoryToAdd, - ]; + const sendFlowHistory = transactionMeta.sendFlowHistory ?? []; + if (currentSendFlowHistoryLength === sendFlowHistory.length) { + const updatedTransactionMeta = { + ...transactionMeta, + sendFlowHistory: [...sendFlowHistory, ...sendFlowHistoryToAdd], + }; this.updateTransaction( - transactionMeta, - 'TransactionController:updateTransactionSendFlowHistory - sendFlowHistory updated', + updatedTransactionMeta, + `${controllerName}:updateTransactionSendFlowHistory - sendFlowHistory updated`, ); } @@ -1489,11 +1778,11 @@ export class TransactionController extends BaseControllerV1< transactionGasFees = pickBy(transactionGasFees); // merge updated gas values with existing transaction meta - const updatedMeta = merge(transactionMeta, transactionGasFees); + const updatedMeta = merge({}, transactionMeta, transactionGasFees); this.updateTransaction( updatedMeta, - 'TransactionController:updateTransactionGasFees - gas values updated', + `${controllerName}:updateTransactionGasFees - gas values updated`, ); return this.getTransaction(transactionId) as TransactionMeta; @@ -1547,11 +1836,11 @@ export class TransactionController extends BaseControllerV1< ); // merge updated previous gas values with existing transaction meta - const updatedMeta = merge(transactionMeta, transactionPreviousGas); + const updatedMeta = merge({}, transactionMeta, transactionPreviousGas); this.updateTransaction( updatedMeta, - 'TransactionController:updatePreviousGasParams - Previous gas values updated', + `${controllerName}:updatePreviousGasParams - Previous gas values updated`, ); return this.getTransaction(transactionId) as TransactionMeta; @@ -1622,7 +1911,7 @@ export class TransactionController extends BaseControllerV1< editableParams.txParams, ) as TransactionParams; - const updatedTransaction = merge(transactionMeta, editableParams); + const updatedTransaction = merge({}, transactionMeta, editableParams); const { type } = await determineTransactionType( updatedTransaction.txParams, this.#multichainTrackingHelper.getEthQuery({ @@ -1771,6 +2060,7 @@ export class TransactionController extends BaseControllerV1< } const updatedTransactionMeta = merge( + {}, transactionMeta, pickBy({ hash, status }), ); @@ -1785,7 +2075,7 @@ export class TransactionController extends BaseControllerV1< this.updateTransaction( updatedTransactionMeta, - `TransactionController:updateCustodialTransaction - Custodial transaction updated`, + `${controllerName}:updateCustodialTransaction - Custodial transaction updated`, ); if ( @@ -1793,7 +2083,10 @@ export class TransactionController extends BaseControllerV1< status as TransactionStatus, ) ) { - this.hub.emit(`${transactionMeta.id}:finished`, updatedTransactionMeta); + this.messagingSystem.publish( + `${controllerName}:transactionFinished`, + updatedTransactionMeta, + ); } } @@ -1963,7 +2256,9 @@ export class TransactionController extends BaseControllerV1< const transactions = this.state.transactions.filter( ({ status }) => status !== TransactionStatus.unapproved, ); - this.update({ transactions: this.trimTransactionsForState(transactions) }); + this.update((state) => { + state.transactions = this.trimTransactionsForState(transactions); + }); } /** @@ -1992,9 +2287,12 @@ export class TransactionController extends BaseControllerV1< } private addMetadata(transactionMeta: TransactionMeta) { - const { transactions } = this.state; - transactions.push(transactionMeta); - this.update({ transactions: this.trimTransactionsForState(transactions) }); + this.update((state) => { + state.transactions = this.trimTransactionsForState([ + ...state.transactions, + transactionMeta, + ]); + }); } private async updateGasProperties(transactionMeta: TransactionMeta) { @@ -2083,18 +2381,8 @@ export class TransactionController extends BaseControllerV1< const acceptResult = await this.requestApproval(transactionMeta, { shouldShowRequest, }); - resultCallbacks = acceptResult.resultCallbacks; - if (resultCallbacks) { - this.hub.once(`${transactionId}:publish-skip`, () => { - resultCallbacks?.success(); - - // Remove the reference to prevent additional reports once submitted. - resultCallbacks = undefined; - }); - } - const approvalValue = acceptResult.value as | { txMeta?: TransactionMeta; @@ -2120,14 +2408,23 @@ export class TransactionController extends BaseControllerV1< this.isTransactionCompleted(transactionId); if (!isTxCompleted) { - await this.approveTransaction(transactionId); + const approvalResult = await this.approveTransaction(transactionId); + if ( + approvalResult === ApprovalState.SkippedViaBeforePublishHook && + resultCallbacks + ) { + resultCallbacks.success(); + } const updatedTransactionMeta = this.getTransaction( transactionId, ) as TransactionMeta; - this.hub.emit('transaction-approved', { - transactionMeta: updatedTransactionMeta, - actionId, - }); + this.messagingSystem.publish( + `${controllerName}:transactionApproved`, + { + transactionMeta: updatedTransactionMeta, + actionId, + }, + ); } // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -2184,6 +2481,8 @@ export class TransactionController extends BaseControllerV1< const releaseLock = await this.mutex.acquire(); const index = transactions.findIndex(({ id }) => transactionId === id); const transactionMeta = transactions[index]; + const updatedTransactionMeta = cloneDeep(transactionMeta); + const { txParams: { from }, networkClientId, @@ -2198,16 +2497,16 @@ export class TransactionController extends BaseControllerV1< transactionMeta, new Error('No sign method defined.'), ); - return; + return ApprovalState.NotApproved; } else if (!transactionMeta.chainId) { releaseLock(); this.failTransaction(transactionMeta, new Error('No chainId defined.')); - return; + return ApprovalState.NotApproved; } if (this.inProcessOfSigning.has(transactionId)) { log('Skipping approval as signing in progress', transactionId); - return; + return ApprovalState.NotApproved; } const [nonce, releaseNonce] = await getNextNonce( @@ -2218,42 +2517,51 @@ export class TransactionController extends BaseControllerV1< releaseNonceLock = releaseNonce; - transactionMeta.status = TransactionStatus.approved; - transactionMeta.txParams.nonce = nonce; - transactionMeta.txParams.chainId = transactionMeta.chainId; + updatedTransactionMeta.status = TransactionStatus.approved; + updatedTransactionMeta.txParams = { + ...updatedTransactionMeta.txParams, + nonce, + chainId: transactionMeta.chainId, + }; const baseTxParams = { - ...transactionMeta.txParams, - gasLimit: transactionMeta.txParams.gas, + ...updatedTransactionMeta.txParams, + gasLimit: updatedTransactionMeta.txParams.gas, }; this.updateTransaction( - transactionMeta, + updatedTransactionMeta, 'TransactionController#approveTransaction - Transaction approved', ); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(updatedTransactionMeta); - const isEIP1559 = isEIP1559Transaction(transactionMeta.txParams); + const isEIP1559 = isEIP1559Transaction(updatedTransactionMeta.txParams); const txParams: TransactionParams = isEIP1559 ? { ...baseTxParams, - estimatedBaseFee: transactionMeta.txParams.estimatedBaseFee, + estimatedBaseFee: updatedTransactionMeta.txParams.estimatedBaseFee, type: TransactionEnvelopeType.feeMarket, } : baseTxParams; - const rawTx = await this.signTransaction(transactionMeta, txParams); + const rawTx = await this.signTransaction( + updatedTransactionMeta, + txParams, + ); - if (!this.beforePublish(transactionMeta)) { + if (!this.beforePublish(updatedTransactionMeta)) { log('Skipping publishing transaction based on hook'); - this.hub.emit(`${transactionMeta.id}:publish-skip`, transactionMeta); - return; + this.messagingSystem.publish( + `${controllerName}:transactionPublishingSkipped`, + updatedTransactionMeta, + ); + return ApprovalState.SkippedViaBeforePublishHook; } if (!rawTx) { - return; + return ApprovalState.NotApproved; } const ethQuery = this.#multichainTrackingHelper.getEthQuery({ @@ -2266,9 +2574,12 @@ export class TransactionController extends BaseControllerV1< const preTxBalance = await query(ethQuery, 'getBalance', [from]); - transactionMeta.preTxBalance = preTxBalance; + updatedTransactionMeta.preTxBalance = preTxBalance; - log('Updated pre-transaction balance', transactionMeta.preTxBalance); + log( + 'Updated pre-transaction balance', + updatedTransactionMeta.preTxBalance, + ); } log('Publishing transaction', txParams); @@ -2284,26 +2595,35 @@ export class TransactionController extends BaseControllerV1< log('Publish successful', hash); - transactionMeta.hash = hash; - transactionMeta.status = TransactionStatus.submitted; - transactionMeta.submittedTime = new Date().getTime(); + updatedTransactionMeta.hash = hash; + updatedTransactionMeta.status = TransactionStatus.submitted; + updatedTransactionMeta.submittedTime = new Date().getTime(); this.updateTransaction( - transactionMeta, + updatedTransactionMeta, 'TransactionController#approveTransaction - Transaction submitted', ); - this.hub.emit('transaction-submitted', { - transactionMeta, + this.messagingSystem.publish(`${controllerName}:transactionSubmitted`, { + transactionMeta: updatedTransactionMeta, }); - this.hub.emit(`${transactionMeta.id}:finished`, transactionMeta); + this.messagingSystem.publish( + `${controllerName}:transactionFinished`, + updatedTransactionMeta, + ); + this.#internalEvents.emit( + `${updatedTransactionMeta.id}:finished`, + updatedTransactionMeta, + ); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(updatedTransactionMeta); + return ApprovalState.Approved; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { this.failTransaction(transactionMeta, error); + return ApprovalState.NotApproved; } finally { this.inProcessOfSigning.delete(transactionId); // must set transaction to submitted/failed before releasing lock @@ -2333,17 +2653,29 @@ export class TransactionController extends BaseControllerV1< if (!transactionMeta) { return; } - transactionMeta.status = TransactionStatus.rejected; - const transactions = this.state.transactions.filter( - ({ id }) => id !== transactionId, + this.update((state) => { + const transactions = state.transactions.filter( + ({ id }) => id !== transactionId, + ); + state.transactions = this.trimTransactionsForState(transactions); + }); + const updatedTransactionMeta = { + ...transactionMeta, + status: TransactionStatus.rejected as const, + }; + this.messagingSystem.publish( + `${controllerName}:transactionFinished`, + updatedTransactionMeta, ); - this.update({ transactions: this.trimTransactionsForState(transactions) }); - this.hub.emit(`${transactionMeta.id}:finished`, transactionMeta); - this.hub.emit('transaction-rejected', { - transactionMeta, + this.#internalEvents.emit( + `${transactionMeta.id}:finished`, + updatedTransactionMeta, + ); + this.messagingSystem.publish(`${controllerName}:transactionRejected`, { + transactionMeta: updatedTransactionMeta, actionId, }); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(updatedTransactionMeta); } /** @@ -2353,9 +2685,9 @@ export class TransactionController extends BaseControllerV1< * Pending or unapproved transactions will not be removed by this * operation. For safety of presenting a fully functional transaction UI * representation, this function will not break apart transactions with the - * same nonce, created on the same day, per network. Not accounting for transactions of the same - * nonce, same day and network combo can result in confusing or broken experiences - * in the UI. The transactions are then updated using the BaseControllerV1 update. + * same nonce, created on the same day, per network. Not accounting for + * transactions of the same nonce, same day and network combo can result in + * confusing or broken experiences in the UI. * * @param transactions - The transactions to be applied to the state. * @returns The trimmed list of transactions. @@ -2365,20 +2697,20 @@ export class TransactionController extends BaseControllerV1< ): TransactionMeta[] { const nonceNetworkSet = new Set(); - const txsToKeep = transactions + const txsToKeep = [...transactions] .sort((a, b) => (a.time > b.time ? -1 : 1)) // Descending time order .filter((tx) => { const { chainId, status, txParams, time } = tx; if (txParams) { - const key = `${txParams.nonce}-${convertHexToDecimal( + const key = `${String(txParams.nonce)}-${convertHexToDecimal( chainId, )}-${new Date(time).toDateString()}`; if (nonceNetworkSet.has(key)) { return true; } else if ( - nonceNetworkSet.size < this.config.txHistoryLimit || + nonceNetworkSet.size < this.#transactionHistoryLimit || !this.isFinalState(status) ) { nonceNetworkSet.add(key); @@ -2515,21 +2847,20 @@ export class TransactionController extends BaseControllerV1< added: TransactionMeta[]; updated: TransactionMeta[]; }) { - const { transactions: currentTransactions } = this.state; - - const updatedTransactions = [ - ...added, - ...currentTransactions.map((originalTransaction) => { - const updatedTransaction = updated.find( - ({ hash }) => hash === originalTransaction.hash, - ); - - return updatedTransaction ?? originalTransaction; - }), - ]; + this.update((state) => { + const { transactions: currentTransactions } = state; + const updatedTransactions = [ + ...added, + ...currentTransactions.map((originalTransaction) => { + const updatedTransaction = updated.find( + ({ hash }) => hash === originalTransaction.hash, + ); + + return updatedTransaction ?? originalTransaction; + }), + ]; - this.update({ - transactions: this.trimTransactionsForState(updatedTransactions), + state.transactions = this.trimTransactionsForState(updatedTransactions); }); } @@ -2542,8 +2873,13 @@ export class TransactionController extends BaseControllerV1< }; blockNumber: number; }) { - this.update({ lastFetchedBlockNumbers }); - this.hub.emit('incomingTransactionBlock', blockNumber); + this.update((state) => { + state.lastFetchedBlockNumbers = lastFetchedBlockNumbers; + }); + this.messagingSystem.publish( + `${controllerName}:incomingTransactionBlockReceived`, + blockNumber, + ); } private generateDappSuggestedGasFees( @@ -2588,6 +2924,7 @@ export class TransactionController extends BaseControllerV1< * Validates and adds external provided transaction to state. * * @param transactionMeta - Nominated external transaction to be added to state. + * @returns The new transaction. */ private addExternalTransaction(transactionMeta: TransactionMeta) { const { chainId } = transactionMeta; @@ -2612,16 +2949,19 @@ export class TransactionController extends BaseControllerV1< ); // Make sure provided external transaction has non empty history array - if (!(transactionMeta.history ?? []).length) { - if (!this.isHistoryDisabled) { - addInitialHistorySnapshot(transactionMeta); - } - } - - const updatedTransactions = [...transactions, transactionMeta]; - this.update({ - transactions: this.trimTransactionsForState(updatedTransactions), + const newTransactionMeta = + (transactionMeta.history ?? []).length === 0 && !this.isHistoryDisabled + ? addInitialHistorySnapshot(transactionMeta) + : transactionMeta; + + this.update((state) => { + state.transactions = this.trimTransactionsForState([ + ...state.transactions, + newTransactionMeta, + ]); }); + + return newTransactionMeta; } /** @@ -2639,7 +2979,7 @@ export class TransactionController extends BaseControllerV1< const from = transactionMeta.txParams?.from; const { chainId } = transactionMeta; - const sameNonceTxs = this.state.transactions.filter( + const sameNonceTransactions = this.state.transactions.filter( (transaction) => transaction.id !== transactionId && transaction.txParams.from === from && @@ -2647,17 +2987,28 @@ export class TransactionController extends BaseControllerV1< transaction.chainId === chainId && transaction.type !== TransactionType.incoming, ); + const sameNonceTransactionIds = sameNonceTransactions.map( + (transaction) => transaction.id, + ); - if (!sameNonceTxs.length) { + if (sameNonceTransactions.length === 0) { return; } - // Mark all same nonce transactions as dropped and give it a replacedBy hash - for (const transaction of sameNonceTxs) { - transaction.replacedBy = transactionMeta.hash; - transaction.replacedById = transactionMeta.id; - // Drop any transaction that wasn't previously failed (off chain failure) - if (transaction.status !== TransactionStatus.failed) { + this.update((state) => { + for (const transaction of state.transactions) { + if (sameNonceTransactionIds.includes(transaction.id)) { + transaction.replacedBy = transactionMeta?.hash; + transaction.replacedById = transactionMeta?.id; + } + } + }); + + for (const transaction of this.state.transactions) { + if ( + sameNonceTransactionIds.includes(transaction.id) && + transaction.status !== TransactionStatus.failed + ) { this.setTransactionStatusDropped(transaction); } } @@ -2669,15 +3020,18 @@ export class TransactionController extends BaseControllerV1< * @param transactionMeta - TransactionMeta of transaction to be marked as dropped. */ private setTransactionStatusDropped(transactionMeta: TransactionMeta) { - transactionMeta.status = TransactionStatus.dropped; - this.hub.emit('transaction-dropped', { - transactionMeta, + const updatedTransactionMeta = { + ...transactionMeta, + status: TransactionStatus.dropped as const, + }; + this.messagingSystem.publish(`${controllerName}:transactionDropped`, { + transactionMeta: updatedTransactionMeta, }); this.updateTransaction( - transactionMeta, + updatedTransactionMeta, 'TransactionController#setTransactionStatusDropped - Transaction dropped', ); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(updatedTransactionMeta); } /** @@ -2696,7 +3050,7 @@ export class TransactionController extends BaseControllerV1< transactionId: string, ): Promise { return new Promise((resolve) => { - this.hub.once(`${transactionId}:finished`, (txMeta) => { + this.#internalEvents.once(`${transactionId}:finished`, (txMeta) => { resolve(txMeta); }); }); @@ -2712,7 +3066,9 @@ export class TransactionController extends BaseControllerV1< private async updateTransactionMetaRSV( transactionMeta: TransactionMeta, signedTx: TypedTransaction, - ): Promise { + ): Promise { + const transactionMetaWithRsv = cloneDeep(transactionMeta); + for (const key of ['r', 's', 'v'] as const) { const value = signedTx[key]; @@ -2720,8 +3076,10 @@ export class TransactionController extends BaseControllerV1< continue; } - transactionMeta[key] = add0x(value.toString(16)); + transactionMetaWithRsv[key] = add0x(value.toString(16)); } + + return transactionMetaWithRsv; } private async getEIP1559Compatibility(networkClientId?: NetworkClientId) { @@ -2779,23 +3137,26 @@ export class TransactionController extends BaseControllerV1< return undefined; } - await this.updateTransactionMetaRSV(transactionMeta, signedTx); - - transactionMeta.status = TransactionStatus.signed; + const transactionMetaWithRsv = { + ...(await this.updateTransactionMetaRSV(transactionMeta, signedTx)), + status: TransactionStatus.signed as const, + }; this.updateTransaction( - transactionMeta, + transactionMetaWithRsv, 'TransactionController#approveTransaction - Transaction signed', ); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(transactionMetaWithRsv); const rawTx = bufferToHex(signedTx.serialize()); - transactionMeta.rawTx = rawTx; + const transactionMetaWithRawTx = merge({}, transactionMetaWithRsv, { + rawTx, + }); this.updateTransaction( - transactionMeta, + transactionMetaWithRawTx, 'TransactionController#approveTransaction - RawTransaction added', ); @@ -2803,7 +3164,9 @@ export class TransactionController extends BaseControllerV1< } private onTransactionStatusChange(transactionMeta: TransactionMeta) { - this.hub.emit('transaction-status-update', { transactionMeta }); + this.messagingSystem.publish(`${controllerName}:transactionStatusUpdated`, { + transactionMeta, + }); } private getNonceTrackerTransactions( @@ -2824,8 +3187,10 @@ export class TransactionController extends BaseControllerV1< this.markNonceDuplicatesDropped(transactionMeta.id); - this.hub.emit('transaction-confirmed', { transactionMeta }); - this.hub.emit(`${transactionMeta.id}:confirmed`, transactionMeta); + this.messagingSystem.publish( + `${controllerName}:transactionConfirmed`, + transactionMeta, + ); this.onTransactionStatusChange(transactionMeta); @@ -2851,10 +3216,13 @@ export class TransactionController extends BaseControllerV1< updateTransaction: this.updateTransaction.bind(this), }); - this.hub.emit('post-transaction-balance-updated', { - transactionMeta: updatedTransactionMeta, - approvalTransactionMeta, - }); + this.messagingSystem.publish( + `${controllerName}:postTransactionBalanceUpdated`, + { + transactionMeta: updatedTransactionMeta, + approvalTransactionMeta, + }, + ); } catch (error) { /* istanbul ignore next */ log('Error while updating post transaction balance', error); @@ -2903,7 +3271,7 @@ export class TransactionController extends BaseControllerV1< isEnabled: this.#incomingTransactionOptions.isEnabled, queryEntireHistory: this.#incomingTransactionOptions.queryEntireHistory, remoteTransactionSource: etherscanRemoteTransactionSource, - transactionLimit: this.config.txHistoryLimit, + transactionLimit: this.#transactionHistoryLimit, updateTransactions: this.#incomingTransactionOptions.updateTransactions, }); @@ -2925,7 +3293,9 @@ export class TransactionController extends BaseControllerV1< const getChainId = chainId ? () => chainId : this.getChainId.bind(this); const pendingTransactionTracker = new PendingTransactionTracker({ - approveTransaction: this.approveTransaction.bind(this), + approveTransaction: async (transactionId: string) => { + await this.approveTransaction(transactionId); + }, blockTracker, getChainId, getEthQuery: () => ethQuery, @@ -3082,21 +3452,26 @@ export class TransactionController extends BaseControllerV1< transactionMeta: TransactionMeta, { note, skipHistory }: { note?: string; skipHistory?: boolean }, ) { - const { transactions } = this.state; - - transactionMeta.txParams = normalizeTransactionParams( - transactionMeta.txParams, - ); - - validateTxParams(transactionMeta.txParams); + const normalizedTransaction = { + ...transactionMeta, + txParams: normalizeTransactionParams(transactionMeta.txParams), + }; - if (skipHistory !== true) { - updateTransactionHistory(transactionMeta, note ?? 'Transaction updated'); - } + validateTxParams(normalizedTransaction.txParams); - const index = transactions.findIndex(({ id }) => transactionMeta.id === id); - transactions[index] = transactionMeta; + const transactionWithUpdatedHistory = + skipHistory === true + ? normalizedTransaction + : updateTransactionHistory( + normalizedTransaction, + note ?? 'Transaction updated', + ); - this.update({ transactions: this.trimTransactionsForState(transactions) }); + this.update((state) => { + const index = state.transactions.findIndex( + ({ id }) => transactionMeta.id === id, + ); + state.transactions[index] = transactionWithUpdatedHistory; + }); } } diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index c54f5adaf4..72c91fa446 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -1,3 +1,8 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import type { + ApprovalControllerActions, + ApprovalControllerEvents, +} from '@metamask/approval-controller'; import { ApprovalController } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; import { @@ -10,7 +15,13 @@ import { NetworkController, NetworkClientType, } from '@metamask/network-controller'; -import type { NetworkClientConfiguration } from '@metamask/network-controller'; +import type { + NetworkClientConfiguration, + NetworkControllerActions, + NetworkControllerEvents, + NetworkClientId, +} from '@metamask/network-controller'; +import assert from 'assert'; import nock from 'nock'; import type { SinonFakeTimers } from 'sinon'; import { useFakeTimers } from 'sinon'; @@ -34,12 +45,25 @@ import { buildEthSendRawTransactionRequestMock, buildEthGetTransactionReceiptRequestMock, } from '../tests/JsonRpcRequestMocks'; +import type { + TransactionControllerActions, + TransactionControllerEvents, +} from './TransactionController'; import { TransactionController } from './TransactionController'; import type { TransactionMeta } from './types'; import { TransactionStatus, TransactionType } from './types'; import { getEtherscanApiHost } from './utils/etherscan'; import * as etherscanUtils from './utils/etherscan'; +type UnrestrictedControllerMessenger = ControllerMessenger< + | NetworkControllerActions + | ApprovalControllerActions + | TransactionControllerActions, + | NetworkControllerEvents + | ApprovalControllerEvents + | TransactionControllerEvents +>; + const ACCOUNT_MOCK = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; const ACCOUNT_2_MOCK = '0x08f137f335ea1b8f193b8f6ea92561a60d23a211'; const ACCOUNT_3_MOCK = '0xe688b84b23f322a994a53dbf8e15fa82cdb71127'; @@ -71,24 +95,28 @@ const customGoerliNetworkClientConfiguration = { rpcUrl: 'https://mock.rpc.url', } as const; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const newController = async (options: any = {}) => { +const setupController = async ( + givenOptions: Partial< + ConstructorParameters[0] + > = {}, +) => { // Mainnet network must be mocked for NetworkController instantiation mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( InfuraNetworkType.mainnet, ), mocks: [ - { - ...buildEthBlockNumberRequestMock('0x1'), - discardAfterMatching: false, - }, + buildEthBlockNumberRequestMock('0x1'), + buildEthGetBlockByNumberRequestMock('0x1'), ], }); - const messenger = new ControllerMessenger(); + const unrestrictedMessenger: UnrestrictedControllerMessenger = + new ControllerMessenger(); const networkController = new NetworkController({ - messenger: messenger.getRestricted({ name: 'NetworkController' }), + messenger: unrestrictedMessenger.getRestricted({ + name: 'NetworkController', + }), trackMetaMetricsEvent: () => { // noop }, @@ -97,51 +125,64 @@ const newController = async (options: any = {}) => { await networkController.initializeProvider(); const { provider, blockTracker } = networkController.getProviderAndBlockTracker(); + assert(provider, 'Provider must be available'); + assert(blockTracker, 'Provider must be available'); const approvalController = new ApprovalController({ - messenger: messenger.getRestricted({ + messenger: unrestrictedMessenger.getRestricted({ name: 'ApprovalController', }), showApprovalRequest: jest.fn(), typesExcludedFromRateLimiting: [ApprovalType.Transaction], }); - const { state, config, ...opts } = options; - - const transactionController = new TransactionController( - { - provider, - blockTracker, - messenger, - onNetworkStateChange: () => { - // noop - }, - getCurrentNetworkEIP1559Compatibility: - networkController.getEIP1559Compatibility.bind(networkController), - getNetworkClientRegistry: - opts.getNetworkClientRegistrySpy || - networkController.getNetworkClientRegistry.bind(networkController), - findNetworkClientIdByChainId: - networkController.findNetworkClientIdByChainId.bind(networkController), - getNetworkClientById: - networkController.getNetworkClientById.bind(networkController), - getNetworkState: () => networkController.state, - getSelectedAddress: () => '0xdeadbeef', - getPermittedAccounts: () => [ACCOUNT_MOCK], - isMultichainEnabled: true, - ...opts, + const messenger = unrestrictedMessenger.getRestricted({ + name: 'TransactionController', + allowedActions: [ + 'ApprovalController:addRequest', + 'NetworkController:getNetworkClientById', + 'NetworkController:findNetworkClientIdByChainId', + ], + allowedEvents: ['NetworkController:stateChange'], + }); + + const options = { + blockTracker, + disableHistory: false, + disableSendFlowHistory: false, + disableSwaps: false, + getCurrentNetworkEIP1559Compatibility: async ( + networkClientId?: NetworkClientId, + ) => { + return ( + (await networkController.getEIP1559Compatibility(networkClientId)) ?? + false + ); }, - { - sign: (transaction) => Promise.resolve(transaction), - ...config, + getNetworkState: () => networkController.state, + getNetworkClientRegistry: + networkController.getNetworkClientRegistry.bind(networkController), + getPermittedAccounts: async () => [ACCOUNT_MOCK], + getSelectedAddress: () => '0xdeadbeef', + hooks: {}, + isMultichainEnabled: false, + messenger, + onNetworkStateChange: () => { + // noop }, - state, - ); + provider, + sign: async (transaction: TypedTransaction) => transaction, + transactionHistoryLimit: 40, + ...givenOptions, + }; + + const transactionController = new TransactionController(options); return { transactionController, approvalController, networkController, + messenger, }; }; @@ -157,7 +198,7 @@ describe('TransactionController Integration', () => { describe('constructor', () => { it('should create a new instance of TransactionController', async () => { - const { transactionController } = await newController({}); + const { transactionController } = await setupController(); expect(transactionController).toBeDefined(); transactionController.destroy(); }); @@ -189,7 +230,8 @@ describe('TransactionController Integration', () => { ], }); - const { transactionController } = await newController({ + const { transactionController } = await setupController({ + isMultichainEnabled: true, state: { transactions: [ { @@ -200,7 +242,7 @@ describe('TransactionController Integration', () => { id: 'ecfe8c60-ba27-11ee-8643-dfd28279a442', origin: undefined, securityAlertResponse: undefined, - status: 'approved', + status: TransactionStatus.approved, time: 1706039113766, txParams: { from: '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207', @@ -213,7 +255,7 @@ describe('TransactionController Integration', () => { }, userEditedGasLimit: false, verifiedOnBlockchain: false, - type: 'simpleSend', + type: TransactionType.simpleSend, networkClientId: 'goerli', simulationFails: undefined, originalGasEstimate: '0x5208', @@ -226,7 +268,6 @@ describe('TransactionController Integration', () => { }, userFeeLevel: 'dappSuggested', sendFlowHistory: [], - history: [{}, []], }, { actionId: undefined, @@ -236,7 +277,7 @@ describe('TransactionController Integration', () => { id: 'c4cc0ff0-ba28-11ee-926f-55a7f9c2c2c6', origin: undefined, securityAlertResponse: undefined, - status: 'approved', + status: TransactionStatus.approved, time: 1706039113766, txParams: { from: '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207', @@ -249,7 +290,7 @@ describe('TransactionController Integration', () => { }, userEditedGasLimit: false, verifiedOnBlockchain: false, - type: 'simpleSend', + type: TransactionType.simpleSend, networkClientId: 'sepolia', simulationFails: undefined, originalGasEstimate: '0x5208', @@ -262,23 +303,24 @@ describe('TransactionController Integration', () => { }, userFeeLevel: 'dappSuggested', sendFlowHistory: [], - history: [{}, []], }, ], }, }); await advanceTime({ clock, duration: 1 }); - expect(transactionController.state.transactions).toHaveLength(2); - expect(transactionController.state.transactions[0].status).toBe( - 'submitted', - ); - expect(transactionController.state.transactions[1].status).toBe( - 'submitted', - ); + expect(transactionController.state.transactions).toMatchObject([ + expect.objectContaining({ + status: 'submitted', + }), + expect.objectContaining({ + status: 'submitted', + }), + ]); transactionController.destroy(); }); }); + describe('multichain transaction lifecycle', () => { describe('when a transaction is added with a networkClientId that does not match the globally selected network', () => { it('should add a new unapproved transaction', async () => { @@ -292,7 +334,9 @@ describe('TransactionController Integration', () => { buildEthGasPriceRequestMock(), ], }); - const { transactionController } = await newController({}); + const { transactionController } = await setupController({ + isMultichainEnabled: true, + }); await transactionController.addTransaction( { from: ACCOUNT_MOCK, @@ -306,6 +350,7 @@ describe('TransactionController Integration', () => { ); transactionController.destroy(); }); + it('should be able to get to submitted state', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -326,7 +371,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await newController({}); + await setupController({ isMultichainEnabled: true }); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -347,6 +392,7 @@ describe('TransactionController Integration', () => { ); transactionController.destroy(); }); + it('should be able to get to confirmed state', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -369,7 +415,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await newController({}); + await setupController({ isMultichainEnabled: true }); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -394,6 +440,7 @@ describe('TransactionController Integration', () => { ); transactionController.destroy(); }); + it('should be able to send and confirm transactions on different chains', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -436,7 +483,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await newController({}); + await setupController({ isMultichainEnabled: true }); const firstTransaction = await transactionController.addTransaction( { from: ACCOUNT_MOCK, @@ -481,6 +528,7 @@ describe('TransactionController Integration', () => { ).toBe('goerli'); transactionController.destroy(); }); + it('should be able to cancel a transaction', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -509,7 +557,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await newController({}); + await setupController({ isMultichainEnabled: true }); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -532,6 +580,7 @@ describe('TransactionController Integration', () => { ); transactionController.destroy(); }); + it('should be able to confirm a cancelled transaction and drop the original transaction', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -565,10 +614,14 @@ describe('TransactionController Integration', () => { }, buildEthGetTransactionReceiptRequestMock('0x2', '0x2', '0x4'), buildEthGetBlockByHashRequestMock('0x2'), + buildEthSendRawTransactionRequestMock( + '0x02e2050101018252089408f137f335ea1b8f193b8f6ea92561a60d23a2118080c0808080', + '0x1', + ), ], }); const { transactionController, approvalController } = - await newController({}); + await setupController({ isMultichainEnabled: true }); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -602,6 +655,7 @@ describe('TransactionController Integration', () => { ); transactionController.destroy(); }); + it('should be able to get to speedup state and drop the original transaction', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -635,10 +689,14 @@ describe('TransactionController Integration', () => { }, buildEthGetTransactionReceiptRequestMock('0x2', '0x2', '0x4'), buildEthGetBlockByHashRequestMock('0x2'), + buildEthSendRawTransactionRequestMock( + '0x02e605018203e88203e88252089408f137f335ea1b8f193b8f6ea92561a60d23a2118080c0808080', + '0x1', + ), ], }); const { transactionController, approvalController } = - await newController({}); + await setupController({ isMultichainEnabled: true }); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -727,8 +785,9 @@ describe('TransactionController Integration', () => { }); const { approvalController, networkController, transactionController } = - await newController({ - getPermittedAccounts: () => [ACCOUNT_MOCK], + await setupController({ + isMultichainEnabled: true, + getPermittedAccounts: async () => [ACCOUNT_MOCK], getSelectedAddress: () => ACCOUNT_MOCK, }); const otherNetworkClientIdOnGoerli = @@ -807,8 +866,9 @@ describe('TransactionController Integration', () => { ], }); const { approvalController, transactionController } = - await newController({ - getPermittedAccounts: () => [ACCOUNT_MOCK], + await setupController({ + isMultichainEnabled: true, + getPermittedAccounts: async () => [ACCOUNT_MOCK], getSelectedAddress: () => ACCOUNT_MOCK, }); @@ -865,7 +925,7 @@ describe('TransactionController Integration', () => { ], }); const { networkController, transactionController } = - await newController(); + await setupController({ isMultichainEnabled: true }); const otherNetworkClientIdOnGoerli = await networkController.upsertNetworkConfiguration( @@ -896,7 +956,7 @@ describe('TransactionController Integration', () => { }); it('should stop tracking when a network is removed', async () => { const { networkController, transactionController } = - await newController(); + await setupController(); const configurationId = await networkController.upsertNetworkConfiguration( @@ -942,9 +1002,10 @@ describe('TransactionController Integration', () => { ], }); - const { networkController, transactionController } = await newController({ - isMultichainEnabled: false, - }); + const { networkController, transactionController } = + await setupController({ + isMultichainEnabled: false, + }); const configurationId = await networkController.upsertNetworkConfiguration( @@ -988,10 +1049,11 @@ describe('TransactionController Integration', () => { }; }); - const { networkController, transactionController } = await newController({ - isMultichainEnabled: false, - getNetworkClientRegistrySpy, - }); + const { networkController, transactionController } = + await setupController({ + isMultichainEnabled: false, + getNetworkClientRegistry: getNetworkClientRegistrySpy, + }); await networkController.upsertNetworkConfiguration( customGoerliNetworkClientConfiguration, @@ -1014,10 +1076,11 @@ describe('TransactionController Integration', () => { }; }); - const { networkController, transactionController } = await newController({ - isMultichainEnabled: true, - getNetworkClientRegistrySpy, - }); + const { networkController, transactionController } = + await setupController({ + isMultichainEnabled: true, + getNetworkClientRegistry: getNetworkClientRegistrySpy, + }); await networkController.upsertNetworkConfiguration( customGoerliNetworkClientConfiguration, @@ -1040,9 +1103,9 @@ describe('TransactionController Integration', () => { }; }); - await newController({ + await setupController({ isMultichainEnabled: true, - getNetworkClientRegistrySpy, + getNetworkClientRegistry: getNetworkClientRegistrySpy, }); expect(getNetworkClientRegistrySpy).toHaveBeenCalled(); @@ -1064,9 +1127,11 @@ describe('TransactionController Integration', () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const { networkController, transactionController } = await newController({ - getSelectedAddress: () => selectedAddress, - }); + const { networkController, transactionController } = + await setupController({ + getSelectedAddress: () => selectedAddress, + isMultichainEnabled: true, + }); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1146,7 +1211,7 @@ describe('TransactionController Integration', () => { ) .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); - const { transactionController } = await newController({ + const { transactionController } = await setupController({ getSelectedAddress: () => selectedAddress, }); @@ -1237,8 +1302,9 @@ describe('TransactionController Integration', () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; const { networkController, transactionController } = - await newController({ + await setupController({ getSelectedAddress: () => selectedAddress, + isMultichainEnabled: true, }); const otherGoerliClientNetworkClientId = @@ -1331,9 +1397,10 @@ describe('TransactionController Integration', () => { it('should not poll for new incoming transactions for the given networkClientId', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const { networkController, transactionController } = await newController({ - getSelectedAddress: () => selectedAddress, - }); + const { networkController, transactionController } = + await setupController({ + getSelectedAddress: () => selectedAddress, + }); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1374,7 +1441,7 @@ describe('TransactionController Integration', () => { it('should stop the global incoming transaction helper when no networkClientIds provided', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const { transactionController } = await newController({ + const { transactionController } = await setupController({ getSelectedAddress: () => selectedAddress, }); @@ -1410,9 +1477,10 @@ describe('TransactionController Integration', () => { it('should not poll for incoming transactions on any network client', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const { networkController, transactionController } = await newController({ - getSelectedAddress: () => selectedAddress, - }); + const { networkController, transactionController } = + await setupController({ + getSelectedAddress: () => selectedAddress, + }); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1453,9 +1521,11 @@ describe('TransactionController Integration', () => { it('should add incoming transactions to state with the correct chainId for the given networkClientId without waiting for the next block', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const { networkController, transactionController } = await newController({ - getSelectedAddress: () => selectedAddress, - }); + const { networkController, transactionController } = + await setupController({ + getSelectedAddress: () => selectedAddress, + isMultichainEnabled: true, + }); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1517,7 +1587,7 @@ describe('TransactionController Integration', () => { it('should update the incoming transactions for the gloablly selected network when no networkClientIds provided', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const { transactionController } = await newController({ + const { transactionController } = await setupController({ getSelectedAddress: () => selectedAddress, }); @@ -1571,9 +1641,8 @@ describe('TransactionController Integration', () => { describe('getNonceLock', () => { it('should get the nonce lock from the nonceTracker for the given networkClientId', async () => { - const { networkController, transactionController } = await newController( - {}, - ); + const { networkController, transactionController } = + await setupController({ isMultichainEnabled: true }); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1607,9 +1676,8 @@ describe('TransactionController Integration', () => { }); it('should block attempts to get the nonce lock for the same address from the nonceTracker for the networkClientId until the previous lock is released', async () => { - const { networkController, transactionController } = await newController( - {}, - ); + const { networkController, transactionController } = + await setupController({ isMultichainEnabled: true }); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1668,9 +1736,8 @@ describe('TransactionController Integration', () => { }); it('should block attempts to get the nonce lock for the same address from the nonceTracker for the different networkClientIds on the same chainId until the previous lock is released', async () => { - const { networkController, transactionController } = await newController( - {}, - ); + const { networkController, transactionController } = + await setupController({ isMultichainEnabled: true }); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( InfuraNetworkType.goerli, @@ -1737,7 +1804,9 @@ describe('TransactionController Integration', () => { }); it('should not block attempts to get the nonce lock for the same addresses from the nonceTracker for different networkClientIds', async () => { - const { transactionController } = await newController({}); + const { transactionController } = await setupController({ + isMultichainEnabled: true, + }); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -1783,9 +1852,8 @@ describe('TransactionController Integration', () => { }); it('should not block attempts to get the nonce lock for different addresses from the nonceTracker for the networkClientId', async () => { - const { networkController, transactionController } = await newController( - {}, - ); + const { networkController, transactionController } = + await setupController({ isMultichainEnabled: true }); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1844,7 +1912,7 @@ describe('TransactionController Integration', () => { ], }); - const { transactionController } = await newController({}); + const { transactionController } = await setupController({}); const nonceLockPromise = transactionController.getNonceLock(ACCOUNT_MOCK); await advanceTime({ clock, duration: 1 }); @@ -1866,7 +1934,7 @@ describe('TransactionController Integration', () => { ], }); - const { transactionController } = await newController({}); + const { transactionController } = await setupController({}); const firstNonceLockPromise = transactionController.getNonceLock(ACCOUNT_MOCK); @@ -1912,7 +1980,7 @@ describe('TransactionController Integration', () => { ], }); - const { transactionController } = await newController({}); + const { transactionController } = await setupController({}); const firstNonceLockPromise = transactionController.getNonceLock(ACCOUNT_MOCK); diff --git a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts index ebddb6b370..cab6442cd6 100644 --- a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts +++ b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts @@ -1,5 +1,7 @@ +import * as sinon from 'sinon'; import { v1 as random } from 'uuid'; +import { advanceTime } from '../../../../tests/helpers'; import { ID_MOCK, ETHERSCAN_TRANSACTION_RESPONSE_EMPTY_MOCK, @@ -27,6 +29,8 @@ jest.mock('../utils/etherscan', () => ({ jest.mock('uuid'); describe('EtherscanRemoteTransactionSource', () => { + let clock: sinon.SinonFakeTimers; + const fetchEtherscanTransactionsMock = fetchEtherscanTransactions as jest.MockedFn< typeof fetchEtherscanTransactions @@ -42,6 +46,8 @@ describe('EtherscanRemoteTransactionSource', () => { beforeEach(() => { jest.resetAllMocks(); + clock = sinon.useFakeTimers(); + fetchEtherscanTransactionsMock.mockResolvedValue( ETHERSCAN_TRANSACTION_RESPONSE_EMPTY_MOCK, ); @@ -53,6 +59,10 @@ describe('EtherscanRemoteTransactionSource', () => { randomMock.mockReturnValue(ID_MOCK); }); + afterEach(() => { + clock.restore(); + }); + describe('isSupportedNetwork', () => { it('returns true if chain ID in constant', () => { expect( @@ -126,9 +136,13 @@ describe('EtherscanRemoteTransactionSource', () => { const remoteSource = new EtherscanRemoteTransactionSource(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); + const promiseForFetchTransactions = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -151,27 +165,43 @@ describe('EtherscanRemoteTransactionSource', () => { const remoteSource = new EtherscanRemoteTransactionSource(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); + const promiseForFetchTransactions1 = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions1; expect(fetchEtherscanTransactionsMock).toHaveBeenCalledTimes(1); expect(fetchEtherscanTokenTransactionsMock).toHaveBeenCalledTimes(0); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); + const promiseForFetchTransactions2 = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions2; expect(fetchEtherscanTransactionsMock).toHaveBeenCalledTimes(1); expect(fetchEtherscanTokenTransactionsMock).toHaveBeenCalledTimes(1); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); + const promiseForFetchTransactions3 = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions3; expect(fetchEtherscanTransactionsMock).toHaveBeenCalledTimes(2); expect(fetchEtherscanTokenTransactionsMock).toHaveBeenCalledTimes(1); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); + const promiseForFetchTransactions4 = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions4; expect(fetchEtherscanTransactionsMock).toHaveBeenCalledTimes(2); expect(fetchEtherscanTokenTransactionsMock).toHaveBeenCalledTimes(2); @@ -191,12 +221,20 @@ describe('EtherscanRemoteTransactionSource', () => { includeTokenTransfers: false, }); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); + const promiseForFetchTransactions1 = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions1; + const promiseForFetchTransactions2 = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions2; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any await remoteSource.fetchTransactions({} as any); @@ -236,9 +274,13 @@ describe('EtherscanRemoteTransactionSource', () => { fetchEtherscanTokenTransactionsMock.mockResolvedValueOnce(response); const remoteSource = new EtherscanRemoteTransactionSource(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await remoteSource.fetchTransactions({} as any); + const promiseForFetchTransactions = remoteSource.fetchTransactions( + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + await advanceTime({ clock, duration: 5000 }); + await promiseForFetchTransactions; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/transaction-controller/src/helpers/GasFeePoller.ts b/packages/transaction-controller/src/helpers/GasFeePoller.ts index 52343389b5..4dcdd13324 100644 --- a/packages/transaction-controller/src/helpers/GasFeePoller.ts +++ b/packages/transaction-controller/src/helpers/GasFeePoller.ts @@ -155,14 +155,17 @@ export class GasFeePoller { return; } - transactionMeta.gasFeeEstimates = gasFeeEstimates; - transactionMeta.gasFeeEstimatesLoaded = true; + const updatedTransactionMeta: TransactionMeta = { + ...transactionMeta, + gasFeeEstimates, + gasFeeEstimatesLoaded: true, + }; - this.hub.emit('transaction-updated', transactionMeta); + this.hub.emit('transaction-updated', updatedTransactionMeta); log('Updated suggested gas fees', { - gasFeeEstimates: transactionMeta.gasFeeEstimates, - transaction: transactionMeta.id, + gasFeeEstimates: updatedTransactionMeta.gasFeeEstimates, + transaction: updatedTransactionMeta.id, }); } diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index bd8b66aeaf..c6600b4893 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -290,9 +290,11 @@ export class IncomingTransactionHelper { return; } - lastFetchedBlockNumbers[lastFetchedKey] = lastFetchedBlockNumber; this.hub.emit('updatedLastFetchedBlockNumbers', { - lastFetchedBlockNumbers, + lastFetchedBlockNumbers: { + ...lastFetchedBlockNumbers, + [lastFetchedKey]: lastFetchedBlockNumber, + }, blockNumber: lastFetchedBlockNumber, }); } diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts index ff7c244ddc..846f8fa9fe 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts @@ -2,6 +2,7 @@ import { query } from '@metamask/controller-utils'; import type { BlockTracker } from '@metamask/network-controller'; +import { freeze } from 'immer'; import type { TransactionMeta } from '../types'; import { TransactionStatus } from '../types'; @@ -73,7 +74,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker.startIfPendingTransactions(); if (transactionsOnCheck) { - options.getTransactions.mockReturnValue(transactionsOnCheck); + options.getTransactions.mockReturnValue( + freeze(transactionsOnCheck, true), + ); } await blockTracker.on.mock.calls[0][1](latestBlockNumber); @@ -101,7 +104,9 @@ describe('PendingTransactionTracker', () => { it('adds block tracker listener if pending transactions', () => { pendingTransactionTracker = new PendingTransactionTracker(options); - options.getTransactions.mockReturnValue([TRANSACTION_SUBMITTED_MOCK]); + options.getTransactions.mockReturnValue( + freeze([TRANSACTION_SUBMITTED_MOCK], true), + ); pendingTransactionTracker.startIfPendingTransactions(); @@ -115,7 +120,9 @@ describe('PendingTransactionTracker', () => { it('does nothing if block tracker listener already added', () => { pendingTransactionTracker = new PendingTransactionTracker(options); - options.getTransactions.mockReturnValue([TRANSACTION_SUBMITTED_MOCK]); + options.getTransactions.mockReturnValue( + freeze([TRANSACTION_SUBMITTED_MOCK], true), + ); pendingTransactionTracker.startIfPendingTransactions(); pendingTransactionTracker.startIfPendingTransactions(); @@ -127,7 +134,9 @@ describe('PendingTransactionTracker', () => { it('removes block tracker listener if no pending transactions and running', () => { pendingTransactionTracker = new PendingTransactionTracker(options); - options.getTransactions.mockReturnValue([TRANSACTION_SUBMITTED_MOCK]); + options.getTransactions.mockReturnValue( + freeze([TRANSACTION_SUBMITTED_MOCK], true), + ); pendingTransactionTracker.startIfPendingTransactions(); @@ -147,7 +156,9 @@ describe('PendingTransactionTracker', () => { it('does nothing if block tracker listener already removed', () => { pendingTransactionTracker = new PendingTransactionTracker(options); - options.getTransactions.mockReturnValue([TRANSACTION_SUBMITTED_MOCK]); + options.getTransactions.mockReturnValue( + freeze([TRANSACTION_SUBMITTED_MOCK], true), + ); pendingTransactionTracker.startIfPendingTransactions(); @@ -217,10 +228,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => + freeze([{ ...TRANSACTION_SUBMITTED_MOCK }], true), + }); pendingTransactionTracker.hub.addListener( 'transaction-dropped', @@ -248,10 +258,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => + freeze([{ ...TRANSACTION_SUBMITTED_MOCK }], true), + }); pendingTransactionTracker.hub.addListener( 'transaction-dropped', @@ -279,10 +288,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => + freeze([{ ...TRANSACTION_SUBMITTED_MOCK }], true), + }); pendingTransactionTracker.hub.addListener( 'transaction-dropped', @@ -317,10 +325,8 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transactionMetaMock], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => freeze([transactionMetaMock], true), + }); pendingTransactionTracker.hub.addListener( 'transaction-failed', @@ -348,14 +354,12 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transactionMetaMock], + getTransactions: () => freeze([transactionMetaMock], true), hooks: { beforeCheckPendingTransaction: () => false, beforePublish: () => false, }, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + }); pendingTransactionTracker.hub.addListener( 'transaction-failed', @@ -372,10 +376,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => + freeze([{ ...TRANSACTION_SUBMITTED_MOCK }], true), + }); pendingTransactionTracker.hub.addListener( 'transaction-failed', @@ -410,13 +413,12 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [ - confirmedTransactionMetaMock, - submittedTransactionMetaMock, - ], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => + freeze( + [confirmedTransactionMetaMock, submittedTransactionMetaMock], + true, + ), + }); pendingTransactionTracker.hub.addListener( 'transaction-dropped', @@ -434,10 +436,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => + freeze([{ ...TRANSACTION_SUBMITTED_MOCK }], true), + }); pendingTransactionTracker.hub.addListener( 'transaction-dropped', @@ -477,9 +478,7 @@ describe('PendingTransactionTracker', () => { confirmedTransactionMetaMock, submittedTransactionMetaMock, ], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + }); pendingTransactionTracker.hub.addListener( 'transaction-dropped', @@ -494,15 +493,17 @@ describe('PendingTransactionTracker', () => { describe('fires confirmed event', () => { it('if receipt has success status', async () => { - const listener = jest.fn(); + const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; + const getTransactions = jest + .fn() + .mockReturnValue(freeze([transaction], true)); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); + const listener = jest.fn(); pendingTransactionTracker.hub.addListener( 'transaction-confirmed', listener, @@ -514,28 +515,35 @@ describe('PendingTransactionTracker', () => { await onLatestBlock(); expect(listener).toHaveBeenCalledTimes(1); - expect(listener).toHaveBeenCalledWith({ - ...TRANSACTION_SUBMITTED_MOCK, - baseFeePerGas: BLOCK_MOCK.baseFeePerGas, - blockTimestamp: BLOCK_MOCK.timestamp, - status: TransactionStatus.confirmed, - txReceipt: RECEIPT_MOCK, - verifiedOnBlockchain: true, - }); + expect(listener).toHaveBeenCalledWith( + expect.objectContaining({ + ...TRANSACTION_SUBMITTED_MOCK, + txParams: expect.objectContaining( + TRANSACTION_SUBMITTED_MOCK.txParams, + ), + baseFeePerGas: BLOCK_MOCK.baseFeePerGas, + blockTimestamp: BLOCK_MOCK.timestamp, + status: TransactionStatus.confirmed, + txReceipt: RECEIPT_MOCK, + verifiedOnBlockchain: true, + }), + ); }); }); describe('fires updated event', () => { it('if receipt has success status', async () => { - const listener = jest.fn(); + const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; + const getTransactions = jest + .fn() + .mockReturnValue(freeze([transaction], true)); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); + const listener = jest.fn(); pendingTransactionTracker.hub.addListener( 'transaction-updated', listener, @@ -548,29 +556,33 @@ describe('PendingTransactionTracker', () => { expect(listener).toHaveBeenCalledTimes(2); expect(listener).toHaveBeenCalledWith( - { + expect.objectContaining({ ...TRANSACTION_SUBMITTED_MOCK, + txParams: expect.objectContaining( + TRANSACTION_SUBMITTED_MOCK.txParams, + ), baseFeePerGas: BLOCK_MOCK.baseFeePerGas, blockTimestamp: BLOCK_MOCK.timestamp, status: TransactionStatus.confirmed, txReceipt: RECEIPT_MOCK, verifiedOnBlockchain: true, - }, + }), 'PendingTransactionTracker:#onTransactionConfirmed - Transaction confirmed', ); }); it('if getTransactionReceipt fails', async () => { - const listener = jest.fn(); const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; + const getTransactions = jest + .fn() + .mockReturnValue(freeze([transaction], true)); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transaction], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); + const listener = jest.fn(); pendingTransactionTracker.hub.addListener( 'transaction-updated', listener, @@ -580,17 +592,28 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(BLOCK_MOCK); await onLatestBlock(BLOCK_NUMBER_MOCK); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); expect(listener).toHaveBeenCalledTimes(2); - expect(listener).toHaveBeenCalledWith( - { + expect(listener).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ ...TRANSACTION_SUBMITTED_MOCK, - firstRetryBlockNumber: BLOCK_NUMBER_MOCK, warning: { error: 'TestError', message: 'There was a problem loading this transaction.', }, - }, + }), 'PendingTransactionTracker:#warnTransaction - Warning added', ); }); @@ -615,10 +638,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [{ ...TRANSACTION_SUBMITTED_MOCK }], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions: () => + freeze([{ ...TRANSACTION_SUBMITTED_MOCK }], true), + }); pendingTransactionTracker.hub.addListener( 'transaction-updated', @@ -641,16 +663,17 @@ describe('PendingTransactionTracker', () => { }); it('if published', async () => { - const listener = jest.fn(); const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; + const getTransactions = jest + .fn() + .mockReturnValue(freeze([transaction], true)); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transaction], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); + const listener = jest.fn(); pendingTransactionTracker.hub.addListener( 'transaction-updated', listener, @@ -660,6 +683,17 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce('0x1'); await onLatestBlock(BLOCK_NUMBER_MOCK); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); await onLatestBlock('0x124'); expect(listener).toHaveBeenCalledTimes(2); @@ -674,22 +708,21 @@ describe('PendingTransactionTracker', () => { }); it('if beforePublish returns false, does not resubmit the transaction', async () => { - const listener = jest.fn(); - const transaction = { - ...TRANSACTION_SUBMITTED_MOCK, - }; + const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; + const getTransactions = jest + .fn() + .mockReturnValue(freeze([transaction], true)); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transaction], + getTransactions, hooks: { beforeCheckPendingTransaction: () => false, beforePublish: () => false, }, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + }); + const listener = jest.fn(); pendingTransactionTracker.hub.addListener( 'transaction-updated', listener, @@ -699,6 +732,17 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce('0x1'); await onLatestBlock(BLOCK_NUMBER_MOCK); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); await onLatestBlock('0x124'); expect(listener).toHaveBeenCalledTimes(1); @@ -713,16 +757,17 @@ describe('PendingTransactionTracker', () => { }); it('if publishing fails', async () => { - const listener = jest.fn(); const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; + const getTransactions = jest + .fn() + .mockReturnValue(freeze([transaction], true)); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transaction], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); + const listener = jest.fn(); pendingTransactionTracker.hub.addListener( 'transaction-updated', listener, @@ -736,6 +781,17 @@ describe('PendingTransactionTracker', () => { ); await onLatestBlock(BLOCK_NUMBER_MOCK); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); await onLatestBlock('0x124'); expect(listener).toHaveBeenCalledTimes(2); @@ -754,16 +810,17 @@ describe('PendingTransactionTracker', () => { }); it('unless publishing fails and known error', async () => { - const listener = jest.fn(); const transaction = { ...TRANSACTION_SUBMITTED_MOCK }; + const getTransactions = jest + .fn() + .mockReturnValue(freeze([transaction], true)); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transaction], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); + const listener = jest.fn(); pendingTransactionTracker.hub.addListener( 'transaction-updated', listener, @@ -777,6 +834,17 @@ describe('PendingTransactionTracker', () => { ); await onLatestBlock(BLOCK_NUMBER_MOCK); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); await onLatestBlock('0x124'); expect(listener).toHaveBeenCalledTimes(1); @@ -790,18 +858,30 @@ describe('PendingTransactionTracker', () => { 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: () => [transaction], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); await onLatestBlock(BLOCK_NUMBER_MOCK); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); await onLatestBlock('0x124'); expect(options.publishTransaction).toHaveBeenCalledTimes(1); @@ -813,31 +893,79 @@ describe('PendingTransactionTracker', () => { 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: () => [transaction], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); await onLatestBlock(BLOCK_NUMBER_MOCK); expect(options.publishTransaction).toHaveBeenCalledTimes(0); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); await onLatestBlock('0x124'); expect(options.publishTransaction).toHaveBeenCalledTimes(1); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + retryCount: 1, + }, + ], + true, + ), + ); await onLatestBlock('0x125'); expect(options.publishTransaction).toHaveBeenCalledTimes(2); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + retryCount: 2, + }, + ], + true, + ), + ); await onLatestBlock('0x126'); expect(options.publishTransaction).toHaveBeenCalledTimes(2); await onLatestBlock('0x127'); expect(options.publishTransaction).toHaveBeenCalledTimes(3); + getTransactions.mockReturnValue( + freeze( + [ + { + ...transaction, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + retryCount: 3, + }, + ], + true, + ), + ); await onLatestBlock('0x12A'); expect(options.publishTransaction).toHaveBeenCalledTimes(3); @@ -851,11 +979,9 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transaction], + getTransactions: () => freeze([transaction], true), isResubmitEnabled: false, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + }); queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); @@ -869,22 +995,39 @@ describe('PendingTransactionTracker', () => { describe('approves transaction', () => { it('if no raw transaction', async () => { - const transaction = { - ...TRANSACTION_SUBMITTED_MOCK, - rawTx: undefined, - }; + const getTransactions = jest.fn().mockReturnValue( + freeze( + [ + { + ...TRANSACTION_SUBMITTED_MOCK, + rawTx: undefined, + }, + ], + true, + ), + ); pendingTransactionTracker = new PendingTransactionTracker({ ...options, - getTransactions: () => [transaction], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); + getTransactions, + }); queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); await onLatestBlock(BLOCK_NUMBER_MOCK); + getTransactions.mockReturnValue( + freeze( + [ + { + ...TRANSACTION_SUBMITTED_MOCK, + rawTx: undefined, + firstRetryBlockNumber: BLOCK_NUMBER_MOCK, + }, + ], + true, + ), + ); await onLatestBlock('0x124'); expect(options.approveTransaction).toHaveBeenCalledTimes(1); @@ -913,11 +1056,22 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(BLOCK_MOCK); options.getTransactions.mockReturnValue([]); + const listener = jest.fn(); + tracker.hub.addListener('transaction-updated', listener); + await tracker.forceCheckTransaction(transactionMeta); - expect(transactionMeta.status).toStrictEqual(TransactionStatus.confirmed); - expect(transactionMeta.txReceipt).toStrictEqual(RECEIPT_MOCK); - expect(transactionMeta.verifiedOnBlockchain).toBe(true); + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith( + expect.objectContaining({ + ...transactionMeta, + txParams: expect.objectContaining(transactionMeta.txParams), + status: TransactionStatus.confirmed, + txReceipt: RECEIPT_MOCK, + verifiedOnBlockchain: true, + }), + 'PendingTransactionTracker:#onTransactionConfirmed - Transaction confirmed', + ); }); it('should fail transaction if receipt status is failure', async () => { diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index c23bfc3e8c..19c738bdec 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -4,10 +4,10 @@ import type { BlockTracker, NetworkClientId, } from '@metamask/network-controller'; -import { createModuleLogger } from '@metamask/utils'; import EventEmitter from 'events'; +import { cloneDeep, merge } from 'lodash'; -import { projectLogger } from '../logger'; +import { createModuleLogger, projectLogger } from '../logger'; import type { TransactionMeta, TransactionReceipt } from '../types'; import { TransactionStatus, TransactionType } from '../types'; @@ -278,8 +278,6 @@ export class PendingTransactionTracker { return; } - log('Resubmitting transaction', txMeta.id); - const { rawTx } = txMeta; if (!this.#beforePublish(txMeta)) { @@ -295,26 +293,27 @@ export class PendingTransactionTracker { const ethQuery = this.#getEthQuery(txMeta.networkClientId); await this.#publishTransaction(ethQuery, rawTx); - txMeta.retryCount = (txMeta.retryCount ?? 0) + 1; + const retryCount = (txMeta.retryCount ?? 0) + 1; this.#updateTransaction( - txMeta, + merge({}, txMeta, { retryCount }), 'PendingTransactionTracker:transaction-retry - Retry count increased', ); } #isResubmitDue(txMeta: TransactionMeta, latestBlockNumber: string): boolean { - if (!txMeta.firstRetryBlockNumber) { - txMeta.firstRetryBlockNumber = latestBlockNumber; + const txMetaWithFirstRetryBlockNumber = cloneDeep(txMeta); + + if (!txMetaWithFirstRetryBlockNumber.firstRetryBlockNumber) { + txMetaWithFirstRetryBlockNumber.firstRetryBlockNumber = latestBlockNumber; this.#updateTransaction( - txMeta, + txMetaWithFirstRetryBlockNumber, 'PendingTransactionTracker:#isResubmitDue - First retry block number set', ); } - const firstRetryBlockNumber = - txMeta.firstRetryBlockNumber || latestBlockNumber; + const { firstRetryBlockNumber } = txMetaWithFirstRetryBlockNumber; const blocksSinceFirstRetry = Number.parseInt(latestBlockNumber, 16) - @@ -411,19 +410,23 @@ export class PendingTransactionTracker { const { baseFeePerGas, timestamp: blockTimestamp } = await this.#getBlockByHash(blockHash, false); - txMeta.baseFeePerGas = baseFeePerGas; - txMeta.blockTimestamp = blockTimestamp; - txMeta.status = TransactionStatus.confirmed; - txMeta.txParams.gasUsed = receipt.gasUsed; - txMeta.txReceipt = receipt; - txMeta.verifiedOnBlockchain = true; + const updatedTxMeta = cloneDeep(txMeta); + updatedTxMeta.baseFeePerGas = baseFeePerGas; + updatedTxMeta.blockTimestamp = blockTimestamp; + updatedTxMeta.status = TransactionStatus.confirmed; + updatedTxMeta.txParams = { + ...updatedTxMeta.txParams, + gasUsed: receipt.gasUsed, + }; + updatedTxMeta.txReceipt = receipt; + updatedTxMeta.verifiedOnBlockchain = true; this.#updateTransaction( - txMeta, + updatedTxMeta, 'PendingTransactionTracker:#onTransactionConfirmed - Transaction confirmed', ); - this.hub.emit('transaction-confirmed', txMeta); + this.hub.emit('transaction-confirmed', updatedTxMeta); } async #isTransactionDropped(txMeta: TransactionMeta) { @@ -488,13 +491,11 @@ export class PendingTransactionTracker { } #warnTransaction(txMeta: TransactionMeta, error: string, message: string) { - txMeta.warning = { - error, - message, - }; - this.#updateTransaction( - txMeta, + { + ...txMeta, + warning: { error, message }, + }, 'PendingTransactionTracker:#warnTransaction - Warning added', ); } diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index c434c29b9f..ea6a577d35 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -1,9 +1,64 @@ -export * from './TransactionController'; +export type { + FeeMarketEIP1559Values, + GasPriceValue, + MethodData, + Result, + TransactionControllerActions, + TransactionControllerEvents, + TransactionControllerGetStateAction, + TransactionControllerIncomingTransactionBlockReceivedEvent, + TransactionControllerPostTransactionBalanceUpdatedEvent, + TransactionControllerSpeedupTransactionAddedEvent, + TransactionControllerState, + TransactionControllerStateChangeEvent, + TransactionControllerTransactionApprovedEvent, + TransactionControllerTransactionConfirmedEvent, + TransactionControllerTransactionDroppedEvent, + TransactionControllerTransactionFailedEvent, + TransactionControllerTransactionFinishedEvent, + TransactionControllerTransactionNewSwapApprovalEvent, + TransactionControllerTransactionNewSwapEvent, + TransactionControllerTransactionPublishingSkipped, + TransactionControllerTransactionRejectedEvent, + TransactionControllerTransactionStatusUpdatedEvent, + TransactionControllerTransactionSubmittedEvent, + TransactionControllerUnapprovedTransactionAddedEvent, + TransactionControllerMessenger, +} from './TransactionController'; +export { + HARDFORK, + CANCEL_RATE, + TransactionController, +} from './TransactionController'; +export type { + DappSuggestedGasFees, + DefaultGasEstimates, + InferTransactionTypeResult, + Log, + RemoteTransactionSource, + RemoteTransactionSourceRequest, + SavedGasFees, + SecurityAlertResponse, + SecurityProviderRequest, + SendFlowHistoryEntry, + TransactionError, + TransactionHistory, + TransactionHistoryEntry, + TransactionMeta, + TransactionParams, + TransactionReceipt, +} from './types'; +export { + TransactionEnvelopeType, + TransactionStatus, + TransactionType, + UserFeeLevel, + WalletDevice, +} from './types'; export type { EtherscanTransactionMeta } from './utils/etherscan'; +export { determineTransactionType } from './utils/transaction-type'; +export { mergeGasFeeEstimates } from './utils/gas-flow'; export { isEIP1559Transaction, normalizeTransactionParams, } from './utils/utils'; -export * from './types'; -export { determineTransactionType } from './utils/transaction-type'; -export { mergeGasFeeEstimates } from './utils/gas-flow'; diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 0d7a9fa564..5838bd108a 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -5,52 +5,34 @@ import type { GasFeeState, } from '@metamask/gas-fee-controller'; import type { NetworkClientId } from '@metamask/network-controller'; -import type { Hex } from '@metamask/utils'; +import type { Hex, Json } from '@metamask/utils'; import type { Operation } from 'fast-json-patch'; -export type Events = { - ['incomingTransactionBlock']: [blockNumber: number]; - ['post-transaction-balance-updated']: [ - { - transactionMeta: TransactionMeta; - approvalTransactionMeta?: TransactionMeta; - }, - ]; - ['transaction-approved']: [ - { transactionMeta: TransactionMeta; actionId?: string }, - ]; - ['transaction-confirmed']: [{ transactionMeta: TransactionMeta }]; - - ['transaction-dropped']: [{ transactionMeta: TransactionMeta }]; - ['transaction-failed']: [ - { - actionId?: string; - error: string; - transactionMeta: TransactionMeta; - }, - ]; - ['transaction-new-swap']: [{ transactionMeta: TransactionMeta }]; - ['transaction-new-swap-approval']: [{ transactionMeta: TransactionMeta }]; - ['transaction-rejected']: [ - { transactionMeta: TransactionMeta; actionId?: string }, - ]; - ['transaction-status-update']: [{ transactionMeta: TransactionMeta }]; - ['transaction-submitted']: [ - { transactionMeta: TransactionMeta; actionId?: string }, - ]; - ['unapprovedTransaction']: [transactionMeta: TransactionMeta]; - [key: `${string}:confirmed`]: [transactionMeta: TransactionMeta]; - [key: `${string}:finished`]: [transactionMeta: TransactionMeta]; - [key: `${string}:publish-skip`]: [tansactionMeta: TransactionMeta]; - [key: `${string}:speedup`]: [transactionMeta: TransactionMeta]; -}; +/** + * Given a record, ensures that each property matches the `Json` type. + */ +type MakeJsonCompatible = T extends Json + ? T + : { + [K in keyof T]: T[K] extends Json ? T[K] : never; + }; + +/** + * `Json` from `@metamask/utils` is defined as a recursive type alias, but + * `Operation` is defined as an interface, and the two are not compatible with + * each other. Therefore, this is a variant of Operation from `fast-json-patch` + * which is guaranteed to be type-compatible with `Json`. + */ +type JsonCompatibleOperation = MakeJsonCompatible; /** * Representation of transaction metadata. */ export type TransactionMeta = TransactionMetaBase & ( - | { status: Exclude } + | { + status: Exclude; + } | { status: TransactionStatus.failed; error: TransactionError; @@ -579,10 +561,7 @@ export enum TransactionType { /** * Standard data concerning a transaction to be processed by the blockchain. */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TransactionParams { +export type TransactionParams = { /** * A list of addresses and storage keys that the transaction plans to access. */ @@ -674,15 +653,12 @@ export interface TransactionParams { * 0x0 indicates a legacy transaction. */ type?: string; -} +}; /** * Standard data concerning a transaction processed by the blockchain. */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TransactionReceipt { +export type TransactionReceipt = { /** * The block hash of the block that this transaction was included in. */ @@ -722,15 +698,12 @@ export interface TransactionReceipt { * The hexadecimal index of this transaction in the list of transactions included in the block this transaction was mined in. */ transactionIndex?: string; -} +}; /** * Represents an event that has been included in a transaction using the EVM `LOG` opcode. */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface Log { +export type Log = { /** * Address of the contract that generated log. */ @@ -739,7 +712,7 @@ export interface Log { * List of topics for log. */ topics?: string; -} +}; /** * The configuration required to fetch transaction data from a RemoteTransactionSource. @@ -800,15 +773,12 @@ export interface RemoteTransactionSource { /** * Gas values initially suggested by the dApp. */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface DappSuggestedGasFees { +export type DappSuggestedGasFees = { gas?: string; gasPrice?: string; maxFeePerGas?: string; maxPriorityFeePerGas?: string; -} +}; /** * Gas values saved by the user for a specific chain. @@ -823,7 +793,7 @@ export interface SavedGasFees { /** * A transaction history operation that includes a note and timestamp. */ -type ExtendedHistoryOperation = Operation & { +type ExtendedHistoryOperation = JsonCompatibleOperation & { note?: string; timestamp?: number; }; @@ -833,7 +803,7 @@ type ExtendedHistoryOperation = Operation & { */ export type TransactionHistoryEntry = [ ExtendedHistoryOperation, - ...Operation[], + ...JsonCompatibleOperation[], ]; /** @@ -965,7 +935,12 @@ export type TransactionError = { /** * The rpc property holds additional information related to the error. */ - rpc?: unknown; + // We are intentionally using `any` here instead of `Json` because it causes + // `WritableDraft` from Immer to cause TypeScript to error + // with "Type instantiation is excessively deep and possibly infinite". See: + // + // eslint-disable-next-line @typescript-eslint/no-explicit-any + rpc?: any; }; /** diff --git a/packages/transaction-controller/src/utils/history.ts b/packages/transaction-controller/src/utils/history.ts index f4819753e7..19759a2886 100644 --- a/packages/transaction-controller/src/utils/history.ts +++ b/packages/transaction-controller/src/utils/history.ts @@ -1,5 +1,5 @@ import jsonDiffer from 'fast-json-patch'; -import { cloneDeep } from 'lodash'; +import { cloneDeep, merge } from 'lodash'; import type { TransactionHistory, @@ -8,39 +8,46 @@ import type { } from '../types'; /** - * Add initial history snapshot to the provided transactionMeta history. + * Build a new version of the provided transaction with an initial history + * entry, which is just a snapshot of the transaction. * * @param transactionMeta - TransactionMeta to add initial history snapshot to. + * @returns A copy of `transactionMeta` with a new `history` property. */ -export function addInitialHistorySnapshot(transactionMeta: TransactionMeta) { +export function addInitialHistorySnapshot( + transactionMeta: TransactionMeta, +): TransactionMeta { const snapshot = snapshotFromTransactionMeta(transactionMeta); - transactionMeta.history = [snapshot]; + return merge({}, transactionMeta, { history: [snapshot] }); } /** - * Compares and adds history entry to the provided transactionMeta history. + * Builds a new version of the transaction with a new history entry if + * it has a `history` property, or just returns the transaction. * * @param transactionMeta - TransactionMeta to add history entry to. * @param note - Note to add to history entry. + * @returns A copy of `transactionMeta` with a new `history` entry if it has an + * existing non-empty `history` array. */ export function updateTransactionHistory( transactionMeta: TransactionMeta, note: string, -): void { +): TransactionMeta { if (!transactionMeta.history) { - return; + return transactionMeta; } const currentState = snapshotFromTransactionMeta(transactionMeta); - const previousState = replayHistory( - transactionMeta.history as TransactionHistory, - ); - + const previousState = replayHistory(transactionMeta.history); const historyEntry = generateHistoryEntry(previousState, currentState, note); - if (historyEntry.length) { - transactionMeta?.history?.push(historyEntry); + if (historyEntry.length > 0) { + return merge({}, transactionMeta, { + history: [...transactionMeta.history, historyEntry], + }); } + return transactionMeta; } /** diff --git a/packages/transaction-controller/src/utils/swaps.test.ts b/packages/transaction-controller/src/utils/swaps.test.ts index 9187a191a0..704441c417 100644 --- a/packages/transaction-controller/src/utils/swaps.test.ts +++ b/packages/transaction-controller/src/utils/swaps.test.ts @@ -1,6 +1,14 @@ +import { ControllerMessenger } from '@metamask/base-controller'; import { query } from '@metamask/controller-utils'; import { CHAIN_IDS } from '../constants'; +import type { + AllowedActions, + AllowedEvents, + TransactionControllerActions, + TransactionControllerEvents, + TransactionControllerMessenger, +} from '../TransactionController'; import type { TransactionMeta } from '../types'; import { TransactionType, TransactionStatus } from '../types'; import { @@ -21,6 +29,7 @@ describe('updateSwapsTransaction', () => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any let request: any; + let messenger: TransactionControllerMessenger; beforeEach(() => { transactionMeta = { @@ -36,35 +45,38 @@ describe('updateSwapsTransaction', () => { destinationTokenSymbol: 'DAI', }, }; + messenger = new ControllerMessenger< + TransactionControllerActions | AllowedActions, + TransactionControllerEvents | AllowedEvents + >().getRestricted({ + name: 'TransactionController', + allowedActions: [ + 'ApprovalController:addRequest', + 'NetworkController:getNetworkClientById', + 'NetworkController:findNetworkClientIdByChainId', + ], + }); request = { isSwapsDisabled: false, cancelTransaction: jest.fn(), - controllerHubEmitter: jest.fn(), + messenger, }; }); it('should not update if swaps are disabled', async () => { request.isSwapsDisabled = true; - await updateSwapsTransaction( - transactionMeta, - transactionType, - swaps, - request, - ); + jest.spyOn(messenger, 'call'); + updateSwapsTransaction(transactionMeta, transactionType, swaps, request); expect(request.cancelTransaction).not.toHaveBeenCalled(); - expect(request.controllerHubEmitter).not.toHaveBeenCalled(); + expect(messenger.call).not.toHaveBeenCalled(); }); it('should not update if transaction type is not swap or swapApproval', async () => { transactionType = TransactionType.deployContract; - await updateSwapsTransaction( - transactionMeta, - transactionType, - swaps, - request, - ); + jest.spyOn(messenger, 'call'); + updateSwapsTransaction(transactionMeta, transactionType, swaps, request); expect(request.cancelTransaction).not.toHaveBeenCalled(); - expect(request.controllerHubEmitter).not.toHaveBeenCalled(); + expect(messenger.call).not.toHaveBeenCalled(); }); it('should cancel transaction if simulation fails', async () => { @@ -73,25 +85,21 @@ describe('updateSwapsTransaction', () => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; - await expect( + expect(() => updateSwapsTransaction(transactionMeta, transactionType, swaps, request), - ).rejects.toThrow('Simulation failed'); + ).toThrow('Simulation failed'); expect(request.cancelTransaction).toHaveBeenCalledWith(transactionMeta.id); }); it('should not update or call swap events if swaps meta is not defined', async () => { swaps.meta = undefined; - await updateSwapsTransaction( - transactionMeta, - transactionType, - swaps, - request, - ); + jest.spyOn(messenger, 'call'); + updateSwapsTransaction(transactionMeta, transactionType, swaps, request); expect(request.cancelTransaction).not.toHaveBeenCalled(); - expect(request.controllerHubEmitter).not.toHaveBeenCalled(); + expect(messenger.call).not.toHaveBeenCalled(); }); - it('should update swap transaction and emit newSwap event', async () => { + it('should update swap transaction and publish TransactionController:transactionNewSwap', async () => { const sourceTokenSymbol = 'ETH'; const destinationTokenSymbol = 'DAI'; const type = TransactionType.swap; @@ -116,33 +124,103 @@ describe('updateSwapsTransaction', () => { approvalTxId, }; - await updateSwapsTransaction( + const transactionNewSwapEventListener = jest.fn(); + messenger.subscribe( + 'TransactionController:transactionNewSwap', + transactionNewSwapEventListener, + ); + + updateSwapsTransaction(transactionMeta, transactionType, swaps, request); + + expect(transactionNewSwapEventListener).toHaveBeenCalledWith({ + transactionMeta: { + ...transactionMeta, + sourceTokenSymbol, + destinationTokenSymbol, + type, + destinationTokenDecimals, + destinationTokenAddress, + swapMetaData, + swapTokenValue, + estimatedBaseFee, + approvalTxId, + }, + }); + }); + + it('should return the swap transaction updated with information', () => { + const sourceTokenSymbol = 'ETH'; + const destinationTokenSymbol = 'DAI'; + const type = TransactionType.swap; + const destinationTokenDecimals = '18'; + const destinationTokenAddress = '0x0'; + const swapMetaData = { + meta: 'data', + }; + const swapTokenValue = '0x123'; + const estimatedBaseFee = '0x123'; + const approvalTxId = '0x123'; + + swaps.meta = { + sourceTokenSymbol, + destinationTokenSymbol, + type, + destinationTokenDecimals, + destinationTokenAddress, + swapMetaData, + swapTokenValue, + estimatedBaseFee, + approvalTxId, + }; + + const updatedSwapsTransaction = updateSwapsTransaction( transactionMeta, transactionType, swaps, request, ); - expect(request.controllerHubEmitter).toHaveBeenCalledTimes(1); - expect(request.controllerHubEmitter).toHaveBeenCalledWith( - 'transaction-new-swap', - { - transactionMeta: { - ...transactionMeta, - sourceTokenSymbol, - destinationTokenSymbol, - type, - destinationTokenDecimals, - destinationTokenAddress, - swapMetaData, - swapTokenValue, - estimatedBaseFee, - approvalTxId, - }, - }, + expect(updatedSwapsTransaction).toStrictEqual({ + ...transactionMeta, + sourceTokenSymbol, + destinationTokenSymbol, + type, + destinationTokenDecimals, + destinationTokenAddress, + swapMetaData, + swapTokenValue, + estimatedBaseFee, + approvalTxId, + }); + }); + + it('should update swap approval transaction and publish TransactionController:transactionNewSwapApproval', async () => { + const sourceTokenSymbol = 'ETH'; + const type = TransactionType.swapApproval; + + swaps.meta = { + sourceTokenSymbol, + type, + }; + transactionType = TransactionType.swapApproval; + + const transactionNewSwapApprovalEventListener = jest.fn(); + messenger.subscribe( + 'TransactionController:transactionNewSwapApproval', + transactionNewSwapApprovalEventListener, ); + + updateSwapsTransaction(transactionMeta, transactionType, swaps, request); + expect(transactionNewSwapApprovalEventListener).toHaveBeenCalledTimes(1); + expect(transactionNewSwapApprovalEventListener).toHaveBeenCalledWith({ + transactionMeta: { + ...transactionMeta, + sourceTokenSymbol, + type, + }, + }); }); - it('should update swap approval transaction and emit newSwapApproval event', async () => { + it('should return the swap approval transaction updated with information', async () => { const sourceTokenSymbol = 'ETH'; const type = TransactionType.swapApproval; @@ -152,23 +230,17 @@ describe('updateSwapsTransaction', () => { }; transactionType = TransactionType.swapApproval; - await updateSwapsTransaction( + const updatedSwapsTransaction = updateSwapsTransaction( transactionMeta, transactionType, swaps, request, ); - expect(request.controllerHubEmitter).toHaveBeenCalledTimes(1); - expect(request.controllerHubEmitter).toHaveBeenCalledWith( - 'transaction-new-swap-approval', - { - transactionMeta: { - ...transactionMeta, - sourceTokenSymbol, - type, - }, - }, - ); + expect(updatedSwapsTransaction).toStrictEqual({ + ...transactionMeta, + sourceTokenSymbol, + type, + }); }); }); diff --git a/packages/transaction-controller/src/utils/swaps.ts b/packages/transaction-controller/src/utils/swaps.ts index 2bf985f208..9256ece7a8 100644 --- a/packages/transaction-controller/src/utils/swaps.ts +++ b/packages/transaction-controller/src/utils/swaps.ts @@ -4,7 +4,8 @@ import { merge, pickBy } from 'lodash'; import { CHAIN_IDS } from '../constants'; import { createModuleLogger, projectLogger } from '../logger'; -import type { Events, TransactionMeta } from '../types'; +import type { TransactionControllerMessenger } from '../TransactionController'; +import type { TransactionMeta } from '../types'; import { TransactionType } from '../types'; import { validateIfTransactionUnapproved } from './utils'; @@ -121,9 +122,11 @@ export const SWAP_TRANSACTION_TYPES = [ * @param updateSwapsTransactionRequest - Dependency bag * @param updateSwapsTransactionRequest.isSwapsDisabled - Whether swaps are disabled * @param updateSwapsTransactionRequest.cancelTransaction - Function to cancel a transaction - * @param updateSwapsTransactionRequest.controllerHubEmitter - Function to emit an event to the controller hub + * @param updateSwapsTransactionRequest.messenger - TransactionController messenger + * @returns A copy of the transaction meta object with updates, or the same + * transaction meta object if no updates were made. */ -export async function updateSwapsTransaction( +export function updateSwapsTransaction( transactionMeta: TransactionMeta, transactionType: TransactionType, swaps: { @@ -133,19 +136,17 @@ export async function updateSwapsTransaction( { isSwapsDisabled, cancelTransaction, - controllerHubEmitter, + messenger, }: { isSwapsDisabled: boolean; cancelTransaction: (transactionId: string) => void; - controllerHubEmitter: ( - eventName: T, - ...args: Events[T] - ) => boolean; + messenger: TransactionControllerMessenger; }, -) { +): TransactionMeta { if (isSwapsDisabled || !SWAP_TRANSACTION_TYPES.includes(transactionType)) { - return; + return transactionMeta; } + // The simulationFails property is added if the estimateGas call fails. In cases // when no swaps approval tx is required, this indicates that the swap will likely // fail. There was an earlier estimateGas call made by the swaps controller, @@ -159,29 +160,36 @@ export async function updateSwapsTransaction( swaps?.hasApproveTx === false && transactionMeta.simulationFails ) { - await cancelTransaction(transactionMeta.id); + cancelTransaction(transactionMeta.id); throw new Error('Simulation failed'); } const swapsMeta = swaps?.meta as Partial; if (!swapsMeta) { - return; + return transactionMeta; } + let updatedTransactionMeta = transactionMeta; + if (transactionType === TransactionType.swapApproval) { - updateSwapApprovalTransaction(transactionMeta, swapsMeta); - controllerHubEmitter('transaction-new-swap-approval', { + updatedTransactionMeta = updateSwapApprovalTransaction( transactionMeta, + swapsMeta, + ); + messenger.publish('TransactionController:transactionNewSwapApproval', { + transactionMeta: updatedTransactionMeta, }); } if (transactionType === TransactionType.swap) { - updateSwapTransaction(transactionMeta, swapsMeta); - controllerHubEmitter('transaction-new-swap', { - transactionMeta, + updatedTransactionMeta = updateSwapTransaction(transactionMeta, swapsMeta); + messenger.publish('TransactionController:transactionNewSwap', { + transactionMeta: updatedTransactionMeta, }); } + + return updatedTransactionMeta; } /** @@ -211,7 +219,8 @@ export async function updatePostTransactionBalance( log('Updating post transaction balance', transactionMeta.id); const transactionId = transactionMeta.id; - let latestTransactionMeta, approvalTransactionMeta; + let latestTransactionMeta: TransactionMeta | undefined; + let approvalTransactionMeta; for (let i = 0; i < UPDATE_POST_TX_BALANCE_ATTEMPTS; i++) { log('Querying balance', { attempt: i }); @@ -220,7 +229,9 @@ export async function updatePostTransactionBalance( transactionMeta.txParams.from, ]); - latestTransactionMeta = getTransaction(transactionId) as TransactionMeta; + latestTransactionMeta = { + ...(getTransaction(transactionId) ?? ({} as TransactionMeta)), + }; approvalTransactionMeta = latestTransactionMeta.approvalTxId ? getTransaction(latestTransactionMeta.approvalTxId) @@ -280,6 +291,7 @@ export async function updatePostTransactionBalance( * @param propsToUpdate.swapTokenValue - Value of the token to be swapped * @param propsToUpdate.estimatedBaseFee - Estimated base fee of the transaction * @param propsToUpdate.approvalTxId - Transaction id of the approval transaction + * @returns The updated transaction meta object. */ function updateSwapTransaction( transactionMeta: TransactionMeta, @@ -294,7 +306,7 @@ function updateSwapTransaction( estimatedBaseFee, approvalTxId, }: Partial, -) { +): TransactionMeta { validateIfTransactionUnapproved(transactionMeta, 'updateSwapTransaction'); let swapTransaction = { @@ -311,7 +323,8 @@ function updateSwapTransaction( // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any swapTransaction = pickBy(swapTransaction) as any; - merge(transactionMeta, swapTransaction); + + return merge({}, transactionMeta, swapTransaction); } /** @@ -321,11 +334,12 @@ function updateSwapTransaction( * @param propsToUpdate - Properties to update * @param propsToUpdate.type - Type of the transaction * @param propsToUpdate.sourceTokenSymbol - Symbol of the token to be swapped + * @returns The updated transaction meta object. */ function updateSwapApprovalTransaction( transactionMeta: TransactionMeta, { type, sourceTokenSymbol }: Partial, -) { +): TransactionMeta { validateIfTransactionUnapproved( transactionMeta, 'updateSwapApprovalTransaction', @@ -338,7 +352,8 @@ function updateSwapApprovalTransaction( type, sourceTokenSymbol, }) as Partial; - merge(transactionMeta, swapApprovalTransaction); + + return merge({}, transactionMeta, swapApprovalTransaction); } /** diff --git a/packages/transaction-controller/src/utils/utils.ts b/packages/transaction-controller/src/utils/utils.ts index 7b14900295..5352c7e33c 100644 --- a/packages/transaction-controller/src/utils/utils.ts +++ b/packages/transaction-controller/src/utils/utils.ts @@ -4,6 +4,7 @@ import { getKnownPropertyNames, isStrictHexString, } from '@metamask/utils'; +import type { Json } from '@metamask/utils'; import type { GasPriceValue, @@ -161,8 +162,8 @@ export function normalizeTxError( name: error.name, message: error.message, stack: error.stack, - code: error?.code, - rpc: error?.value, + code: error.code, + rpc: isJsonCompatible(error.value) ? error.value : undefined, }; } @@ -192,6 +193,21 @@ export function normalizeGasFeeValues( }; } +/** + * Determines whether the given value can be encoded as JSON. + * + * @param value - The value. + * @returns True if the value is JSON-encodable, false if not. + */ +function isJsonCompatible(value: unknown): value is Json { + try { + JSON.parse(JSON.stringify(value)); + return true; + } catch { + return false; + } +} + /** * Ensure a hex string is of even length by adding a leading 0 if necessary. * Any existing `0x` prefix is preserved but is not added if missing. diff --git a/packages/transaction-controller/tsconfig.json b/packages/transaction-controller/tsconfig.json index 01c431b29e..52940e5592 100644 --- a/packages/transaction-controller/tsconfig.json +++ b/packages/transaction-controller/tsconfig.json @@ -1,7 +1,8 @@ { "extends": "../../tsconfig.packages.json", "compilerOptions": { - "baseUrl": "./" + "baseUrl": "./", + "target": "ES2022" }, "references": [ { "path": "../approval-controller" }, diff --git a/yarn.lock b/yarn.lock index 887de24e8c..3edcebf497 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2923,6 +2923,7 @@ __metadata: deepmerge: ^4.2.2 eth-method-registry: ^4.0.0 fast-json-patch: ^3.1.1 + immer: ^9.0.6 jest: ^27.5.1 lodash: ^4.17.21 nock: ^13.3.1