feat: Add pending transaction reconcile max attempt and max age in TransactionService

packages/snap/.env.example

@@ -74,4 +74,10 @@ SECURITY_ALERTS_API_BASE_URL=https://security-alerts.api.cx.metamask.io
 #MAX_FEE_THRESHOLD_IN_XLM=
 
 # Maximum background reschedules for the track-transaction cron job while Horizon has not
-#TRACK_TRANSACTION_MAX_RESCHEDULES=10
+#TRACK_TRANSACTION_MAX_RESCHEDULES=10
+
+# Maximum number of reconcile attempts for a pending transaction
+#MAX_RECONCILE_ATTEMPTS=5
+
+# Maximum age of a pending transaction in milliseconds
+#MAX_PENDING_TRANSACTION_AGE=30000

packages/snap/snap.config.ts

@@ -44,6 +44,8 @@ const config: SnapConfig = {
     BASE_FEE_MULTIPLIER: process.env.BASE_FEE_MULTIPLIER ?? '',
     SIMULATION_FEE_MULTIPLIER: process.env.SIMULATION_FEE_MULTIPLIER ?? '',
     MAX_FEE_THRESHOLD_IN_XLM: process.env.MAX_FEE_THRESHOLD_IN_XLM ?? '',
+    MAX_RECONCILE_ATTEMPTS: process.env.MAX_RECONCILE_ATTEMPTS ?? '',
+    MAX_PENDING_TRANSACTION_AGE: process.env.MAX_PENDING_TRANSACTION_AGE ?? '',
   },
   polyfills: true,
 };

packages/snap/src/config.ts

@@ -110,6 +110,20 @@ const ConfigStruct = object({
      * The maximum fee threshold in XLM for the Stellar network.
      */
     maxFeeThresholdInXLM: parseFloatStruct(1, 1),
+    /**
+     * The maximum number of Horizon not-found reconcile attempts for a pending transaction.
+     * Used with `maxPendingTransactionAge` to evict stale pending txs from snap state;
+     * both limits must be exceeded before a pending tx is dropped.
+     * Minimum value is 2 to avoid dropping the pending transaction too early.
+     */
+    maxReconcileAttempts: parseIntegerStruct(2, 5),
+    /**
+     * The maximum age of a pending transaction in milliseconds.
+     * Used with `maxReconcileAttempts` to evict stale pending txs from snap state;
+     * both limits must be exceeded before a pending tx is dropped.
+     * Minimum value is 15000 to avoid dropping the pending transaction too early.
+     */
+    maxPendingTransactionAge: parseIntegerStruct(15000, 30000),
   }),
   api: object({
     tokenApi: object({
@@ -185,6 +199,8 @@ export const AppConfig = create(
       maxFeeThresholdInXLM: process.env.MAX_FEE_THRESHOLD_IN_XLM,
       trackTransactionMaxReschedules:
         process.env.TRACK_TRANSACTION_MAX_RESCHEDULES,
+      maxReconcileAttempts: process.env.MAX_RECONCILE_ATTEMPTS,
+      maxPendingTransactionAge: process.env.MAX_PENDING_TRANSACTION_AGE,
     },
     api: {
       tokenApi: {

packages/snap/src/services/network/exceptions.ts

@@ -73,8 +73,11 @@ export class TransactionRetryableException extends TransactionSendException {}
 
 /** Thrown when a transaction is not found. */
 export class TransactionNotFoundException extends NetworkServiceException {
+  readonly transactionHash: string;
+
   constructor(transactionHash: string, options?: StellarSnapExceptionOptions) {
     super(`Transaction ${transactionHash} not found`, options);
+    this.transactionHash = transactionHash;
   }
 }
 

packages/snap/src/services/transaction/TransactionRepository.test.ts

@@ -0,0 +1,203 @@
+import { TransactionStatus } from '@metamask/keyring-api';
+
+import { KnownCaip2ChainId } from '../../api';
+import { AppConfig } from '../../config';
+import { generateMockTransactions } from './__mocks__/transaction.fixtures';
+import type { StellarKeyringTransaction } from './api';
+import type { TransactionStateValue } from './TransactionRepository';
+import { TransactionRepository } from './TransactionRepository';
+import { getSnapProvider } from '../../utils/snap';
+import { State } from '../state/State';
+
+jest.mock('../../utils/snap');
+
+describe('TransactionRepository', () => {
+  const scope = KnownCaip2ChainId.Mainnet;
+  const accountId = 'account-1';
+
+  const defaultState: TransactionStateValue = {
+    transactions: {},
+    lastScanTokens: {},
+  };
+
+  const recentTimestampSeconds = () => Math.floor(Date.now() / 1000);
+
+  const expiredTimestampSeconds = () =>
+    Math.floor(
+      (Date.now() - AppConfig.transaction.maxPendingTransactionAge - 1000) /
+        1000,
+    );
+
+  let mockState: TransactionStateValue;
+
+  const createRepository = () =>
+    new TransactionRepository(
+      new State({
+        encrypted: false,
+        defaultState,
+      }),
+    );
+
+  beforeEach(() => {
+    mockState = structuredClone(defaultState);
+    const snapProvider = getSnapProvider() as { request: jest.Mock };
+    snapProvider.request.mockImplementation(async ({ method, params }) => {
+      if (method === 'snap_getState') {
+        if (params.key) {
+          return mockState[params.key as keyof TransactionStateValue];
+        }
+        return mockState;
+      }
+
+      if (method === 'snap_manageState' && params.operation === 'update') {
+        mockState = params.newState as TransactionStateValue;
+      }
+
+      return null;
+    });
+  });
+
+  afterEach(() => {
+    (getSnapProvider() as { request: jest.Mock }).request.mockReset();
+  });
+
+  it('removes confirmed incoming transactions from snap state', async () => {
+    const repository = createRepository();
+    const pendingTransaction = generateMockTransactions(1, {
+      id: 'tx-hash-1',
+      account: accountId,
+      scope,
+      status: TransactionStatus.Unconfirmed,
+    })[0] as StellarKeyringTransaction;
+
+    await repository.saveMany([pendingTransaction]);
+
+    const confirmedTransaction = generateMockTransactions(1, {
+      id: 'tx-hash-1',
+      account: accountId,
+      scope,
+      status: TransactionStatus.Confirmed,
+    })[0] as StellarKeyringTransaction;
+
+    await repository.saveMany([confirmedTransaction]);
+
+    const stored = await repository.findStellarTransactionsByAccountIds([
+      accountId,
+    ]);
+    expect(stored).toStrictEqual([]);
+  });
+
+  it('keeps incoming pending over existing when reconcileAttemptCount is higher', async () => {
+    const repository = createRepository();
+    const pendingTransaction = generateMockTransactions(1, {
+      id: 'tx-hash-1',
+      account: accountId,
+      scope,
+      status: TransactionStatus.Unconfirmed,
+      timestamp: 100,
+    })[0] as StellarKeyringTransaction;
+
+    await repository.saveMany([
+      {
+        ...pendingTransaction,
+        reconcileAttemptCount: 1,
+      } as StellarKeyringTransaction,
+    ]);
+
+    await repository.saveMany([
+      {
+        ...pendingTransaction,
+        reconcileAttemptCount: 2,
+      } as StellarKeyringTransaction,
+    ]);
+
+    const stored = await repository.findStellarTransactionsByAccountIds([
+      accountId,
+    ]);
+    expect(stored).toStrictEqual([
+      expect.objectContaining({
+        id: 'tx-hash-1',
+        reconcileAttemptCount: 2,
+      }),
+    ]);
+  });
+
+  it('drops pending transactions when reconcile attempts and max age are both exceeded', async () => {
+    const repository = createRepository();
+    const pendingTransaction = generateMockTransactions(1, {
+      id: 'tx-hash-1',
+      account: accountId,
+      scope,
+      status: TransactionStatus.Unconfirmed,
+      timestamp: expiredTimestampSeconds(),
+    })[0] as StellarKeyringTransaction;
+
+    await repository.saveMany([
+      {
+        ...pendingTransaction,
+        reconcileAttemptCount: 5,
+      } as StellarKeyringTransaction,
+    ]);
+
+    const stored = await repository.findStellarTransactionsByAccountIds([
+      accountId,
+    ]);
+    expect(stored).toStrictEqual([]);
+  });
+
+  it('keeps pending transactions when reconcile attempts exceeded but max age is not', async () => {
+    const repository = createRepository();
+    const pendingTransaction = generateMockTransactions(1, {
+      id: 'tx-hash-1',
+      account: accountId,
+      scope,
+      status: TransactionStatus.Unconfirmed,
+      timestamp: recentTimestampSeconds(),
+    })[0] as StellarKeyringTransaction;
+
+    await repository.saveMany([
+      {
+        ...pendingTransaction,
+        reconcileAttemptCount: 5,
+      } as StellarKeyringTransaction,
+    ]);
+
+    const stored = await repository.findStellarTransactionsByAccountIds([
+      accountId,
+    ]);
+    expect(stored).toStrictEqual([
+      expect.objectContaining({
+        id: 'tx-hash-1',
+        reconcileAttemptCount: 5,
+      }),
+    ]);
+  });
+
+  it('keeps pending transactions when max age exceeded but reconcile attempts are not', async () => {
+    const repository = createRepository();
+    const pendingTransaction = generateMockTransactions(1, {
+      id: 'tx-hash-1',
+      account: accountId,
+      scope,
+      status: TransactionStatus.Unconfirmed,
+      timestamp: expiredTimestampSeconds(),
+    })[0] as StellarKeyringTransaction;
+
+    await repository.saveMany([
+      {
+        ...pendingTransaction,
+        reconcileAttemptCount: 1,
+      } as StellarKeyringTransaction,
+    ]);
+
+    const stored = await repository.findStellarTransactionsByAccountIds([
+      accountId,
+    ]);
+    expect(stored).toStrictEqual([
+      expect.objectContaining({
+        id: 'tx-hash-1',
+        reconcileAttemptCount: 1,
+      }),
+    ]);
+  });
+});

packages/snap/src/services/transaction/TransactionRepository.ts

@@ -3,12 +3,17 @@ import { groupBy } from 'lodash';
 import sortBy from 'lodash/sortBy';
 import uniqBy from 'lodash/uniqBy';
 
-import { isPendingTransactionStatus } from './utils';
+import type { StellarKeyringTransaction } from './api';
+import {
+  isPendingTransactionStatus,
+  shouldDropPendingTransaction,
+  toKeyringTransactions,
+} from './utils';
 import type { KnownCaip2ChainId } from '../../api';
 import type { State } from '../state/State';
 
 export type TransactionStateValue = {
-  transactions: Record<string, KeyringTransaction[]>;
+  transactions: Record<string, StellarKeyringTransaction[]>;
   lastScanTokens: Record<string, Record<KnownCaip2ChainId, string | null>>;
 };
 
@@ -28,18 +33,29 @@ export class TransactionRepository {
       TransactionStateValue['transactions']
     >(this.#stateKey);
 
-    return Object.values(transactionsByAccount ?? {}).flat();
+    return toKeyringTransactions(
+      Object.values(transactionsByAccount ?? {}).flat(),
+    );
   }
 
   async findByAccountIds(
     accountIds: string[],
     scope?: KnownCaip2ChainId,
   ): Promise<KeyringTransaction[]> {
+    return toKeyringTransactions(
+      await this.findStellarTransactionsByAccountIds(accountIds, scope),
+    );
+  }
+
+  async findStellarTransactionsByAccountIds(
+    accountIds: string[],
+    scope?: KnownCaip2ChainId,
+  ): Promise<StellarKeyringTransaction[]> {
     const transactionsByAccount = await this.#state.getKey<
       TransactionStateValue['transactions']
     >(this.#stateKey);
 
-    const transactions: KeyringTransaction[] = [];
+    const transactions: StellarKeyringTransaction[] = [];
     for (const accountId of accountIds) {
       const accountTransactions = transactionsByAccount?.[accountId] ?? [];
       transactions.push(
@@ -58,7 +74,7 @@ export class TransactionRepository {
       TransactionStateValue['transactions']
     >(this.#stateKey);
 
-    return transactionsByAccount?.[accountId] ?? [];
+    return toKeyringTransactions(transactionsByAccount?.[accountId] ?? []);
   }
 
   async findLastScanTokenByAccountIds(
@@ -78,7 +94,7 @@ export class TransactionRepository {
     return lastScanTokenByAccountId;
   }
 
-  async save(transaction: KeyringTransaction): Promise<void> {
+  async save(transaction: StellarKeyringTransaction): Promise<void> {
     await this.saveMany([transaction]);
   }
 
@@ -87,13 +103,14 @@ export class TransactionRepository {
    *
    * Submitted transactions are upserted locally. Confirmed and failed transactions
    * remove any matching id from snap state — durable history lives in the controller
-   * after AccountTransactionsUpdated is emitted.
+   * after AccountTransactionsUpdated is emitted. Pending transactions that exceed both
+   * `maxReconcileAttempts` and `maxPendingTransactionAge` are also evicted.
    *
    * @param transactions - Transactions to reconcile into snap state.
    * @param lastScanTokens - Optional scan cursors to persist per account and scope.
    */
   async saveMany(
-    transactions: KeyringTransaction[],
+    transactions: StellarKeyringTransaction[],
     lastScanTokens?: Record<string, Record<KnownCaip2ChainId, string | null>>,
   ): Promise<void> {
     if (transactions.length === 0 && !lastScanTokens) {
@@ -142,16 +159,31 @@ export class TransactionRepository {
   }
 
   #applyTransactionUpdate(
-    existingTransactions: KeyringTransaction[],
-    incomingTransactions: KeyringTransaction[],
-  ): KeyringTransaction[] {
+    existingTransactions: StellarKeyringTransaction[],
+    incomingTransactions: StellarKeyringTransaction[],
+  ): StellarKeyringTransaction[] {
     const merged = [...incomingTransactions, ...existingTransactions];
-    // Merge the transactions based on transaction id and keep the latest one
+    // Merge the transactions based on transaction id and choose the incoming one if there are duplicates
     // Sort it by timestamp in descending order
-    // Filter out the completed transactions
+    // Filter out completed transactions and pending txs eligible for eviction
     return sortBy(
       uniqBy(merged, 'id'),
       (item) => -(item.timestamp ?? 0),
-    ).filter((transaction) => isPendingTransactionStatus(transaction.status));
+    ).filter((transaction) => this.#canSave(transaction));
+  }
+
+  /**
+   * Checks if the transaction can be saved to snap state.
+   *
+   * Keeps pending transactions unless both reconcile attempts and max age are exceeded.
+   *
+   * @param transaction - The transaction to check.
+   * @returns Whether the transaction can be saved to snap state.
+   */
+  #canSave(transaction: StellarKeyringTransaction): boolean {
+    return (
+      isPendingTransactionStatus(transaction.status) &&
+      !shouldDropPendingTransaction(transaction)
+    );
   }
 }