diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index d280c457cf..f62ded09ca 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -100,7 +100,7 @@ export type ProviderProxy = SwappableProxy; type BlockTracker = any; -type BlockTrackerProxy = SwappableProxy; +export type BlockTrackerProxy = SwappableProxy; export type NetworkControllerStateChangeEvent = { type: `NetworkController:stateChange`; diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index 3368fdd4f6..83157551ec 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -40,6 +40,7 @@ "eth-query": "^2.1.2", "eth-rpc-errors": "^4.0.0", "ethereumjs-util": "^7.0.10", + "nonce-tracker": "^1.1.0", "uuid": "^8.3.2" }, "devDependencies": { diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 13b3d03f2d..a05d5b6d2f 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1,5 +1,6 @@ import * as sinon from 'sinon'; import HttpProvider from 'ethjs-provider-http'; +import NonceTracker from 'nonce-tracker'; import { NetworksChainId, NetworkType } from '@metamask/controller-utils'; import type { NetworkState } from '@metamask/network-controller'; import { ESTIMATE_GAS_ERROR } from './utils'; @@ -17,10 +18,14 @@ import { txsInStateWithOutdatedStatusAndGasDataMock, } from './mocks/txsMock'; +const v1Stub = jest + .fn() + .mockImplementation(() => '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'); + jest.mock('uuid', () => { return { ...jest.requireActual('uuid'), - v1: () => '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d', + v1: () => v1Stub(), }; }); @@ -122,14 +127,19 @@ function mockFetchWithDynamicResponse(dataForUrl: any) { } const MOCK_PRFERENCES = { state: { selectedAddress: 'foo' } }; -const PROVIDER = new HttpProvider( +const GOERLI_PROVIDER = new HttpProvider( 'https://goerli.infura.io/v3/341eacb578dd44a1a049cbc5f6fd4035', ); const MAINNET_PROVIDER = new HttpProvider( 'https://mainnet.infura.io/v3/341eacb578dd44a1a049cbc5f6fd4035', ); +const PALM_PROVIDER = new HttpProvider( + 'https://palm-mainnet.infura.io/v3/3a961d6501e54add9a41aa53f15de99b', +); + const MOCK_NETWORK = { - getProvider: () => PROVIDER, + provider: MAINNET_PROVIDER, + blockTracker: { getLatestBlock: () => '0x102833C' }, state: { network: '5', isCustomNetwork: false, @@ -142,22 +152,9 @@ const MOCK_NETWORK = { }, subscribe: () => undefined, }; -const MOCK_NETWORK_CUSTOM = { - getProvider: () => PROVIDER, - state: { - network: '10', - isCustomNetwork: true, - networkDetails: { isEIP1559Compatible: false }, - providerConfig: { - type: NetworkType.rpc, - chainId: '10', - }, - networkConfigurations: {}, - }, - subscribe: () => undefined, -}; const MOCK_NETWORK_WITHOUT_CHAIN_ID = { - getProvider: () => PROVIDER, + provider: GOERLI_PROVIDER, + blockTracker: { getLatestBlock: () => '0x102833C' }, isCustomNetwork: false, state: { network: '5', @@ -167,7 +164,8 @@ const MOCK_NETWORK_WITHOUT_CHAIN_ID = { subscribe: () => undefined, }; const MOCK_MAINNET_NETWORK = { - getProvider: () => MAINNET_PROVIDER, + provider: MAINNET_PROVIDER, + blockTracker: { getLatestBlock: () => '0x102833C' }, state: { network: '1', isCustomNetwork: false, @@ -181,14 +179,15 @@ const MOCK_MAINNET_NETWORK = { subscribe: () => undefined, }; const MOCK_CUSTOM_NETWORK = { - getProvider: () => MAINNET_PROVIDER, + provider: PALM_PROVIDER, + blockTracker: { getLatestBlock: () => '0xA6EDFC' }, state: { - network: '80001', + network: '11297108109', isCustomNetwork: true, networkDetails: { isEIP1559Compatible: false }, providerConfig: { type: NetworkType.rpc, - chainId: '80001', + chainId: '11297108109', }, networkConfigurations: {}, }, @@ -288,6 +287,7 @@ describe('TransactionController', () => { }); afterEach(() => { + jest.clearAllMocks(); sinon.restore(); }); @@ -295,8 +295,10 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); + expect(controller.state).toStrictEqual({ methodData: {}, transactions: [], @@ -307,7 +309,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); expect(controller.config).toStrictEqual({ interval: 15000, @@ -325,7 +328,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { interval: 10 }, ); @@ -344,7 +348,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { interval: 1337 }, ); @@ -363,7 +368,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { interval: 10 }, ); @@ -379,7 +385,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); mockFlags.estimateGasValue = '0x12a05f200'; const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c'; @@ -392,7 +399,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); mockFlags.estimateGasValue = '0x12a05f200'; mockFlags.estimateGasError = ESTIMATE_GAS_ERROR; @@ -407,7 +415,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_CUSTOM_NETWORK.state, onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe, - getProvider: MOCK_CUSTOM_NETWORK.getProvider, + provider: MOCK_CUSTOM_NETWORK.provider, + blockTracker: MOCK_CUSTOM_NETWORK.blockTracker, }); const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c'; const result = await controller.estimateGas({ from, to: from }); @@ -418,7 +427,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_CUSTOM_NETWORK.state, onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe, - getProvider: MOCK_CUSTOM_NETWORK.getProvider, + provider: MOCK_CUSTOM_NETWORK.provider, + blockTracker: MOCK_CUSTOM_NETWORK.blockTracker, }); mockFlags.estimateGasError = ESTIMATE_GAS_ERROR; const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c'; @@ -430,7 +440,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_CUSTOM_NETWORK.state, onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe, - getProvider: MOCK_CUSTOM_NETWORK.getProvider, + provider: MOCK_CUSTOM_NETWORK.provider, + blockTracker: MOCK_CUSTOM_NETWORK.blockTracker, }); mockFlags.getBlockByNumberValue = '0x12a05f200'; @@ -444,7 +455,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_CUSTOM_NETWORK.state, onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe, - getProvider: MOCK_CUSTOM_NETWORK.getProvider, + provider: MOCK_CUSTOM_NETWORK.provider, + blockTracker: MOCK_CUSTOM_NETWORK.blockTracker, }); mockFlags.getBlockByNumberValue = '0x12a05f200'; @@ -460,7 +472,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); mockFlags.getBlockByNumberValue = '0x12a05f200'; @@ -475,7 +488,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); mockFlags.getBlockByNumberValue = '0x12a05f200'; @@ -491,7 +505,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); await expect( controller.addTransaction({ from: 'foo' } as any), @@ -502,7 +517,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; await controller.addTransaction({ @@ -530,16 +546,17 @@ describe('TransactionController', () => { const onNetworkStateChange = (listener: (state: NetworkState) => void) => { networkStateChangeListener = listener; }; - const getProvider = sinon.stub().returns(PROVIDER); + const controller = new TransactionController({ getNetworkState, onNetworkStateChange, - getProvider, + provider: GOERLI_PROVIDER, + blockTracker: undefined, }); // switch from Goerli to Mainnet getNetworkState.returns(MOCK_MAINNET_NETWORK.state); - getProvider.returns(MAINNET_PROVIDER); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion networkStateChangeListener!(MOCK_MAINNET_NETWORK.state); @@ -569,18 +586,19 @@ describe('TransactionController', () => { const onNetworkStateChange = (listener: (state: NetworkState) => void) => { networkStateChangeListener = listener; }; - const getProvider = sinon.stub().returns(PROVIDER); + const controller = new TransactionController({ getNetworkState, onNetworkStateChange, - getProvider, + provider: MOCK_CUSTOM_NETWORK.provider, + blockTracker: MOCK_CUSTOM_NETWORK.blockTracker, }); // switch from Goerli to Mainnet - getNetworkState.returns(MOCK_NETWORK_CUSTOM.state); - getProvider.returns(PROVIDER); + getNetworkState.returns(MOCK_CUSTOM_NETWORK.state); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - networkStateChangeListener!(MOCK_NETWORK_CUSTOM.state); + networkStateChangeListener!(MOCK_CUSTOM_NETWORK.state); const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; await controller.addTransaction({ @@ -589,11 +607,11 @@ describe('TransactionController', () => { }); expect(controller.state.transactions[0].transaction.from).toBe(from); expect(controller.state.transactions[0].networkID).toBe( - MOCK_NETWORK_CUSTOM.state.network, + MOCK_CUSTOM_NETWORK.state.network, ); expect(controller.state.transactions[0].chainId).toBe( - MOCK_NETWORK_CUSTOM.state.providerConfig.chainId, + MOCK_CUSTOM_NETWORK.state.providerConfig.chainId, ); expect(controller.state.transactions[0].status).toBe( @@ -605,7 +623,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; const { result } = await controller.addTransaction({ @@ -634,7 +653,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); controller.wipeTransactions(); const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; @@ -651,7 +671,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); controller.wipeTransactions(); controller.state.transactions.push({ @@ -665,46 +686,13 @@ describe('TransactionController', () => { expect(controller.state.transactions).toHaveLength(0); }); - it('should approve custom network transaction', async () => { - await new Promise(async (resolve) => { - const controller = new TransactionController( - { - getNetworkState: () => MOCK_CUSTOM_NETWORK.state, - onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe, - getProvider: MOCK_CUSTOM_NETWORK.getProvider, - }, - { - sign: async (transaction: any) => transaction, - }, - ); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - await controller.addTransaction({ - from, - gas: '0x0', - gasPrice: '0x0', - to: from, - value: '0x0', - }); - - controller.hub.once( - `${controller.state.transactions[0].id}:finished`, - () => { - const { transaction, status } = controller.state.transactions[0]; - expect(transaction.from).toBe(from); - expect(status).toBe(TransactionStatus.submitted); - resolve(''); - }, - ); - controller.approveTransaction(controller.state.transactions[0].id); - }); - }); - it('should fail to approve an invalid transaction', async () => { const controller = new TransactionController( { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: () => { @@ -727,7 +715,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }); mockFlags.estimateGasError = ESTIMATE_GAS_ERROR; const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; @@ -750,7 +739,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_CUSTOM_NETWORK.state, onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe, - getProvider: MOCK_CUSTOM_NETWORK.getProvider, + provider: MOCK_CUSTOM_NETWORK.provider, + blockTracker: MOCK_CUSTOM_NETWORK.blockTracker, }); mockFlags.estimateGasError = ESTIMATE_GAS_ERROR; const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; @@ -774,7 +764,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, {}, ); @@ -795,7 +786,8 @@ describe('TransactionController', () => { getNetworkState: () => MOCK_NETWORK_WITHOUT_CHAIN_ID.state as NetworkState, onNetworkStateChange: MOCK_NETWORK_WITHOUT_CHAIN_ID.subscribe, - getProvider: MOCK_NETWORK_WITHOUT_CHAIN_ID.getProvider, + provider: MOCK_NETWORK_WITHOUT_CHAIN_ID.provider, + blockTracker: MOCK_NETWORK_WITHOUT_CHAIN_ID.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -818,7 +810,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -852,7 +845,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -888,7 +882,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -921,7 +916,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -945,7 +941,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -973,7 +970,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); controller.wipeTransactions(); expect(controller.state.transactions).toHaveLength(0); @@ -990,7 +988,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }); controller.wipeTransactions(); expect(controller.state.transactions).toHaveLength(0); @@ -1007,7 +1006,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }); const from = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; controller.wipeTransactions(); @@ -1029,7 +1029,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }); controller.wipeTransactions(); expect(controller.state.transactions).toHaveLength(0); @@ -1046,7 +1047,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }); const from = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; controller.wipeTransactions(); @@ -1072,7 +1074,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }); const from = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; controller.wipeTransactions(); @@ -1099,7 +1102,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }); const from = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; controller.wipeTransactions(); @@ -1128,7 +1132,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); controller.wipeTransactions(); expect(controller.state.transactions).toHaveLength(0); @@ -1143,7 +1148,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }, {}, ); @@ -1163,7 +1169,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_MAINNET_NETWORK.state, onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - getProvider: MOCK_MAINNET_NETWORK.getProvider, + provider: MOCK_MAINNET_NETWORK.provider, + blockTracker: MOCK_MAINNET_NETWORK.blockTracker, }, {}, ); @@ -1182,7 +1189,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -1204,7 +1212,8 @@ describe('TransactionController', () => { const controller = new TransactionController({ getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }); const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; @@ -1221,7 +1230,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { sign: async (transaction: any) => transaction, @@ -1235,10 +1245,11 @@ describe('TransactionController', () => { to: from, value: '0x0', }); + await controller.approveTransaction(controller.state.transactions[0].id); await controller.speedUpTransaction(controller.state.transactions[0].id); expect(controller.state.transactions).toHaveLength(2); expect(controller.state.transactions[1].transaction.gasPrice).toBe( - '0x5916a6d6', + '0x5916a6d6', // 1.1 * 0x50fd51da ); resolve(''); }); @@ -1251,7 +1262,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { interval: 5000, @@ -1283,7 +1295,8 @@ describe('TransactionController', () => { { getNetworkState: () => MOCK_NETWORK.state, onNetworkStateChange: MOCK_NETWORK.subscribe, - getProvider: MOCK_NETWORK.getProvider, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, }, { interval: 5000, @@ -1305,4 +1318,141 @@ describe('TransactionController', () => { resolve(''); }); }); + + it('should increment nonce when adding a new non-cancel non-speedup transaction', async () => { + v1Stub + .mockImplementationOnce(() => 'aaaab1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d') + .mockImplementationOnce(() => 'bbbb1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'); + + await new Promise(async (resolve) => { + const controller = new TransactionController( + { + getNetworkState: () => MOCK_NETWORK.state, + onNetworkStateChange: MOCK_NETWORK.subscribe, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, + }, + { + sign: async (transaction: any) => transaction, + }, + ); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + await controller.addTransaction({ + from, + gas: '0x0', + gasPrice: '0x50fd51da', + to: from, + value: '0x0', + }); + + const firstTransaction = controller.state.transactions[0]; + await controller.approveTransaction(firstTransaction.id); + await controller.addTransaction({ + from, + gas: '0x2', + gasPrice: '0x50fd51da', + to: from, + value: '0x1290', + }); + expect(controller.state.transactions).toHaveLength(2); + const secondTransaction = controller.state.transactions[1]; + await controller.approveTransaction(secondTransaction.id); + + expect(firstTransaction.transaction.nonce).toStrictEqual('0x0'); + expect(secondTransaction.transaction.nonce).toStrictEqual('0x1'); + resolve(''); + }); + }); + + describe('NonceTracker integration', () => { + let getNonceLockSpy: jest.Mock; + let originalGetNonceLock: any; + const testNonce = 12; + + beforeEach(async () => { + originalGetNonceLock = NonceTracker.prototype.getNonceLock; + + getNonceLockSpy = jest.fn().mockResolvedValue({ + nextNonce: testNonce, + releaseLock: () => Promise.resolve(), + }); + + NonceTracker.prototype.getNonceLock = getNonceLockSpy; + }); + + afterEach(() => { + NonceTracker.prototype.getNonceLock = originalGetNonceLock; + }); + + it('should submit transaction with nonce from NonceTracker', async () => { + await new Promise(async (resolve) => { + const controller = new TransactionController( + { + getNetworkState: () => MOCK_CUSTOM_NETWORK.state, + onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe, + provider: MOCK_CUSTOM_NETWORK.provider, + blockTracker: MOCK_CUSTOM_NETWORK.blockTracker, + }, + { + sign: async (transaction: any) => transaction, + }, + ); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + await controller.addTransaction({ + from, + gas: '0x0', + gasPrice: '0x0', + to: from, + value: '0x0', + }); + + controller.hub.once( + `${controller.state.transactions[0].id}:finished`, + () => { + const { transaction, status } = controller.state.transactions[0]; + expect(transaction.from).toBe(from); + expect(transaction.nonce).toBe(`0x${testNonce.toString(16)}`); + expect(getNonceLockSpy).toHaveBeenCalledTimes(1); + expect(status).toBe(TransactionStatus.submitted); + resolve(''); + }, + ); + controller.approveTransaction(controller.state.transactions[0].id); + }); + }); + + it('should use the same nonce when speeding up a transaction', async () => { + await new Promise(async (resolve) => { + const controller = new TransactionController( + { + getNetworkState: () => MOCK_NETWORK.state, + onNetworkStateChange: MOCK_NETWORK.subscribe, + provider: MOCK_NETWORK.provider, + blockTracker: MOCK_NETWORK.blockTracker, + }, + { + sign: async (transaction: any) => transaction, + }, + ); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + await controller.addTransaction({ + from, + gas: '0x0', + gasPrice: '0x50fd51da', + to: from, + value: '0x0', + }); + + const originalTransaction = controller.state.transactions[0]; + await controller.approveTransaction(originalTransaction.id); + await controller.speedUpTransaction(originalTransaction.id); + expect(getNonceLockSpy).toHaveBeenCalledTimes(1); + expect(controller.state.transactions).toHaveLength(2); + expect(originalTransaction.transaction.nonce).toStrictEqual( + controller.state.transactions[1].transaction.nonce, + ); + resolve(''); + }); + }); + }); }); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 8020fc7605..dfa17643fc 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -12,7 +12,11 @@ import { BaseConfig, BaseState, } from '@metamask/base-controller'; -import type { NetworkState, ProviderProxy } from '@metamask/network-controller'; +import type { + NetworkState, + ProviderProxy, + BlockTrackerProxy, +} from '@metamask/network-controller'; import { BNToHex, fractionBN, @@ -23,7 +27,9 @@ import { NetworkType, RPC, } from '@metamask/controller-utils'; +import NonceTracker from 'nonce-tracker'; import { + getAndFormatTransactionsForNonceTracker, normalizeTransaction, validateTransaction, handleTransactionFetch, @@ -267,8 +273,12 @@ export class TransactionController extends BaseController< > { private ethQuery: any; + private nonceTracker: NonceTracker; + private registry: any; + private provider: ProviderProxy; + private handle?: ReturnType; private mutex = new Mutex(); @@ -410,7 +420,8 @@ export class TransactionController extends BaseController< * @param options - The controller options. * @param options.getNetworkState - Gets the state of the network controller. * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. - * @param options.getProvider - Returns a provider for the current network. + * @param options.provider - The provider used to create the underlying EthQuery instance. + * @param options.blockTracker - The block tracker used to poll for new blocks data. * @param config - Initial options used to configure this controller. * @param state - Initial state to set on this controller. */ @@ -418,11 +429,13 @@ export class TransactionController extends BaseController< { getNetworkState, onNetworkStateChange, - getProvider, + provider, + blockTracker, }: { getNetworkState: () => NetworkState; onNetworkStateChange: (listener: (state: NetworkState) => void) => void; - getProvider: () => ProviderProxy; + provider: ProviderProxy; + blockTracker: BlockTrackerProxy; }, config?: Partial, state?: Partial, @@ -438,14 +451,30 @@ export class TransactionController extends BaseController< transactions: [], }; this.initialize(); - const provider = getProvider(); + this.provider = provider; this.getNetworkState = getNetworkState; this.ethQuery = new EthQuery(provider); this.registry = new MethodRegistry({ provider }); + this.nonceTracker = new NonceTracker({ + provider, + blockTracker, + getPendingTransactions: (address) => + getAndFormatTransactionsForNonceTracker( + address, + TransactionStatus.submitted, + this.state.transactions, + ), + getConfirmedTransactions: (address) => + getAndFormatTransactionsForNonceTracker( + address, + TransactionStatus.confirmed, + this.state.transactions, + ), + }); + onNetworkStateChange(() => { - const newProvider = getProvider(); - this.ethQuery = new EthQuery(newProvider); - this.registry = new MethodRegistry({ provider: newProvider }); + this.ethQuery = new EthQuery(this.provider); + this.registry = new MethodRegistry({ provider: this.provider }); }); this.poll(); } @@ -625,10 +654,11 @@ export class TransactionController extends BaseController< const { chainId: currentChainId } = providerConfig; const index = transactions.findIndex(({ id }) => transactionID === id); const transactionMeta = transactions[index]; - const { nonce } = transactionMeta.transaction; - + const { + transaction: { nonce, from }, + } = transactionMeta; + let nonceLock; try { - const { from } = transactionMeta.transaction; if (!this.sign) { releaseLock(); this.failTransaction( @@ -644,21 +674,21 @@ export class TransactionController extends BaseController< const chainId = parseInt(currentChainId, undefined); const { approved: status } = TransactionStatus; - - const txNonce = - nonce || - (await query(this.ethQuery, 'getTransactionCount', [from, 'pending'])); + let nonceToUse = nonce; + // if a nonce already exists on the transactionMeta it means this is a speedup or cancel transaction + // so we want to reuse that nonce and hope that it beats the previous attempt to chain. Otherwise use a new locked nonce + if (!nonceToUse) { + nonceLock = await this.nonceTracker.getNonceLock(from); + nonceToUse = addHexPrefix(nonceLock.nextNonce.toString(16)); + } transactionMeta.status = status; - transactionMeta.transaction.nonce = txNonce; + transactionMeta.transaction.nonce = nonceToUse; transactionMeta.transaction.chainId = chainId; const baseTxParams = { ...transactionMeta.transaction, gasLimit: transactionMeta.transaction.gas, - chainId, - nonce: txNonce, - status, }; const isEIP1559 = isEIP1559Transaction(transactionMeta.transaction); @@ -698,6 +728,10 @@ export class TransactionController extends BaseController< } catch (error: any) { this.failTransaction(transactionMeta, error); } finally { + // must set transaction to submitted/failed before releasing lock + if (nonceLock) { + nonceLock.releaseLock(); + } releaseLock(); } } diff --git a/packages/transaction-controller/src/utils.test.ts b/packages/transaction-controller/src/utils.test.ts index dca3347f66..df6a93babb 100644 --- a/packages/transaction-controller/src/utils.test.ts +++ b/packages/transaction-controller/src/utils.test.ts @@ -1,9 +1,13 @@ +import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker'; import { Transaction, GasPriceValue, FeeMarketEIP1559Values, + TransactionStatus, } from './TransactionController'; import * as util from './utils'; +import type { TransactionMeta } from './TransactionController'; +import { getAndFormatTransactionsForNonceTracker } from './utils'; const MAX_FEE_PER_GAS = 'maxFeePerGas'; const MAX_PRIORITY_FEE_PER_GAS = 'maxPriorityFeePerGas'; @@ -281,4 +285,65 @@ describe('utils', () => { ).not.toThrow(Error); }); }); + + describe('getAndFormatTransactionsForNonceTracker', () => { + it('should return an array of formatted NonceTrackerTransaction objects filtered by fromAddress and transactionStatus', () => { + const fromAddress = '0x123'; + const inputTransactions: TransactionMeta[] = [ + { + id: '1', + time: 123456, + transaction: { + from: fromAddress, + gas: '0x100', + value: '0x200', + nonce: '0x1', + }, + status: TransactionStatus.confirmed, + }, + { + id: '2', + time: 123457, + transaction: { + from: '0x124', + gas: '0x101', + value: '0x201', + nonce: '0x2', + }, + status: TransactionStatus.submitted, + }, + { + id: '3', + time: 123458, + transaction: { + from: fromAddress, + gas: '0x102', + value: '0x202', + nonce: '0x3', + }, + status: TransactionStatus.approved, + }, + ]; + + const expectedResult: NonceTrackerTransaction[] = [ + { + status: TransactionStatus.confirmed, + history: [{}], + txParams: { + from: fromAddress, + gas: '0x100', + value: '0x200', + nonce: '0x1', + }, + }, + ]; + + const result = getAndFormatTransactionsForNonceTracker( + fromAddress, + TransactionStatus.confirmed, + inputTransactions, + ); + expect(result).toStrictEqual(expectedResult); + }); + }); }); diff --git a/packages/transaction-controller/src/utils.ts b/packages/transaction-controller/src/utils.ts index c6d6560fe7..f8dc8e8a5f 100644 --- a/packages/transaction-controller/src/utils.ts +++ b/packages/transaction-controller/src/utils.ts @@ -5,12 +5,15 @@ import { handleFetch, isValidHexAddress, } from '@metamask/controller-utils'; +import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker'; import { Transaction, FetchAllOptions, GasPriceValue, FeeMarketEIP1559Values, + TransactionStatus, } from './TransactionController'; +import type { TransactionMeta } from './TransactionController'; export const ESTIMATE_GAS_ERROR = 'eth_estimateGas rpc method error'; @@ -261,3 +264,39 @@ export function validateMinimumIncrease(proposed: string, min: string) { const errorMsg = `The proposed value: ${proposedDecimal} should meet or exceed the minimum value: ${minDecimal}`; throw new Error(errorMsg); } + +/** + * Helper function to filter and format transactions for the nonce tracker. + * + * @param fromAddress - Address of the account from which the transactions to filter from are sent. + * @param transactionStatus - Status of the transactions for which to filter. + * @param transactions - Array of transactionMeta objects that have been prefiltered. + * @returns Array of transactions formatted for the nonce tracker. + */ +export function getAndFormatTransactionsForNonceTracker( + fromAddress: string, + transactionStatus: TransactionStatus, + transactions: TransactionMeta[], +): NonceTrackerTransaction[] { + return transactions + .filter( + ({ status, transaction: { from } }) => + status === transactionStatus && + from.toLowerCase() === fromAddress.toLowerCase(), + ) + .map(({ status, transaction: { from, gas, value, nonce } }) => { + // the only value we care about is the nonce + // but we need to return the other values to satisfy the type + // TODO: refactor nonceTracker to not require this + return { + status, + history: [{}], + txParams: { + from: from ?? '', + gas: gas ?? '', + value: value ?? '', + nonce: nonce ?? '', + }, + }; + }); +} diff --git a/yarn.lock b/yarn.lock index 0d42c75eeb..627be8eda4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2045,6 +2045,7 @@ __metadata: ethereumjs-util: ^7.0.10 ethjs-provider-http: ^0.1.6 jest: ^26.4.2 + nonce-tracker: ^1.1.0 sinon: ^9.2.4 ts-jest: ^26.5.2 typedoc: ^0.22.15 @@ -3189,6 +3190,16 @@ __metadata: languageName: node linkType: hard +"assert@npm:^1.4.1": + version: 1.5.0 + resolution: "assert@npm:1.5.0" + dependencies: + object-assign: ^4.1.1 + util: 0.10.3 + checksum: 9be48435f726029ae7020c5888a3566bf4d617687aab280827f2e4029644b6515a9519ea10d018b342147c02faf73d9e9419e780e8937b3786ee4945a0ca71e5 + languageName: node + linkType: hard + "assert@npm:^2.0.0": version: 2.0.0 resolution: "assert@npm:2.0.0" @@ -3286,6 +3297,13 @@ __metadata: languageName: node linkType: hard +"await-semaphore@npm:^0.1.3": + version: 0.1.3 + resolution: "await-semaphore@npm:0.1.3" + checksum: 334c86541e446378dd832168de431327a77146f70cd80b57c99cd483ce5996e3bfdadea9d795e36f0b4faacb5121f5f7a99d94297ac2bdafbc690e5b0aa5cc32 + languageName: node + linkType: hard + "aws-sign2@npm:~0.7.0": version: 0.7.0 resolution: "aws-sign2@npm:0.7.0" @@ -5541,6 +5559,18 @@ __metadata: languageName: node linkType: hard +"ethjs-query@npm:^0.3.8": + version: 0.3.8 + resolution: "ethjs-query@npm:0.3.8" + dependencies: + babel-runtime: ^6.26.0 + ethjs-format: 0.2.7 + ethjs-rpc: 0.2.0 + promise-to-callback: ^1.0.0 + checksum: 6673167101e793dfdbb212f3ee2e7449a7c1eb7f4d72aba04a1c504eb36d9148f18c42a702839ebce7534709d821f32adcca84c56b7900788b2875cba8a371f1 + languageName: node + linkType: hard + "ethjs-rpc@npm:0.2.0": version: 0.2.0 resolution: "ethjs-rpc@npm:0.2.0" @@ -6742,6 +6772,13 @@ __metadata: languageName: node linkType: hard +"inherits@npm:2.0.1": + version: 2.0.1 + resolution: "inherits@npm:2.0.1" + checksum: 6536b9377296d4ce8ee89c5c543cb75030934e61af42dba98a428e7d026938c5985ea4d1e3b87743a5b834f40ed1187f89c2d7479e9d59e41d2d1051aefba07b + languageName: node + linkType: hard + "internal-slot@npm:^1.0.3": version: 1.0.3 resolution: "internal-slot@npm:1.0.3" @@ -9029,6 +9066,17 @@ __metadata: languageName: node linkType: hard +"nonce-tracker@npm:^1.1.0": + version: 1.1.0 + resolution: "nonce-tracker@npm:1.1.0" + dependencies: + assert: ^1.4.1 + await-semaphore: ^0.1.3 + ethjs-query: ^0.3.8 + checksum: fbed4eac51a5df3922a9ee93f2d40f700d3c86e03ac02acc5c7983e9fd5c9d97278f1f5d8422f2dad0f0108c09a2d880145906cae19604717cc898d385bbdff9 + languageName: node + linkType: hard + "nopt@npm:^5.0.0": version: 5.0.0 resolution: "nopt@npm:5.0.0" @@ -9148,7 +9196,7 @@ __metadata: languageName: node linkType: hard -"object-assign@npm:^4.1.0": +"object-assign@npm:^4.1.0, object-assign@npm:^4.1.1": version: 4.1.1 resolution: "object-assign@npm:4.1.1" checksum: fcc6e4ea8c7fe48abfbb552578b1c53e0d194086e2e6bbbf59e0a536381a292f39943c6e9628af05b5528aa5e3318bb30d6b2e53cadaf5b8fe9e12c4b69af23f @@ -11835,6 +11883,15 @@ __metadata: languageName: node linkType: hard +"util@npm:0.10.3": + version: 0.10.3 + resolution: "util@npm:0.10.3" + dependencies: + inherits: 2.0.1 + checksum: bd800f5d237a82caddb61723a6cbe45297d25dd258651a31335a4d5d981fd033cb4771f82db3d5d59b582b187cb69cfe727dc6f4d8d7826f686ee6c07ce611e0 + languageName: node + linkType: hard + "util@npm:^0.12.0": version: 0.12.4 resolution: "util@npm:0.12.4"