diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index bf8e207d0fe..5e36ec77fe4 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Deprecate `history` and `sendFlowHistory` properties from `TransactionMeta` and `TransactionController` options ([#7326](https://github.com/MetaMask/core/pull/7326)) + ## [62.6.0] ### Added diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 1685eb58cae..f208bdf0ad9 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -69,7 +69,6 @@ import type { TransactionMeta, DappSuggestedGasFees, TransactionParams, - TransactionHistoryEntry, TransactionError, GasFeeFlow, GasFeeFlowResponse, @@ -363,11 +362,10 @@ function waitForTransactionFinished( }); } -const MOCK_PREFERENCES = { state: { selectedAddress: 'foo' } }; const INFURA_PROJECT_ID = 'testinfuraid'; const HTTP_PROVIDERS = { sepolia: new HttpProvider('https://sepolia.infura.io/v3/sepolia-pid'), - // TODO: Investigate and address why tests break when mainet has a different INFURA_PROJECT_ID + // TODO: Investigate and address why tests break when mainnet has a different INFURA_PROJECT_ID mainnet: new HttpProvider( `https://mainnet.infura.io/v3/${INFURA_PROJECT_ID}`, ), @@ -716,8 +714,6 @@ describe('TransactionController', () => { messenger: givenRestrictedMessenger, ...otherOptions }: Partial = { - disableHistory: false, - disableSendFlowHistory: false, disableSwaps: false, getCurrentNetworkEIP1559Compatibility: async () => false, getNetworkState: () => networkState, @@ -729,7 +725,6 @@ describe('TransactionController', () => { isEIP7702GasFeeTokensEnabled: isEIP7702GasFeeTokensEnabledMock, publicKeyEIP7702: '0x1234', sign: signMock, - transactionHistoryLimit: 40, ...givenOptions, }; @@ -1087,42 +1082,36 @@ describe('TransactionController', () => { const mockedTransactions = [ { id: '123', - history: [{ ...mockTransactionMeta, id: '123' }], chainId: toHex(5), status: TransactionStatus.approved, ...mockTransactionMeta, }, { id: '456', - history: [{ ...mockTransactionMeta, id: '456' }], chainId: toHex(1), status: TransactionStatus.approved, ...mockTransactionMeta, }, { id: '789', - history: [{ ...mockTransactionMeta, id: '789' }], chainId: toHex(16), status: TransactionStatus.approved, ...mockTransactionMeta, }, { id: '111', - history: [{ ...mockTransactionMeta, id: '111' }], chainId: toHex(5), status: TransactionStatus.signed, ...mockTransactionMeta, }, { id: '222', - history: [{ ...mockTransactionMeta, id: '222' }], chainId: toHex(1), status: TransactionStatus.signed, ...mockTransactionMeta, }, { id: '333', - history: [{ ...mockTransactionMeta, id: '333' }], chainId: toHex(16), status: TransactionStatus.signed, ...mockTransactionMeta, @@ -1166,7 +1155,6 @@ describe('TransactionController', () => { const mockedTransactions = [ { id: '123', - history: [{ ...mockTransactionMeta, id: '123' }], chainId: toHex(5), status: TransactionStatus.approved, requiredTransactionIds: ['222', '333'], @@ -1174,21 +1162,18 @@ describe('TransactionController', () => { }, { id: '111', - history: [{ ...mockTransactionMeta, id: '111' }], chainId: toHex(5), status: TransactionStatus.signed, ...mockTransactionMeta, }, { id: '222', - history: [{ ...mockTransactionMeta, id: '222' }], chainId: toHex(1), status: TransactionStatus.confirmed, ...mockTransactionMeta, }, { id: '333', - history: [{ ...mockTransactionMeta, id: '333' }], chainId: toHex(16), status: TransactionStatus.submitted, ...mockTransactionMeta, @@ -1216,10 +1201,10 @@ describe('TransactionController', () => { expect( transactions.map((transaction) => [transaction.id, transaction.status]), ).toStrictEqual([ - ['333', TransactionStatus.failed], - ['222', TransactionStatus.confirmed], - ['111', TransactionStatus.failed], ['123', TransactionStatus.failed], + ['111', TransactionStatus.failed], + ['222', TransactionStatus.confirmed], + ['333', TransactionStatus.failed], ]); }); @@ -1235,21 +1220,18 @@ describe('TransactionController', () => { const mockedTransactions = [ { id: '123', - history: [{ ...mockTransactionMeta, id: '123' }], chainId: toHex(5), status: TransactionStatus.unapproved, ...mockTransactionMeta, }, { id: '456', - history: [{ ...mockTransactionMeta, id: '456' }], chainId: toHex(1), status: TransactionStatus.unapproved, ...mockTransactionMeta, }, { id: '789', - history: [{ ...mockTransactionMeta, id: '789' }], chainId: toHex(16), status: TransactionStatus.unapproved, ...mockTransactionMeta, @@ -1609,13 +1591,6 @@ describe('TransactionController', () => { operator: '0x92a3b9773b1763efa556f55ccbeb20441962d9b2', }, }; - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; await controller.addTransaction( { from: ACCOUNT_MOCK, @@ -1629,7 +1604,6 @@ describe('TransactionController', () => { deviceConfirmedOn: mockDeviceConfirmedOn, origin: mockOrigin, securityAlertResponse: mockSecurityAlertResponse, - sendFlowHistory: mockSendFlowHistory, networkClientId: NETWORK_CLIENT_ID_MOCK, }, ); @@ -1651,9 +1625,6 @@ describe('TransactionController', () => { expect(transactionMeta.securityAlertResponse).toStrictEqual( mockSecurityAlertResponse, ); - expect(controller.state.transactions[0].sendFlowHistory).toStrictEqual( - mockSendFlowHistory, - ); expect(updateFirstTimeInteractionMock).toHaveBeenCalledTimes(1); }); @@ -1761,52 +1732,6 @@ describe('TransactionController', () => { }); }); - it('generates initial history', async () => { - const { controller } = setupController(); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - const expectedInitialSnapshot = { - actionId: undefined, - assetsFiatValues: undefined, - batchId: undefined, - chainId: expect.any(String), - dappSuggestedGasFees: undefined, - deviceConfirmedOn: undefined, - disableGasBuffer: undefined, - id: expect.any(String), - isFirstTimeInteraction: undefined, - isGasFeeIncluded: undefined, - isGasFeeSponsored: undefined, - isGasFeeTokenIgnoredIfBalance: false, - nestedTransactions: undefined, - networkClientId: NETWORK_CLIENT_ID_MOCK, - origin: undefined, - securityAlertResponse: undefined, - selectedGasFeeToken: undefined, - sendFlowHistory: expect.any(Array), - status: TransactionStatus.unapproved as const, - time: expect.any(Number), - txParams: expect.anything(), - userEditedGasLimit: false, - type: TransactionType.simpleSend, - verifiedOnBlockchain: expect.any(Boolean), - }; - - // Expect initial snapshot to be in place - expect(controller.state.transactions[0].history).toStrictEqual([ - expectedInitialSnapshot, - ]); - }); - describe('adds dappSuggestedGasFees to transaction', () => { it.each([ ['origin is MM', ORIGIN_METAMASK], @@ -4415,38 +4340,6 @@ describe('TransactionController', () => { expect(transactions[1].originalGasEstimate).toBe('0x1'); }); - it('allows transaction count to exceed txHistorylimit', async () => { - const { controller } = setupController({ - options: { - transactionHistoryLimit: 1, - }, - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - }); - - const { transactionMeta, result } = await controller.addTransaction( - { - from: ACCOUNT_MOCK, - nonce: '1111111', - gas: '0x0', - gasPrice: '0x50fd51da', - to: ACCOUNT_MOCK, - value: '0x0', - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await result; - await controller.speedUpTransaction(transactionMeta.id); - - expect(controller.state.transactions).toHaveLength(2); - }); - it('publishes transaction events', async () => { const { controller, messenger } = setupController({ network: MOCK_LINEA_MAINNET_NETWORK, @@ -4603,81 +4496,6 @@ describe('TransactionController', () => { ); }); - it('generates initial history', async () => { - const { controller } = setupController(); - - const externalTransactionToConfirm = { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - id: '1', - chainId: toHex(1), - networkClientId: NETWORK_CLIENT_ID_MOCK, - status: TransactionStatus.confirmed as const, - time: 123456789, - txParams: { - gasUsed: undefined, - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - }, - }; - - const externalTransactionReceipt = { - gasUsed: '0x5208', - }; - - const externalBaseFeePerGas = '0x14'; - - await controller.confirmExternalTransaction( - externalTransactionToConfirm, - externalTransactionReceipt, - externalBaseFeePerGas, - ); - - const expectedInitialSnapshot = { - chainId: '0x1', - from: ACCOUNT_MOCK, - id: '1', - networkClientId: NETWORK_CLIENT_ID_MOCK, - time: 123456789, - status: TransactionStatus.confirmed as const, - to: ACCOUNT_2_MOCK, - txParams: { - from: ACCOUNT_MOCK, - to: ACCOUNT_2_MOCK, - gasUsed: undefined, - }, - }; - - // Expect initial snapshot to be the first history item - expect(controller.state.transactions[0]?.history?.[0]).toStrictEqual( - expectedInitialSnapshot, - ); - // Expect modification history to be present - expect(controller.state.transactions[0]?.history?.[1]).toStrictEqual([ - { - note: expect.any(String), - op: 'remove', - path: '/txParams/gasUsed', - timestamp: expect.any(Number), - }, - { - op: 'add', - path: '/txParams/value', - value: '0x0', - }, - { - op: 'add', - path: '/txReceipt', - value: expect.anything(), - }, - { - op: 'add', - path: '/baseFeePerGas', - value: expect.any(String), - }, - ]); - }); - it('marks local transactions with the same nonce and chainId as status dropped and defines replacedBy properties', async () => { const externalTransactionId = '1'; const externalTransactionHash = '0x1'; @@ -4709,7 +4527,6 @@ describe('TransactionController', () => { const { controller, messenger } = setupController({ options: { - disableHistory: true, state: { transactions: [ // Local unapproved transaction with the same chainId and nonce @@ -4949,194 +4766,6 @@ describe('TransactionController', () => { }); }); - describe('updateTransactionSendFlowHistory', () => { - it('appends sendFlowHistory entries to transaction meta', async () => { - const { controller } = setupController(); - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { networkClientId: NETWORK_CLIENT_ID_MOCK }, - ); - const addedTxId = controller.state.transactions[0].id; - controller.updateTransactionSendFlowHistory( - addedTxId, - 0, - mockSendFlowHistory, - ); - - expect(controller.state.transactions[0].sendFlowHistory).toStrictEqual( - mockSendFlowHistory, - ); - }); - - it('appends sendFlowHistory entries to existing entries in transaction meta', async () => { - const { controller } = setupController(); - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; - const mockExistingSendFlowHistory = [ - { - entry: 'sendFlow - user selected transfer to my accounts', - timestamp: 1650663928210, - }, - ]; - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - sendFlowHistory: mockExistingSendFlowHistory, - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - const addedTxId = controller.state.transactions[0].id; - controller.updateTransactionSendFlowHistory( - addedTxId, - 1, - mockSendFlowHistory, - ); - - expect(controller.state.transactions[0].sendFlowHistory).toStrictEqual([ - ...mockExistingSendFlowHistory, - ...mockSendFlowHistory, - ]); - }); - - it('doesnt append if current sendFlowHistory lengths doesnt match', async () => { - const { controller } = setupController(); - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - const addedTxId = controller.state.transactions[0].id; - controller.updateTransactionSendFlowHistory( - addedTxId, - 5, - mockSendFlowHistory, - ); - - expect(controller.state.transactions[0].sendFlowHistory).toStrictEqual( - [], - ); - }); - - it('throws if sendFlowHistory persistence is disabled', async () => { - const { controller } = setupController({ - options: { disableSendFlowHistory: true }, - }); - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - const addedTxId = controller.state.transactions[0].id; - expect(() => - controller.updateTransactionSendFlowHistory( - addedTxId, - 0, - mockSendFlowHistory, - ), - ).toThrow( - 'Send flow history is disabled for the current transaction controller', - ); - }); - - it('throws if transactionMeta is not found', async () => { - const { controller } = setupController(); - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; - expect(() => - controller.updateTransactionSendFlowHistory( - 'foo', - 0, - mockSendFlowHistory, - ), - ).toThrow( - 'Cannot update send flow history as no transaction metadata found', - ); - }); - - it('throws if the transaction is not unapproved status', async () => { - const { controller } = setupController({ - options: { - state: { - transactions: [ - { - id: 'foo', - chainId: toHex(5), - networkClientId: NETWORK_CLIENT_ID_MOCK, - hash: '1337', - status: TransactionStatus.submitted as const, - time: 123456789, - txParams: { - from: MOCK_PREFERENCES.state.selectedAddress, - }, - }, - ], - }, - }, - }); - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; - expect(() => - controller.updateTransactionSendFlowHistory( - 'foo', - 0, - mockSendFlowHistory, - ), - ) - .toThrow(`TransactionsController: Can only call updateTransactionSendFlowHistory on an unapproved transaction. - Current tx status: submitted`); - }); - }); - describe('clearUnapprovedTransactions', () => { it('clears unapproved transactions', async () => { const firstUnapprovedTxId = '1'; @@ -5219,23 +4848,6 @@ describe('TransactionController', () => { ]); }); - it('limits max transactions when adding to state', async () => { - const { controller } = setupController({ - options: { transactionHistoryLimit: 1 }, - }); - - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (incomingTransactionHelperMock.hub.on as any).mock.calls[0][1]([ - TRANSACTION_META_MOCK, - TRANSACTION_META_2_MOCK, - ]); - - expect(controller.state.transactions).toStrictEqual([ - { ...TRANSACTION_META_2_MOCK, networkClientId: NETWORK_CLIENT_ID_MOCK }, - ]); - }); - it('publishes TransactionController:incomingTransactionsReceived', async () => { const listener = jest.fn(); @@ -5357,10 +4969,6 @@ describe('TransactionController', () => { networkClientId: NETWORK_CLIENT_ID_MOCK, time: 123456789, status: TransactionStatus.unapproved as const, - history: [ - {} as TransactionMeta, - ...([{}] as TransactionHistoryEntry[]), - ], txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, @@ -5424,10 +5032,6 @@ describe('TransactionController', () => { networkClientId: NETWORK_CLIENT_ID_MOCK, time: 123456789, status: TransactionStatus.unapproved as const, - history: [ - {} as TransactionMeta, - ...([{}] as TransactionHistoryEntry[]), - ], txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, @@ -5776,7 +5380,6 @@ describe('TransactionController', () => { { id: transactionId, status: TransactionStatus.unapproved, - history: [{}], txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, @@ -5975,7 +5578,6 @@ describe('TransactionController', () => { it('sets status to dropped on transaction-dropped event', async () => { const { controller } = setupController({ options: { - disableHistory: true, state: { transactions: [{ ...TRANSACTION_META_MOCK }], }, @@ -5996,7 +5598,6 @@ describe('TransactionController', () => { const statusUpdatedEventListener = jest.fn(); const { controller, messenger } = setupController({ options: { - disableHistory: true, state: { transactions: [{ ...TRANSACTION_META_MOCK }], }, @@ -6042,7 +5643,6 @@ describe('TransactionController', () => { state: { transactions: [TRANSACTION_META_MOCK], }, - disableHistory: true, }, }); @@ -6509,14 +6109,6 @@ describe('TransactionController', () => { }); describe('updateSecurityAlertResponse', () => { - const mockSendFlowHistory = [ - { - entry: - 'sendFlow - user selected transfer to my accounts on recipient screen', - timestamp: 1650663928211, - }, - ]; - it('add securityAlertResponse to transaction meta', async () => { const transactionMeta = TRANSACTION_META_MOCK; const { controller } = setupController({ @@ -6603,10 +6195,7 @@ describe('TransactionController', () => { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, }, - history: mockSendFlowHistory, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, + } as unknown as TransactionMeta, ], }, }, @@ -6646,7 +6235,6 @@ describe('TransactionController', () => { }; transactionMeta = { ...baseTransaction, - history: [{ ...baseTransaction }], }; }); @@ -7445,7 +7033,6 @@ describe('TransactionController', () => { }; const transactionMeta: TransactionMeta = { ...baseTransaction, - history: [{ ...baseTransaction }], }; it('updates editable params and returns updated transaction metadata', async () => { @@ -7522,7 +7109,6 @@ describe('TransactionController', () => { expect.objectContaining({ transactionMeta: { ...updatedTransaction, - history: expect.any(Array), }, }), ); @@ -7860,7 +7446,6 @@ describe('TransactionController', () => { { id: transactionId, status: TransactionStatus.unapproved, - history: [{}], txParams: { from: ACCOUNT_MOCK, to: ACCOUNT_2_MOCK, diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index fdaa1f1a668..cf955edd722 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -19,7 +19,6 @@ import { query, ApprovalType, ORIGIN_METAMASK, - convertHexToDecimal, } from '@metamask/controller-utils'; import type { TraceCallback, TraceContext } from '@metamask/controller-utils'; import EthQuery from '@metamask/eth-query'; @@ -90,7 +89,6 @@ import type { Layer1GasFeeFlow, SavedGasFees, SecurityProviderRequest, - SendFlowHistoryEntry, TransactionParams, TransactionMeta, TransactionReceipt, @@ -151,10 +149,6 @@ import { } from './utils/gas-fee-tokens'; import { updateGasFees } from './utils/gas-fees'; import { getGasFeeFlow } from './utils/gas-flow'; -import { - addInitialHistorySnapshot, - updateTransactionHistory, -} from './utils/history'; import { getTransactionLayer1GasFee, updateTransactionLayer1GasFee, @@ -355,7 +349,7 @@ export type TransactionControllerGetTransactionsAction = { * Updates an existing transaction in state. * * @param transactionMeta - The new transaction to store in state. - * @param note - A note or update reason to include in the transaction history. + * @param note - A note or update reason to be logged. */ export type TransactionControllerUpdateTransactionAction = { type: `${typeof controllerName}:updateTransaction`; @@ -430,10 +424,10 @@ export type PendingTransactionOptions = { /** TransactionController constructor options. */ export type TransactionControllerOptions = { - /** Whether to disable storing history in transaction metadata. */ + /** @deprecated Whether to disable storing history in transaction metadata. */ disableHistory: boolean; - /** Explicitly disable transaction metadata history. */ + /** @deprecated Explicitly disable transaction metadata history. */ disableSendFlowHistory: boolean; /** Whether to disable additional processing on swaps transactions. */ @@ -523,7 +517,7 @@ export type TransactionControllerOptions = { testGasFeeFlows?: boolean; trace?: TraceCallback; - /** Transaction history limit. */ + /** @deprecated Transaction history limit. */ transactionHistoryLimit: number; /** The controller hooks. */ @@ -897,10 +891,6 @@ export class TransactionController extends BaseController< readonly #isFirstTimeInteractionEnabled: () => boolean; - readonly #isHistoryDisabled: boolean; - - readonly #isSendFlowHistoryDisabled: boolean; - readonly #isSimulationEnabled: () => boolean; readonly #isSwapsDisabled: boolean; @@ -940,7 +930,7 @@ export class TransactionController extends BaseController< readonly #trace: TraceCallback; - readonly #transactionHistoryLimit: number; + // readonly #transactionHistoryLimit: number; /** * Constructs a TransactionController. @@ -949,8 +939,6 @@ export class TransactionController extends BaseController< */ constructor(options: TransactionControllerOptions) { const { - disableHistory, - disableSendFlowHistory, disableSwaps, getCurrentAccountEIP1559Compatibility, getCurrentNetworkEIP1559Compatibility, @@ -975,7 +963,6 @@ export class TransactionController extends BaseController< state, testGasFeeFlows, trace, - transactionHistoryLimit = 40, } = options; super({ @@ -1034,8 +1021,6 @@ export class TransactionController extends BaseController< ((): Promise => Promise.resolve(false)); this.#isFirstTimeInteractionEnabled = isFirstTimeInteractionEnabled ?? ((): boolean => true); - this.#isHistoryDisabled = disableHistory ?? false; - this.#isSendFlowHistoryDisabled = disableSendFlowHistory ?? false; this.#isSimulationEnabled = isSimulationEnabled ?? ((): boolean => true); this.#isSwapsDisabled = disableSwaps ?? false; this.#isTimeoutEnabled = hooks?.isTimeoutEnabled ?? ((): boolean => true); @@ -1050,7 +1035,6 @@ export class TransactionController extends BaseController< this.#sign = sign; this.#testGasFeeFlows = testGasFeeFlows === true; this.#trace = trace ?? (((_request, fn) => fn?.()) as TraceCallback); - this.#transactionHistoryLimit = transactionHistoryLimit; const findNetworkClientIdByChainId = (chainId: Hex): string => { return this.messenger.call( @@ -1134,7 +1118,6 @@ export class TransactionController extends BaseController< isEnabled: this.#incomingTransactionOptions.isEnabled, messenger: this.messenger, remoteTransactionSource: new AccountsApiRemoteTransactionSource(), - trimTransactions: this.#trimTransactionsForState.bind(this), updateTransactions: this.#incomingTransactionOptions.updateTransactions, }); @@ -1282,7 +1265,6 @@ export class TransactionController extends BaseController< publishHook, requireApproval, securityAlertResponse, - sendFlowHistory, skipInitialGasEstimate, swaps = {}, traceContext, @@ -1418,7 +1400,6 @@ export class TransactionController extends BaseController< this.#updateTransactionInternal( { transactionId: newTransactionMeta.id, - skipHistory: true, skipResimulateCheck: true, skipValidation: true, }, @@ -1450,15 +1431,6 @@ export class TransactionController extends BaseController< securityProviderResponse; } - if (!this.#isSendFlowHistoryDisabled) { - // eslint-disable-next-line require-atomic-updates - addedTransactionMeta.sendFlowHistory = sendFlowHistory ?? []; - } - // Initial history push - if (!this.#isHistoryDisabled) { - addedTransactionMeta = addInitialHistorySnapshot(addedTransactionMeta); - } - addedTransactionMeta = updateSwapsTransaction( addedTransactionMeta, transactionType, @@ -1477,7 +1449,6 @@ export class TransactionController extends BaseController< this.#updateTransactionInternal( { transactionId: addedTransactionMeta.id, - skipHistory: true, skipResimulateCheck: true, skipValidation: true, }, @@ -1868,14 +1839,16 @@ export class TransactionController extends BaseController< * Updates an existing transaction in state. * * @param transactionMeta - The new transaction to store in state. - * @param note - A note or update reason to include in the transaction history. + * @param note - A note or update reason to be logged. */ updateTransaction(transactionMeta: TransactionMeta, note: string): void { const { id: transactionId } = transactionMeta; - this.#updateTransactionInternal({ transactionId, note }, () => ({ + this.#updateTransactionInternal({ transactionId }, () => ({ ...transactionMeta, })); + + log(`Transaction ${transactionId} updated. ${note}`); } /** @@ -1950,7 +1923,7 @@ export class TransactionController extends BaseController< ); this.update((state) => { - state.transactions = this.#trimTransactionsForState(newTransactions); + state.transactions = newTransactions; }); } @@ -2008,53 +1981,6 @@ export class TransactionController extends BaseController< } } - /** - * Append new send flow history to a transaction. - * - * @param transactionID - The ID of the transaction to update. - * @param currentSendFlowHistoryLength - The length of the current sendFlowHistory array. - * @param sendFlowHistoryToAdd - The sendFlowHistory entries to add. - * @returns The updated transactionMeta. - */ - updateTransactionSendFlowHistory( - transactionID: string, - currentSendFlowHistoryLength: number, - sendFlowHistoryToAdd: SendFlowHistoryEntry[], - ): TransactionMeta { - if (this.#isSendFlowHistoryDisabled) { - throw new Error( - 'Send flow history is disabled for the current transaction controller', - ); - } - - const transactionMeta = this.#getTransaction(transactionID); - - if (!transactionMeta) { - throw new Error( - `Cannot update send flow history as no transaction metadata found`, - ); - } - - validateIfTransactionUnapproved( - transactionMeta, - 'updateTransactionSendFlowHistory', - ); - - const sendFlowHistory = transactionMeta.sendFlowHistory ?? []; - if (currentSendFlowHistoryLength === sendFlowHistory.length) { - const updatedTransactionMeta = { - ...transactionMeta, - sendFlowHistory: [...sendFlowHistory, ...sendFlowHistoryToAdd], - }; - this.updateTransaction( - updatedTransactionMeta, - `${controllerName}:updateTransactionSendFlowHistory - sendFlowHistory updated`, - ); - } - - return this.#getTransaction(transactionID) as TransactionMeta; - } - /** * Update the gas values of a transaction. * @@ -2169,7 +2095,6 @@ export class TransactionController extends BaseController< this.#updateTransactionInternal( { transactionId, - note: `${controllerName}:updateTransactionGasFees - gas values updated`, skipResimulateCheck: true, }, (draftTxMeta) => { @@ -2372,8 +2297,6 @@ export class TransactionController extends BaseController< this.#updateTransactionInternal( { transactionId, - note: 'TransactionController#setTransactionActive - Transaction isActive updated', - skipHistory: true, skipValidation: true, skipResimulateCheck: true, }, @@ -2763,7 +2686,7 @@ export class TransactionController extends BaseController< ({ status }) => status !== TransactionStatus.unapproved, ); this.update((state) => { - state.transactions = this.#trimTransactionsForState(transactions); + state.transactions = transactions; }); } @@ -2820,7 +2743,6 @@ export class TransactionController extends BaseController< const updatedTransactionMeta = this.#updateTransactionInternal( { transactionId, - note: 'TransactionController#updateAtomicBatchData - Atomic batch data updated', }, (transactionMeta) => { const { nestedTransactions, txParams } = transactionMeta; @@ -2858,7 +2780,6 @@ export class TransactionController extends BaseController< this.#updateTransactionInternal( { transactionId, - note: 'TransactionController#updateAtomicBatchData - Gas estimate updated', }, (transactionMeta) => { transactionMeta.txParams.gas = draftTransaction.txParams.gas; @@ -2890,7 +2811,6 @@ export class TransactionController extends BaseController< this.#updateTransactionInternal( { transactionId, - note: 'TransactionController#updateBatchTransactions - Batch transactions updated', }, (transactionMeta) => { transactionMeta.batchTransactions = batchTransactions; @@ -3015,10 +2935,7 @@ export class TransactionController extends BaseController< #addMetadata(transactionMeta: TransactionMeta): void { validateTxParams(transactionMeta.txParams); this.update((state) => { - state.transactions = this.#trimTransactionsForState([ - ...state.transactions, - transactionMeta, - ]); + state.transactions = [...state.transactions, transactionMeta]; }); } @@ -3305,7 +3222,6 @@ export class TransactionController extends BaseController< transactionMeta = this.#updateTransactionInternal( { transactionId, - note: 'TransactionController#approveTransaction - Transaction approved', }, (draftTxMeta) => { const { chainId, txParams } = draftTxMeta; @@ -3389,7 +3305,6 @@ export class TransactionController extends BaseController< transactionMeta = this.#updateTransactionInternal( { transactionId, - note: 'TransactionController#approveTransaction - Transaction submitted', }, (draftTxMeta) => { draftTxMeta.hash = hash; @@ -3500,53 +3415,6 @@ export class TransactionController extends BaseController< this.#onTransactionStatusChange(updatedTransactionMeta); } - /** - * Trim the amount of transactions that are set on the state. Checks - * if the length of the tx history is longer then desired persistence - * limit and then if it is removes the oldest confirmed or rejected tx. - * Pending or unapproved transactions will not be removed by this - * operation. For safety of presenting a fully functional transaction UI - * representation, this function will not break apart transactions with the - * same nonce, created on the same day, per network. Not accounting for - * transactions of the same nonce, same day and network combo can result in - * confusing or broken experiences in the UI. - * - * @param transactions - The transactions to be applied to the state. - * @returns The trimmed list of transactions. - */ - #trimTransactionsForState( - transactions: TransactionMeta[], - ): TransactionMeta[] { - const nonceNetworkSet = new Set(); - - const txsToKeep = [...transactions] - .sort((a, b) => (a.time > b.time ? -1 : 1)) // Descending time order - .filter((tx) => { - const { chainId, status, txParams, time } = tx; - - if (txParams) { - const key = `${String(txParams.nonce)}-${convertHexToDecimal( - chainId, - )}-${new Date(time).toDateString()}`; - - if (nonceNetworkSet.has(key)) { - return true; - } else if ( - nonceNetworkSet.size < this.#transactionHistoryLimit || - !this.#isFinalState(status) - ) { - nonceNetworkSet.add(key); - return true; - } - } - - return false; - }); - - txsToKeep.reverse(); // Ascending time order - return txsToKeep; - } - /** * Determines if the transaction is in a final state. * @@ -3719,10 +3587,7 @@ export class TransactionController extends BaseController< this.update((state) => { const { transactions: currentTransactions } = state; - state.transactions = this.#trimTransactionsForState([ - ...finalTransactions, - ...currentTransactions, - ]); + state.transactions = [...finalTransactions, ...currentTransactions]; log( 'Added incoming transactions to state', @@ -3803,20 +3668,11 @@ export class TransactionController extends BaseController< pendingTxs, ); - // Make sure provided external transaction has non empty history array - const newTransactionMeta = - (transactionMeta.history ?? []).length === 0 && !this.#isHistoryDisabled - ? addInitialHistorySnapshot(transactionMeta) - : transactionMeta; - this.update((state) => { - state.transactions = this.#trimTransactionsForState([ - ...state.transactions, - newTransactionMeta, - ]); + state.transactions = [...state.transactions, transactionMeta]; }); - return newTransactionMeta; + return transactionMeta; } /** @@ -3966,7 +3822,7 @@ export class TransactionController extends BaseController< if (updateTransaction) { this.#updateTransactionInternal( - { transactionId, skipResimulateCheck: true, note: 'beforeSign Hook' }, + { transactionId, skipResimulateCheck: true }, updateTransaction, ); @@ -4334,14 +4190,10 @@ export class TransactionController extends BaseController< #updateTransactionInternal( { transactionId, - note, - skipHistory, skipValidation, skipResimulateCheck, }: { transactionId: string; - note?: string; - skipHistory?: boolean; skipValidation?: boolean; skipResimulateCheck?: boolean; }, @@ -4381,14 +4233,6 @@ export class TransactionController extends BaseController< ); } - const shouldSkipHistory = this.#isHistoryDisabled || skipHistory; - - if (!shouldSkipHistory) { - transactionMeta = updateTransactionHistory( - transactionMeta, - note ?? 'Transaction updated', - ); - } state.transactions[index] = transactionMeta; }); @@ -4495,7 +4339,6 @@ export class TransactionController extends BaseController< const updatedTransactionMeta = this.#updateTransactionInternal( { transactionId, - note: 'TransactionController#updateSimulationData - Update simulation data', skipResimulateCheck: Boolean(blockTime), }, (txMeta) => { @@ -4525,18 +4368,15 @@ export class TransactionController extends BaseController< gasFeeEstimatesLoaded?: boolean; layer1GasFee?: Hex; }): void { - this.#updateTransactionInternal( - { transactionId, skipHistory: true }, - (txMeta) => { - updateTransactionGasProperties({ - txMeta, - gasFeeEstimates, - gasFeeEstimatesLoaded, - isTxParamsGasFeeUpdatesEnabled: this.#isAutomaticGasFeeUpdateEnabled, - layer1GasFee, - }); - }, - ); + this.#updateTransactionInternal({ transactionId }, (txMeta) => { + updateTransactionGasProperties({ + txMeta, + gasFeeEstimates, + gasFeeEstimatesLoaded, + isTxParamsGasFeeUpdatesEnabled: this.#isAutomaticGasFeeUpdateEnabled, + layer1GasFee, + }); + }); } #onGasFeePollerTransactionBatchUpdate({ @@ -4707,7 +4547,7 @@ export class TransactionController extends BaseController< ({ id }) => id !== transactionId, ); - state.transactions = this.#trimTransactionsForState(transactions); + state.transactions = transactions; }); } @@ -4746,7 +4586,6 @@ export class TransactionController extends BaseController< newTransactionMeta = this.#updateTransactionInternal( { transactionId: transactionMeta.id, - note: 'TransactionController#failTransaction - Add error message and set status to failed', skipValidation: true, }, (draftTransactionMeta) => { @@ -4813,7 +4652,6 @@ export class TransactionController extends BaseController< { transactionId, skipResimulateCheck: true, - note: 'afterSimulate Hook', }, (txMeta) => { txMeta.txParamsOriginal = cloneDeep(txMeta.txParams); diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 334785a8e3c..0374c5075cc 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -407,7 +407,6 @@ describe('TransactionController Integration', () => { estimateType: 'dappSuggested', }, userFeeLevel: 'dappSuggested', - sendFlowHistory: [], }, { actionId: undefined, @@ -442,7 +441,6 @@ describe('TransactionController Integration', () => { estimateType: 'dappSuggested', }, userFeeLevel: 'dappSuggested', - sendFlowHistory: [], }, ], }, @@ -664,13 +662,13 @@ describe('TransactionController Integration', () => { 'confirmed', ); expect( - transactionController.state.transactions[0].networkClientId, + transactionController.state.transactions[1].networkClientId, ).toBe('sepolia'); expect(transactionController.state.transactions[1].status).toBe( 'confirmed', ); expect( - transactionController.state.transactions[1].networkClientId, + transactionController.state.transactions[0].networkClientId, ).toBe('linea-sepolia'); transactionController.destroy(); }); diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts index 9cd2f9ecf97..f7fc5cf1f29 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts @@ -44,7 +44,6 @@ const CONTROLLER_ARGS_MOCK: ConstructorParameters< getLocalTransactions: () => [], messenger: MESSENGER_MOCK, remoteTransactionSource: {} as RemoteTransactionSource, - trimTransactions: (transactions) => transactions, }; const TRANSACTION_MOCK: TransactionMeta = { @@ -320,7 +319,6 @@ describe('IncomingTransactionHelper', () => { it('does not if all unique transactions are truncated', async () => { const helper = new IncomingTransactionHelper({ ...CONTROLLER_ARGS_MOCK, - trimTransactions: (): TransactionMeta[] => [], remoteTransactionSource: createRemoteTransactionSourceMock([ TRANSACTION_MOCK, ]), diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index 180fc3dd034..76aac9d7c4a 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -53,10 +53,6 @@ export class IncomingTransactionHelper { #timeoutId?: unknown; - readonly #trimTransactions: ( - transactions: TransactionMeta[], - ) => TransactionMeta[]; - readonly #updateTransactions?: boolean; constructor({ @@ -67,7 +63,6 @@ export class IncomingTransactionHelper { isEnabled, messenger, remoteTransactionSource, - trimTransactions, updateTransactions, }: { client?: string; @@ -79,7 +74,6 @@ export class IncomingTransactionHelper { isEnabled?: () => boolean; messenger: TransactionControllerMessenger; remoteTransactionSource: RemoteTransactionSource; - trimTransactions: (transactions: TransactionMeta[]) => TransactionMeta[]; updateTransactions?: boolean; }) { this.hub = new EventEmitter(); @@ -93,7 +87,6 @@ export class IncomingTransactionHelper { this.#isUpdating = false; this.#messenger = messenger; this.#remoteTransactionSource = remoteTransactionSource; - this.#trimTransactions = trimTransactions; this.#updateTransactions = updateTransactions; } @@ -229,10 +222,7 @@ export class IncomingTransactionHelper { uniqueTransactions, ); - const trimmedTransactions = this.#trimTransactions([ - ...uniqueTransactions, - ...localTransactions, - ]); + const trimmedTransactions = [...uniqueTransactions, ...localTransactions]; const uniqueTransactionIds = uniqueTransactions.map((tx) => tx.id); diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index a9dee8d6212..eb635910979 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -107,10 +107,6 @@ export { UserFeeLevel, WalletDevice, } from './types'; -export { - DISPLAYED_TRANSACTION_HISTORY_PATHS, - MAX_TRANSACTION_HISTORY_LENGTH, -} from './utils/history'; export { determineTransactionType } from './utils/transaction-type'; export { mergeGasFeeEstimates } from './utils/gas-flow'; export { diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index a6e3cce50a8..52c24f34b12 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -252,7 +252,7 @@ export type TransactionMeta = { hash?: string; /** - * A history of mutations to TransactionMeta. + * @deprecated A history of mutations to TransactionMeta. */ history?: TransactionHistory; @@ -419,7 +419,7 @@ export type TransactionMeta = { selectedGasFeeToken?: Hex; /** - * An array of entries that describe the user's journey through the send flow. + * @deprecated An array of entries that describe the user's journey through the send flow. * This is purely attached to state logs for troubleshooting and support. */ sendFlowHistory?: SendFlowHistoryEntry[]; @@ -601,6 +601,7 @@ export type TransactionBatchMeta = { transactions?: NestedTransactionMetadata[]; }; +/** @deprecated An entry in the send flow history. */ export type SendFlowHistoryEntry = { /** * String to indicate user interaction information. @@ -1168,7 +1169,7 @@ type ExtendedHistoryOperation = JsonCompatibleOperation & { }; /** - * A transaction history entry that includes the ExtendedHistoryOperation as the first element. + * @deprecated A transaction history entry that includes the ExtendedHistoryOperation as the first element. */ export type TransactionHistoryEntry = [ ExtendedHistoryOperation, @@ -1176,7 +1177,7 @@ export type TransactionHistoryEntry = [ ]; /** - * A transaction history that includes the transaction meta as the first element. + * @deprecated A transaction history that includes the transaction meta as the first element. * And the rest of the elements are the operation arrays that were applied to the transaction meta. */ export type TransactionHistory = [ diff --git a/packages/transaction-controller/src/utils/history.test.ts b/packages/transaction-controller/src/utils/history.test.ts deleted file mode 100644 index d4828639b67..00000000000 --- a/packages/transaction-controller/src/utils/history.test.ts +++ /dev/null @@ -1,401 +0,0 @@ -import { toHex } from '@metamask/controller-utils'; -import { add0x, Hex } from '@metamask/utils'; -import { cloneDeep } from 'lodash'; - -import { - MAX_TRANSACTION_HISTORY_LENGTH, - updateTransactionHistory, -} from './history'; -import { TransactionStatus } from '../types'; -import type { - TransactionHistory, - TransactionMeta, - TransactionHistoryEntry, -} from '../types'; - -describe('History', () => { - describe('updateTransactionHistory', () => { - it('does nothing if the history property is missing', () => { - const mockTransaction = createMinimalMockTransaction(); - expect(mockTransaction.history).toBeUndefined(); - const originalInputTransaction = cloneDeep(mockTransaction); - - const updatedTransaction = updateTransactionHistory( - mockTransaction, - 'test update', - ); - - expect(updatedTransaction).toBe(mockTransaction); - expect(updatedTransaction).toStrictEqual(originalInputTransaction); - }); - - it('does nothing if there have been no changes', () => { - const originalTransaction = createMinimalMockTransaction(); - const mockTransaction = createMockTransaction({ originalTransaction }); - const mockTransactionClone = cloneDeep(mockTransaction); - - const updatedTransaction = updateTransactionHistory( - mockTransaction, - 'test update', - ); - - expect(updatedTransaction).toBe(mockTransaction); - expect(updatedTransaction).toStrictEqual(mockTransactionClone); - }); - - it('adds a new history entry', () => { - const originalTransaction = createMinimalMockTransaction(); - const mockTransaction = createMockTransaction({ - originalTransaction, - partialTransaction: { - history: [originalTransaction], - txParams: { from: generateAddress(123) }, - }, - }); - - const updatedTransaction = updateTransactionHistory( - mockTransaction, - 'test update', - ); - - expect(updatedTransaction).not.toBe(mockTransaction); - expect(updatedTransaction).toStrictEqual({ - ...mockTransaction, - history: [ - originalTransaction, - [ - { - note: 'test update', - op: 'replace', - path: '/txParams/from', - timestamp: expect.any(Number), - value: generateAddress(123), - }, - ], - ], - }); - }); - - describe('when history is past max size with non-displayed entries', () => { - it('merges a non-displayed entry when adding a new entry after max history size is reached', () => { - const originalTransaction = createMinimalMockTransaction(); - const mockTransaction = createMockTransaction({ - partialTransaction: { - history: generateMockHistory({ - originalTransaction, - length: MAX_TRANSACTION_HISTORY_LENGTH, - }), - // This is the changed value - txParams: { from: generateAddress(123) }, - }, - }); - // Validate that last history entry is correct - expect( - mockTransaction.history?.[mockTransaction.history.length - 1], - ).toStrictEqual([ - { - note: 'Mock non-displayed change', - op: 'replace', - path: '/txParams/from', - value: generateAddress(MAX_TRANSACTION_HISTORY_LENGTH - 1), - }, - ]); - expect(mockTransaction.history).toHaveLength( - MAX_TRANSACTION_HISTORY_LENGTH, - ); - - const updatedTransaction = updateTransactionHistory( - mockTransaction, - 'test update', - ); - - expect(updatedTransaction).not.toBe(mockTransaction); - expect(updatedTransaction).toStrictEqual({ - ...mockTransaction, - history: [ - originalTransaction, - // This is the merged entry of mockTransaction.history[1] and mockTransaction.history[2] - [ - { - note: 'Mock non-displayed change, Mock non-displayed change', - op: 'replace', - path: '/txParams/from', - timestamp: expect.any(Number), - value: generateAddress(2), - }, - ], - ...mockTransaction.history.slice(3), - // This is the new entry: - [ - { - note: 'test update', - op: 'replace', - path: '/txParams/from', - timestamp: expect.any(Number), - value: generateAddress(123), - }, - ], - ], - }); - expect(updatedTransaction.history).toHaveLength( - MAX_TRANSACTION_HISTORY_LENGTH, - ); - }); - }); - - describe('when history is past max size with a single non-displayed entry at the end', () => { - it('merges a non-displayed entry when adding a new entry after max history size is reached', () => { - const originalTransaction = createMinimalMockTransaction(); - // This matches the last gas price change in the mock history - const mockTransactionGasPrice = toHex( - MAX_TRANSACTION_HISTORY_LENGTH - 1, - ); - const mockTransaction = createMockTransaction({ - partialTransaction: { - history: generateMockHistory({ - numberOfDisplayEntries: MAX_TRANSACTION_HISTORY_LENGTH - 1, - originalTransaction, - length: MAX_TRANSACTION_HISTORY_LENGTH, - }), - txParams: { - // This is the changed value - from: generateAddress(123), - // This matches the last gas price change in the mock history - gasPrice: mockTransactionGasPrice, - }, - }, - }); - // Validate that last history entry is correct - expect( - mockTransaction.history?.[mockTransaction.history.length - 1], - ).toStrictEqual([ - { - note: 'Mock displayed change', - op: 'replace', - path: '/txParams/gasPrice', - value: mockTransactionGasPrice, - }, - ]); - const mockTransactionClone = cloneDeep(mockTransaction); - expect(mockTransaction.history).toHaveLength( - MAX_TRANSACTION_HISTORY_LENGTH, - ); - - const updatedTransaction = updateTransactionHistory( - mockTransaction, - 'test update', - ); - - expect(updatedTransaction).not.toBe(mockTransaction); - expect(updatedTransaction).toStrictEqual({ - ...mockTransaction, - history: [ - ...mockTransactionClone.history.slice( - 0, - MAX_TRANSACTION_HISTORY_LENGTH - 1, - ), - // This is the new merged entry: - [ - { - note: 'Mock displayed change, test update', - op: 'replace', - path: '/txParams/gasPrice', - timestamp: expect.any(Number), - value: mockTransactionGasPrice, - }, - { - op: 'replace', - path: '/txParams/from', - value: generateAddress(123), - }, - ], - ], - }); - expect(updatedTransaction.history).toHaveLength( - MAX_TRANSACTION_HISTORY_LENGTH, - ); - }); - }); - - describe('when history is past max size with only displayed entries', () => { - it('adds a new history entry, exceeding max size', () => { - const originalTransaction = createMinimalMockTransaction(); - const mockTransaction = createMockTransaction({ - partialTransaction: { - history: generateMockHistory({ - numberOfDisplayEntries: MAX_TRANSACTION_HISTORY_LENGTH - 1, - originalTransaction, - length: MAX_TRANSACTION_HISTORY_LENGTH, - }), - txParams: { - from: originalTransaction.txParams.from, - // This is the changed value - gasPrice: toHex(1337), - }, - }, - }); - // Validate that last history entry is correct - expect( - mockTransaction.history?.[mockTransaction.history.length - 1], - ).toStrictEqual([ - { - note: 'Mock displayed change', - op: 'replace', - path: '/txParams/gasPrice', - value: toHex(MAX_TRANSACTION_HISTORY_LENGTH - 1), - }, - ]); - expect(mockTransaction.history).toHaveLength( - MAX_TRANSACTION_HISTORY_LENGTH, - ); - - const updatedTransaction = updateTransactionHistory( - mockTransaction, - 'test update', - ); - - expect(updatedTransaction).not.toBe(mockTransaction); - expect(updatedTransaction).toStrictEqual({ - ...mockTransaction, - history: [ - ...mockTransaction.history, - // This is the new entry: - [ - { - note: 'test update', - op: 'replace', - path: '/txParams/gasPrice', - timestamp: expect.any(Number), - value: toHex(1337), - }, - ], - ], - }); - expect(updatedTransaction.history).toHaveLength( - MAX_TRANSACTION_HISTORY_LENGTH + 1, - ); - }); - }); - }); -}); - -/** - * Create a minimal mock transaction. It has just enough to satify the type. - * - * @returns A minimal transaction. - */ -function createMinimalMockTransaction(): TransactionMeta { - return { - chainId: toHex(1337), - id: 'mock-id', - networkClientId: 'testNetworkClientId', - time: 0, - status: TransactionStatus.submitted as const, - txParams: { - from: '', - }, - }; -} - -/** - * Create a mock transaction. - * - * Optionally an incomplete transaction can be passed in, and any missing required proeprties will - * be filled out. The 'status' property is not allowed as input only because it was complicated to - * get the types to be correct if it was provided. - * - * A `history` property is included only if an original transaction is passed in. - * - * @param options - Options. - * @param options.partialTransaction - A partial transaction, without a 'status' property. - * @param options.originalTransaction - The original transaction object to include in the transaction - * history. - * @returns A mock transaction. - */ -function createMockTransaction({ - partialTransaction, - originalTransaction, -}: { - partialTransaction?: Omit, 'status'>; - originalTransaction?: TransactionMeta; -} = {}): TransactionMeta & Required> { - const minimalTransaction = createMinimalMockTransaction(); - - if (originalTransaction) { - if (originalTransaction.history) { - throw new Error( - 'The original transaction should be an initial snapshot of the transaction, with no history property', - ); - } - minimalTransaction.history = [originalTransaction]; - } else { - minimalTransaction.history = [{ ...minimalTransaction }]; - } - - return { - // Cast used here because TypeScript wasn't able to infer that `history` was guaranteed to be set - ...(minimalTransaction as TransactionMeta & - Required>), - ...partialTransaction, - }; -} - -/** - * Generate a mock transaction history. - * - * @param args - Arguments. - * @param args.numberOfDisplayEntries - The number of displayed history entries to generate. - * @param args.originalTransaction - The original transaction, before any history changes. - * @param args.length - The total length of history to generate. - * @returns Mock transaction history. - */ -function generateMockHistory({ - numberOfDisplayEntries = 0, - originalTransaction, - length, -}: { - numberOfDisplayEntries?: number; - originalTransaction: TransactionMeta; - length: number; -}): TransactionHistory { - if (length < 1) { - throw new Error('Invalid length'); - } else if (numberOfDisplayEntries >= length) { - throw new Error('Length must exceed number of displayed entries'); - } - - const historyEntries: TransactionHistoryEntry[] = [ - ...Array(length - 1).keys(), - ].map((index: number) => { - // Use index of this entry in history array, for better readability/debugging of mock values - const historyIndex = index + 1; - - return [ - numberOfDisplayEntries < historyIndex - ? { - note: 'Mock non-displayed change', - op: 'replace', - path: '/txParams/from', - value: generateAddress(historyIndex), - } - : { - note: 'Mock displayed change', - op: index === 0 ? 'add' : 'replace', - path: '/txParams/gasPrice', - value: toHex(historyIndex), - }, - ]; - }); - - return [originalTransaction, ...historyEntries]; -} - -/** - * Generate a mock lowercase Ethereum address. - * - * @param number - The address as a decimal number. - * @returns The mock address - */ -function generateAddress(number: number): Hex { - return add0x(number.toString(16).padStart(40, '0')); -} diff --git a/packages/transaction-controller/src/utils/history.ts b/packages/transaction-controller/src/utils/history.ts deleted file mode 100644 index 84238cef6e5..00000000000 --- a/packages/transaction-controller/src/utils/history.ts +++ /dev/null @@ -1,216 +0,0 @@ -import jsonDiffer from 'fast-json-patch'; -import { cloneDeep, merge } from 'lodash'; - -import type { - TransactionHistory, - TransactionHistoryEntry, - TransactionMeta, -} from '../types'; - -/** - * The maximum allowed length of the `transaction.history` property. - */ -export const MAX_TRANSACTION_HISTORY_LENGTH = 100; - -/** - * A list of trarnsaction history paths that may be used for display. These entries will not be - * compressed. - */ -export const DISPLAYED_TRANSACTION_HISTORY_PATHS = [ - '/status', - '/txParams/gasPrice', - '/txParams/gas', - '/estimatedBaseFee', - '/blockTimestamp', -]; - -/** - * Build a new version of the provided transaction with an initial history - * entry, which is just a snapshot of the transaction. - * - * @param transactionMeta - TransactionMeta to add initial history snapshot to. - * @returns A copy of `transactionMeta` with a new `history` property. - */ -export function addInitialHistorySnapshot( - transactionMeta: TransactionMeta, -): TransactionMeta { - const snapshot = snapshotFromTransactionMeta(transactionMeta); - return merge({}, transactionMeta, { history: [snapshot] }); -} - -/** - * Builds a new version of the transaction with a new history entry if - * it has a `history` property, or just returns the transaction. - * - * @param transactionMeta - TransactionMeta to add history entry to. - * @param note - Note to add to history entry. - * @returns A copy of `transactionMeta` with a new `history` entry if it has an - * existing non-empty `history` array. - */ -export function updateTransactionHistory( - transactionMeta: TransactionMeta, - note: string, -): TransactionMeta { - if (!transactionMeta.history) { - return transactionMeta; - } - - const currentState = snapshotFromTransactionMeta(transactionMeta); - const previousState = replayHistory(transactionMeta.history); - const newHistoryEntry = generateHistoryEntry( - previousState, - currentState, - note, - ); - - if (newHistoryEntry.length === 0) { - return transactionMeta; - } - - // Casts required here because this list has two separate types of entries: - // TransactionMeta and TransactionHistoryEntry. The only TransactionMeta is the first - // entry, but TypeScript loses that type information when `slice` is called for some reason. - let updatedHistory = [ - ...transactionMeta.history, - newHistoryEntry, - ] as TransactionHistory; - - if (updatedHistory.length > MAX_TRANSACTION_HISTORY_LENGTH) { - updatedHistory = compressTransactionHistory(updatedHistory); - } - - return merge({}, transactionMeta, { - history: updatedHistory, - }); -} - -/** - * Compress the transaction history, if it is possible to do so without compressing entries used - * for display. History entries are merged together to make room for a single new entry. - * - * @param transactionHistory - The transaction history to compress. - * @returns A compressed transaction history. - */ -function compressTransactionHistory( - transactionHistory: TransactionHistory, -): TransactionHistory { - const initialEntry = transactionHistory[0]; - // Casts required here because this list has two separate types of entries: - // TransactionMeta and TransactionHistoryEntry. The only TransactionMeta is the first - // entry, but TypeScript loses that type information when `slice` is called for some reason. - const historyEntries = transactionHistory.slice( - 1, - ) as TransactionHistoryEntry[]; - - const firstNonDisplayedEntryIndex = historyEntries.findIndex( - (historyEntry) => { - return !historyEntry.some(({ path }) => - DISPLAYED_TRANSACTION_HISTORY_PATHS.includes(path), - ); - }, - ); - - // If no non-displayed entry is found, let history exceed max size. - // TODO: Move data used for display to another property, so that we can more reliably limit - // history size or remove it altogether. - if (firstNonDisplayedEntryIndex === -1) { - return transactionHistory; - } - - // If a non-displayed entry is found that we can remove, merge it with another entry. - // The entry we're merging with might be a "displayed" entry, but that's OK, merging more changes - // in does not break our display logic. - const mergeTargetEntryIndex = - // Merge with previous entry if there is no next entry. - // We default to merging with next because the next entry might also be non-displayed, so it - // might be removed in a future trim, saving more space. - firstNonDisplayedEntryIndex === historyEntries.length - 1 - ? firstNonDisplayedEntryIndex - 1 - : firstNonDisplayedEntryIndex + 1; - const firstIndexToMerge = Math.min( - firstNonDisplayedEntryIndex, - mergeTargetEntryIndex, - ); - const firstEntryToMerge = historyEntries[firstIndexToMerge]; - const secondEntryToMerge = historyEntries[firstIndexToMerge + 1]; - - const beforeMergeState = replayHistory([ - initialEntry, - ...historyEntries.slice(0, firstIndexToMerge), - ]); - const afterMergeState = replayHistory([ - beforeMergeState, - firstEntryToMerge, - secondEntryToMerge, - ]); - const mergedHistoryEntry = generateHistoryEntry( - beforeMergeState, - afterMergeState, - `${String(firstEntryToMerge[0].note)}, ${String( - secondEntryToMerge[0].note, - )}`, - ); - - historyEntries.splice(firstIndexToMerge, 2, mergedHistoryEntry); - return [initialEntry, ...historyEntries]; -} - -/** - * Generates a history entry from the previous and new transaction metadata. - * - * @param previousState - The previous transaction metadata. - * @param currentState - The new transaction metadata. - * @param note - A note for the transaction metada update. - * @returns An array of history operation. - */ -function generateHistoryEntry( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - previousState: any, - currentState: TransactionMeta, - note: string, -): TransactionHistoryEntry { - const historyOperationsEntry = jsonDiffer.compare( - previousState, - currentState, - ) as TransactionHistoryEntry; - // Add a note to the first operation, since it breaks if we append it to the entry - if (historyOperationsEntry[0]) { - if (note) { - historyOperationsEntry[0].note = note; - } - historyOperationsEntry[0].timestamp = Date.now(); - } - return historyOperationsEntry; -} - -/** - * Recovers previous transactionMeta from passed history array. - * - * @param transactionHistory - The transaction metadata to replay. - * @returns The transaction metadata. - */ -function replayHistory( - transactionHistory: TransactionHistory, -): TransactionMeta { - const shortHistory = cloneDeep(transactionHistory); - return shortHistory.reduce( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (val, entry: any) => jsonDiffer.applyPatch(val, entry).newDocument, - ) as TransactionMeta; -} - -/** - * Clone the transaction meta data without the history property. - * - * @param transactionMeta - The transaction metadata to snapshot. - * @returns A deep clone of transaction metadata without history property. - */ -function snapshotFromTransactionMeta( - transactionMeta: TransactionMeta, -): TransactionMeta { - const snapshot = { ...transactionMeta }; - delete snapshot.history; - return cloneDeep(snapshot); -}