From 943c1222ef7cd6a2d9804d8a9ffef4f6bf25a5cf Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 13 Nov 2024 10:47:06 +0000 Subject: [PATCH 01/12] Remove all usages of global network Remove multichain flag. Remove provider and block tracker constructor options. Temporarily disable handleMethodData method. Remove unnecessary unit tests. Fix unit tests. --- .../src/TransactionController.test.ts | 747 +++++++++++------- .../src/TransactionController.ts | 248 ++---- .../TransactionControllerIntegration.test.ts | 355 +-------- .../helpers/MultichainTrackingHelper.test.ts | 273 ------- .../src/helpers/MultichainTrackingHelper.ts | 71 +- 5 files changed, 540 insertions(+), 1154 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 05bfd86bf4..2ce3fdca4f 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -31,7 +31,6 @@ import { NetworkStatus, getDefaultNetworkControllerState, } from '@metamask/network-controller'; -import * as NonceTrackerPackage from '@metamask/nonce-tracker'; import { errorCodes, providerErrors, rpcErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; import { createDeferredPromise } from '@metamask/utils'; @@ -59,6 +58,7 @@ import type { AllowedEvents, TransactionControllerActions, TransactionControllerEvents, + TransactionControllerOptions, } from './TransactionController'; import { TransactionController } from './TransactionController'; import type { @@ -545,7 +545,11 @@ describe('TransactionController', () => { network = {}, messengerOptions = {}, selectedAccount = INTERNAL_ACCOUNT_MOCK, - mockNetworkClientConfigurationsByNetworkClientId = {}, + mockNetworkClientConfigurationsByNetworkClientId = { + [NETWORK_CLIENT_ID_MOCK]: buildCustomNetworkClientConfiguration({ + chainId: CHAIN_ID_MOCK, + }), + }, updateToInitialState = false, }: { options?: Partial[0]>; @@ -574,9 +578,6 @@ describe('TransactionController', () => { selectedNetworkClientId: MOCK_NETWORK.state.selectedNetworkClientId, ...network.state, }; - const provider = network.provider ?? new FakeProvider(); - const blockTracker = - network.blockTracker ?? new FakeBlockTracker({ provider }); const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; const changeNetwork = ({ selectedNetworkClientId, @@ -591,10 +592,6 @@ describe('TransactionController', () => { listener(networkState); }); }; - const onNetworkStateChange = ( - listener: (networkState: NetworkState) => void, - ) => onNetworkDidChangeListeners.push(listener); - const unrestrictedMessenger: UnrestrictedControllerMessenger = new ControllerMessenger(); const getNetworkClientById = buildMockGetNetworkClientById( @@ -612,21 +609,20 @@ describe('TransactionController', () => { addTransactionApprovalRequest, ); - const { messenger: givenRestrictedMessenger, ...otherOptions } = { - blockTracker, + const { + messenger: givenRestrictedMessenger, + ...otherOptions + }: Partial = { disableHistory: false, disableSendFlowHistory: false, disableSwaps: false, getCurrentNetworkEIP1559Compatibility: async () => false, getNetworkState: () => networkState, - // TODO: Replace with a real type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getNetworkClientRegistry: () => ({} as any), + getNetworkClientRegistry: () => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockNetworkClientConfigurationsByNetworkClientId as any, getPermittedAccounts: async () => [ACCOUNT_MOCK], - isMultichainEnabled: false, hooks: {}, - onNetworkStateChange, - provider, sign: async (transaction: TypedTransaction) => transaction, transactionHistoryLimit: 40, ...givenOptions, @@ -654,7 +650,7 @@ describe('TransactionController', () => { const controller = new TransactionController({ ...otherOptions, messenger: restrictedMessenger, - }); + } as TransactionControllerOptions); const state = givenOptions?.state; @@ -663,6 +659,16 @@ describe('TransactionController', () => { (controller as any).update(() => state); } + multichainTrackingHelperClassMock.mock.calls[0][0].createIncomingTransactionHelper( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + + multichainTrackingHelperClassMock.mock.calls[0][0].createPendingTransactionTracker( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any, + ); + return { controller, messenger: unrestrictedMessenger, @@ -821,18 +827,18 @@ describe('TransactionController', () => { return pendingTransactionTrackerMock; }); - multichainTrackingHelperClassMock.mockImplementation(({ provider }) => { + multichainTrackingHelperClassMock.mockImplementation(() => { multichainTrackingHelperMock = { getEthQuery: jest.fn().mockImplementation(() => { - return new EthQuery(provider); + return new EthQuery(new FakeProvider()); }), getProvider: jest.fn().mockImplementation(() => { - return provider; + return new FakeProvider(); }), checkForPendingTransactionAndStartPolling: jest.fn(), getNonceLock: getNonceLockSpy, initialize: jest.fn(), - has: jest.fn().mockReturnValue(false), + has: jest.fn().mockReturnValue(true), } as unknown as jest.Mocked; return multichainTrackingHelperMock; }); @@ -909,57 +915,6 @@ describe('TransactionController', () => { ); }); - describe('nonce tracker', () => { - it('uses external pending transactions', async () => { - const nonceTrackerMock = jest - .spyOn(NonceTrackerPackage, 'NonceTracker') - .mockImplementation(); - - const externalPendingTransactions = [ - { - from: '0x1', - }, - { from: '0x2' }, - ]; - - const getExternalPendingTransactions = jest - .fn() - .mockReturnValueOnce(externalPendingTransactions); - - setupController({ - options: { - getExternalPendingTransactions, - state: { - transactions: [ - { - ...TRANSACTION_META_MOCK, - chainId: MOCK_NETWORK.chainId, - status: TransactionStatus.submitted, - }, - ], - }, - }, - }); - - const pendingTransactions = - nonceTrackerMock.mock.calls[0][0].getPendingTransactions( - ACCOUNT_MOCK, - ); - - expect(nonceTrackerMock).toHaveBeenCalledTimes(1); - expect(pendingTransactions).toStrictEqual([ - expect.any(Object), - ...externalPendingTransactions, - ]); - expect(getExternalPendingTransactions).toHaveBeenCalledTimes(1); - expect(getExternalPendingTransactions).toHaveBeenCalledWith( - ACCOUNT_MOCK, - // This is undefined for the base nonceTracker - undefined, - ); - }); - }); - it('fails approved and signed transactions for all chains', async () => { const mockTransactionMeta = { from: ACCOUNT_MOCK, @@ -1188,6 +1143,7 @@ describe('TransactionController', () => { { origin: mockOrigin, actionId: ACTION_ID_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -1201,6 +1157,7 @@ describe('TransactionController', () => { { origin: mockOrigin, actionId: ACTION_ID_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); const secondTransactionCount = controller.state.transactions.length; @@ -1238,6 +1195,7 @@ describe('TransactionController', () => { { origin: mockOrigin, actionId: ACTION_ID_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -1255,6 +1213,7 @@ describe('TransactionController', () => { { origin: mockOrigin, actionId: ACTION_ID_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); secondResult @@ -1306,6 +1265,7 @@ describe('TransactionController', () => { { origin: mockOrigin, actionId: firstActionId, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -1317,6 +1277,7 @@ describe('TransactionController', () => { { origin: mockOrigin, actionId: secondActionId, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); const { transactions } = controller.state; @@ -1352,13 +1313,19 @@ describe('TransactionController', () => { const { controller } = setupController(); const signSpy = jest.spyOn(controller, 'sign'); - const { transactionMeta } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + await controller.speedUpTransaction(transactionMeta.id, undefined, { actionId: ACTION_ID_MOCK, }); @@ -1409,6 +1376,7 @@ describe('TransactionController', () => { origin: mockOrigin, securityAlertResponse: mockSecurityAlertResponse, sendFlowHistory: mockSendFlowHistory, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -1430,9 +1398,7 @@ describe('TransactionController', () => { describe('networkClientId exists in the MultichainTrackingHelper', () => { it('adds unapproved transaction to state when using networkClientId', async () => { - const { controller } = setupController({ - options: { isMultichainEnabled: true }, - }); + const { controller } = setupController(); const sepoliaTxParams: TransactionParams = { chainId: ChainId.sepolia, from: ACCOUNT_MOCK, @@ -1459,7 +1425,6 @@ describe('TransactionController', () => { it('adds unapproved transaction with networkClientId and can be updated to submitted', async () => { const { controller, messenger } = setupController({ - options: { isMultichainEnabled: true }, messengerOptions: { addTransactionApprovalRequest: { state: 'approved', @@ -1501,10 +1466,15 @@ describe('TransactionController', () => { it('generates initial history', async () => { const { controller } = setupController(); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); const expectedInitialSnapshot = { actionId: undefined, @@ -1512,7 +1482,7 @@ describe('TransactionController', () => { dappSuggestedGasFees: undefined, deviceConfirmedOn: undefined, id: expect.any(String), - networkClientId: MOCK_NETWORK.state.selectedNetworkClientId, + networkClientId: NETWORK_CLIENT_ID_MOCK, origin: undefined, securityAlertResponse: undefined, sendFlowHistory: expect.any(Array), @@ -1544,6 +1514,7 @@ describe('TransactionController', () => { }, { origin, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); expect( @@ -1570,6 +1541,7 @@ describe('TransactionController', () => { }, { origin: mockDappOrigin, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); expect( @@ -1591,13 +1563,18 @@ describe('TransactionController', () => { }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.mainnet }); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); expect(controller.state.transactions[0].txParams.from).toBe(ACCOUNT_MOCK); - expect(controller.state.transactions[0].chainId).toBe(ChainId.mainnet); + expect(controller.state.transactions[0].chainId).toBe(CHAIN_ID_MOCK); expect(controller.state.transactions[0].status).toBe( TransactionStatus.unapproved, ); @@ -1618,10 +1595,15 @@ describe('TransactionController', () => { }); changeNetwork({ selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD' }); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: 'AAAA-BBBB-CCCC-DDDD', + }, + ); expect(controller.state.transactions[0].txParams.from).toBe(ACCOUNT_MOCK); expect(controller.state.transactions[0].chainId).toBe('0x1337'); @@ -1632,9 +1614,12 @@ describe('TransactionController', () => { it('throws if address invalid', async () => { const { controller } = setupController(); - await expect(controller.addTransaction({ from: 'foo' })).rejects.toThrow( - 'Invalid "from" address', - ); + await expect( + controller.addTransaction( + { from: 'foo' }, + { networkClientId: NETWORK_CLIENT_ID_MOCK }, + ), + ).rejects.toThrow('Invalid "from" address'); }); it('increments nonce when adding a new non-cancel non-speedup transaction', async () => { @@ -1650,13 +1635,18 @@ describe('TransactionController', () => { }, }); - const { result: firstResult } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { result: firstResult } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await firstResult.catch(() => undefined); @@ -1668,13 +1658,18 @@ describe('TransactionController', () => { releaseLock: () => Promise.resolve(), }); - const { result: secondResult } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x2', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x1290', - }); + const { result: secondResult } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x2', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x1290', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await secondResult.catch(() => undefined); @@ -1694,10 +1689,15 @@ describe('TransactionController', () => { const { controller, messenger } = setupController(); const messengerCallSpy = jest.spyOn(messenger, 'call'); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); expect( messengerCallSpy.mock.calls.filter( @@ -1728,6 +1728,7 @@ describe('TransactionController', () => { }, { requireApproval: false, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -1759,6 +1760,7 @@ describe('TransactionController', () => { }, { method: mockRPCMethodName, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -1779,17 +1781,21 @@ describe('TransactionController', () => { it('updates gas properties', async () => { const { controller } = setupController(); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); expect(updateGasMock).toHaveBeenCalledTimes(1); expect(updateGasMock).toHaveBeenCalledWith({ ethQuery: expect.any(Object), - chainId: MOCK_NETWORK.chainId, - // XXX: Is this right? - isCustomNetwork: false, + chainId: CHAIN_ID_MOCK, + isCustomNetwork: true, txMeta: expect.any(Object), }); }); @@ -1802,10 +1808,15 @@ describe('TransactionController', () => { }, }); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); expect(updateGasFeesMock).toHaveBeenCalledTimes(1); expect(updateGasFeesMock).toHaveBeenCalledWith({ @@ -1824,10 +1835,15 @@ describe('TransactionController', () => { const { controller } = setupController(); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await flushPromises(); @@ -1857,10 +1873,15 @@ describe('TransactionController', () => { options: { isSimulationEnabled: () => false }, }); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); expect(getSimulationDataMock).toHaveBeenCalledTimes(0); expect(controller.state.transactions[0].simulationData).toStrictEqual({ @@ -1882,7 +1903,7 @@ describe('TransactionController', () => { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }, - { requireApproval: false }, + { requireApproval: false, networkClientId: NETWORK_CLIENT_ID_MOCK }, ); expect(getSimulationDataMock).toHaveBeenCalledTimes(0); @@ -1905,13 +1926,18 @@ describe('TransactionController', () => { submittedEventListener, ); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x0', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await result; @@ -1957,10 +1983,15 @@ describe('TransactionController', () => { }, }); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await result; @@ -1986,10 +2017,15 @@ describe('TransactionController', () => { }, }); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); try { await result; @@ -2004,10 +2040,15 @@ describe('TransactionController', () => { const { controller, mockTransactionApprovalRequest } = setupController(); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); const transaction = controller.state.transactions[0]; @@ -2037,10 +2078,15 @@ describe('TransactionController', () => { controller: TransactionController, expectedError: string, ) { - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await expect(result).rejects.toThrow(expectedError); @@ -2085,17 +2131,12 @@ describe('TransactionController', () => { it('if no chainId defined', async () => { const { controller } = setupController({ mockNetworkClientConfigurationsByNetworkClientId: { - 'AAAA-BBBB-CCCC-DDDD': buildCustomNetworkClientConfiguration({ + [NETWORK_CLIENT_ID_MOCK]: buildCustomNetworkClientConfiguration({ rpcUrl: 'https://test.network', ticker: 'TEST', chainId: undefined, }), }, - network: { - state: { - selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD', - }, - }, messengerOptions: { addTransactionApprovalRequest: { state: 'approved', @@ -2116,13 +2157,18 @@ describe('TransactionController', () => { }, }); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x0', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await expect(result).rejects.toThrow('Unknown problem'); }); @@ -2137,13 +2183,18 @@ describe('TransactionController', () => { }, }); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x0', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await expect(result).rejects.toThrow('TestError'); }); @@ -2152,13 +2203,18 @@ describe('TransactionController', () => { const { controller, mockTransactionApprovalRequest } = setupController(); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x0', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); controller.clearUnapprovedTransactions(); mockTransactionApprovalRequest.reject(new Error('Unknown problem')); @@ -2178,10 +2234,15 @@ describe('TransactionController', () => { }, }); - const { result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); const finishedPromise = waitForTransactionFinished(messenger); @@ -2218,6 +2279,7 @@ describe('TransactionController', () => { }, { actionId: mockActionId, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -2249,7 +2311,10 @@ describe('TransactionController', () => { from: notSelectedFromAddress, to: ACCOUNT_MOCK, }, - { origin: ORIGIN_METAMASK }, + { + origin: ORIGIN_METAMASK, + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, ), ).rejects.toThrow( rpcErrors.internal({ @@ -2269,7 +2334,7 @@ describe('TransactionController', () => { await expect( controller.addTransaction( { from: ACCOUNT_2_MOCK, to: ACCOUNT_MOCK }, - { origin: expectedOrigin }, + { origin: expectedOrigin, networkClientId: NETWORK_CLIENT_ID_MOCK }, ), ).rejects.toThrow( providerErrors.unauthorized({ data: { origin: expectedOrigin } }), @@ -2301,7 +2366,7 @@ describe('TransactionController', () => { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }, - { origin: ORIGIN_METAMASK }, + { origin: ORIGIN_METAMASK, networkClientId: NETWORK_CLIENT_ID_MOCK }, ); await result; @@ -2310,8 +2375,8 @@ describe('TransactionController', () => { { chainId: MOCK_NETWORK.chainId, hash: TRANSACTION_HASH_MOCK, - networkType: NetworkType.goerli, - networkUrl: expect.stringContaining('goerli.infura.io'), + networkType: NETWORK_CLIENT_ID_MOCK, + networkUrl: undefined, origin: ORIGIN_METAMASK, rawTransaction: expect.stringContaining('0x'), time: expect.any(Number), @@ -2355,6 +2420,7 @@ describe('TransactionController', () => { }, { origin: ORIGIN_METAMASK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -2378,10 +2444,15 @@ describe('TransactionController', () => { controller.wipeTransactions(); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); controller.wipeTransactions(); @@ -2419,7 +2490,7 @@ describe('TransactionController', () => { }, }); - controller.wipeTransactions(true, mockFromAccount2); + controller.wipeTransactions(undefined, mockFromAccount2); expect(controller.state.transactions).toHaveLength(1); expect(controller.state.transactions[0].id).toBe('1'); @@ -2456,14 +2527,14 @@ describe('TransactionController', () => { }, }); - controller.wipeTransactions(false, mockFromAccount1); + controller.wipeTransactions(mockCurrentChainId, mockFromAccount1); expect(controller.state.transactions).toHaveLength(1); expect(controller.state.transactions[0].id).toBe('4'); }); }); - describe('handleMethodData', () => { + describe.skip('handleMethodData', () => { it('loads method data from registry', async () => { const { controller } = setupController({ network: MOCK_MAINNET_NETWORK }); mockNetwork({ @@ -2767,7 +2838,10 @@ describe('TransactionController', () => { it('throws if no sign method', async () => { const { controller } = setupController({ options: { sign: undefined } }); - await controller.addTransaction({ from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }); + await controller.addTransaction( + { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }, + { networkClientId: NETWORK_CLIENT_ID_MOCK }, + ); await expect( controller.stopTransaction(controller.state.transactions[0].id), @@ -2793,13 +2867,18 @@ describe('TransactionController', () => { submittedEventListener, ); - const { transactionMeta } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x1', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x1', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); messenger.subscribe( 'TransactionController:transactionFinished', @@ -2859,6 +2938,7 @@ describe('TransactionController', () => { }, { origin: ORIGIN_METAMASK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -2872,8 +2952,8 @@ describe('TransactionController', () => { { chainId: MOCK_NETWORK.chainId, hash: TRANSACTION_HASH_MOCK, - networkType: NetworkType.goerli, - networkUrl: expect.stringContaining('goerli.infura.io'), + networkType: NETWORK_CLIENT_ID_MOCK, + networkUrl: undefined, origin: 'cancel', rawTransaction: expect.stringContaining('0x'), time: expect.any(Number), @@ -2903,13 +2983,18 @@ describe('TransactionController', () => { }, }); - const { transactionMeta } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await controller.speedUpTransaction(transactionMeta.id); @@ -3028,13 +3113,18 @@ describe('TransactionController', () => { }, }); - const { transactionMeta } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await controller.speedUpTransaction(transactionMeta.id); @@ -3059,13 +3149,18 @@ describe('TransactionController', () => { }, }); - const { transactionMeta } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await controller.speedUpTransaction(transactionMeta.id); @@ -3095,13 +3190,18 @@ describe('TransactionController', () => { }, }); - const { transactionMeta } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await controller.speedUpTransaction(transactionMeta.id); @@ -3121,13 +3221,18 @@ describe('TransactionController', () => { }, }); - const { transactionMeta } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await controller.speedUpTransaction(transactionMeta.id, { gasPrice: '0x62DEF4DA', @@ -3147,13 +3252,18 @@ describe('TransactionController', () => { }, }); - const { transactionMeta, result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x1', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta, result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x1', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await result; await controller.speedUpTransaction(transactionMeta.id, undefined, { @@ -3183,14 +3293,19 @@ describe('TransactionController', () => { }, }); - const { transactionMeta, result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - nonce: '1111111', - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }); + const { transactionMeta, result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + nonce: '1111111', + gas: '0x0', + gasPrice: '0x50fd51da', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); await result; await controller.speedUpTransaction(transactionMeta.id); @@ -3219,13 +3334,18 @@ describe('TransactionController', () => { ); const { transactionMeta: firstTransactionMeta } = - await controller.addTransaction({ - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x1', - to: ACCOUNT_MOCK, - value: '0x0', - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x1', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); messenger.subscribe( 'TransactionController:speedupTransactionAdded', @@ -3274,6 +3394,7 @@ describe('TransactionController', () => { }, { origin: ORIGIN_METAMASK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -3287,8 +3408,8 @@ describe('TransactionController', () => { { chainId: MOCK_NETWORK.chainId, hash: TRANSACTION_HASH_MOCK, - networkType: NetworkType.goerli, - networkUrl: expect.stringContaining('goerli.infura.io'), + networkType: NETWORK_CLIENT_ID_MOCK, + networkUrl: undefined, origin: 'speed up', rawTransaction: expect.stringContaining('0x'), time: expect.any(Number), @@ -3696,10 +3817,13 @@ describe('TransactionController', () => { timestamp: 1650663928211, }, ]; - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { networkClientId: NETWORK_CLIENT_ID_MOCK }, + ); const addedTxId = controller.state.transactions[0].id; controller.updateTransactionSendFlowHistory( addedTxId, @@ -3734,6 +3858,7 @@ describe('TransactionController', () => { }, { sendFlowHistory: mockExistingSendFlowHistory, + networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); const addedTxId = controller.state.transactions[0].id; @@ -3758,10 +3883,15 @@ describe('TransactionController', () => { timestamp: 1650663928211, }, ]; - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); const addedTxId = controller.state.transactions[0].id; controller.updateTransactionSendFlowHistory( addedTxId, @@ -3785,10 +3915,15 @@ describe('TransactionController', () => { timestamp: 1650663928211, }, ]; - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); const addedTxId = controller.state.transactions[0].id; expect(() => controller.updateTransactionSendFlowHistory( @@ -4740,6 +4875,7 @@ describe('TransactionController', () => { await controller.addTransaction(paramsMock, { origin: 'origin', actionId: ACTION_ID_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }); mockTransactionApprovalRequest.approve({ @@ -4782,6 +4918,7 @@ describe('TransactionController', () => { await controller.addTransaction(paramsMock, { origin: 'origin', actionId: ACTION_ID_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }); mockTransactionApprovalRequest.approve({ @@ -4808,6 +4945,7 @@ describe('TransactionController', () => { await controller.addTransaction(paramsMock, { origin: 'origin', actionId: ACTION_ID_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, }); mockTransactionApprovalRequest.approve(); @@ -4846,7 +4984,9 @@ describe('TransactionController', () => { }); jest.spyOn(mockEthQuery, 'sendRawTransaction'); - const { result } = await controller.addTransaction(paramsMock); + const { result } = await controller.addTransaction(paramsMock, { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }); await result; @@ -4875,7 +5015,9 @@ describe('TransactionController', () => { }, }); - const { result } = await controller.addTransaction(paramsMock); + const { result } = await controller.addTransaction(paramsMock, { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }); await result; @@ -4904,7 +5046,9 @@ describe('TransactionController', () => { }, }); - const { result } = await controller.addTransaction(paramsMock); + const { result } = await controller.addTransaction(paramsMock, { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }); await result; @@ -5274,7 +5418,6 @@ describe('TransactionController', () => { expect( controller.getTransactions({ searchCriteria: { time: 1 }, - filterToCurrentNetwork: false, }), ).toStrictEqual([transactions[0], transactions[2]]); }); @@ -5314,7 +5457,6 @@ describe('TransactionController', () => { expect( controller.getTransactions({ searchCriteria: { from: '0x1' }, - filterToCurrentNetwork: false, }), ).toStrictEqual([transactions[0], transactions[2]]); }); @@ -5354,7 +5496,6 @@ describe('TransactionController', () => { expect( controller.getTransactions({ searchCriteria: { from: '0x1', time: 1 }, - filterToCurrentNetwork: false, }), ).toStrictEqual([transactions[0], transactions[2]]); }); @@ -5396,12 +5537,11 @@ describe('TransactionController', () => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any searchCriteria: { time: (v: any) => v === 1 }, - filterToCurrentNetwork: false, }), ).toStrictEqual([transactions[0], transactions[2]]); }); - it('returns transactions matching current network', () => { + it('returns transactions matching specified chain', () => { const transactions: TransactionMeta[] = [ { chainId: MOCK_NETWORK.chainId, @@ -5435,7 +5575,7 @@ describe('TransactionController', () => { expect( controller.getTransactions({ - filterToCurrentNetwork: true, + searchCriteria: { chainId: MOCK_NETWORK.chainId }, }), ).toStrictEqual([transactions[0], transactions[2]]); }); @@ -5471,7 +5611,6 @@ describe('TransactionController', () => { controller.getTransactions({ searchCriteria: { time: 1 }, initialList: transactions, - filterToCurrentNetwork: false, }), ).toStrictEqual([transactions[0], transactions[2]]); }); @@ -5518,7 +5657,6 @@ describe('TransactionController', () => { expect( controller.getTransactions({ searchCriteria: { from: '0x1' }, - filterToCurrentNetwork: false, limit: 2, }), ).toStrictEqual([transactions[1], transactions[3]]); @@ -5567,7 +5705,6 @@ describe('TransactionController', () => { expect( controller.getTransactions({ searchCriteria: { from: '0x1' }, - filterToCurrentNetwork: false, limit: 2, }), ).toStrictEqual([transactions[0], transactions[2], transactions[3]]); @@ -5714,10 +5851,15 @@ describe('TransactionController', () => { }, }); - const { transactionMeta, result } = await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const { transactionMeta, result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); result.catch(() => { // Ignore error @@ -5774,6 +5916,7 @@ describe('TransactionController', () => { const result = await controller.estimateGasFee({ transactionParams: TRANSACTION_META_MOCK.txParams, + networkClientId: NETWORK_CLIENT_ID_MOCK, }); expect(result).toStrictEqual(GAS_FEE_ESTIMATES_MOCK); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 64fb2d3064..f16cb8102f 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -19,7 +19,6 @@ import { ApprovalType, ORIGIN_METAMASK, convertHexToDecimal, - isInfuraNetworkType, } from '@metamask/controller-utils'; import type { TraceCallback, TraceContext } from '@metamask/controller-utils'; import EthQuery from '@metamask/eth-query'; @@ -47,7 +46,6 @@ import { errorCodes, rpcErrors, providerErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; import { add0x } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { MethodRegistry } from 'eth-method-registry'; import { EventEmitter } from 'events'; import { cloneDeep, mapValues, merge, pickBy, sortBy } from 'lodash'; import { v1 as random } from 'uuid'; @@ -57,7 +55,7 @@ import { LineaGasFeeFlow } from './gas-flows/LineaGasFeeFlow'; import { OptimismLayer1GasFeeFlow } from './gas-flows/OptimismLayer1GasFeeFlow'; import { ScrollLayer1GasFeeFlow } from './gas-flows/ScrollLayer1GasFeeFlow'; import { TestGasFeeFlow } from './gas-flows/TestGasFeeFlow'; -import { EtherscanRemoteTransactionSource } from './helpers/EtherscanRemoteTransactionSource'; +import type { EtherscanRemoteTransactionSource } from './helpers/EtherscanRemoteTransactionSource'; import { GasFeePoller } from './helpers/GasFeePoller'; import type { IncomingTransactionOptions } from './helpers/IncomingTransactionHelper'; import { IncomingTransactionHelper } from './helpers/IncomingTransactionHelper'; @@ -242,7 +240,6 @@ export type PendingTransactionOptions = { /** * TransactionController constructor options. * - * @property blockTracker - The block tracker used to poll for new blocks data. * @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. @@ -256,12 +253,9 @@ 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 isSimulationEnabled - Whether new transactions will be automatically simulated. * @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. @@ -275,7 +269,6 @@ export type PendingTransactionOptions = { * @property hooks.publish - Alternate logic to publish a transaction. */ export type TransactionControllerOptions = { - blockTracker: BlockTracker; disableHistory: boolean; disableSendFlowHistory: boolean; disableSwaps: boolean; @@ -296,12 +289,9 @@ export type TransactionControllerOptions = { /** API keys to be used for Etherscan requests to prevent rate limiting. */ etherscanApiKeysByChainId?: Record; }; - isMultichainEnabled: boolean; isSimulationEnabled?: () => boolean; messenger: TransactionControllerMessenger; - onNetworkStateChange: (listener: (state: NetworkState) => void) => void; pendingTransactions?: PendingTransactionOptions; - provider: Provider; securityProviderRequest?: SecurityProviderRequest; sign?: ( transaction: TypedTransaction, @@ -589,10 +579,6 @@ export class TransactionController extends BaseController< private readonly approvingTransactionIds: Set = new Set(); - private readonly nonceTracker: NonceTracker; - - private readonly registry: MethodRegistry; - private readonly mutex = new Mutex(); private readonly gasFeeFlows: GasFeeFlow[]; @@ -624,14 +610,10 @@ export class TransactionController extends BaseController< readonly #incomingTransactionOptions: IncomingTransactionOptions; - private readonly incomingTransactionHelper: IncomingTransactionHelper; - private readonly securityProviderRequest?: SecurityProviderRequest; readonly #pendingTransactionOptions: PendingTransactionOptions; - private readonly pendingTransactionTracker: PendingTransactionTracker; - private readonly signAbortCallbacks: Map void> = new Map(); #trace: TraceCallback; @@ -715,17 +697,17 @@ export class TransactionController extends BaseController< ); } - 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 }; - } + // 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; @@ -742,7 +724,6 @@ export class TransactionController extends BaseController< * 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. @@ -755,12 +736,9 @@ 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.incomingTransactions - Configuration options for incoming transaction support. - * @param options.isMultichainEnabled - Enable multichain support. * @param options.isSimulationEnabled - Whether new transactions will be automatically simulated. * @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. @@ -770,7 +748,6 @@ export class TransactionController extends BaseController< * @param options.hooks - The controller hooks. */ constructor({ - blockTracker, disableHistory, disableSendFlowHistory, disableSwaps, @@ -783,12 +760,9 @@ export class TransactionController extends BaseController< getPermittedAccounts, getSavedGasFees, incomingTransactions = {}, - isMultichainEnabled = false, isSimulationEnabled, messenger, - onNetworkStateChange, pendingTransactions = {}, - provider, securityProviderRequest, sign, state, @@ -813,8 +787,6 @@ export class TransactionController extends BaseController< this.isHistoryDisabled = disableHistory ?? false; this.isSwapsDisabled = disableSwaps ?? false; this.#isSimulationEnabled = isSimulationEnabled ?? (() => true); - // @ts-expect-error the type in eth-method-registry is inappropriate and should be changed - this.registry = new MethodRegistry({ provider }); this.getSavedGasFees = getSavedGasFees ?? ((_chainId) => undefined); this.getCurrentAccountEIP1559Compatibility = getCurrentAccountEIP1559Compatibility ?? (() => Promise.resolve(true)); @@ -844,11 +816,6 @@ export class TransactionController extends BaseController< this.publish = hooks?.publish ?? (() => Promise.resolve({ transactionHash: undefined })); - this.nonceTracker = this.#createNonceTracker({ - provider, - blockTracker, - }); - const findNetworkClientIdByChainId = (chainId: Hex) => { return this.messagingSystem.call( `NetworkController:findNetworkClientIdByChainId`, @@ -857,9 +824,6 @@ export class TransactionController extends BaseController< }; this.#multichainTrackingHelper = new MultichainTrackingHelper({ - isMultichainEnabled, - provider, - nonceTracker: this.nonceTracker, incomingTransactionOptions: incomingTransactions, findNetworkClientIdByChainId, getNetworkClientById: ((networkClientId: NetworkClientId) => { @@ -887,22 +851,6 @@ export class TransactionController extends BaseController< }); this.#multichainTrackingHelper.initialize(); - const etherscanRemoteTransactionSource = - new EtherscanRemoteTransactionSource({ - apiKeysByChainId: incomingTransactions.etherscanApiKeysByChainId, - includeTokenTransfers: incomingTransactions.includeTokenTransfers, - }); - - this.incomingTransactionHelper = this.#createIncomingTransactionHelper({ - blockTracker, - etherscanRemoteTransactionSource, - }); - - this.pendingTransactionTracker = this.#createPendingTransactionTracker({ - provider, - blockTracker, - }); - this.gasFeeFlows = this.#getGasFeeFlows(); this.layer1GasFeeFlows = this.#getLayer1GasFeeFlows(); @@ -937,13 +885,6 @@ export class TransactionController extends BaseController< this.#checkForPendingTransactionAndStartPolling, ); - // TODO once v2 is merged make sure this only runs when - // selectedNetworkClientId changes - onNetworkStateChange(() => { - log('Detected network change', this.getChainId()); - this.pendingTransactionTracker.startIfPendingTransactions(); - }); - this.onBootCleanup(); this.#checkForPendingTransactionAndStartPolling(); } @@ -958,27 +899,11 @@ export class TransactionController extends BaseController< /** * Handle new method data request. * - * @param fourBytePrefix - The method prefix. + * @param _fourBytePrefix - The method prefix. * @returns The method data object corresponding to the given signature prefix. */ - async handleMethodData(fourBytePrefix: string): Promise { - const releaseLock = await this.mutex.acquire(); - try { - const { methodData } = this.state; - const knownMethod = Object.keys(methodData).find( - (knownFourBytePrefix) => fourBytePrefix === knownFourBytePrefix, - ); - if (knownMethod) { - return methodData[fourBytePrefix]; - } - const registry = await this.registryLookup(fourBytePrefix); - this.update((state) => { - state.methodData[fourBytePrefix] = registry; - }); - return registry; - } finally { - releaseLock(); - } + async handleMethodData(_fourBytePrefix: string): Promise { + throw new Error('Method not implemented.'); } /** @@ -1009,6 +934,7 @@ export class TransactionController extends BaseController< actionId, deviceConfirmedOn, method, + networkClientId, origin, requireApproval, securityAlertResponse, @@ -1016,11 +942,11 @@ export class TransactionController extends BaseController< swaps = {}, traceContext, type, - networkClientId: requestNetworkClientId, }: { actionId?: string; deviceConfirmedOn?: WalletDevice; method?: string; + networkClientId: NetworkClientId; origin?: string; requireApproval?: boolean | undefined; securityAlertResponse?: SecurityAlertResponse; @@ -1031,24 +957,18 @@ export class TransactionController extends BaseController< }; traceContext?: unknown; type?: TransactionType; - networkClientId?: NetworkClientId; - } = {}, + }, ): Promise { log('Adding transaction', txParams); txParams = normalizeTransactionParams(txParams); - if ( - requestNetworkClientId && - !this.#multichainTrackingHelper.has(requestNetworkClientId) - ) { + + if (!this.#multichainTrackingHelper.has(networkClientId)) { throw new Error( 'The networkClientId for this transaction could not be found', ); } - const networkClientId = - requestNetworkClientId ?? this.#getGlobalNetworkClientId(); - const isEIP1559Compatible = await this.getEIP1559Compatibility( networkClientId, ); @@ -1070,6 +990,7 @@ export class TransactionController extends BaseController< ); const chainId = this.getChainId(networkClientId); + const ethQuery = this.#multichainTrackingHelper.getEthQuery({ networkClientId, chainId, @@ -1171,35 +1092,22 @@ export class TransactionController extends BaseController< } startIncomingTransactionPolling(networkClientIds: NetworkClientId[] = []) { - if (networkClientIds.length === 0) { - this.incomingTransactionHelper.start(); - return; - } this.#multichainTrackingHelper.startIncomingTransactionPolling( networkClientIds, ); } stopIncomingTransactionPolling(networkClientIds: NetworkClientId[] = []) { - if (networkClientIds.length === 0) { - this.incomingTransactionHelper.stop(); - return; - } this.#multichainTrackingHelper.stopIncomingTransactionPolling( networkClientIds, ); } stopAllIncomingTransactionPolling() { - this.incomingTransactionHelper.stop(); this.#multichainTrackingHelper.stopAllIncomingTransactionPolling(); } async updateIncomingTransactions(networkClientIds: NetworkClientId[] = []) { - if (networkClientIds.length === 0) { - await this.incomingTransactionHelper.update(); - return; - } await this.#multichainTrackingHelper.updateIncomingTransactions( networkClientIds, ); @@ -1505,23 +1413,22 @@ export class TransactionController extends BaseController< /** * Removes all transactions from state, optionally based on the current network. * - * @param ignoreNetwork - Determines whether to wipe all transactions, or just those on the - * current network. If `true`, all transactions are wiped. + * @param chainId - Remove transactions for the specified chain only. Defaults to all chains. * @param address - If specified, only transactions originating from this address will be * wiped on current network. */ - wipeTransactions(ignoreNetwork?: boolean, address?: string) { - /* istanbul ignore next */ - if (ignoreNetwork && !address) { + wipeTransactions(chainId?: string, address?: string) { + if (!chainId && !address) { this.update((state) => { state.transactions = []; }); + return; } - const currentChainId = this.getChainId(); + const newTransactions = this.state.transactions.filter( - ({ chainId, txParams }) => { - const isMatchingNetwork = ignoreNetwork || chainId === currentChainId; + ({ chainId: txChainId, txParams }) => { + const isMatchingNetwork = !chainId || chainId === txChainId; if (!isMatchingNetwork) { return true; @@ -2048,26 +1955,22 @@ export class TransactionController extends BaseController< * Search transaction metadata for matching entries. * * @param opts - Options bag. - * @param opts.searchCriteria - An object containing values or functions for transaction properties to filter transactions with. * @param opts.initialList - The transactions to search. Defaults to the current state. - * @param opts.filterToCurrentNetwork - Whether to filter the results to the current network. Defaults to true. * @param opts.limit - The maximum number of transactions to return. No limit by default. + * @param opts.searchCriteria - An object containing values or functions for transaction properties to filter transactions with. * @returns An array of transactions matching the provided options. */ getTransactions({ - searchCriteria = {}, initialList, - filterToCurrentNetwork = true, limit, + searchCriteria = {}, }: { + initialList?: TransactionMeta[]; + limit?: number; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any searchCriteria?: any; - initialList?: TransactionMeta[]; - filterToCurrentNetwork?: boolean; - limit?: number; } = {}): TransactionMeta[] { - const chainId = this.getChainId(); // searchCriteria is an object that might have values that aren't predicate // methods. When providing any other value type (string, number, etc), we // consider this shorthand for "check the value at key for strict equality @@ -2088,9 +1991,6 @@ export class TransactionController extends BaseController< // matching transactions that are sorted by time. const filteredTransactions = sortBy( pickBy(transactionsToFilter, (transaction) => { - if (filterToCurrentNetwork && transaction.chainId !== chainId) { - return false; - } // iterate over the predicateMethods keys to check if the transaction // matches the searchCriteria for (const [key, predicate] of Object.entries(predicateMethods)) { @@ -2310,7 +2210,7 @@ export class TransactionController extends BaseController< const { networkClientId, chainId } = transactionMeta; - const isCustomNetwork = this.#isCustomNetwork(networkClientId); + const isCustomNetwork = this.#isCustomNetwork(networkClientId as string); const ethQuery = this.#multichainTrackingHelper.getEthQuery({ networkClientId, @@ -2879,14 +2779,7 @@ export class TransactionController extends BaseController< return { meta: transaction, isCompleted }; } - private getChainId(networkClientId?: NetworkClientId): Hex { - const globalChainId = this.#getGlobalChainId(); - const globalNetworkClientId = this.#getGlobalNetworkClientId(); - - if (!networkClientId || networkClientId === globalNetworkClientId) { - return globalChainId; - } - + private getChainId(networkClientId: NetworkClientId): Hex { return this.messagingSystem.call( `NetworkController:getNetworkClientById`, networkClientId, @@ -3256,7 +3149,7 @@ export class TransactionController extends BaseController< private getNonceTrackerTransactions( status: TransactionStatus, address: string, - chainId: string = this.getChainId(), + chainId: string, ) { return getAndFormatTransactionsForNonceTracker( chainId, @@ -3322,7 +3215,7 @@ export class TransactionController extends BaseController< }: { provider: Provider; blockTracker: BlockTracker; - chainId?: Hex; + chainId: Hex; }): NonceTracker { return new NonceTracker({ // TODO: Fix types @@ -3337,6 +3230,7 @@ export class TransactionController extends BaseController< getConfirmedTransactions: this.getNonceTrackerTransactions.bind( this, TransactionStatus.confirmed, + chainId, ), }); } @@ -3348,14 +3242,14 @@ export class TransactionController extends BaseController< }: { blockTracker: BlockTracker; etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; - chainId?: Hex; + chainId: Hex; }): IncomingTransactionHelper { const incomingTransactionHelper = new IncomingTransactionHelper({ blockTracker, getCurrentAccount: () => this.#getSelectedAccount(), getLastFetchedBlockNumbers: () => this.state.lastFetchedBlockNumbers, getLocalTransactions: () => this.state.transactions, - getChainId: chainId ? () => chainId : this.getChainId.bind(this), + getChainId: () => chainId, isEnabled: this.#incomingTransactionOptions.isEnabled, queryEntireHistory: this.#incomingTransactionOptions.queryEntireHistory, remoteTransactionSource: etherscanRemoteTransactionSource, @@ -3375,20 +3269,19 @@ export class TransactionController extends BaseController< }: { provider: Provider; blockTracker: BlockTracker; - chainId?: Hex; + chainId: Hex; }): PendingTransactionTracker { const ethQuery = new EthQuery(provider); - const getChainId = chainId ? () => chainId : this.getChainId.bind(this); const pendingTransactionTracker = new PendingTransactionTracker({ blockTracker, - getChainId, + getChainId: () => chainId, getEthQuery: () => ethQuery, getTransactions: () => this.state.transactions, isResubmitEnabled: this.#pendingTransactionOptions.isResubmitEnabled, getGlobalLock: () => this.#multichainTrackingHelper.acquireNonceLockForChainIdKey({ - chainId: getChainId(), + chainId, }), publishTransaction: (_ethQuery, transactionMeta) => this.publishTransaction(_ethQuery, transactionMeta, { @@ -3407,21 +3300,10 @@ export class TransactionController extends BaseController< } #checkForPendingTransactionAndStartPolling = () => { - // PendingTransactionTracker reads state through its getTransactions hook - this.pendingTransactionTracker.startIfPendingTransactions(); this.#multichainTrackingHelper.checkForPendingTransactionAndStartPolling(); }; #stopAllTracking() { - this.pendingTransactionTracker.stop(); - this.#removePendingTransactionTrackerListeners( - this.pendingTransactionTracker, - ); - this.incomingTransactionHelper.stop(); - this.#removeIncomingTransactionHelperListeners( - this.incomingTransactionHelper, - ); - this.#multichainTrackingHelper.stopAllTracking(); } @@ -3480,10 +3362,7 @@ export class TransactionController extends BaseController< ); } - #getNonceTrackerPendingTransactions( - chainId: string | undefined, - address: string, - ) { + #getNonceTrackerPendingTransactions(chainId: string, address: string) { const standardPendingTransactions = this.getNonceTrackerTransactions( TransactionStatus.submitted, address, @@ -3505,9 +3384,6 @@ export class TransactionController extends BaseController< return await this.publishTransaction(ethQuery, transactionMeta); } catch (error: unknown) { if (this.isTransactionAlreadyConfirmedError(error as Error)) { - await this.pendingTransactionTracker.forceCheckTransaction( - transactionMeta, - ); throw new Error('Previous transaction is already confirmed'); } throw error; @@ -3728,49 +3604,27 @@ export class TransactionController extends BaseController< } #getNetworkClientId({ - networkClientId: requestNetworkClientId, chainId, + networkClientId, }: { - networkClientId?: NetworkClientId; chainId?: Hex; + networkClientId?: NetworkClientId; }) { - const globalChainId = this.#getGlobalChainId(); - const globalNetworkClientId = this.#getGlobalNetworkClientId(); - - if (requestNetworkClientId) { - return requestNetworkClientId; + if (!chainId && !networkClientId) { + throw new Error('Must provide either chainId or networkClientId'); } - if (!chainId || chainId === globalChainId) { - return globalNetworkClientId; + if (networkClientId) { + return networkClientId; } return this.messagingSystem.call( `NetworkController:findNetworkClientIdByChainId`, - chainId, + chainId as Hex, ); } - #getGlobalNetworkClientId() { - return this.getNetworkState().selectedNetworkClientId; - } - - #getGlobalChainId() { - return this.messagingSystem.call( - `NetworkController:getNetworkClientById`, - this.getNetworkState().selectedNetworkClientId, - ).configuration.chainId; - } - - #isCustomNetwork(networkClientId?: NetworkClientId) { - const globalNetworkClientId = this.#getGlobalNetworkClientId(); - - if (!networkClientId || networkClientId === globalNetworkClientId) { - return !isInfuraNetworkType( - this.getNetworkState().selectedNetworkClientId, - ); - } - + #isCustomNetwork(networkClientId: NetworkClientId) { return ( this.messagingSystem.call( `NetworkController:getNetworkClientById`, diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 9abb968d87..3438f975f4 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -218,7 +218,6 @@ const setupController = async ( ); const options: TransactionControllerOptions = { - blockTracker, disableHistory: false, disableSendFlowHistory: false, disableSwaps: false, @@ -235,15 +234,10 @@ const setupController = async ( networkController.getNetworkClientRegistry(), getPermittedAccounts: async () => [ACCOUNT_MOCK], hooks: {}, - isMultichainEnabled: false, messenger, - onNetworkStateChange: () => { - // noop - }, pendingTransactions: { isResubmitEnabled: () => false, }, - provider, sign: async (transaction: TypedTransaction) => transaction, transactionHistoryLimit: 40, ...givenOptions, @@ -314,7 +308,6 @@ describe('TransactionController Integration', () => { }); const { transactionController } = await setupController({ - isMultichainEnabled: true, state: { transactions: [ { @@ -419,9 +412,7 @@ describe('TransactionController Integration', () => { buildEthGasPriceRequestMock(), ], }); - const { transactionController } = await setupController({ - isMultichainEnabled: true, - }); + const { transactionController } = await setupController(); await transactionController.addTransaction( { from: ACCOUNT_MOCK, @@ -456,7 +447,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -500,7 +491,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -568,7 +559,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const firstTransaction = await transactionController.addTransaction( { from: ACCOUNT_MOCK, @@ -642,7 +633,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -706,7 +697,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -781,7 +772,7 @@ describe('TransactionController Integration', () => { ], }); const { transactionController, approvalController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const { result, transactionMeta } = await transactionController.addTransaction( { @@ -886,7 +877,6 @@ describe('TransactionController Integration', () => { const { approvalController, networkController, transactionController } = await setupController({ - isMultichainEnabled: true, getPermittedAccounts: async () => [ACCOUNT_MOCK], }); const existingGoerliNetworkConfiguration = @@ -982,7 +972,6 @@ describe('TransactionController Integration', () => { const { approvalController, transactionController } = await setupController( { - isMultichainEnabled: true, getPermittedAccounts: async () => [ACCOUNT_MOCK], }, { selectedAccount: INTERNAL_ACCOUNT_MOCK }, @@ -1054,9 +1043,8 @@ describe('TransactionController Integration', () => { buildEthGasPriceRequestMock(), ], }); - const { networkController, transactionController } = await setupController({ - isMultichainEnabled: true, - }); + const { networkController, transactionController } = + await setupController(); const existingGoerliNetworkConfiguration = networkController.getNetworkConfigurationByChainId(ChainId.goerli); @@ -1127,76 +1115,11 @@ describe('TransactionController Integration', () => { }); describe('feature flag', () => { - it('should not allow transaction to be added with a networkClientId when feature flag is disabled', async () => { - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.mainnet, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthBlockNumberRequestMock('0x2'), - buildEthGetBlockByNumberRequestMock('0x1'), - buildEthGasPriceRequestMock(), - buildEthGetCodeRequestMock(ACCOUNT_2_MOCK), - ], - }); - - const { networkController, transactionController } = - await setupController({ - isMultichainEnabled: false, - }); - - const networkConfiguration = networkController.addNetwork( - buildAddNetworkFields(), - ); - - // add a transaction with the networkClientId of the newly added network - // and expect it to throw since the networkClientId won't be found in the trackingMap - await expect( - transactionController.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - }, - { - networkClientId: - networkConfiguration.rpcEndpoints[0].networkClientId, - }, - ), - ).rejects.toThrow( - 'The networkClientId for this transaction could not be found', - ); - - // adding a transaction without a networkClientId should work - expect( - await transactionController.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - }), - ).toBeDefined(); - transactionController.destroy(); - }); - - it('should not call getNetworkClientRegistry on networkController:stateChange when feature flag is disabled', async () => { - const getNetworkClientRegistrySpy = jest.fn(); - - const { networkController, transactionController } = - await setupController({ - isMultichainEnabled: false, - getNetworkClientRegistry: getNetworkClientRegistrySpy, - }); - - networkController.addNetwork(buildAddNetworkFields()); - - expect(getNetworkClientRegistrySpy).not.toHaveBeenCalled(); - transactionController.destroy(); - }); - it('should call getNetworkClientRegistry on networkController:stateChange when feature flag is enabled', async () => { uuidV4Mock.mockReturnValue('AAAA-AAAA-AAAA-AAAA'); const { networkController, transactionController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const getNetworkClientRegistrySpy = jest.spyOn( networkController, 'getNetworkClientRegistry', @@ -1218,7 +1141,6 @@ describe('TransactionController Integration', () => { }); await setupController({ - isMultichainEnabled: true, getNetworkClientRegistry: getNetworkClientRegistrySpy, }); @@ -1246,12 +1168,7 @@ describe('TransactionController Integration', () => { }); const { networkController, transactionController } = - await setupController( - { - isMultichainEnabled: true, - }, - { selectedAccount: selectedAccountMock }, - ); + await setupController({}, { selectedAccount: selectedAccountMock }); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1315,66 +1232,6 @@ describe('TransactionController Integration', () => { transactionController.destroy(); }); - it('should start the global incoming transaction helper when no networkClientIds provided', async () => { - const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const selectedAccountMock = createMockInternalAccount({ - address: selectedAddress, - }); - - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.mainnet, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthBlockNumberRequestMock('0x2'), - ], - }); - nock(getEtherscanApiHost(BUILT_IN_NETWORKS[NetworkType.mainnet].chainId)) - .get( - `/api?module=account&address=${selectedAddress}&offset=40&sort=desc&action=txlist&tag=latest&page=1`, - ) - .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); - - const { transactionController } = await setupController( - {}, - { selectedAccount: selectedAccountMock }, - ); - - transactionController.startIncomingTransactionPolling(); - - await advanceTime({ clock, duration: BLOCK_TRACKER_POLLING_INTERVAL }); - - expect(transactionController.state.transactions).toHaveLength(2); - expect(transactionController.state.transactions).toStrictEqual( - expect.arrayContaining([ - expect.objectContaining({ - blockNumber: ETHERSCAN_TRANSACTION_BASE_MOCK.blockNumber, - chainId: '0x1', - type: TransactionType.incoming, - verifiedOnBlockchain: false, - status: TransactionStatus.confirmed, - }), - expect.objectContaining({ - blockNumber: ETHERSCAN_TRANSACTION_BASE_MOCK.blockNumber, - chainId: '0x1', - type: TransactionType.incoming, - verifiedOnBlockchain: false, - status: TransactionStatus.failed, - }), - ]), - ); - expect(transactionController.state.lastFetchedBlockNumbers).toStrictEqual( - { - [`0x1#${selectedAddress}#normal`]: parseInt( - ETHERSCAN_TRANSACTION_BASE_MOCK.blockNumber, - 10, - ), - }, - ); - transactionController.destroy(); - }); - describe('when called with multiple networkClients which share the same chainId', () => { it('should only call the etherscan API max every 5 seconds, alternating between the token and txlist endpoints', async () => { const fetchEtherscanNativeTxFetchSpy = jest.spyOn( @@ -1430,12 +1287,7 @@ describe('TransactionController Integration', () => { }); const { networkController, transactionController } = - await setupController( - { - isMultichainEnabled: true, - }, - { selectedAccount: selectedAccountMock }, - ); + await setupController({}, { selectedAccount: selectedAccountMock }); const existingGoerliNetworkConfiguration = networkController.getNetworkConfigurationByChainId(ChainId.goerli); @@ -1678,12 +1530,7 @@ describe('TransactionController Integration', () => { }); const { networkController, transactionController } = - await setupController( - { - isMultichainEnabled: true, - }, - { selectedAccount: selectedAccountMock }, - ); + await setupController({}, { selectedAccount: selectedAccountMock }); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1746,72 +1593,12 @@ describe('TransactionController Integration', () => { ); transactionController.destroy(); }); - - it('should update the incoming transactions for the gloablly selected network when no networkClientIds provided', async () => { - const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const selectedAccountMock = createMockInternalAccount({ - address: selectedAddress, - }); - - const { transactionController } = await setupController( - {}, - { selectedAccount: selectedAccountMock }, - ); - - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.mainnet, - ), - mocks: [buildEthBlockNumberRequestMock('0x1')], - }); - nock(getEtherscanApiHost(BUILT_IN_NETWORKS[NetworkType.mainnet].chainId)) - .get( - `/api?module=account&address=${selectedAddress}&offset=40&sort=desc&action=txlist&tag=latest&page=1`, - ) - .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); - - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - transactionController.updateIncomingTransactions(); - - // we have to wait for the mutex to be released after the 5 second API rate limit timer - await advanceTime({ clock, duration: 1 }); - - expect(transactionController.state.transactions).toHaveLength(2); - expect(transactionController.state.transactions).toStrictEqual( - expect.arrayContaining([ - expect.objectContaining({ - blockNumber: ETHERSCAN_TRANSACTION_BASE_MOCK.blockNumber, - chainId: '0x1', - type: TransactionType.incoming, - verifiedOnBlockchain: false, - status: TransactionStatus.confirmed, - }), - expect.objectContaining({ - blockNumber: ETHERSCAN_TRANSACTION_BASE_MOCK.blockNumber, - chainId: '0x1', - type: TransactionType.incoming, - verifiedOnBlockchain: false, - status: TransactionStatus.failed, - }), - ]), - ); - expect(transactionController.state.lastFetchedBlockNumbers).toStrictEqual( - { - [`0x1#${selectedAddress}#normal`]: parseInt( - ETHERSCAN_TRANSACTION_BASE_MOCK.blockNumber, - 10, - ), - }, - ); - transactionController.destroy(); - }); }); describe('getNonceLock', () => { it('should get the nonce lock from the nonceTracker for the given networkClientId', async () => { const { networkController, transactionController } = - await setupController({ isMultichainEnabled: true }); + await setupController(); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1846,7 +1633,7 @@ 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 setupController({ isMultichainEnabled: true }); + await setupController(); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1910,7 +1697,7 @@ 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 setupController({ isMultichainEnabled: true }); + await setupController(); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( InfuraNetworkType.goerli, @@ -2001,9 +1788,7 @@ 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 setupController({ - isMultichainEnabled: true, - }); + const { transactionController } = await setupController(); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -2050,7 +1835,7 @@ 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 setupController({ isMultichainEnabled: true }); + await setupController(); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -2097,109 +1882,5 @@ describe('TransactionController Integration', () => { ); transactionController.destroy(); }); - - it('should get the nonce lock from the globally selected nonceTracker if no networkClientId is provided', async () => { - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.mainnet, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthGetTransactionCountRequestMock(ACCOUNT_MOCK, '0x1', '0xa'), - ], - }); - - const { transactionController } = await setupController({}); - - const nonceLockPromise = transactionController.getNonceLock(ACCOUNT_MOCK); - await advanceTime({ clock, duration: 1 }); - - const nonceLock = await nonceLockPromise; - - expect(nonceLock.nextNonce).toBe(10); - transactionController.destroy(); - }); - - it('should block attempts to get the nonce lock from the globally selected NonceTracker for the same address until the previous lock is released', async () => { - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.mainnet, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthGetTransactionCountRequestMock(ACCOUNT_MOCK, '0x1', '0xa'), - ], - }); - - const { transactionController } = await setupController({}); - - const firstNonceLockPromise = - transactionController.getNonceLock(ACCOUNT_MOCK); - await advanceTime({ clock, duration: 1 }); - - const firstNonceLock = await firstNonceLockPromise; - - expect(firstNonceLock.nextNonce).toBe(10); - - const secondNonceLockPromise = - transactionController.getNonceLock(ACCOUNT_MOCK); - const delay = () => - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/no-misused-promises - new Promise(async (resolve) => { - await advanceTime({ clock, duration: 100 }); - resolve(null); - }); - - let secondNonceLockIfAcquired = await Promise.race([ - secondNonceLockPromise, - delay(), - ]); - expect(secondNonceLockIfAcquired).toBeNull(); - - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - await firstNonceLock.releaseLock(); - - secondNonceLockIfAcquired = await Promise.race([ - secondNonceLockPromise, - delay(), - ]); - expect(secondNonceLockIfAcquired?.nextNonce).toBe(10); - transactionController.destroy(); - }); - - it('should not block attempts to get the nonce lock from the globally selected nonceTracker for different addresses', async () => { - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.mainnet, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthGetTransactionCountRequestMock(ACCOUNT_MOCK, '0x1', '0xa'), - buildEthGetTransactionCountRequestMock(ACCOUNT_2_MOCK, '0x1', '0xf'), - ], - }); - - const { transactionController } = await setupController({}); - - const firstNonceLockPromise = - transactionController.getNonceLock(ACCOUNT_MOCK); - await advanceTime({ clock, duration: 1 }); - - const firstNonceLock = await firstNonceLockPromise; - - expect(firstNonceLock.nextNonce).toBe(10); - - const secondNonceLockPromise = - transactionController.getNonceLock(ACCOUNT_2_MOCK); - await advanceTime({ clock, duration: 1 }); - - const secondNonceLock = await secondNonceLockPromise; - - expect(secondNonceLock.nextNonce).toBe(15); - - transactionController.destroy(); - }); }); }); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts index 2b5fa6c546..13b2401768 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts @@ -264,21 +264,6 @@ describe('MultichainTrackingHelper', () => { expect(helper.has('sepolia')).toBe(true); expect(helper.has('customNetworkClientId-1')).toBe(true); }); - - it('does not refresh the tracking map when isMultichainEnabled: false', () => { - const { options, helper } = newMultichainTrackingHelper({ - isMultichainEnabled: false, - }); - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (options.onNetworkStateChange as any).mock.calls[0][0]({}, []); - - expect(options.getNetworkClientRegistry).not.toHaveBeenCalled(); - expect(helper.has('mainnet')).toBe(false); - expect(helper.has('goerli')).toBe(false); - expect(helper.has('sepolia')).toBe(false); - expect(helper.has('customNetworkClientId-1')).toBe(false); - }); }); describe('initialize', () => { @@ -293,20 +278,6 @@ describe('MultichainTrackingHelper', () => { expect(helper.has('sepolia')).toBe(true); expect(helper.has('customNetworkClientId-1')).toBe(true); }); - - it('does not initialize the tracking map when isMultichainEnabled: false', () => { - const { options, helper } = newMultichainTrackingHelper({ - isMultichainEnabled: false, - }); - - helper.initialize(); - - expect(options.getNetworkClientRegistry).not.toHaveBeenCalled(); - expect(helper.has('mainnet')).toBe(false); - expect(helper.has('goerli')).toBe(false); - expect(helper.has('sepolia')).toBe(false); - expect(helper.has('customNetworkClientId-1')).toBe(false); - }); }); describe('stopAllTracking', () => { @@ -580,106 +551,6 @@ describe('MultichainTrackingHelper', () => { expect(releaseLockForChainIdKey).toHaveBeenCalled(); }); }); - - describe('when no networkClientId given', () => { - it('does not get the shared nonce lock by chainId', async () => { - const { helper } = newMultichainTrackingHelper(); - - const mockAcquireNonceLockForChainIdKey = jest - .spyOn(helper, 'acquireNonceLockForChainIdKey') - .mockResolvedValue(jest.fn()); - - helper.initialize(); - - await helper.getNonceLock('0xdeadbeef'); - - expect(mockAcquireNonceLockForChainIdKey).not.toHaveBeenCalled(); - }); - - it('gets the nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({}); - - helper.initialize(); - - await helper.getNonceLock('0xdeadbeef'); - - expect(options.nonceTracker.getNonceLock).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); - - it('throws an error if unable to acquire nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({}); - - helper.initialize(); - - options.nonceTracker.getNonceLock.mockRejectedValue( - 'failed to acquire lock from nonceTracker', - ); - - // for some reason jest expect().rejects.toThrow doesn't work here - let error = ''; - try { - await helper.getNonceLock('0xdeadbeef'); - } catch (err: unknown) { - error = err as string; - } - expect(error).toBe('failed to acquire lock from nonceTracker'); - }); - }); - - describe('when passed a networkClientId and isMultichainEnabled: false', () => { - it('does not get the shared nonce lock by chainId', async () => { - const { helper } = newMultichainTrackingHelper({ - isMultichainEnabled: false, - }); - - const mockAcquireNonceLockForChainIdKey = jest - .spyOn(helper, 'acquireNonceLockForChainIdKey') - .mockResolvedValue(jest.fn()); - - helper.initialize(); - - await helper.getNonceLock('0xdeadbeef', '0xabc'); - - expect(mockAcquireNonceLockForChainIdKey).not.toHaveBeenCalled(); - }); - - it('gets the nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({ - isMultichainEnabled: false, - }); - - helper.initialize(); - - await helper.getNonceLock('0xdeadbeef', '0xabc'); - - expect(options.nonceTracker.getNonceLock).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); - - it('throws an error if unable to acquire nonce lock from the global NonceTracker', async () => { - const { options, helper } = newMultichainTrackingHelper({ - isMultichainEnabled: false, - }); - - helper.initialize(); - - options.nonceTracker.getNonceLock.mockRejectedValue( - 'failed to acquire lock from nonceTracker', - ); - - // for some reason jest expect().rejects.toThrow doesn't work here - let error = ''; - try { - await helper.getNonceLock('0xdeadbeef', '0xabc'); - } catch (err: unknown) { - error = err as string; - } - expect(error).toBe('failed to acquire lock from nonceTracker'); - }); - }); }); describe('acquireNonceLockForChainIdKey', () => { @@ -779,24 +650,6 @@ describe('MultichainTrackingHelper', () => { 'customNetworkClientId-1', ); }); - - it('returns EthQuery with the fallback global provider if networkClientId and chainId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ - networkClientId: 'missingNetworkClientId', - chainId: '0xdeadbeef', - }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'missingNetworkClientId', - ); - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); }); describe('when given only networkClientId', () => { @@ -809,20 +662,6 @@ describe('MultichainTrackingHelper', () => { expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); expect(options.getNetworkClientById).toHaveBeenCalledWith('goerli'); }); - - it('returns EthQuery with the fallback global provider if networkClientId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ - networkClientId: 'missingNetworkClientId', - }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'missingNetworkClientId', - ); - }); }); describe('when given only chainId', () => { @@ -842,46 +681,6 @@ describe('MultichainTrackingHelper', () => { 'customNetworkClientId-1', ); }); - - it('returns EthQuery with the fallback global provider if chainId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ chainId: '0xdeadbeef' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); - }); - - it('returns EthQuery with the global provider when no arguments are provided', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery(); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).not.toHaveBeenCalled(); - }); - - it('always returns EthQuery with the global provider when isMultichainEnabled: false', () => { - const { options, helper } = newMultichainTrackingHelper({ - isMultichainEnabled: false, - }); - - let ethQuery = helper.getEthQuery({ - networkClientId: 'goerli', - chainId: '0x5', - }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - ethQuery = helper.getEthQuery({ networkClientId: 'goerli' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - ethQuery = helper.getEthQuery({ chainId: '0x5' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - ethQuery = helper.getEthQuery(); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).not.toHaveBeenCalled(); }); }); @@ -920,24 +719,6 @@ describe('MultichainTrackingHelper', () => { 'customNetworkClientId-1', ); }); - - it('returns the fallback global provider if networkClientId and chainId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const provider = helper.getProvider({ - networkClientId: 'missingNetworkClientId', - chainId: '0xdeadbeef', - }); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'missingNetworkClientId', - ); - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); }); describe('when given only networkClientId', () => { @@ -950,20 +731,6 @@ describe('MultichainTrackingHelper', () => { expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); expect(options.getNetworkClientById).toHaveBeenCalledWith('goerli'); }); - - it('returns the fallback global provider if networkClientId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const provider = helper.getProvider({ - networkClientId: 'missingNetworkClientId', - }); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'missingNetworkClientId', - ); - }); }); describe('when given only chainId', () => { @@ -981,46 +748,6 @@ describe('MultichainTrackingHelper', () => { 'customNetworkClientId-1', ); }); - - it('returns the fallback global provider if chainId cannot be satisfied', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const provider = helper.getProvider({ chainId: '0xdeadbeef' }); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xdeadbeef', - ); - }); - }); - - it('returns the global provider when no arguments are provided', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const provider = helper.getProvider(); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).not.toHaveBeenCalled(); - }); - - it('always returns the global provider when isMultichainEnabled: false', () => { - const { options, helper } = newMultichainTrackingHelper({ - isMultichainEnabled: false, - }); - - let provider = helper.getProvider({ - networkClientId: 'goerli', - chainId: '0x5', - }); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - provider = helper.getProvider({ networkClientId: 'goerli' }); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - provider = helper.getProvider({ chainId: '0x5' }); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - provider = helper.getProvider(); - expect(provider).toBe(MOCK_PROVIDERS.mainnet); - - expect(options.getNetworkClientById).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts index dcc6aded73..9b6f570e71 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts @@ -27,9 +27,6 @@ type NetworkClientRegistry = ReturnType< >; export type MultichainTrackingHelperOptions = { - isMultichainEnabled: boolean; - provider: Provider; - nonceTracker: NonceTracker; incomingTransactionOptions: IncomingTransactionOptions; findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; @@ -45,17 +42,17 @@ export type MultichainTrackingHelperOptions = { createNonceTracker: (opts: { provider: Provider; blockTracker: BlockTracker; - chainId?: Hex; + chainId: Hex; }) => NonceTracker; createIncomingTransactionHelper: (opts: { blockTracker: BlockTracker; etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; - chainId?: Hex; + chainId: Hex; }) => IncomingTransactionHelper; createPendingTransactionTracker: (opts: { provider: Provider; blockTracker: BlockTracker; - chainId?: Hex; + chainId: Hex; }) => PendingTransactionTracker; onNetworkStateChange: ( listener: ( @@ -65,12 +62,6 @@ export type MultichainTrackingHelperOptions = { }; export class MultichainTrackingHelper { - #isMultichainEnabled: boolean; - - readonly #provider: Provider; - - readonly #nonceTracker: NonceTracker; - readonly #incomingTransactionOptions: IncomingTransactionOptions; readonly #findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; @@ -90,19 +81,19 @@ export class MultichainTrackingHelper { readonly #createNonceTracker: (opts: { provider: Provider; blockTracker: BlockTracker; - chainId?: Hex; + chainId: Hex; }) => NonceTracker; readonly #createIncomingTransactionHelper: (opts: { blockTracker: BlockTracker; - chainId?: Hex; + chainId: Hex; etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; }) => IncomingTransactionHelper; readonly #createPendingTransactionTracker: (opts: { provider: Provider; blockTracker: BlockTracker; - chainId?: Hex; + chainId: Hex; }) => PendingTransactionTracker; readonly #nonceMutexesByChainId = new Map>(); @@ -122,9 +113,6 @@ export class MultichainTrackingHelper { > = new Map(); constructor({ - isMultichainEnabled, - provider, - nonceTracker, incomingTransactionOptions, findNetworkClientIdByChainId, getNetworkClientById, @@ -136,9 +124,6 @@ export class MultichainTrackingHelper { createPendingTransactionTracker, onNetworkStateChange, }: MultichainTrackingHelperOptions) { - this.#isMultichainEnabled = isMultichainEnabled; - this.#provider = provider; - this.#nonceTracker = nonceTracker; this.#incomingTransactionOptions = incomingTransactionOptions; this.#findNetworkClientIdByChainId = findNetworkClientIdByChainId; @@ -154,24 +139,19 @@ export class MultichainTrackingHelper { this.#createPendingTransactionTracker = createPendingTransactionTracker; onNetworkStateChange((_, patches) => { - if (this.#isMultichainEnabled) { - const networkClients = this.#getNetworkClientRegistry(); - patches.forEach(({ op, path }) => { - if (op === 'remove' && path[0] === 'networkConfigurations') { - const networkClientId = path[1] as NetworkClientId; - delete networkClients[networkClientId]; - } - }); - - this.#refreshTrackingMap(networkClients); - } + const networkClients = this.#getNetworkClientRegistry(); + patches.forEach(({ op, path }) => { + if (op === 'remove' && path[0] === 'networkConfigurations') { + const networkClientId = path[1] as NetworkClientId; + delete networkClients[networkClientId]; + } + }); + + this.#refreshTrackingMap(networkClients); }); } initialize() { - if (!this.#isMultichainEnabled) { - return; - } const networkClients = this.#getNetworkClientRegistry(); this.#refreshTrackingMap(networkClients); } @@ -187,9 +167,6 @@ export class MultichainTrackingHelper { networkClientId?: NetworkClientId; chainId?: Hex; } = {}): EthQuery { - if (!this.#isMultichainEnabled) { - return new EthQuery(this.getProvider()); - } return new EthQuery(this.getProvider({ networkClientId, chainId })); } @@ -200,16 +177,16 @@ export class MultichainTrackingHelper { networkClientId?: NetworkClientId; chainId?: Hex; } = {}): Provider { - if (!this.#isMultichainEnabled) { - return this.#provider; - } - const networkClient = this.#getNetworkClient({ networkClientId, chainId, }); - return networkClient?.provider || this.#provider; + if (!networkClient) { + throw new Error('missing networkClient'); + } + + return networkClient?.provider; } /** @@ -254,8 +231,8 @@ export class MultichainTrackingHelper { networkClientId?: NetworkClientId, ): Promise { let releaseLockForChainIdKey: (() => void) | undefined; - let nonceTracker = this.#nonceTracker; - if (networkClientId && this.#isMultichainEnabled) { + let nonceTracker; + if (networkClientId) { const networkClient = this.#getNetworkClientById(networkClientId); releaseLockForChainIdKey = await this.acquireNonceLockForChainIdKey({ chainId: networkClient.configuration.chainId, @@ -268,6 +245,10 @@ export class MultichainTrackingHelper { nonceTracker = trackers.nonceTracker; } + if (!nonceTracker) { + throw new Error('missing nonceTracker'); + } + // Acquires the lock for the chainId + address and the nonceLock from the nonceTracker, then // couples them together by replacing the nonceLock's releaseLock method with // an anonymous function that calls releases both the original nonceLock and the From 56927ecd4b3d37ea749f190b1fda5b035010fac4 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 13 Nov 2024 13:28:45 +0000 Subject: [PATCH 02/12] Make networkClientId required in transaction metadata Fix logging in multichain helper. Change wipeTransactions arguments to object. --- .../src/TransactionController.test.ts | 118 +++++--- .../src/TransactionController.ts | 261 +++++++++--------- .../TransactionControllerIntegration.test.ts | 2 +- .../src/gas-flows/DefaultGasFeeFlow.test.ts | 1 + .../src/gas-flows/LineaGasFeeFlow.test.ts | 1 + .../OptimismLayer1GasFeeFlow.test.ts | 1 + .../gas-flows/OracleLayer1GasFeeFlow.test.ts | 1 + .../gas-flows/ScrollLayer1GasFeeFlow.test.ts | 1 + .../EtherscanRemoteTransactionSource.ts | 1 + .../src/helpers/GasFeePoller.test.ts | 22 -- .../src/helpers/GasFeePoller.ts | 12 +- .../helpers/MultichainTrackingHelper.test.ts | 97 +------ .../src/helpers/MultichainTrackingHelper.ts | 158 +++++------ packages/transaction-controller/src/types.ts | 2 +- .../src/utils/gas-flow.test.ts | 1 + .../src/utils/history.test.ts | 1 + .../src/utils/layer1-gas-fee-flow.test.ts | 3 + .../src/utils/nonce.test.ts | 6 + .../src/utils/resimulate.test.ts | 2 + .../tests/EtherscanMocks.ts | 1 + .../src/utils/transaction.ts | 1 + 21 files changed, 330 insertions(+), 363 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 2ce3fdca4f..d704409581 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -829,11 +829,14 @@ describe('TransactionController', () => { multichainTrackingHelperClassMock.mockImplementation(() => { multichainTrackingHelperMock = { - getEthQuery: jest.fn().mockImplementation(() => { - return new EthQuery(new FakeProvider()); - }), - getProvider: jest.fn().mockImplementation(() => { - return new FakeProvider(); + getNetworkClient: jest.fn().mockImplementation(() => { + return { + configuration: { + chainId: CHAIN_ID_MOCK, + }, + id: NETWORK_CLIENT_ID_MOCK, + provider: new FakeProvider(), + } as unknown as NetworkClientConfiguration; }), checkForPendingTransactionAndStartPolling: jest.fn(), getNonceLock: getNonceLockSpy, @@ -1066,10 +1069,13 @@ describe('TransactionController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); - const { gas, simulationFails } = await controller.estimateGas({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const { gas, simulationFails } = await controller.estimateGas( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + NETWORK_CLIENT_ID_MOCK, + ); expect(gas).toBe(gasMock); expect(simulationFails).toBe(simulationFailsMock); @@ -1109,6 +1115,7 @@ describe('TransactionController', () => { const { gas, simulationFails } = await controller.estimateGasBuffered( transactionParamsMock, multiplierMock, + NETWORK_CLIENT_ID_MOCK, ); expect(estimateGasMock).toHaveBeenCalledTimes(1); @@ -1418,7 +1425,7 @@ describe('TransactionController', () => { expect(transactionMeta.txParams.from).toStrictEqual( sepoliaTxParams.from, ); - expect(transactionMeta.chainId).toStrictEqual(sepoliaTxParams.chainId); + expect(transactionMeta.chainId).toStrictEqual(CHAIN_ID_MOCK); expect(transactionMeta.networkClientId).toBe('sepolia'); expect(transactionMeta.origin).toBe('metamask'); }); @@ -1458,7 +1465,7 @@ describe('TransactionController', () => { expect(submittedEventListener).toHaveBeenCalledTimes(1); expect(txParams.from).toBe(ACCOUNT_MOCK); expect(networkClientId).toBe('sepolia'); - expect(chainId).toBe(ChainId.sepolia); + expect(chainId).toBe(CHAIN_ID_MOCK); expect(status).toBe(TransactionStatus.submitted); }); }); @@ -1606,7 +1613,7 @@ describe('TransactionController', () => { ); expect(controller.state.transactions[0].txParams.from).toBe(ACCOUNT_MOCK); - expect(controller.state.transactions[0].chainId).toBe('0x1337'); + expect(controller.state.transactions[0].chainId).toBe(CHAIN_ID_MOCK); expect(controller.state.transactions[0].status).toBe( TransactionStatus.unapproved, ); @@ -1795,7 +1802,7 @@ describe('TransactionController', () => { expect(updateGasMock).toHaveBeenCalledWith({ ethQuery: expect.any(Object), chainId: CHAIN_ID_MOCK, - isCustomNetwork: true, + isCustomNetwork: false, txMeta: expect.any(Object), }); }); @@ -2128,25 +2135,6 @@ describe('TransactionController', () => { await expectTransactionToFail(controller, 'No sign method defined'); }); - it('if no chainId defined', async () => { - const { controller } = setupController({ - mockNetworkClientConfigurationsByNetworkClientId: { - [NETWORK_CLIENT_ID_MOCK]: buildCustomNetworkClientConfiguration({ - rpcUrl: 'https://test.network', - ticker: 'TEST', - chainId: undefined, - }), - }, - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - }); - - await expectTransactionToFail(controller, 'No chainId defined'); - }); - it('if unexpected status', async () => { const { controller } = setupController({ messengerOptions: { @@ -2470,6 +2458,7 @@ describe('TransactionController', () => { { id: '1', chainId: mockCurrentChainId, + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed as const, time: 123456789, txParams: { @@ -2479,6 +2468,7 @@ describe('TransactionController', () => { { id: '2', chainId: mockCurrentChainId, + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed as const, time: 987654321, txParams: { @@ -2490,7 +2480,7 @@ describe('TransactionController', () => { }, }); - controller.wipeTransactions(undefined, mockFromAccount2); + controller.wipeTransactions({ address: mockFromAccount2 }); expect(controller.state.transactions).toHaveLength(1); expect(controller.state.transactions[0].id).toBe('1'); @@ -2507,6 +2497,7 @@ describe('TransactionController', () => { { id: '1', chainId: mockCurrentChainId, + networkClientId: NETWORK_CLIENT_ID_MOCK, txParams: { from: mockFromAccount1, }, @@ -2516,6 +2507,7 @@ describe('TransactionController', () => { { id: '4', chainId: mockDifferentChainId, + networkClientId: NETWORK_CLIENT_ID_MOCK, txParams: { from: mockFromAccount1, }, @@ -2527,7 +2519,10 @@ describe('TransactionController', () => { }, }); - controller.wipeTransactions(mockCurrentChainId, mockFromAccount1); + controller.wipeTransactions({ + chainId: mockCurrentChainId, + address: mockFromAccount1, + }); expect(controller.state.transactions).toHaveLength(1); expect(controller.state.transactions[0].id).toBe('4'); @@ -2629,6 +2624,7 @@ describe('TransactionController', () => { actionId: mockActionId, id: '2', chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, type: TransactionType.cancel, time: 123456789, @@ -2656,6 +2652,7 @@ describe('TransactionController', () => { { id: '2', chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, type: TransactionType.cancel, time: 123456789, @@ -2693,6 +2690,7 @@ describe('TransactionController', () => { { id: '2', chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, type: TransactionType.cancel, time: 123456789, @@ -2739,6 +2737,7 @@ describe('TransactionController', () => { { id: simpleSendTransactionId, chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, type: TransactionType.simpleSend, time: 123456789, @@ -2783,6 +2782,7 @@ describe('TransactionController', () => { { id: simpleSendTransactionId, chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, type: TransactionType.simpleSend, time: 123456789, @@ -3014,6 +3014,7 @@ describe('TransactionController', () => { { actionId: mockActionId, id: '2', + networkClientId: NETWORK_CLIENT_ID_MOCK, chainId: toHex(5), status: TransactionStatus.submitted, type: TransactionType.retry, @@ -3042,6 +3043,7 @@ describe('TransactionController', () => { { id: '2', chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, type: TransactionType.retry, time: 123456789, @@ -3079,6 +3081,7 @@ describe('TransactionController', () => { { id: '2', chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, type: TransactionType.retry, time: 123456789, @@ -3437,6 +3440,7 @@ describe('TransactionController', () => { const externalTransactionToConfirm = { id: '1', chainId: toHex(1), + networkClientId: NETWORK_CLIENT_ID_MOCK, time: 123456789, status: TransactionStatus.confirmed as const, txParams: { @@ -3475,6 +3479,7 @@ describe('TransactionController', () => { to: ACCOUNT_2_MOCK, id: '1', chainId: toHex(1), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed as const, time: 123456789, txParams: { @@ -3500,6 +3505,7 @@ describe('TransactionController', () => { chainId: '0x1', from: ACCOUNT_MOCK, id: '1', + networkClientId: NETWORK_CLIENT_ID_MOCK, time: 123456789, status: TransactionStatus.confirmed as const, to: ACCOUNT_2_MOCK, @@ -3549,6 +3555,7 @@ describe('TransactionController', () => { hash: externalTransactionHash, id: externalTransactionId, chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed as const, time: 123456789, txParams: { @@ -3577,6 +3584,7 @@ describe('TransactionController', () => { { id: localTransactionIdWithSameNonce, chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved as const, time: 123456789, txParams: { @@ -3640,6 +3648,7 @@ describe('TransactionController', () => { to: ACCOUNT_2_MOCK, hash: externalTransactionHash, id: externalTransactionId, + networkClientId: NETWORK_CLIENT_ID_MOCK, chainId: toHex(5), status: TransactionStatus.confirmed as const, time: 123456789, @@ -3663,6 +3672,7 @@ describe('TransactionController', () => { // Off-chain failed local transaction with the same chainId and nonce id: localTransactionIdWithSameNonce, chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.failed as const, error: new Error('mock error'), time: 123456789, @@ -3964,6 +3974,7 @@ describe('TransactionController', () => { { id: 'foo', chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, hash: '1337', status: TransactionStatus.submitted as const, time: 123456789, @@ -4003,6 +4014,7 @@ describe('TransactionController', () => { const transactionMeta = { chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved as const, time: 123456789, txParams: { @@ -4187,6 +4199,7 @@ describe('TransactionController', () => { status, error: new Error('mock error'), chainId: '0x1', + networkClientId: NETWORK_CLIENT_ID_MOCK, time: 123456789, txParams: {} as TransactionParams, }, @@ -4212,6 +4225,7 @@ describe('TransactionController', () => { { id: transactionId, chainId: '0x1', + networkClientId: NETWORK_CLIENT_ID_MOCK, time: 123456789, status: TransactionStatus.unapproved as const, history: [ @@ -4278,6 +4292,7 @@ describe('TransactionController', () => { { id: transactionId, chainId: '0x1', + networkClientId: NETWORK_CLIENT_ID_MOCK, time: 123456789, status: TransactionStatus.unapproved as const, history: [ @@ -4841,7 +4856,10 @@ describe('TransactionController', () => { mockTransactionParam2, ]); - expect(getNonceLockSpy).toHaveBeenCalledWith(ACCOUNT_MOCK, 'goerli'); + expect(getNonceLockSpy).toHaveBeenCalledWith( + ACCOUNT_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); }); }); @@ -4854,6 +4872,7 @@ describe('TransactionController', () => { const metadataMock: TransactionMeta = { txParams: paramsMock, chainId: '0x1' as const, + networkClientId: NETWORK_CLIENT_ID_MOCK, id: '1', time: 0, status: TransactionStatus.approved, @@ -5196,6 +5215,7 @@ describe('TransactionController', () => { baseTransaction = { id: transactionId, chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: statusMock, time: 123456789, txParams: { @@ -5388,6 +5408,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1' }, @@ -5395,6 +5416,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId2', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 2, txParams: { from: '0x2' }, @@ -5402,6 +5424,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 1, txParams: { from: '0x3' }, @@ -5427,6 +5450,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1' }, @@ -5434,6 +5458,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId2', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 2, txParams: { from: '0x2' }, @@ -5441,6 +5466,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 3, txParams: { from: '0x1' }, @@ -5466,6 +5492,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1' }, @@ -5473,6 +5500,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId2', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 2, txParams: { from: '0x2' }, @@ -5480,6 +5508,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 1, txParams: { from: '0x1' }, @@ -5505,6 +5534,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1' }, @@ -5512,6 +5542,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId2', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 2, txParams: { from: '0x2' }, @@ -5519,6 +5550,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 1, txParams: { from: '0x3' }, @@ -5546,6 +5578,7 @@ describe('TransactionController', () => { { chainId: MOCK_NETWORK.chainId, id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1' }, @@ -5553,6 +5586,7 @@ describe('TransactionController', () => { { chainId: '0x2', id: 'testId2', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 2, txParams: { from: '0x2' }, @@ -5560,6 +5594,7 @@ describe('TransactionController', () => { { chainId: MOCK_NETWORK.chainId, id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 1, txParams: { from: '0x3' }, @@ -5587,6 +5622,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1' }, @@ -5594,6 +5630,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId2', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 2, txParams: { from: '0x2' }, @@ -5601,6 +5638,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 1, txParams: { from: '0x3' }, @@ -5620,6 +5658,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1', nonce: '0x1' }, @@ -5627,6 +5666,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId2', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 2, txParams: { from: '0x1', nonce: '0x2' }, @@ -5634,6 +5674,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 3, txParams: { from: '0x2', nonce: '0x3' }, @@ -5641,6 +5682,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId4', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 4, txParams: { from: '0x1', nonce: '0x4' }, @@ -5667,6 +5709,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId1', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.confirmed, time: 1, txParams: { from: '0x1', nonce: '0x1' }, @@ -5674,7 +5717,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId2', - + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved, time: 2, txParams: { from: '0x2', nonce: '0x2' }, @@ -5682,6 +5725,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId3', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 3, txParams: { from: '0x1', nonce: '0x1' }, @@ -5689,6 +5733,7 @@ describe('TransactionController', () => { { chainId: '0x1', id: 'testId4', + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.submitted, time: 4, txParams: { from: '0x1', nonce: '0x3' }, @@ -5725,6 +5770,7 @@ describe('TransactionController', () => { const baseTransaction = { id: transactionId, chainId: toHex(5), + networkClientId: NETWORK_CLIENT_ID_MOCK, status: TransactionStatus.unapproved as const, time: 123456789, txParams: { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index f16cb8102f..59890a0f5d 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -858,11 +858,7 @@ export class TransactionController extends BaseController< findNetworkClientIdByChainId, gasFeeFlows: this.gasFeeFlows, getGasFeeControllerEstimates: this.getGasFeeEstimates, - getProvider: (chainId, networkClientId) => - this.#multichainTrackingHelper.getProvider({ - networkClientId, - chainId, - }), + getProvider: (networkClientId) => this.#getProvider({ networkClientId }), getTransactions: () => this.state.transactions, layer1GasFeeFlows: this.layer1GasFeeFlows, onStateChange: (listener) => { @@ -912,37 +908,25 @@ export class TransactionController extends BaseController< * if not provided. If A `:unapproved` hub event will be emitted once added. * * @param txParams - Standard parameters for an Ethereum transaction. - * @param opts - Additional options to control how the transaction is added. - * @param opts.actionId - Unique ID to prevent duplicate requests. - * @param opts.deviceConfirmedOn - An enum to indicate what device confirmed the transaction. - * @param opts.method - RPC method that requested the transaction. - * @param opts.origin - The origin of the transaction request, such as a dApp hostname. - * @param opts.requireApproval - Whether the transaction requires approval by the user, defaults to true unless explicitly disabled. - * @param opts.securityAlertResponse - Response from security validator. - * @param opts.sendFlowHistory - The sendFlowHistory entries to add. - * @param opts.type - Type of transaction to add, such as 'cancel' or 'swap'. - * @param opts.swaps - Options for swaps transactions. - * @param opts.swaps.hasApproveTx - Whether the transaction has an approval transaction. - * @param opts.swaps.meta - Metadata for swap transaction. - * @param opts.networkClientId - The id of the network client for this transaction. - * @param opts.traceContext - The parent context for any new traces. + * @param options - Additional options to control how the transaction is added. + * @param options.actionId - Unique ID to prevent duplicate requests. + * @param options.deviceConfirmedOn - An enum to indicate what device confirmed the transaction. + * @param options.method - RPC method that requested the transaction. + * @param options.origin - The origin of the transaction request, such as a dApp hostname. + * @param options.requireApproval - Whether the transaction requires approval by the user, defaults to true unless explicitly disabled. + * @param options.securityAlertResponse - Response from security validator. + * @param options.sendFlowHistory - The sendFlowHistory entries to add. + * @param options.type - Type of transaction to add, such as 'cancel' or 'swap'. + * @param options.swaps - Options for swaps transactions. + * @param options.swaps.hasApproveTx - Whether the transaction has an approval transaction. + * @param options.swaps.meta - Metadata for swap transaction. + * @param options.networkClientId - The id of the network client for this transaction. + * @param options.traceContext - The parent context for any new traces. * @returns Object containing a promise resolving to the transaction hash if approved. */ async addTransaction( txParams: TransactionParams, - { - actionId, - deviceConfirmedOn, - method, - networkClientId, - origin, - requireApproval, - securityAlertResponse, - sendFlowHistory, - swaps = {}, - traceContext, - type, - }: { + options: { actionId?: string; deviceConfirmedOn?: WalletDevice; method?: string; @@ -959,14 +943,26 @@ export class TransactionController extends BaseController< type?: TransactionType; }, ): Promise { - log('Adding transaction', txParams); + log('Adding transaction', txParams, options); + + const { + actionId, + deviceConfirmedOn, + method, + networkClientId, + origin, + requireApproval, + securityAlertResponse, + sendFlowHistory, + swaps = {}, + traceContext, + type, + } = options; txParams = normalizeTransactionParams(txParams); if (!this.#multichainTrackingHelper.has(networkClientId)) { - throw new Error( - 'The networkClientId for this transaction could not be found', - ); + throw new Error(`Network client not found - ${networkClientId}`); } const isEIP1559Compatible = await this.getEIP1559Compatibility( @@ -989,11 +985,10 @@ export class TransactionController extends BaseController< origin, ); - const chainId = this.getChainId(networkClientId); + const chainId = this.#getChainId(networkClientId); - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQuery({ networkClientId, - chainId, }); const transactionType = @@ -1274,10 +1269,8 @@ export class TransactionController extends BaseController< txParams: newTxParams, }); - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ - networkClientId: transactionMeta.networkClientId, - chainId: transactionMeta.chainId, - }); + const { networkClientId } = transactionMeta; + const ethQuery = this.#getEthQuery({ networkClientId }); const newTransactionMeta = { ...transactionMetaWithRsv, @@ -1324,11 +1317,12 @@ export class TransactionController extends BaseController< */ async estimateGas( transaction: TransactionParams, - networkClientId?: NetworkClientId, + networkClientId: NetworkClientId, ) { - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQuery({ networkClientId, }); + const { estimatedGas, simulationFails } = await estimateGas( transaction, ethQuery, @@ -1347,11 +1341,12 @@ export class TransactionController extends BaseController< async estimateGasBuffered( transaction: TransactionParams, multiplier: number, - networkClientId?: NetworkClientId, + networkClientId: NetworkClientId, ) { - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ + const ethQuery = this.#getEthQuery({ networkClientId, }); + const { blockGasLimit, estimatedGas, simulationFails } = await estimateGas( transaction, ethQuery, @@ -1411,13 +1406,19 @@ export class TransactionController extends BaseController< } /** - * Removes all transactions from state, optionally based on the current network. + * Remove transactions from state. * - * @param chainId - Remove transactions for the specified chain only. Defaults to all chains. - * @param address - If specified, only transactions originating from this address will be - * wiped on current network. + * @param options - The options bag. + * @param options.address - Remove transactions from this account only. Defaults to all accounts. + * @param options.chainId - Remove transactions for the specified chain only. Defaults to all chains. */ - wipeTransactions(chainId?: string, address?: string) { + wipeTransactions({ + address, + chainId, + }: { + address?: string; + chainId?: string; + } = {}) { if (!chainId && !address) { this.update((state) => { state.transactions = []; @@ -1698,7 +1699,7 @@ export class TransactionController extends BaseController< async getNonceLock( address: string, - networkClientId?: NetworkClientId, + networkClientId: NetworkClientId, ): Promise { return this.#multichainTrackingHelper.getNonceLock( address, @@ -1762,15 +1763,16 @@ export class TransactionController extends BaseController< ) as TransactionParams; const updatedTransaction = merge({}, transactionMeta, editableParams); - const provider = this.#multichainTrackingHelper.getProvider({ - chainId: transactionMeta.chainId, - networkClientId: transactionMeta.networkClientId, - }); + + const { networkClientId } = transactionMeta; + const provider = this.#getProvider({ networkClientId }); const ethQuery = new EthQuery(provider); + const { type } = await determineTransactionType( updatedTransaction.txParams, ethQuery, ); + updatedTransaction.type = type; await updateTransactionLayer1GasFee({ @@ -1783,6 +1785,7 @@ export class TransactionController extends BaseController< updatedTransaction, `Update Editable Params for ${txId}`, ); + return this.getTransaction(txId); } @@ -1807,26 +1810,14 @@ export class TransactionController extends BaseController< } const initialTx = listOfTxParams[0]; - const common = this.getCommonConfiguration(initialTx.chainId); - - // We need to ensure we get the nonce using the the NonceTracker on the chain matching - // the txParams. In this context we only have chainId available to us, but the - // NonceTrackers are keyed by networkClientId. To workaround this, we attempt to find - // a networkClientId that matches the chainId. As a fallback, the globally selected - // network's NonceTracker will be used instead. - let networkClientId: NetworkClientId | undefined; - try { - networkClientId = this.messagingSystem.call( - `NetworkController:findNetworkClientIdByChainId`, - initialTx.chainId, - ); - } catch (err) { - log('failed to find networkClientId from chainId', err); - } + const { chainId } = initialTx; + const common = this.getCommonConfiguration(chainId); + const networkClientId = this.#getNetworkClientId({ chainId }); const initialTxAsEthTx = TransactionFactory.fromTxData(initialTx, { common, }); + const initialTxAsSerializedHex = bufferToHex(initialTxAsEthTx.serialize()); if (this.approvingTransactionIds.has(initialTxAsSerializedHex)) { @@ -2056,10 +2047,11 @@ export class TransactionController extends BaseController< chainId?: Hex; networkClientId?: NetworkClientId; }): Promise { - const networkClientId = this.#getNetworkClientId({ - networkClientId: requestNetworkClientId, - chainId, - }); + const { id: networkClientId, provider } = + this.#multichainTrackingHelper.getNetworkClient({ + chainId, + networkClientId: requestNetworkClientId, + }); const transactionMeta = { txParams: transactionParams, @@ -2073,10 +2065,7 @@ export class TransactionController extends BaseController< this.gasFeeFlows, ) as GasFeeFlow; - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ - networkClientId, - chainId, - }); + const ethQuery = new EthQuery(provider); const gasFeeControllerData = await this.getGasFeeEstimates({ networkClientId, @@ -2106,9 +2095,9 @@ export class TransactionController extends BaseController< chainId?: Hex; networkClientId?: NetworkClientId; }): Promise { - const provider = this.#multichainTrackingHelper.getProvider({ - networkClientId, + const provider = this.#getProvider({ chainId, + networkClientId, }); return await getTransactionLayer1GasFee({ @@ -2210,17 +2199,12 @@ export class TransactionController extends BaseController< const { networkClientId, chainId } = transactionMeta; - const isCustomNetwork = this.#isCustomNetwork(networkClientId as string); - - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ - networkClientId, - chainId, - }); + const isCustomNetwork = + this.#multichainTrackingHelper.getNetworkClient({ networkClientId }) + .configuration.type === NetworkClientType.Custom; - const provider = this.#multichainTrackingHelper.getProvider({ - networkClientId, - chainId, - }); + const ethQuery = this.#getEthQuery({ networkClientId }); + const provider = this.#getProvider({ networkClientId }); await this.#trace( { name: 'Update Gas', parentContext: traceContext }, @@ -2496,10 +2480,8 @@ export class TransactionController extends BaseController< return ApprovalState.NotApproved; } - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ - networkClientId: transactionMeta.networkClientId, - chainId: transactionMeta.chainId, - }); + const { networkClientId } = transactionMeta; + const ethQuery = this.#getEthQuery({ networkClientId }); let preTxBalance: string | undefined; const shouldUpdatePreTxBalance = @@ -2779,11 +2761,48 @@ export class TransactionController extends BaseController< return { meta: transaction, isCompleted }; } - private getChainId(networkClientId: NetworkClientId): Hex { - return this.messagingSystem.call( - `NetworkController:getNetworkClientById`, + #getChainId(networkClientId: NetworkClientId): Hex { + return this.#multichainTrackingHelper.getNetworkClient({ networkClientId }) + .configuration.chainId; + } + + #getNetworkClientId({ + chainId, + networkClientId, + }: { + chainId?: Hex; + networkClientId?: NetworkClientId; + }) { + if (networkClientId) { + return networkClientId; + } + + return this.#multichainTrackingHelper.getNetworkClient({ + chainId, + }).id; + } + + #getEthQuery({ + chainId, + networkClientId, + }: { + chainId?: Hex; + networkClientId?: NetworkClientId; + }): EthQuery { + return new EthQuery(this.#getProvider({ chainId, networkClientId })); + } + + #getProvider({ + chainId, + networkClientId, + }: { + chainId?: Hex; + networkClientId?: NetworkClientId; + }): Provider { + return this.#multichainTrackingHelper.getNetworkClient({ + chainId, networkClientId, - ).configuration.chainId; + }).provider; } private prepareUnsignedEthTx( @@ -3180,14 +3199,14 @@ export class TransactionController extends BaseController< private async updatePostBalance(transactionMeta: TransactionMeta) { try { - if (transactionMeta.type !== TransactionType.swap) { + const { networkClientId, type } = transactionMeta; + + if (type !== TransactionType.swap) { return; } - const ethQuery = this.#multichainTrackingHelper.getEthQuery({ - networkClientId: transactionMeta.networkClientId, - chainId: transactionMeta.chainId, - }); + const ethQuery = this.#getEthQuery({ networkClientId }); + const { updatedTransactionMeta, approvalTransactionMeta } = await updatePostTransactionBalance(transactionMeta, { ethQuery, @@ -3603,36 +3622,6 @@ export class TransactionController extends BaseController< ); } - #getNetworkClientId({ - chainId, - networkClientId, - }: { - chainId?: Hex; - networkClientId?: NetworkClientId; - }) { - if (!chainId && !networkClientId) { - throw new Error('Must provide either chainId or networkClientId'); - } - - if (networkClientId) { - return networkClientId; - } - - return this.messagingSystem.call( - `NetworkController:findNetworkClientIdByChainId`, - chainId as Hex, - ); - } - - #isCustomNetwork(networkClientId: NetworkClientId) { - return ( - this.messagingSystem.call( - `NetworkController:getNetworkClientById`, - networkClientId, - ).configuration.type === NetworkClientType.Custom - ); - } - #getSelectedAccount() { return this.messagingSystem.call('AccountsController:getSelectedAccount'); } diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 3438f975f4..9584386139 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -1107,7 +1107,7 @@ describe('TransactionController Integration', () => { }, ), ).rejects.toThrow( - 'The networkClientId for this transaction could not be found', + `Network client not found - ${networkConfiguration.rpcEndpoints[0].networkClientId}`, ); expect(transactionController).toBeDefined(); diff --git a/packages/transaction-controller/src/gas-flows/DefaultGasFeeFlow.test.ts b/packages/transaction-controller/src/gas-flows/DefaultGasFeeFlow.test.ts index 178fa1ccd7..9bb490cc4d 100644 --- a/packages/transaction-controller/src/gas-flows/DefaultGasFeeFlow.test.ts +++ b/packages/transaction-controller/src/gas-flows/DefaultGasFeeFlow.test.ts @@ -21,6 +21,7 @@ const ETH_QUERY_MOCK = {} as EthQuery; const TRANSACTION_META_MOCK: TransactionMeta = { id: '1', chainId: '0x123', + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 0, txParams: { diff --git a/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts b/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts index 64d84fbdec..de73118270 100644 --- a/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts +++ b/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts @@ -24,6 +24,7 @@ jest.mock('@metamask/controller-utils', () => ({ const TRANSACTION_META_MOCK: TransactionMeta = { id: '1', chainId: '0x123', + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 0, txParams: { diff --git a/packages/transaction-controller/src/gas-flows/OptimismLayer1GasFeeFlow.test.ts b/packages/transaction-controller/src/gas-flows/OptimismLayer1GasFeeFlow.test.ts index b8f473a672..b3502e5d8f 100644 --- a/packages/transaction-controller/src/gas-flows/OptimismLayer1GasFeeFlow.test.ts +++ b/packages/transaction-controller/src/gas-flows/OptimismLayer1GasFeeFlow.test.ts @@ -6,6 +6,7 @@ import { OptimismLayer1GasFeeFlow } from './OptimismLayer1GasFeeFlow'; const TRANSACTION_META_MOCK: TransactionMeta = { id: '1', chainId: CHAIN_IDS.OPTIMISM, + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 0, txParams: { diff --git a/packages/transaction-controller/src/gas-flows/OracleLayer1GasFeeFlow.test.ts b/packages/transaction-controller/src/gas-flows/OracleLayer1GasFeeFlow.test.ts index 38de3ff43a..f567e50e7b 100644 --- a/packages/transaction-controller/src/gas-flows/OracleLayer1GasFeeFlow.test.ts +++ b/packages/transaction-controller/src/gas-flows/OracleLayer1GasFeeFlow.test.ts @@ -26,6 +26,7 @@ const TRANSACTION_PARAMS_MOCK = { const TRANSACTION_META_MOCK: TransactionMeta = { id: '1', chainId: CHAIN_IDS.OPTIMISM, + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 0, txParams: TRANSACTION_PARAMS_MOCK, diff --git a/packages/transaction-controller/src/gas-flows/ScrollLayer1GasFeeFlow.test.ts b/packages/transaction-controller/src/gas-flows/ScrollLayer1GasFeeFlow.test.ts index ccb376b15a..a5f45c3815 100644 --- a/packages/transaction-controller/src/gas-flows/ScrollLayer1GasFeeFlow.test.ts +++ b/packages/transaction-controller/src/gas-flows/ScrollLayer1GasFeeFlow.test.ts @@ -6,6 +6,7 @@ import { ScrollLayer1GasFeeFlow } from './ScrollLayer1GasFeeFlow'; const TRANSACTION_META_MOCK: TransactionMeta = { id: '1', chainId: CHAIN_IDS.OPTIMISM, + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 0, txParams: { diff --git a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts index 9a4c891370..d890a9a681 100644 --- a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts +++ b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts @@ -207,6 +207,7 @@ export class EtherscanRemoteTransactionSource chainId: currentChainId, hash: txMeta.hash, id: random({ msecs: time }), + networkClientId: 'incoming', status: TransactionStatus.confirmed, time, txParams: { diff --git a/packages/transaction-controller/src/helpers/GasFeePoller.test.ts b/packages/transaction-controller/src/helpers/GasFeePoller.test.ts index bd62f1fa2d..4db5de53f6 100644 --- a/packages/transaction-controller/src/helpers/GasFeePoller.test.ts +++ b/packages/transaction-controller/src/helpers/GasFeePoller.test.ts @@ -231,28 +231,6 @@ describe('GasFeePoller', () => { networkClientId: 'networkClientId4', }); }); - - it('using found network client ID if none in metadata', async () => { - getTransactionsMock.mockReturnValue([ - { - ...TRANSACTION_META_MOCK, - chainId: '0x1', - networkClientId: undefined, - }, - ]); - - findNetworkClientIdByChainIdMock.mockReturnValue('networkClientId1'); - - new GasFeePoller(constructorOptions); - - triggerOnStateChange(); - await flushPromises(); - - expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledTimes(1); - expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledWith({ - networkClientId: 'networkClientId1', - }); - }); }); }); diff --git a/packages/transaction-controller/src/helpers/GasFeePoller.ts b/packages/transaction-controller/src/helpers/GasFeePoller.ts index 1aaff0ea06..1e8a94a1c4 100644 --- a/packages/transaction-controller/src/helpers/GasFeePoller.ts +++ b/packages/transaction-controller/src/helpers/GasFeePoller.ts @@ -37,7 +37,7 @@ export class GasFeePoller { options: FetchGasFeeEstimateOptions, ) => Promise; - #getProvider: (chainId: Hex, networkClientId?: NetworkClientId) => Provider; + #getProvider: (networkClientId: NetworkClientId) => Provider; #getTransactions: () => TransactionMeta[]; @@ -72,7 +72,7 @@ export class GasFeePoller { getGasFeeControllerEstimates: ( options: FetchGasFeeEstimateOptions, ) => Promise; - getProvider: (chainId: Hex, networkClientId?: NetworkClientId) => Provider; + getProvider: (networkClientId: NetworkClientId) => Provider; getTransactions: () => TransactionMeta[]; layer1GasFeeFlows: Layer1GasFeeFlow[]; onStateChange: (listener: () => void) => void; @@ -190,9 +190,9 @@ export class GasFeePoller { | { gasFeeEstimates?: GasFeeEstimates; gasFeeEstimatesLoaded: boolean } | undefined > { - const { chainId, networkClientId } = transactionMeta; + const { networkClientId } = transactionMeta; - const ethQuery = new EthQuery(this.#getProvider(chainId, networkClientId)); + const ethQuery = new EthQuery(this.#getProvider(networkClientId)); const gasFeeFlow = getGasFeeFlow(transactionMeta, this.#gasFeeFlows); if (gasFeeFlow) { @@ -235,8 +235,8 @@ export class GasFeePoller { async #updateTransactionLayer1GasFee( transactionMeta: TransactionMeta, ): Promise { - const { chainId, networkClientId } = transactionMeta; - const provider = this.#getProvider(chainId, networkClientId); + const { networkClientId } = transactionMeta; + const provider = this.#getProvider(networkClientId); const layer1GasFee = await getTransactionLayer1GasFee({ layer1GasFeeFlows: this.#layer1GasFeeFlows, diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts index 13b2401768..524fb06700 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts @@ -1,5 +1,5 @@ /* eslint-disable jsdoc/require-jsdoc */ -import { ChainId } from '@metamask/controller-utils'; +import { ChainId, NetworkType } from '@metamask/controller-utils'; import type { NetworkClientId, Provider } from '@metamask/network-controller'; import type { NonceTracker } from '@metamask/nonce-tracker'; import type { Hex } from '@metamask/utils'; @@ -523,7 +523,9 @@ describe('MultichainTrackingHelper', () => { await expect( helper.getNonceLock('0xdeadbeef', 'mainnet'), - ).rejects.toThrow('missing nonceTracker for networkClientId'); + ).rejects.toThrow( + `Missing nonce tracker for network client ID - ${NetworkType.mainnet}`, + ); }); it('throws an error and releases nonce lock by chainId if unable to acquire nonce lock from the NonceTracker', async () => { @@ -613,83 +615,12 @@ describe('MultichainTrackingHelper', () => { }); }); - describe('getEthQuery', () => { + describe('getNetworkClient', () => { describe('when given networkClientId and chainId', () => { - it('returns EthQuery with the networkClientId provider when available', () => { + it('returns the network client of the networkClientId when available', () => { const { options, helper } = newMultichainTrackingHelper(); - const ethQuery = helper.getEthQuery({ - networkClientId: 'goerli', - chainId: '0xa', - }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.goerli); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith('goerli'); - }); - - it('returns EthQuery with a fallback networkClient provider matching the chainId when available', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ - networkClientId: 'missingNetworkClientId', - chainId: '0xa', - }); - expect(ethQuery.provider).toBe( - MOCK_PROVIDERS['customNetworkClientId-1'], - ); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(2); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'missingNetworkClientId', - ); - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xa', - ); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'customNetworkClientId-1', - ); - }); - }); - - describe('when given only networkClientId', () => { - it('returns EthQuery with the networkClientId provider when available', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ networkClientId: 'goerli' }); - expect(ethQuery.provider).toBe(MOCK_PROVIDERS.goerli); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.getNetworkClientById).toHaveBeenCalledWith('goerli'); - }); - }); - - describe('when given only chainId', () => { - it('returns EthQuery with a fallback networkClient provider matching the chainId when available', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const ethQuery = helper.getEthQuery({ chainId: '0xa' }); - expect(ethQuery.provider).toBe( - MOCK_PROVIDERS['customNetworkClientId-1'], - ); - - expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); - expect(options.findNetworkClientIdByChainId).toHaveBeenCalledWith( - '0xa', - ); - expect(options.getNetworkClientById).toHaveBeenCalledWith( - 'customNetworkClientId-1', - ); - }); - }); - }); - - describe('getProvider', () => { - describe('when given networkClientId and chainId', () => { - it('returns the provider of the networkClientId when available', () => { - const { options, helper } = newMultichainTrackingHelper(); - - const provider = helper.getProvider({ + const { provider } = helper.getNetworkClient({ networkClientId: 'goerli', chainId: '0xa', }); @@ -699,10 +630,10 @@ describe('MultichainTrackingHelper', () => { expect(options.getNetworkClientById).toHaveBeenCalledWith('goerli'); }); - it('returns a fallback networkClient provider matching the chainId when available', () => { + it('returns a fallback network client matching the chainId when available', () => { const { options, helper } = newMultichainTrackingHelper(); - const provider = helper.getProvider({ + const { provider } = helper.getNetworkClient({ networkClientId: 'missingNetworkClientId', chainId: '0xa', }); @@ -722,10 +653,12 @@ describe('MultichainTrackingHelper', () => { }); describe('when given only networkClientId', () => { - it('returns the provider of the networkClientId when available', () => { + it('returns the network client of the networkClientId when available', () => { const { options, helper } = newMultichainTrackingHelper(); - const provider = helper.getProvider({ networkClientId: 'goerli' }); + const { provider } = helper.getNetworkClient({ + networkClientId: 'goerli', + }); expect(provider).toBe(MOCK_PROVIDERS.goerli); expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); @@ -734,10 +667,10 @@ describe('MultichainTrackingHelper', () => { }); describe('when given only chainId', () => { - it('returns a fallback networkClient provider matching the chainId when available', () => { + it('returns a fallback network client matching the chainId when available', () => { const { options, helper } = newMultichainTrackingHelper(); - const provider = helper.getProvider({ chainId: '0xa' }); + const { provider } = helper.getNetworkClient({ chainId: '0xa' }); expect(provider).toBe(MOCK_PROVIDERS['customNetworkClientId-1']); expect(options.getNetworkClientById).toHaveBeenCalledTimes(1); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts index 9b6f570e71..fe38c0ed66 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts @@ -1,4 +1,3 @@ -import EthQuery from '@metamask/eth-query'; import type { NetworkClientId, NetworkController, @@ -11,7 +10,7 @@ import type { NonceLock, NonceTracker } from '@metamask/nonce-tracker'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { incomingTransactionsLogger as log } from '../logger'; +import { createModuleLogger, projectLogger } from '../logger'; import { EtherscanRemoteTransactionSource } from './EtherscanRemoteTransactionSource'; import type { IncomingTransactionHelper, @@ -26,6 +25,8 @@ type NetworkClientRegistry = ReturnType< NetworkController['getNetworkClientRegistry'] >; +const log = createModuleLogger(projectLogger, 'multichain-tracking'); + export type MultichainTrackingHelperOptions = { incomingTransactionOptions: IncomingTransactionOptions; @@ -140,6 +141,7 @@ export class MultichainTrackingHelper { onNetworkStateChange((_, patches) => { const networkClients = this.#getNetworkClientRegistry(); + patches.forEach(({ op, path }) => { if (op === 'remove' && path[0] === 'networkConfigurations') { const networkClientId = path[1] as NetworkClientId; @@ -153,42 +155,16 @@ export class MultichainTrackingHelper { initialize() { const networkClients = this.#getNetworkClientRegistry(); + this.#refreshTrackingMap(networkClients); + + log('Initialized'); } has(networkClientId: NetworkClientId) { return this.#trackingMap.has(networkClientId); } - getEthQuery({ - networkClientId, - chainId, - }: { - networkClientId?: NetworkClientId; - chainId?: Hex; - } = {}): EthQuery { - return new EthQuery(this.getProvider({ networkClientId, chainId })); - } - - getProvider({ - networkClientId, - chainId, - }: { - networkClientId?: NetworkClientId; - chainId?: Hex; - } = {}): Provider { - const networkClient = this.#getNetworkClient({ - networkClientId, - chainId, - }); - - if (!networkClient) { - throw new Error('missing networkClient'); - } - - return networkClient?.provider; - } - /** * Gets the mutex intended to guard the nonceTracker for a particular chainId and key . * @@ -228,25 +204,21 @@ export class MultichainTrackingHelper { */ async getNonceLock( address: string, - networkClientId?: NetworkClientId, + networkClientId: NetworkClientId, ): Promise { - let releaseLockForChainIdKey: (() => void) | undefined; - let nonceTracker; - if (networkClientId) { - const networkClient = this.#getNetworkClientById(networkClientId); - releaseLockForChainIdKey = await this.acquireNonceLockForChainIdKey({ - chainId: networkClient.configuration.chainId, - key: address, - }); - const trackers = this.#trackingMap.get(networkClientId); - if (!trackers) { - throw new Error('missing nonceTracker for networkClientId'); - } - nonceTracker = trackers.nonceTracker; - } + const networkClient = this.#getNetworkClientById(networkClientId); + + const releaseLockForChainIdKey = await this.acquireNonceLockForChainIdKey({ + chainId: networkClient.configuration.chainId, + key: address, + }); + + const nonceTracker = this.#trackingMap.get(networkClientId)?.nonceTracker; if (!nonceTracker) { - throw new Error('missing nonceTracker'); + throw new Error( + `Missing nonce tracker for network client ID - ${networkClientId}`, + ); } // Acquires the lock for the chainId + address and the nonceLock from the nonceTracker, then @@ -255,12 +227,15 @@ export class MultichainTrackingHelper { // lock for the chainId. try { const nonceLock = await nonceTracker.getNonceLock(address); + + const releaseLock = () => { + nonceLock.releaseLock(); + releaseLockForChainIdKey?.(); + }; + return { ...nonceLock, - releaseLock: () => { - nonceLock.releaseLock(); - releaseLockForChainIdKey?.(); - }, + releaseLock, }; } catch (err) { releaseLockForChainIdKey?.(); @@ -317,6 +292,45 @@ export class MultichainTrackingHelper { } } + getNetworkClient({ + chainId, + networkClientId: requestNetworkClientId, + }: { + chainId?: Hex; + networkClientId?: NetworkClientId; + }): NetworkClient & { id: NetworkClientId } { + if (!requestNetworkClientId && !chainId) { + throw new Error( + 'Cannot locate network client without networkClientId or chainId', + ); + } + + let networkClient: NetworkClient | undefined; + let networkClientId = requestNetworkClientId; + + try { + if (requestNetworkClientId) { + networkClient = this.#getNetworkClientById(requestNetworkClientId); + } + } catch (error) { + log('No network client found with ID', requestNetworkClientId); + + if (!chainId) { + throw error; + } + } + + if (!networkClient && chainId) { + networkClientId = this.#findNetworkClientIdByChainId(chainId); + networkClient = this.#getNetworkClientById(networkClientId); + } + + return { + ...(networkClient as NetworkClient), + id: networkClientId as NetworkClientId, + }; + } + #refreshTrackingMap = (networkClients: NetworkClientRegistry) => { this.#refreshEtherscanRemoteTransactionSources(networkClients); @@ -327,6 +341,7 @@ export class MultichainTrackingHelper { const networkClientIdsToRemove = existingNetworkClientIds.filter( (id) => !networkClientIds.includes(id), ); + networkClientIdsToRemove.forEach((id) => { this.#stopTrackingByNetworkClientId(id); }); @@ -335,9 +350,18 @@ export class MultichainTrackingHelper { const networkClientIdsToAdd = networkClientIds.filter( (id) => !existingNetworkClientIds.includes(id), ); + networkClientIdsToAdd.forEach((id) => { this.#startTrackingByNetworkClientId(id); }); + + if (networkClientIdsToAdd.length) { + log('Added trackers', networkClientIdsToAdd); + } + + if (networkClientIdsToRemove.length) { + log('Removed trackers', networkClientIdsToRemove); + } }; #stopTrackingByNetworkClientId(networkClientId: NetworkClientId) { @@ -369,11 +393,15 @@ export class MultichainTrackingHelper { let etherscanRemoteTransactionSource = this.#etherscanRemoteTransactionSourcesMap.get(chainId); + if (!etherscanRemoteTransactionSource) { etherscanRemoteTransactionSource = new EtherscanRemoteTransactionSource({ includeTokenTransfers: this.#incomingTransactionOptions.includeTokenTransfers, }); + + log('Created Etherscan remote transaction source', chainId); + this.#etherscanRemoteTransactionSourcesMap.set( chainId, etherscanRemoteTransactionSource, @@ -426,32 +454,4 @@ export class MultichainTrackingHelper { this.#etherscanRemoteTransactionSourcesMap.delete(chainId); }); }; - - #getNetworkClient({ - networkClientId, - chainId, - }: { - networkClientId?: NetworkClientId; - chainId?: Hex; - } = {}): NetworkClient | undefined { - let networkClient: NetworkClient | undefined; - - if (networkClientId) { - try { - networkClient = this.#getNetworkClientById(networkClientId); - } catch (err) { - log('failed to get network client by networkClientId'); - } - } - if (!networkClient && chainId) { - try { - const networkClientIdForChainId = - this.#findNetworkClientIdByChainId(chainId); - networkClient = this.#getNetworkClientById(networkClientIdForChainId); - } catch (err) { - log('failed to get network client by chainId'); - } - } - return networkClient; - } } diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index f70679a6bc..fe8b3f3bc9 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -206,7 +206,7 @@ type TransactionMetaBase = { /** * The ID of the network client used by the transaction. */ - networkClientId?: NetworkClientId; + networkClientId: NetworkClientId; /** * Network code as per EIP-155 for this transaction diff --git a/packages/transaction-controller/src/utils/gas-flow.test.ts b/packages/transaction-controller/src/utils/gas-flow.test.ts index d49f0c49fc..af66b3e691 100644 --- a/packages/transaction-controller/src/utils/gas-flow.test.ts +++ b/packages/transaction-controller/src/utils/gas-flow.test.ts @@ -13,6 +13,7 @@ import { getGasFeeFlow, mergeGasFeeEstimates } from './gas-flow'; const TRANSACTION_META_MOCK: TransactionMeta = { id: '1', chainId: '0x123', + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 0, txParams: { diff --git a/packages/transaction-controller/src/utils/history.test.ts b/packages/transaction-controller/src/utils/history.test.ts index 97ab9b9e10..6fd2ca5f5c 100644 --- a/packages/transaction-controller/src/utils/history.test.ts +++ b/packages/transaction-controller/src/utils/history.test.ts @@ -288,6 +288,7 @@ function createMinimalMockTransaction(): TransactionMeta { return { chainId: toHex(1337), id: 'mock-id', + networkClientId: 'testNetworkClientId', time: 0, status: TransactionStatus.submitted as const, txParams: { diff --git a/packages/transaction-controller/src/utils/layer1-gas-fee-flow.test.ts b/packages/transaction-controller/src/utils/layer1-gas-fee-flow.test.ts index f9f9421819..4c3e3042b3 100644 --- a/packages/transaction-controller/src/utils/layer1-gas-fee-flow.test.ts +++ b/packages/transaction-controller/src/utils/layer1-gas-fee-flow.test.ts @@ -52,10 +52,13 @@ describe('updateTransactionLayer1GasFee', () => { layer1Fee: LAYER1_GAS_FEE_VALUE_MATCH_MOCK, }), ]; + providerMock = {} as Provider; + transactionMetaMock = { id: '1', chainId: '0x123', + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 0, txParams: { diff --git a/packages/transaction-controller/src/utils/nonce.test.ts b/packages/transaction-controller/src/utils/nonce.test.ts index ce26077d4f..e25bdf1ea5 100644 --- a/packages/transaction-controller/src/utils/nonce.test.ts +++ b/packages/transaction-controller/src/utils/nonce.test.ts @@ -10,6 +10,7 @@ import { getAndFormatTransactionsForNonceTracker, getNextNonce } from './nonce'; const TRANSACTION_META_MOCK: TransactionMeta = { chainId: '0x1', id: 'testId1', + networkClientId: 'testNetworkClientId', status: TransactionStatus.unapproved, time: 1, txParams: { @@ -86,6 +87,7 @@ describe('nonce', () => { { id: '1', chainId: '0x1', + networkClientId: 'testNetworkClientId', time: 123456, txParams: { from: fromAddress, @@ -98,6 +100,7 @@ describe('nonce', () => { { id: '2', chainId: '0x1', + networkClientId: 'testNetworkClientId', time: 123457, txParams: { from: '0x124', @@ -110,6 +113,7 @@ describe('nonce', () => { { id: '3', chainId: '0x1', + networkClientId: 'testNetworkClientId', time: 123458, txParams: { from: fromAddress, @@ -122,6 +126,7 @@ describe('nonce', () => { { id: '4', chainId: '0x2', + networkClientId: 'testNetworkClientId', time: 123459, txParams: { from: fromAddress, @@ -134,6 +139,7 @@ describe('nonce', () => { { id: '5', chainId: '0x2', + networkClientId: 'testNetworkClientId', isTransfer: true, time: 123460, txParams: { diff --git a/packages/transaction-controller/src/utils/resimulate.test.ts b/packages/transaction-controller/src/utils/resimulate.test.ts index 1c34f22065..f27e19498c 100644 --- a/packages/transaction-controller/src/utils/resimulate.test.ts +++ b/packages/transaction-controller/src/utils/resimulate.test.ts @@ -1,4 +1,5 @@ /* eslint-disable @typescript-eslint/naming-convention */ +import { NetworkType } from '@metamask/controller-utils'; import { BN } from 'bn.js'; import { CHAIN_IDS } from '../constants'; @@ -61,6 +62,7 @@ const SIMULATION_DATA_2_MOCK: SimulationData = { const TRANSACTION_META_MOCK: TransactionMeta = { chainId: CHAIN_IDS.MAINNET, id: '123-456', + networkClientId: NetworkType.mainnet, securityAlertResponse: SECURITY_ALERT_RESPONSE_MOCK, status: TransactionStatus.unapproved, time: 1234567890, diff --git a/packages/transaction-controller/tests/EtherscanMocks.ts b/packages/transaction-controller/tests/EtherscanMocks.ts index c547279218..8641f5bcdc 100644 --- a/packages/transaction-controller/tests/EtherscanMocks.ts +++ b/packages/transaction-controller/tests/EtherscanMocks.ts @@ -95,6 +95,7 @@ const EXPECTED_NORMALISED_TRANSACTION_BASE = { chainId: undefined, hash: ETHERSCAN_TRANSACTION_SUCCESS_MOCK.hash, id: ID_MOCK, + networkClientId: 'incoming', status: TransactionStatus.confirmed, time: 1543596356000, txParams: { diff --git a/packages/user-operation-controller/src/utils/transaction.ts b/packages/user-operation-controller/src/utils/transaction.ts index b9406cd449..0e606536af 100644 --- a/packages/user-operation-controller/src/utils/transaction.ts +++ b/packages/user-operation-controller/src/utils/transaction.ts @@ -131,6 +131,7 @@ export function getTransactionMetadata( hash: transactionHash ?? undefined, id, isUserOperation: true, + networkClientId: 'user-operation', origin, status, time, From 9d46543795a5cae21972418e08f0e7102e8ed016 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 13 Nov 2024 20:32:05 +0000 Subject: [PATCH 03/12] Support start and stop of all incoming transaction polling Remove stopAllIncomingTransactionPolling method. Create separate helper loggers per chain. --- .../src/TransactionController.ts | 8 +--- .../TransactionControllerIntegration.test.ts | 46 ++----------------- .../src/helpers/IncomingTransactionHelper.ts | 22 +++++++-- .../src/helpers/MultichainTrackingHelper.ts | 16 +++++-- .../src/helpers/PendingTransactionTracker.ts | 43 +++++++++-------- .../src/utils/etherscan.ts | 6 ++- 6 files changed, 62 insertions(+), 79 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 59890a0f5d..7eed6a559d 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1086,22 +1086,18 @@ export class TransactionController extends BaseController< }; } - startIncomingTransactionPolling(networkClientIds: NetworkClientId[] = []) { + startIncomingTransactionPolling(networkClientIds?: NetworkClientId[]) { this.#multichainTrackingHelper.startIncomingTransactionPolling( networkClientIds, ); } - stopIncomingTransactionPolling(networkClientIds: NetworkClientId[] = []) { + stopIncomingTransactionPolling(networkClientIds?: NetworkClientId[]) { this.#multichainTrackingHelper.stopIncomingTransactionPolling( networkClientIds, ); } - stopAllIncomingTransactionPolling() { - this.#multichainTrackingHelper.stopAllIncomingTransactionPolling(); - } - async updateIncomingTransactions(networkClientIds: NetworkClientId[] = []) { await this.#multichainTrackingHelper.updateIncomingTransactions( networkClientIds, diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 9584386139..1f0766b5db 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -400,7 +400,7 @@ describe('TransactionController Integration', () => { }); describe('multichain transaction lifecycle', () => { - describe('when a transaction is added with a networkClientId that does not match the globally selected network', () => { + describe('when a transaction is added with a networkClientId', () => { it('should add a new unapproved transaction', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -1437,47 +1437,7 @@ describe('TransactionController Integration', () => { transactionController.destroy(); }); - it('should stop the global incoming transaction helper when no networkClientIds provided', async () => { - const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; - const selectedAccountMock = createMockInternalAccount({ - address: selectedAddress, - }); - - const { transactionController } = await setupController( - {}, - { selectedAccount: selectedAccountMock }, - ); - - mockNetwork({ - networkClientConfiguration: buildInfuraNetworkClientConfiguration( - InfuraNetworkType.mainnet, - ), - mocks: [ - buildEthBlockNumberRequestMock('0x1'), - buildEthBlockNumberRequestMock('0x2'), - ], - }); - nock(getEtherscanApiHost(BUILT_IN_NETWORKS[NetworkType.mainnet].chainId)) - .get( - `/api?module=account&address=${selectedAddress}&offset=40&sort=desc&action=txlist&tag=latest&page=1`, - ) - .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); - - transactionController.startIncomingTransactionPolling(); - - transactionController.stopIncomingTransactionPolling(); - await advanceTime({ clock, duration: BLOCK_TRACKER_POLLING_INTERVAL }); - - expect(transactionController.state.transactions).toStrictEqual([]); - expect(transactionController.state.lastFetchedBlockNumbers).toStrictEqual( - {}, - ); - transactionController.destroy(); - }); - }); - - describe('stopAllIncomingTransactionPolling', () => { - it('should not poll for incoming transactions on any network client', async () => { + it('should not poll for incoming transactions on any network client if no network client IDs provided', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; const selectedAccountMock = createMockInternalAccount({ address: selectedAddress, @@ -1510,7 +1470,7 @@ describe('TransactionController Integration', () => { }), ); - transactionController.stopAllIncomingTransactionPolling(); + transactionController.stopIncomingTransactionPolling(); await advanceTime({ clock, duration: BLOCK_TRACKER_POLLING_INTERVAL }); expect(transactionController.state.transactions).toStrictEqual([]); diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index ed9b0f1cd3..016ba2ebeb 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -4,7 +4,10 @@ import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import EventEmitter from 'events'; -import { incomingTransactionsLogger as log } from '../logger'; +import { + createModuleLogger, + incomingTransactionsLogger as log, +} from '../logger'; import type { RemoteTransactionSource, TransactionMeta } from '../types'; const RECENT_HISTORY_BLOCK_RANGE = 10; @@ -50,6 +53,8 @@ export class IncomingTransactionHelper { #isRunning: boolean; + #log: debug.Debugger; + #mutex = new Mutex(); #onLatestBlock: (blockNumberHex: Hex) => Promise; @@ -96,6 +101,7 @@ export class IncomingTransactionHelper { this.#getChainId = getChainId; this.#isEnabled = isEnabled ?? (() => true); this.#isRunning = false; + this.#log = createModuleLogger(log, getChainId()); this.#queryEntireHistory = queryEntireHistory ?? true; this.#remoteTransactionSource = remoteTransactionSource; this.#transactionLimit = transactionLimit; @@ -121,6 +127,8 @@ export class IncomingTransactionHelper { return; } + this.#log('Starting polling'); + // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/no-misused-promises this.#blockTracker.addListener('latest', this.#onLatestBlock); @@ -128,16 +136,22 @@ export class IncomingTransactionHelper { } stop() { + if (!this.#isRunning) { + return; + } + // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/no-misused-promises this.#blockTracker.removeListener('latest', this.#onLatestBlock); this.#isRunning = false; + + this.#log('Stopped polling'); } async update(latestBlockNumberHex?: Hex): Promise { const releaseLock = await this.#mutex.acquire(); - log('Checking for incoming transactions'); + this.#log('Checking'); try { if (!this.#canStart()) { @@ -169,7 +183,7 @@ export class IncomingTransactionHelper { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { - log('Error while fetching remote transactions', error); + this.#log('Error while fetching remote transactions', error); return; } if (!this.#updateTransactions) { @@ -197,7 +211,7 @@ export class IncomingTransactionHelper { this.#sortTransactionsByTime(newTransactions); this.#sortTransactionsByTime(updatedTransactions); - log('Found incoming transactions', { + this.#log('Found incoming transactions', { new: newTransactions, updated: updatedTransactions, }); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts index fe38c0ed66..92ea1665ca 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts @@ -243,14 +243,22 @@ export class MultichainTrackingHelper { } } - startIncomingTransactionPolling(networkClientIds: NetworkClientId[] = []) { - networkClientIds.forEach((networkClientId) => { + startIncomingTransactionPolling(networkClientIds?: NetworkClientId[]) { + const finalNetworkClientIds = networkClientIds ?? [ + ...this.#trackingMap.keys(), + ]; + + finalNetworkClientIds.forEach((networkClientId) => { this.#trackingMap.get(networkClientId)?.incomingTransactionHelper.start(); }); } - stopIncomingTransactionPolling(networkClientIds: NetworkClientId[] = []) { - networkClientIds.forEach((networkClientId) => { + stopIncomingTransactionPolling(networkClientIds?: NetworkClientId[]) { + const finalNetworkClientIds = networkClientIds ?? [ + ...this.#trackingMap.keys(), + ]; + + finalNetworkClientIds.forEach((networkClientId) => { this.#trackingMap.get(networkClientId)?.incomingTransactionHelper.stop(); }); } diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index dc4e12d8c7..461903e3cd 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -79,6 +79,8 @@ export class PendingTransactionTracker { // eslint-disable-next-line @typescript-eslint/no-explicit-any #listener: any; + #log: debug.Debugger; + #getGlobalLock: () => Promise<() => void>; #publishTransaction: ( @@ -128,6 +130,7 @@ export class PendingTransactionTracker { this.#getTransactions = getTransactions; this.#isResubmitEnabled = isResubmitEnabled ?? (() => true); this.#listener = this.#onLatestBlock.bind(this); + this.#log = createModuleLogger(log, getChainId()); this.#getGlobalLock = getGlobalLock; this.#publishTransaction = publishTransaction; this.#running = false; @@ -158,7 +161,7 @@ export class PendingTransactionTracker { await this.#checkTransaction(txMeta); } catch (error) { /* istanbul ignore next */ - log('Failed to check transaction', error); + this.#log('Failed to check transaction', error); } finally { releaseLock(); } @@ -172,7 +175,7 @@ export class PendingTransactionTracker { this.#blockTracker.on('latest', this.#listener); this.#running = true; - log('Started polling'); + this.#log('Started polling'); } stop() { @@ -183,7 +186,7 @@ export class PendingTransactionTracker { this.#blockTracker.removeListener('latest', this.#listener); this.#running = false; - log('Stopped polling'); + this.#log('Stopped polling'); } async #onLatestBlock(latestBlockNumber: string) { @@ -193,7 +196,7 @@ export class PendingTransactionTracker { await this.#checkTransactions(); } catch (error) { /* istanbul ignore next */ - log('Failed to check transactions', error); + this.#log('Failed to check transactions', error); } finally { releaseLock(); } @@ -202,21 +205,21 @@ export class PendingTransactionTracker { await this.#resubmitTransactions(latestBlockNumber); } catch (error) { /* istanbul ignore next */ - log('Failed to resubmit transactions', error); + this.#log('Failed to resubmit transactions', error); } } async #checkTransactions() { - log('Checking transactions'); + this.#log('Checking transactions'); const pendingTransactions = this.#getPendingTransactions(); if (!pendingTransactions.length) { - log('No pending transactions to check'); + this.#log('No pending transactions to check'); return; } - log('Found pending transactions to check', { + this.#log('Found pending transactions to check', { count: pendingTransactions.length, ids: pendingTransactions.map((tx) => tx.id), }); @@ -231,16 +234,16 @@ export class PendingTransactionTracker { return; } - log('Resubmitting transactions'); + this.#log('Resubmitting transactions'); const pendingTransactions = this.#getPendingTransactions(); if (!pendingTransactions.length) { - log('No pending transactions to resubmit'); + this.#log('No pending transactions to resubmit'); return; } - log('Found pending transactions to resubmit', { + this.#log('Found pending transactions to resubmit', { count: pendingTransactions.length, ids: pendingTransactions.map((tx) => tx.id), }); @@ -258,7 +261,7 @@ export class PendingTransactionTracker { String(error); if (this.#isKnownTransactionError(errorMessage)) { - log('Ignoring known transaction error', errorMessage); + this.#log('Ignoring known transaction error', errorMessage); continue; } @@ -346,7 +349,7 @@ export class PendingTransactionTracker { } if (this.#isNonceTaken(txMeta)) { - log('Nonce already taken', id); + this.#log('Nonce already taken', id); this.#dropTransaction(txMeta); return; } @@ -357,7 +360,7 @@ export class PendingTransactionTracker { const isFailure = receipt?.status === RECEIPT_STATUS_FAILURE; if (isFailure) { - log('Transaction receipt has failed status'); + this.#log('Transaction receipt has failed status'); this.#failTransaction( txMeta, @@ -381,7 +384,7 @@ export class PendingTransactionTracker { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { - log('Failed to check transaction', id, error); + this.#log('Failed to check transaction', id, error); this.#warnTransaction( txMeta, @@ -404,7 +407,7 @@ export class PendingTransactionTracker { const { id } = txMeta; const { blockHash } = receipt; - log('Transaction confirmed', id); + this.#log('Transaction confirmed', id); const { baseFeePerGas, timestamp: blockTimestamp } = await this.#getBlockByHash(blockHash, false); @@ -456,12 +459,12 @@ export class PendingTransactionTracker { } if (droppedBlockCount < DROPPED_BLOCK_COUNT) { - log('Incrementing dropped block count', { id, droppedBlockCount }); + this.#log('Incrementing dropped block count', { id, droppedBlockCount }); this.#droppedBlockCountByHash.set(hash, droppedBlockCount + 1); return false; } - log('Hit dropped block count', id); + this.#log('Hit dropped block count', id); this.#droppedBlockCountByHash.delete(hash); return true; @@ -500,12 +503,12 @@ export class PendingTransactionTracker { } #failTransaction(txMeta: TransactionMeta, error: Error) { - log('Transaction failed', txMeta.id, error); + this.#log('Transaction failed', txMeta.id, error); this.hub.emit('transaction-failed', txMeta, error); } #dropTransaction(txMeta: TransactionMeta) { - log('Transaction dropped', txMeta.id); + this.#log('Transaction dropped', txMeta.id); this.hub.emit('transaction-dropped', txMeta); } diff --git a/packages/transaction-controller/src/utils/etherscan.ts b/packages/transaction-controller/src/utils/etherscan.ts index 703871f499..6ef829e323 100644 --- a/packages/transaction-controller/src/utils/etherscan.ts +++ b/packages/transaction-controller/src/utils/etherscan.ts @@ -2,7 +2,9 @@ import { handleFetch } from '@metamask/controller-utils'; import type { Hex } from '@metamask/utils'; import { ETHERSCAN_SUPPORTED_NETWORKS } from '../constants'; -import { incomingTransactionsLogger as log } from '../logger'; +import { createModuleLogger, projectLogger } from '../logger'; + +const log = createModuleLogger(projectLogger, 'etherscan'); // This interface was created before this ESLint rule was added. // Convert to a `type` in a future major version. @@ -174,7 +176,7 @@ async function fetchTransactions< apikey: apiKey, }); - log('Sending Etherscan request', etherscanTxUrl); + log('Sending request', etherscanTxUrl); const response = (await handleFetch( etherscanTxUrl, From 038f931af425468b030f4423e2819d929d9d6091 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 13 Nov 2024 20:51:32 +0000 Subject: [PATCH 04/12] Fix linting --- packages/transaction-controller/src/TransactionController.ts | 4 +++- .../src/TransactionControllerIntegration.test.ts | 4 +++- .../src/helpers/MultichainTrackingHelper.test.ts | 4 +++- .../src/helpers/MultichainTrackingHelper.ts | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 7eed6a559d..5ff3208355 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -962,7 +962,9 @@ export class TransactionController extends BaseController< txParams = normalizeTransactionParams(txParams); if (!this.#multichainTrackingHelper.has(networkClientId)) { - throw new Error(`Network client not found - ${networkClientId}`); + throw new Error( + `Network client not found - ${networkClientId as string}`, + ); } const isEIP1559Compatible = await this.getEIP1559Compatibility( diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 1f0766b5db..c181a06bf1 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -1107,7 +1107,9 @@ describe('TransactionController Integration', () => { }, ), ).rejects.toThrow( - `Network client not found - ${networkConfiguration.rpcEndpoints[0].networkClientId}`, + `Network client not found - ${ + networkConfiguration.rpcEndpoints[0].networkClientId as string + }`, ); expect(transactionController).toBeDefined(); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts index 524fb06700..596eb0a6dd 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts @@ -524,7 +524,9 @@ describe('MultichainTrackingHelper', () => { await expect( helper.getNonceLock('0xdeadbeef', 'mainnet'), ).rejects.toThrow( - `Missing nonce tracker for network client ID - ${NetworkType.mainnet}`, + `Missing nonce tracker for network client ID - ${ + NetworkType.mainnet as string + }`, ); }); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts index 92ea1665ca..f1512d97fd 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts @@ -217,7 +217,9 @@ export class MultichainTrackingHelper { if (!nonceTracker) { throw new Error( - `Missing nonce tracker for network client ID - ${networkClientId}`, + `Missing nonce tracker for network client ID - ${ + networkClientId as string + }`, ); } From e6757fdbe4bfa30ed8c1f8c2ef88d6a64c6c28f1 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 13 Nov 2024 21:03:34 +0000 Subject: [PATCH 05/12] Fix coverage --- packages/transaction-controller/jest.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index d84ee83366..a9c537a4fe 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 93.74, - functions: 97.51, - lines: 98.34, - statements: 98.35, + branches: 93.38, + functions: 97.72, + lines: 98.23, + statements: 98.24, }, }, From 9a80297679457b9a2c43f7ddd1a8541d31b804c1 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 15 Nov 2024 20:27:20 +0000 Subject: [PATCH 06/12] Add MethodDataHelper --- .../transaction-controller/jest.config.js | 8 +- .../src/TransactionController.test.ts | 137 +++++---------- .../src/TransactionController.ts | 39 +++-- .../src/helpers/MethodDataHelper.test.ts | 158 ++++++++++++++++++ .../src/helpers/MethodDataHelper.ts | 99 +++++++++++ 5 files changed, 327 insertions(+), 114 deletions(-) create mode 100644 packages/transaction-controller/src/helpers/MethodDataHelper.test.ts create mode 100644 packages/transaction-controller/src/helpers/MethodDataHelper.ts diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index a9c537a4fe..2fff8bcbab 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 93.38, - functions: 97.72, - lines: 98.23, - statements: 98.24, + branches: 93.41, + functions: 97.51, + lines: 97.51, + statements: 98.23, }, }, diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index d704409581..e6b8939584 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -10,7 +10,6 @@ import { ChainId, NetworkType, toHex, - BUILT_IN_NETWORKS, ORIGIN_METAMASK, InfuraNetworkType, } from '@metamask/controller-utils'; @@ -27,7 +26,6 @@ import type { Provider, } from '@metamask/network-controller'; import { - NetworkClientType, NetworkStatus, getDefaultNetworkControllerState, } from '@metamask/network-controller'; @@ -40,7 +38,6 @@ import * as uuidModule from 'uuid'; import { FakeBlockTracker } from '../../../tests/fake-block-tracker'; import { FakeProvider } from '../../../tests/fake-provider'; import { flushPromises } from '../../../tests/helpers'; -import { mockNetwork } from '../../../tests/mock-network'; import { buildCustomNetworkClientConfiguration, buildMockGetNetworkClientById, @@ -51,11 +48,13 @@ import { LineaGasFeeFlow } from './gas-flows/LineaGasFeeFlow'; import { TestGasFeeFlow } from './gas-flows/TestGasFeeFlow'; import { GasFeePoller } from './helpers/GasFeePoller'; import { IncomingTransactionHelper } from './helpers/IncomingTransactionHelper'; +import { MethodDataHelper } from './helpers/MethodDataHelper'; import { MultichainTrackingHelper } from './helpers/MultichainTrackingHelper'; import { PendingTransactionTracker } from './helpers/PendingTransactionTracker'; import type { AllowedActions, AllowedEvents, + MethodData, TransactionControllerActions, TransactionControllerEvents, TransactionControllerOptions, @@ -108,6 +107,7 @@ jest.mock('./gas-flows/LineaGasFeeFlow'); jest.mock('./gas-flows/TestGasFeeFlow'); jest.mock('./helpers/GasFeePoller'); jest.mock('./helpers/IncomingTransactionHelper'); +jest.mock('./helpers/MethodDataHelper'); jest.mock('./helpers/MultichainTrackingHelper'); jest.mock('./helpers/PendingTransactionTracker'); jest.mock('./utils/gas'); @@ -347,23 +347,6 @@ const MOCK_NETWORK: MockNetwork = { subscribe: () => undefined, }; -const MOCK_MAINNET_NETWORK: MockNetwork = { - chainId: ChainId.mainnet, - provider: HTTP_PROVIDERS.mainnet, - blockTracker: buildMockBlockTracker('0x102833C', HTTP_PROVIDERS.mainnet), - state: { - selectedNetworkClientId: NetworkType.mainnet, - networksMetadata: { - [NetworkType.mainnet]: { - EIPS: { 1559: false }, - status: NetworkStatus.Available, - }, - }, - networkConfigurationsByChainId: {}, - }, - subscribe: () => undefined, -}; - const MOCK_LINEA_MAINNET_NETWORK: MockNetwork = { chainId: ChainId['linea-mainnet'], provider: HTTP_PROVIDERS.linea, @@ -467,6 +450,14 @@ const GAS_FEE_ESTIMATES_MOCK: GasFeeFlowResponse = { }, }; +const METHOD_DATA_MOCK: MethodData = { + registryMethod: 'testMethod(uint256,uint256)', + parsedRegistryMethod: { + name: 'testMethod', + args: [{ type: 'uint256' }, { type: 'uint256' }], + }, +}; + describe('TransactionController', () => { const uuidModuleMock = jest.mocked(uuidModule); const EthQueryMock = jest.mocked(EthQuery); @@ -488,6 +479,7 @@ describe('TransactionController', () => { ); const getGasFeeFlowMock = jest.mocked(getGasFeeFlow); const shouldResimulateMock = jest.mocked(shouldResimulate); + const methodDataHelperClassMock = jest.mocked(MethodDataHelper); let mockEthQuery: EthQuery; let getNonceLockSpy: jest.Mock; @@ -498,6 +490,7 @@ describe('TransactionController', () => { let lineaGasFeeFlowMock: jest.Mocked; let testGasFeeFlowMock: jest.Mocked; let gasFeePollerMock: jest.Mocked; + let methodDataHelperMock: jest.Mocked; let timeCounter = 0; const incomingTransactionHelperClassMock = @@ -876,6 +869,16 @@ describe('TransactionController', () => { return gasFeePollerMock; }); + methodDataHelperClassMock.mockImplementation(() => { + methodDataHelperMock = { + lookup: jest.fn(), + hub: { + on: jest.fn(), + }, + } as unknown as jest.Mocked; + return methodDataHelperMock; + }); + updateSwapsTransactionMock.mockImplementation( (transactionMeta) => transactionMeta, ); @@ -2529,87 +2532,31 @@ describe('TransactionController', () => { }); }); - describe.skip('handleMethodData', () => { - it('loads method data from registry', async () => { - const { controller } = setupController({ network: MOCK_MAINNET_NETWORK }); - mockNetwork({ - networkClientConfiguration: { - chainId: BUILT_IN_NETWORKS.mainnet.chainId, - ticker: BUILT_IN_NETWORKS.mainnet.ticker, - type: NetworkClientType.Infura, - network: 'mainnet', - infuraProjectId: INFURA_PROJECT_ID, - }, - mocks: [ - { - request: { - method: 'eth_call', - params: [ - { - to: '0x44691B39d1a75dC4E0A0346CBB15E310e6ED1E86', - data: '0xb46bcdaaf39b5b9b00000000000000000000000000000000000000000000000000000000', - }, - 'latest', - ], - }, - response: { - result: - '0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000024657468546f546f6b656e53776170496e7075742875696e743235362c75696e743235362900000000000000000000000000000000000000000000000000000000', - }, - }, - ], - }); - const registry = await controller.handleMethodData('0xf39b5b9b'); + describe('handleMethodData', () => { + it('invokes helper', async () => { + const { controller } = setupController(); - expect(registry.parsedRegistryMethod).toStrictEqual({ - args: [{ type: 'uint256' }, { type: 'uint256' }], - name: 'Eth To Token Swap Input', - }); - expect(registry.registryMethod).toBe( - 'ethToTokenSwapInput(uint256,uint256)', - ); - }); + methodDataHelperMock.lookup.mockResolvedValueOnce(METHOD_DATA_MOCK); - it('skips reading registry if already cached in state', async () => { - const { controller } = setupController({ network: MOCK_MAINNET_NETWORK }); - mockNetwork({ - networkClientConfiguration: { - ticker: BUILT_IN_NETWORKS.mainnet.ticker, - chainId: BUILT_IN_NETWORKS.mainnet.chainId, - type: NetworkClientType.Infura, - network: 'mainnet', - infuraProjectId: INFURA_PROJECT_ID, - }, - mocks: [ - { - request: { - method: 'eth_call', - params: [ - { - to: '0x44691B39d1a75dC4E0A0346CBB15E310e6ED1E86', - data: '0xb46bcdaaf39b5b9b00000000000000000000000000000000000000000000000000000000', - }, - 'latest', - ], - }, - response: { - result: - '0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000024657468546f546f6b656e53776170496e7075742875696e743235362c75696e743235362900000000000000000000000000000000000000000000000000000000', - }, - }, - ], - }); + const result = await controller.handleMethodData( + 'mockMethodData', + NETWORK_CLIENT_ID_MOCK, + ); - await controller.handleMethodData('0xf39b5b9b'); + expect(result).toStrictEqual(METHOD_DATA_MOCK); + }); - const registryLookup = jest.spyOn( - controller, - 'registryLookup' as never, - ); + it('updates state when helper emits update event', async () => { + const { controller } = setupController(); - await controller.handleMethodData('0xf39b5b9b'); + jest.mocked(methodDataHelperMock.hub.on).mock.calls[0][1]({ + fourBytePrefix: '0x12345678', + methodData: METHOD_DATA_MOCK, + }); - expect(registryLookup).not.toHaveBeenCalled(); + expect(controller.state.methodData).toStrictEqual({ + '0x12345678': METHOD_DATA_MOCK, + }); }); }); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 5ff3208355..d80beb8c43 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -59,6 +59,7 @@ import type { EtherscanRemoteTransactionSource } from './helpers/EtherscanRemote import { GasFeePoller } from './helpers/GasFeePoller'; import type { IncomingTransactionOptions } from './helpers/IncomingTransactionHelper'; import { IncomingTransactionHelper } from './helpers/IncomingTransactionHelper'; +import { MethodDataHelper } from './helpers/MethodDataHelper'; import { MultichainTrackingHelper } from './helpers/MultichainTrackingHelper'; import { PendingTransactionTracker } from './helpers/PendingTransactionTracker'; import { projectLogger as log } from './logger'; @@ -579,6 +580,8 @@ export class TransactionController extends BaseController< private readonly approvingTransactionIds: Set = new Set(); + #methodDataHelper: MethodDataHelper; + private readonly mutex = new Mutex(); private readonly gasFeeFlows: GasFeeFlow[]; @@ -697,18 +700,6 @@ export class TransactionController extends BaseController< ); } - // 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; /** @@ -874,6 +865,20 @@ export class TransactionController extends BaseController< this.#onGasFeePollerTransactionUpdate.bind(this), ); + this.#methodDataHelper = new MethodDataHelper({ + getProvider: (networkClientId) => this.#getProvider({ networkClientId }), + getState: () => this.state.methodData, + }); + + this.#methodDataHelper.hub.on( + 'update', + ({ fourBytePrefix, methodData }) => { + this.update((_state) => { + _state.methodData[fourBytePrefix] = methodData; + }); + }, + ); + // when transactionsController state changes // check for pending transactions and start polling if there are any this.messagingSystem.subscribe( @@ -895,11 +900,15 @@ export class TransactionController extends BaseController< /** * Handle new method data request. * - * @param _fourBytePrefix - The method prefix. + * @param fourBytePrefix - The method prefix. + * @param networkClientId - The ID of the network client used to fetch the method data. * @returns The method data object corresponding to the given signature prefix. */ - async handleMethodData(_fourBytePrefix: string): Promise { - throw new Error('Method not implemented.'); + async handleMethodData( + fourBytePrefix: string, + networkClientId: NetworkClientId, + ): Promise { + return this.#methodDataHelper.lookup(fourBytePrefix, networkClientId); } /** diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts new file mode 100644 index 0000000000..934819c122 --- /dev/null +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts @@ -0,0 +1,158 @@ +import { MethodRegistry } from 'eth-method-registry'; + +import type { MethodData } from '../TransactionController'; +import { MethodDataHelper } from './MethodDataHelper'; + +jest.mock('eth-method-registry'); + +const FOUR_BYTE_PREFIX_MOCK = '0x12345678'; +const NETWORK_CLIENT_ID_MOCK = 'testNetworkClientId'; +const SIGNATURE_MOCK = 'testMethod(uint256,uint256)'; + +const METHOD_DATA_MOCK: MethodData = { + registryMethod: SIGNATURE_MOCK, + parsedRegistryMethod: { + name: 'testMethod', + args: [{ type: 'uint256' }, { type: 'uint256' }], + }, +}; + +/** + * Creates a mock MethodRegistry instance. + * @returns The mocked MethodRegistry instance. + */ +function createMethodRegistryMock() { + return { + lookup: jest.fn(), + parse: jest.fn(), + } as unknown as jest.Mocked; +} + +describe('MethodDataHelper', () => { + const methodRegistryClassMock = jest.mocked(MethodRegistry); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('lookup', () => { + it('returns method data from cache', async () => { + const methodDataHelper = new MethodDataHelper({ + getProvider: jest.fn(), + getState: () => ({ [FOUR_BYTE_PREFIX_MOCK]: METHOD_DATA_MOCK }), + }); + + const result = await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + expect(result).toStrictEqual(METHOD_DATA_MOCK); + }); + + it('returns method data from registry lookup', async () => { + const methodRegistryMock = createMethodRegistryMock(); + methodRegistryMock.lookup.mockResolvedValueOnce(SIGNATURE_MOCK); + methodRegistryMock.parse.mockReturnValueOnce( + METHOD_DATA_MOCK.parsedRegistryMethod, + ); + + methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + + const methodDataHelper = new MethodDataHelper({ + getProvider: jest.fn(), + getState: () => ({}), + }); + + const result = await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + expect(result).toStrictEqual(METHOD_DATA_MOCK); + }); + + it('returns empty method data if not found in registry', async () => { + const methodRegistryMock = createMethodRegistryMock(); + methodRegistryMock.lookup.mockResolvedValueOnce(undefined); + + methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + + const methodDataHelper = new MethodDataHelper({ + getProvider: jest.fn(), + getState: () => ({}), + }); + + const result = await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + expect(result).toStrictEqual({ + registryMethod: '', + parsedRegistryMethod: { name: undefined, args: undefined }, + }); + }); + + it('creates registry instance for each unique network client ID', async () => { + const getProviderMock = jest.fn(); + + const methodRegistryMock = createMethodRegistryMock(); + methodRegistryMock.lookup.mockResolvedValueOnce(undefined); + + methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + + const methodDataHelper = new MethodDataHelper({ + getProvider: getProviderMock, + getState: () => ({}), + }); + + await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + 'anotherNetworkClientId', + ); + + expect(methodRegistryClassMock).toHaveBeenCalledTimes(2); + expect(getProviderMock).toHaveBeenCalledTimes(2); + }); + + it('emits event when method data is fetched', async () => { + const methodRegistryMock = createMethodRegistryMock(); + methodRegistryMock.lookup.mockResolvedValueOnce(SIGNATURE_MOCK); + methodRegistryMock.parse.mockReturnValueOnce( + METHOD_DATA_MOCK.parsedRegistryMethod, + ); + + methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + + const methodDataHelper = new MethodDataHelper({ + getProvider: jest.fn(), + getState: () => ({}), + }); + + const updateListener = jest.fn(); + methodDataHelper.hub.on('update', updateListener); + + await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + expect(updateListener).toHaveBeenCalledTimes(1); + expect(updateListener).toHaveBeenCalledWith({ + fourBytePrefix: FOUR_BYTE_PREFIX_MOCK, + methodData: METHOD_DATA_MOCK, + }); + }); + }); +}); diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.ts new file mode 100644 index 0000000000..864aee8c2c --- /dev/null +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.ts @@ -0,0 +1,99 @@ +import type { NetworkClientId, Provider } from '@metamask/network-controller'; +import { createModuleLogger } from '@metamask/utils'; +import { Mutex } from 'async-mutex'; +import { MethodRegistry } from 'eth-method-registry'; +import EventEmitter from 'events'; + +import { projectLogger } from '../logger'; +import type { MethodData } from '../TransactionController'; + +const log = createModuleLogger(projectLogger, 'method-data'); + +export class MethodDataHelper { + hub: EventEmitter; + + #getProvider: (networkClientId: NetworkClientId) => Provider; + + #getState: () => Record; + + #methodRegistryByNetworkClientId: Map; + + #mutex = new Mutex(); + + constructor({ + getProvider, + getState, + }: { + getProvider: (networkClientId: NetworkClientId) => Provider; + getState: () => Record; + }) { + this.hub = new EventEmitter(); + + this.#getProvider = getProvider; + this.#getState = getState; + this.#methodRegistryByNetworkClientId = new Map(); + } + + async lookup( + fourBytePrefix: string, + networkClientId: NetworkClientId, + ): Promise { + log('lookup', fourBytePrefix, networkClientId); + + const releaseLock = await this.#mutex.acquire(); + + try { + const cachedResult = this.#getState()[fourBytePrefix]; + + if (cachedResult) { + log('Cached', cachedResult); + return cachedResult; + } + + let registry = this.#methodRegistryByNetworkClientId.get(networkClientId); + + if (!registry) { + const provider = this.#getProvider(networkClientId); + + // @ts-expect-error Type in eth-method-registry is inappropriate and should be changed + registry = new MethodRegistry({ provider }); + + this.#methodRegistryByNetworkClientId.set(networkClientId, registry); + + log('Created registry', networkClientId); + } + + const methodData = await this.#registryLookup(fourBytePrefix, registry); + + log('Result', methodData); + + this.hub.emit('update', { fourBytePrefix, methodData }); + + return methodData; + } finally { + releaseLock(); + } + } + + async #registryLookup( + fourBytePrefix: string, + methodRegistry: MethodRegistry, + ): Promise { + const registryMethod = await methodRegistry.lookup(fourBytePrefix); + + if (!registryMethod) { + log('No method found', fourBytePrefix); + + return { + registryMethod: '', + parsedRegistryMethod: { name: undefined, args: undefined }, + }; + } + + log('Parsing', registryMethod); + + const parsedRegistryMethod = methodRegistry.parse(registryMethod); + + return { registryMethod, parsedRegistryMethod }; + } +} From 61aae4b51c90488844209914e38c6d88f9f2df0c Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 15 Nov 2024 20:40:13 +0000 Subject: [PATCH 07/12] Update changelog --- packages/transaction-controller/CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index fd8065a8f2..1c3bd17fca 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **BREAKING:** Remove global network usage ([#4920](https://github.com/MetaMask/core/pull/4920)) + - Add required `networkClientId` argument to `handleMethodData` method. + +### Changed + +- **BREAKING:** Remove global network usage ([#4920](https://github.com/MetaMask/core/pull/4920)) + - Require `networkClientId` option in `addTransaction` method. + - Require `networkClientId` property in `TransactionMeta` type. + - Change `wipeTransactions` method arguments to optional object containing `address` and `chainId` properties. + - Require `networkClientId` argument in `estimateGas`, `estimateGasBuffered` and `getNonceLock` methods. + +### Removed + +- **BREAKING:** Remove global network usage ([#4920](https://github.com/MetaMask/core/pull/4920)) + - Remove the `blockTracker`, `isMultichainEnabled`, `onNetworkStateChange` and `provider` constructor options. + - Remove `filterToCurrentNetwork` option from `getTransactions` method. + ## [39.0.0] ### Changed From 49553b36d282f34ffa662a64ecebf840392a2c44 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 15 Nov 2024 21:14:36 +0000 Subject: [PATCH 08/12] Fix remote transaction sources --- .../src/TransactionController.ts | 25 +++++-- .../helpers/MultichainTrackingHelper.test.ts | 20 ++++-- .../src/helpers/MultichainTrackingHelper.ts | 65 +++++++------------ 3 files changed, 59 insertions(+), 51 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index d80beb8c43..4e43e85d75 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -55,7 +55,7 @@ import { LineaGasFeeFlow } from './gas-flows/LineaGasFeeFlow'; import { OptimismLayer1GasFeeFlow } from './gas-flows/OptimismLayer1GasFeeFlow'; import { ScrollLayer1GasFeeFlow } from './gas-flows/ScrollLayer1GasFeeFlow'; import { TestGasFeeFlow } from './gas-flows/TestGasFeeFlow'; -import type { EtherscanRemoteTransactionSource } from './helpers/EtherscanRemoteTransactionSource'; +import { EtherscanRemoteTransactionSource } from './helpers/EtherscanRemoteTransactionSource'; import { GasFeePoller } from './helpers/GasFeePoller'; import type { IncomingTransactionOptions } from './helpers/IncomingTransactionHelper'; import { IncomingTransactionHelper } from './helpers/IncomingTransactionHelper'; @@ -81,6 +81,7 @@ import type { GasPriceValue, FeeMarketEIP1559Values, SubmitHistoryEntry, + RemoteTransactionSource, } from './types'; import { TransactionEnvelopeType, @@ -611,7 +612,9 @@ export class TransactionController extends BaseController< private readonly layer1GasFeeFlows: Layer1GasFeeFlow[]; - readonly #incomingTransactionOptions: IncomingTransactionOptions; + readonly #incomingTransactionOptions: IncomingTransactionOptions & { + etherscanApiKeysByChainId?: Record; + }; private readonly securityProviderRequest?: SecurityProviderRequest; @@ -815,7 +818,6 @@ export class TransactionController extends BaseController< }; this.#multichainTrackingHelper = new MultichainTrackingHelper({ - incomingTransactionOptions: incomingTransactions, findNetworkClientIdByChainId, getNetworkClientById: ((networkClientId: NetworkClientId) => { return this.messagingSystem.call( @@ -833,6 +835,8 @@ export class TransactionController extends BaseController< this.#createIncomingTransactionHelper.bind(this), createPendingTransactionTracker: this.#createPendingTransactionTracker.bind(this), + createRemoteTransactionSource: + this.#createRemoteTransactionSource.bind(this), onNetworkStateChange: (listener) => { this.messagingSystem.subscribe( 'NetworkController:stateChange', @@ -3261,13 +3265,22 @@ export class TransactionController extends BaseController< }); } + #createRemoteTransactionSource(): RemoteTransactionSource { + return new EtherscanRemoteTransactionSource({ + apiKeysByChainId: + this.#incomingTransactionOptions.etherscanApiKeysByChainId, + includeTokenTransfers: + this.#incomingTransactionOptions.includeTokenTransfers, + }); + } + #createIncomingTransactionHelper({ blockTracker, - etherscanRemoteTransactionSource, + remoteTransactionSource, chainId, }: { blockTracker: BlockTracker; - etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; + remoteTransactionSource: RemoteTransactionSource; chainId: Hex; }): IncomingTransactionHelper { const incomingTransactionHelper = new IncomingTransactionHelper({ @@ -3278,7 +3291,7 @@ export class TransactionController extends BaseController< getChainId: () => chainId, isEnabled: this.#incomingTransactionOptions.isEnabled, queryEntireHistory: this.#incomingTransactionOptions.queryEntireHistory, - remoteTransactionSource: etherscanRemoteTransactionSource, + remoteTransactionSource, transactionLimit: this.#transactionHistoryLimit, updateTransactions: this.#incomingTransactionOptions.updateTransactions, }); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts index 596eb0a6dd..e44423446b 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts @@ -1,4 +1,3 @@ -/* eslint-disable jsdoc/require-jsdoc */ import { ChainId, NetworkType } from '@metamask/controller-utils'; import type { NetworkClientId, Provider } from '@metamask/network-controller'; import type { NonceTracker } from '@metamask/nonce-tracker'; @@ -6,7 +5,7 @@ import type { Hex } from '@metamask/utils'; import { useFakeTimers } from 'sinon'; import { advanceTime } from '../../../../tests/helpers'; -import { EtherscanRemoteTransactionSource } from './EtherscanRemoteTransactionSource'; +import type { RemoteTransactionSource } from '../types'; import type { IncomingTransactionHelper } from './IncomingTransactionHelper'; import { MultichainTrackingHelper } from './MultichainTrackingHelper'; import type { PendingTransactionTracker } from './PendingTransactionTracker'; @@ -19,12 +18,22 @@ jest.mock( }, ); +/** + * Build a mock provider object. + * @param networkClientId - The network client ID to use for the mock provider. + * @returns The mock provider object. + */ function buildMockProvider(networkClientId: NetworkClientId) { return { mockProvider: networkClientId, }; } +/** + * Build a mock block tracker object. + * @param networkClientId - The network client ID to use for the mock block tracker. + * @returns The mock block tracker object. + */ function buildMockBlockTracker(networkClientId: NetworkClientId) { return { mockBlockTracker: networkClientId, @@ -181,6 +190,8 @@ function newMultichainTrackingHelper( mockPendingTransactionTrackers[chainId] = mockPendingTransactionTracker; return mockPendingTransactionTracker; }); + const mockCreateRemoteTransactionSource = () => + ({} as RemoteTransactionSource); const options = { isMultichainEnabled: true, @@ -203,6 +214,7 @@ function newMultichainTrackingHelper( createNonceTracker: mockCreateNonceTracker, createIncomingTransactionHelper: mockCreateIncomingTransactionHelper, createPendingTransactionTracker: mockCreatePendingTransactionTracker, + createRemoteTransactionSource: mockCreateRemoteTransactionSource, onNetworkStateChange: jest.fn(), ...opts, }; @@ -324,9 +336,7 @@ describe('MultichainTrackingHelper', () => { expect(options.createIncomingTransactionHelper).toHaveBeenCalledTimes(1); expect(options.createIncomingTransactionHelper).toHaveBeenCalledWith({ blockTracker: MOCK_BLOCK_TRACKERS.mainnet, - etherscanRemoteTransactionSource: expect.any( - EtherscanRemoteTransactionSource, - ), + remoteTransactionSource: expect.anything(), chainId: '0x1', }); diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts index f1512d97fd..fd2dc4e050 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts @@ -11,11 +11,8 @@ import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { createModuleLogger, projectLogger } from '../logger'; -import { EtherscanRemoteTransactionSource } from './EtherscanRemoteTransactionSource'; -import type { - IncomingTransactionHelper, - IncomingTransactionOptions, -} from './IncomingTransactionHelper'; +import type { RemoteTransactionSource } from '../types'; +import type { IncomingTransactionHelper } from './IncomingTransactionHelper'; import type { PendingTransactionTracker } from './PendingTransactionTracker'; /** @@ -28,12 +25,9 @@ type NetworkClientRegistry = ReturnType< const log = createModuleLogger(projectLogger, 'multichain-tracking'); export type MultichainTrackingHelperOptions = { - incomingTransactionOptions: IncomingTransactionOptions; - findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; getNetworkClientById: NetworkController['getNetworkClientById']; getNetworkClientRegistry: NetworkController['getNetworkClientRegistry']; - removeIncomingTransactionHelperListeners: ( IncomingTransactionHelper: IncomingTransactionHelper, ) => void; @@ -47,7 +41,7 @@ export type MultichainTrackingHelperOptions = { }) => NonceTracker; createIncomingTransactionHelper: (opts: { blockTracker: BlockTracker; - etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; + remoteTransactionSource: RemoteTransactionSource; chainId: Hex; }) => IncomingTransactionHelper; createPendingTransactionTracker: (opts: { @@ -55,6 +49,7 @@ export type MultichainTrackingHelperOptions = { blockTracker: BlockTracker; chainId: Hex; }) => PendingTransactionTracker; + createRemoteTransactionSource: () => RemoteTransactionSource; onNetworkStateChange: ( listener: ( ...payload: NetworkControllerStateChangeEvent['payload'] @@ -63,8 +58,6 @@ export type MultichainTrackingHelperOptions = { }; export class MultichainTrackingHelper { - readonly #incomingTransactionOptions: IncomingTransactionOptions; - readonly #findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; readonly #getNetworkClientById: NetworkController['getNetworkClientById']; @@ -88,7 +81,7 @@ export class MultichainTrackingHelper { readonly #createIncomingTransactionHelper: (opts: { blockTracker: BlockTracker; chainId: Hex; - etherscanRemoteTransactionSource: EtherscanRemoteTransactionSource; + remoteTransactionSource: RemoteTransactionSource; }) => IncomingTransactionHelper; readonly #createPendingTransactionTracker: (opts: { @@ -97,6 +90,8 @@ export class MultichainTrackingHelper { chainId: Hex; }) => PendingTransactionTracker; + readonly #createRemoteTransactionSource: () => RemoteTransactionSource; + readonly #nonceMutexesByChainId = new Map>(); readonly #trackingMap: Map< @@ -108,13 +103,10 @@ export class MultichainTrackingHelper { } > = new Map(); - readonly #etherscanRemoteTransactionSourcesMap: Map< - Hex, - EtherscanRemoteTransactionSource - > = new Map(); + readonly #remoteTransactionSourcesMap: Map = + new Map(); constructor({ - incomingTransactionOptions, findNetworkClientIdByChainId, getNetworkClientById, getNetworkClientRegistry, @@ -123,10 +115,9 @@ export class MultichainTrackingHelper { createNonceTracker, createIncomingTransactionHelper, createPendingTransactionTracker, + createRemoteTransactionSource, onNetworkStateChange, }: MultichainTrackingHelperOptions) { - this.#incomingTransactionOptions = incomingTransactionOptions; - this.#findNetworkClientIdByChainId = findNetworkClientIdByChainId; this.#getNetworkClientById = getNetworkClientById; this.#getNetworkClientRegistry = getNetworkClientRegistry; @@ -138,6 +129,7 @@ export class MultichainTrackingHelper { this.#createNonceTracker = createNonceTracker; this.#createIncomingTransactionHelper = createIncomingTransactionHelper; this.#createPendingTransactionTracker = createPendingTransactionTracker; + this.#createRemoteTransactionSource = createRemoteTransactionSource; onNetworkStateChange((_, patches) => { const networkClients = this.#getNetworkClientRegistry(); @@ -342,7 +334,7 @@ export class MultichainTrackingHelper { } #refreshTrackingMap = (networkClients: NetworkClientRegistry) => { - this.#refreshEtherscanRemoteTransactionSources(networkClients); + this.#refreshRemoteTransactionSources(networkClients); const networkClientIds = Object.keys(networkClients); const existingNetworkClientIds = Array.from(this.#trackingMap.keys()); @@ -401,21 +393,14 @@ export class MultichainTrackingHelper { configuration: { chainId }, } = this.#getNetworkClientById(networkClientId); - let etherscanRemoteTransactionSource = - this.#etherscanRemoteTransactionSourcesMap.get(chainId); + let remoteTransactionSource = + this.#remoteTransactionSourcesMap.get(chainId); - if (!etherscanRemoteTransactionSource) { - etherscanRemoteTransactionSource = new EtherscanRemoteTransactionSource({ - includeTokenTransfers: - this.#incomingTransactionOptions.includeTokenTransfers, - }); - - log('Created Etherscan remote transaction source', chainId); + if (!remoteTransactionSource) { + remoteTransactionSource = this.#createRemoteTransactionSource(); + this.#remoteTransactionSourcesMap.set(chainId, remoteTransactionSource); - this.#etherscanRemoteTransactionSourcesMap.set( - chainId, - etherscanRemoteTransactionSource, - ); + log('Created remote transaction source', chainId); } const nonceTracker = this.#createNonceTracker({ @@ -426,7 +411,7 @@ export class MultichainTrackingHelper { const incomingTransactionHelper = this.#createIncomingTransactionHelper({ blockTracker, - etherscanRemoteTransactionSource, + remoteTransactionSource, chainId, }); @@ -443,25 +428,25 @@ export class MultichainTrackingHelper { }); } - #refreshEtherscanRemoteTransactionSources = ( + #refreshRemoteTransactionSources = ( networkClients: NetworkClientRegistry, ) => { - // this will be prettier when we have consolidated network clients with a single chainId: - // check if there are still other network clients using the same chainId - // if not remove the etherscanRemoteTransaction source from the map const chainIdsInRegistry = new Set(); + Object.values(networkClients).forEach((networkClient) => chainIdsInRegistry.add(networkClient.configuration.chainId), ); + const existingChainIds = Array.from( - this.#etherscanRemoteTransactionSourcesMap.keys(), + this.#remoteTransactionSourcesMap.keys(), ); + const chainIdsToRemove = existingChainIds.filter( (chainId) => !chainIdsInRegistry.has(chainId), ); chainIdsToRemove.forEach((chainId) => { - this.#etherscanRemoteTransactionSourcesMap.delete(chainId); + this.#remoteTransactionSourcesMap.delete(chainId); }); }; } From 913113cfbe9aa348ef2688a65bcddb8b41c2269f Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 15 Nov 2024 21:55:31 +0000 Subject: [PATCH 09/12] Default to updating all incoming transactions --- .../src/helpers/MultichainTrackingHelper.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts index fd2dc4e050..6fc0ab8bf9 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.ts @@ -263,9 +263,13 @@ export class MultichainTrackingHelper { } } - async updateIncomingTransactions(networkClientIds: NetworkClientId[] = []) { + async updateIncomingTransactions(networkClientIds?: NetworkClientId[]) { + const finalNetworkClientIds = networkClientIds ?? [ + ...this.#trackingMap.keys(), + ]; + const promises = await Promise.allSettled( - networkClientIds.map(async (networkClientId) => { + finalNetworkClientIds.map(async (networkClientId) => { return await this.#trackingMap .get(networkClientId) ?.incomingTransactionHelper.update(); @@ -276,7 +280,7 @@ export class MultichainTrackingHelper { .filter((result) => result.status === 'rejected') .forEach((result) => { log( - 'failed to update incoming transactions', + 'Failed to update incoming transactions', (result as PromiseRejectedResult).reason, ); }); From 14d3599e8c5276edb859e392d32ced7b4b4aa4ef Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 19 Nov 2024 12:35:13 +0000 Subject: [PATCH 10/12] Add network client ID to incoming transactions --- .../src/TransactionController.test.ts | 9 ++++--- .../src/TransactionController.ts | 27 ++++++++++++------- .../EtherscanRemoteTransactionSource.test.ts | 4 +-- .../EtherscanRemoteTransactionSource.ts | 26 +++++++++--------- .../helpers/IncomingTransactionHelper.test.ts | 2 +- .../src/helpers/IncomingTransactionHelper.ts | 13 ++++----- packages/transaction-controller/src/types.ts | 4 +-- .../tests/EtherscanMocks.ts | 2 +- 8 files changed, 48 insertions(+), 39 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index e6b8939584..1c9b83026d 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -4029,8 +4029,8 @@ describe('TransactionController', () => { }); expect(controller.state.transactions).toStrictEqual([ - TRANSACTION_META_MOCK, - TRANSACTION_META_2_MOCK, + { ...TRANSACTION_META_MOCK, networkClientId: NETWORK_CLIENT_ID_MOCK }, + { ...TRANSACTION_META_2_MOCK, networkClientId: NETWORK_CLIENT_ID_MOCK }, ]); }); @@ -4045,6 +4045,7 @@ describe('TransactionController', () => { const updatedTransaction = { ...TRANSACTION_META_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, status: 'failed', }; @@ -4057,7 +4058,7 @@ describe('TransactionController', () => { expect(controller.state.transactions).toStrictEqual([ updatedTransaction, - TRANSACTION_META_2_MOCK, + { ...TRANSACTION_META_2_MOCK, networkClientId: NETWORK_CLIENT_ID_MOCK }, ]); }); @@ -4074,7 +4075,7 @@ describe('TransactionController', () => { }); expect(controller.state.transactions).toStrictEqual([ - TRANSACTION_META_2_MOCK, + { ...TRANSACTION_META_2_MOCK, networkClientId: NETWORK_CLIENT_ID_MOCK }, ]); }); }); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 4e43e85d75..04745a040a 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2853,17 +2853,23 @@ export class TransactionController extends BaseController< updated: TransactionMeta[]; }) { this.update((state) => { - const { transactions: currentTransactions } = state; - const updatedTransactions = [ - ...added, - ...currentTransactions.map((originalTransaction) => { - const updatedTransaction = updated.find( - ({ hash }) => hash === originalTransaction.hash, - ); + const { transactions } = state; - return updatedTransaction ?? originalTransaction; - }), - ]; + const existingTransactions = transactions.map( + (tx) => updated.find(({ hash }) => hash === tx.hash) ?? tx, + ); + + const updatedTransactions = [...added, ...existingTransactions].map( + (tx) => { + const { chainId } = tx; + const networkClientId = this.#getNetworkClientId({ chainId }); + + return { + ...tx, + networkClientId, + }; + }, + ); state.transactions = this.trimTransactionsForState(updatedTransactions); }); @@ -3362,6 +3368,7 @@ export class TransactionController extends BaseController< 'transactions', this.onIncomingTransactions.bind(this), ); + incomingTransactionHelper.hub.on( 'updatedLastFetchedBlockNumbers', this.onUpdatedLastFetchedBlockNumbers.bind(this), diff --git a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts index 8519fbdb2b..00d3ff6e69 100644 --- a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts +++ b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts @@ -254,7 +254,7 @@ describe('EtherscanRemoteTransactionSource', () => { [CHAIN_IDS.MAINNET]: API_KEY_MOCK, }, }).fetchTransactions({ - currentChainId: CHAIN_IDS.MAINNET, + chainId: CHAIN_IDS.MAINNET, } as unknown as RemoteTransactionSourceRequest); expect(fetchEtherscanTransactionsMock).toHaveBeenCalledTimes(1); @@ -275,7 +275,7 @@ describe('EtherscanRemoteTransactionSource', () => { [CHAIN_IDS.MAINNET]: API_KEY_MOCK, }, }).fetchTransactions({ - currentChainId: CHAIN_IDS.SEPOLIA, + chainId: CHAIN_IDS.SEPOLIA, } as unknown as RemoteTransactionSourceRequest); expect(fetchEtherscanTransactionsMock).toHaveBeenCalledTimes(1); diff --git a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts index d890a9a681..faed54c753 100644 --- a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts +++ b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.ts @@ -65,7 +65,7 @@ export class EtherscanRemoteTransactionSource ): Promise { const releaseLock = await this.#mutex.acquire(); const acquiredTime = Date.now(); - const { currentChainId: chainId } = request; + const { chainId } = request; const apiKey = this.#apiKeysByChainId?.[chainId]; if (apiKey) { @@ -111,14 +111,14 @@ export class EtherscanRemoteTransactionSource request: RemoteTransactionSourceRequest, etherscanRequest: EtherscanTransactionRequest, ) => { - const { currentChainId } = request; + const { chainId } = request; const etherscanTransactions = await fetchEtherscanTransactions( etherscanRequest, ); return this.#getResponseTransactions(etherscanTransactions).map((tx) => - this.#normalizeTransaction(tx, currentChainId), + this.#normalizeTransaction(tx, chainId), ); }; @@ -126,14 +126,14 @@ export class EtherscanRemoteTransactionSource request: RemoteTransactionSourceRequest, etherscanRequest: EtherscanTransactionRequest, ) => { - const { currentChainId } = request; + const { chainId } = request; const etherscanTransactions = await fetchEtherscanTokenTransactions( etherscanRequest, ); return this.#getResponseTransactions(etherscanTransactions).map((tx) => - this.#normalizeTokenTransaction(tx, currentChainId), + this.#normalizeTokenTransaction(tx, chainId), ); }; @@ -160,9 +160,9 @@ export class EtherscanRemoteTransactionSource #normalizeTransaction( txMeta: EtherscanTransactionMeta, - currentChainId: Hex, + chainId: Hex, ): TransactionMeta { - const base = this.#normalizeTransactionBase(txMeta, currentChainId); + const base = this.#normalizeTransactionBase(txMeta, chainId); return { ...base, @@ -181,9 +181,9 @@ export class EtherscanRemoteTransactionSource #normalizeTokenTransaction( txMeta: EtherscanTokenTransactionMeta, - currentChainId: Hex, + chainId: Hex, ): TransactionMeta { - const base = this.#normalizeTransactionBase(txMeta, currentChainId); + const base = this.#normalizeTransactionBase(txMeta, chainId); return { ...base, @@ -198,20 +198,20 @@ export class EtherscanRemoteTransactionSource #normalizeTransactionBase( txMeta: EtherscanTransactionMetaBase, - currentChainId: Hex, + chainId: Hex, ): TransactionMeta { const time = parseInt(txMeta.timeStamp, 10) * 1000; return { blockNumber: txMeta.blockNumber, - chainId: currentChainId, + chainId, hash: txMeta.hash, id: random({ msecs: time }), - networkClientId: 'incoming', + networkClientId: '', status: TransactionStatus.confirmed, time, txParams: { - chainId: currentChainId, + chainId, from: txMeta.from, gas: BNToHex(new BN(txMeta.gas)), gasPrice: BNToHex(new BN(txMeta.gasPrice)), diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts index 837a561210..a5250d33c4 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts @@ -159,7 +159,7 @@ describe('IncomingTransactionHelper', () => { expect(remoteTransactionSource.fetchTransactions).toHaveBeenCalledWith({ address: ADDRESS_MOCK, - currentChainId: CHAIN_ID_MOCK, + chainId: CHAIN_ID_MOCK, fromBlock: undefined, limit: CONTROLLER_ARGS_MOCK.transactionLimit, }); diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index 016ba2ebeb..e1ce6f0d07 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -168,7 +168,7 @@ export class IncomingTransactionHelper { const fromBlock = this.#getFromBlock(latestBlockNumber); const account = this.#getCurrentAccount(); - const currentChainId = this.#getChainId(); + const chainId = this.#getChainId(); let remoteTransactions = []; @@ -176,7 +176,7 @@ export class IncomingTransactionHelper { remoteTransactions = await this.#remoteTransactionSource.fetchTransactions({ address: account.address, - currentChainId, + chainId, fromBlock, limit: this.#transactionLimit, }); @@ -186,6 +186,7 @@ export class IncomingTransactionHelper { this.#log('Error while fetching remote transactions', error); return; } + if (!this.#updateTransactions) { const address = account.address.toLowerCase(); remoteTransactions = remoteTransactions.filter( @@ -324,18 +325,18 @@ export class IncomingTransactionHelper { } #getBlockNumberKey(additionalKeys: string[]): string { - const currentChainId = this.#getChainId(); + const chainId = this.#getChainId(); const currentAccount = this.#getCurrentAccount()?.address.toLowerCase(); - return [currentChainId, currentAccount, ...additionalKeys].join('#'); + return [chainId, currentAccount, ...additionalKeys].join('#'); } #canStart(): boolean { const isEnabled = this.#isEnabled(); - const currentChainId = this.#getChainId(); + const chainId = this.#getChainId(); const isSupportedNetwork = - this.#remoteTransactionSource.isSupportedNetwork(currentChainId); + this.#remoteTransactionSource.isSupportedNetwork(chainId); return isEnabled && isSupportedNetwork; } diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index fe8b3f3bc9..dda87ea640 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -862,9 +862,9 @@ export interface RemoteTransactionSourceRequest { address: string; /** - * The chainId of the current network. + * The ID of the chain to query transactions for. */ - currentChainId: Hex; + chainId: Hex; /** * Block number to start fetching transactions from. diff --git a/packages/transaction-controller/tests/EtherscanMocks.ts b/packages/transaction-controller/tests/EtherscanMocks.ts index 8641f5bcdc..0d8c60b68f 100644 --- a/packages/transaction-controller/tests/EtherscanMocks.ts +++ b/packages/transaction-controller/tests/EtherscanMocks.ts @@ -95,7 +95,7 @@ const EXPECTED_NORMALISED_TRANSACTION_BASE = { chainId: undefined, hash: ETHERSCAN_TRANSACTION_SUCCESS_MOCK.hash, id: ID_MOCK, - networkClientId: 'incoming', + networkClientId: '', status: TransactionStatus.confirmed, time: 1543596356000, txParams: { From 3644eea2fbde3d1423e099c9dca161146230cdf5 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 26 Nov 2024 09:35:59 +0000 Subject: [PATCH 11/12] Test commit --- packages/transaction-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 7b3c7fbf58..31ceb9f55f 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed -- **BREAKING:** Remove global network usage ([#4920](https://github.com/MetaMask/core/pull/4920)) +- **BREAKING:** Remove global network usage ([#4920](https://github.com/MetaMask/core/pull/4920)) - Remove the `blockTracker`, `isMultichainEnabled`, `onNetworkStateChange` and `provider` constructor options. - Remove `filterToCurrentNetwork` option from `getTransactions` method. From f4bb67195579383e7498147b03930383c0036dde Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 26 Nov 2024 09:42:19 +0000 Subject: [PATCH 12/12] Revert test commit --- packages/transaction-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 31ceb9f55f..7b3c7fbf58 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed -- **BREAKING:** Remove global network usage ([#4920](https://github.com/MetaMask/core/pull/4920)) +- **BREAKING:** Remove global network usage ([#4920](https://github.com/MetaMask/core/pull/4920)) - Remove the `blockTracker`, `isMultichainEnabled`, `onNetworkStateChange` and `provider` constructor options. - Remove `filterToCurrentNetwork` option from `getTransactions` method.