Skip to content

Commit 31724ca

Browse files
JakeUrbanclaude
andauthored
feat(analytics): report signing for every transaction, and give each flow stage one meaning (#3008)
* feat(analytics): emit signing metrics on the hardware wallet branch Hardware signers produced no signing.* event. `useSetupSigningFlow` diverts them to the HardwareSign overlay and never dispatches the sign thunk. The handlers in popup/metrics/access.ts key on that thunk, so every hardware approval was lost. Rejections still fired, because `rejectAndClose` dispatches the reject thunk for both key types. The funnel therefore showed hardware rejections with no approvals. This change adds the missing events for all three signing views: transaction, message, and auth entry. Add popup/metrics/signing.ts. This module owns the signing event schema. It selects the event name, adds the constant `message_type`, derives `origin`, and scrubs `reason_code`. Both key types emit through it, so the two paths cannot drift apart. Move the software-key handlers in access.ts onto the same module. The emitted payloads do not change. Emit from the HardwareSign overlay: - Success fires after `handleSignedHwPayload` resolves. The software event fires when the background resolves the dApp request, and that call is the equivalent point. A device signature that never reaches the dApp is not an approval. - Failure fires on a rejected sign thunk and on any throw. This covers no device attached, the mismatched-account refusal, and a payload delivery failure. - Internal send, swap, and trustline flows emit nothing. They report their outcome as payment.completed, swap.completed, or asset.added. Thread the dApp url from the three views into the overlay so hardware events carry the same `origin` as software events. A transaction runtime failure emits nothing, on both key types. The shared catalog has no such event, and access.ts registers no `signTransaction.rejected` handler. FAILED_EVENT records the gap explicitly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z * fix(analytics): report a device decline as a rejection, not a failure A hardware decline is a user decision. The overlay reported every hardware error as a signing failure, so a decline landed on `signing.*_failed` next to real faults. Add `isDeviceRefusalError` beside `parseWalletError`, which already matches the same two messages. hw-app-str raises StellarUserRefusedError("User refused the request") for the deny status word on every sign call. Older apps and transports worded the same decision differently, so match both. Route a decline to `signing.*_rejected`, the event that already carries a popup reject. A rejection carries no `reason_code`: nothing went wrong. Every other hardware error stays on `signing.*_failed` — no device attached, a transport fault, the wrong device, an app too old. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z * feat(analytics): give each flow stage one meaning and add swap parity The `confirm` stage marked the submitting screen, which the user only reaches after approving. Mobile marks the review sheet, before the user decides. The two clients therefore counted different things. Move `confirm` to the review modal in the send and swap amount screens. Emit it from an effect on the modal's open state, so every entry point counts once and a reopen counts again. Drop the submitting screen from both step maps. Its stages are already reported by the submission-status effect (processing, then success), so a third event would double-count and reuse a name that now belongs to the review modal. Add the processing and success stages to the swap flow. Swap reported neither, so a swap could not be followed past the review screen. Reset the stage guards on ERROR as well as IDLE. A retry goes ERROR -> PENDING without passing through IDLE, because returning from the failure screen does not reset the submission. Guarding on IDLE alone dropped every retried attempt. Bump the schema version to 4. The `confirm` stage changes meaning, so consumers must tell an old client from a new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z * feat(analytics): report signing for internal transactions Internal transactions reported nothing for the signing action. Only a dApp request did, so approvals and signing outcomes were unmeasurable for the transactions the wallet composes itself. Add `signing.transaction_failed`. The message and auth-entry families each carry approved, rejected and failed. The transaction family had no failure event, so a signing fault went unreported on every path. Register it for the dApp thunk as well. Add a `source` property to every signing event: `dapp_api` for a website request, `internal` for a wallet-composed one. Both origins now emit the same events with the same properties, so one query counts all signing and `source` splits it. An internal transaction has no origin, so it omits that property. Emit from every place internal signing ends: - the submission hook, for software keys; - the hardware overlay, for a device. A device signs before the submission hook runs, and a decline keeps the user on the overlay, so the flow never reaches the hook. This is the case that reported nothing at all; - the review screen's cancel, which is the internal rejection; - the trustline flow, which runs its own signing step. Split the ownership by key type so an internal hardware signing is reported once: the overlay owns the device attempt, the submission hook owns the software attempt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z * fix(analytics): make signing outcomes reachable and correctly paired The signing wrappers discarded every failure. The background answers with an error object rather than throwing, and the wrappers ignored both that answer and any transport exception. A failed signing therefore resolved like a success: the approval event fired, and the failure event was unreachable. Surface both kinds of failure, matching the pattern the token-add wrapper already uses. This applies to the hardware payload handover as well. That path reported an approval even when the request never reached the website. Report a message rejection when the user declines a message prompt. The message view dispatched the transaction reject request, so declining a message reported a transaction rejection and omitted the message type. Report a rejection when the user cancels a trustline review. A dedicated handler keeps the success and close paths out of the count, because they reuse the same cancel callback. Correct the failure helper's description, which still described the transaction outcome as disabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z * fix(analytics): cover every way a user leaves a review, and read real errors Report a rejection when the user leaves a review by any route. The handlers covered the Cancel buttons only, so dismissing through the modal backdrop reported a review stage with no outcome. Each flow now keys the rejection on the review closing, and marks an approval first so it is not counted. Leaving the send review to edit the memo is not a decision, so it is not counted either. Report a trustline rejection on the same basis. That review lives in a modal whose backdrop no button handler sees. Ignore a submission status left behind by an earlier submission. The status lives in the store, so a view that mounted and found a terminal status reported a stage the user never reached. Both flows now wait until the status has been seen idle, which the reset on mount guarantees. The swap stage effect moves below that reset, matching the send flow. Read a real message out of a background error. The error is an object on some paths, which stringified to "{}" and reached the reason code with no information. Keep a string error unquoted. The hardware overlay passes the message directly on one branch, and stringifying it wrapped the reason code in quotes. Expect a signing failure in the integration helper. It drives the signing wrappers with a placeholder request id, so they now reject, and an unhandled rejection would stop the remaining checks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z * fix(analytics): latch every trustline approval, and model the signing sets as enums Route every approval of the trustline review through one handler. The path behind the security warning skipped the approval mark, so a transaction approved that way reported an approval and then a rejection. Model the signing kind and the signing origin as enums. The repository requires a finite named set of string values to be an enum, not a union type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z * fix(metrics): keep the hardware error message a string JSON.stringify returns undefined for a value it cannot represent. Fall back to the empty string so the helper keeps its declared return type. emitSigningFailed reports that as "unknown". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 19781cc commit 31724ca

29 files changed

Lines changed: 1666 additions & 110 deletions

File tree

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,56 @@ describe("internalApi", () => {
1717
jest.clearAllMocks();
1818
jest.restoreAllMocks();
1919
});
20+
describe("signing wrappers surface failure", () => {
21+
// The background answers with `{ error }` rather than throwing. These
22+
// wrappers used to discard both that answer and any transport exception,
23+
// so a failed signing resolved like a success and telemetry recorded an
24+
// approval that never happened.
25+
const SIGNERS = [
26+
[
27+
"signTransaction",
28+
() => internalApi.signTransaction({ activePublicKey: "G1", uuid: "u" }),
29+
],
30+
[
31+
"signBlob",
32+
() => internalApi.signBlob({ activePublicKey: "G1", uuid: "u" }),
33+
],
34+
[
35+
"signAuthEntry",
36+
() => internalApi.signAuthEntry({ activePublicKey: "G1", uuid: "u" }),
37+
],
38+
[
39+
"handleSignedHwPayload",
40+
() =>
41+
internalApi.handleSignedHwPayload({ signedPayload: "x", uuid: "u" }),
42+
],
43+
] as const;
44+
45+
it.each(SIGNERS)(
46+
"%s rejects when the background reports an error",
47+
async (_name, call) => {
48+
mockedSend.mockResolvedValue({ error: "Transaction not found" });
49+
50+
await expect(call()).rejects.toThrow("Transaction not found");
51+
},
52+
);
53+
54+
it.each(SIGNERS)(
55+
"%s rejects when the message transport throws",
56+
async (_name, call) => {
57+
mockedSend.mockRejectedValue(new Error("Receiving end does not exist"));
58+
59+
await expect(call()).rejects.toThrow("Receiving end does not exist");
60+
},
61+
);
62+
63+
it.each(SIGNERS)("%s resolves on success", async (_name, call) => {
64+
mockedSend.mockResolvedValue({});
65+
66+
await expect(call()).resolves.toBeUndefined();
67+
});
68+
});
69+
2070
describe("getAssetDomains", () => {
2171
it("should return a list of domains from a list of issuers", async () => {
2272
jest

@shared/api/internal.ts

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,33 @@ export const grantAccess = async ({
15461546
}
15471547
};
15481548

1549+
/**
1550+
* Reads a reportable message out of a background `{ error }` payload.
1551+
*
1552+
* The background returns whatever it caught, so the value is a string on some
1553+
* paths and an Error on others. `JSON.stringify` renders an Error as "{}",
1554+
* which reaches telemetry as a reason code with no information, so read the
1555+
* usual message fields first.
1556+
*/
1557+
const backgroundErrorMessage = (error: unknown): string => {
1558+
if (typeof error === "string") {
1559+
return error;
1560+
}
1561+
if (error && typeof error === "object") {
1562+
const { message, errorMessage } = error as {
1563+
message?: unknown;
1564+
errorMessage?: unknown;
1565+
};
1566+
if (typeof message === "string" && message) {
1567+
return message;
1568+
}
1569+
if (typeof errorMessage === "string" && errorMessage) {
1570+
return errorMessage;
1571+
}
1572+
}
1573+
return "Unknown error";
1574+
};
1575+
15491576
export const handleSignedHwPayload = async ({
15501577
signedPayload,
15511578
signerAddress,
@@ -1556,15 +1583,26 @@ export const handleSignedHwPayload = async ({
15561583
uuid: string;
15571584
}): Promise<void> => {
15581585
try {
1559-
await sendMessageToBackground({
1586+
const res = await sendMessageToBackground<{
1587+
error?: unknown;
1588+
}>({
15601589
activePublicKey: null,
15611590
signedPayload,
15621591
signerAddress,
15631592
uuid,
15641593
type: SERVICE_TYPES.HANDLE_SIGNED_HW_PAYLOAD,
15651594
});
1595+
1596+
// The background answers with `{ error }` rather than throwing, so a
1597+
// signing failure previously looked identical to success: the caller
1598+
// resolved, and telemetry recorded an approval that never happened.
1599+
// Surface both kinds of failure so the caller can report the real outcome.
1600+
if (res && res.error) {
1601+
throw new Error(backgroundErrorMessage(res.error));
1602+
}
15661603
} catch (e) {
15671604
console.error(e);
1605+
throw e;
15681606
}
15691607
};
15701608

@@ -1597,13 +1635,24 @@ export const signTransaction = async ({
15971635
uuid: string;
15981636
}): Promise<void> => {
15991637
try {
1600-
await sendMessageToBackground({
1638+
const res = await sendMessageToBackground<{
1639+
error?: unknown;
1640+
}>({
16011641
activePublicKey,
16021642
uuid,
16031643
type: SERVICE_TYPES.SIGN_TRANSACTION,
16041644
});
1645+
1646+
// The background answers with `{ error }` rather than throwing, so a
1647+
// signing failure previously looked identical to success: the caller
1648+
// resolved, and telemetry recorded an approval that never happened.
1649+
// Surface both kinds of failure so the caller can report the real outcome.
1650+
if (res && res.error) {
1651+
throw new Error(backgroundErrorMessage(res.error));
1652+
}
16051653
} catch (e) {
16061654
console.error(e);
1655+
throw e;
16071656
}
16081657
};
16091658

@@ -1617,14 +1666,25 @@ export const signBlob = async ({
16171666
uuid: string;
16181667
}): Promise<void> => {
16191668
try {
1620-
await sendMessageToBackground({
1669+
const res = await sendMessageToBackground<{
1670+
error?: unknown;
1671+
}>({
16211672
apiVersion,
16221673
activePublicKey,
16231674
uuid,
16241675
type: SERVICE_TYPES.SIGN_BLOB,
16251676
});
1677+
1678+
// The background answers with `{ error }` rather than throwing, so a
1679+
// signing failure previously looked identical to success: the caller
1680+
// resolved, and telemetry recorded an approval that never happened.
1681+
// Surface both kinds of failure so the caller can report the real outcome.
1682+
if (res && res.error) {
1683+
throw new Error(backgroundErrorMessage(res.error));
1684+
}
16261685
} catch (e) {
16271686
console.error(e);
1687+
throw e;
16281688
}
16291689
};
16301690

@@ -1636,13 +1696,24 @@ export const signAuthEntry = async ({
16361696
uuid: string;
16371697
}): Promise<void> => {
16381698
try {
1639-
await sendMessageToBackground({
1699+
const res = await sendMessageToBackground<{
1700+
error?: unknown;
1701+
}>({
16401702
activePublicKey,
16411703
uuid,
16421704
type: SERVICE_TYPES.SIGN_AUTH_ENTRY,
16431705
});
1706+
1707+
// The background answers with `{ error }` rather than throwing, so a
1708+
// signing failure previously looked identical to success: the caller
1709+
// resolved, and telemetry recorded an approval that never happened.
1710+
// Surface both kinds of failure so the caller can report the real outcome.
1711+
if (res && res.error) {
1712+
throw new Error(backgroundErrorMessage(res.error));
1713+
}
16441714
} catch (e) {
16451715
console.error(e);
1716+
throw e;
16461717
}
16471718
};
16481719

extension/src/helpers/metrics.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ describe("buildCommonContext (four-bucket property model)", () => {
145145
);
146146
});
147147

148-
it("stamps schema_version '3'", () => {
149-
expect(buildCommonContext({} as never).schema_version).toBe("3");
148+
it("stamps schema_version '4'", () => {
149+
expect(buildCommonContext({} as never).schema_version).toBe("4");
150150
});
151151

152152
it("emits the reshaped event-level bucket", () => {
@@ -212,7 +212,7 @@ describe("buildCommonContext (four-bucket property model)", () => {
212212
expect(ctx).not.toHaveProperty("account_funded");
213213
expect(ctx).not.toHaveProperty("is_hardware_account");
214214
// non-account context is still present
215-
expect(ctx).toMatchObject({ schema_version: "3", network: "TESTNET" });
215+
expect(ctx).toMatchObject({ schema_version: "4", network: "TESTNET" });
216216
expect(ctx.surface).toBeDefined();
217217
});
218218

@@ -524,7 +524,7 @@ describe("emitScreenViewed (screen.viewed consolidation)", () => {
524524
expect(body).toMatchObject({
525525
screen_name: "send_payment_amount",
526526
flow: "send",
527-
schema_version: "3",
527+
schema_version: "4",
528528
});
529529
// surface comes from the Slice-A common context (getSurface()).
530530
expect(body.surface).toBeDefined();
@@ -610,7 +610,7 @@ describe("app.opened", () => {
610610
expect(call![1]).toMatchObject({
611611
connection_type: "wifi",
612612
effective_type: "4g",
613-
schema_version: "3",
613+
schema_version: "4",
614614
});
615615
expect(call![1].surface).toBeDefined();
616616
});

extension/src/helpers/metrics.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,12 @@ const AMPLITUDE_FLUSH_INTERVAL_MS = 500;
123123

124124
/**
125125
* Schema generation marker for the cross-platform property model. Bumped to
126-
* "3" for the swap/send USD volume telemetry: without a bump, an event with
127-
* no `amount_usd` is ambiguous between a pre-change client and a post-change
128-
* client that genuinely had no price.
126+
* "4" for the signing/step alignment: `step: "confirm"` now marks the review
127+
* screen (before the user decides) rather than the submitting screen, so
128+
* without a bump a `confirm` event is ambiguous between a pre-change and a
129+
* post-change client.
129130
*/
130-
export const SCHEMA_VERSION = "3";
131+
export const SCHEMA_VERSION = "4";
131132

132133
/** Maps the internal account type to the RFC's wire value for `account_type`. */
133134
const ACCOUNT_TYPE_WIRE: Record<AccountType, string> = {

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ import { makeDummyStore } from "popup/__testHelpers__";
2121
import { initialState as txSubmissionInitialState } from "popup/ducks/transactionSubmission";
2222
import { METRIC_NAMES } from "popup/constants/metricsNames";
2323
import { emitMetric } from "helpers/metrics";
24+
import {
25+
emitSigningApproved,
26+
emitSigningFailed,
27+
SigningKind,
28+
SigningSource,
29+
} from "popup/metrics/signing";
2430
import { useSubmitTxData } from "../useSubmitTxData";
2531

2632
// The emit site is the unit under test — emitMetric itself is mocked so no
@@ -30,6 +36,16 @@ jest.mock("helpers/metrics", () => ({
3036
emitMetric: jest.fn(),
3137
}));
3238

39+
// The signing events are a separate contract with their own suite
40+
// (popup/metrics/__tests__/signing.test.ts). Mock the helpers so they do not
41+
// reach emitMetric — this suite asserts on the terminal event and counts
42+
// calls, and a signing event landing in the same mock would break that.
43+
jest.mock("popup/metrics/signing", () => ({
44+
...jest.requireActual("popup/metrics/signing"),
45+
emitSigningApproved: jest.fn(),
46+
emitSigningFailed: jest.fn(),
47+
}));
48+
3349
// Post-success refetches are outside the telemetry contract; stub them so the
3450
// test never touches the balance/collectible backends.
3551
jest.mock("helpers/hooks/useGetBalances", () => ({
@@ -205,6 +221,8 @@ describe("useSubmitTxData terminal-event telemetry", () => {
205221
afterEach(() => {
206222
jest.restoreAllMocks();
207223
(emitMetric as jest.Mock).mockClear();
224+
(emitSigningApproved as jest.Mock).mockClear();
225+
(emitSigningFailed as jest.Mock).mockClear();
208226
});
209227

210228
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", () => {
544562

545563
expect(emitMetric).not.toHaveBeenCalled();
546564
});
565+
describe("internal signing events", () => {
566+
// Internal transactions report signing with the same events a dApp
567+
// request uses; `source` separates the two. Without these, a wallet-
568+
// composed transaction reported nothing for the signing action.
569+
const mockSigningFailure = () =>
570+
jest
571+
.spyOn(ApiInternal, "signFreighterTransaction")
572+
.mockRejectedValue(new Error("Incorrect password"));
573+
574+
it("reports an approval once a software key produces a signature", async () => {
575+
mockSubmitOk(buildResultXdr("880000000"));
576+
577+
const { result } = renderSubmitHook(makeState({ asset: "native" }));
578+
await act(async () => {
579+
await result.current.fetchData({ isSwap: false });
580+
});
581+
582+
expect(emitSigningApproved).toHaveBeenCalledWith(
583+
SigningKind.Transaction,
584+
{ source: SigningSource.Internal },
585+
);
586+
expect(emitSigningFailed).not.toHaveBeenCalled();
587+
});
588+
589+
it("reports a failure when signing throws", async () => {
590+
// The user already approved at the review screen, so a signing error is
591+
// a fault, never a decision.
592+
mockSigningFailure();
593+
594+
const { result } = renderSubmitHook(makeState({ asset: "native" }));
595+
await act(async () => {
596+
await result.current.fetchData({ isSwap: false });
597+
});
598+
599+
expect(emitSigningFailed).toHaveBeenCalledWith(
600+
SigningKind.Transaction,
601+
expect.anything(),
602+
{ source: SigningSource.Internal },
603+
);
604+
expect(emitSigningApproved).not.toHaveBeenCalled();
605+
});
606+
607+
it("reports nothing for a hardware flow, which the overlay owns", async () => {
608+
// A hardware device signs in the HardwareSign overlay, which reports
609+
// that attempt itself. This hook only receives the result, so emitting
610+
// here would double-count.
611+
mockSubmitOk(buildResultXdr("880000000"));
612+
613+
const { result } = renderSubmitHook(
614+
makeState({ asset: "native" }),
615+
MAINNET_NETWORK_DETAILS,
616+
{ isHardwareWallet: true },
617+
);
618+
await act(async () => {
619+
await result.current.fetchData({ isSwap: false });
620+
});
621+
622+
expect(emitSigningApproved).not.toHaveBeenCalled();
623+
expect(emitSigningFailed).not.toHaveBeenCalled();
624+
});
625+
});
626+
547627
describe("pre-submission (signing) failure", () => {
548628
const mockSigningFailure = () =>
549629
jest

0 commit comments

Comments
 (0)