applied suggestions

packages/transaction-controller/src/TransactionController.test.ts

@@ -725,6 +725,7 @@ describe('TransactionController', () => {
       isEIP7702GasFeeTokensEnabled: isEIP7702GasFeeTokensEnabledMock,
       publicKeyEIP7702: '0x1234',
       sign: signMock,
+      transactionHistoryLimit: 40,
       ...givenOptions,
     };
 
@@ -1201,10 +1202,10 @@ describe('TransactionController', () => {
       expect(
         transactions.map((transaction) => [transaction.id, transaction.status]),
       ).toStrictEqual([
-        ['123', TransactionStatus.failed],
-        ['111', TransactionStatus.failed],
-        ['222', TransactionStatus.confirmed],
         ['333', TransactionStatus.failed],
+        ['222', TransactionStatus.confirmed],
+        ['111', TransactionStatus.failed],
+        ['123', TransactionStatus.failed],
       ]);
     });
 
@@ -4848,6 +4849,23 @@ 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();
 

packages/transaction-controller/src/TransactionController.ts

@@ -19,6 +19,7 @@ import {
   query,
   ApprovalType,
   ORIGIN_METAMASK,
+  convertHexToDecimal,
 } from '@metamask/controller-utils';
 import type { TraceCallback, TraceContext } from '@metamask/controller-utils';
 import EthQuery from '@metamask/eth-query';
@@ -120,6 +121,7 @@ import type {
   PublishHookResult,
   GetGasFeeTokensRequest,
   InternalAccount,
+  SendFlowHistoryEntry,
 } from './types';
 import {
   GasFeeEstimateLevel,
@@ -424,10 +426,10 @@ export type PendingTransactionOptions = {
 
 /** TransactionController constructor options. */
 export type TransactionControllerOptions = {
-  /** @deprecated Whether to disable storing history in transaction metadata. */
+  /** @deprecated No longer used — kept only for backward compatibility. */
   disableHistory: boolean;
 
-  /** @deprecated Explicitly disable transaction metadata history. */
+  /** @deprecated No longer used — kept only for backward compatibility. */
   disableSendFlowHistory: boolean;
 
   /** Whether to disable additional processing on swaps transactions. */
@@ -517,7 +519,7 @@ export type TransactionControllerOptions = {
   testGasFeeFlows?: boolean;
   trace?: TraceCallback;
 
-  /** @deprecated Transaction history limit. */
+  /** Transaction history limit. */
   transactionHistoryLimit: number;
 
   /** The controller hooks. */
@@ -930,7 +932,7 @@ export class TransactionController extends BaseController<
 
   readonly #trace: TraceCallback;
 
-  // readonly #transactionHistoryLimit: number;
+  readonly #transactionHistoryLimit: number;
 
   /**
    * Constructs a TransactionController.
@@ -963,6 +965,7 @@ export class TransactionController extends BaseController<
       state,
       testGasFeeFlows,
       trace,
+      transactionHistoryLimit = 40,
     } = options;
 
     super({
@@ -1035,6 +1038,7 @@ 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(
@@ -1118,6 +1122,7 @@ export class TransactionController extends BaseController<
       isEnabled: this.#incomingTransactionOptions.isEnabled,
       messenger: this.messenger,
       remoteTransactionSource: new AccountsApiRemoteTransactionSource(),
+      trimTransactions: this.#trimTransactionsForState.bind(this),
       updateTransactions: this.#incomingTransactionOptions.updateTransactions,
     });
 
@@ -1923,10 +1928,26 @@ export class TransactionController extends BaseController<
     );
 
     this.update((state) => {
-      state.transactions = newTransactions;
+      state.transactions = this.#trimTransactionsForState(newTransactions);
     });
   }
 
+  /**
+   * @deprecated No longer used. Kept only to avoid breaking changes. It now performs no operations.
+   * @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 transactionMeta.
+   */
+  updateTransactionSendFlowHistory(
+    transactionID: string,
+    _currentSendFlowHistoryLength: number,
+    _sendFlowHistoryToAdd: SendFlowHistoryEntry[],
+  ): TransactionMeta {
+    // Return the transaction unchanged
+    return this.#getTransaction(transactionID) as TransactionMeta;
+  }
+
   /**
    * Adds external provided transaction to state as confirmed transaction.
    *
@@ -2686,7 +2707,7 @@ export class TransactionController extends BaseController<
       ({ status }) => status !== TransactionStatus.unapproved,
     );
     this.update((state) => {
-      state.transactions = transactions;
+      state.transactions = this.#trimTransactionsForState(transactions);
     });
   }
 
@@ -2935,7 +2956,10 @@ export class TransactionController extends BaseController<
   #addMetadata(transactionMeta: TransactionMeta): void {
     validateTxParams(transactionMeta.txParams);
     this.update((state) => {
-      state.transactions = [...state.transactions, transactionMeta];
+      state.transactions = this.#trimTransactionsForState([
+        ...state.transactions,
+        transactionMeta,
+      ]);
     });
   }
 
@@ -3587,7 +3611,10 @@ export class TransactionController extends BaseController<
     this.update((state) => {
       const { transactions: currentTransactions } = state;
 
-      state.transactions = [...finalTransactions, ...currentTransactions];
+      state.transactions = this.#trimTransactionsForState([
+        ...finalTransactions,
+        ...currentTransactions,
+      ]);
 
       log(
         'Added incoming transactions to state',
@@ -3746,6 +3773,53 @@ 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;
+  }
+
   /**
    * Get transaction with provided actionId.
    *
@@ -4547,7 +4621,7 @@ export class TransactionController extends BaseController<
         ({ id }) => id !== transactionId,
       );
 
-      state.transactions = transactions;
+      state.transactions = this.#trimTransactionsForState(transactions);
     });
   }
 

packages/transaction-controller/src/TransactionControllerIntegration.test.ts

@@ -662,13 +662,13 @@ describe('TransactionController Integration', () => {
           'confirmed',
         );
         expect(
-          transactionController.state.transactions[1].networkClientId,
+          transactionController.state.transactions[0].networkClientId,
         ).toBe('sepolia');
         expect(transactionController.state.transactions[1].status).toBe(
           'confirmed',
         );
         expect(
-          transactionController.state.transactions[0].networkClientId,
+          transactionController.state.transactions[1].networkClientId,
         ).toBe('linea-sepolia');
         transactionController.destroy();
       });

packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts

@@ -44,6 +44,7 @@ const CONTROLLER_ARGS_MOCK: ConstructorParameters<
   getLocalTransactions: () => [],
   messenger: MESSENGER_MOCK,
   remoteTransactionSource: {} as RemoteTransactionSource,
+  trimTransactions: (transactions) => transactions,
 };
 
 const TRANSACTION_MOCK: TransactionMeta = {
@@ -319,6 +320,7 @@ 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,
           ]),

packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts

@@ -53,6 +53,10 @@ export class IncomingTransactionHelper {
 
   #timeoutId?: unknown;
 
+  readonly #trimTransactions: (
+    transactions: TransactionMeta[],
+  ) => TransactionMeta[];
+
   readonly #updateTransactions?: boolean;
 
   constructor({
@@ -63,6 +67,7 @@ export class IncomingTransactionHelper {
     isEnabled,
     messenger,
     remoteTransactionSource,
+    trimTransactions,
     updateTransactions,
   }: {
     client?: string;
@@ -74,6 +79,7 @@ export class IncomingTransactionHelper {
     isEnabled?: () => boolean;
     messenger: TransactionControllerMessenger;
     remoteTransactionSource: RemoteTransactionSource;
+    trimTransactions: (transactions: TransactionMeta[]) => TransactionMeta[];
     updateTransactions?: boolean;
   }) {
     this.hub = new EventEmitter();
@@ -87,6 +93,7 @@ export class IncomingTransactionHelper {
     this.#isUpdating = false;
     this.#messenger = messenger;
     this.#remoteTransactionSource = remoteTransactionSource;
+    this.#trimTransactions = trimTransactions;
     this.#updateTransactions = updateTransactions;
   }
 
@@ -222,7 +229,10 @@ export class IncomingTransactionHelper {
       uniqueTransactions,
     );
 
-    const trimmedTransactions = [...uniqueTransactions, ...localTransactions];
+    const trimmedTransactions = this.#trimTransactions([
+      ...uniqueTransactions,
+      ...localTransactions,
+    ]);
 
     const uniqueTransactionIds = uniqueTransactions.map((tx) => tx.id);