Refactor/rebuild tx expiry for scan

packages/snap/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snap-stellar-wallet.git"
   },
   "source": {
-    "shasum": "0bXKXi1JKlz9Tl+t7sHnw2HInhPpNtkrTIrUdRD/n4I=",
+    "shasum": "z4mud5yiJBM62ns46mKRfzbW44lxw+ZOwH1nMTCq4CM=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.test.ts

@@ -321,6 +321,68 @@ describe('RefreshConfirmationContextHandler', () => {
     );
   });
 
+  it('runs the transaction refresher first and feeds its patch to the remaining refreshers', async () => {
+    jest
+      .mocked(getInterfaceContextIfExists)
+      .mockResolvedValueOnce(baseContext)
+      .mockResolvedValueOnce(baseContext);
+
+    const transactionRefresher = createMockRefresher(
+      ConfirmationContextRefresherKey.Transaction,
+      {
+        refresh: jest.fn().mockResolvedValue({
+          result: { securityScanRequest: { transaction: 'FRESH_XDR' } },
+          reschedule: false,
+        }),
+      },
+    );
+    const scanRefresher = createMockRefresher(
+      ConfirmationContextRefresherKey.Scan,
+      {
+        refresh: jest.fn().mockResolvedValue({
+          result: { scanFetchStatus: FetchStatus.Fetched },
+          reschedule: true,
+        }),
+      },
+    );
+
+    // Register scan first to prove ordering is driven by key, not array order.
+    const { handler, updateConfirmation } = setup([
+      scanRefresher,
+      transactionRefresher,
+    ]);
+
+    await handler.handle({
+      jsonrpc: '2.0',
+      id: '1',
+      method: BackgroundEventMethod.RefreshConfirmationContext,
+      params: {
+        ...confirmationContextRequestParams,
+        refresherKeys: [
+          ConfirmationContextRefresherKey.Scan,
+          ConfirmationContextRefresherKey.Transaction,
+        ],
+      },
+    });
+
+    // The transaction refresher sees the original context...
+    expect(transactionRefresher.refresh).toHaveBeenCalledWith(baseContext);
+    // ...and the scan refresher sees it already patched with the rebuilt envelope.
+    expect(scanRefresher.refresh).toHaveBeenCalledWith(
+      expect.objectContaining({
+        securityScanRequest: { transaction: 'FRESH_XDR' },
+      }),
+    );
+    expect(updateConfirmation).toHaveBeenCalledWith(
+      expect.objectContaining({
+        updatedContext: expect.objectContaining({
+          securityScanRequest: { transaction: 'FRESH_XDR' },
+          scanFetchStatus: FetchStatus.Fetched,
+        }),
+      }),
+    );
+  });
+
   it('does not run a refresher when its key is omitted from refresherKeys', async () => {
     jest.mocked(getInterfaceContextIfExists).mockResolvedValue(baseContext);
 

packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.ts

@@ -1,9 +1,9 @@
 import type { Json } from '@metamask/utils';
 
+import { ConfirmationContextRefresherKey } from './api';
 import type {
   ConfirmationContextRefreshResult,
   ConfirmationContextRefreshers,
-  ConfirmationContextRefresherKey,
   ConfirmationDataContext,
   IConfirmationContextRefresher,
 } from './api';
@@ -170,8 +170,17 @@ export class RefreshConfirmationContextHandler extends CronjobBaseHandler<Refres
   }
 
   /**
-   * Runs enabled refreshers in parallel. Uses `allSettled` so one rejection
-   * does not prevent other refreshers from completing.
+   * Runs enabled refreshers for one cycle.
+   *
+   * The transaction refresher runs first and in isolation: it rebuilds the
+   * pending transaction with a fresh time bound and writes it into the
+   * security-scan request. Its patch is merged into the context the remaining
+   * refreshers see, so the scan refresher simulates/validates the renewed
+   * envelope rather than a (possibly expired) snapshot. The remaining refreshers
+   * then run in parallel.
+   *
+   * Each refresher is isolated so one rejection does not prevent the others from
+   * completing.
    *
    * @param ctx - Confirmation interface context passed to each refresher.
    * @param activeRefreshers - Refreshers selected by `refresherKeys`.
@@ -181,22 +190,58 @@ export class RefreshConfirmationContextHandler extends CronjobBaseHandler<Refres
     ctx: ConfirmationDataContext,
     activeRefreshers: readonly IConfirmationContextRefresher[],
   ): Promise<ConfirmationContextRefreshResult[]> {
-    const settled = await Promise.allSettled(
-      activeRefreshers.map(async (refresher) =>
-        this.#runRefresher(refresher, ctx),
-      ),
+    const transactionRefresher = activeRefreshers.find(
+      (refresher) =>
+        refresher.key === ConfirmationContextRefresherKey.Transaction,
     );
+    const remainingRefreshers = activeRefreshers.filter(
+      (refresher) => refresher !== transactionRefresher,
+    );
+
+    const results: ConfirmationContextRefreshResult[] = [];
+    let workingContext = ctx;
 
-    return settled.map((outcome, index) => {
-      if (outcome.status === 'fulfilled') {
-        return outcome.value;
+    if (transactionRefresher) {
+      const transactionResult = await this.#settleRefresher(
+        transactionRefresher,
+        workingContext,
+      );
+      results.push(transactionResult);
+      if (transactionResult?.result) {
+        workingContext = { ...workingContext, ...transactionResult.result };
       }
+    }
+
+    const remainingResults = await Promise.all(
+      remainingRefreshers.map(async (refresher) =>
+        this.#settleRefresher(refresher, workingContext),
+      ),
+    );
+
+    return [...results, ...remainingResults];
+  }
+
+  /**
+   * Runs a single refresher and converts an unexpected rejection into `null`,
+   * so a failing refresher never blocks the others.
+   *
+   * @param refresher - The refresher to run.
+   * @param ctx - The context passed to the refresher.
+   * @returns The refresher result, or `null` when it rejected.
+   */
+  async #settleRefresher(
+    refresher: IConfirmationContextRefresher,
+    ctx: ConfirmationDataContext,
+  ): Promise<ConfirmationContextRefreshResult> {
+    try {
+      return await this.#runRefresher(refresher, ctx);
+    } catch (error) {
       this.logger.error(
-        `Refresher "${activeRefreshers[index]?.key}" rejected unexpectedly`,
-        outcome.reason,
+        `Refresher "${refresher.key}" rejected unexpectedly`,
+        error,
       );
       return null;
-    });
+    }
   }
 
   async #runRefresher(

packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.test.ts

@@ -6,6 +6,7 @@ import { ConfirmationTransactionRefresher } from './transactionRefresher';
 import { KnownCaip2ChainId } from '../../../api';
 import type { AssetMetadataService } from '../../../services/asset-metadata';
 import type { TransactionService } from '../../../services/transaction';
+import type { MockClassicOperation } from '../../../services/transaction/__mocks__/transaction.fixtures';
 import { buildMockClassicTransaction } from '../../../services/transaction/__mocks__/transaction.fixtures';
 import { FetchStatus } from '../../../ui/confirmation/api';
 import { getSlip44AssetId } from '../../../utils';
@@ -21,17 +22,36 @@ describe('ConfirmationTransactionRefresher', () => {
   const accountId = '11111111-1111-4111-8111-111111111111';
   const toAddress = 'GDPMFLKUGASUTWBN2XGYYKD27QGHCYH4BUFUTER4L23INYQ4JHDWFOIE';
 
-  const transaction = buildMockClassicTransaction(
-    [
-      {
-        type: 'payment',
-        params: { destination: toAddress, asset: 'native', amount: '1' },
-      },
-    ],
-    { networkPassphrase: Networks.TESTNET },
-  );
+  const paymentOperations: MockClassicOperation[] = [
+    {
+      type: 'payment',
+      params: { destination: toAddress, asset: 'native', amount: '1' },
+    },
+  ];
+
+  const transaction = buildMockClassicTransaction(paymentOperations, {
+    networkPassphrase: Networks.TESTNET,
+  });
   const transactionXdr = transaction.getRaw().toXDR();
 
+  // A scan envelope far from expiry (kept as-is) versus one inside the refresh
+  // buffer (swapped for the rebuilt transaction).
+  const freshScanTransactionXdr = buildMockClassicTransaction(
+    paymentOperations,
+    {
+      networkPassphrase: Networks.TESTNET,
+      timeout: 600,
+    },
+  )
+    .getRaw()
+    .toXDR();
+  const staleScanTransactionXdr = buildMockClassicTransaction(
+    paymentOperations,
+    { networkPassphrase: Networks.TESTNET, timeout: 30 },
+  )
+    .getRaw()
+    .toXDR();
+
   const sendRequest = {
     jsonrpc: '2.0' as const,
     id: 1,
@@ -192,6 +212,97 @@ describe('ConfirmationTransactionRefresher', () => {
     });
   });
 
+  it('renews the security-scan transaction when the scanned envelope nears expiry', async () => {
+    const { refresher } = setup();
+    const securityScanRequest = {
+      accountAddress: toAddress,
+      origin: 'https://dapp.example',
+      scope,
+      transaction: staleScanTransactionXdr,
+    };
+
+    const result = await refresher.refresh(
+      createTransactionContext({ securityScanRequest }),
+    );
+
+    expect(result).toStrictEqual({
+      result: {
+        securityScanRequest: {
+          ...securityScanRequest,
+          transaction: transactionXdr,
+        },
+      },
+      reschedule: false,
+    });
+  });
+
+  it('renews the security-scan transaction for change-trust flows', async () => {
+    const { refresher } = setup();
+    const securityScanRequest = {
+      accountAddress: toAddress,
+      origin: 'https://dapp.example',
+      scope,
+      transaction: staleScanTransactionXdr,
+    };
+
+    const result = await refresher.refresh(
+      createTransactionContext({
+        request: changeTrustAddRequest,
+        securityScanRequest,
+      }),
+    );
+
+    expect(result).toStrictEqual({
+      result: {
+        securityScanRequest: {
+          ...securityScanRequest,
+          transaction: transactionXdr,
+        },
+      },
+      reschedule: false,
+    });
+  });
+
+  it('keeps the scanned envelope while it is still far from expiry', async () => {
+    const { refresher } = setup();
+    const securityScanRequest = {
+      accountAddress: toAddress,
+      origin: 'https://dapp.example',
+      scope,
+      transaction: freshScanTransactionXdr,
+    };
+
+    const result = await refresher.refresh(
+      createTransactionContext({ securityScanRequest }),
+    );
+
+    expect(result).toBeNull();
+  });
+
+  it('renews the security-scan transaction when the scanned envelope is unparseable', async () => {
+    const { refresher } = setup();
+    const securityScanRequest = {
+      accountAddress: toAddress,
+      origin: 'https://dapp.example',
+      scope,
+      transaction: 'OUTDATED_XDR',
+    };
+
+    const result = await refresher.refresh(
+      createTransactionContext({ securityScanRequest }),
+    );
+
+    expect(result).toStrictEqual({
+      result: {
+        securityScanRequest: {
+          ...securityScanRequest,
+          transaction: transactionXdr,
+        },
+      },
+      reschedule: false,
+    });
+  });
+
   it('marks the transaction invalid when the original envelope has expired', async () => {
     const { refresher, transactionService } = setup();
     // The stored XDR being signed is expired, even though the rebuilt draft would be valid.