diff --git a/eslint-warning-thresholds.json b/eslint-warning-thresholds.json index 4b387a4f112..19a192a361b 100644 --- a/eslint-warning-thresholds.json +++ b/eslint-warning-thresholds.json @@ -565,7 +565,6 @@ "jsdoc/tag-lines": 4 }, "packages/transaction-controller/src/TransactionController.test.ts": { - "@typescript-eslint/no-unused-vars": 1, "import-x/namespace": 1, "import-x/order": 4, "jsdoc/tag-lines": 1, @@ -701,13 +700,6 @@ "packages/transaction-controller/src/utils/nonce.test.ts": { "import-x/order": 1 }, - "packages/transaction-controller/src/utils/resimulate.test.ts": { - "import-x/order": 2 - }, - "packages/transaction-controller/src/utils/resimulate.ts": { - "import-x/order": 1, - "jsdoc/tag-lines": 7 - }, "packages/transaction-controller/src/utils/retry.test.ts": { "import-x/order": 1 }, diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 1ce358fd59f..01428f01bce 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Adds ability of re-simulating transaction depending on the `isActive` property on `transactionMeta` ([#5189](https://github.com/MetaMask/core/pull/5189)) + - `isActive` property is expected to set by client. + - Re-simulation of transactions will occur every 3 seconds if `isActive` is `true`. +- Adds `setTransactionActive` function to update the `isActive` property on `transactionMeta`. ([#5189](https://github.com/MetaMask/core/pull/5189)) + ## [45.1.0] ### Added diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index 8072dc420d7..3ae6e5e21b6 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: 91.76, - functions: 94.62, + functions: 94.57, lines: 96.83, statements: 96.82, }, diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index d788cf471d4..4571fbe2014 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -50,6 +50,7 @@ import { IncomingTransactionHelper } from './helpers/IncomingTransactionHelper'; import { MethodDataHelper } from './helpers/MethodDataHelper'; import { MultichainTrackingHelper } from './helpers/MultichainTrackingHelper'; import { PendingTransactionTracker } from './helpers/PendingTransactionTracker'; +import { shouldResimulate } from './helpers/ResimulateHelper'; import type { AllowedActions, AllowedEvents, @@ -86,7 +87,6 @@ import { getTransactionLayer1GasFee, updateTransactionLayer1GasFee, } from './utils/layer1-gas-fee-flow'; -import { shouldResimulate } from './utils/resimulate'; import { getSimulationData } from './utils/simulation'; import { updatePostTransactionBalance, @@ -115,7 +115,10 @@ jest.mock('./utils/gas'); jest.mock('./utils/gas-fees'); jest.mock('./utils/gas-flow'); jest.mock('./utils/layer1-gas-fee-flow'); -jest.mock('./utils/resimulate'); +jest.mock('./helpers/ResimulateHelper', () => ({ + ...jest.requireActual('./helpers/ResimulateHelper'), + shouldResimulate: jest.fn(), +})); jest.mock('./utils/simulation'); jest.mock('./utils/swaps'); jest.mock('uuid'); @@ -2354,7 +2357,7 @@ describe('TransactionController', () => { try { await result; - } catch (error) { + } catch { // Ignore user rejected error as it is expected } await finishedPromise; @@ -6075,4 +6078,41 @@ describe('TransactionController', () => { ); }); }); + + describe('setTransactionActive', () => { + it('throws if transaction does not exist', async () => { + const { controller } = setupController(); + expect(() => controller.setTransactionActive('123', true)).toThrow( + 'Transaction with id 123 not found', + ); + }); + + it('updates the isActive state of a transaction', async () => { + const transactionId = '123'; + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + id: transactionId, + status: TransactionStatus.unapproved, + history: [{}], + txParams: { + from: ACCOUNT_MOCK, + to: ACCOUNT_2_MOCK, + }, + } as unknown as TransactionMeta, + ], + }, + }, + updateToInitialState: true, + }); + + controller.setTransactionActive(transactionId, true); + + const transaction = controller.state.transactions[0]; + + expect(transaction?.isActive).toBe(true); + }); + }); }); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 54e763c1077..60ffcb37e83 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -65,6 +65,12 @@ import { IncomingTransactionHelper } from './helpers/IncomingTransactionHelper'; import { MethodDataHelper } from './helpers/MethodDataHelper'; import { MultichainTrackingHelper } from './helpers/MultichainTrackingHelper'; import { PendingTransactionTracker } from './helpers/PendingTransactionTracker'; +import type { ResimulateResponse } from './helpers/ResimulateHelper'; +import { + ResimulateHelper, + hasSimulationDataChanged, + shouldResimulate, +} from './helpers/ResimulateHelper'; import { projectLogger as log } from './logger'; import type { DappSuggestedGasFees, @@ -110,8 +116,6 @@ import { getNextNonce, } from './utils/nonce'; import { prepareTransaction, serializeTransaction } from './utils/prepare'; -import type { ResimulateResponse } from './utils/resimulate'; -import { hasSimulationDataChanged, shouldResimulate } from './utils/resimulate'; import { getTransactionParamsWithIncreasedGasFee } from './utils/retry'; import { getSimulationData } from './utils/simulation'; import { @@ -926,6 +930,18 @@ export class TransactionController extends BaseController< this.#checkForPendingTransactionAndStartPolling, ); + new ResimulateHelper({ + simulateTransaction: this.#updateSimulationData.bind(this), + onTransactionsUpdate: (listener) => { + this.messagingSystem.subscribe( + 'TransactionController:stateChange', + listener, + (controllerState) => controllerState.transactions, + ); + }, + getTransactions: () => this.state.transactions, + }); + this.onBootCleanup(); this.#checkForPendingTransactionAndStartPolling(); } @@ -1862,6 +1878,33 @@ export class TransactionController extends BaseController< return this.getTransaction(txId); } + /** + * Update the isActive state of a transaction. + * + * @param transactionId - The ID of the transaction to update. + * @param isActive - The active state. + */ + setTransactionActive(transactionId: string, isActive: boolean) { + const transactionMeta = this.getTransaction(transactionId); + + if (!transactionMeta) { + throw new Error(`Transaction with id ${transactionId} not found`); + } + + this.#updateTransactionInternal( + { + transactionId, + note: 'TransactionController#setTransactionActive - Transaction isActive updated', + skipHistory: true, + skipValidation: true, + skipResimulateCheck: true, + }, + (updatedTransactionMeta) => { + updatedTransactionMeta.isActive = isActive; + }, + ); + } + /** * Signs and returns the raw transaction data for provided transaction params list. * diff --git a/packages/transaction-controller/src/utils/resimulate.test.ts b/packages/transaction-controller/src/helpers/ResimulateHelper.test.ts similarity index 70% rename from packages/transaction-controller/src/utils/resimulate.test.ts rename to packages/transaction-controller/src/helpers/ResimulateHelper.test.ts index f27e19498cd..9c372dd4264 100644 --- a/packages/transaction-controller/src/utils/resimulate.test.ts +++ b/packages/transaction-controller/src/helpers/ResimulateHelper.test.ts @@ -1,26 +1,27 @@ -/* eslint-disable @typescript-eslint/naming-convention */ import { NetworkType } from '@metamask/controller-utils'; +import type { NetworkClientId } from '@metamask/network-controller'; import { BN } from 'bn.js'; -import { CHAIN_IDS } from '../constants'; -import type { - SecurityAlertResponse, - SimulationData, - SimulationTokenBalanceChange, - TransactionMeta, -} from '../types'; -import { SimulationTokenStandard, TransactionStatus } from '../types'; import { + type ResimulateHelperOptions, + ResimulateHelper, BLOCK_TIME_ADDITIONAL_SECONDS, BLOCKAID_RESULT_TYPE_MALICIOUS, hasSimulationDataChanged, RESIMULATE_PARAMS, shouldResimulate, VALUE_COMPARISON_PERCENT_THRESHOLD, -} from './resimulate'; -import { getPercentageChange } from './utils'; - -jest.mock('./utils'); + RESIMULATE_INTERVAL_MS, +} from './ResimulateHelper'; +import { CHAIN_IDS } from '../constants'; +import type { + TransactionMeta, + SecurityAlertResponse, + SimulationData, + SimulationTokenBalanceChange, +} from '../types'; +import { TransactionStatus, SimulationTokenStandard } from '../types'; +import { getPercentageChange } from '../utils/utils'; const CURRENT_TIME_MOCK = 1234567890; const CURRENT_TIME_SECONDS_MOCK = 1234567; @@ -74,6 +75,139 @@ const TRANSACTION_META_MOCK: TransactionMeta = { }, }; +const mockTransactionMeta = { + id: '1', + networkClientId: 'network1' as NetworkClientId, + isActive: true, + status: TransactionStatus.unapproved, +} as TransactionMeta; + +jest.mock('../utils/utils'); + +describe('ResimulateHelper', () => { + let getTransactionsMock: jest.Mock<() => TransactionMeta[]>; + let simulateTransactionMock: jest.Mock< + (transactionMeta: TransactionMeta) => Promise + >; + let onTransactionsUpdateMock: jest.Mock<(listener: () => void) => void>; + + /** + * Triggers onStateChange callback + */ + function triggerStateChange() { + onTransactionsUpdateMock.mock.calls[0][0](); + } + + /** + * Mocks getTransactions to return given transactions argument + * + * @param transactions - Transactions to be returned + */ + function mockGetTransactionsOnce(transactions: TransactionMeta[]) { + getTransactionsMock.mockReturnValueOnce( + transactions as unknown as ResimulateHelperOptions['getTransactions'], + ); + } + + beforeEach(() => { + jest.useFakeTimers(); + getTransactionsMock = jest.fn(); + onTransactionsUpdateMock = jest.fn(); + simulateTransactionMock = jest.fn().mockResolvedValue(undefined); + + new ResimulateHelper({ + getTransactions: getTransactionsMock, + onTransactionsUpdate: onTransactionsUpdateMock, + simulateTransaction: simulateTransactionMock, + } as unknown as ResimulateHelperOptions); + }); + + afterEach(() => { + jest.clearAllTimers(); + }); + + it(`resimulates unapproved active transaction every ${RESIMULATE_INTERVAL_MS} milliseconds`, async () => { + mockGetTransactionsOnce([mockTransactionMeta]); + triggerStateChange(); + + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS); + await Promise.resolve(); + + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS); + await Promise.resolve(); + + jest.runAllTimers(); + + expect(simulateTransactionMock).toHaveBeenCalledWith(mockTransactionMeta); + expect(simulateTransactionMock).toHaveBeenCalledTimes(2); + }); + + it(`does not resimulate twice the same transaction even if state change is triggered twice`, async () => { + mockGetTransactionsOnce([mockTransactionMeta]); + triggerStateChange(); + + // Halfway through the interval + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS / 2); + + // Assume state change is triggered again + mockGetTransactionsOnce([mockTransactionMeta]); + triggerStateChange(); + + // Halfway through the interval + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS / 2); + + expect(simulateTransactionMock).toHaveBeenCalledTimes(1); + }); + + it('does not resimulate a transaction that is no longer active', () => { + mockGetTransactionsOnce([mockTransactionMeta]); + triggerStateChange(); + + // Halfway through the interval + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS / 2); + + const inactiveTransactionMeta = { + ...mockTransactionMeta, + isActive: false, + } as TransactionMeta; + + mockGetTransactionsOnce([inactiveTransactionMeta]); + triggerStateChange(); + + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS / 2); + + expect(simulateTransactionMock).toHaveBeenCalledTimes(0); + }); + + it('does not resimulate a transaction that is not active', () => { + const inactiveTransactionMeta = { + ...mockTransactionMeta, + isActive: false, + } as TransactionMeta; + + mockGetTransactionsOnce([inactiveTransactionMeta]); + triggerStateChange(); + + jest.advanceTimersByTime(2 * RESIMULATE_INTERVAL_MS); + + expect(simulateTransactionMock).toHaveBeenCalledTimes(0); + }); + + it('stops resimulating a transaction that is no longer in the transaction list', () => { + mockGetTransactionsOnce([mockTransactionMeta]); + triggerStateChange(); + + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS); + + mockGetTransactionsOnce([]); + triggerStateChange(); + + jest.advanceTimersByTime(RESIMULATE_INTERVAL_MS); + + expect(simulateTransactionMock).toHaveBeenCalledTimes(1); + }); +}); + describe('Resimulate Utils', () => { const getPercentageChangeMock = jest.mocked(getPercentageChange); diff --git a/packages/transaction-controller/src/utils/resimulate.ts b/packages/transaction-controller/src/helpers/ResimulateHelper.ts similarity index 70% rename from packages/transaction-controller/src/utils/resimulate.ts rename to packages/transaction-controller/src/helpers/ResimulateHelper.ts index b339356c7a2..6bb1a51c23c 100644 --- a/packages/transaction-controller/src/utils/resimulate.ts +++ b/packages/transaction-controller/src/helpers/ResimulateHelper.ts @@ -1,31 +1,142 @@ import type { Hex } from '@metamask/utils'; -import { createModuleLogger, remove0x } from '@metamask/utils'; +import { remove0x } from '@metamask/utils'; import { BN } from 'bn.js'; import { isEqual } from 'lodash'; -import { projectLogger } from '../logger'; +import { createModuleLogger, projectLogger } from '../logger'; +import { TransactionStatus } from '../types'; import type { SimulationBalanceChange, SimulationData, TransactionMeta, TransactionParams, } from '../types'; -import { getPercentageChange } from './utils'; +import { getPercentageChange } from '../utils/utils'; -const log = createModuleLogger(projectLogger, 'resimulate'); +const log = createModuleLogger(projectLogger, 'resimulate-helper'); export const RESIMULATE_PARAMS = ['to', 'value', 'data'] as const; export const BLOCKAID_RESULT_TYPE_MALICIOUS = 'Malicious'; export const VALUE_COMPARISON_PERCENT_THRESHOLD = 5; export const BLOCK_TIME_ADDITIONAL_SECONDS = 60; +export const RESIMULATE_INTERVAL_MS = 3000; export type ResimulateResponse = { blockTime?: number; resimulate: boolean; }; +export type ResimulateHelperOptions = { + getTransactions: () => TransactionMeta[]; + onTransactionsUpdate: (listener: () => void) => void; + simulateTransaction: (transactionMeta: TransactionMeta) => Promise; +}; + +export class ResimulateHelper { + // Map of transactionId <=> timeoutId + readonly #timeoutIds: Map = new Map(); + + readonly #getTransactions: () => TransactionMeta[]; + + readonly #simulateTransaction: ( + transactionMeta: TransactionMeta, + ) => Promise; + + constructor({ + getTransactions, + simulateTransaction, + onTransactionsUpdate, + }: ResimulateHelperOptions) { + this.#getTransactions = getTransactions; + this.#simulateTransaction = simulateTransaction; + + onTransactionsUpdate(this.#onTransactionsUpdate.bind(this)); + } + + #onTransactionsUpdate() { + const unapprovedTransactions = this.#getTransactions().filter( + (tx) => tx.status === TransactionStatus.unapproved, + ); + + const unapprovedTransactionIds = new Set( + unapprovedTransactions.map((tx) => tx.id), + ); + + // Combine unapproved transaction IDs and currently active resimulations + const allTransactionIds = new Set([ + ...unapprovedTransactionIds, + ...this.#timeoutIds.keys(), + ]); + + allTransactionIds.forEach((transactionId) => { + const transactionMeta = unapprovedTransactions.find( + (tx) => tx.id === transactionId, + ) as TransactionMeta; + + if (transactionMeta?.isActive) { + this.#start(transactionMeta); + } else { + this.#stop(transactionId); + } + }); + } + + #start(transactionMeta: TransactionMeta) { + const { id: transactionId } = transactionMeta; + if (this.#timeoutIds.has(transactionId)) { + return; + } + + const listener = () => { + // eslint-disable-next-line promise/catch-or-return + this.#simulateTransaction(transactionMeta) + .catch((error) => { + /* istanbul ignore next */ + log('Error during transaction resimulation', error); + }) + .finally(() => { + // Schedule the next execution + if (this.#timeoutIds.has(transactionId)) { + this.#queueUpdate(transactionId, listener); + } + }); + }; + + // Start the first execution + this.#queueUpdate(transactionId, listener); + log( + `Started resimulating transaction ${transactionId} every ${RESIMULATE_INTERVAL_MS} milliseconds`, + ); + } + + #queueUpdate(transactionId: string, listener: () => void) { + const timeoutId = setTimeout(listener, RESIMULATE_INTERVAL_MS); + this.#timeoutIds.set(transactionId, timeoutId); + } + + #stop(transactionId: string) { + if (!this.#timeoutIds.has(transactionId)) { + return; + } + + this.#removeListener(transactionId); + log( + `Stopped resimulating transaction ${transactionId} every ${RESIMULATE_INTERVAL_MS} milliseconds`, + ); + } + + #removeListener(id: string) { + const timeoutId = this.#timeoutIds.get(id); + if (timeoutId) { + clearTimeout(timeoutId); + this.#timeoutIds.delete(id); + } + } +} + /** * Determine if a transaction should be resimulated. + * * @param originalTransactionMeta - The original transaction metadata. * @param newTransactionMeta - The new transaction metadata. * @returns Whether the transaction should be resimulated. @@ -79,6 +190,7 @@ export function shouldResimulate( /** * Determine if the simulation data has changed. + * * @param originalSimulationData - The original simulation data. * @param newSimulationData - The new simulation data. * @returns Whether the simulation data has changed. @@ -141,6 +253,7 @@ export function hasSimulationDataChanged( /** * Determine if the transaction parameters have been updated. + * * @param originalTransactionMeta - The original transaction metadata. * @param newTransactionMeta - The new transaction metadata. * @returns Whether the transaction parameters have been updated. @@ -174,6 +287,7 @@ function isParametersUpdated( /** * Determine if a transaction has a new security alert. + * * @param originalTransactionMeta - The original transaction metadata. * @param newTransactionMeta - The new transaction metadata. * @returns Whether the transaction has a new security alert. @@ -205,6 +319,7 @@ function hasNewSecurityAlert( /** * Determine if a transaction has a value and simulation native balance mismatch. + * * @param originalTransactionMeta - The original transaction metadata. * @param newTransactionMeta - The new transaction metadata. * @returns Whether the transaction has a value and simulation native balance mismatch. @@ -240,6 +355,7 @@ function hasValueAndNativeBalanceMismatch( /** * Determine if a balance change has been updated. + * * @param originalBalanceChange - The original balance change. * @param newBalanceChange - The new balance change. * @returns Whether the balance change has been updated. @@ -258,6 +374,7 @@ function isBalanceChangeUpdated( /** * Determine if the percentage change between two values is within a threshold. + * * @param originalValue - The original value. * @param newValue - The new value. * @param originalNegative - Whether the original value is negative. diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 5d7ec9d5895..33eecc9a9c0 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -171,6 +171,11 @@ type TransactionMetaBase = { */ firstRetryBlockNumber?: string; + /** + * Whether the transaction is active. + */ + isActive?: boolean; + /** * Whether the transaction is the first time interaction. */