Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Re-simulate active transactions every 3000 ms #5189

Merged
merged 50 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
462ebe6
Resimulate transactions on focused transaction confirmations
OGPoyraz Jan 23, 2025
9df90b5
Add console.logs
OGPoyraz Jan 23, 2025
8a720d1
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Jan 24, 2025
7c32eb6
Add unit tests
OGPoyraz Jan 24, 2025
8753b53
Reorganise
OGPoyraz Jan 24, 2025
4dd7520
Finalise force stop
OGPoyraz Jan 24, 2025
81fa0e3
Fix lint
OGPoyraz Jan 24, 2025
ee4c784
Fix tests
OGPoyraz Jan 24, 2025
e7362fc
Relocate resimulate helpers
OGPoyraz Jan 24, 2025
f3d5e31
Fix lint
OGPoyraz Jan 24, 2025
ea69888
Change resimulation logic to simulate every 3 seconds
OGPoyraz Jan 29, 2025
9cf877a
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Jan 29, 2025
e51c876
Fix lint
OGPoyraz Jan 29, 2025
e8993c9
Remove any usage
OGPoyraz Jan 29, 2025
86dba33
Remove unnecessary comments
OGPoyraz Jan 29, 2025
bd04b24
Final lint fixes
OGPoyraz Jan 29, 2025
d18f782
Fix lint issues
OGPoyraz Jan 29, 2025
48ed1b5
Fix lint issues
OGPoyraz Jan 29, 2025
253680c
Update packages/transaction-controller/src/TransactionController.ts
OGPoyraz Jan 30, 2025
248556b
Update packages/transaction-controller/src/helpers/ResimulateHelper.ts
OGPoyraz Jan 30, 2025
5e888db
Update packages/transaction-controller/src/helpers/ResimulateHelper.ts
OGPoyraz Jan 30, 2025
96fdf18
Fix lint
OGPoyraz Jan 30, 2025
43722be
Fix lint
OGPoyraz Jan 30, 2025
6eeb704
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Jan 30, 2025
2e318bd
Fix suggestions
OGPoyraz Feb 3, 2025
765f920
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Feb 3, 2025
02e876e
Fix test
OGPoyraz Feb 3, 2025
c21aa70
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Feb 3, 2025
cf973e4
Update eslint warning thresholds
OGPoyraz Feb 3, 2025
dfb4874
Reformat changelog
OGPoyraz Feb 3, 2025
43593c4
Update coverage threshold
OGPoyraz Feb 3, 2025
7b2025b
Remove unnecessary comments and renamed props
OGPoyraz Feb 3, 2025
d21875e
Rename
OGPoyraz Feb 3, 2025
cf9f794
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Feb 6, 2025
6d20642
Fix lint
OGPoyraz Feb 6, 2025
07707a4
Update eslint threshold
OGPoyraz Feb 6, 2025
50d3e33
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Feb 12, 2025
1bc9e23
Fix thresholds
OGPoyraz Feb 12, 2025
aad4d2f
Fix selector on statechange
OGPoyraz Feb 12, 2025
afbf49e
Fix suggestions
OGPoyraz Feb 12, 2025
bf2c82a
Remove unnecessary check
OGPoyraz Feb 12, 2025
3eee208
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Feb 12, 2025
ff80918
Fix lint
OGPoyraz Feb 12, 2025
a270dac
Fix lint
OGPoyraz Feb 12, 2025
3ade7fa
Fix threshold
OGPoyraz Feb 12, 2025
aa1b5ca
Fix lint
OGPoyraz Feb 12, 2025
4e2889c
Fix unit tests
OGPoyraz Feb 12, 2025
5a4859d
Update eslint thresholds
OGPoyraz Feb 12, 2025
f9c87d1
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Feb 12, 2025
fd8e3e5
Merge branch 'main' into 3922-re-simulate-transactions-for-every-new-…
OGPoyraz Feb 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
},
Expand Down
7 changes: 7 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/transaction-controller/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -2354,7 +2357,7 @@ describe('TransactionController', () => {

try {
await result;
} catch (error) {
} catch {
// Ignore user rejected error as it is expected
}
await finishedPromise;
Expand Down Expand Up @@ -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);
});
});
});
47 changes: 45 additions & 2 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
(controllerState) => controllerState.transactions,
);
},
getTransactions: () => this.state.transactions,
});

this.onBootCleanup();
this.#checkForPendingTransactionAndStartPolling();
}
Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<void>
>;
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);

Expand Down
Loading
Loading