From a16af56f965a28744fb66c2ef7ef87d7778527ce Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Sun, 6 Aug 2023 13:44:33 +0100 Subject: [PATCH 1/3] Use block tracker to trigger incoming transaction check --- .../transaction-controller/jest.config.js | 2 +- .../EtherscanRemoteTransactionSource.test.ts | 36 +- .../src/EtherscanRemoteTransactionSource.ts | 38 +- .../src/IncomingTransactionHelper.test.ts | 549 ++++++++++++------ .../src/IncomingTransactionHelper.ts | 358 ++++++------ .../src/TransactionController.test.ts | 128 +--- .../src/TransactionController.ts | 74 +-- .../transaction-controller/src/constants.ts | 122 ++++ .../src/etherscan.test.ts | 74 ++- .../transaction-controller/src/etherscan.ts | 89 +-- packages/transaction-controller/src/types.ts | 12 +- 11 files changed, 903 insertions(+), 579 deletions(-) create mode 100644 packages/transaction-controller/src/constants.ts diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index fe9ca89fab..19823619e9 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -18,7 +18,7 @@ module.exports = merge(baseConfig, { coverageThreshold: { global: { branches: 81.29, - functions: 95.14, + functions: 94.64, lines: 95.24, statements: 95.34, }, diff --git a/packages/transaction-controller/src/EtherscanRemoteTransactionSource.test.ts b/packages/transaction-controller/src/EtherscanRemoteTransactionSource.test.ts index 47a00542fa..7d95282376 100644 --- a/packages/transaction-controller/src/EtherscanRemoteTransactionSource.test.ts +++ b/packages/transaction-controller/src/EtherscanRemoteTransactionSource.test.ts @@ -1,5 +1,6 @@ import { v1 as random } from 'uuid'; +import { CHAIN_IDS } from './constants'; import type { EtherscanTokenTransactionMeta, EtherscanTransactionMeta, @@ -63,7 +64,6 @@ const ETHERSCAN_TOKEN_TRANSACTION_MOCK: EtherscanTokenTransactionMeta = { const ETHERSCAN_TRANSACTION_RESPONSE_MOCK: EtherscanTransactionResponse = { - status: '1', result: [ ETHERSCAN_TRANSACTION_SUCCESS_MOCK, ETHERSCAN_TRANSACTION_ERROR_MOCK, @@ -72,7 +72,6 @@ const ETHERSCAN_TRANSACTION_RESPONSE_MOCK: EtherscanTransactionResponse = { - status: '1', result: [ ETHERSCAN_TOKEN_TRANSACTION_MOCK, ETHERSCAN_TOKEN_TRANSACTION_MOCK, @@ -81,7 +80,6 @@ const ETHERSCAN_TOKEN_TRANSACTION_RESPONSE_MOCK: EtherscanTransactionResponse = { - status: '0', result: [], }; @@ -159,6 +157,26 @@ describe('EtherscanRemoteTransactionSource', () => { randomMock.mockReturnValue(ID_MOCK); }); + describe('isSupportedNetwork', () => { + it('returns true if chain ID in constant', () => { + expect( + new EtherscanRemoteTransactionSource().isSupportedNetwork( + CHAIN_IDS.MAINNET, + '1', + ), + ).toBe(true); + }); + + it('returns false if chain ID not in constant', () => { + expect( + new EtherscanRemoteTransactionSource().isSupportedNetwork( + '0x1324567891234', + '1', + ), + ).toBe(false); + }); + }); + describe('fetchTransactions', () => { it('returns normalized transactions fetched from Etherscan', async () => { fetchEtherscanTransactionsMock.mockResolvedValueOnce( @@ -191,5 +209,17 @@ describe('EtherscanRemoteTransactionSource', () => { EXPECTED_NORMALISED_TOKEN_TRANSACTION, ]); }); + + it('returns no normalized token transactions if flag disabled', async () => { + fetchEtherscanTokenTransactionsMock.mockResolvedValueOnce( + ETHERSCAN_TOKEN_TRANSACTION_RESPONSE_MOCK, + ); + + const transactions = await new EtherscanRemoteTransactionSource({ + includeTokenTransfers: false, + }).fetchTransactions({} as any); + + expect(transactions).toStrictEqual([]); + }); }); }); diff --git a/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts b/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts index 77af7b741d..49b44437b2 100644 --- a/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts +++ b/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts @@ -3,10 +3,12 @@ import type { Hex } from '@metamask/utils'; import { BN } from 'ethereumjs-util'; import { v1 as random } from 'uuid'; +import { ETHERSCAN_SUPPORTED_NETWORKS } from './constants'; import type { EtherscanTokenTransactionMeta, EtherscanTransactionMeta, EtherscanTransactionMetaBase, + EtherscanTransactionResponse, } from './etherscan'; import { fetchEtherscanTokenTransactions, @@ -25,20 +27,36 @@ import { TransactionStatus } from './types'; export class EtherscanRemoteTransactionSource implements RemoteTransactionSource { - /** - * Retrieve transaction data from Etherscan. - * - * @param request - The configuration required to fetch Etherscan transaction data. - * @returns An array of transaction metadata. - */ + #includeTokenTransfers: boolean; + + constructor({ + includeTokenTransfers, + }: { includeTokenTransfers?: boolean } = {}) { + this.#includeTokenTransfers = includeTokenTransfers ?? true; + } + + isSupportedNetwork(chainId: Hex, _networkId: string): boolean { + return Object.keys(ETHERSCAN_SUPPORTED_NETWORKS).includes(chainId); + } + async fetchTransactions( request: RemoteTransactionSourceRequest, ): Promise { + const etherscanRequest = { + ...request, + chainId: request.currentChainId, + }; + + const transactionPromise = fetchEtherscanTransactions(etherscanRequest); + + const tokenTransactionPromise = this.#includeTokenTransfers + ? fetchEtherscanTokenTransactions(etherscanRequest) + : Promise.resolve({ + result: [] as EtherscanTokenTransactionMeta[], + } as EtherscanTransactionResponse); + const [etherscanTransactions, etherscanTokenTransactions] = - await Promise.all([ - fetchEtherscanTransactions(request), - fetchEtherscanTokenTransactions(request), - ]); + await Promise.all([transactionPromise, tokenTransactionPromise]); const transactions = etherscanTransactions.result.map((tx) => this.#normalizeTransaction( diff --git a/packages/transaction-controller/src/IncomingTransactionHelper.test.ts b/packages/transaction-controller/src/IncomingTransactionHelper.test.ts index 10bddfc42e..cfdba86bff 100644 --- a/packages/transaction-controller/src/IncomingTransactionHelper.test.ts +++ b/packages/transaction-controller/src/IncomingTransactionHelper.test.ts @@ -1,10 +1,14 @@ -import { NetworkType, isSmartContractCode } from '@metamask/controller-utils'; -import type EthQuery from '@metamask/eth-query'; -import type { NetworkState } from '@metamask/network-controller'; +/* eslint-disable jsdoc/require-jsdoc */ + +import { NetworkType } from '@metamask/controller-utils'; +import type { BlockTracker, NetworkState } from '@metamask/network-controller'; import { IncomingTransactionHelper } from './IncomingTransactionHelper'; -import type { RemoteTransactionSource, TransactionMeta } from './types'; -import { TransactionStatus } from './types'; +import { + TransactionStatus, + type RemoteTransactionSource, + type TransactionMeta, +} from './types'; jest.mock('@metamask/controller-utils', () => ({ ...jest.requireActual('@metamask/controller-utils'), @@ -20,56 +24,104 @@ const NETWORK_STATE_MOCK: NetworkState = { networkId: '1', } as unknown as NetworkState; +const ADDERSS_MOCK = '0x1'; +const FROM_BLOCK_HEX_MOCK = '0x20'; +const FROM_BLOCK_DECIMAL_MOCK = 32; + +const BLOCK_TRACKER_MOCK = { + addListener: jest.fn(), + removeListener: jest.fn(), +} as unknown as jest.Mocked; + const CONTROLLER_ARGS_MOCK = { + blockTracker: BLOCK_TRACKER_MOCK, + getCurrentAccount: () => ADDERSS_MOCK, getNetworkState: () => NETWORK_STATE_MOCK, - getEthQuery: () => ({} as unknown as EthQuery), - transactionLimit: 1, remoteTransactionSource: {} as RemoteTransactionSource, + transactionLimit: 1, }; const TRANSACTION_MOCK: TransactionMeta = { - transactionHash: '0x1', - transaction: { to: '0x1', gasUsed: '0x1' }, - time: 0, - status: TransactionStatus.submitted, - blockNumber: '1', + blockNumber: '123', chainId: '0x1', + status: TransactionStatus.submitted, + time: 0, + transaction: { to: '0x1', gasUsed: '0x1' }, + transactionHash: '0x1', } as unknown as TransactionMeta; const TRANSACTION_MOCK_2: TransactionMeta = { - transactionHash: '0x2', - transaction: { to: '0x1' }, - time: 1, - blockNumber: '2', + blockNumber: '234', chainId: '0x1', + time: 1, + transaction: { to: '0x1' }, + transactionHash: '0x2', } as unknown as TransactionMeta; -const RECONCILE_ARGS_MOCK = { - address: '0x1', - localTransactions: [], - fromBlock: '0x3', - apiKey: 'testApiKey', -}; - const createRemoteTransactionSourceMock = ( remoteTransactions: TransactionMeta[], + { + isSupportedNetwork, + error, + }: { isSupportedNetwork?: boolean; error?: boolean } = {}, ): RemoteTransactionSource => ({ - fetchTransactions: jest.fn(() => Promise.resolve(remoteTransactions)), + isSupportedNetwork: jest.fn(() => isSupportedNetwork ?? true), + fetchTransactions: jest.fn(() => + error + ? Promise.reject(new Error('Test Error')) + : Promise.resolve(remoteTransactions), + ), }); -describe('IncomingTransactionHelper', () => { - const isSmartContractCodeMock = isSmartContractCode as jest.MockedFn< - typeof isSmartContractCode - >; +async function emitBlockTrackerLatestEvent( + helper: IncomingTransactionHelper, + { start, error }: { start?: boolean; error?: boolean } = {}, +) { + const transactionsListener = jest.fn(); + const blockNumberListener = jest.fn(); + + if (error) { + transactionsListener.mockImplementation(() => { + throw new Error('Test Error'); + }); + } + helper.hub.addListener('transactions', transactionsListener); + helper.hub.addListener('updatedLastFetchedBlockNumbers', blockNumberListener); + + if (start !== false) { + helper.start(); + } + + await BLOCK_TRACKER_MOCK.addListener.mock.calls[0]?.[1]?.( + FROM_BLOCK_HEX_MOCK, + ); + + return { + transactions: transactionsListener.mock.calls[0]?.[0], + lastFetchBlockNumbers: blockNumberListener.mock.calls[0]?.[0], + transactionsListener, + blockNumberListener, + }; +} + +describe('IncomingTransactionHelper', () => { beforeEach(() => { jest.resetAllMocks(); - - TRANSACTION_MOCK.toSmartContract = undefined; - TRANSACTION_MOCK_2.toSmartContract = undefined; }); - describe('reconcile', () => { + describe('on block tracker latest event', () => { + it('handles errors', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([ + TRANSACTION_MOCK_2, + ]), + }); + + await emitBlockTrackerLatestEvent(helper, { error: true }); + }); + describe('fetches remote transactions', () => { it('using remote transaction source', async () => { const remoteTransactionSource = createRemoteTransactionSourceMock([]); @@ -79,62 +131,70 @@ describe('IncomingTransactionHelper', () => { remoteTransactionSource, }); - await helper.reconcile(RECONCILE_ARGS_MOCK); + await emitBlockTrackerLatestEvent(helper); expect(remoteTransactionSource.fetchTransactions).toHaveBeenCalledTimes( 1, ); expect(remoteTransactionSource.fetchTransactions).toHaveBeenCalledWith({ - address: RECONCILE_ARGS_MOCK.address, - apiKey: RECONCILE_ARGS_MOCK.apiKey, + address: ADDERSS_MOCK, currentChainId: NETWORK_STATE_MOCK.providerConfig.chainId, currentNetworkId: NETWORK_STATE_MOCK.networkId, - fromBlock: RECONCILE_ARGS_MOCK.fromBlock, + fromBlock: expect.any(Number), limit: CONTROLLER_ARGS_MOCK.transactionLimit, - networkType: NETWORK_STATE_MOCK.providerConfig.type, }); }); - it('sorts transactions by time in ascending order', async () => { - const firstTransaction = { ...TRANSACTION_MOCK_2, time: 5 }; - const secondTransaction = { ...TRANSACTION_MOCK, time: 6 }; + it('using from block as latest block minus ten if no last fetched data', async () => { + const remoteTransactionSource = createRemoteTransactionSourceMock([]); const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, - remoteTransactionSource: createRemoteTransactionSourceMock([ - firstTransaction, - ]), + remoteTransactionSource, }); - const { transactions } = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [secondTransaction], - }); + await emitBlockTrackerLatestEvent(helper); - expect(transactions).toStrictEqual([ - firstTransaction, - secondTransaction, - ]); + expect(remoteTransactionSource.fetchTransactions).toHaveBeenCalledTimes( + 1, + ); + + expect(remoteTransactionSource.fetchTransactions).toHaveBeenCalledWith( + expect.objectContaining({ + fromBlock: FROM_BLOCK_DECIMAL_MOCK - 10, + }), + ); }); - }); - describe('returns update required', () => { - it('as false if current network is not supported', async () => { + it('using from block as last fetched value plus one', async () => { + const remoteTransactionSource = createRemoteTransactionSourceMock([]); + const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, - getNetworkState: () => ({ ...NETWORK_STATE_MOCK, networkId: '0x2' }), + remoteTransactionSource, + lastFetchedBlockNumbers: { + [`${NETWORK_STATE_MOCK.providerConfig.chainId}#${ADDERSS_MOCK}`]: + FROM_BLOCK_DECIMAL_MOCK, + }, }); - const result = await helper.reconcile(RECONCILE_ARGS_MOCK); + await emitBlockTrackerLatestEvent(helper); - expect(result).toStrictEqual({ - updateRequired: false, - transactions: [], - }); + expect(remoteTransactionSource.fetchTransactions).toHaveBeenCalledTimes( + 1, + ); + + expect(remoteTransactionSource.fetchTransactions).toHaveBeenCalledWith( + expect.objectContaining({ + fromBlock: FROM_BLOCK_DECIMAL_MOCK + 1, + }), + ); }); + }); - it('as true if new transaction fetched', async () => { + describe('emits transactions event', () => { + it('if new transaction fetched', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ @@ -142,19 +202,35 @@ describe('IncomingTransactionHelper', () => { ]), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [TRANSACTION_MOCK], + const { transactions } = await emitBlockTrackerLatestEvent(helper); + + expect(transactions).toStrictEqual([TRANSACTION_MOCK_2]); + }); + + it('if new outgoing transaction fetched and update transactions enabled', async () => { + const outgoingTransaction = { + ...TRANSACTION_MOCK_2, + transaction: { + ...TRANSACTION_MOCK_2.transaction, + from: '0x1', + to: '0x2', + }, + }; + + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([ + outgoingTransaction, + ]), + updateTransactions: true, }); - expect(result.updateRequired).toBe(true); - expect(result.transactions).toStrictEqual([ - TRANSACTION_MOCK, - TRANSACTION_MOCK_2, - ]); + const { transactions } = await emitBlockTrackerLatestEvent(helper); + + expect(transactions).toStrictEqual([outgoingTransaction]); }); - it('as true if existing transaction fetched with different status', async () => { + it('if existing transaction fetched with different status and update transactions enabled', async () => { const updatedTransaction = { ...TRANSACTION_MOCK, status: TransactionStatus.confirmed, @@ -165,18 +241,16 @@ describe('IncomingTransactionHelper', () => { remoteTransactionSource: createRemoteTransactionSourceMock([ updatedTransaction, ]), + getLocalTransactions: () => [TRANSACTION_MOCK], + updateTransactions: true, }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [TRANSACTION_MOCK], - }); + const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(result.updateRequired).toBe(true); - expect(result.transactions).toStrictEqual([updatedTransaction]); + expect(transactions).toStrictEqual([updatedTransaction]); }); - it('as true if existing transaction fetched with different gas used', async () => { + it('if existing transaction fetched with different gas used and update transactions enabled', async () => { const updatedTransaction = { ...TRANSACTION_MOCK, transaction: { @@ -190,133 +264,160 @@ describe('IncomingTransactionHelper', () => { remoteTransactionSource: createRemoteTransactionSourceMock([ updatedTransaction, ]), + getLocalTransactions: () => [TRANSACTION_MOCK], + updateTransactions: true, }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [TRANSACTION_MOCK], - }); + const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(result.updateRequired).toBe(true); - expect(result.transactions).toStrictEqual([updatedTransaction]); + expect(transactions).toStrictEqual([updatedTransaction]); }); - }); - describe('returns latest block number', () => { - it('of transactions with matching chain ID and to address', async () => { + it('sorted by time in ascending order', async () => { + const firstTransaction = { ...TRANSACTION_MOCK, time: 5 }; + const secondTransaction = { ...TRANSACTION_MOCK, time: 6 }; + const thirdTransaction = { ...TRANSACTION_MOCK, time: 7 }; + const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ - TRANSACTION_MOCK_2, + firstTransaction, + thirdTransaction, + secondTransaction, ]), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [TRANSACTION_MOCK], - }); + const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(result.latestBlockNumber).toBeDefined(); - expect(result.latestBlockNumber).toStrictEqual( - TRANSACTION_MOCK_2.blockNumber, - ); + expect(transactions).toStrictEqual([ + firstTransaction, + secondTransaction, + thirdTransaction, + ]); }); - it('of transactions with matching network ID if no chain ID', async () => { + it('does not if identical transaction fetched and update transactions enabled', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ - { ...TRANSACTION_MOCK_2, chainId: undefined, networkID: '1' }, + TRANSACTION_MOCK, ]), + getLocalTransactions: () => [TRANSACTION_MOCK], + updateTransactions: true, }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [ - { ...TRANSACTION_MOCK, chainId: undefined, networkID: '1' }, - ], - }); - - expect(result.latestBlockNumber).toBeDefined(); - expect(result.latestBlockNumber).toStrictEqual( - TRANSACTION_MOCK_2.blockNumber, + const { transactionsListener } = await emitBlockTrackerLatestEvent( + helper, ); + + expect(transactionsListener).not.toHaveBeenCalled(); }); - it('of transactions with block number', async () => { + it('does not if disabled', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ - { ...TRANSACTION_MOCK_2, blockNumber: undefined }, + TRANSACTION_MOCK, ]), + isEnabled: jest + .fn() + .mockReturnValueOnce(true) + .mockReturnValueOnce(false), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [TRANSACTION_MOCK], + const { transactionsListener } = await emitBlockTrackerLatestEvent( + helper, + ); + + expect(transactionsListener).not.toHaveBeenCalled(); + }); + + it('does not if current network is not supported by remote transaction source', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock( + [TRANSACTION_MOCK], + { isSupportedNetwork: false }, + ), }); - expect(result.latestBlockNumber).toBeDefined(); - expect(result.latestBlockNumber).toStrictEqual( - TRANSACTION_MOCK.blockNumber, + const { transactionsListener } = await emitBlockTrackerLatestEvent( + helper, ); + + expect(transactionsListener).not.toHaveBeenCalled(); }); - }); - describe('updates toSmartContract property on transactions', () => { - it('to false if no to address', async () => { + it('does not if no remote transactions', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([]), + }); + + const { transactionsListener } = await emitBlockTrackerLatestEvent( + helper, + ); + + expect(transactionsListener).not.toHaveBeenCalled(); + }); + + it('does not if update transactions disabled and no incoming transactions', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ { - ...TRANSACTION_MOCK_2, - transaction: { ...TRANSACTION_MOCK_2.transaction, to: undefined }, - }, + ...TRANSACTION_MOCK, + transaction: { to: '0x2' }, + } as TransactionMeta, + { + ...TRANSACTION_MOCK, + transaction: { to: undefined }, + } as TransactionMeta, ]), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [ - { - ...TRANSACTION_MOCK, - transaction: { ...TRANSACTION_MOCK.transaction, to: undefined }, - }, - ], + const { transactionsListener } = await emitBlockTrackerLatestEvent( + helper, + ); + + expect(transactionsListener).not.toHaveBeenCalled(); + }); + + it('does not if error fetching transactions', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock( + [TRANSACTION_MOCK], + { error: true }, + ), }); - expect(result.transactions[0].toSmartContract).toBe(false); - expect(result.transactions[1].toSmartContract).toBe(false); + const { transactionsListener } = await emitBlockTrackerLatestEvent( + helper, + ); + + expect(transactionsListener).not.toHaveBeenCalled(); }); - it('to false if data is explicitly empty', async () => { + it('does not if not started', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ - { - ...TRANSACTION_MOCK_2, - transaction: { ...TRANSACTION_MOCK_2.transaction, data: '0x' }, - }, + TRANSACTION_MOCK, ]), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [ - { - ...TRANSACTION_MOCK, - transaction: { ...TRANSACTION_MOCK.transaction, data: '0x' }, - }, - ], - }); + const { transactionsListener } = await emitBlockTrackerLatestEvent( + helper, + { start: false }, + ); - expect(result.transactions[0].toSmartContract).toBe(false); - expect(result.transactions[1].toSmartContract).toBe(false); + expect(transactionsListener).not.toHaveBeenCalled(); }); + }); - it('to false if isSmartContractCode returns false', async () => { - isSmartContractCodeMock.mockReturnValue(false); - + describe('emits updatedLastFetchedBlockNumbers event', () => { + it('if fetched transaction has higher block number', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ @@ -324,50 +425,154 @@ describe('IncomingTransactionHelper', () => { ]), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [TRANSACTION_MOCK], - }); + const { lastFetchBlockNumbers } = await emitBlockTrackerLatestEvent( + helper, + ); - expect(result.transactions[0].toSmartContract).toBe(false); - expect(result.transactions[1].toSmartContract).toBe(false); + expect(lastFetchBlockNumbers).toStrictEqual({ + [`${NETWORK_STATE_MOCK.providerConfig.chainId}#${ADDERSS_MOCK}`]: + parseInt(TRANSACTION_MOCK_2.blockNumber as string, 10), + }); }); - it('to true if isSmartContractCode returns true', async () => { - isSmartContractCodeMock.mockReturnValue(true); + it('does not if no fetched transactions', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([]), + }); + const { blockNumberListener } = await emitBlockTrackerLatestEvent( + helper, + ); + + expect(blockNumberListener).not.toHaveBeenCalled(); + }); + + it('does not if no block number on fetched transaction', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ - TRANSACTION_MOCK_2, + { ...TRANSACTION_MOCK_2, blockNumber: undefined }, ]), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [TRANSACTION_MOCK], - }); + const { blockNumberListener } = await emitBlockTrackerLatestEvent( + helper, + ); - expect(result.transactions[0].toSmartContract).toBe(true); - expect(result.transactions[1].toSmartContract).toBe(true); + expect(blockNumberListener).not.toHaveBeenCalled(); }); - it('to existing value if already set', async () => { + it('does not if fetch transaction not to current account', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, remoteTransactionSource: createRemoteTransactionSourceMock([ - { ...TRANSACTION_MOCK_2, toSmartContract: false }, + { + ...TRANSACTION_MOCK_2, + transaction: { to: '0x2' }, + } as TransactionMeta, ]), }); - const result = await helper.reconcile({ - ...RECONCILE_ARGS_MOCK, - localTransactions: [{ ...TRANSACTION_MOCK, toSmartContract: true }], + const { blockNumberListener } = await emitBlockTrackerLatestEvent( + helper, + ); + + expect(blockNumberListener).not.toHaveBeenCalled(); + }); + + it('does not if fetched transaction has same block number', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([ + TRANSACTION_MOCK_2, + ]), + lastFetchedBlockNumbers: { + [`${NETWORK_STATE_MOCK.providerConfig.chainId}#${ADDERSS_MOCK}`]: + parseInt(TRANSACTION_MOCK_2.blockNumber as string, 10), + }, }); - expect(result.transactions[0].toSmartContract).toBe(true); - expect(result.transactions[1].toSmartContract).toBe(false); + const { blockNumberListener } = await emitBlockTrackerLatestEvent( + helper, + ); + + expect(blockNumberListener).not.toHaveBeenCalled(); + }); + }); + }); + + describe('start', () => { + it('adds listener to block tracker', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([]), }); + + helper.start(); + + expect( + CONTROLLER_ARGS_MOCK.blockTracker.addListener, + ).toHaveBeenCalledTimes(1); + }); + + it('does nothing if already started', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([]), + }); + + helper.start(); + helper.start(); + + expect( + CONTROLLER_ARGS_MOCK.blockTracker.addListener, + ).toHaveBeenCalledTimes(1); + }); + + it('does nothing if disabled', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + isEnabled: () => false, + remoteTransactionSource: createRemoteTransactionSourceMock([]), + }); + + helper.start(); + + expect( + CONTROLLER_ARGS_MOCK.blockTracker.addListener, + ).not.toHaveBeenCalled(); + }); + + it('does nothing if network not supported by remote transaction source', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([], { + isSupportedNetwork: false, + }), + }); + + helper.start(); + + expect( + CONTROLLER_ARGS_MOCK.blockTracker.addListener, + ).not.toHaveBeenCalled(); + }); + }); + + describe('stop', () => { + it('removes listener from block tracker', async () => { + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([]), + }); + + helper.start(); + helper.stop(); + + expect( + CONTROLLER_ARGS_MOCK.blockTracker.removeListener, + ).toHaveBeenCalledTimes(1); }); }); }); diff --git a/packages/transaction-controller/src/IncomingTransactionHelper.ts b/packages/transaction-controller/src/IncomingTransactionHelper.ts index f47a280a4f..1d2eb4966c 100644 --- a/packages/transaction-controller/src/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/IncomingTransactionHelper.ts @@ -1,162 +1,143 @@ -import { isSmartContractCode, query } from '@metamask/controller-utils'; -import type EthQuery from '@metamask/eth-query'; -import type { NetworkState } from '@metamask/network-controller'; +import type { BlockTracker, NetworkState } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; +import EventEmitter from 'events'; -import type { - RemoteTransactionSource, - Transaction, - TransactionMeta, - TransactionStatus, -} from './types'; - -const SUPPORTED_NETWORK_IDS = [ - '1', // Mainnet - '5', // Goerli - '11155111', // Sepolia +import type { RemoteTransactionSource, TransactionMeta } from './types'; + +const UPDATE_CHECKS: ((txMeta: TransactionMeta) => any)[] = [ + (txMeta) => txMeta.status, + (txMeta) => txMeta.transaction.gasUsed, ]; export class IncomingTransactionHelper { + hub: EventEmitter; + + #blockTracker: BlockTracker; + + #getCurrentAccount: () => string; + + #getLocalTransactions: () => TransactionMeta[]; + #getNetworkState: () => NetworkState; - #getEthQuery: () => EthQuery; + #isEnabled: () => boolean; - #transactionLimit: number; + #isRunning: boolean; + + #lastFetchedBlockNumbers: Record; + + #onLatestBlock: (blockNumberHex: Hex) => Promise; #remoteTransactionSource: RemoteTransactionSource; + #transactionLimit?: number; + + #updateTransactions: boolean; + constructor({ + blockTracker, + getCurrentAccount, + getLocalTransactions, getNetworkState, - getEthQuery, - transactionLimit, + isEnabled, + lastFetchedBlockNumbers, remoteTransactionSource, + transactionLimit, + updateTransactions, }: { + blockTracker: BlockTracker; + getCurrentAccount: () => string; getNetworkState: () => NetworkState; - getEthQuery: () => EthQuery; - transactionLimit: number; + getLocalTransactions?: () => TransactionMeta[]; + isEnabled?: () => boolean; + lastFetchedBlockNumbers?: Record; remoteTransactionSource: RemoteTransactionSource; + transactionLimit?: number; + updateTransactions?: boolean; }) { + this.hub = new EventEmitter(); + + this.#blockTracker = blockTracker; + this.#getCurrentAccount = getCurrentAccount; + this.#getLocalTransactions = getLocalTransactions || (() => []); this.#getNetworkState = getNetworkState; - this.#getEthQuery = getEthQuery; - this.#transactionLimit = transactionLimit; + this.#isEnabled = isEnabled ?? (() => true); + this.#isRunning = false; + this.#lastFetchedBlockNumbers = lastFetchedBlockNumbers ?? {}; this.#remoteTransactionSource = remoteTransactionSource; + this.#transactionLimit = transactionLimit; + this.#updateTransactions = updateTransactions ?? false; + + // Using a property instead of a method to provide a listener reference + // with the correct scope that we can remove later if stopped. + this.#onLatestBlock = async (blockNumberHex: Hex) => { + try { + await this.#update(blockNumberHex); + } catch (error) { + console.error('Error while checking incoming transactions', error); + } + }; } - async reconcile({ - address, - localTransactions, - fromBlock, - apiKey, - }: { - address: string; - localTransactions: TransactionMeta[]; - fromBlock?: string; - apiKey?: string; - }): Promise<{ - updateRequired: boolean; - transactions: TransactionMeta[]; - latestBlockNumber?: string; - }> { - const { providerConfig, networkId: currentNetworkId } = - this.#getNetworkState(); - const { chainId: currentChainId, type: networkType } = providerConfig; - - if ( - currentNetworkId === null || - !SUPPORTED_NETWORK_IDS.includes(currentNetworkId) - ) { - return { updateRequired: false, transactions: [] }; + start() { + if (this.#isRunning) { + return; } - const remoteTransactions = - await this.#remoteTransactionSource.fetchTransactions({ - address, - networkType, - limit: this.#transactionLimit, - currentChainId, - currentNetworkId, - fromBlock, - apiKey, - }); - - const [updateRequired, transactions] = this.#reconcileTransactions( - localTransactions, - remoteTransactions, - ); - - this.#sortTransactionsByTime(transactions); - - const latestBlockNumber = this.#getLatestBlockNumber( - transactions, - address, - currentChainId, - currentNetworkId, - ); - - await this.#updateSmartContractProperty(transactions); + if (!this.#canStart()) { + return; + } - return { updateRequired, transactions, latestBlockNumber }; + this.#blockTracker.addListener('latest', this.#onLatestBlock); + this.#isRunning = true; } - async #updateSmartContractProperty(transactions: TransactionMeta[]) { - await Promise.all( - transactions.map(async (tx) => { - tx.toSmartContract ??= await this.#isToSmartContract(tx.transaction); - }), - ); + stop() { + this.#blockTracker.removeListener('latest', this.#onLatestBlock); + this.#isRunning = false; } - #getLatestBlockNumber( - transactions: TransactionMeta[], - address: string, - currentChainId: Hex, - currentNetworkId: string, - ): string | undefined { - let latestBlockNumber: string | undefined; - - for (const tx of transactions) { - const onCurrentChain = - tx.chainId === currentChainId || - (!tx.chainId && tx.networkID === currentNetworkId); - - const toCurrentAccount = - tx.transaction.to?.toLowerCase() === address.toLowerCase(); - - const currentBlockNumberValue = tx.blockNumber - ? parseInt(tx.blockNumber, 10) - : -1; - - const latestBlockNumberValue = latestBlockNumber - ? parseInt(latestBlockNumber, 10) - : -1; - - if ( - onCurrentChain && - toCurrentAccount && - latestBlockNumberValue < currentBlockNumberValue - ) { - latestBlockNumber = tx.blockNumber; - } + async #update(latestBlockNumberHex: Hex): Promise { + if (!this.#canStart()) { + return; } - return latestBlockNumber; - } - - async #isToSmartContract(transaction: Transaction): Promise { - // Contract Deploy - if (!transaction.to) { - return false; + const latestBlockNumber = parseInt(latestBlockNumberHex, 16); + const fromBlock = this.#getFromBlock(latestBlockNumber); + const address = this.#getCurrentAccount(); + const currentChainId = this.#getCurrentChainId(); + const currentNetworkId = this.#getCurrentNetworkId(); + + let remoteTransactions = []; + + try { + remoteTransactions = + await this.#remoteTransactionSource.fetchTransactions({ + address, + currentChainId, + currentNetworkId, + fromBlock, + limit: this.#transactionLimit, + }); + } catch (error: any) { + return; } - // Send - if (transaction.data === '0x') { - return false; + if (!this.#updateTransactions) { + remoteTransactions = remoteTransactions.filter( + (tx) => tx.transaction.to?.toLowerCase() === address.toLowerCase(), + ); } - const ethQuery = this.#getEthQuery(); - const code = await query(ethQuery, 'getCode', [transaction.to]); + this.#updateLastFetchedBlockNumber(remoteTransactions); - return isSmartContractCode(code); + const [updateRequired, transactions] = + this.#reconcileTransactions(remoteTransactions); + + if (updateRequired) { + this.#sortTransactionsByTime(transactions); + this.hub.emit('transactions', transactions); + } } #sortTransactionsByTime(transactions: TransactionMeta[]) { @@ -164,9 +145,14 @@ export class IncomingTransactionHelper { } #reconcileTransactions( - localTxs: TransactionMeta[], remoteTxs: TransactionMeta[], ): [boolean, TransactionMeta[]] { + if (!this.#updateTransactions) { + return [remoteTxs.length > 0, remoteTxs]; + } + + const localTxs = this.#getLocalTransactions(); + const updatedTxs: TransactionMeta[] = this.#getUpdatedTransactions( remoteTxs, localTxs, @@ -177,15 +163,8 @@ export class IncomingTransactionHelper { localTxs, ); - const updatedLocalTxs = localTxs.map((tx: TransactionMeta) => { - const txIdx = updatedTxs.findIndex( - ({ transactionHash }) => transactionHash === tx.transactionHash, - ); - return txIdx === -1 ? tx : updatedTxs[txIdx]; - }); - const updateRequired = newTxs.length > 0 || updatedTxs.length > 0; - const transactions = [...newTxs, ...updatedLocalTxs]; + const transactions = [...newTxs, ...updatedTxs]; return [updateRequired, transactions]; } @@ -194,61 +173,106 @@ export class IncomingTransactionHelper { remoteTxs: TransactionMeta[], localTxs: TransactionMeta[], ): TransactionMeta[] { - return remoteTxs.filter((tx) => { - const alreadyInTransactions = localTxs.find( - ({ transactionHash }) => transactionHash === tx.transactionHash, - ); - return !alreadyInTransactions; - }); + return remoteTxs.filter( + (tx) => + !localTxs.some( + ({ transactionHash }) => transactionHash === tx.transactionHash, + ), + ); } #getUpdatedTransactions( remoteTxs: TransactionMeta[], localTxs: TransactionMeta[], ): TransactionMeta[] { - return remoteTxs.filter((remoteTx) => { - const isTxOutdated = localTxs.find((localTx) => { - return ( + return remoteTxs.filter((remoteTx) => + localTxs.some( + (localTx) => remoteTx.transactionHash === localTx.transactionHash && - this.#isTransactionOutdated(remoteTx, localTx) - ); - }); - return isTxOutdated; - }); + this.#isTransactionOutdated(remoteTx, localTx), + ), + ); } #isTransactionOutdated( remoteTx: TransactionMeta, localTx: TransactionMeta, ): boolean { - const statusOutdated = this.#isStatusOutdated( - remoteTx.transactionHash, - localTx.transactionHash, - remoteTx.status, - localTx.status, + return UPDATE_CHECKS.some( + (getValue) => getValue(remoteTx) !== getValue(localTx), + ); + } + + #getFromBlock(latestBlockNumber: number): number { + const lastFetchedKey = this.#getBlockNumberKey(); + + const lastFetchedBlockNumber = + this.#lastFetchedBlockNumbers[lastFetchedKey]; + + if (lastFetchedBlockNumber) { + return lastFetchedBlockNumber + 1; + } + + // Avoid using latest block as remote transaction source + // may not have indexed it yet + return Math.max(latestBlockNumber - 10, 0); + } + + #updateLastFetchedBlockNumber(remoteTxs: TransactionMeta[]) { + let lastFetchedBlockNumber = -1; + + for (const tx of remoteTxs) { + const currentBlockNumberValue = tx.blockNumber + ? parseInt(tx.blockNumber, 10) + : -1; + + lastFetchedBlockNumber = Math.max( + lastFetchedBlockNumber, + currentBlockNumberValue, + ); + } + + if (lastFetchedBlockNumber === -1) { + return; + } + + const lastFetchedKey = this.#getBlockNumberKey(); + const previousValue = this.#lastFetchedBlockNumbers[lastFetchedKey]; + + if (previousValue === lastFetchedBlockNumber) { + return; + } + + this.#lastFetchedBlockNumbers[lastFetchedKey] = lastFetchedBlockNumber; + + this.hub.emit( + 'updatedLastFetchedBlockNumbers', + this.#lastFetchedBlockNumbers, ); + } + + #getBlockNumberKey(): string { + return `${this.#getCurrentChainId()}#${this.#getCurrentAccount().toLowerCase()}`; + } - const gasDataOutdated = this.#isGasDataOutdated( - remoteTx.transaction.gasUsed, - localTx.transaction.gasUsed, + #canStart(): boolean { + const isEnabled = this.#isEnabled(); + const currentChainId = this.#getCurrentChainId(); + const currentNetworkId = this.#getCurrentNetworkId(); + + const isSupportedNetwork = this.#remoteTransactionSource.isSupportedNetwork( + currentChainId, + currentNetworkId, ); - return statusOutdated || gasDataOutdated; + return isEnabled && isSupportedNetwork; } - #isStatusOutdated( - remoteTxHash: string | undefined, - localTxHash: string | undefined, - remoteTxStatus: TransactionStatus, - localTxStatus: TransactionStatus, - ): boolean { - return remoteTxHash === localTxHash && remoteTxStatus !== localTxStatus; + #getCurrentChainId(): Hex { + return this.#getNetworkState().providerConfig.chainId; } - #isGasDataOutdated( - remoteGasUsed: string | undefined, - localGasUsed: string | undefined, - ): boolean { - return remoteGasUsed !== localGasUsed; + #getCurrentNetworkId(): string { + return this.#getNetworkState().networkId as string; } } diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 305168da70..7415b28d34 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -18,7 +18,6 @@ import { errorCodes } from 'eth-rpc-errors'; import HttpProvider from 'ethjs-provider-http'; import NonceTracker from 'nonce-tracker'; -import { IncomingTransactionHelper } from './IncomingTransactionHelper'; import type { TransactionControllerMessenger, TransactionConfig, @@ -370,12 +369,6 @@ const MOCK_CUSTOM_NETWORK: MockNetwork = { const ACCOUNT_MOCK = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; const NONCE_MOCK = 12; -const BLOCK_NUMBER_MOCK = '999'; -const ETHERSCAN_API_KEY_MOCK = 'testApiKey'; - -const TRANSACTION_META_MOCK = { - transaction: { from: ACCOUNT_MOCK }, -} as TransactionMeta; describe('TransactionController', () => { let resultCallbacksMock: AcceptResultCallbacks; @@ -422,11 +415,11 @@ describe('TransactionController', () => { return new TransactionController( { + blockTracker: finalNetwork.blockTracker, getNetworkState: () => finalNetwork.state, + messenger, onNetworkStateChange: finalNetwork.subscribe, provider: finalNetwork.provider, - blockTracker: finalNetwork.blockTracker, - messenger, ...options, }, { @@ -1143,93 +1136,6 @@ describe('TransactionController', () => { }); }); - describe('fetchAll', () => { - const mockIncomingTransactionHelperConstructor = - IncomingTransactionHelper as jest.MockedClass< - typeof IncomingTransactionHelper - >; - - const mockIncomingTransactionHelper = { - reconcile: jest.fn(), - } as unknown as jest.Mocked; - - beforeEach(() => { - mockIncomingTransactionHelper.reconcile.mockResolvedValueOnce({ - updateRequired: false, - transactions: [], - }); - - mockIncomingTransactionHelperConstructor.mockReturnValueOnce( - mockIncomingTransactionHelper, - ); - }); - - it('reconciles incoming transactions using helper', async () => { - const controller = newController(); - controller.state.transactions = [ - TRANSACTION_META_MOCK, - TRANSACTION_META_MOCK, - ]; - - controller.fetchAll(ACCOUNT_MOCK, { - fromBlock: BLOCK_NUMBER_MOCK, - etherscanApiKey: ETHERSCAN_API_KEY_MOCK, - }); - - expect(mockIncomingTransactionHelper.reconcile).toHaveBeenCalledTimes(1); - expect(mockIncomingTransactionHelper.reconcile).toHaveBeenCalledWith({ - address: ACCOUNT_MOCK, - apiKey: ETHERSCAN_API_KEY_MOCK, - fromBlock: BLOCK_NUMBER_MOCK, - localTransactions: [TRANSACTION_META_MOCK, TRANSACTION_META_MOCK], - }); - }); - - it('updates state with transactions from helper if update is required', async () => { - mockIncomingTransactionHelper.reconcile.mockReset(); - mockIncomingTransactionHelper.reconcile.mockResolvedValueOnce({ - updateRequired: true, - transactions: [TRANSACTION_META_MOCK, TRANSACTION_META_MOCK], - }); - - const controller = newController(); - - await controller.fetchAll(ACCOUNT_MOCK); - - expect(controller.state.transactions).toStrictEqual([ - TRANSACTION_META_MOCK, - TRANSACTION_META_MOCK, - ]); - }); - - it('does not updates state if update is not required', async () => { - mockIncomingTransactionHelper.reconcile.mockReset(); - mockIncomingTransactionHelper.reconcile.mockResolvedValueOnce({ - updateRequired: false, - transactions: [TRANSACTION_META_MOCK, TRANSACTION_META_MOCK], - }); - - const controller = newController(); - - await controller.fetchAll(ACCOUNT_MOCK); - - expect(controller.state.transactions).toStrictEqual([]); - }); - - it('returns latest block number from helper', async () => { - mockIncomingTransactionHelper.reconcile.mockReset(); - mockIncomingTransactionHelper.reconcile.mockResolvedValueOnce({ - updateRequired: false, - transactions: [], - latestBlockNumber: BLOCK_NUMBER_MOCK, - }); - - const latestBlockNumber = await newController().fetchAll(ACCOUNT_MOCK); - - expect(latestBlockNumber).toBe(BLOCK_NUMBER_MOCK); - }); - }); - describe('handleMethodData', () => { it('loads method data from registry', async () => { const controller = newController({ network: MOCK_MAINNET_NETWORK }); @@ -1408,16 +1314,11 @@ describe('TransactionController', () => { }); describe('getCommonConfiguration', () => { - it('should get the common network configuration for mainnet', async () => { - const controller = new TransactionController({ - getNetworkState: () => MOCK_MAINNET_NETWORK.state, - onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe, - provider: MOCK_MAINNET_NETWORK.provider, - blockTracker: MOCK_MAINNET_NETWORK.blockTracker, - messenger: messengerMock, - }); + it('gets the common network configuration for mainnet', () => { + const controller = newController({ network: MOCK_MAINNET_NETWORK }); + + const config = controller.getCommonConfiguration(); - const config = await controller.getCommonConfiguration(); expect(config).toStrictEqual( new Common({ chain: 'mainnet', hardfork: HARDFORK }), ); @@ -1427,21 +1328,12 @@ describe('TransactionController', () => { ['linea-mainnet', MOCK_LINEA_MAINNET_NETWORK, 59144], ['linea-goerli', MOCK_LINEA_GOERLI_NETWORK, 59140], ])( - 'should get a custom network configuration for %s', - async ( - _, - { state, subscribe, provider, blockTracker }: MockNetwork, - chainId: number, - ) => { - const controller = new TransactionController({ - getNetworkState: () => state, - onNetworkStateChange: subscribe, - provider, - blockTracker, - messenger: messengerMock, - }); + 'gets a custom network configuration for %s', + async (_, network: MockNetwork, chainId: number) => { + const controller = newController({ network }); const config = controller.getCommonConfiguration(); + expect(config).toStrictEqual( Common.custom({ name: undefined, diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 22ad692a32..356c1f98d7 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -218,32 +218,46 @@ export class TransactionController extends BaseController< * Creates a TransactionController instance. * * @param options - The controller options. + * @param options.blockTracker - The block tracker used to poll for new blocks data. * @param options.getNetworkState - Gets the state of the network controller. + * @param options.getSelectedAddress - Gets the address of the currently selected account. + * @param options.incomingTransactions - Configuration options for incoming transaction support. + * @param options.incomingTransactions.includeTokenTransfers - Whether or not to include ERC20 token transfers. + * @param options.incomingTransactions.isEnabled - Whether or not incoming transaction retrieval is enabled. + * @param options.incomingTransactions.updateTransactions - Whether or not to update local transactions using remote transaction data. + * @param options.messenger - The controller messenger. * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. * @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 options.messenger - The controller messenger. * @param config - Initial options used to configure this controller. * @param state - Initial state to set on this controller. */ constructor( { + blockTracker, getNetworkState, + getSelectedAddress, + incomingTransactions = {}, + messenger, onNetworkStateChange, provider, - blockTracker, - messenger, }: { + blockTracker: BlockTracker; getNetworkState: () => NetworkState; + getSelectedAddress: () => string; + incomingTransactions: { + includeTokenTransfers?: boolean; + isEnabled?: () => boolean; + updateTransactions?: boolean; + }; + messenger: TransactionControllerMessenger; onNetworkStateChange: (listener: (state: NetworkState) => void) => void; provider: Provider; - blockTracker: BlockTracker; - messenger: TransactionControllerMessenger; }, config?: Partial, state?: Partial, ) { super(config, state); + this.defaultConfig = { interval: 15000, txHistoryLimit: 40, @@ -253,12 +267,15 @@ export class TransactionController extends BaseController< methodData: {}, transactions: [], }; + this.initialize(); + this.provider = provider; this.messagingSystem = messenger; this.getNetworkState = getNetworkState; this.ethQuery = new EthQuery(provider); this.registry = new MethodRegistry({ provider }); + this.nonceTracker = new NonceTracker({ provider, blockTracker, @@ -275,17 +292,24 @@ export class TransactionController extends BaseController< this.state.transactions, ), }); + this.incomingTransactionHelper = new IncomingTransactionHelper({ + blockTracker, + getCurrentAccount: getSelectedAddress, getNetworkState, - getEthQuery: () => this.ethQuery, + isEnabled: incomingTransactions.isEnabled, + remoteTransactionSource: new EtherscanRemoteTransactionSource({ + includeTokenTransfers: incomingTransactions.includeTokenTransfers, + }), transactionLimit: this.config.txHistoryLimit, - remoteTransactionSource: new EtherscanRemoteTransactionSource(), + updateTransactions: incomingTransactions.updateTransactions, }); onNetworkStateChange(() => { this.ethQuery = new EthQuery(this.provider); this.registry = new MethodRegistry({ provider: this.provider }); }); + this.poll(); } @@ -814,36 +838,12 @@ export class TransactionController extends BaseController< }); } - /** - * Get transactions from Etherscan for the given address. By default all transactions are - * returned, but the `fromBlock` option can be given to filter just for transactions from a - * specific block onward. - * - * @param address - The address to fetch the transactions for. - * @param opt - Object containing optional data, fromBlock and Etherscan API key. - * @returns The block number of the latest incoming transaction. - */ - async fetchAll( - address: string, - opt?: FetchAllOptions, - ): Promise { - const { transactions: localTransactions } = this.state; - - const { updateRequired, transactions, latestBlockNumber } = - await this.incomingTransactionHelper.reconcile({ - address, - localTransactions, - fromBlock: opt?.fromBlock, - apiKey: opt?.etherscanApiKey, - }); - - if (updateRequired) { - this.update({ - transactions: this.trimTransactionsForState(transactions), - }); - } + startIncomingTransactionProcessing() { + this.incomingTransactionHelper.start(); + } - return latestBlockNumber; + stopIncomingTransactionProcessing() { + this.incomingTransactionHelper.stop(); } private async processApproval( diff --git a/packages/transaction-controller/src/constants.ts b/packages/transaction-controller/src/constants.ts new file mode 100644 index 0000000000..3cd179c7af --- /dev/null +++ b/packages/transaction-controller/src/constants.ts @@ -0,0 +1,122 @@ +export const CHAIN_IDS = { + MAINNET: '0x1', + GOERLI: '0x5', + BSC: '0x38', + BSC_TESTNET: '0x61', + OPTIMISM: '0xa', + OPTIMISM_TESTNET: '0x1a4', + POLYGON: '0x89', + POLYGON_TESTNET: '0x13881', + AVALANCHE: '0xa86a', + AVALANCHE_TESTNET: '0xa869', + FANTOM: '0xfa', + FANTOM_TESTNET: '0xfa2', + SEPOLIA: '0xaa36a7', + LINEA_GOERLI: '0xe704', + LINEA_MAINNET: '0xe708', + MOONBEAM: '0x504', + MOONBEAM_TESTNET: '0x507', + MOONRIVER: '0x505', + GNOSIS: '0x64', +} as const; + +const DEFAULT_ETHERSCAN_DOMAIN = 'etherscan.io'; +const DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX = 'api'; + +export const ETHERSCAN_SUPPORTED_NETWORKS = { + [CHAIN_IDS.GOERLI]: { + domain: DEFAULT_ETHERSCAN_DOMAIN, + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-goerli`, + networkId: parseInt(CHAIN_IDS.GOERLI, 16).toString(), + }, + [CHAIN_IDS.MAINNET]: { + domain: DEFAULT_ETHERSCAN_DOMAIN, + subdomain: DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX, + networkId: parseInt(CHAIN_IDS.MAINNET, 16).toString(), + }, + [CHAIN_IDS.SEPOLIA]: { + domain: DEFAULT_ETHERSCAN_DOMAIN, + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-sepolia`, + networkId: parseInt(CHAIN_IDS.SEPOLIA, 16).toString(), + }, + [CHAIN_IDS.LINEA_GOERLI]: { + domain: 'lineascan.build', + subdomain: 'goerli', + networkId: parseInt(CHAIN_IDS.LINEA_GOERLI, 16).toString(), + }, + [CHAIN_IDS.LINEA_MAINNET]: { + domain: 'lineascan.build', + subdomain: DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX, + networkId: parseInt(CHAIN_IDS.LINEA_MAINNET, 16).toString(), + }, + [CHAIN_IDS.BSC]: { + domain: 'bscscan.com', + subdomain: DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX, + networkId: parseInt(CHAIN_IDS.BSC, 16).toString(), + }, + [CHAIN_IDS.BSC_TESTNET]: { + domain: 'bscscan.com', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-testnet`, + networkId: parseInt(CHAIN_IDS.BSC_TESTNET, 16).toString(), + }, + [CHAIN_IDS.OPTIMISM]: { + domain: DEFAULT_ETHERSCAN_DOMAIN, + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-optimistic`, + networkId: parseInt(CHAIN_IDS.OPTIMISM, 16).toString(), + }, + [CHAIN_IDS.OPTIMISM_TESTNET]: { + domain: DEFAULT_ETHERSCAN_DOMAIN, + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-goerli-optimistic`, + networkId: parseInt(CHAIN_IDS.OPTIMISM_TESTNET, 16).toString(), + }, + [CHAIN_IDS.POLYGON]: { + domain: 'polygonscan.com', + subdomain: DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX, + networkId: parseInt(CHAIN_IDS.POLYGON, 16).toString(), + }, + [CHAIN_IDS.POLYGON_TESTNET]: { + domain: 'polygonscan.com', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-mumbai`, + networkId: parseInt(CHAIN_IDS.POLYGON_TESTNET, 16).toString(), + }, + [CHAIN_IDS.AVALANCHE]: { + domain: 'snowtrace.io', + subdomain: DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX, + networkId: parseInt(CHAIN_IDS.AVALANCHE, 16).toString(), + }, + [CHAIN_IDS.AVALANCHE_TESTNET]: { + domain: 'snowtrace.io', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-testnet`, + networkId: parseInt(CHAIN_IDS.AVALANCHE_TESTNET, 16).toString(), + }, + [CHAIN_IDS.FANTOM]: { + domain: 'ftmscan.com', + subdomain: DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX, + networkId: parseInt(CHAIN_IDS.FANTOM, 16).toString(), + }, + [CHAIN_IDS.FANTOM_TESTNET]: { + domain: 'ftmscan.com', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-testnet`, + networkId: parseInt(CHAIN_IDS.FANTOM_TESTNET, 16).toString(), + }, + [CHAIN_IDS.MOONBEAM]: { + domain: 'moonscan.io', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-moonbeam`, + networkId: parseInt(CHAIN_IDS.MOONBEAM, 16).toString(), + }, + [CHAIN_IDS.MOONBEAM_TESTNET]: { + domain: 'moonscan.io', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-moonbase`, + networkId: parseInt(CHAIN_IDS.MOONBEAM_TESTNET, 16).toString(), + }, + [CHAIN_IDS.MOONRIVER]: { + domain: 'moonscan.io', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-moonriver`, + networkId: parseInt(CHAIN_IDS.MOONRIVER, 16).toString(), + }, + [CHAIN_IDS.GNOSIS]: { + domain: 'gnosisscan.io', + subdomain: `${DEFAULT_ETHERSCAN_SUBDOMAIN_PREFIX}-gnosis`, + networkId: parseInt(CHAIN_IDS.GNOSIS, 16).toString(), + }, +}; diff --git a/packages/transaction-controller/src/etherscan.test.ts b/packages/transaction-controller/src/etherscan.test.ts index 7d81833d40..16b204a0c8 100644 --- a/packages/transaction-controller/src/etherscan.test.ts +++ b/packages/transaction-controller/src/etherscan.test.ts @@ -1,5 +1,6 @@ -import { NetworkType, handleFetch } from '@metamask/controller-utils'; +import { handleFetch } from '@metamask/controller-utils'; +import { CHAIN_IDS, ETHERSCAN_SUPPORTED_NETWORKS } from './constants'; import type { EtherscanTransactionMeta, EtherscanTransactionRequest, @@ -16,14 +17,13 @@ const ADDERSS_MOCK = '0x2A2D72308838A6A46a0B5FDA3055FE915b5D99eD'; const REQUEST_MOCK: EtherscanTransactionRequest = { address: ADDERSS_MOCK, - networkType: NetworkType.goerli, + chainId: CHAIN_IDS.GOERLI, limit: 3, - fromBlock: '0x2', + fromBlock: 2, apiKey: 'testApiKey', }; const RESPONSE_MOCK: EtherscanTransactionResponse = { - status: '1', result: [ { from: ADDERSS_MOCK, nonce: '0x1' } as EtherscanTransactionMeta, { from: ADDERSS_MOCK, nonce: '0x2' } as EtherscanTransactionMeta, @@ -58,7 +58,9 @@ describe('Etherscan', () => { expect(handleFetchMock).toHaveBeenCalledTimes(1); expect(handleFetchMock).toHaveBeenCalledWith( - `https://api-${REQUEST_MOCK.networkType}.etherscan.io/api?` + + `https://${ETHERSCAN_SUPPORTED_NETWORKS[CHAIN_IDS.GOERLI].subdomain}.${ + ETHERSCAN_SUPPORTED_NETWORKS[CHAIN_IDS.GOERLI].domain + }/api?` + `module=account` + `&address=${REQUEST_MOCK.address}` + `&startBlock=${REQUEST_MOCK.fromBlock}` + @@ -71,32 +73,54 @@ describe('Etherscan', () => { ); }); - it('returns empty result if response status is 0', async () => { - handleFetchMock.mockResolvedValueOnce({ - status: '0', - }); - - const result = await (Etherscan as any)[method](REQUEST_MOCK); + it('supports alternate networks', async () => { + handleFetchMock.mockResolvedValueOnce(RESPONSE_MOCK); - expect(result).toStrictEqual({ - status: '0', - result: [], + await (Etherscan as any)[method]({ + ...REQUEST_MOCK, + chainId: CHAIN_IDS.MAINNET, }); + + expect(handleFetchMock).toHaveBeenCalledTimes(1); + expect(handleFetchMock).toHaveBeenCalledWith( + `https://${ETHERSCAN_SUPPORTED_NETWORKS[CHAIN_IDS.MAINNET].subdomain}.${ + ETHERSCAN_SUPPORTED_NETWORKS[CHAIN_IDS.MAINNET].domain + }/api?` + + `module=account` + + `&address=${REQUEST_MOCK.address}` + + `&startBlock=${REQUEST_MOCK.fromBlock}` + + `&apikey=${REQUEST_MOCK.apiKey}` + + `&offset=${REQUEST_MOCK.limit}` + + `&order=desc` + + `&action=${action}` + + `&tag=latest` + + `&page=1`, + ); }); - it('returns standard response if result is empty', async () => { + it('throws if message is not ok', async () => { handleFetchMock.mockResolvedValueOnce({ status: '0', - result: [], - error: 'testError', + message: 'NOTOK', + result: 'test error', }); - const result = await (Etherscan as any)[method](REQUEST_MOCK); + await expect((Etherscan as any)[method](REQUEST_MOCK)).rejects.toThrow( + 'Etherscan request failed - test error', + ); + }); - expect(result).toStrictEqual({ - status: '0', - result: [], - }); + it('throws if chain is not supported', async () => { + const unsupportedChainId = '0x11111111111111111111'; + + await expect( + (Etherscan as any)[method]({ + ...REQUEST_MOCK, + chainId: unsupportedChainId, + }), + ).rejects.toThrow( + `Etherscan does not support chain with ID: ${unsupportedChainId}`, + ); }); it('does not include empty values in fetched URL', async () => { @@ -106,14 +130,16 @@ describe('Etherscan', () => { ...REQUEST_MOCK, fromBlock: undefined, apiKey: undefined, + limit: undefined, }); expect(handleFetchMock).toHaveBeenCalledTimes(1); expect(handleFetchMock).toHaveBeenCalledWith( - `https://api-${REQUEST_MOCK.networkType}.etherscan.io/api?` + + `https://${ETHERSCAN_SUPPORTED_NETWORKS[CHAIN_IDS.GOERLI].subdomain}.${ + ETHERSCAN_SUPPORTED_NETWORKS[CHAIN_IDS.GOERLI].domain + }/api?` + `module=account` + `&address=${REQUEST_MOCK.address}` + - `&offset=${REQUEST_MOCK.limit}` + `&order=desc` + `&action=${action}` + `&tag=latest` + diff --git a/packages/transaction-controller/src/etherscan.ts b/packages/transaction-controller/src/etherscan.ts index f6135eaf0c..490166f746 100644 --- a/packages/transaction-controller/src/etherscan.ts +++ b/packages/transaction-controller/src/etherscan.ts @@ -1,4 +1,7 @@ -import { NetworkType, handleFetch } from '@metamask/controller-utils'; +import { handleFetch } from '@metamask/controller-utils'; +import type { Hex } from '@metamask/utils'; + +import { ETHERSCAN_SUPPORTED_NETWORKS } from './constants'; export interface EtherscanTransactionMetaBase { blockNumber: string; @@ -36,16 +39,21 @@ export interface EtherscanTokenTransactionMeta export interface EtherscanTransactionResponse< T extends EtherscanTransactionMetaBase, > { - status: '0' | '1'; result: T[]; } export interface EtherscanTransactionRequest { address: string; - networkType: NetworkType; - limit: number; - fromBlock?: string; apiKey?: string; + chainId: Hex; + fromBlock?: number; + limit?: number; +} + +interface RawEtherscanResponse { + status: '0' | '1'; + message: string; + result: string | T[]; } /** @@ -53,27 +61,27 @@ export interface EtherscanTransactionRequest { * * @param request - Configuration required to fetch transactions. * @param request.address - Address to retrieve transactions for. - * @param request.networkType - Current network type used to determine the Etherscan subdomain. - * @param request.limit - Maximum number of transactions to retrieve. * @param request.apiKey - Etherscan API key. + * @param request.chainId - Current chain ID used to determine subdomain and domain. * @param request.fromBlock - Block number to start fetching transactions from. + * @param request.limit - Number of transactions to retrieve. * @returns An Etherscan response object containing the request status and an array of token transaction data. */ export async function fetchEtherscanTransactions({ address, - networkType, - limit, apiKey, + chainId, fromBlock, + limit, }: EtherscanTransactionRequest): Promise< EtherscanTransactionResponse > { return await fetchTransactions('txlist', { address, - networkType, - limit, - fromBlock, apiKey, + chainId, + fromBlock, + limit, }); } @@ -82,27 +90,27 @@ export async function fetchEtherscanTransactions({ * * @param request - Configuration required to fetch token transactions. * @param request.address - Address to retrieve token transactions for. - * @param request.networkType - Current network type used to determine the Etherscan subdomain. - * @param request.limit - Maximum number of token transactions to retrieve. * @param request.apiKey - Etherscan API key. + * @param request.chainId - Current chain ID used to determine subdomain and domain. * @param request.fromBlock - Block number to start fetching token transactions from. + * @param request.limit - Number of token transactions to retrieve. * @returns An Etherscan response object containing the request status and an array of token transaction data. */ export async function fetchEtherscanTokenTransactions({ address, - networkType, - limit, apiKey, + chainId, fromBlock, + limit, }: EtherscanTransactionRequest): Promise< EtherscanTransactionResponse > { return await fetchTransactions('tokentx', { address, - networkType, - limit, - fromBlock, apiKey, + chainId, + fromBlock, + limit, }); } @@ -112,73 +120,76 @@ export async function fetchEtherscanTokenTransactions({ * @param action - The Etherscan endpoint to use. * @param options - Options bag. * @param options.address - Address to retrieve transactions for. - * @param options.networkType - Current network type used to determine the Etherscan subdomain. - * @param options.limit - Maximum number of transactions to retrieve. * @param options.apiKey - Etherscan API key. + * @param options.chainId - Current chain ID used to determine subdomain and domain. * @param options.fromBlock - Block number to start fetching transactions from. + * @param options.limit - Number of transactions to retrieve. * @returns An object containing the request status and an array of transaction data. */ async function fetchTransactions( action: string, { address, - networkType, - limit, apiKey, + chainId, fromBlock, + limit, }: { address: string; - networkType: NetworkType; - limit: number; - fromBlock?: string; apiKey?: string; + chainId: Hex; + fromBlock?: number; + limit?: number; }, ): Promise> { const urlParams = { module: 'account', address, - startBlock: fromBlock, + startBlock: fromBlock?.toString(), apikey: apiKey, - offset: limit.toString(), + offset: limit?.toString(), order: 'desc', }; - const etherscanTxUrl = getEtherscanApiUrl(networkType, { + const etherscanTxUrl = getEtherscanApiUrl(chainId, { ...urlParams, action, }); const response = (await handleFetch( etherscanTxUrl, - )) as EtherscanTransactionResponse; + )) as RawEtherscanResponse; - if (response.status === '0' || response.result.length <= 0) { - return { status: response.status, result: [] }; + if (response.status === '0' && response.message === 'NOTOK') { + throw new Error(`Etherscan request failed - ${response.result}`); } - return response; + return { result: response.result as T[] }; } /** * Return a URL that can be used to fetch data from Etherscan. * - * @param networkType - Network type of desired network. + * @param chainId - Current chain ID used to determine subdomain and domain. * @param urlParams - The parameters used to construct the URL. * @returns URL to access Etherscan data. */ function getEtherscanApiUrl( - networkType: string, + chainId: Hex, urlParams: Record, ): string { - let etherscanSubdomain = 'api'; + type SupportedChainId = keyof typeof ETHERSCAN_SUPPORTED_NETWORKS; + + const networkInfo = ETHERSCAN_SUPPORTED_NETWORKS[chainId as SupportedChainId]; - if (networkType !== NetworkType.mainnet) { - etherscanSubdomain = `api-${networkType}`; + if (!networkInfo) { + throw new Error(`Etherscan does not support chain with ID: ${chainId}`); } - const apiUrl = `https://${etherscanSubdomain}.etherscan.io`; + const apiUrl = `https://${networkInfo.subdomain}.${networkInfo.domain}`; let url = `${apiUrl}/api?`; + // eslint-disable-next-line guard-for-in for (const paramKey in urlParams) { const value = urlParams[paramKey]; diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 069bcf34ba..80afe79bc0 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -1,4 +1,3 @@ -import type { NetworkType } from '@metamask/controller-utils'; import type { Hex } from '@metamask/utils'; /** @@ -127,17 +126,12 @@ export interface RemoteTransactionSourceRequest { /** * Block number to start fetching transactions from. */ - fromBlock?: string; + fromBlock?: number; /** * Maximum number of transactions to retrieve. */ - limit: number; - - /** - * The type of the current network. - */ - networkType: NetworkType; + limit?: number; } /** @@ -145,6 +139,8 @@ export interface RemoteTransactionSourceRequest { * Used by the IncomingTransactionHelper to retrieve remote transaction data. */ export interface RemoteTransactionSource { + isSupportedNetwork: (chainId: Hex, networkId: string) => boolean; + fetchTransactions: ( request: RemoteTransactionSourceRequest, ) => Promise; From ae32b6fc988eb48e30648c231c652a09ed9e7c37 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 8 Aug 2023 09:50:54 +0100 Subject: [PATCH 2/3] Add helper listeners to controller Add start, stop, and update methods to controller. Add mutex to helper. Add API key to remote transaction source. --- .../src/EtherscanRemoteTransactionSource.ts | 10 +- .../src/IncomingTransactionHelper.test.ts | 60 +++++++-- .../src/IncomingTransactionHelper.ts | 126 +++++++++--------- .../src/TransactionController.test.ts | 126 ++++++++++++++++++ .../src/TransactionController.ts | 66 +++++++++ 5 files changed, 313 insertions(+), 75 deletions(-) diff --git a/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts b/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts index 49b44437b2..c2db545a7e 100644 --- a/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts +++ b/packages/transaction-controller/src/EtherscanRemoteTransactionSource.ts @@ -8,6 +8,7 @@ import type { EtherscanTokenTransactionMeta, EtherscanTransactionMeta, EtherscanTransactionMetaBase, + EtherscanTransactionRequest, EtherscanTransactionResponse, } from './etherscan'; import { @@ -27,11 +28,15 @@ import { TransactionStatus } from './types'; export class EtherscanRemoteTransactionSource implements RemoteTransactionSource { + #apiKey?: string; + #includeTokenTransfers: boolean; constructor({ + apiKey, includeTokenTransfers, - }: { includeTokenTransfers?: boolean } = {}) { + }: { apiKey?: string; includeTokenTransfers?: boolean } = {}) { + this.#apiKey = apiKey; this.#includeTokenTransfers = includeTokenTransfers ?? true; } @@ -42,8 +47,9 @@ export class EtherscanRemoteTransactionSource async fetchTransactions( request: RemoteTransactionSourceRequest, ): Promise { - const etherscanRequest = { + const etherscanRequest: EtherscanTransactionRequest = { ...request, + apiKey: this.#apiKey, chainId: request.currentChainId, }; diff --git a/packages/transaction-controller/src/IncomingTransactionHelper.test.ts b/packages/transaction-controller/src/IncomingTransactionHelper.test.ts index cfdba86bff..43a09cd595 100644 --- a/packages/transaction-controller/src/IncomingTransactionHelper.test.ts +++ b/packages/transaction-controller/src/IncomingTransactionHelper.test.ts @@ -31,6 +31,7 @@ const FROM_BLOCK_DECIMAL_MOCK = 32; const BLOCK_TRACKER_MOCK = { addListener: jest.fn(), removeListener: jest.fn(), + getLatestBlock: jest.fn(() => FROM_BLOCK_HEX_MOCK), } as unknown as jest.Mocked; const CONTROLLER_ARGS_MOCK = { @@ -99,7 +100,8 @@ async function emitBlockTrackerLatestEvent( return { transactions: transactionsListener.mock.calls[0]?.[0], - lastFetchBlockNumbers: blockNumberListener.mock.calls[0]?.[0], + lastFetchedBlockNumbers: + blockNumberListener.mock.calls[0]?.[0].lastFetchedBlockNumbers, transactionsListener, blockNumberListener, }; @@ -204,7 +206,10 @@ describe('IncomingTransactionHelper', () => { const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(transactions).toStrictEqual([TRANSACTION_MOCK_2]); + expect(transactions).toStrictEqual({ + added: [TRANSACTION_MOCK_2], + updated: [], + }); }); it('if new outgoing transaction fetched and update transactions enabled', async () => { @@ -227,7 +232,10 @@ describe('IncomingTransactionHelper', () => { const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(transactions).toStrictEqual([outgoingTransaction]); + expect(transactions).toStrictEqual({ + added: [outgoingTransaction], + updated: [], + }); }); it('if existing transaction fetched with different status and update transactions enabled', async () => { @@ -247,7 +255,10 @@ describe('IncomingTransactionHelper', () => { const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(transactions).toStrictEqual([updatedTransaction]); + expect(transactions).toStrictEqual({ + added: [], + updated: [updatedTransaction], + }); }); it('if existing transaction fetched with different gas used and update transactions enabled', async () => { @@ -270,7 +281,10 @@ describe('IncomingTransactionHelper', () => { const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(transactions).toStrictEqual([updatedTransaction]); + expect(transactions).toStrictEqual({ + added: [], + updated: [updatedTransaction], + }); }); it('sorted by time in ascending order', async () => { @@ -289,11 +303,10 @@ describe('IncomingTransactionHelper', () => { const { transactions } = await emitBlockTrackerLatestEvent(helper); - expect(transactions).toStrictEqual([ - firstTransaction, - secondTransaction, - thirdTransaction, - ]); + expect(transactions).toStrictEqual({ + added: [firstTransaction, secondTransaction, thirdTransaction], + updated: [], + }); }); it('does not if identical transaction fetched and update transactions enabled', async () => { @@ -425,11 +438,11 @@ describe('IncomingTransactionHelper', () => { ]), }); - const { lastFetchBlockNumbers } = await emitBlockTrackerLatestEvent( + const { lastFetchedBlockNumbers } = await emitBlockTrackerLatestEvent( helper, ); - expect(lastFetchBlockNumbers).toStrictEqual({ + expect(lastFetchedBlockNumbers).toStrictEqual({ [`${NETWORK_STATE_MOCK.providerConfig.chainId}#${ADDERSS_MOCK}`]: parseInt(TRANSACTION_MOCK_2.blockNumber as string, 10), }); @@ -575,4 +588,27 @@ describe('IncomingTransactionHelper', () => { ).toHaveBeenCalledTimes(1); }); }); + + describe('update', () => { + it('emits transactions event', async () => { + const listener = jest.fn(); + + const helper = new IncomingTransactionHelper({ + ...CONTROLLER_ARGS_MOCK, + remoteTransactionSource: createRemoteTransactionSourceMock([ + TRANSACTION_MOCK_2, + ]), + }); + + helper.hub.on('transactions', listener); + + await helper.update(); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith({ + added: [TRANSACTION_MOCK_2], + updated: [], + }); + }); + }); }); diff --git a/packages/transaction-controller/src/IncomingTransactionHelper.ts b/packages/transaction-controller/src/IncomingTransactionHelper.ts index 1d2eb4966c..67aca14c75 100644 --- a/packages/transaction-controller/src/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/IncomingTransactionHelper.ts @@ -1,5 +1,6 @@ import type { BlockTracker, NetworkState } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; +import { Mutex } from 'async-mutex'; import EventEmitter from 'events'; import type { RemoteTransactionSource, TransactionMeta } from './types'; @@ -26,6 +27,8 @@ export class IncomingTransactionHelper { #lastFetchedBlockNumbers: Record; + #mutex = new Mutex(); + #onLatestBlock: (blockNumberHex: Hex) => Promise; #remoteTransactionSource: RemoteTransactionSource; @@ -72,7 +75,7 @@ export class IncomingTransactionHelper { // with the correct scope that we can remove later if stopped. this.#onLatestBlock = async (blockNumberHex: Hex) => { try { - await this.#update(blockNumberHex); + await this.update(blockNumberHex); } catch (error) { console.error('Error while checking incoming transactions', error); } @@ -97,76 +100,77 @@ export class IncomingTransactionHelper { this.#isRunning = false; } - async #update(latestBlockNumberHex: Hex): Promise { - if (!this.#canStart()) { - return; - } - - const latestBlockNumber = parseInt(latestBlockNumberHex, 16); - const fromBlock = this.#getFromBlock(latestBlockNumber); - const address = this.#getCurrentAccount(); - const currentChainId = this.#getCurrentChainId(); - const currentNetworkId = this.#getCurrentNetworkId(); - - let remoteTransactions = []; + async update(latestBlockNumberHex?: Hex): Promise { + const releaseLock = await this.#mutex.acquire(); try { - remoteTransactions = - await this.#remoteTransactionSource.fetchTransactions({ - address, - currentChainId, - currentNetworkId, - fromBlock, - limit: this.#transactionLimit, - }); - } catch (error: any) { - return; - } + if (!this.#canStart()) { + return; + } - if (!this.#updateTransactions) { - remoteTransactions = remoteTransactions.filter( - (tx) => tx.transaction.to?.toLowerCase() === address.toLowerCase(), + const latestBlockNumber = parseInt( + latestBlockNumberHex || (await this.#blockTracker.getLatestBlock()), + 16, ); - } - this.#updateLastFetchedBlockNumber(remoteTransactions); + const fromBlock = this.#getFromBlock(latestBlockNumber); + const address = this.#getCurrentAccount(); + const currentChainId = this.#getCurrentChainId(); + const currentNetworkId = this.#getCurrentNetworkId(); - const [updateRequired, transactions] = - this.#reconcileTransactions(remoteTransactions); + let remoteTransactions = []; - if (updateRequired) { - this.#sortTransactionsByTime(transactions); - this.hub.emit('transactions', transactions); - } - } + try { + remoteTransactions = + await this.#remoteTransactionSource.fetchTransactions({ + address, + currentChainId, + currentNetworkId, + fromBlock, + limit: this.#transactionLimit, + }); + } catch (error: any) { + return; + } - #sortTransactionsByTime(transactions: TransactionMeta[]) { - transactions.sort((a, b) => (a.time < b.time ? -1 : 1)); - } + if (!this.#updateTransactions) { + remoteTransactions = remoteTransactions.filter( + (tx) => tx.transaction.to?.toLowerCase() === address.toLowerCase(), + ); + } - #reconcileTransactions( - remoteTxs: TransactionMeta[], - ): [boolean, TransactionMeta[]] { - if (!this.#updateTransactions) { - return [remoteTxs.length > 0, remoteTxs]; - } + const localTransactions = !this.#updateTransactions + ? [] + : this.#getLocalTransactions(); - const localTxs = this.#getLocalTransactions(); + const newTransactions = this.#getNewTransactions( + remoteTransactions, + localTransactions, + ); - const updatedTxs: TransactionMeta[] = this.#getUpdatedTransactions( - remoteTxs, - localTxs, - ); + const updatedTransactions = this.#getUpdatedTransactions( + remoteTransactions, + localTransactions, + ); - const newTxs: TransactionMeta[] = this.#getNewTransactions( - remoteTxs, - localTxs, - ); + if (newTransactions.length > 0 || updatedTransactions.length > 0) { + this.#sortTransactionsByTime(newTransactions); + this.#sortTransactionsByTime(updatedTransactions); - const updateRequired = newTxs.length > 0 || updatedTxs.length > 0; - const transactions = [...newTxs, ...updatedTxs]; + this.hub.emit('transactions', { + added: newTransactions, + updated: updatedTransactions, + }); + } + + this.#updateLastFetchedBlockNumber(remoteTransactions); + } finally { + releaseLock(); + } + } - return [updateRequired, transactions]; + #sortTransactionsByTime(transactions: TransactionMeta[]) { + transactions.sort((a, b) => (a.time < b.time ? -1 : 1)); } #getNewTransactions( @@ -245,10 +249,10 @@ export class IncomingTransactionHelper { this.#lastFetchedBlockNumbers[lastFetchedKey] = lastFetchedBlockNumber; - this.hub.emit( - 'updatedLastFetchedBlockNumbers', - this.#lastFetchedBlockNumbers, - ); + this.hub.emit('updatedLastFetchedBlockNumbers', { + lastFetchedBlockNumbers: this.#lastFetchedBlockNumbers, + blockNumber: lastFetchedBlockNumber, + }); } #getBlockNumberKey(): string { diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 7415b28d34..58d3b9cfcd 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -18,6 +18,7 @@ import { errorCodes } from 'eth-rpc-errors'; import HttpProvider from 'ethjs-provider-http'; import NonceTracker from 'nonce-tracker'; +import { IncomingTransactionHelper } from './IncomingTransactionHelper'; import type { TransactionControllerMessenger, TransactionConfig, @@ -370,6 +371,24 @@ const MOCK_CUSTOM_NETWORK: MockNetwork = { const ACCOUNT_MOCK = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; const NONCE_MOCK = 12; +const TRANSACTION_META_MOCK = { + status: TransactionStatus.confirmed, + transaction: { + from: ACCOUNT_MOCK, + }, + transactionHash: '0x1', + time: 123456789, +} as TransactionMeta; + +const TRANSACTION_META_2_MOCK = { + status: TransactionStatus.confirmed, + transaction: { + from: '0x3', + }, + transactionHash: '0x2', + time: 987654321, +} as TransactionMeta; + describe('TransactionController', () => { let resultCallbacksMock: AcceptResultCallbacks; let messengerMock: TransactionControllerMessenger; @@ -377,6 +396,12 @@ describe('TransactionController', () => { let delayMessengerMock: TransactionControllerMessenger; let approveTransaction: () => void; let getNonceLockSpy: jest.Mock; + let incomingTransactionHelperMock: jest.Mocked; + + const incomingTransactionHelperClassMock = + IncomingTransactionHelper as jest.MockedClass< + typeof IncomingTransactionHelper + >; /** * Create a new instance of the TransactionController. @@ -467,6 +492,16 @@ describe('TransactionController', () => { }); NonceTracker.prototype.getNonceLock = getNonceLockSpy; + + incomingTransactionHelperMock = { + hub: { + on: jest.fn(), + }, + } as any; + + incomingTransactionHelperClassMock.mockReturnValue( + incomingTransactionHelperMock, + ); }); afterEach(() => { @@ -479,6 +514,7 @@ describe('TransactionController', () => { expect(controller.state).toStrictEqual({ methodData: {}, transactions: [], + lastFetchedBlockNumbers: {}, }); }); @@ -1345,4 +1381,94 @@ describe('TransactionController', () => { }, ); }); + + describe('on incoming transaction helper transactions event', () => { + it('adds new transactions to state', async () => { + const controller = newController(); + + await (incomingTransactionHelperMock.hub.on as any).mock.calls[0][1]({ + added: [TRANSACTION_META_MOCK, TRANSACTION_META_2_MOCK], + updated: [], + }); + + expect(controller.state.transactions).toStrictEqual([ + TRANSACTION_META_MOCK, + TRANSACTION_META_2_MOCK, + ]); + }); + + it('updates existing transactions in state', async () => { + const controller = newController(); + + controller.state.transactions = [ + TRANSACTION_META_MOCK, + TRANSACTION_META_2_MOCK, + ]; + + const updatedTransaction = { + ...TRANSACTION_META_MOCK, + status: 'failed', + }; + + await (incomingTransactionHelperMock.hub.on as any).mock.calls[0][1]({ + added: [], + updated: [updatedTransaction], + }); + + expect(controller.state.transactions).toStrictEqual([ + updatedTransaction, + TRANSACTION_META_2_MOCK, + ]); + }); + + it('limits max transactions when adding to state', async () => { + const controller = newController({ config: { txHistoryLimit: 1 } }); + + await (incomingTransactionHelperMock.hub.on as any).mock.calls[0][1]({ + added: [TRANSACTION_META_MOCK, TRANSACTION_META_2_MOCK], + updated: [], + }); + + expect(controller.state.transactions).toStrictEqual([ + TRANSACTION_META_2_MOCK, + ]); + }); + }); + + describe('on incoming transaction helper lastFetchedBlockNumbers event', () => { + it('updates state', async () => { + const controller = newController(); + + const lastFetchedBlockNumbers = { + key: 234, + }; + + await (incomingTransactionHelperMock.hub.on as any).mock.calls[1][1]({ + lastFetchedBlockNumbers, + blockNumber: 123, + }); + + expect(controller.state.lastFetchedBlockNumbers).toStrictEqual( + lastFetchedBlockNumbers, + ); + }); + + it('emits incomingTransactionBlock event', async () => { + const blockNumber = 123; + const listener = jest.fn(); + + const controller = newController(); + controller.hub.on('incomingTransactionBlock', listener); + + await (incomingTransactionHelperMock.hub.on as any).mock.calls[1][1]({ + lastFetchedBlockNumbers: { + key: 234, + }, + blockNumber, + }); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(blockNumber); + }); + }); }); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 356c1f98d7..8112572468 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -122,6 +122,7 @@ export interface MethodData { export interface TransactionState extends BaseState { transactions: TransactionMeta[]; methodData: { [key: string]: MethodData }; + lastFetchedBlockNumbers: { [key: string]: number }; } /** @@ -222,6 +223,7 @@ export class TransactionController extends BaseController< * @param options.getNetworkState - Gets the state of the network controller. * @param options.getSelectedAddress - Gets the address of the currently selected account. * @param options.incomingTransactions - Configuration options for incoming transaction support. + * @param options.incomingTransactions.apiKey - An optional API key to use when fetching remote transaction data. * @param options.incomingTransactions.includeTokenTransfers - Whether or not to include ERC20 token transfers. * @param options.incomingTransactions.isEnabled - Whether or not incoming transaction retrieval is enabled. * @param options.incomingTransactions.updateTransactions - Whether or not to update local transactions using remote transaction data. @@ -245,6 +247,7 @@ export class TransactionController extends BaseController< getNetworkState: () => NetworkState; getSelectedAddress: () => string; incomingTransactions: { + apiKey?: string; includeTokenTransfers?: boolean; isEnabled?: () => boolean; updateTransactions?: boolean; @@ -266,6 +269,7 @@ export class TransactionController extends BaseController< this.defaultState = { methodData: {}, transactions: [], + lastFetchedBlockNumbers: {}, }; this.initialize(); @@ -299,12 +303,23 @@ export class TransactionController extends BaseController< getNetworkState, isEnabled: incomingTransactions.isEnabled, remoteTransactionSource: new EtherscanRemoteTransactionSource({ + apiKey: incomingTransactions.apiKey, includeTokenTransfers: incomingTransactions.includeTokenTransfers, }), transactionLimit: this.config.txHistoryLimit, updateTransactions: incomingTransactions.updateTransactions, }); + this.incomingTransactionHelper.hub.on( + 'transactions', + this.onIncomingTransactions.bind(this), + ); + + this.incomingTransactionHelper.hub.on( + 'updatedLastFetchedBlockNumbers', + this.onUpdatedLastFetchedBlockNumbers.bind(this), + ); + onNetworkStateChange(() => { this.ethQuery = new EthQuery(this.provider); this.registry = new MethodRegistry({ provider: this.provider }); @@ -404,6 +419,18 @@ export class TransactionController extends BaseController< }; } + startIncomingTransactionPolling() { + this.incomingTransactionHelper.start(); + } + + stopIncomingTransactionPolling() { + this.incomingTransactionHelper.stop(); + } + + async updateIncomingTransactions() { + await this.incomingTransactionHelper.update(); + } + prepareUnsignedEthTx(txParams: Record): TypedTransaction { return TransactionFactory.fromTxData(txParams, { common: this.getCommonConfiguration(), @@ -1226,6 +1253,45 @@ export class TransactionController extends BaseController< return { meta: transaction, isCompleted }; } + + private onIncomingTransactions({ + added, + updated, + }: { + added: TransactionMeta[]; + updated: TransactionMeta[]; + }) { + const { transactions: currentTransactions } = this.state; + + const updatedTransactions = [ + ...added, + ...currentTransactions.map((originalTransaction) => { + const updatedTransaction = updated.find( + ({ transactionHash }) => + transactionHash === originalTransaction.transactionHash, + ); + + return updatedTransaction ?? originalTransaction; + }), + ]; + + this.update({ + transactions: this.trimTransactionsForState(updatedTransactions), + }); + } + + private onUpdatedLastFetchedBlockNumbers({ + lastFetchedBlockNumbers, + blockNumber, + }: { + lastFetchedBlockNumbers: { + [key: string]: number; + }; + blockNumber: number; + }) { + this.update({ lastFetchedBlockNumbers }); + this.hub.emit('incomingTransactionBlock', blockNumber); + } } export default TransactionController; From 8294ba604db3ad00974549a497ecc6a410f5d949 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Mon, 14 Aug 2023 13:44:25 +0100 Subject: [PATCH 3/3] Update coverage targets --- packages/transaction-controller/jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index 19823619e9..808bb10db7 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -17,9 +17,9 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 81.29, - functions: 94.64, - lines: 95.24, + branches: 83.33, + functions: 93.22, + lines: 95.22, statements: 95.34, }, },