Skip to content

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

Merged
JakeUrban merged 8 commits into
masterfrom
claude/fervent-albattani-yh5r0j
Sep 11, 2026
Merged

feat(analytics): report signing for every transaction, and give each flow stage one meaning#3008
JakeUrban merged 8 commits into
masterfrom
claude/fervent-albattani-yh5r0j

Conversation

@JakeUrban

@JakeUrban JakeUrban commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Context

Signing is unmeasurable in three ways.

Hardware signers report nothing. A software key reports a signing event. A hardware device reports none. Rejections still report, so the numbers show hardware users declining and almost never approving.

Transactions the wallet composes report nothing. A send, a swap, a collectible send, and a trustline change all report nothing for the signing action. Only a request from a website reports it.

The flow stages do not each carry one meaning. The review stage marks the screen after the review screen, which the user reaches only by approving. The swap flow reports no in-flight stage and no success stage. Nobody can therefore follow a swap past its review screen.

What this changes

Every transaction now reports how signing ended. The record names one of three outcomes: a signature, a decline by the user, or a failure the user did not choose. Both kinds of transaction use the same three. Failure is new for transactions, so a signing fault previously went unrecorded.

A decline on the device now counts as the user declining. It previously counted as a fault, next to a missing device or a connection problem.

Every signing event now names its origin, either a website or the wallet itself. Both origins carry the same properties. A single query therefore counts all signing, and the origin splits that count.

Each flow stage now carries one meaning. The review stage marks the review screen, before the user decides. The in-flight stage marks the work that follows an approval. The success stage marks settlement. The swap flow gains the two stages it lacked, so send and swap now compare directly.

A retried send now counts. The second attempt after a failure previously went unrecorded.

The schema version moves up. One stage changes meaning, so consumers need to tell an old client from a new one.

Companion change

stellar/freighter-mobile#1008 applies the same schema to the mobile app. Both clients then report the same events with the same shape.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Qkq7j6uvUMU1bBQdcuGX8Z

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
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-66e8e492eb50c3e47ac7
Backend: V1 prod + V2 beta (no sandbox configured for @JakeUrban). SDF collaborators only — install instructions in the release description.

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
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
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
@JakeUrban JakeUrban changed the title feat(analytics): emit signing metrics on the hardware wallet branch feat(analytics): report signing for every transaction, and give each flow stage one meaning Sep 10, 2026
@JakeUrban
JakeUrban marked this pull request as ready for review September 10, 2026 23:34
Copilot AI balanced review requested due to automatic review settings September 10, 2026 23:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Centralizes signing analytics across dApp, hardware, and wallet-composed transactions while aligning send/swap funnel stages.

Changes:

  • Adds unified approved, rejected, and failed signing events with source/origin metadata.
  • Instruments hardware and internal transaction signing, including retries.
  • Moves confirmation telemetry to review screens and bumps schema version to 4.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
views/Swap/index.tsx Adds processing and success stages.
views/SignTransaction/index.tsx Passes dApp URL to hardware signing.
views/SignMessage/index.tsx Passes message origin to hardware signing.
views/SignAuthEntry/index.tsx Passes authorization origin to hardware signing.
views/Send/index.tsx Corrects stages and retry handling.
views/__tests__/Swap.telemetry.test.tsx Tests swap stage telemetry.
views/__tests__/Send.test.tsx Tests send retry telemetry.
metrics/signing.ts Defines unified signing telemetry helpers.
metrics/access.ts Routes dApp signing outcomes through helpers.
metrics/__tests__/signing.test.ts Tests signing event schemas.
helpers/useSetupSigningFlow.ts Documents software/hardware telemetry ownership.
helpers/hardwareConnect.ts Classifies device rejection errors.
helpers/__tests__/hardwareConnect.test.ts Tests rejection classification.
constants/metricsNames.ts Adds transaction-failure metric.
constants/__tests__/metricsNames.test.ts Verifies metric names.
swap/SwapAmount/index.tsx Emits review and rejection events.
swap/SwapAmount/__tests__/SwapAmount.telemetry.test.tsx Updates telemetry mocking.
send/SendAmount/index.tsx Emits review and rejection events.
ChangeTrustInternal/hooks/useChangeTrust.tsx Adds trustline signing outcomes.
useSubmitTxData.tsx Adds internal software-signing outcomes.
useSubmitTxData.telemetry.test.tsx Tests internal signing telemetry.
HardwareSign/index.tsx Adds hardware signing outcomes and origin.
HardwareSign.test.tsx Tests hardware signing telemetry.
helpers/metrics.ts Bumps analytics schema to version 4.
helpers/metrics.test.ts Updates schema assertions.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread extension/src/popup/helpers/useSetupSigningFlow.ts
Comment thread extension/src/popup/metrics/access.ts
Comment thread extension/src/popup/metrics/signing.ts Outdated
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
Copilot AI review requested due to automatic review settings September 11, 2026 03:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

extension/src/popup/components/send/SendAmount/index.tsx:982

  • The rejection is only emitted from ReviewTx's Cancel button. SlideupModal also dismisses this review when its backdrop is clicked, but that path invokes setIsModalOpen directly, so the send closes without the required signing.transaction_rejected outcome. Emit the rejection from the backdrop callback as well (the companion mobile flow likewise instruments dismissal rather than only the button).
    extension/src/popup/components/swap/SwapAmount/index.tsx:873
  • This only records cancellation through the review footer. Clicking the SlideupModal backdrop calls setIsModalOpen instead, so that equally valid dismissal closes the swap review with no rejection outcome. Route the backdrop callback through the same telemetry (as the companion mobile change does for sheet dismissals).

Comment thread @shared/api/internal.ts Outdated
Comment thread extension/src/popup/components/hardwareConnect/HardwareSign/index.tsx Outdated
Comment thread extension/src/popup/components/send/SendAmount/index.tsx Outdated
Comment thread extension/src/popup/views/Swap/index.tsx Outdated
Comment thread @shared/api/internal.ts Outdated
… 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
Copilot AI review requested due to automatic review settings September 11, 2026 20:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.

Note

Copilot is running an experiment and ran this review at Balanced.

Comment thread extension/src/popup/metrics/signing.ts Outdated
Comment thread extension/src/popup/metrics/signing.ts Outdated
… 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
Copilot AI review requested due to automatic review settings September 11, 2026 21:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

extension/src/popup/views/tests/Swap.telemetry.test.tsx:203

  • This dispatch does not seed the store used by the component under test. getTestStore() still references the previous test's Wrapper store (or is null when this test runs alone), and renderSwap() immediately creates a new store with an IDLE status. The assertion therefore passes even if the stale-status guard is removed. Pass a SUCCESS status through renderSwap's Wrapper state so the mounted Swap actually observes the terminal status.

Note

Copilot is running an experiment and ran this review at Balanced.

Comment thread extension/src/popup/components/hardwareConnect/HardwareSign/index.tsx Outdated
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
Copilot AI review requested due to automatic review settings September 11, 2026 22:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/index.tsx:115

  • This cleanup also runs when the component is dismissed during its initial IDLE/LOADING state or from the transaction-build error UI (rendered at lines 172–194). In those cases no review was ever shown, yet the code records signing.transaction_rejected. Track that the review reached its success/details state and only emit a rejection after that point.

This issue also appears on line 129 of the same file.
extension/src/popup/views/tests/Swap.telemetry.test.tsx:205

  • getTestStore() still points to the previous test's store here (and is null when this test runs alone). renderSwap() then creates a fresh store with an idle status, so this test passes even if the stale-terminal guard is removed. Seed SUCCESS in the state used by the newly rendered Wrapper—for example by letting renderSwap accept an initial submission status—so the test exercises the claimed mount condition.

extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/index.tsx:132

  • This latch is never reset when SubmitTransaction returns to the review: both the hardware overlay's close action and the password dialog's Cancel invoke the goBack callback passed at line 580. If the user then cancels or dismisses the review, cleanup still sees hasApprovedRef.current === true, so that signing flow emits no outcome. Re-arm the rejection latch whenever the submit step returns to details.
  const onApproveReview = () => {
    hasApprovedRef.current = true;
    setActiveBodyContent(ActiveBodyContent.submitTx);
  };

Note

Copilot is running an experiment and ran this review at Balanced.

@JakeUrban
JakeUrban merged commit 31724ca into master Sep 11, 2026
12 checks passed
@JakeUrban
JakeUrban deleted the claude/fervent-albattani-yh5r0j branch September 11, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants