From 484a76a36986e56ed2ea4c2ca66a288b357813b8 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 31 May 2024 16:12:12 -0600 Subject: [PATCH 1/4] TransactionController: providerConfig -> selectedNetworkClientId The `providerConfig` state property is being removed from NetworkController. Currently this property is used in TransactionController to get the currently selected chain, but `selectedNetworkClientId` can be used instead. This commit makes that transition so that we can fully drop `providerConfig`. --- .../src/TransactionController.test.ts | 468 +++++++++--------- .../src/TransactionController.ts | 15 +- 2 files changed, 250 insertions(+), 233 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 5cfbdb33fb..f23ba1e135 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -13,25 +13,37 @@ import { toHex, BUILT_IN_NETWORKS, ORIGIN_METAMASK, + InfuraNetworkType, } from '@metamask/controller-utils'; import EthQuery from '@metamask/eth-query'; import HttpProvider from '@metamask/ethjs-provider-http'; import type { BlockTracker, - NetworkController, + NetworkClientConfiguration, + NetworkClientId, NetworkState, Provider, } from '@metamask/network-controller'; -import { NetworkClientType, NetworkStatus } from '@metamask/network-controller'; +import { + NetworkClientType, + NetworkStatus, + defaultState as defaultNetworkState, +} 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'; import assert from 'assert'; 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, +} from '../../network-controller/tests/helpers'; import { DefaultGasFeeFlow } from './gas-flows/DefaultGasFeeFlow'; import { LineaGasFeeFlow } from './gas-flows/LineaGasFeeFlow'; import { TestGasFeeFlow } from './gas-flows/TestGasFeeFlow'; @@ -82,13 +94,6 @@ type UnrestrictedControllerMessenger = ControllerMessenger< TransactionControllerEvents | AllowedEvents >; -type NetworkClient = ReturnType; - -type NetworkClientConfiguration = Pick< - NetworkClient['configuration'], - 'chainId' ->; - const MOCK_V1_UUID = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'; jest.mock('@metamask/eth-query'); @@ -294,9 +299,6 @@ function waitForTransactionFinished( const MOCK_PREFERENCES = { state: { selectedAddress: 'foo' } }; const INFURA_PROJECT_ID = '341eacb578dd44a1a049cbc5f6fd4035'; -const GOERLI_PROVIDER = new HttpProvider( - `https://goerli.infura.io/v3/${INFURA_PROJECT_ID}`, -); const MAINNET_PROVIDER = new HttpProvider( `https://mainnet.infura.io/v3/${INFURA_PROJECT_ID}`, ); @@ -305,6 +307,7 @@ const PALM_PROVIDER = new HttpProvider( ); type MockNetwork = { + chainId: Hex; provider: Provider; blockTracker: BlockTracker; state: NetworkState; @@ -312,6 +315,7 @@ type MockNetwork = { }; const MOCK_NETWORK: MockNetwork = { + chainId: ChainId.goerli, provider: MAINNET_PROVIDER, blockTracker: buildMockBlockTracker('0x102833C'), state: { @@ -331,25 +335,9 @@ const MOCK_NETWORK: MockNetwork = { }, subscribe: () => undefined, }; -const MOCK_NETWORK_WITHOUT_CHAIN_ID: MockNetwork = { - provider: GOERLI_PROVIDER, - blockTracker: buildMockBlockTracker('0x102833C'), - state: { - selectedNetworkClientId: NetworkType.goerli, - networksMetadata: { - [NetworkType.goerli]: { - EIPS: { 1559: false }, - status: NetworkStatus.Available, - }, - }, - providerConfig: { - type: NetworkType.goerli, - } as NetworkState['providerConfig'], - networkConfigurations: {}, - }, - subscribe: () => undefined, -}; + const MOCK_MAINNET_NETWORK: MockNetwork = { + chainId: ChainId.mainnet, provider: MAINNET_PROVIDER, blockTracker: buildMockBlockTracker('0x102833C'), state: { @@ -371,6 +359,7 @@ const MOCK_MAINNET_NETWORK: MockNetwork = { }; const MOCK_LINEA_MAINNET_NETWORK: MockNetwork = { + chainId: ChainId['linea-mainnet'], provider: PALM_PROVIDER, blockTracker: buildMockBlockTracker('0xA6EDFC'), state: { @@ -383,7 +372,7 @@ const MOCK_LINEA_MAINNET_NETWORK: MockNetwork = { }, providerConfig: { type: NetworkType['linea-mainnet'], - chainId: toHex(59144), + chainId: ChainId['linea-mainnet'], ticker: NetworksTicker['linea-mainnet'], }, networkConfigurations: {}, @@ -392,6 +381,7 @@ const MOCK_LINEA_MAINNET_NETWORK: MockNetwork = { }; const MOCK_LINEA_GOERLI_NETWORK: MockNetwork = { + chainId: ChainId['linea-goerli'], provider: PALM_PROVIDER, blockTracker: buildMockBlockTracker('0xA6EDFC'), state: { @@ -404,7 +394,7 @@ const MOCK_LINEA_GOERLI_NETWORK: MockNetwork = { }, providerConfig: { type: NetworkType['linea-goerli'], - chainId: toHex(59140), + chainId: ChainId['linea-goerli'], ticker: NetworksTicker['linea-goerli'], }, networkConfigurations: {}, @@ -412,32 +402,11 @@ const MOCK_LINEA_GOERLI_NETWORK: MockNetwork = { subscribe: () => undefined, }; -const MOCK_CUSTOM_NETWORK: MockNetwork = { - provider: PALM_PROVIDER, - blockTracker: buildMockBlockTracker('0xA6EDFC'), - state: { - selectedNetworkClientId: 'uuid-1', - networksMetadata: { - 'uuid-1': { - EIPS: { 1559: false }, - status: NetworkStatus.Available, - }, - }, - providerConfig: { - type: NetworkType.rpc, - chainId: toHex(11297108109), - ticker: 'TEST', - }, - networkConfigurations: {}, - }, - subscribe: () => undefined, -}; - const ACCOUNT_MOCK = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; const ACCOUNT_2_MOCK = '0x08f137f335ea1b8f193b8f6ea92561a60d23a211'; const NONCE_MOCK = 12; const ACTION_ID_MOCK = '123456'; -const CHAIN_ID_MOCK = MOCK_NETWORK.state.providerConfig.chainId; +const CHAIN_ID_MOCK = MOCK_NETWORK.chainId; const NETWORK_CLIENT_ID_MOCK = 'networkClientIdMock'; const TRANSACTION_META_MOCK = { @@ -542,27 +511,80 @@ describe('TransactionController', () => { * @param args - The arguments to this function. * @param args.options - TransactionController options. * @param args.network - The mock network to use with the controller. + * @param args.network.blockTracker - The desired block tracker associated + * with the network. + * @param args.network.provider - The desired provider associated with the + * provider. + * @param args.network.state - The desired NetworkController state. + * @param args.network.onNetworkStateChange - The function to subscribe to + * changes in the NetworkController state. * @param args.messengerOptions - Options to build the mock unrestricted * messenger. - * @param args.messengerOptions.addTransactionApprovalRequest - Options to mock - * the `ApprovalController:addRequest` action call for transactions. + * @param args.messengerOptions.addTransactionApprovalRequest - Options to + * mock the `ApprovalController:addRequest` action call for transactions. + * @param args.mockNetworkClientConfigurationsByNetworkClientId - Network + * client configurations by network client ID. * @returns The new TransactionController instance. */ function setupController({ options: givenOptions = {}, - network = MOCK_NETWORK, + network = {}, messengerOptions = {}, + mockNetworkClientConfigurationsByNetworkClientId = {}, }: { options?: Partial[0]>; - network?: MockNetwork; + network?: { + blockTracker?: BlockTracker; + provider?: Provider; + state?: Partial; + onNetworkStateChange?: ( + listener: (networkState: NetworkState) => void, + ) => void; + }; messengerOptions?: { addTransactionApprovalRequest?: Parameters< typeof mockAddTransactionApprovalRequest >[1]; }; + mockNetworkClientConfigurationsByNetworkClientId?: Record< + NetworkClientId, + NetworkClientConfiguration + >; } = {}) { + let networkState = { + ...defaultNetworkState, + selectedNetworkClientId: MOCK_NETWORK.state.selectedNetworkClientId, + ...network.state, + }; + const blockTracker = network.blockTracker ?? new FakeBlockTracker(); + const provider = network.provider ?? new FakeProvider(); + const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; + const changeNetwork = ({ + selectedNetworkClientId, + }: { + selectedNetworkClientId: NetworkClientId; + }) => { + networkState = { + ...networkState, + selectedNetworkClientId, + }; + onNetworkDidChangeListeners.forEach((listener) => { + listener(networkState); + }); + }; + const onNetworkStateChange = ( + listener: (networkState: NetworkState) => void, + ) => onNetworkDidChangeListeners.push(listener); + const unrestrictedMessenger: UnrestrictedControllerMessenger = new ControllerMessenger(); + const getNetworkClientById = buildMockGetNetworkClientById( + mockNetworkClientConfigurationsByNetworkClientId, + ); + unrestrictedMessenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + getNetworkClientById, + ); const { addTransactionApprovalRequest = { state: 'pending' } } = messengerOptions; @@ -572,12 +594,12 @@ describe('TransactionController', () => { ); const { messenger: givenRestrictedMessenger, ...otherOptions } = { - blockTracker: network.blockTracker, + blockTracker, disableHistory: false, disableSendFlowHistory: false, disableSwaps: false, getCurrentNetworkEIP1559Compatibility: async () => false, - getNetworkState: () => network.state, + getNetworkState: () => networkState, // TODO: Replace with a real type // eslint-disable-next-line @typescript-eslint/no-explicit-any getNetworkClientRegistry: () => ({} as any), @@ -585,8 +607,8 @@ describe('TransactionController', () => { getSelectedAddress: () => ACCOUNT_MOCK, isMultichainEnabled: false, hooks: {}, - onNetworkStateChange: network.subscribe, - provider: network.provider, + onNetworkStateChange, + provider, sign: async (transaction: TypedTransaction) => transaction, transactionHistoryLimit: 40, ...givenOptions, @@ -613,6 +635,7 @@ describe('TransactionController', () => { controller, messenger: unrestrictedMessenger, mockTransactionApprovalRequest, + changeNetwork, }; } @@ -621,17 +644,18 @@ describe('TransactionController', () => { * TransactionController calls as it creates transactions. * * This helper allows the `addRequest` action to be in one of three states: - * approved, rejected, or pending. In the approved state, the promise which the - * action returns is resolved ahead of time, and in the rejected state, the - * promise is rejected ahead of time. Otherwise, in the pending state, the - * promise is unresolved and it is assumed that the test will resolve or reject - * the promise. + * approved, rejected, or pending. In the approved state, the promise which + * the action returns is resolved ahead of time, and in the rejected state, + * the promise is rejected ahead of time. Otherwise, in the pending state, the + * promise is unresolved and it is assumed that the test will resolve or + * reject the promise. * * @param messenger - The unrestricted messenger. - * @param options - Options for the mock. `state` controls the state of the - * promise as outlined above. Note, if the `state` is approved, then its - * `result` may be specified; if the `state` is rejected, then its `error` may - * be specified. + * @param options - An options bag which will be used to create an action + * handler that places the approval request in a certain state. The `state` + * option controls the state of the promise as outlined above: if the `state` + * is approved, then its `result` may be specified; if the `state` is + * rejected, then its `error` may be specified. * @returns An object which contains the aforementioned promise, functions to * manually approve or reject the approval (and therefore the promise), and * finally the mocked version of the action handler itself. @@ -687,7 +711,6 @@ describe('TransactionController', () => { ReturnType, Parameters > = jest.fn().mockReturnValue(promise); - if (options.state === 'approved') { approveTransaction(options.result); } else if (options.state === 'rejected') { @@ -707,22 +730,6 @@ describe('TransactionController', () => { }; } - /** - * Builds a network client that is only used in tests to get a chain ID. - * - * @param networkClient - The properties in the desired network client. - * Only needs to contain `configuration`. - * @param networkClient.configuration - The desired network client - * configuration. Only needs to contain `chainId`> - * @returns The network client. - */ - function buildMockNetworkClient(networkClient: { - configuration: NetworkClientConfiguration; - }): NetworkClient { - // Type assertion: We don't expect anything but the configuration to get used. - return networkClient as unknown as NetworkClient; - } - /** * Wait for a specified number of milliseconds. * @@ -765,18 +772,19 @@ describe('TransactionController', () => { return incomingTransactionHelperMock; }); + pendingTransactionTrackerMock = { + start: jest.fn(), + stop: jest.fn(), + startIfPendingTransactions: jest.fn(), + hub: { + on: jest.fn(), + removeAllListeners: jest.fn(), + }, + onStateChange: jest.fn(), + forceCheckTransaction: jest.fn(), + } as unknown as jest.Mocked; + pendingTransactionTrackerClassMock.mockImplementation(() => { - pendingTransactionTrackerMock = { - start: jest.fn(), - stop: jest.fn(), - startIfPendingTransactions: jest.fn(), - hub: { - on: jest.fn(), - removeAllListeners: jest.fn(), - }, - onStateChange: jest.fn(), - forceCheckTransaction: jest.fn(), - } as unknown as jest.Mocked; return pendingTransactionTrackerMock; }); @@ -907,7 +915,7 @@ describe('TransactionController', () => { transactions: [ { ...TRANSACTION_META_MOCK, - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, status: TransactionStatus.submitted, }, ], @@ -1315,9 +1323,7 @@ describe('TransactionController', () => { expect(updateSwapsTransactionMock).toHaveBeenCalledTimes(1); expect(transactionMeta.txParams.from).toBe(ACCOUNT_MOCK); - expect(transactionMeta.chainId).toBe( - MOCK_NETWORK.state.providerConfig.chainId, - ); + expect(transactionMeta.chainId).toBe(MOCK_NETWORK.chainId); expect(transactionMeta.deviceConfirmedOn).toBe(mockDeviceConfirmedOn); expect(transactionMeta.origin).toBe(mockOrigin); expect(transactionMeta.status).toBe(TransactionStatus.unapproved); @@ -1331,26 +1337,9 @@ describe('TransactionController', () => { describe('networkClientId exists in the MultichainTrackingHelper', () => { it('adds unapproved transaction to state when using networkClientId', async () => { - const { controller, messenger } = setupController({ + const { controller } = setupController({ options: { isMultichainEnabled: true }, }); - messenger.registerActionHandler( - 'NetworkController:getNetworkClientById', - (networkClientId) => { - switch (networkClientId) { - case 'sepolia': - return buildMockNetworkClient({ - configuration: { - chainId: ChainId.sepolia, - }, - }); - default: - throw new Error( - `Unknown network client ID: ${networkClientId}`, - ); - } - }, - ); const sepoliaTxParams: TransactionParams = { chainId: ChainId.sepolia, from: ACCOUNT_MOCK, @@ -1362,7 +1351,7 @@ describe('TransactionController', () => { await controller.addTransaction(sepoliaTxParams, { origin: 'metamask', actionId: ACTION_ID_MOCK, - networkClientId: 'sepolia', + networkClientId: InfuraNetworkType.sepolia, }); const transactionMeta = controller.state.transactions[0]; @@ -1384,23 +1373,6 @@ describe('TransactionController', () => { }, }, }); - messenger.registerActionHandler( - 'NetworkController:getNetworkClientById', - (networkClientId) => { - switch (networkClientId) { - case 'sepolia': - return buildMockNetworkClient({ - configuration: { - chainId: ChainId.sepolia, - }, - }); - default: - throw new Error( - `Unknown network client ID: ${networkClientId}`, - ); - } - }, - ); multichainTrackingHelperMock.has.mockReturnValue(true); const submittedEventListener = jest.fn(); @@ -1418,7 +1390,7 @@ describe('TransactionController', () => { const { result } = await controller.addTransaction(sepoliaTxParams, { origin: 'metamask', actionId: ACTION_ID_MOCK, - networkClientId: 'sepolia', + networkClientId: InfuraNetworkType.sepolia, }); await result; @@ -1516,49 +1488,54 @@ describe('TransactionController', () => { ); }); - it.each([ - ['mainnet', MOCK_MAINNET_NETWORK], - ['custom network', MOCK_CUSTOM_NETWORK], - ])( - 'adds unapproved transaction to state after switching to %s', - async (_networkName, newNetwork) => { - const getNetworkState = jest.fn().mockReturnValue(MOCK_NETWORK.state); - - let networkStateChangeListener: ((state: NetworkState) => void) | null = - null; - - const onNetworkStateChange = ( - listener: (state: NetworkState) => void, - ) => { - networkStateChangeListener = listener; - }; + it('adds unapproved transaction to state after switching to Infura network', async () => { + const { controller, changeNetwork } = setupController({ + network: { + state: { + selectedNetworkClientId: InfuraNetworkType.goerli, + }, + }, + }); + changeNetwork({ selectedNetworkClientId: InfuraNetworkType.mainnet }); - const { controller } = setupController({ - options: { getNetworkState, onNetworkStateChange }, - }); + await controller.addTransaction({ + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }); - // switch from Goerli to Mainnet - getNetworkState.mockReturnValue(newNetwork.state); + expect(controller.state.transactions[0].txParams.from).toBe(ACCOUNT_MOCK); + expect(controller.state.transactions[0].chainId).toBe(ChainId.mainnet); + expect(controller.state.transactions[0].status).toBe( + TransactionStatus.unapproved, + ); + }); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - networkStateChangeListener!(newNetwork.state); + it('adds unapproved transaction to state after switching to custom network', async () => { + const { controller, changeNetwork } = setupController({ + network: { + state: { + selectedNetworkClientId: InfuraNetworkType.goerli, + }, + }, + mockNetworkClientConfigurationsByNetworkClientId: { + 'AAAA-BBBB-CCCC-DDDD': buildCustomNetworkClientConfiguration({ + chainId: '0x1337', + }), + }, + }); + changeNetwork({ selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD' }); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + await controller.addTransaction({ + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }); - expect(controller.state.transactions[0].txParams.from).toBe( - ACCOUNT_MOCK, - ); - expect(controller.state.transactions[0].chainId).toBe( - newNetwork.state.providerConfig.chainId, - ); - expect(controller.state.transactions[0].status).toBe( - TransactionStatus.unapproved, - ); - }, - ); + expect(controller.state.transactions[0].txParams.from).toBe(ACCOUNT_MOCK); + expect(controller.state.transactions[0].chainId).toBe('0x1337'); + expect(controller.state.transactions[0].status).toBe( + TransactionStatus.unapproved, + ); + }); it('throws if address invalid', async () => { const { controller } = setupController(); @@ -1622,15 +1599,19 @@ describe('TransactionController', () => { it('requests approval using the approval controller', async () => { const { controller, messenger } = setupController(); - jest.spyOn(messenger, 'call'); + const messengerCallSpy = jest.spyOn(messenger, 'call'); await controller.addTransaction({ from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }); - expect(messenger.call).toHaveBeenCalledTimes(1); - expect(messenger.call).toHaveBeenCalledWith( + expect( + messengerCallSpy.mock.calls.filter( + (args) => args[0] === 'ApprovalController:addRequest', + ), + ).toHaveLength(1); + expect(messengerCallSpy).toHaveBeenCalledWith( 'ApprovalController:addRequest', { id: expect.any(String), @@ -1657,7 +1638,9 @@ describe('TransactionController', () => { }, ); - expect(messenger.call).toHaveBeenCalledTimes(0); + expect(messenger.call).not.toHaveBeenCalledWith( + 'ApprovalController:addRequest', + ); }); it('calls security provider with transaction meta and sets response in to securityProviderResponse', async () => { @@ -1711,9 +1694,9 @@ describe('TransactionController', () => { expect(updateGasMock).toHaveBeenCalledTimes(1); expect(updateGasMock).toHaveBeenCalledWith({ ethQuery: expect.any(Object), - chainId: MOCK_NETWORK.state.providerConfig.chainId, - isCustomNetwork: - MOCK_NETWORK.state.providerConfig.type === NetworkType.rpc, + chainId: MOCK_NETWORK.chainId, + // XXX: Is this right? + isCustomNetwork: false, txMeta: expect.any(Object), }); }); @@ -1757,7 +1740,7 @@ describe('TransactionController', () => { expect(getSimulationDataMock).toHaveBeenCalledTimes(1); expect(getSimulationDataMock).toHaveBeenCalledWith({ - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, data: undefined, from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, @@ -2003,7 +1986,18 @@ describe('TransactionController', () => { it('if no chainId defined', async () => { const { controller } = setupController({ - network: MOCK_NETWORK_WITHOUT_CHAIN_ID, + mockNetworkClientConfigurationsByNetworkClientId: { + 'AAAA-BBBB-CCCC-DDDD': buildCustomNetworkClientConfiguration({ + rpcUrl: 'https://test.network', + ticker: 'TEST', + chainId: undefined, + }), + }, + network: { + state: { + selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD', + }, + }, messengerOptions: { addTransactionApprovalRequest: { state: 'approved', @@ -2015,10 +2009,13 @@ describe('TransactionController', () => { }); it('if unexpected status', async () => { - const { controller, messenger } = setupController(); - - jest.spyOn(messenger, 'call').mockImplementationOnce(() => { - throw new Error('Unknown problem'); + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'rejected', + error: new Error('Unknown problem'), + }, + }, }); const { result } = await controller.addTransaction({ @@ -2033,10 +2030,13 @@ describe('TransactionController', () => { }); it('if unrecognised error', async () => { - const { controller, messenger } = setupController(); - - jest.spyOn(messenger, 'call').mockImplementationOnce(() => { - throw new Error('TestError'); + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'rejected', + error: new Error('TestError'), + }, + }, }); const { result } = await controller.addTransaction({ @@ -2051,12 +2051,8 @@ describe('TransactionController', () => { }); it('if transaction removed', async () => { - const { controller, messenger } = setupController(); - - jest.spyOn(messenger, 'call').mockImplementationOnce(() => { - controller.clearUnapprovedTransactions(); - throw new Error('Unknown problem'); - }); + const { controller, mockTransactionApprovalRequest } = + setupController(); const { result } = await controller.addTransaction({ from: ACCOUNT_MOCK, @@ -2066,6 +2062,9 @@ describe('TransactionController', () => { value: '0x0', }); + controller.clearUnapprovedTransactions(); + mockTransactionApprovalRequest.reject(new Error('Unknown problem')); + await expect(result).rejects.toThrow('Unknown problem'); }); }); @@ -4051,7 +4050,7 @@ describe('TransactionController', () => { const confirmed = { ...TRANSACTION_META_MOCK, id: 'testId1', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, hash: '0x3', status: TransactionStatus.confirmed, txParams: { ...TRANSACTION_META_MOCK.txParams, nonce: '0x1' }, @@ -4263,7 +4262,7 @@ describe('TransactionController', () => { gas: '0x222', to: ACCOUNT_2_MOCK, value: '0x1', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; await expect( @@ -4293,7 +4292,7 @@ describe('TransactionController', () => { gas: '0x5208', to: ACCOUNT_2_MOCK, value: '0x0', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; // Send the transaction to put it in the process of being signed @@ -4324,7 +4323,7 @@ describe('TransactionController', () => { gas: '0x111', to: ACCOUNT_2_MOCK, value: '0x0', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; const mockTransactionParam2 = { from: ACCOUNT_MOCK, @@ -4332,7 +4331,7 @@ describe('TransactionController', () => { gas: '0x222', to: ACCOUNT_2_MOCK, value: '0x1', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; const result = await controller.approveTransactionsWithSameNonce([ @@ -4363,7 +4362,7 @@ describe('TransactionController', () => { gas: '0x111', to: ACCOUNT_2_MOCK, value: '0x0', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; const mockTransactionParam2 = { from: ACCOUNT_MOCK, @@ -4371,7 +4370,7 @@ describe('TransactionController', () => { gas: '0x222', to: ACCOUNT_2_MOCK, value: '0x1', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; await expect( @@ -4391,7 +4390,7 @@ describe('TransactionController', () => { gas: '0x111', to: ACCOUNT_2_MOCK, value: '0x0', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; const mockTransactionParam2 = { @@ -4400,7 +4399,7 @@ describe('TransactionController', () => { gas: '0x222', to: ACCOUNT_2_MOCK, value: '0x1', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; await controller.approveTransactionsWithSameNonce( @@ -4424,7 +4423,7 @@ describe('TransactionController', () => { gas: '0x111', to: ACCOUNT_2_MOCK, value: '0x0', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; const mockTransactionParam2 = { @@ -4433,7 +4432,7 @@ describe('TransactionController', () => { gas: '0x222', to: ACCOUNT_2_MOCK, value: '0x1', - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, }; await controller.approveTransactionsWithSameNonce([ @@ -5005,13 +5004,17 @@ describe('TransactionController', () => { state: mockedControllerState as any, }, }); - jest.spyOn(messenger, 'call'); + const messengerCallSpy = jest.spyOn(messenger, 'call'); controller.initApprovals(); await flushPromises(); - expect(messenger.call).toHaveBeenCalledTimes(2); - expect(messenger.call).toHaveBeenCalledWith( + expect( + messengerCallSpy.mock.calls.filter( + (args) => args[0] === 'ApprovalController:addRequest', + ), + ).toHaveLength(2); + expect(messengerCallSpy).toHaveBeenCalledWith( 'ApprovalController:addRequest', { expectsResult: true, @@ -5022,7 +5025,7 @@ describe('TransactionController', () => { }, false, ); - expect(messenger.call).toHaveBeenCalledWith( + expect(messengerCallSpy).toHaveBeenCalledWith( 'ApprovalController:addRequest', { expectsResult: true, @@ -5067,16 +5070,17 @@ describe('TransactionController', () => { const mockedErrorMessage = 'mocked error'; - const { controller, messenger } = setupController({ - options: { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - state: mockedControllerState as any, - }, - }); + const { controller, messenger, mockTransactionApprovalRequest } = + setupController({ + options: { + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + state: mockedControllerState as any, + }, + }); + const messengerCallSpy = jest.spyOn(messenger, 'call'); // Expect both calls to throw error, one with code property to check if it is handled - jest - .spyOn(messenger, 'call') + mockTransactionApprovalRequest.actionHandlerMock .mockImplementationOnce(() => { // eslint-disable-next-line @typescript-eslint/no-throw-literal throw { message: mockedErrorMessage }; @@ -5099,7 +5103,11 @@ describe('TransactionController', () => { 'Error during persisted transaction approval', new Error(mockedErrorMessage), ); - expect(messenger.call).toHaveBeenCalledTimes(2); + expect( + messengerCallSpy.mock.calls.filter((args) => { + return args[0] === 'ApprovalController:addRequest'; + }), + ).toHaveLength(2); }); it('does not create any approval when there is no unapproved transaction', async () => { @@ -5107,7 +5115,9 @@ describe('TransactionController', () => { jest.spyOn(messenger, 'call'); controller.initApprovals(); await flushPromises(); - expect(messenger.call).not.toHaveBeenCalled(); + expect(messenger.call).not.toHaveBeenCalledWith( + 'ApprovalController:addRequest', + ); }); }); @@ -5273,7 +5283,7 @@ describe('TransactionController', () => { it('returns transactions matching current network', () => { const transactions: TransactionMeta[] = [ { - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, id: 'testId1', status: TransactionStatus.confirmed, time: 1, @@ -5287,7 +5297,7 @@ describe('TransactionController', () => { txParams: { from: '0x2' }, }, { - chainId: MOCK_NETWORK.state.providerConfig.chainId, + chainId: MOCK_NETWORK.chainId, id: 'testId3', status: TransactionStatus.submitted, time: 1, diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 06010fc314..11939ab94f 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -15,10 +15,10 @@ import type { import { BaseController } from '@metamask/base-controller'; import { query, - NetworkType, ApprovalType, ORIGIN_METAMASK, convertHexToDecimal, + isInfuraNetworkType, } from '@metamask/controller-utils'; import EthQuery from '@metamask/eth-query'; import type { @@ -36,11 +36,11 @@ import type { NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; import { NetworkClientType } from '@metamask/network-controller'; -import { NonceTracker } from '@metamask/nonce-tracker'; import type { NonceLock, Transaction as NonceTrackerTransaction, } from '@metamask/nonce-tracker'; +import { NonceTracker } from '@metamask/nonce-tracker'; import { errorCodes, rpcErrors, providerErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; import { add0x } from '@metamask/utils'; @@ -3745,6 +3745,8 @@ export class TransactionController extends BaseController< const finalTransactionMeta = this.getTransaction(transactionId); + /* istanbul ignore if */ + // TODO: Add test for this if (!finalTransactionMeta) { log( 'Cannot update simulation data as transaction not found', @@ -3826,14 +3828,19 @@ export class TransactionController extends BaseController< } #getGlobalChainId() { - return this.getNetworkState().providerConfig.chainId; + return this.messagingSystem.call( + `NetworkController:getNetworkClientById`, + this.getNetworkState().selectedNetworkClientId, + ).configuration.chainId; } #isCustomNetwork(networkClientId?: NetworkClientId) { const globalNetworkClientId = this.#getGlobalNetworkClientId(); if (!networkClientId || networkClientId === globalNetworkClientId) { - return this.getNetworkState().providerConfig.type === NetworkType.rpc; + return !isInfuraNetworkType( + this.getNetworkState().selectedNetworkClientId, + ); } return ( From af7bdc2828cb90e6e0cdb599f41ca8ce5949eb26 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 7 Jun 2024 09:29:59 -0600 Subject: [PATCH 2/4] Remove TODO --- packages/transaction-controller/src/TransactionController.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 11939ab94f..3d0a3f5cc2 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -3746,7 +3746,6 @@ export class TransactionController extends BaseController< const finalTransactionMeta = this.getTransaction(transactionId); /* istanbul ignore if */ - // TODO: Add test for this if (!finalTransactionMeta) { log( 'Cannot update simulation data as transaction not found', From e5196f7563ccde09baf74857c743809918b6e61a Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 7 Jun 2024 14:09:53 -0600 Subject: [PATCH 3/4] Update packages/transaction-controller/src/TransactionController.test.ts Co-authored-by: Jongsun Suh --- .../transaction-controller/src/TransactionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index f23ba1e135..eef7c6acc7 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -2012,7 +2012,7 @@ describe('TransactionController', () => { const { controller } = setupController({ messengerOptions: { addTransactionApprovalRequest: { - state: 'rejected', + state: TransactionStatus.rejected, error: new Error('Unknown problem'), }, }, From 1c9351400caf609341ad1290a2a526ec8570bc52 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 7 Jun 2024 14:10:05 -0600 Subject: [PATCH 4/4] Update packages/transaction-controller/src/TransactionController.test.ts Co-authored-by: Jongsun Suh --- .../transaction-controller/src/TransactionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index eef7c6acc7..c6be592273 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -2033,7 +2033,7 @@ describe('TransactionController', () => { const { controller } = setupController({ messengerOptions: { addTransactionApprovalRequest: { - state: 'rejected', + state: TransactionStatus.rejected, error: new Error('TestError'), }, },