From aecf37774429f3a57c657a92af780d29c7ecee1c Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Mon, 8 Jan 2024 17:07:52 -0700 Subject: [PATCH 01/19] Migrate TransactionController to BaseController v2 Fundamentally, this commit changes TransactionController so that it inherits from BaseController v2 instead of BaseController v1. This affects the signature of the constructor and the way that state is updated inside of the controller, but it also prompts other changes: - Previously, the controller used a `hub` property, which was an instance of EventEmitter, to emit events at key points in the transaction processing flow. Now, a controller messenger is used instead, and all of the events now use a standard name. - Due to the superclass change, the controller state type is now `Record`. As a result, all state property types needed to be `Json`-compatible. Some types were defined as interfaces and could merely be changed to type aliases, but concessions needed to be made in other cases to achieve compatibility. - Due to the superclass change, once `update` is called, state is recursively frozen at runtime, which means that transactions cannot be modified directly. Hence, numerous places within TransactionController needed to be updated so that transactions were copied before being modified. Some methods were even changed so that instead of directly modifying a transaction, they now return a new version. - Tests did not reset Jest mocks correctly, and modules were mocked in a way where changes in one test bled over into others. This has been fixed. - Some tests made a new instance of TransactionController and then directly updated the TransactionController state. Since it is possible to instantiate TransactionController by passing initial state, these tests have been updated to do exactly this. --- .../transaction-controller/jest.config.js | 6 +- packages/transaction-controller/package.json | 1 + .../src/TransactionController.test.ts | 2262 +++++++++-------- .../src/TransactionController.ts | 1002 +++++--- .../TransactionControllerIntegration.test.ts | 292 ++- .../EtherscanRemoteTransactionSource.test.ts | 90 +- .../src/helpers/GasFeePoller.ts | 13 +- .../src/helpers/IncomingTransactionHelper.ts | 6 +- .../helpers/PendingTransactionTracker.test.ts | 398 ++- .../src/helpers/PendingTransactionTracker.ts | 55 +- packages/transaction-controller/src/index.ts | 59 +- packages/transaction-controller/src/types.ts | 97 +- .../src/utils/history.ts | 32 +- .../src/utils/swaps.test.ts | 157 +- .../transaction-controller/src/utils/swaps.ts | 79 +- .../transaction-controller/src/utils/utils.ts | 20 +- packages/transaction-controller/tsconfig.json | 3 +- yarn.lock | 1 + 18 files changed, 2781 insertions(+), 1792 deletions(-) diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index 10fa492868..7b44033691 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -18,9 +18,9 @@ module.exports = merge(baseConfig, { coverageThreshold: { global: { branches: 91.79, - functions: 98.56, - lines: 98.9, - statements: 98.91, + functions: 98.37, + lines: 98.83, + statements: 98.84, }, }, diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index 554490c014..2505f83f7a 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 eb2f5af802..7e43408ab2 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; @@ -566,146 +468,6 @@ describe('TransactionController', () => { typeof MultichainTrackingHelper >; - /** - * Create a new instance of the TransactionController. - * - * @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. - * @returns The new TransactionController instance. - */ - function newController({ - options, - config, - network, - approve, - reject, - state, - }: { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - options?: any; - config?: Partial; - 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, - }; - } - - 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 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"); - } - }); - - ({ 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, - }, - { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sign: async (transaction: any) => transaction, - ...config, - }, - state ?? undefined, - ); - } - /** * Wait for a specified number of milliseconds. * @@ -725,6 +487,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 +560,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 +580,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 +608,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 +684,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 +711,7 @@ describe('TransactionController', () => { errorKey: 'testKey', }; - const controller = newController(); + const { controller } = setupController(); estimateGasMock.mockResolvedValue({ estimatedGas: gasMock, @@ -982,7 +750,7 @@ describe('TransactionController', () => { }, }; - const controller = newController(); + const { controller } = setupController(); estimateGasMock.mockResolvedValue({ estimatedGas: gasMock, @@ -1017,7 +785,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 +815,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 +833,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 +877,7 @@ describe('TransactionController', () => { expect(firstTransactionCompleted).toBe(false); expect(secondTransactionCompleted).toBe(false); - approveTransaction(); + mockTransactionApprovalRequest.approve(); await firstResult; await secondResult; @@ -1130,7 +901,8 @@ describe('TransactionController', () => { ])( '%s', async (_, firstActionId, secondActionId, expectedTransactionCount) => { - const controller = newController(); + const { controller, mockTransactionApprovalRequest } = + setupController(); const expectedRequestApprovalCalledTimes = expectedTransactionCount; const mockOrigin = 'origin'; @@ -1159,9 +931,9 @@ describe('TransactionController', () => { const { transactions } = controller.state; expect(transactions).toHaveLength(expectedTransactionCount); - expect(messengerMock.call).toHaveBeenCalledTimes( - expectedRequestApprovalCalledTimes, - ); + expect( + mockTransactionApprovalRequest.actionHandlerMock, + ).toHaveBeenCalledTimes(expectedRequestApprovalCalledTimes); }, ); @@ -1186,7 +958,7 @@ describe('TransactionController', () => { expectedTransactionCount, expectedSignCalledTimes, ) => { - const controller = newController(); + const { controller } = setupController(); const signSpy = jest.spyOn(controller, 'sign'); const { transactionMeta } = await controller.addTransaction({ @@ -1213,7 +985,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'; @@ -1267,9 +1039,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 +1084,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 +1142,7 @@ describe('TransactionController', () => { }); it('generates initial history', async () => { - const controller = newController(); + const { controller } = setupController(); await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1356,14 +1168,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 +1195,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 +1218,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 +1257,7 @@ describe('TransactionController', () => { networkStateChangeListener = listener; }; - const controller = newController({ + const { controller } = setupController({ options: { getNetworkState, onNetworkStateChange }, }); @@ -1473,18 +1285,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 +1345,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 +1368,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 +1381,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 +1394,7 @@ describe('TransactionController', () => { .fn() .mockResolvedValue(mockSecurityProviderResponse); - const controller = newController({ + const { controller } = setupController({ options: { securityProviderRequest: securityProviderRequestMock, }, @@ -1603,7 +1423,7 @@ describe('TransactionController', () => { }); it('updates gas properties', async () => { - const controller = newController(); + const { controller } = setupController(); await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -1621,7 +1441,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 +1466,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 +1489,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 +1538,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 +1571,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 +1585,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 +1625,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 +1688,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 +1706,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 +1728,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.', @@ -1868,11 +1753,19 @@ describe('TransactionController', () => { }); it('emits rejected and finished event', async () => { - const controller = newController({ reject: true }); + 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 +1779,7 @@ describe('TransactionController', () => { }, ); - controller.hub.on( - `${transactionMeta.id}:finished`, - finishedEventListener, - ); - - const finishedPromise = waitForTransactionFinished(controller); + const finishedPromise = waitForTransactionFinished(messenger); try { await result; @@ -1902,18 +1790,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 +1822,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 +1838,7 @@ describe('TransactionController', () => { describe('wipeTransactions', () => { it('removes all transactions on current network', async () => { - const controller = newController(); + const { controller } = setupController(); controller.wipeTransactions(); @@ -1968,31 +1853,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 +1890,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 +1929,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 +1969,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 +2014,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 +2042,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 +2115,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 +2154,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 +2163,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 +2205,7 @@ describe('TransactionController', () => { }); it('rejects unknown transaction', async () => { - const controller = newController({ + const { controller } = setupController({ network: MOCK_LINEA_GOERLI_NETWORK, }); @@ -2313,7 +2219,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 }); @@ -2323,9 +2229,8 @@ describe('TransactionController', () => { }); it('emits transaction events', async () => { - const controller = newController({ - network: MOCK_LINEA_GOERLI_NETWORK, - }); + const { controller, messenger, mockTransactionApprovalRequest } = + setupController({ network: MOCK_LINEA_GOERLI_NETWORK }); const approvedEventListener = jest.fn(); const submittedEventListener = jest.fn(); @@ -2333,8 +2238,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 +2255,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 +2297,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 +2323,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 +2345,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(mockEthQuery.sendRawTransaction).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, + 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 +2439,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 +2465,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 +2507,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 +2532,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,11 +2565,15 @@ describe('TransactionController', () => { }); it('allows transaction count to exceed txHistorylimit', async () => { - const controller = newController({ - approve: true, - config: { + const { controller } = setupController({ + options: { txHistoryLimit: 1, }, + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, }); const { transactionMeta, result } = await controller.addTransaction({ @@ -2669,18 +2592,24 @@ describe('TransactionController', () => { }); it('emits transaction events', async () => { - const controller = newController({ + 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 +2620,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 +2644,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 +2687,7 @@ describe('TransactionController', () => { }); it('generates initial history', async () => { - const controller = newController(); + const { controller } = setupController(); const externalTransactionToConfirm = { from: ACCOUNT_MOCK, @@ -2831,19 +2760,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 +2773,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 +2782,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 +2827,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 +2864,7 @@ describe('TransactionController', () => { txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, - nonce: String(NONCE_MOCK), + nonce: toHex(NONCE_MOCK), }, }; const externalTransactionReceipt = { @@ -2935,18 +2872,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 +2911,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 +2931,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 +2970,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, @@ -3040,11 +2984,14 @@ describe('TransactionController', () => { }); it('emits confirmed event', async () => { - const controller = newController(); + 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,12 +3018,10 @@ 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 () => { @@ -3091,13 +3036,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 +3071,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 +3104,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 +3141,7 @@ describe('TransactionController', () => { }); it('doesnt append if current sendFlowHistory lengths doesnt match', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3218,7 +3166,7 @@ describe('TransactionController', () => { }); it('throws if sendFlowHistory persistence is disabled', async () => { - const controller = newController({ + const { controller } = setupController({ options: { disableSendFlowHistory: true }, }); const mockSendFlowHistory = [ @@ -3245,7 +3193,7 @@ describe('TransactionController', () => { }); it('throws if transactionMeta is not found', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3265,7 +3213,7 @@ describe('TransactionController', () => { }); it('throws if the transaction is not unapproved status', async () => { - const controller = newController(); + const { controller } = setupController(); const mockSendFlowHistory = [ { entry: @@ -3297,7 +3245,7 @@ describe('TransactionController', () => { describe('clearUnapprovedTransactions', () => { it('clears unapproved transactions', async () => { - const controller = newController(); + const { controller } = setupController(); const firstUnapprovedTxId = '1'; const secondUnapprovedTxId = '2'; @@ -3358,7 +3306,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 +3322,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 +3349,9 @@ describe('TransactionController', () => { }); it('limits max transactions when adding to state', async () => { - const controller = newController({ config: { txHistoryLimit: 1 } }); + const { controller } = setupController({ + options: { txHistoryLimit: 1 }, + }); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -3417,7 +3368,7 @@ describe('TransactionController', () => { describe('on incoming transaction helper lastFetchedBlockNumbers event', () => { it('updates state', async () => { - const controller = newController(); + const { controller } = setupController(); const lastFetchedBlockNumbers = { key: 234, @@ -3439,8 +3390,11 @@ describe('TransactionController', () => { const blockNumber = 123; const listener = jest.fn(); - const controller = newController(); - controller.hub.on('incomingTransactionBlock', listener); + const { messenger } = setupController(); + messenger.subscribe( + 'TransactionController:incomingTransactionBlock', + listener, + ); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -3458,7 +3412,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 +3424,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 +3444,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 +3503,7 @@ describe('TransactionController', () => { const maxFeePerGas = '0xmaxFeePerGas'; const transactionId = '123'; - const controller = newController(); + const { controller } = setupController(); controller.state.transactions.push({ id: transactionId, @@ -3586,7 +3540,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 +3552,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 +3570,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 +3626,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 +3652,6 @@ describe('TransactionController', () => { }); it('marks duplicate nonce transactions as dropped', async () => { - const controller = newController(); - const confirmed = { ...TRANSACTION_META_MOCK, id: 'testId1', @@ -3747,16 +3705,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 +3766,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 +3786,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 +3809,6 @@ describe('TransactionController', () => { rpc: undefined, }; - controller.state.transactions = [{ ...TRANSACTION_META_MOCK }]; - firePendingTransactionTrackerEvent( 'transaction-failed', TRANSACTION_META_MOCK, @@ -3853,19 +3823,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 +3856,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 +3876,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 +3917,8 @@ describe('TransactionController', () => { .mockImplementation(async (transactionParams) => Promise.resolve(TransactionFactory.fromTxData(transactionParams)), ); - const controller = newController({ - config: { + const { controller } = setupController({ + options: { sign: signMock, }, }); @@ -3983,8 +3956,8 @@ describe('TransactionController', () => { .mockImplementation(async () => Promise.reject(new Error(mockSignError)), ); - const controller = newController({ - config: { + const { controller } = setupController({ + options: { sign: signMock, }, }); @@ -4014,7 +3987,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 +4016,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 +4050,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 +4074,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 +4082,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 +4109,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 +4117,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 +4129,9 @@ describe('TransactionController', () => { actionId: ACTION_ID_MOCK, }); - approveTransaction(); + mockTransactionApprovalRequest.approve({ + value: TRANSACTION_META_MOCK, + }); await wait(0); expect(signSpy).toHaveBeenCalledTimes(1); @@ -4157,7 +4139,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 +4148,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 +4156,7 @@ describe('TransactionController', () => { actionId: ACTION_ID_MOCK, }); - approveTransaction(); + mockTransactionApprovalRequest.approve(); await wait(0); expect(signSpy).toHaveBeenCalledTimes(1); @@ -4197,7 +4177,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 +4185,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 +4227,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 +4244,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 +4264,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 +4286,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 +4310,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 +4335,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 +4374,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 +4402,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 +4424,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 +4451,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 +4474,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 +4486,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 +4499,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 +4553,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 +4577,7 @@ describe('TransactionController', () => { }, false, ); - expect(messengerMock.call).toHaveBeenCalledWith( + expect(messenger.call).toHaveBeenCalledWith( 'ApprovalController:addRequest', { expectsResult: true, @@ -4616,11 +4630,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 +4675,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 +4709,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 +4747,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4743,8 +4762,6 @@ describe('TransactionController', () => { }); it('returns transactions matching param values in search criteria', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4769,7 +4786,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4780,8 +4801,6 @@ describe('TransactionController', () => { }); it('returns transactions matching multiple values in search criteria', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4806,7 +4825,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4817,8 +4840,6 @@ describe('TransactionController', () => { }); it('returns transactions matching function in search criteria', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4843,7 +4864,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4856,8 +4881,6 @@ describe('TransactionController', () => { }); it('returns transactions matching current network', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: MOCK_NETWORK.state.providerConfig.chainId, @@ -4882,7 +4905,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4892,7 +4919,7 @@ describe('TransactionController', () => { }); it('returns transactions from specified list', () => { - const controller = newController(); + const { controller } = setupController(); const transactions: TransactionMeta[] = [ { @@ -4928,8 +4955,6 @@ describe('TransactionController', () => { }); it('returns limited number of transactions sorted by ascending time', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -4961,7 +4986,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -4973,8 +5002,6 @@ describe('TransactionController', () => { }); it('returns limited number of transactions except for duplicate nonces', () => { - const controller = newController(); - const transactions: TransactionMeta[] = [ { chainId: '0x1', @@ -5007,7 +5034,11 @@ describe('TransactionController', () => { }, ]; - controller.state.transactions = transactions; + const { controller } = setupController({ + options: { + state: { transactions }, + }, + }); expect( controller.getTransactions({ @@ -5051,7 +5082,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 +5094,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 +5103,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 +5117,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 +5125,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 +5141,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({ @@ -5141,3 +5180,190 @@ describe('TransactionController', () => { }); }); }); + +/** + * Constructs an instance of the TransactionController and supporting data for + * use in tests. + * + * @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 setupController({ + options: givenOptions = {}, + network = MOCK_NETWORK, + messengerOptions = {}, +}: { + options?: Partial[0]>; + network?: MockNetwork; + messengerOptions?: { + addTransactionApprovalRequest?: Parameters< + typeof mockAddTransactionApprovalRequest + >[1]; + }; +} = {}) { + const unrestrictedMessenger: UnrestrictedControllerMessenger = + new ControllerMessenger(); + + const { addTransactionApprovalRequest = { state: 'pending' } } = + messengerOptions; + const mockTransactionApprovalRequest = mockAddTransactionApprovalRequest( + unrestrictedMessenger, + addTransactionApprovalRequest, + ); + + 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, + txHistoryLimit: 40, + ...givenOptions, + }; + + const restrictedMessenger = + givenRestrictedMessenger ?? + unrestrictedMessenger.getRestricted({ + name: 'TransactionController', + allowedActions: [ + 'ApprovalController:addRequest', + 'NetworkController:getNetworkClientById', + 'NetworkController:findNetworkClientIdByChainId', + ], + }); + + 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, + }); + }; + + const rejectTransaction = ( + rejectionError: unknown = { + code: errorCodes.provider.userRejectedRequest, + }, + ) => { + 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; +} diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 2b1efd48df..75723ef1a9 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, @@ -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 * @@ -233,6 +257,9 @@ export type PendingTransactionOptions = { * @property hooks.beforePublish - Additional logic to execute before publishing a transaction. Return false to prevent the broadcast of the transaction. * @property hooks.getAdditionalSignArguments - Returns additional arguments required to sign a transaction. * @property hooks.publish - Alternate logic to publish a transaction. + * @property sign - Function used to sign transactions. + * @property state - Initial state to set on this controller. + * @property txHistoryLimit - Transaction history limit. */ export type TransactionControllerOptions = { blockTracker: BlockTracker; @@ -275,6 +302,13 @@ export type TransactionControllerOptions = { transactionMeta: TransactionMeta, ) => Promise<{ transactionHash: string }>; }; + sign?: ( + transaction: TypedTransaction, + from: string, + transactionMeta?: TransactionMeta, + ) => Promise; + state?: Partial; + txHistoryLimit: number; }; /** @@ -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 TransactionControllerIncomingTransactionBlockEvent = { + type: `${typeof controllerName}:incomingTransactionBlock`; + 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 = + | TransactionControllerIncomingTransactionBlockEvent + | 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() { + 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(); + #txHistoryLimit: number; + private readonly afterSign: ( transactionMeta: TransactionMeta, signedTx: TypedTransaction, @@ -409,7 +636,7 @@ export class TransactionController extends BaseControllerV1< error: normalizeTxError(error), status: TransactionStatus.failed, }; - this.hub.emit('transaction-failed', { + this.messagingSystem.publish(`${controllerName}:transactionFailed`, { actionId, error: error.message, transactionMeta: newTransactionMeta, @@ -418,28 +645,33 @@ export class TransactionController extends BaseControllerV1< newTransactionMeta, 'TransactionController#failTransaction - Add error message and set status to failed', ); - this.onTransactionStatusChange(newTransactionMeta); - this.hub.emit(`${transactionMeta.id}:finished`, newTransactionMeta); + this.messagingSystem.publish(`${controllerName}:transactionStatusUpdated`, { + transactionMeta: 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 +681,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.hooks - The controller hooks. + * @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.txHistoryLimit - Transaction history limit. + */ + constructor({ + blockTracker, + disableHistory, + disableSendFlowHistory, + disableSwaps, + getCurrentAccountEIP1559Compatibility, + getCurrentNetworkEIP1559Compatibility, + getExternalPendingTransactions, + getGasFeeEstimates, + getNetworkState, + getPermittedAccounts, + getSavedGasFees, + getSelectedAddress, + incomingTransactions = {}, + messenger, + onNetworkStateChange, + pendingTransactions = {}, + provider, + securityProviderRequest, + getNetworkClientRegistry, + isMultichainEnabled = false, + hooks, + sign, + state, + txHistoryLimit = 40, + }: 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 +767,8 @@ export class TransactionController extends BaseControllerV1< this.securityProviderRequest = securityProviderRequest; this.#incomingTransactionOptions = incomingTransactions; this.#pendingTransactionOptions = pendingTransactions; + this.#txHistoryLimit = txHistoryLimit; + this.sign = sign; this.afterSign = hooks?.afterSign ?? (() => true); this.beforeApproveOnInit = hooks?.beforeApproveOnInit ?? (() => true); @@ -591,7 +851,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 +864,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 +904,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 +1007,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: TransactionMeta = existingTransactionMeta + ? { ...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, + }; - 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, }; } @@ -992,17 +1267,21 @@ export class TransactionController extends BaseControllerV1< 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 +1397,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 }); @@ -1140,7 +1422,7 @@ export class TransactionController extends BaseControllerV1< ); const baseTransactionMeta: TransactionMeta = { - ...transactionMeta, + ...transactionMetaWithRsv, estimatedBaseFee, id: random(), time: Date.now(), @@ -1172,17 +1454,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 +1554,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 +1575,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 +1596,8 @@ export class TransactionController extends BaseControllerV1< }, ); - this.update({ - transactions: this.trimTransactionsForState(newTransactions), + this.update((state) => { + state.transactions = this.trimTransactionsForState(newTransactions); }); } @@ -1324,16 +1614,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: TransactionMeta = { + ...newTransactionMeta, + status: TransactionStatus.confirmed, + txReceipt: transactionReceipt, + }; if (baseFeePerGas) { - transactionMeta.baseFeePerGas = baseFeePerGas; + updatedTransactionMeta.baseFeePerGas = baseFeePerGas; } // Update same nonce local transactions as dropped and define replacedBy properties. @@ -1341,18 +1634,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 +1684,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`, ); } @@ -1493,7 +1785,7 @@ export class TransactionController extends BaseControllerV1< this.updateTransaction( updatedMeta, - 'TransactionController:updateTransactionGasFees - gas values updated', + `${controllerName}:updateTransactionGasFees - gas values updated`, ); return this.getTransaction(transactionId) as TransactionMeta; @@ -1551,7 +1843,7 @@ export class TransactionController extends BaseControllerV1< this.updateTransaction( updatedMeta, - 'TransactionController:updatePreviousGasParams - Previous gas values updated', + `${controllerName}:updatePreviousGasParams - Previous gas values updated`, ); return this.getTransaction(transactionId) as TransactionMeta; @@ -1771,6 +2063,7 @@ export class TransactionController extends BaseControllerV1< } const updatedTransactionMeta = merge( + {}, transactionMeta, pickBy({ hash, status }), ); @@ -1785,7 +2078,7 @@ export class TransactionController extends BaseControllerV1< this.updateTransaction( updatedTransactionMeta, - `TransactionController:updateCustodialTransaction - Custodial transaction updated`, + `${controllerName}:updateCustodialTransaction - Custodial transaction updated`, ); if ( @@ -1793,7 +2086,10 @@ export class TransactionController extends BaseControllerV1< status as TransactionStatus, ) ) { - this.hub.emit(`${transactionMeta.id}:finished`, updatedTransactionMeta); + this.messagingSystem.publish( + `${controllerName}:transactionFinished`, + updatedTransactionMeta, + ); } } @@ -1962,7 +2258,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); + }); } /** @@ -1991,9 +2289,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) { @@ -2082,18 +2383,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; @@ -2119,14 +2410,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 @@ -2183,6 +2483,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: TransactionMeta = { ...transactionMeta }; + const { txParams: { from }, networkClientId, @@ -2197,16 +2499,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( @@ -2217,42 +2519,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({ @@ -2265,9 +2576,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); @@ -2283,26 +2597,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 @@ -2332,17 +2655,31 @@ 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 = { + ...transactionMeta, + status: TransactionStatus.rejected, + }; + 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); } /** @@ -2352,9 +2689,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. @@ -2364,20 +2701,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( - chainId, + const key = `${String(txParams.nonce)}-${String( + convertHexToDecimal(chainId), )}-${new Date(time).toDateString()}`; if (nonceNetworkSet.has(key)) { return true; } else if ( - nonceNetworkSet.size < this.config.txHistoryLimit || + nonceNetworkSet.size < this.#txHistoryLimit || !this.isFinalState(status) ) { nonceNetworkSet.add(key); @@ -2389,6 +2726,7 @@ export class TransactionController extends BaseControllerV1< }); txsToKeep.reverse(); // Ascending time order + return txsToKeep; } @@ -2514,21 +2852,19 @@ 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({ - transactions: this.trimTransactionsForState(updatedTransactions), + this.update((state) => { + const { transactions: currentTransactions } = state; + const updatedTransactions = [ + ...added, + ...currentTransactions.map((originalTransaction) => { + const updatedTransaction = updated.find( + ({ hash }) => hash === originalTransaction.hash, + ); + + return updatedTransaction ?? originalTransaction; + }), + ]; + state.transactions = this.trimTransactionsForState(updatedTransactions); }); } @@ -2541,8 +2877,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}:incomingTransactionBlock`, + blockNumber, + ); } private generateDappSuggestedGasFees( @@ -2587,6 +2928,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; @@ -2611,16 +2953,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; } /** @@ -2638,7 +2983,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 && @@ -2646,17 +2991,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); } } @@ -2668,15 +3024,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 = { + ...transactionMeta, + status: TransactionStatus.dropped, + }; + this.messagingSystem.publish(`${controllerName}:transactionDropped`, { + transactionMeta: updatedTransactionMeta, }); this.updateTransaction( - transactionMeta, + updatedTransactionMeta, 'TransactionController#setTransactionStatusDropped - Transaction dropped', ); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(updatedTransactionMeta); } /** @@ -2695,7 +3054,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); }); }); @@ -2711,7 +3070,9 @@ export class TransactionController extends BaseControllerV1< private async updateTransactionMetaRSV( transactionMeta: TransactionMeta, signedTx: TypedTransaction, - ): Promise { + ): Promise { + const transactionMetaWithRsv = { ...transactionMeta }; + for (const key of ['r', 's', 'v'] as const) { const value = signedTx[key]; @@ -2719,8 +3080,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) { @@ -2778,23 +3141,24 @@ export class TransactionController extends BaseControllerV1< return undefined; } - await this.updateTransactionMetaRSV(transactionMeta, signedTx); - - transactionMeta.status = TransactionStatus.signed; + const transactionMetaWithRsv: TransactionMeta = { + ...(await this.updateTransactionMetaRSV(transactionMeta, signedTx)), + status: TransactionStatus.signed, + }; this.updateTransaction( - transactionMeta, + transactionMetaWithRsv, 'TransactionController#approveTransaction - Transaction signed', ); - this.onTransactionStatusChange(transactionMeta); + this.onTransactionStatusChange(transactionMetaWithRsv); const rawTx = bufferToHex(signedTx.serialize()); - transactionMeta.rawTx = rawTx; + const transactionMetaWithRawTx = { ...transactionMetaWithRsv, rawTx }; this.updateTransaction( - transactionMeta, + transactionMetaWithRawTx, 'TransactionController#approveTransaction - RawTransaction added', ); @@ -2802,7 +3166,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( @@ -2823,8 +3189,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); @@ -2850,10 +3218,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); @@ -2902,7 +3273,7 @@ export class TransactionController extends BaseControllerV1< isEnabled: this.#incomingTransactionOptions.isEnabled, queryEntireHistory: this.#incomingTransactionOptions.queryEntireHistory, remoteTransactionSource: etherscanRemoteTransactionSource, - transactionLimit: this.config.txHistoryLimit, + transactionLimit: this.#txHistoryLimit, updateTransactions: this.#incomingTransactionOptions.updateTransactions, }); @@ -3081,19 +3452,26 @@ export class TransactionController extends BaseControllerV1< transactionMeta: TransactionMeta, { note, skipHistory }: { note?: string; skipHistory?: boolean }, ) { - const { transactions } = this.state; - - transactionMeta.txParams = normalizeTxParams(transactionMeta.txParams); - - validateTxParams(transactionMeta.txParams); + const normalizedTransaction: TransactionMeta = { + ...transactionMeta, + txParams: normalizeTxParams(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..3048ee53f5 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, + txHistoryLimit: 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..6afdf77601 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 { projectLogger } from '../logger'; +import { createModuleLogger, projectLogger } from '../logger'; +import type { ApprovalState } from '../TransactionController'; import type { TransactionMeta, TransactionReceipt } from '../types'; import { TransactionStatus, TransactionType } from '../types'; @@ -59,7 +59,7 @@ export interface PendingTransactionTrackerEventEmitter extends EventEmitter { export class PendingTransactionTracker { hub: PendingTransactionTrackerEventEmitter; - #approveTransaction: (transactionId: string) => Promise; + #approveTransaction: (transactionId: string) => Promise; #blockTracker: BlockTracker; @@ -98,7 +98,7 @@ export class PendingTransactionTracker { publishTransaction, hooks, }: { - approveTransaction: (transactionId: string) => Promise; + approveTransaction: (transactionId: string) => Promise; blockTracker: BlockTracker; getChainId: () => string; getEthQuery: (networkClientId?: NetworkClientId) => EthQuery; @@ -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, + { ...txMeta, retryCount }, 'PendingTransactionTracker:transaction-retry - Retry count increased', ); } #isResubmitDue(txMeta: TransactionMeta, latestBlockNumber: string): boolean { - if (!txMeta.firstRetryBlockNumber) { - txMeta.firstRetryBlockNumber = latestBlockNumber; + const txMetaWithFirstRetryBlockNumber = { ...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 = { ...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 ea4f0a914c..2fe3c5bea6 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -1,6 +1,61 @@ -export * from './TransactionController'; +export type { + FeeMarketEIP1559Values, + GasPriceValue, + MethodData, + Result, + TransactionControllerActions, + TransactionControllerEvents, + TransactionControllerGetStateAction, + TransactionControllerIncomingTransactionBlockEvent, + 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 { EtherscanTransactionMeta } from './utils/etherscan'; export { isEIP1559Transaction } from './utils/utils'; -export * from './types'; +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 { 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..a98b2fccc8 100644 --- a/packages/transaction-controller/src/utils/history.ts +++ b/packages/transaction-controller/src/utils/history.ts @@ -8,39 +8,47 @@ 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 { ...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 a + * `history`, or `transactionMeta` unchanged. */ 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 { + ...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..34042be57a 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,22 +85,18 @@ 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 () => { @@ -116,33 +124,76 @@ 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 emit newSwapApproval event', async () => { + it('should return the swap approval transaction updated with information', async () => { const sourceTokenSymbol = 'ETH'; const type = TransactionType.swapApproval; @@ -152,23 +203,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..d50f2fe91c 100644 --- a/packages/transaction-controller/src/utils/swaps.ts +++ b/packages/transaction-controller/src/utils/swaps.ts @@ -1,10 +1,11 @@ import { query } from '@metamask/controller-utils'; import type EthQuery from '@metamask/eth-query'; -import { merge, pickBy } from 'lodash'; +import { 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,34 @@ 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; - } - - if (transactionType === TransactionType.swapApproval) { - updateSwapApprovalTransaction(transactionMeta, swapsMeta); - controllerHubEmitter('transaction-new-swap-approval', { - transactionMeta, - }); + let updatedTransactionMeta = transactionMeta; + if (swapsMeta) { + if (transactionType === TransactionType.swapApproval) { + updatedTransactionMeta = updateSwapApprovalTransaction( + transactionMeta, + swapsMeta, + ); + messenger.publish('TransactionController:transactionNewSwapApproval', { + transactionMeta: updatedTransactionMeta, + }); + } else if (transactionType === TransactionType.swap) { + updatedTransactionMeta = updateSwapTransaction( + transactionMeta, + swapsMeta, + ); + messenger.publish('TransactionController:transactionNewSwap', { + transactionMeta: updatedTransactionMeta, + }); + } } - if (transactionType === TransactionType.swap) { - updateSwapTransaction(transactionMeta, swapsMeta); - controllerHubEmitter('transaction-new-swap', { - transactionMeta, - }); - } + return updatedTransactionMeta; } /** @@ -211,7 +217,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 +227,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 +289,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 +304,7 @@ function updateSwapTransaction( estimatedBaseFee, approvalTxId, }: Partial, -) { +): TransactionMeta { validateIfTransactionUnapproved(transactionMeta, 'updateSwapTransaction'); let swapTransaction = { @@ -311,7 +321,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 { ...transactionMeta, ...swapTransaction }; } /** @@ -321,11 +332,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 +350,8 @@ function updateSwapApprovalTransaction( type, sourceTokenSymbol, }) as Partial; - merge(transactionMeta, swapApprovalTransaction); + + return { ...transactionMeta, ...swapApprovalTransaction }; } /** diff --git a/packages/transaction-controller/src/utils/utils.ts b/packages/transaction-controller/src/utils/utils.ts index b4382414bf..a35236a1e3 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, }; } @@ -191,3 +192,18 @@ export function normalizeGasFeeValues( maxPriorityFeePerGas: normalize(gasFeeValues.maxPriorityFeePerGas), }; } + +/** + * 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; + } +} 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 74711dc673..abee887326 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2922,6 +2922,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 From a20638be0244a7a22411b0b58443b3dc23acd1ac Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 29 Feb 2024 11:00:21 -0700 Subject: [PATCH 02/19] Move setupController to the top of the test file --- .../src/TransactionController.test.ts | 374 +++++++++--------- 1 file changed, 187 insertions(+), 187 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 7e43408ab2..ce0c8fd92b 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -468,6 +468,193 @@ describe('TransactionController', () => { typeof MultichainTrackingHelper >; + /** + * Constructs an instance of the TransactionController and supporting data for + * use in tests. + * + * @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 setupController({ + options: givenOptions = {}, + network = MOCK_NETWORK, + messengerOptions = {}, + }: { + options?: Partial[0]>; + network?: MockNetwork; + messengerOptions?: { + addTransactionApprovalRequest?: Parameters< + typeof mockAddTransactionApprovalRequest + >[1]; + }; + } = {}) { + const unrestrictedMessenger: UnrestrictedControllerMessenger = + new ControllerMessenger(); + + const { addTransactionApprovalRequest = { state: 'pending' } } = + messengerOptions; + const mockTransactionApprovalRequest = mockAddTransactionApprovalRequest( + unrestrictedMessenger, + addTransactionApprovalRequest, + ); + + 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, + txHistoryLimit: 40, + ...givenOptions, + }; + + const restrictedMessenger = + givenRestrictedMessenger ?? + unrestrictedMessenger.getRestricted({ + name: 'TransactionController', + allowedActions: [ + 'ApprovalController:addRequest', + 'NetworkController:getNetworkClientById', + 'NetworkController:findNetworkClientIdByChainId', + ], + }); + + 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, + }); + }; + + const rejectTransaction = ( + rejectionError: unknown = { + code: errorCodes.provider.userRejectedRequest, + }, + ) => { + 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; + } + /** * Wait for a specified number of milliseconds. * @@ -5180,190 +5367,3 @@ describe('TransactionController', () => { }); }); }); - -/** - * Constructs an instance of the TransactionController and supporting data for - * use in tests. - * - * @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 setupController({ - options: givenOptions = {}, - network = MOCK_NETWORK, - messengerOptions = {}, -}: { - options?: Partial[0]>; - network?: MockNetwork; - messengerOptions?: { - addTransactionApprovalRequest?: Parameters< - typeof mockAddTransactionApprovalRequest - >[1]; - }; -} = {}) { - const unrestrictedMessenger: UnrestrictedControllerMessenger = - new ControllerMessenger(); - - const { addTransactionApprovalRequest = { state: 'pending' } } = - messengerOptions; - const mockTransactionApprovalRequest = mockAddTransactionApprovalRequest( - unrestrictedMessenger, - addTransactionApprovalRequest, - ); - - 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, - txHistoryLimit: 40, - ...givenOptions, - }; - - const restrictedMessenger = - givenRestrictedMessenger ?? - unrestrictedMessenger.getRestricted({ - name: 'TransactionController', - allowedActions: [ - 'ApprovalController:addRequest', - 'NetworkController:getNetworkClientById', - 'NetworkController:findNetworkClientIdByChainId', - ], - }); - - 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, - }); - }; - - const rejectTransaction = ( - rejectionError: unknown = { - code: errorCodes.provider.userRejectedRequest, - }, - ) => { - 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; -} From a471219ef08fff826cb97b2e2570711c36a9d07a Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 29 Feb 2024 11:15:43 -0700 Subject: [PATCH 03/19] Convert another instance of transactionStatusUpdated back to onTransactionStatusChange --- packages/transaction-controller/src/TransactionController.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 75723ef1a9..438402bcf4 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -645,9 +645,7 @@ export class TransactionController extends BaseController< newTransactionMeta, 'TransactionController#failTransaction - Add error message and set status to failed', ); - this.messagingSystem.publish(`${controllerName}:transactionStatusUpdated`, { - transactionMeta: newTransactionMeta, - }); + this.onTransactionStatusChange(newTransactionMeta); this.messagingSystem.publish( `${controllerName}:transactionFinished`, newTransactionMeta, From 7dacf8e137d80312078b04ebeda2ded88b2caba5 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 29 Feb 2024 11:49:02 -0700 Subject: [PATCH 04/19] Add back or remove whitespace changes --- packages/transaction-controller/src/TransactionController.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 438402bcf4..4ee7014e25 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2653,14 +2653,12 @@ export class TransactionController extends BaseController< if (!transactionMeta) { return; } - this.update((state) => { const transactions = state.transactions.filter( ({ id }) => id !== transactionId, ); state.transactions = this.trimTransactionsForState(transactions); }); - const updatedTransactionMeta: TransactionMeta = { ...transactionMeta, status: TransactionStatus.rejected, @@ -2724,7 +2722,6 @@ export class TransactionController extends BaseController< }); txsToKeep.reverse(); // Ascending time order - return txsToKeep; } @@ -2862,6 +2859,7 @@ export class TransactionController extends BaseController< return updatedTransaction ?? originalTransaction; }), ]; + state.transactions = this.trimTransactionsForState(updatedTransactions); }); } From 981b33cb803d25ac3c538fcd618fac6123fb572d Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 29 Feb 2024 11:53:51 -0700 Subject: [PATCH 05/19] Bring back removed test --- .../src/utils/swaps.test.ts | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/utils/swaps.test.ts b/packages/transaction-controller/src/utils/swaps.test.ts index 34042be57a..704441c417 100644 --- a/packages/transaction-controller/src/utils/swaps.test.ts +++ b/packages/transaction-controller/src/utils/swaps.test.ts @@ -99,7 +99,7 @@ describe('updateSwapsTransaction', () => { 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; @@ -193,6 +193,33 @@ describe('updateSwapsTransaction', () => { }); }); + 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 return the swap approval transaction updated with information', async () => { const sourceTokenSymbol = 'ETH'; const type = TransactionType.swapApproval; From 71ccbc8dfdcb4161c00c1201e1a6b52b0fc1f818 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 29 Feb 2024 11:56:54 -0700 Subject: [PATCH 06/19] Bring back this merge call --- packages/transaction-controller/src/utils/swaps.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/src/utils/swaps.ts b/packages/transaction-controller/src/utils/swaps.ts index d50f2fe91c..9e0b58d295 100644 --- a/packages/transaction-controller/src/utils/swaps.ts +++ b/packages/transaction-controller/src/utils/swaps.ts @@ -1,6 +1,6 @@ import { query } from '@metamask/controller-utils'; import type EthQuery from '@metamask/eth-query'; -import { pickBy } from 'lodash'; +import { merge, pickBy } from 'lodash'; import { CHAIN_IDS } from '../constants'; import { createModuleLogger, projectLogger } from '../logger'; @@ -322,7 +322,7 @@ function updateSwapTransaction( // eslint-disable-next-line @typescript-eslint/no-explicit-any swapTransaction = pickBy(swapTransaction) as any; - return { ...transactionMeta, ...swapTransaction }; + return merge({}, transactionMeta, swapTransaction); } /** From 62c198cc902ef6f322d2d813b7efecd40791e075 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 29 Feb 2024 11:59:46 -0700 Subject: [PATCH 07/19] Address one more merge --- packages/transaction-controller/src/utils/swaps.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/utils/swaps.ts b/packages/transaction-controller/src/utils/swaps.ts index 9e0b58d295..8206fff22f 100644 --- a/packages/transaction-controller/src/utils/swaps.ts +++ b/packages/transaction-controller/src/utils/swaps.ts @@ -351,7 +351,7 @@ function updateSwapApprovalTransaction( sourceTokenSymbol, }) as Partial; - return { ...transactionMeta, ...swapApprovalTransaction }; + return merge({}, transactionMeta, swapApprovalTransaction); } /** From 5a49f71be4505dbbe1e22ea4d1733a3b66aea499 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 29 Feb 2024 16:01:53 -0500 Subject: [PATCH 08/19] Remove all `: TransactionMeta` annotations that are not part of a function signature or type. Fix resulting errors with `as const` --- .../src/TransactionController.ts | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 4ee7014e25..aba41369e2 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -634,7 +634,7 @@ export class TransactionController extends BaseController< const newTransactionMeta = { ...transactionMeta, error: normalizeTxError(error), - status: TransactionStatus.failed, + status: TransactionStatus.failed as const, }; this.messagingSystem.publish(`${controllerName}:transactionFailed`, { actionId, @@ -1005,7 +1005,7 @@ export class TransactionController extends BaseController< 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. - let addedTransactionMeta: TransactionMeta = existingTransactionMeta + let addedTransactionMeta = existingTransactionMeta ? { ...existingTransactionMeta } : { // Add actionId to txMeta to check if same actionId is seen again @@ -1016,7 +1016,7 @@ export class TransactionController extends BaseController< id: random(), origin, securityAlertResponse, - status: TransactionStatus.unapproved as TransactionStatus.unapproved, + status: TransactionStatus.unapproved as const, time: Date.now(), txParams, userEditedGasLimit: false, @@ -1248,7 +1248,7 @@ export class TransactionController extends BaseController< transactionMeta, ); - const cancelTransactionMeta: TransactionMeta = { + const cancelTransactionMeta = { actionId, chainId: transactionMeta.chainId, networkClientId: transactionMeta.networkClientId, @@ -1256,9 +1256,9 @@ export class TransactionController extends BaseController< 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, }; @@ -1419,7 +1419,7 @@ export class TransactionController extends BaseController< transactionMeta, ); - const baseTransactionMeta: TransactionMeta = { + const baseTransactionMeta = { ...transactionMetaWithRsv, estimatedBaseFee, id: random(), @@ -1427,7 +1427,7 @@ export class TransactionController extends BaseController< hash, actionId, originalGasEstimate: transactionMeta.txParams.gas, - type: TransactionType.retry, + type: TransactionType.retry as const, originalType: transactionMeta.type, }; @@ -1618,9 +1618,9 @@ export class TransactionController extends BaseController< const transactionId = newTransactionMeta.id; // Make sure status is confirmed and define gasUsed as in receipt. - const updatedTransactionMeta: TransactionMeta = { + const updatedTransactionMeta = { ...newTransactionMeta, - status: TransactionStatus.confirmed, + status: TransactionStatus.confirmed as const, txReceipt: transactionReceipt, }; if (baseFeePerGas) { @@ -2481,7 +2481,7 @@ export class TransactionController extends BaseController< const releaseLock = await this.mutex.acquire(); const index = transactions.findIndex(({ id }) => transactionId === id); const transactionMeta = transactions[index]; - const updatedTransactionMeta: TransactionMeta = { ...transactionMeta }; + const updatedTransactionMeta = { ...transactionMeta }; const { txParams: { from }, @@ -2659,9 +2659,9 @@ export class TransactionController extends BaseController< ); state.transactions = this.trimTransactionsForState(transactions); }); - const updatedTransactionMeta: TransactionMeta = { + const updatedTransactionMeta = { ...transactionMeta, - status: TransactionStatus.rejected, + status: TransactionStatus.rejected as const, }; this.messagingSystem.publish( `${controllerName}:transactionFinished`, @@ -3020,9 +3020,9 @@ export class TransactionController extends BaseController< * @param transactionMeta - TransactionMeta of transaction to be marked as dropped. */ private setTransactionStatusDropped(transactionMeta: TransactionMeta) { - const updatedTransactionMeta: TransactionMeta = { + const updatedTransactionMeta = { ...transactionMeta, - status: TransactionStatus.dropped, + status: TransactionStatus.dropped as const, }; this.messagingSystem.publish(`${controllerName}:transactionDropped`, { transactionMeta: updatedTransactionMeta, @@ -3137,9 +3137,9 @@ export class TransactionController extends BaseController< return undefined; } - const transactionMetaWithRsv: TransactionMeta = { + const transactionMetaWithRsv = { ...(await this.updateTransactionMetaRSV(transactionMeta, signedTx)), - status: TransactionStatus.signed, + status: TransactionStatus.signed as const, }; this.updateTransaction( @@ -3448,7 +3448,7 @@ export class TransactionController extends BaseController< transactionMeta: TransactionMeta, { note, skipHistory }: { note?: string; skipHistory?: boolean }, ) { - const normalizedTransaction: TransactionMeta = { + const normalizedTransaction = { ...transactionMeta, txParams: normalizeTxParams(transactionMeta.txParams), }; From 55dd546e6d0143fffa04861b107f80d4fe92c723 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 10:51:48 -0700 Subject: [PATCH 09/19] Rename incomingTransactionBlock to incomingTransactionBlockReceived --- .../src/TransactionController.test.ts | 4 ++-- .../transaction-controller/src/TransactionController.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ea75932066..a47cb1c65c 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -3573,13 +3573,13 @@ describe('TransactionController', () => { ); }); - it('emits incomingTransactionBlock event', async () => { + it('publishes TransactionController:incomingTransactionBlockReceived', async () => { const blockNumber = 123; const listener = jest.fn(); const { messenger } = setupController(); messenger.subscribe( - 'TransactionController:incomingTransactionBlock', + 'TransactionController:incomingTransactionBlockReceived', listener, ); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 5b63073836..c727d352e9 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -340,8 +340,8 @@ export type TransactionControllerStateChangeEvent = ControllerStateChangeEvent< /** * Represents the `TransactionController:incomingTransactionBlockReceived` event. */ -export type TransactionControllerIncomingTransactionBlockEvent = { - type: `${typeof controllerName}:incomingTransactionBlock`; +export type TransactionControllerIncomingTransactionBlockReceivedEvent = { + type: `${typeof controllerName}:incomingTransactionBlockReceived`; payload: [blockNumber: number]; }; @@ -491,7 +491,7 @@ export type TransactionControllerUnapprovedTransactionAddedEvent = { * The internal events available to the {@link TransactionController}. */ export type TransactionControllerEvents = - | TransactionControllerIncomingTransactionBlockEvent + | TransactionControllerIncomingTransactionBlockReceivedEvent | TransactionControllerPostTransactionBalanceUpdatedEvent | TransactionControllerSpeedupTransactionAddedEvent | TransactionControllerStateChangeEvent @@ -2878,7 +2878,7 @@ export class TransactionController extends BaseController< state.lastFetchedBlockNumbers = lastFetchedBlockNumbers; }); this.messagingSystem.publish( - `${controllerName}:incomingTransactionBlock`, + `${controllerName}:incomingTransactionBlockReceived`, blockNumber, ); } From 07b1a68dd5a6b601f0079ab0afda6d4b90866953 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 10:53:33 -0700 Subject: [PATCH 10/19] Tweak test names to reflect new name of messenger events, and that they are published --- .../src/TransactionController.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index a47cb1c65c..b583f313fc 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1939,7 +1939,7 @@ describe('TransactionController', () => { expect(status).toBe(TransactionStatus.rejected); }); - it('emits rejected and finished event', async () => { + it('publishes TransactionController:transactionRejected and TransactionController:transactionFinished', async () => { const { controller, messenger } = setupController({ messengerOptions: { addTransactionApprovalRequest: { @@ -2415,7 +2415,7 @@ describe('TransactionController', () => { ).rejects.toThrow('No sign method defined'); }); - it('emits transaction events', async () => { + it('publishes transaction events', async () => { const { controller, messenger, mockTransactionApprovalRequest } = setupController({ network: MOCK_LINEA_GOERLI_NETWORK }); @@ -2778,7 +2778,7 @@ describe('TransactionController', () => { expect(controller.state.transactions).toHaveLength(2); }); - it('emits transaction events', async () => { + it('publishes transaction events', async () => { const { controller, messenger } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, }); @@ -3170,7 +3170,7 @@ describe('TransactionController', () => { ); }); - it('emits confirmed event', async () => { + it('publishes TransactionController:transactionConfirmed', async () => { const { controller, messenger } = setupController(); const confirmedEventListener = jest.fn(); @@ -3211,7 +3211,7 @@ describe('TransactionController', () => { ); }); - 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: { From 06fae6d1165fd34dd60ad25ca86484bf900ba796 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 11:01:28 -0700 Subject: [PATCH 11/19] No need to be _so_ explicit --- .../transaction-controller/src/TransactionController.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index c727d352e9..845c833d9f 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -533,7 +533,7 @@ export enum ApprovalState { * * @returns The default TransactionsController state. */ -function getDefaultTransactionControllerState() { +function getDefaultTransactionControllerState(): TransactionControllerState { return { methodData: {}, transactions: [], @@ -2704,8 +2704,8 @@ export class TransactionController extends BaseController< const { chainId, status, txParams, time } = tx; if (txParams) { - const key = `${String(txParams.nonce)}-${String( - convertHexToDecimal(chainId), + const key = `${String(txParams.nonce)}-${convertHexToDecimal( + chainId, )}-${new Date(time).toDateString()}`; if (nonceNetworkSet.has(key)) { From de1f551f6b810e1e2b95c07a9157d6314c014240 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 11:09:24 -0700 Subject: [PATCH 12/19] Tweak JSDoc --- packages/transaction-controller/src/utils/history.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/src/utils/history.ts b/packages/transaction-controller/src/utils/history.ts index a98b2fccc8..26747377f1 100644 --- a/packages/transaction-controller/src/utils/history.ts +++ b/packages/transaction-controller/src/utils/history.ts @@ -27,8 +27,8 @@ export function addInitialHistorySnapshot( * * @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 a - * `history`, or `transactionMeta` unchanged. + * @returns A copy of `transactionMeta` with a new `history` entry if it has an + * existing non-empty `history` array. */ export function updateTransactionHistory( transactionMeta: TransactionMeta, From 07b4147ab5f9164452e1ecc93a72582167b748f8 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 11:13:39 -0700 Subject: [PATCH 13/19] Rename txHistoryLimit to transactionHistoryLimit --- .../src/TransactionController.test.ts | 6 +++--- .../src/TransactionController.ts | 16 ++++++++-------- .../src/TransactionControllerIntegration.test.ts | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index b583f313fc..0ed23bf912 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -521,7 +521,7 @@ describe('TransactionController', () => { onNetworkStateChange: network.subscribe, provider: network.provider, sign: async (transaction: TypedTransaction) => transaction, - txHistoryLimit: 40, + transactionHistoryLimit: 40, ...givenOptions, }; @@ -2754,7 +2754,7 @@ describe('TransactionController', () => { it('allows transaction count to exceed txHistorylimit', async () => { const { controller } = setupController({ options: { - txHistoryLimit: 1, + transactionHistoryLimit: 1, }, messengerOptions: { addTransactionApprovalRequest: { @@ -3537,7 +3537,7 @@ describe('TransactionController', () => { it('limits max transactions when adding to state', async () => { const { controller } = setupController({ - options: { txHistoryLimit: 1 }, + options: { transactionHistoryLimit: 1 }, }); // TODO: Replace `any` with type diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 845c833d9f..9290d96050 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -259,7 +259,7 @@ export type PendingTransactionOptions = { * @property hooks.publish - Alternate logic to publish a transaction. * @property sign - Function used to sign transactions. * @property state - Initial state to set on this controller. - * @property txHistoryLimit - Transaction history limit. + * @property transactionHistoryLimit - Transaction history limit. */ export type TransactionControllerOptions = { blockTracker: BlockTracker; @@ -308,7 +308,7 @@ export type TransactionControllerOptions = { transactionMeta?: TransactionMeta, ) => Promise; state?: Partial; - txHistoryLimit: number; + transactionHistoryLimit: number; }; /** @@ -600,7 +600,7 @@ export class TransactionController extends BaseController< private readonly signAbortCallbacks: Map void> = new Map(); - #txHistoryLimit: number; + #transactionHistoryLimit: number; private readonly afterSign: ( transactionMeta: TransactionMeta, @@ -706,7 +706,7 @@ export class TransactionController extends BaseController< * @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.txHistoryLimit - Transaction history limit. + * @param options.transactionHistoryLimit - Transaction history limit. */ constructor({ blockTracker, @@ -732,7 +732,7 @@ export class TransactionController extends BaseController< hooks, sign, state, - txHistoryLimit = 40, + transactionHistoryLimit = 40, }: TransactionControllerOptions) { super({ name: controllerName, @@ -765,7 +765,7 @@ export class TransactionController extends BaseController< this.securityProviderRequest = securityProviderRequest; this.#incomingTransactionOptions = incomingTransactions; this.#pendingTransactionOptions = pendingTransactions; - this.#txHistoryLimit = txHistoryLimit; + this.#transactionHistoryLimit = transactionHistoryLimit; this.sign = sign; this.afterSign = hooks?.afterSign ?? (() => true); @@ -2711,7 +2711,7 @@ export class TransactionController extends BaseController< if (nonceNetworkSet.has(key)) { return true; } else if ( - nonceNetworkSet.size < this.#txHistoryLimit || + nonceNetworkSet.size < this.#transactionHistoryLimit || !this.isFinalState(status) ) { nonceNetworkSet.add(key); @@ -3270,7 +3270,7 @@ export class TransactionController extends BaseController< isEnabled: this.#incomingTransactionOptions.isEnabled, queryEntireHistory: this.#incomingTransactionOptions.queryEntireHistory, remoteTransactionSource: etherscanRemoteTransactionSource, - transactionLimit: this.#txHistoryLimit, + transactionLimit: this.#transactionHistoryLimit, updateTransactions: this.#incomingTransactionOptions.updateTransactions, }); diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 3048ee53f5..72c91fa446 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -172,7 +172,7 @@ const setupController = async ( }, provider, sign: async (transaction: TypedTransaction) => transaction, - txHistoryLimit: 40, + transactionHistoryLimit: 40, ...givenOptions, }; From 5cc6411b2036721a27347d55f98d8dfe4521a7a0 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 11:16:46 -0700 Subject: [PATCH 14/19] Alphabetize options (minus hooks) --- .../src/TransactionController.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 9290d96050..8b0790e66d 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -234,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. @@ -245,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. @@ -257,9 +260,6 @@ export type PendingTransactionOptions = { * @property hooks.beforePublish - Additional logic to execute before publishing a transaction. Return false to prevent the broadcast of the transaction. * @property hooks.getAdditionalSignArguments - Returns additional arguments required to sign a transaction. * @property hooks.publish - Alternate logic to publish a transaction. - * @property sign - Function used to sign transactions. - * @property state - Initial state to set on this controller. - * @property transactionHistoryLimit - Transaction history limit. */ export type TransactionControllerOptions = { blockTracker: BlockTracker; @@ -273,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, @@ -302,13 +309,6 @@ export type TransactionControllerOptions = { transactionMeta: TransactionMeta, ) => Promise<{ transactionHash: string }>; }; - sign?: ( - transaction: TypedTransaction, - from: string, - transactionMeta?: TransactionMeta, - ) => Promise; - state?: Partial; - transactionHistoryLimit: number; }; /** @@ -696,7 +696,6 @@ export class TransactionController extends BaseController< * @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.hooks - The controller hooks. * @param options.incomingTransactions - Configuration options for incoming transaction support. * @param options.isMultichainEnabled - Enable multichain support. * @param options.messenger - The controller messenger. @@ -707,6 +706,7 @@ export class TransactionController extends BaseController< * @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, @@ -717,22 +717,22 @@ export class TransactionController extends BaseController< getCurrentNetworkEIP1559Compatibility, getExternalPendingTransactions, getGasFeeEstimates, + getNetworkClientRegistry, getNetworkState, getPermittedAccounts, getSavedGasFees, getSelectedAddress, incomingTransactions = {}, + isMultichainEnabled = false, messenger, onNetworkStateChange, pendingTransactions = {}, provider, securityProviderRequest, - getNetworkClientRegistry, - isMultichainEnabled = false, - hooks, sign, state, transactionHistoryLimit = 40, + hooks, }: TransactionControllerOptions) { super({ name: controllerName, From 563ec545f619ce4a6c1d8158a7c7507d713628ca Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 16:57:11 -0700 Subject: [PATCH 15/19] PendingTransactionTracker doesn't use the return value of approveTransaction --- packages/transaction-controller/src/TransactionController.ts | 4 +++- .../src/helpers/PendingTransactionTracker.ts | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 8b0790e66d..495ee458a6 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -3292,7 +3292,9 @@ export class TransactionController extends BaseController< 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, diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index 6afdf77601..197bd01776 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -7,7 +7,6 @@ import type { import EventEmitter from 'events'; import { createModuleLogger, projectLogger } from '../logger'; -import type { ApprovalState } from '../TransactionController'; import type { TransactionMeta, TransactionReceipt } from '../types'; import { TransactionStatus, TransactionType } from '../types'; @@ -59,7 +58,7 @@ export interface PendingTransactionTrackerEventEmitter extends EventEmitter { export class PendingTransactionTracker { hub: PendingTransactionTrackerEventEmitter; - #approveTransaction: (transactionId: string) => Promise; + #approveTransaction: (transactionId: string) => Promise; #blockTracker: BlockTracker; @@ -98,7 +97,7 @@ export class PendingTransactionTracker { publishTransaction, hooks, }: { - approveTransaction: (transactionId: string) => Promise; + approveTransaction: (transactionId: string) => Promise; blockTracker: BlockTracker; getChainId: () => string; getEthQuery: (networkClientId?: NetworkClientId) => EthQuery; From 38824a37e125968b6514f10d959b68c1384c6c3e Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 17:01:08 -0700 Subject: [PATCH 16/19] Keep early return --- .../transaction-controller/src/utils/swaps.ts | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/transaction-controller/src/utils/swaps.ts b/packages/transaction-controller/src/utils/swaps.ts index 8206fff22f..9256ece7a8 100644 --- a/packages/transaction-controller/src/utils/swaps.ts +++ b/packages/transaction-controller/src/utils/swaps.ts @@ -166,25 +166,27 @@ export function updateSwapsTransaction( const swapsMeta = swaps?.meta as Partial; + if (!swapsMeta) { + return transactionMeta; + } + let updatedTransactionMeta = transactionMeta; - if (swapsMeta) { - if (transactionType === TransactionType.swapApproval) { - updatedTransactionMeta = updateSwapApprovalTransaction( - transactionMeta, - swapsMeta, - ); - messenger.publish('TransactionController:transactionNewSwapApproval', { - transactionMeta: updatedTransactionMeta, - }); - } else if (transactionType === TransactionType.swap) { - updatedTransactionMeta = updateSwapTransaction( - transactionMeta, - swapsMeta, - ); - messenger.publish('TransactionController:transactionNewSwap', { - transactionMeta: updatedTransactionMeta, - }); - } + + if (transactionType === TransactionType.swapApproval) { + updatedTransactionMeta = updateSwapApprovalTransaction( + transactionMeta, + swapsMeta, + ); + messenger.publish('TransactionController:transactionNewSwapApproval', { + transactionMeta: updatedTransactionMeta, + }); + } + + if (transactionType === TransactionType.swap) { + updatedTransactionMeta = updateSwapTransaction(transactionMeta, swapsMeta); + messenger.publish('TransactionController:transactionNewSwap', { + transactionMeta: updatedTransactionMeta, + }); } return updatedTransactionMeta; From 3d930c3899f47accffec4281c055495d061fdd87 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 5 Mar 2024 17:02:40 -0700 Subject: [PATCH 17/19] Fix index.ts --- packages/transaction-controller/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index 9440887be8..ea6a577d35 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -6,7 +6,7 @@ export type { TransactionControllerActions, TransactionControllerEvents, TransactionControllerGetStateAction, - TransactionControllerIncomingTransactionBlockEvent, + TransactionControllerIncomingTransactionBlockReceivedEvent, TransactionControllerPostTransactionBalanceUpdatedEvent, TransactionControllerSpeedupTransactionAddedEvent, TransactionControllerState, From 40f49f3bc816ce8909ab835faa16fbfceaa267a9 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 6 Mar 2024 13:40:19 -0700 Subject: [PATCH 18/19] Fix e2e test failures on extension by deep cloning --- .../src/TransactionController.test.ts | 6 +++-- .../src/TransactionController.ts | 23 ++++++++++--------- .../src/helpers/PendingTransactionTracker.ts | 7 +++--- .../src/utils/history.ts | 9 ++++---- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 0ed23bf912..7461bef811 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1216,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( @@ -1606,7 +1606,9 @@ describe('TransactionController', () => { ); const { securityProviderResponse } = controller.state.transactions[0]; - expect(securityProviderResponse).toBe(mockSecurityProviderResponse); + expect(securityProviderResponse).toStrictEqual( + mockSecurityProviderResponse, + ); }); it('updates gas properties', async () => { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 495ee458a6..8634d24f26 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -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, @@ -631,11 +631,10 @@ export class TransactionController extends BaseController< error: Error, actionId?: string, ) { - const newTransactionMeta = { - ...transactionMeta, + const newTransactionMeta = merge({}, transactionMeta, { error: normalizeTxError(error), status: TransactionStatus.failed as const, - }; + }); this.messagingSystem.publish(`${controllerName}:transactionFailed`, { actionId, error: error.message, @@ -1006,7 +1005,7 @@ export class TransactionController extends BaseController< // If a request to add a transaction with the same actionId is submitted again, a new transaction will not be created for it. let addedTransactionMeta = existingTransactionMeta - ? { ...existingTransactionMeta } + ? cloneDeep(existingTransactionMeta) : { // Add actionId to txMeta to check if same actionId is seen again actionId, @@ -1779,7 +1778,7 @@ export class TransactionController extends BaseController< transactionGasFees = pickBy(transactionGasFees); // merge updated gas values with existing transaction meta - const updatedMeta = merge(transactionMeta, transactionGasFees); + const updatedMeta = merge({}, transactionMeta, transactionGasFees); this.updateTransaction( updatedMeta, @@ -1837,7 +1836,7 @@ export class TransactionController extends BaseController< ); // merge updated previous gas values with existing transaction meta - const updatedMeta = merge(transactionMeta, transactionPreviousGas); + const updatedMeta = merge({}, transactionMeta, transactionPreviousGas); this.updateTransaction( updatedMeta, @@ -1912,7 +1911,7 @@ export class TransactionController extends BaseController< editableParams.txParams, ) as TransactionParams; - const updatedTransaction = merge(transactionMeta, editableParams); + const updatedTransaction = merge({}, transactionMeta, editableParams); const { type } = await determineTransactionType( updatedTransaction.txParams, this.#multichainTrackingHelper.getEthQuery({ @@ -2482,7 +2481,7 @@ export class TransactionController extends BaseController< const releaseLock = await this.mutex.acquire(); const index = transactions.findIndex(({ id }) => transactionId === id); const transactionMeta = transactions[index]; - const updatedTransactionMeta = { ...transactionMeta }; + const updatedTransactionMeta = cloneDeep(transactionMeta); const { txParams: { from }, @@ -3068,7 +3067,7 @@ export class TransactionController extends BaseController< transactionMeta: TransactionMeta, signedTx: TypedTransaction, ): Promise { - const transactionMetaWithRsv = { ...transactionMeta }; + const transactionMetaWithRsv = cloneDeep(transactionMeta); for (const key of ['r', 's', 'v'] as const) { const value = signedTx[key]; @@ -3152,7 +3151,9 @@ export class TransactionController extends BaseController< const rawTx = bufferToHex(signedTx.serialize()); - const transactionMetaWithRawTx = { ...transactionMetaWithRsv, rawTx }; + const transactionMetaWithRawTx = merge({}, transactionMetaWithRsv, { + rawTx, + }); this.updateTransaction( transactionMetaWithRawTx, diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index 197bd01776..19c738bdec 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -5,6 +5,7 @@ import type { NetworkClientId, } from '@metamask/network-controller'; import EventEmitter from 'events'; +import { cloneDeep, merge } from 'lodash'; import { createModuleLogger, projectLogger } from '../logger'; import type { TransactionMeta, TransactionReceipt } from '../types'; @@ -295,13 +296,13 @@ export class PendingTransactionTracker { const retryCount = (txMeta.retryCount ?? 0) + 1; this.#updateTransaction( - { ...txMeta, retryCount }, + merge({}, txMeta, { retryCount }), 'PendingTransactionTracker:transaction-retry - Retry count increased', ); } #isResubmitDue(txMeta: TransactionMeta, latestBlockNumber: string): boolean { - const txMetaWithFirstRetryBlockNumber = { ...txMeta }; + const txMetaWithFirstRetryBlockNumber = cloneDeep(txMeta); if (!txMetaWithFirstRetryBlockNumber.firstRetryBlockNumber) { txMetaWithFirstRetryBlockNumber.firstRetryBlockNumber = latestBlockNumber; @@ -409,7 +410,7 @@ export class PendingTransactionTracker { const { baseFeePerGas, timestamp: blockTimestamp } = await this.#getBlockByHash(blockHash, false); - const updatedTxMeta = { ...txMeta }; + const updatedTxMeta = cloneDeep(txMeta); updatedTxMeta.baseFeePerGas = baseFeePerGas; updatedTxMeta.blockTimestamp = blockTimestamp; updatedTxMeta.status = TransactionStatus.confirmed; diff --git a/packages/transaction-controller/src/utils/history.ts b/packages/transaction-controller/src/utils/history.ts index 26747377f1..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, @@ -18,7 +18,7 @@ export function addInitialHistorySnapshot( transactionMeta: TransactionMeta, ): TransactionMeta { const snapshot = snapshotFromTransactionMeta(transactionMeta); - return { ...transactionMeta, history: [snapshot] }; + return merge({}, transactionMeta, { history: [snapshot] }); } /** @@ -43,10 +43,9 @@ export function updateTransactionHistory( const historyEntry = generateHistoryEntry(previousState, currentState, note); if (historyEntry.length > 0) { - return { - ...transactionMeta, + return merge({}, transactionMeta, { history: [...transactionMeta.history, historyEntry], - }; + }); } return transactionMeta; } From d8339f7ebddd228438ca61f2bf2c26aa0eb6d41b Mon Sep 17 00:00:00 2001 From: MetaMask Bot <37885440+metamaskbot@users.noreply.github.com> Date: Wed, 6 Mar 2024 14:29:13 -0500 Subject: [PATCH 19/19] Update security code scanner file (#4025) ## Automated PR This pull request replaces mixpanel token with more generic naming. --- .github/workflows/security-code-scanner.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security-code-scanner.yml b/.github/workflows/security-code-scanner.yml index d2be2998f4..a449cbc3fa 100644 --- a/.github/workflows/security-code-scanner.yml +++ b/.github/workflows/security-code-scanner.yml @@ -26,5 +26,5 @@ jobs: node_modules merged-packages/ '**/jest.environment.js' - mixpanel_project_token: ${{secrets.SECURITY_CODE_SCANNER_MIXPANEL_TOKEN}} + project_metrics_token: ${{secrets.SECURITY_SCAN_METRICS_TOKEN}} slack_webhook: ${{ secrets.APPSEC_BOT_SLACK_WEBHOOK }}