Don't attach volume telemetry to pre-submission failures

extension/src/popup/components/InternalTransaction/SubmitTransaction/hooks/__tests__/useSubmitTxData.telemetry.test.tsx

@@ -15,6 +15,7 @@ import {
   NetworkDetails,
 } from "@shared/constants/stellar";
 import { CUSTOM_NETWORK } from "@shared/helpers/stellar";
+import { ActionStatus } from "@shared/api/types";
 import * as ApiInternal from "@shared/api/internal";
 import { makeDummyStore } from "popup/__testHelpers__";
 import { initialState as txSubmissionInitialState } from "popup/ducks/transactionSubmission";
@@ -147,21 +148,25 @@ const makeState = ({
 const renderSubmitHook = (
   state: ReturnType<typeof makeState>,
   networkDetails: NetworkDetails = MAINNET_NETWORK_DETAILS,
+  { isHardwareWallet = false }: { isHardwareWallet?: boolean } = {},
 ) => {
   const store = makeDummyStore(state);
   const wrapper = ({ children }: { children: React.ReactNode }) => (
     <Provider store={store}>{children}</Provider>
   );
-  return renderHook(
+  const rendered = renderHook(
     () =>
       useSubmitTxData({
-        isHardwareWallet: false,
+        isHardwareWallet,
         networkDetails,
         publicKey: PUBLIC_KEY,
         xdr: buildSwapXdr(),
       }),
     { wrapper },
   );
+  // The store is a real one over rootReducer, so submitStatus is observable —
+  // it is what TransactionConfirm switches on to render SubmitFail.
+  return { ...rendered, store };
 };
 
 const mockSubmitOk = (resultXdr: string) => {
@@ -539,4 +544,179 @@ describe("useSubmitTxData terminal-event telemetry", () => {
 
     expect(emitMetric).not.toHaveBeenCalled();
   });
+  describe("pre-submission (signing) failure", () => {
+    const mockSigningFailure = () =>
+      jest
+        .spyOn(ApiInternal, "signFreighterTransaction")
+        .mockRejectedValue(new Error("Incorrect password"));
+
+    it("emits payment.failed with its failure properties and no volume data", async () => {
+      mockSigningFailure();
+      jest
+        .spyOn(ApiInternal, "getTokenPrices")
+        .mockResolvedValue({ native: { currentPrice: "0.5" } });
+
+      const { result } = renderSubmitHook(makeState({ asset: "native" }));
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      const props = emitted(METRIC_NAMES.paymentFailed);
+      expect(props).toEqual({
+        payment_type: "payment",
+        asset_code: "XLM",
+        reason_code: "unknown",
+      });
+      // The transaction never left the device, so it has no attempted volume.
+      expect(props).not.toHaveProperty("amount");
+      expect(props).not.toHaveProperty("amount_usd");
+      expect(props).not.toHaveProperty("amount_usd_status");
+      // ...and it is emphatically not a transport failure, which per the
+      // catalog reads as "unresolved — may have settled".
+      expect(props).not.toHaveProperty("failure_category");
+    });
+
+    it("emits swap.failed with both asset codes and no volume data", async () => {
+      mockSigningFailure();
+      jest.spyOn(ApiInternal, "getTokenPrices").mockResolvedValue({
+        native: { currentPrice: "0.5" },
+        [USDC_CANONICAL]: { currentPrice: "1.0" },
+      });
+
+      const { result } = renderSubmitHook(
+        makeState({
+          asset: "native",
+          destinationAsset: USDC_CANONICAL,
+          destinationAmount: "90",
+        }),
+      );
+      await act(async () => {
+        await result.current.fetchData({ isSwap: true });
+      });
+
+      expect(emitted(METRIC_NAMES.swapFailed)).toEqual({
+        from_asset_code: "XLM",
+        to_asset_code: "USDC",
+        reason_code: "unknown",
+      });
+    });
+
+    it("does not submit a classic payment whose signature failed", async () => {
+      mockSigningFailure();
+      const fetchSpy = jest.fn();
+      global.fetch = fetchSpy as unknown as typeof fetch;
+
+      const { result } = renderSubmitHook(
+        // preparedTransaction: null is the classic-payment shape, where a
+        // failed signature used to leave signedXDR as "" and submit it.
+        makeState({ asset: "native", preparedTransaction: null }),
+      );
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      expect(fetchSpy).not.toHaveBeenCalled();
+    });
+
+    it("does not submit the UNSIGNED prepared XDR when a token transfer's signature failed", async () => {
+      mockSigningFailure();
+      const fetchSpy = jest.fn();
+      global.fetch = fetchSpy as unknown as typeof fetch;
+
+      const { result } = renderSubmitHook(
+        // A Soroban/token transfer carries a prepared XDR, so `signedXDR` is
+        // truthy even when signing failed — the guard has to key off whether
+        // signing actually succeeded, not off the XDR being empty.
+        makeState({ asset: "native", preparedTransaction: buildSwapXdr() }),
+      );
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      expect(fetchSpy).not.toHaveBeenCalled();
+    });
+
+    it("puts the flow into ActionStatus.ERROR so SubmitFail renders (signature threw)", async () => {
+      mockSigningFailure();
+
+      const { result, store } = renderSubmitHook(
+        makeState({ asset: "native" }),
+      );
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      // TransactionConfirm switches on submitStatus to decide between
+      // SendingTransaction and SubmitFail.
+      expect(store.getState().transactionSubmission.submitStatus).toBe(
+        ActionStatus.ERROR,
+      );
+    });
+
+    it("reaches ERROR even when signing resolves fulfilled with an empty payload", async () => {
+      // No rejected action is dispatched on this path, so the reducer never
+      // sets the status — without setSubmitError the view would sit on
+      // PENDING and strand the user on the sending spinner.
+      jest
+        .spyOn(ApiInternal, "signFreighterTransaction")
+        .mockResolvedValue({ signedTransaction: "" });
+      const fetchSpy = jest.fn();
+      global.fetch = fetchSpy as unknown as typeof fetch;
+
+      const { result, store } = renderSubmitHook(
+        makeState({ asset: "native" }),
+      );
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      expect(store.getState().transactionSubmission.submitStatus).toBe(
+        ActionStatus.ERROR,
+      );
+      expect(fetchSpy).not.toHaveBeenCalled();
+      expect(emitted(METRIC_NAMES.paymentFailed)).toEqual({
+        payment_type: "payment",
+        asset_code: "XLM",
+        reason_code: "unknown",
+      });
+    });
+
+    it("reaches ERROR for a hardware flow that arrives with no signed XDR", async () => {
+      // Hardware skips the signing dispatch entirely (HardwareSign stores the
+      // signed XDR in preparedTransaction), so nothing sets the status here
+      // either.
+      const fetchSpy = jest.fn();
+      global.fetch = fetchSpy as unknown as typeof fetch;
+
+      const { result, store } = renderSubmitHook(
+        makeState({ asset: "native", preparedTransaction: null }),
+        MAINNET_NETWORK_DETAILS,
+        { isHardwareWallet: true },
+      );
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      expect(store.getState().transactionSubmission.submitStatus).toBe(
+        ActionStatus.ERROR,
+      );
+      expect(fetchSpy).not.toHaveBeenCalled();
+    });
+
+    it("issues no confirmation price fetch when signing fails", async () => {
+      mockSigningFailure();
+      const pricesSpy = jest
+        .spyOn(ApiInternal, "getTokenPrices")
+        .mockResolvedValue({ native: { currentPrice: "0.5" } });
+
+      const { result } = renderSubmitHook(makeState({ asset: "native" }));
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      // The snapshot starts only once signing has succeeded, so a signing
+      // failure never issues a price request it would just have to abort.
+      expect(pricesSpy).not.toHaveBeenCalled();
+    });
+  });
 });

extension/src/popup/components/InternalTransaction/SubmitTransaction/hooks/useSubmitTxData.tsx

@@ -7,6 +7,7 @@ import { initialState, reducer, isError } from "helpers/request";
 import { AppDispatch } from "popup/App";
 import {
   addRecentAddress,
+  setSubmitError,
   signFreighterTransaction,
   submitFreighterTransaction,
   transactionSubmissionSelector,
@@ -22,7 +23,7 @@ import {
   isMainnet,
 } from "helpers/stellar";
 import { getSdk, isCustomNetwork } from "@shared/helpers/stellar";
-import { AssetIcons } from "@shared/api/types";
+import { AssetIcons, ErrorMessage } from "@shared/api/types";
 import { allAccountsSelector } from "popup/ducks/accountServices";
 import { balancesSelector, tokenPricesSelector } from "popup/ducks/cache";
 import { tokenPricesV2Selector } from "popup/ducks/remoteConfig";
@@ -52,6 +53,14 @@ interface SubmitTxData {
   error?: string;
 }
 
+/**
+ * `reason_code` for a terminal event that never reached the network, so there
+ * is no Horizon result code to report. Deliberately the same bounded literal
+ * the post-submission path falls back to rather than the raw signing-error
+ * text, which would give `reason_code` unbounded cardinality.
+ */
+const PRE_SUBMISSION_REASON_CODE = "unknown";
+
 /**
  * The `amount_usd`-family properties are named identically on all four
  * terminal events so `SUM(amount_usd)` works across event names.
@@ -134,12 +143,11 @@ function useSubmitTxData({
         status: "success",
       } as SubmitTxData;
 
-      // Everything the volume telemetry needs is snapshotted here, at
-      // confirmation, before signing/submission — amounts and prices are
-      // frozen together and carried to whichever terminal event fires.
-      // Skipped entirely for collectible sends (unpriced, out of scope) and
-      // for custom networks (not real economic activity, shouldn't pollute
-      // volume metrics).
+      // Asset identities for the volume telemetry. Classified up front (the
+      // price snapshot they feed starts after signing, below). Skipped
+      // entirely for collectible sends (unpriced, out of scope) and for custom
+      // networks (not real economic activity, shouldn't pollute volume
+      // metrics).
       const isCustom = isCustomNetwork(networkDetails);
       const accountBalances =
         allBalancesCache[networkDetails.network]?.[publicKey]?.balances ?? null;
@@ -165,28 +173,6 @@ function useSubmitTxData({
           )
         : null;
 
-      const cachedDisplayPrices =
-        allTokenPricesCache[networkDetails.networkPassphrase]?.[publicKey] ??
-        null;
-      snapshotHandle = sourceIdentity
-        ? startConfirmationPriceSnapshot({
-            canonicalIds: [
-              getCanonicalFromAsset(sourceIdentity.code, sourceIdentity.issuer),
-              ...(destIdentity
-                ? [
-                    getCanonicalFromAsset(
-                      destIdentity.code,
-                      destIdentity.issuer,
-                    ),
-                  ]
-                : []),
-            ],
-            networkDetails,
-            useV2: useTokenPricesV2,
-            cachedDisplayPrices,
-          })
-        : null;
-
       // Not a non-null assertion and not a guard: `preparedTransaction` is
       // legitimately null for a classic payment (simulateTx's "classic" arm
       // returns a fee and no payload at all — the built XDR arrives via the
@@ -196,6 +182,11 @@ function useSubmitTxData({
       // replaces it, so `?? ""` — the same fallback Send/index.tsx uses on
       // this field — is the honest starting value.
       let signedXDR = transactionSimulation.preparedTransaction ?? "";
+      // Tracked explicitly rather than inferred from `signedXDR` being empty:
+      // on the Soroban/token path a failed signature leaves `signedXDR`
+      // holding the *unsigned* prepared XDR, which is truthy.
+      let isSigned = isHardwareWallet && !!signedXDR;
+      let signingError: ErrorMessage | undefined;
       if (!isHardwareWallet) {
         const res = await reduxDispatch(
           signFreighterTransaction({
@@ -208,9 +199,91 @@ function useSubmitTxData({
           res.payload.signedTransaction
         ) {
           signedXDR = res.payload.signedTransaction;
+          isSigned = true;
+        } else {
+          signingError = signFreighterTransaction.rejected.match(res)
+            ? res.payload
+            : undefined;
         }
       }
 
+      if (!isSigned) {
+        // Pre-submission failure: signing rejected, or a hardware flow arrived
+        // without a signed XDR. Submitting anyway is what this guard exists to
+        // prevent — the transaction never left the device, so it has no
+        // attempted volume and no meaningful Horizon result code. The terminal
+        // event still fires (this is the flow's outcome and the funnel counts
+        // on it), but carries only its pre-existing failure properties.
+        //
+        // The ordinary case — a signature that threw — has already been put
+        // into ActionStatus.ERROR by `signFreighterTransaction.rejected`, so
+        // TransactionConfirm renders SubmitFail as before, now showing that
+        // real error instead of one manufactured by submitting a bad XDR.
+        //
+        // The two paths that dispatch no rejected action — a sign that
+        // resolves fulfilled with an empty payload, and a hardware flow with
+        // no signed XDR — would otherwise leave Redux on PENDING and strand
+        // the user on the sending spinner, since nothing downstream sets the
+        // status any more. `setSubmitError` closes that: it is idempotent
+        // with the reducer above (same status, same error) on the path where
+        // both run.
+        if (!isCustom) {
+          if (isCollectible) {
+            emitMetric(METRIC_NAMES.collectibleSendFailed, {
+              reason_code: PRE_SUBMISSION_REASON_CODE,
+            });
+          } else if (isSwap) {
+            emitMetric(METRIC_NAMES.swapFailed, {
+              from_asset_code: getAssetFromCanonical(asset).code,
+              to_asset_code: getAssetFromCanonical(destinationAsset).code,
+              reason_code: PRE_SUBMISSION_REASON_CODE,
+            });
+          } else {
+            emitMetric(METRIC_NAMES.paymentFailed, {
+              payment_type: "payment",
+              asset_code: sourceAsset.code,
+              reason_code: PRE_SUBMISSION_REASON_CODE,
+            });
+          }
+        }
+
+        const error =
+          signingError ??
+          ({ errorMessage: "Failed to sign transaction" } as ErrorMessage);
+        reduxDispatch(setSubmitError(error));
+        dispatch({ type: "FETCH_DATA_ERROR", payload: error });
+        return error;
+      }
+
+      // Everything the volume telemetry needs is snapshotted here — after
+      // signing succeeded and immediately before submission, so the prices sit
+      // as close as possible to the transaction's actual execution time.
+      // Amounts and prices are frozen together and carried to whichever
+      // terminal event fires. Skipped entirely for collectible sends (unpriced,
+      // out of scope) and for custom networks (not real economic activity,
+      // shouldn't pollute volume metrics).
+      const cachedDisplayPrices =
+        allTokenPricesCache[networkDetails.networkPassphrase]?.[publicKey] ??
+        null;
+      snapshotHandle = sourceIdentity
+        ? startConfirmationPriceSnapshot({
+            canonicalIds: [
+              getCanonicalFromAsset(sourceIdentity.code, sourceIdentity.issuer),
+              ...(destIdentity
+                ? [
+                    getCanonicalFromAsset(
+                      destIdentity.code,
+                      destIdentity.issuer,
+                    ),
+                  ]
+                : []),
+            ],
+            networkDetails,
+            useV2: useTokenPricesV2,
+            cachedDisplayPrices,
+          })
+        : null;
+
       const submitResp = await reduxDispatch(
         submitFreighterTransaction({
           publicKey,