From 73b9eb92666917b7358e9e7066684b7cf4f745a0 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 20 Nov 2024 11:18:16 -0800 Subject: [PATCH 1/2] Release 252.0.0 (#4951) Minor release of assets controllers to include https://github.com/MetaMask/core/pull/4949 --------- Co-authored-by: Elliot Winkler --- package.json | 2 +- packages/assets-controllers/CHANGELOG.md | 9 ++++++++- packages/assets-controllers/package.json | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index d663ba860a..f5813a858d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "251.0.0", + "version": "252.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 6840e2e527..a67ac02963 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [44.1.0] + +### Changed + +- An argument `networkClientId` is added to `TokensController.ignoreTokens`, allowing tokens to be ignored on specific chains. ([#4949](https://github.com/MetaMask/core/pull/4949)) + ## [44.0.1] ### Changed @@ -1238,7 +1244,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Use Ethers for AssetsContractController ([#845](https://github.com/MetaMask/core/pull/845)) -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@44.0.1...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@44.1.0...HEAD +[44.1.0]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@44.0.1...@metamask/assets-controllers@44.1.0 [44.0.1]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@44.0.0...@metamask/assets-controllers@44.0.1 [44.0.0]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@43.1.1...@metamask/assets-controllers@44.0.0 [43.1.1]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@43.1.0...@metamask/assets-controllers@43.1.1 diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index 234230bb9e..d8fa6c0cf1 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/assets-controllers", - "version": "44.0.1", + "version": "44.1.0", "description": "Controllers which manage interactions involving ERC-20, ERC-721, and ERC-1155 tokens (including NFTs)", "keywords": [ "MetaMask", From bdd11ba1ca4cd94aed0899bc1a91b508ca609e89 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 20 Nov 2024 21:32:09 +0000 Subject: [PATCH 2/2] feat: increase pending transaction polling rate (#4917) Create the `TransactionPoller` helper class to encapsulate the polling logic for pending transactions. Initially, poll every 2 seconds up to a maximum of 5 times using `setTimeout`. Then fallback to polling via the `latest` event using the `BlockTracker`. If the pending transaction IDs are changed, reset to the initial `setTimeout` polling. --- .../transaction-controller/jest.config.js | 8 +- .../helpers/PendingTransactionTracker.test.ts | 166 ++++++----- .../src/helpers/PendingTransactionTracker.ts | 17 +- .../src/helpers/TransactionPoller.test.ts | 264 ++++++++++++++++++ .../src/helpers/TransactionPoller.ts | 168 +++++++++++ 5 files changed, 536 insertions(+), 87 deletions(-) create mode 100644 packages/transaction-controller/src/helpers/TransactionPoller.test.ts create mode 100644 packages/transaction-controller/src/helpers/TransactionPoller.ts diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index d84ee83366..7867d905e7 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 93.74, - functions: 97.51, - lines: 98.34, - statements: 98.35, + branches: 93.92, + functions: 97.56, + lines: 98.39, + statements: 98.4, }, }, diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts index b28b969392..a093ef5992 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts @@ -1,19 +1,19 @@ -/* eslint-disable jsdoc/require-jsdoc */ - import { query } from '@metamask/controller-utils'; +import type EthQuery from '@metamask/eth-query'; import type { BlockTracker } from '@metamask/network-controller'; import { freeze } from 'immer'; import type { TransactionMeta } from '../types'; import { TransactionStatus } from '../types'; import { PendingTransactionTracker } from './PendingTransactionTracker'; +import { TransactionPoller } from './TransactionPoller'; const ID_MOCK = 'testId'; const CHAIN_ID_MOCK = '0x1'; const NONCE_MOCK = '0x2'; const BLOCK_NUMBER_MOCK = '0x123'; -const ETH_QUERY_MOCK = {}; +const ETH_QUERY_MOCK = {} as unknown as EthQuery; const TRANSACTION_SUBMITTED_MOCK = { id: ID_MOCK, @@ -24,7 +24,7 @@ const TRANSACTION_SUBMITTED_MOCK = { txParams: { nonce: NONCE_MOCK, }, -}; +} as unknown as TransactionMeta; const RECEIPT_MOCK = { blockNumber: BLOCK_NUMBER_MOCK, @@ -38,6 +38,8 @@ const BLOCK_MOCK = { timestamp: 123456, }; +jest.mock('./TransactionPoller'); + jest.mock('@metamask/controller-utils', () => ({ query: jest.fn(), // TODO: Replace `any` with type @@ -45,25 +47,45 @@ jest.mock('@metamask/controller-utils', () => ({ safelyExecute: (fn: () => any) => fn(), })); +/** + * Creates a mock block tracker instance. + * @returns The mock block tracker instance. + */ function createBlockTrackerMock(): jest.Mocked { return { on: jest.fn(), removeListener: jest.fn(), - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; + } as unknown as jest.Mocked; +} + +/** + * Creates a mock transaction poller instance. + * @returns The mock transaction poller instance. + */ +function createTransactionPollerMock(): jest.Mocked { + return { + start: jest.fn(), + stop: jest.fn(), + setPendingTransactions: jest.fn(), + } as unknown as jest.Mocked; } describe('PendingTransactionTracker', () => { const queryMock = jest.mocked(query); let blockTracker: jest.Mocked; - let failTransaction: jest.Mock; let pendingTransactionTracker: PendingTransactionTracker; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let options: any; - - async function onLatestBlock( + let transactionPoller: jest.Mocked; + + let options: jest.Mocked< + ConstructorParameters[0] + >; + + /** + * Simulates a poll event. + * @param latestBlockNumber - The latest block number. + * @param transactionsOnCheck - The current transactions during the check. + */ + async function onPoll( latestBlockNumber?: string, transactionsOnCheck?: TransactionMeta[], ) { @@ -79,29 +101,27 @@ describe('PendingTransactionTracker', () => { ); } - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - await blockTracker.on.mock.calls[0][1](latestBlockNumber); + await transactionPoller.start.mock.calls[0][0](latestBlockNumber as string); } beforeEach(() => { blockTracker = createBlockTrackerMock(); - failTransaction = jest.fn(); + transactionPoller = createTransactionPollerMock(); + + jest.mocked(TransactionPoller).mockImplementation(() => transactionPoller); options = { - approveTransaction: jest.fn(), blockTracker, - failTransaction, - getChainId: () => CHAIN_ID_MOCK, - getEthQuery: () => ETH_QUERY_MOCK, + getChainId: jest.fn(() => CHAIN_ID_MOCK), + getEthQuery: jest.fn(() => ETH_QUERY_MOCK), getTransactions: jest.fn(), - getGlobalLock: () => Promise.resolve(jest.fn()), + getGlobalLock: jest.fn(() => Promise.resolve(jest.fn())), publishTransaction: jest.fn(), }; }); describe('on state change', () => { - it('adds block tracker listener if pending transactions', () => { + it('adds listener if pending transactions', () => { pendingTransactionTracker = new PendingTransactionTracker(options); options.getTransactions.mockReturnValue( @@ -110,14 +130,13 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker.startIfPendingTransactions(); - expect(blockTracker.on).toHaveBeenCalledTimes(1); - expect(blockTracker.on).toHaveBeenCalledWith( - 'latest', + expect(transactionPoller.start).toHaveBeenCalledTimes(1); + expect(transactionPoller.start).toHaveBeenCalledWith( expect.any(Function), ); }); - it('does nothing if block tracker listener already added', () => { + it('does nothing if listener already added', () => { pendingTransactionTracker = new PendingTransactionTracker(options); options.getTransactions.mockReturnValue( @@ -127,11 +146,11 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker.startIfPendingTransactions(); pendingTransactionTracker.startIfPendingTransactions(); - expect(blockTracker.on).toHaveBeenCalledTimes(1); - expect(blockTracker.removeListener).toHaveBeenCalledTimes(0); + expect(transactionPoller.start).toHaveBeenCalledTimes(1); + expect(transactionPoller.stop).toHaveBeenCalledTimes(0); }); - it('removes block tracker listener if no pending transactions and running', () => { + it('removes listener if no pending transactions and running', () => { pendingTransactionTracker = new PendingTransactionTracker(options); options.getTransactions.mockReturnValue( @@ -140,20 +159,16 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker.startIfPendingTransactions(); - expect(blockTracker.removeListener).toHaveBeenCalledTimes(0); + expect(transactionPoller.stop).toHaveBeenCalledTimes(0); options.getTransactions.mockReturnValue([]); pendingTransactionTracker.startIfPendingTransactions(); - expect(blockTracker.removeListener).toHaveBeenCalledTimes(1); - expect(blockTracker.removeListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); + expect(transactionPoller.stop).toHaveBeenCalledTimes(1); }); - it('does nothing if block tracker listener already removed', () => { + it('does nothing if listener already removed', () => { pendingTransactionTracker = new PendingTransactionTracker(options); options.getTransactions.mockReturnValue( @@ -168,11 +183,11 @@ describe('PendingTransactionTracker', () => { pendingTransactionTracker.startIfPendingTransactions(); - expect(blockTracker.removeListener).toHaveBeenCalledTimes(1); + expect(transactionPoller.stop).toHaveBeenCalledTimes(1); pendingTransactionTracker.startIfPendingTransactions(); - expect(blockTracker.removeListener).toHaveBeenCalledTimes(1); + expect(transactionPoller.stop).toHaveBeenCalledTimes(1); }); }); @@ -201,7 +216,7 @@ describe('PendingTransactionTracker', () => { listener, ); - await onLatestBlock(undefined, [ + await onPoll(undefined, [ { ...TRANSACTION_SUBMITTED_MOCK, status: TransactionStatus.dropped, @@ -248,7 +263,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(0); }); @@ -278,7 +293,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce({ ...RECEIPT_MOCK, status: null }); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(0); }); @@ -308,7 +323,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce({ ...RECEIPT_MOCK, status: '0x3' }); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(0); }); @@ -333,7 +348,7 @@ describe('PendingTransactionTracker', () => { listener, ); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith( @@ -366,7 +381,7 @@ describe('PendingTransactionTracker', () => { listener, ); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(0); }); @@ -387,7 +402,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce({ ...RECEIPT_MOCK, status: '0x0' }); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith( @@ -405,7 +420,7 @@ describe('PendingTransactionTracker', () => { ...TRANSACTION_SUBMITTED_MOCK, id: `${ID_MOCK}2`, status: TransactionStatus.confirmed, - }; + } as unknown as TransactionMeta; const submittedTransactionMetaMock = { ...TRANSACTION_SUBMITTED_MOCK, @@ -425,7 +440,7 @@ describe('PendingTransactionTracker', () => { listener, ); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith(submittedTransactionMetaMock); @@ -451,7 +466,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x3'); - await onLatestBlock(); + await onPoll(); } expect(listener).toHaveBeenCalledTimes(1); @@ -466,7 +481,7 @@ describe('PendingTransactionTracker', () => { id: `${ID_MOCK}2`, chainId: '0x2', status: TransactionStatus.confirmed, - }; + } as unknown as TransactionMeta; const submittedTransactionMetaMock = { ...TRANSACTION_SUBMITTED_MOCK, @@ -485,7 +500,7 @@ describe('PendingTransactionTracker', () => { listener, ); - await onLatestBlock(); + await onPoll(); expect(listener).not.toHaveBeenCalled(); }); @@ -512,7 +527,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(RECEIPT_MOCK); queryMock.mockResolvedValueOnce(BLOCK_MOCK); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith( @@ -552,7 +567,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(RECEIPT_MOCK); queryMock.mockResolvedValueOnce(BLOCK_MOCK); - await onLatestBlock(); + await onPoll(); expect(listener).toHaveBeenCalledTimes(2); expect(listener).toHaveBeenCalledWith( @@ -591,7 +606,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockRejectedValueOnce(new Error('TestError')); queryMock.mockResolvedValueOnce(BLOCK_MOCK); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); getTransactions.mockReturnValue( freeze( [ @@ -625,9 +640,8 @@ describe('PendingTransactionTracker', () => { it('if no pending transactions', async () => { pendingTransactionTracker = new PendingTransactionTracker(options); - await onLatestBlock(undefined, []); + await onPoll(undefined, []); - expect(options.approveTransaction).toHaveBeenCalledTimes(0); expect(options.publishTransaction).toHaveBeenCalledTimes(0); }); }); @@ -650,7 +664,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith( @@ -682,7 +696,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); getTransactions.mockReturnValue( freeze( [ @@ -694,7 +708,7 @@ describe('PendingTransactionTracker', () => { true, ), ); - await onLatestBlock('0x124'); + await onPoll('0x124'); expect(listener).toHaveBeenCalledTimes(2); expect(listener).toHaveBeenCalledWith( @@ -731,7 +745,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); getTransactions.mockReturnValue( freeze( [ @@ -743,7 +757,7 @@ describe('PendingTransactionTracker', () => { true, ), ); - await onLatestBlock('0x124'); + await onPoll('0x124'); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith( @@ -780,7 +794,7 @@ describe('PendingTransactionTracker', () => { new Error('TestError'), ); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); getTransactions.mockReturnValue( freeze( [ @@ -792,7 +806,7 @@ describe('PendingTransactionTracker', () => { true, ), ); - await onLatestBlock('0x124'); + await onPoll('0x124'); expect(listener).toHaveBeenCalledTimes(2); expect(listener).toHaveBeenCalledWith( @@ -833,7 +847,7 @@ describe('PendingTransactionTracker', () => { new Error('test gas price too low to replace test'), ); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); getTransactions.mockReturnValue( freeze( [ @@ -845,7 +859,7 @@ describe('PendingTransactionTracker', () => { true, ), ); - await onLatestBlock('0x124'); + await onPoll('0x124'); expect(listener).toHaveBeenCalledTimes(1); expect(listener).not.toHaveBeenCalledWith( @@ -870,7 +884,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); getTransactions.mockReturnValue( freeze( [ @@ -882,7 +896,7 @@ describe('PendingTransactionTracker', () => { true, ), ); - await onLatestBlock('0x124'); + await onPoll('0x124'); expect(options.publishTransaction).toHaveBeenCalledTimes(1); expect(options.publishTransaction).toHaveBeenCalledWith( @@ -908,7 +922,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); expect(options.publishTransaction).toHaveBeenCalledTimes(0); getTransactions.mockReturnValue( freeze( @@ -922,7 +936,7 @@ describe('PendingTransactionTracker', () => { ), ); - await onLatestBlock('0x124'); + await onPoll('0x124'); expect(options.publishTransaction).toHaveBeenCalledTimes(1); getTransactions.mockReturnValue( freeze( @@ -937,7 +951,7 @@ describe('PendingTransactionTracker', () => { ), ); - await onLatestBlock('0x125'); + await onPoll('0x125'); expect(options.publishTransaction).toHaveBeenCalledTimes(2); getTransactions.mockReturnValue( freeze( @@ -952,10 +966,10 @@ describe('PendingTransactionTracker', () => { ), ); - await onLatestBlock('0x126'); + await onPoll('0x126'); expect(options.publishTransaction).toHaveBeenCalledTimes(2); - await onLatestBlock('0x127'); + await onPoll('0x127'); expect(options.publishTransaction).toHaveBeenCalledTimes(3); getTransactions.mockReturnValue( freeze( @@ -970,10 +984,10 @@ describe('PendingTransactionTracker', () => { ), ); - await onLatestBlock('0x12A'); + await onPoll('0x12A'); expect(options.publishTransaction).toHaveBeenCalledTimes(3); - await onLatestBlock('0x12B'); + await onPoll('0x12B'); expect(options.publishTransaction).toHaveBeenCalledTimes(4); }); @@ -992,7 +1006,7 @@ describe('PendingTransactionTracker', () => { queryMock.mockResolvedValueOnce(undefined); queryMock.mockResolvedValueOnce('0x1'); - await onLatestBlock(BLOCK_NUMBER_MOCK); + await onPoll(BLOCK_NUMBER_MOCK); getTransactions.mockReturnValue( freeze( @@ -1006,7 +1020,7 @@ describe('PendingTransactionTracker', () => { ), ); - await onLatestBlock('0x124'); + await onPoll('0x124'); expect(options.publishTransaction).toHaveBeenCalledTimes(0); }); diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index dc4e12d8c7..2ee8802ab4 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -10,6 +10,7 @@ import { cloneDeep, merge } from 'lodash'; import { createModuleLogger, projectLogger } from '../logger'; import type { TransactionMeta, TransactionReceipt } from '../types'; import { TransactionStatus, TransactionType } from '../types'; +import { TransactionPoller } from './TransactionPoller'; /** * We wait this many blocks before emitting a 'transaction-dropped' event @@ -63,8 +64,6 @@ export interface PendingTransactionTrackerEventEmitter extends EventEmitter { export class PendingTransactionTracker { hub: PendingTransactionTrackerEventEmitter; - #blockTracker: BlockTracker; - #droppedBlockCountByHash: Map; #getChainId: () => string; @@ -88,6 +87,8 @@ export class PendingTransactionTracker { #running: boolean; + #transactionPoller: TransactionPoller; + #beforeCheckPendingTransaction: (transactionMeta: TransactionMeta) => boolean; #beforePublish: (transactionMeta: TransactionMeta) => boolean; @@ -121,7 +122,6 @@ export class PendingTransactionTracker { }) { this.hub = new EventEmitter() as PendingTransactionTrackerEventEmitter; - this.#blockTracker = blockTracker; this.#droppedBlockCountByHash = new Map(); this.#getChainId = getChainId; this.#getEthQuery = getEthQuery; @@ -131,6 +131,7 @@ export class PendingTransactionTracker { this.#getGlobalLock = getGlobalLock; this.#publishTransaction = publishTransaction; this.#running = false; + this.#transactionPoller = new TransactionPoller(blockTracker); this.#beforePublish = hooks?.beforePublish ?? (() => true); this.#beforeCheckPendingTransaction = hooks?.beforeCheckPendingTransaction ?? (() => true); @@ -140,7 +141,7 @@ export class PendingTransactionTracker { const pendingTransactions = this.#getPendingTransactions(); if (pendingTransactions.length) { - this.#start(); + this.#start(pendingTransactions); } else { this.stop(); } @@ -164,12 +165,14 @@ export class PendingTransactionTracker { } } - #start() { + #start(pendingTransactions: TransactionMeta[]) { + this.#transactionPoller.setPendingTransactions(pendingTransactions); + if (this.#running) { return; } - this.#blockTracker.on('latest', this.#listener); + this.#transactionPoller.start(this.#listener); this.#running = true; log('Started polling'); @@ -180,7 +183,7 @@ export class PendingTransactionTracker { return; } - this.#blockTracker.removeListener('latest', this.#listener); + this.#transactionPoller.stop(); this.#running = false; log('Stopped polling'); diff --git a/packages/transaction-controller/src/helpers/TransactionPoller.test.ts b/packages/transaction-controller/src/helpers/TransactionPoller.test.ts new file mode 100644 index 0000000000..c3dfd28f4c --- /dev/null +++ b/packages/transaction-controller/src/helpers/TransactionPoller.test.ts @@ -0,0 +1,264 @@ +import type { BlockTracker } from '@metamask/network-controller'; + +import { flushPromises } from '../../../../tests/helpers'; +import type { TransactionMeta } from '../types'; +import { ACCELERATED_COUNT_MAX, TransactionPoller } from './TransactionPoller'; + +jest.useFakeTimers(); + +const BLOCK_NUMBER_MOCK = '0x123'; + +const BLOCK_TRACKER_MOCK = { + getLatestBlock: jest.fn(), + on: jest.fn(), + removeListener: jest.fn(), +} as unknown as jest.Mocked; + +/** + * Creates a mock transaction metadata object. + * @param id - The transaction ID. + * @returns The mock transaction metadata object. + */ +function createTransactionMetaMock(id: string) { + return { id } as TransactionMeta; +} + +describe('TransactionPoller', () => { + beforeEach(() => { + jest.resetAllMocks(); + jest.clearAllTimers(); + }); + + describe('Accelerated Polling', () => { + it('invokes listener after timeout', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + const listener = jest.fn(); + poller.start(listener); + + expect(jest.getTimerCount()).toBe(1); + + jest.runOnlyPendingTimers(); + await flushPromises(); + + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('stops creating timeouts after max reached', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + const listener = jest.fn(); + poller.start(listener); + + for (let i = 0; i < ACCELERATED_COUNT_MAX * 3; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + expect(listener).toHaveBeenCalledTimes(ACCELERATED_COUNT_MAX); + }); + + it('invokes listener with latest block number from block tracker', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + BLOCK_TRACKER_MOCK.getLatestBlock.mockResolvedValue(BLOCK_NUMBER_MOCK); + + const listener = jest.fn(); + poller.start(listener); + + jest.runOnlyPendingTimers(); + await flushPromises(); + + expect(listener).toHaveBeenCalledWith(BLOCK_NUMBER_MOCK); + }); + + it('does not create timeout if stopped while listener being invoked', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + const listener = jest.fn(); + listener.mockImplementation(() => poller.stop()); + + poller.start(listener); + + jest.runOnlyPendingTimers(); + await flushPromises(); + + expect(jest.getTimerCount()).toBe(0); + }); + }); + + describe('Block Tracker Polling', () => { + it('invokes listener on block tracker update after accelerated limit reached', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + const listener = jest.fn(); + poller.start(listener); + + for (let i = 0; i < ACCELERATED_COUNT_MAX; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + BLOCK_TRACKER_MOCK.on.mock.calls[0][1](); + await flushPromises(); + + BLOCK_TRACKER_MOCK.on.mock.calls[0][1](); + await flushPromises(); + + expect(listener).toHaveBeenCalledTimes(ACCELERATED_COUNT_MAX + 2); + }); + + it('invokes listener with latest block number from event', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + const listener = jest.fn(); + poller.start(listener); + + for (let i = 0; i < ACCELERATED_COUNT_MAX; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + BLOCK_TRACKER_MOCK.on.mock.calls[0][1](BLOCK_NUMBER_MOCK); + await flushPromises(); + + expect(listener).toHaveBeenCalledWith(BLOCK_NUMBER_MOCK); + }); + }); + + describe('start', () => { + it('does nothing if already started', () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + poller.start(jest.fn()); + poller.start(jest.fn()); + + expect(jest.getTimerCount()).toBe(1); + }); + }); + + describe('stop', () => { + it('removes timeout', () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + const listener = jest.fn(); + poller.start(listener); + poller.stop(); + + expect(jest.getTimerCount()).toBe(0); + expect(listener).not.toHaveBeenCalled(); + }); + + it('removes block tracker listener', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + const listener = jest.fn(); + poller.start(listener); + + for (let i = 0; i < ACCELERATED_COUNT_MAX; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + poller.stop(); + + expect(BLOCK_TRACKER_MOCK.removeListener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledTimes(ACCELERATED_COUNT_MAX); + }); + + it('does nothing if not started', async () => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + poller.stop(); + + expect(jest.getTimerCount()).toBe(0); + expect(BLOCK_TRACKER_MOCK.removeListener).not.toHaveBeenCalled(); + }); + }); + + describe('setPendingTransactions', () => { + it.each([ + [ + 'added', + [ + createTransactionMetaMock('1'), + createTransactionMetaMock('2'), + createTransactionMetaMock('3'), + ], + ], + ['removed', [createTransactionMetaMock('1')]], + ])( + 'resets accelerated count if transaction IDs %s', + async (_title, newPendingTransactions) => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + poller.setPendingTransactions([ + createTransactionMetaMock('1'), + createTransactionMetaMock('2'), + ]); + + const listener = jest.fn(); + poller.start(listener); + + for (let i = 0; i < 3; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + poller.setPendingTransactions(newPendingTransactions); + + for (let i = 0; i < ACCELERATED_COUNT_MAX; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + expect(listener).toHaveBeenCalledTimes(ACCELERATED_COUNT_MAX + 3); + }, + ); + + it.each([ + [ + 'added', + [ + createTransactionMetaMock('1'), + createTransactionMetaMock('2'), + createTransactionMetaMock('3'), + ], + ], + ['removed', [createTransactionMetaMock('1')]], + ])( + 'resets to accelerated polling if transaction IDs added', + async (_title, newPendingTransactions) => { + const poller = new TransactionPoller(BLOCK_TRACKER_MOCK); + + poller.setPendingTransactions([ + createTransactionMetaMock('1'), + createTransactionMetaMock('2'), + ]); + + const listener = jest.fn(); + poller.start(listener); + + for (let i = 0; i < ACCELERATED_COUNT_MAX; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + BLOCK_TRACKER_MOCK.on.mock.calls[0][1](BLOCK_NUMBER_MOCK); + await flushPromises(); + + BLOCK_TRACKER_MOCK.on.mock.calls[0][1](BLOCK_NUMBER_MOCK); + await flushPromises(); + + poller.setPendingTransactions(newPendingTransactions); + + for (let i = 0; i < ACCELERATED_COUNT_MAX; i++) { + jest.runOnlyPendingTimers(); + await flushPromises(); + } + + expect(listener).toHaveBeenCalledTimes(ACCELERATED_COUNT_MAX * 2 + 2); + }, + ); + }); +}); diff --git a/packages/transaction-controller/src/helpers/TransactionPoller.ts b/packages/transaction-controller/src/helpers/TransactionPoller.ts new file mode 100644 index 0000000000..4cf71f09e8 --- /dev/null +++ b/packages/transaction-controller/src/helpers/TransactionPoller.ts @@ -0,0 +1,168 @@ +import type { BlockTracker } from '@metamask/network-controller'; +import { createModuleLogger } from '@metamask/utils'; +import { isEqual } from 'lodash'; + +import { projectLogger } from '../logger'; +import type { TransactionMeta } from '../types'; + +export const ACCELERATED_COUNT_MAX = 5; +export const ACCELERATED_INTERVAL = 1000 * 2; // 2 Seconds + +const log = createModuleLogger(projectLogger, 'transaction-poller'); + +/** + * Helper class to orchestrate when to poll pending transactions. + * Initially starts polling via a timeout chain every 2 seconds up to 5 times. + * Following that, it will poll on every new block via the block tracker. + */ +export class TransactionPoller { + #acceleratedCount = 0; + + #blockTracker: BlockTracker; + + #blockTrackerListener?: (latestBlockNumber: string) => void; + + #listener?: (latestBlockNumber: string) => Promise; + + #pendingTransactions?: TransactionMeta[]; + + #running = false; + + #timeout?: NodeJS.Timeout; + + constructor(blockTracker: BlockTracker) { + this.#blockTracker = blockTracker; + } + + /** + * Start the poller with a listener that will be called on every interval. + * @param listener - The listener to call on every interval. + */ + start(listener: (latestBlockNumber: string) => Promise) { + if (this.#running) { + return; + } + + this.#listener = listener; + this.#running = true; + + this.#queue(); + + log('Started'); + } + + /** + * Stop the poller. + * Remove all timeouts and block tracker listeners. + */ + stop() { + if (!this.#running) { + return; + } + + this.#running = false; + this.#listener = undefined; + this.#acceleratedCount = 0; + this.#pendingTransactions = undefined; + + this.#stopTimeout(); + this.#stopBlockTracker(); + + log('Stopped'); + } + + /** + * Notify the poller of the pending transactions being monitored. + * This will reset to the accelerated polling and reset the count + * when new transactions are added or removed. + * @param pendingTransactions - The pending transactions to poll. + */ + setPendingTransactions(pendingTransactions: TransactionMeta[]) { + const currentPendingTransactionIds = (this.#pendingTransactions ?? []).map( + (tx) => tx.id, + ); + + this.#pendingTransactions = pendingTransactions; + + const newPendingTransactionIds = pendingTransactions.map((tx) => tx.id); + + const hasUpdatedIds = !isEqual( + currentPendingTransactionIds, + newPendingTransactionIds, + ); + + if (!this.#running || !hasUpdatedIds) { + return; + } + + log('Detected new pending transactions', newPendingTransactionIds); + + this.#acceleratedCount = 0; + + if (this.#blockTrackerListener) { + this.#stopBlockTracker(); + this.#queue(); + } + } + + #queue() { + if (!this.#running) { + return; + } + + if (this.#acceleratedCount >= ACCELERATED_COUNT_MAX) { + // eslint-disable-next-line @typescript-eslint/no-misused-promises + this.#blockTrackerListener = (latestBlockNumber) => + this.#interval(false, latestBlockNumber); + + this.#blockTracker.on('latest', this.#blockTrackerListener); + + log('Added block tracker listener'); + + return; + } + + this.#stopTimeout(); + + // eslint-disable-next-line @typescript-eslint/no-misused-promises + this.#timeout = setTimeout(async () => { + await this.#interval(true); + this.#queue(); + }, ACCELERATED_INTERVAL); + } + + async #interval(isAccelerated: boolean, latestBlockNumber?: string) { + if (isAccelerated) { + log('Accelerated interval', this.#acceleratedCount + 1); + } else { + log('Block tracker interval', latestBlockNumber); + } + + const latestBlockNumberFinal = + latestBlockNumber ?? (await this.#blockTracker.getLatestBlock()); + + await this.#listener?.(latestBlockNumberFinal); + + if (isAccelerated && this.#running) { + this.#acceleratedCount += 1; + } + } + + #stopTimeout() { + if (!this.#timeout) { + return; + } + + clearTimeout(this.#timeout); + this.#timeout = undefined; + } + + #stopBlockTracker() { + if (!this.#blockTrackerListener) { + return; + } + + this.#blockTracker.removeListener('latest', this.#blockTrackerListener); + this.#blockTrackerListener = undefined; + } +}