feat(analytics): report signing for every transaction, and give each flow stage one meaning

@shared/api/__tests__/internal.test.ts

@@ -17,6 +17,56 @@ describe("internalApi", () => {
     jest.clearAllMocks();
     jest.restoreAllMocks();
   });
+  describe("signing wrappers surface failure", () => {
+    // The background answers with `{ error }` rather than throwing. These
+    // wrappers used to discard both that answer and any transport exception,
+    // so a failed signing resolved like a success and telemetry recorded an
+    // approval that never happened.
+    const SIGNERS = [
+      [
+        "signTransaction",
+        () => internalApi.signTransaction({ activePublicKey: "G1", uuid: "u" }),
+      ],
+      [
+        "signBlob",
+        () => internalApi.signBlob({ activePublicKey: "G1", uuid: "u" }),
+      ],
+      [
+        "signAuthEntry",
+        () => internalApi.signAuthEntry({ activePublicKey: "G1", uuid: "u" }),
+      ],
+      [
+        "handleSignedHwPayload",
+        () =>
+          internalApi.handleSignedHwPayload({ signedPayload: "x", uuid: "u" }),
+      ],
+    ] as const;
+
+    it.each(SIGNERS)(
+      "%s rejects when the background reports an error",
+      async (_name, call) => {
+        mockedSend.mockResolvedValue({ error: "Transaction not found" });
+
+        await expect(call()).rejects.toThrow("Transaction not found");
+      },
+    );
+
+    it.each(SIGNERS)(
+      "%s rejects when the message transport throws",
+      async (_name, call) => {
+        mockedSend.mockRejectedValue(new Error("Receiving end does not exist"));
+
+        await expect(call()).rejects.toThrow("Receiving end does not exist");
+      },
+    );
+
+    it.each(SIGNERS)("%s resolves on success", async (_name, call) => {
+      mockedSend.mockResolvedValue({});
+
+      await expect(call()).resolves.toBeUndefined();
+    });
+  });
+
   describe("getAssetDomains", () => {
     it("should return a list of domains from a list of issuers", async () => {
       jest

@shared/api/internal.ts

@@ -1546,6 +1546,33 @@ export const grantAccess = async ({
   }
 };
 
+/**
+ * Reads a reportable message out of a background `{ error }` payload.
+ *
+ * The background returns whatever it caught, so the value is a string on some
+ * paths and an Error on others. `JSON.stringify` renders an Error as "{}",
+ * which reaches telemetry as a reason code with no information, so read the
+ * usual message fields first.
+ */
+const backgroundErrorMessage = (error: unknown): string => {
+  if (typeof error === "string") {
+    return error;
+  }
+  if (error && typeof error === "object") {
+    const { message, errorMessage } = error as {
+      message?: unknown;
+      errorMessage?: unknown;
+    };
+    if (typeof message === "string" && message) {
+      return message;
+    }
+    if (typeof errorMessage === "string" && errorMessage) {
+      return errorMessage;
+    }
+  }
+  return "Unknown error";
+};
+
 export const handleSignedHwPayload = async ({
   signedPayload,
   signerAddress,
@@ -1556,15 +1583,26 @@ export const handleSignedHwPayload = async ({
   uuid: string;
 }): Promise<void> => {
   try {
-    await sendMessageToBackground({
+    const res = await sendMessageToBackground<{
+      error?: unknown;
+    }>({
       activePublicKey: null,
       signedPayload,
       signerAddress,
       uuid,
       type: SERVICE_TYPES.HANDLE_SIGNED_HW_PAYLOAD,
     });
+
+    // The background answers with `{ error }` rather than throwing, so a
+    // signing failure previously looked identical to success: the caller
+    // resolved, and telemetry recorded an approval that never happened.
+    // Surface both kinds of failure so the caller can report the real outcome.
+    if (res && res.error) {
+      throw new Error(backgroundErrorMessage(res.error));
+    }
   } catch (e) {
     console.error(e);
+    throw e;
   }
 };
 
@@ -1597,13 +1635,24 @@ export const signTransaction = async ({
   uuid: string;
 }): Promise<void> => {
   try {
-    await sendMessageToBackground({
+    const res = await sendMessageToBackground<{
+      error?: unknown;
+    }>({
       activePublicKey,
       uuid,
       type: SERVICE_TYPES.SIGN_TRANSACTION,
     });
+
+    // The background answers with `{ error }` rather than throwing, so a
+    // signing failure previously looked identical to success: the caller
+    // resolved, and telemetry recorded an approval that never happened.
+    // Surface both kinds of failure so the caller can report the real outcome.
+    if (res && res.error) {
+      throw new Error(backgroundErrorMessage(res.error));
+    }
   } catch (e) {
     console.error(e);
+    throw e;
   }
 };
 
@@ -1617,14 +1666,25 @@ export const signBlob = async ({
   uuid: string;
 }): Promise<void> => {
   try {
-    await sendMessageToBackground({
+    const res = await sendMessageToBackground<{
+      error?: unknown;
+    }>({
       apiVersion,
       activePublicKey,
       uuid,
       type: SERVICE_TYPES.SIGN_BLOB,
     });
+
+    // The background answers with `{ error }` rather than throwing, so a
+    // signing failure previously looked identical to success: the caller
+    // resolved, and telemetry recorded an approval that never happened.
+    // Surface both kinds of failure so the caller can report the real outcome.
+    if (res && res.error) {
+      throw new Error(backgroundErrorMessage(res.error));
+    }
   } catch (e) {
     console.error(e);
+    throw e;
   }
 };
 
@@ -1636,13 +1696,24 @@ export const signAuthEntry = async ({
   uuid: string;
 }): Promise<void> => {
   try {
-    await sendMessageToBackground({
+    const res = await sendMessageToBackground<{
+      error?: unknown;
+    }>({
       activePublicKey,
       uuid,
       type: SERVICE_TYPES.SIGN_AUTH_ENTRY,
     });
+
+    // The background answers with `{ error }` rather than throwing, so a
+    // signing failure previously looked identical to success: the caller
+    // resolved, and telemetry recorded an approval that never happened.
+    // Surface both kinds of failure so the caller can report the real outcome.
+    if (res && res.error) {
+      throw new Error(backgroundErrorMessage(res.error));
+    }
   } catch (e) {
     console.error(e);
+    throw e;
   }
 };
 

extension/src/helpers/metrics.test.ts

@@ -145,8 +145,8 @@ describe("buildCommonContext (four-bucket property model)", () => {
     );
   });
 
-  it("stamps schema_version '3'", () => {
-    expect(buildCommonContext({} as never).schema_version).toBe("3");
+  it("stamps schema_version '4'", () => {
+    expect(buildCommonContext({} as never).schema_version).toBe("4");
   });
 
   it("emits the reshaped event-level bucket", () => {
@@ -212,7 +212,7 @@ describe("buildCommonContext (four-bucket property model)", () => {
     expect(ctx).not.toHaveProperty("account_funded");
     expect(ctx).not.toHaveProperty("is_hardware_account");
     // non-account context is still present
-    expect(ctx).toMatchObject({ schema_version: "3", network: "TESTNET" });
+    expect(ctx).toMatchObject({ schema_version: "4", network: "TESTNET" });
     expect(ctx.surface).toBeDefined();
   });
 
@@ -524,7 +524,7 @@ describe("emitScreenViewed (screen.viewed consolidation)", () => {
     expect(body).toMatchObject({
       screen_name: "send_payment_amount",
       flow: "send",
-      schema_version: "3",
+      schema_version: "4",
     });
     // surface comes from the Slice-A common context (getSurface()).
     expect(body.surface).toBeDefined();
@@ -610,7 +610,7 @@ describe("app.opened", () => {
     expect(call![1]).toMatchObject({
       connection_type: "wifi",
       effective_type: "4g",
-      schema_version: "3",
+      schema_version: "4",
     });
     expect(call![1].surface).toBeDefined();
   });

extension/src/helpers/metrics.ts

@@ -123,11 +123,12 @@ const AMPLITUDE_FLUSH_INTERVAL_MS = 500;
 
 /**
  * Schema generation marker for the cross-platform property model. Bumped to
- * "3" for the swap/send USD volume telemetry: without a bump, an event with
- * no `amount_usd` is ambiguous between a pre-change client and a post-change
- * client that genuinely had no price.
+ * "4" for the signing/step alignment: `step: "confirm"` now marks the review
+ * screen (before the user decides) rather than the submitting screen, so
+ * without a bump a `confirm` event is ambiguous between a pre-change and a
+ * post-change client.
  */
-export const SCHEMA_VERSION = "3";
+export const SCHEMA_VERSION = "4";
 
 /** Maps the internal account type to the RFC's wire value for `account_type`. */
 const ACCOUNT_TYPE_WIRE: Record<AccountType, string> = {

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

@@ -21,6 +21,12 @@ import { makeDummyStore } from "popup/__testHelpers__";
 import { initialState as txSubmissionInitialState } from "popup/ducks/transactionSubmission";
 import { METRIC_NAMES } from "popup/constants/metricsNames";
 import { emitMetric } from "helpers/metrics";
+import {
+  emitSigningApproved,
+  emitSigningFailed,
+  SigningKind,
+  SigningSource,
+} from "popup/metrics/signing";
 import { useSubmitTxData } from "../useSubmitTxData";
 
 // The emit site is the unit under test — emitMetric itself is mocked so no
@@ -30,6 +36,16 @@ jest.mock("helpers/metrics", () => ({
   emitMetric: jest.fn(),
 }));
 
+// The signing events are a separate contract with their own suite
+// (popup/metrics/__tests__/signing.test.ts). Mock the helpers so they do not
+// reach emitMetric — this suite asserts on the terminal event and counts
+// calls, and a signing event landing in the same mock would break that.
+jest.mock("popup/metrics/signing", () => ({
+  ...jest.requireActual("popup/metrics/signing"),
+  emitSigningApproved: jest.fn(),
+  emitSigningFailed: jest.fn(),
+}));
+
 // Post-success refetches are outside the telemetry contract; stub them so the
 // test never touches the balance/collectible backends.
 jest.mock("helpers/hooks/useGetBalances", () => ({
@@ -205,6 +221,8 @@ describe("useSubmitTxData terminal-event telemetry", () => {
   afterEach(() => {
     jest.restoreAllMocks();
     (emitMetric as jest.Mock).mockClear();
+    (emitSigningApproved as jest.Mock).mockClear();
+    (emitSigningFailed as jest.Mock).mockClear();
   });
 
   it("payment.completed carries identity, token amount, and the source-leg USD family (confirmation_fetch)", async () => {
@@ -544,6 +562,68 @@ describe("useSubmitTxData terminal-event telemetry", () => {
 
     expect(emitMetric).not.toHaveBeenCalled();
   });
+  describe("internal signing events", () => {
+    // Internal transactions report signing with the same events a dApp
+    // request uses; `source` separates the two. Without these, a wallet-
+    // composed transaction reported nothing for the signing action.
+    const mockSigningFailure = () =>
+      jest
+        .spyOn(ApiInternal, "signFreighterTransaction")
+        .mockRejectedValue(new Error("Incorrect password"));
+
+    it("reports an approval once a software key produces a signature", async () => {
+      mockSubmitOk(buildResultXdr("880000000"));
+
+      const { result } = renderSubmitHook(makeState({ asset: "native" }));
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      expect(emitSigningApproved).toHaveBeenCalledWith(
+        SigningKind.Transaction,
+        { source: SigningSource.Internal },
+      );
+      expect(emitSigningFailed).not.toHaveBeenCalled();
+    });
+
+    it("reports a failure when signing throws", async () => {
+      // The user already approved at the review screen, so a signing error is
+      // a fault, never a decision.
+      mockSigningFailure();
+
+      const { result } = renderSubmitHook(makeState({ asset: "native" }));
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      expect(emitSigningFailed).toHaveBeenCalledWith(
+        SigningKind.Transaction,
+        expect.anything(),
+        { source: SigningSource.Internal },
+      );
+      expect(emitSigningApproved).not.toHaveBeenCalled();
+    });
+
+    it("reports nothing for a hardware flow, which the overlay owns", async () => {
+      // A hardware device signs in the HardwareSign overlay, which reports
+      // that attempt itself. This hook only receives the result, so emitting
+      // here would double-count.
+      mockSubmitOk(buildResultXdr("880000000"));
+
+      const { result } = renderSubmitHook(
+        makeState({ asset: "native" }),
+        MAINNET_NETWORK_DETAILS,
+        { isHardwareWallet: true },
+      );
+      await act(async () => {
+        await result.current.fetchData({ isSwap: false });
+      });
+
+      expect(emitSigningApproved).not.toHaveBeenCalled();
+      expect(emitSigningFailed).not.toHaveBeenCalled();
+    });
+  });
+
   describe("pre-submission (signing) failure", () => {
     const mockSigningFailure = () =>
       jest