Add volume telemetry for swaps and payments with USD pricing

[JakeUrban]

Summary

Adds analytics for swap and send transactions: how much value (in USD) each transaction moved, how the price moved between quote and settlement, and why a transaction failed when it did. Prices are captured once, when the user confirms, so the numbers we report stay consistent even if the market moves before the transaction finishes.

Key Changes

New Telemetry Infrastructure

• usdVolume.ts: The math and classification behind the numbers we report

  • Converts a token amount into a USD value using the price we captured at confirmation
  • Calculates two kinds of slippage: how much USD value was lost, and how far the settled amount was from the quoted amount
  • Figures out whether an asset is XLM, a classic Stellar asset, or a Soroban token — including recognizing when a Soroban token is really just a classic asset wrapped in a contract, so we don't double-count the same asset under two different names
  • Sorts a failed transaction's error into a small set of understandable categories (bad slippage, fee too low, insufficient balance, missing trustline, unknown destination, timing issue, authorization problem, or a connection problem where we never heard back from the network) instead of reporting raw error codes

• confirmationPriceSnapshot.ts: Captures prices at the moment the user confirms

  • Kicks off one price lookup covering both the asset being sent and, for swaps, the asset being received, without making the user wait for it
  • Locks in those prices once the transaction finishes, so the report reflects what the user saw, not whatever the price happens to be later
  • If the price lookup hasn't finished in time, falls back to the last price already shown on screen, and cancels the request rather than let it run forever in the background

• transactionResult.ts: Reads what actually happened on-chain

  • Finds the swap operation inside a submitted transaction
  • Reads the amount the swap actually settled for, not the amount it was originally quoted at

Enhanced Transaction Submission

• useSubmitTxData.tsx is now the one place that reports a transaction's outcome, for both successes and failures
  • Successful swaps report both assets' details, how much was sent and received, the settled amount vs. the quote, and both slippage numbers
  • Successful sends report the asset and USD value moved
  • Failed sends and swaps report the reason for failure, using the plain-language categories above
  • A swap that fails because its quote expired now gets counted the same way as any other failed swap, instead of being tracked separately and left out of the failure stats
  • Removed an older piece of code that reported failures a second time whenever the failure screen reappeared, which was double-counting

Supporting Changes

• Bumped the analytics schema version to reflect this change
• The price-lookup function can now actually be cancelled instead of just having its result ignored
• Added tests for all the new pieces, including a full test of what gets reported for each outcome (success, failure, quote expiry, price unavailable), with all external calls mocked
• The failure screen component no longer reports anything itself — it just shows the error message to the user; reporting now happens in the one central place described above

[github-actions]

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

[Copilot]

Pull request overview

Adds USD-volume, slippage, asset classification, and failure telemetry to internal swap and payment flows.

Changes:

• Captures confirmation-time prices with cancellation and cache fallback.
• Reports settled swap/payment outcomes and categorized failures.
• Adds telemetry helpers, schema v3, and unit/integration coverage.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
useSubmitTxData.tsx	Centralizes terminal transaction telemetry.
useSubmitTxData.telemetry.test.tsx	Tests emitted transaction metrics.
SubmitFail/index.tsx	Removes duplicate failure emission.
usdVolume.ts	Implements USD, slippage, identity, and failure calculations.
usdVolume.test.ts	Tests volume helpers.
transactionResult.ts	Extracts settled swap amounts from result XDR.
transactionResult.test.ts	Tests result-XDR parsing.
metrics.ts	Bumps analytics schema to v3.
metrics.test.ts	Updates schema assertions.
confirmationPriceSnapshot.ts	Captures cancellable confirmation-time prices.
confirmationPriceSnapshot.test.ts	Tests snapshot and fallback behavior.
@shared/api/internal.ts	Adds cancellation support to price requests.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (1)

extension/src/helpers/confirmationPriceSnapshot.ts:97

• On the cached fallback path, source describes the endpoint requested now, not the endpoint that produced cachedDisplayPrices. If the use_token_prices_v2 kill switch changes after the display cache was populated, a v1 cached rate is labeled token_prices_v2 (or vice versa), corrupting source attribution. Persist the cache's price-source metadata and use it here, or report an explicit cached/unknown source.

        source,

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 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/InternalTransaction/SubmitTransaction/hooks/useSubmitTxData.tsx:403

• A rejected signing action does reach this branch indirectly: the signing block above only replaces signedXDR on fulfillment and otherwise continues by submitting the unsigned preparedTransaction. That makes a local signing failure look like a network/protocol failure (commonly tx_bad_auth) in the new telemetry. Stop the flow when signFreighterTransaction is rejected before dispatching submitFreighterTransaction.

        // Submission was attempted and we're reacting to its outcome — the
        // single, centralized failure-emit site (fixes the old effect-based
        // double-emit-on-remount bug in SubmitFail). A pre-submission failure
        // (signing, simulation) never reaches here, since nothing above this
        // point calls submitFreighterTransaction.

extension/src/helpers/transactionResult.ts:56

• A txFailed result is atomic and did not settle any operation, even if an earlier per-operation entry is op_success. Accepting it here can therefore return a destination amount that never moved, contrary to this helper's contract. Only decode operation results when the inner transaction result is txSuccess.

    if (
      innerTxResult.type !== "txSuccess" &&
      innerTxResult.type !== "txFailed"
    ) {

extension/src/helpers/usdVolume.ts:227

• Strict-send/path-payment failures can return op_src_no_trust or op_src_not_authorized for the source trustline. These source-side counterparts currently fall through to protocol_other, so the bounded failure category is inaccurate. Map them alongside the equivalent destination trustline codes.

  op_no_trust: "trustline",
  op_src_no_trust: "trustline",
  op_line_full: "trustline",
  op_not_authorized: "trustline",

[Copilot]

Pull request overview

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

Suppressed comments (1)

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

extension/src/helpers/usdVolume.ts:176

• This lookup misses a supported SAC-only balance. injectLocalTokenBalances explicitly represents a locally saved SAC without a classic trustline as CODE:CONTRACT_ID while retaining the wrapped CODE:G_ISSUER in balance.name (@shared/api/helpers/injectLocalTokenBalances.ts:80-113,138-153). Because this loop skips every contract issuer, that asset is reported as soroban; its contract canonical ID is then filtered out by getTokenPrices, so the send also loses USD volume. Resolve SAC identity from the matching contract balance's retained metadata (not only from a separate classic balance), and add this fixture as a regression test.

    const classicMatch = Object.values(balances ?? {}).find(
      (balance): boolean => {
        if (!("issuer" in balance.token)) {
          return false;
        }
        const classicIssuer = balance.token.issuer.key;
        if (isContractId(classicIssuer) || balance.token.code !== code) {

[Copilot]

Pull request overview

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

Suppressed comments (1)

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

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

• This starts a telemetry-only price request even when usage-data sharing is disabled. emitMetric returns immediately for opted-out users (helpers/metrics.ts:574-577), so the fetched snapshot is guaranteed to be discarded while still adding a backend request to every send/swap. Gate snapshot creation and the associated telemetry derivation on settingsDataSharingSelector, or make the snapshot helper return a no-request handle when consent is off.

      snapshotHandle = sourceIdentity
        ? startConfirmationPriceSnapshot({
            canonicalIds: [
              getCanonicalFromAsset(sourceIdentity.code, sourceIdentity.issuer),
              ...(destIdentity
                ? [
                    getCanonicalFromAsset(
                      destIdentity.code,
                      destIdentity.issuer,
                    ),
                  ]
                : []),
            ],
            networkDetails,
            useV2: useTokenPricesV2,
            cachedDisplayPrices,
          })

[piyalbasu]

The custom-network fix reverted in 9b3a3cc was dropped on a premise that doesn't hold

Correction of record / low-severity data quality — not a merge blocker. The affected events are filterable in Amplitude, so nothing pollutes pubnet aggregates.

TL;DR: One of the two fixes reverted in 9b3a3cc was reverted because "this telemetry isn't emitted for custom networks at all." That isn't the case — the failure metrics do fire on custom networks; there's no network gate anywhere between the rejected submission and Amplitude. The practical consequence is small (every custom-network failure reports as an undifferentiated transport failure, and those events carry a network property so they can simply be excluded from analysis), but the one-line fix that was dropped was correct and does work. Flagging mainly so the reasoning doesn't get reapplied — the conclusion "not worth fixing" may still be right, just not for the stated reason.

---

Detailed explanation (for agents)

The revert: 9b3a3cc dropped response: (e as any)?.response from the custom-network catch added in b4dee37, with the rationale "this telemetry isn't emitted for custom networks at all, so the fix was moot."

Why the premise fails — three independent checks:

1. No isCustomNetwork reference exists anywhere in useSubmitTxData.tsx, including the submitFreighterTransaction.rejected branch that emits swapFailed / paymentFailed / collectibleSendFailed.

2. emitMetric gates on exactly two conditions — the data-sharing setting and AMPLITUDE_KEY/hasInitialized. There is no network check:

freighter/extension/src/helpers/metrics.ts

Lines 566 to 585 in d486db9

	export const emitMetric = (name: string, body?: Record<string, unknown>) => {
	const state = store.getState();
	const eventProperties = {
	...buildCommonContext(state),
	...body,
	};
	const isDataSharingAllowed = settingsDataSharingSelector(state);
	if (!isDataSharingAllowed) {
	return;
	}
	if (!AMPLITUDE_KEY || !hasInitialized) {
	console.log(LOG_MESSAGES.EVENT_NOT_UPLOADED, name, eventProperties);
	return;
	}
	amplitude.track(name, eventProperties);
	};

3. The custom-network submit path runs through the SDK's Horizon server (submitTx({ server, tx }) in @shared/api/internal.ts), and SDK Horizon errors carry the problem-details body on .response — so the data the reverted line forwarded was genuinely there and genuinely dropped.

Resulting behaviour on custom networks:

The custom branch rejects with errorMessage only, where the non-custom branch forwards response in both of its reject paths:

freighter/extension/src/popup/ducks/transactionSubmission.ts

Lines 109 to 123 in d486db9

	if (isCustomNetwork(networkDetails)) {
	try {
	const txRes = await internalSubmitFreighterTransaction({
	signedXDR,
	networkDetails,
	});
	return txRes;
	} catch (e) {
	const message = e instanceof Error ? e.message : JSON.stringify(e);
	return thunkApi.rejectWithValue({
	errorMessage: message,
	});
	}
	} else {

Downstream, isDefiniteProtocolAnswer tests error?.response and short-circuits to "transport" when it's absent, and getResultCodes reads response.extras.result_codes.* and yields "unknown":

freighter/extension/src/helpers/usdVolume.ts

Lines 275 to 291 in d486db9

	export const getFailureCategory = (
	error: ErrorMessage | undefined,
	reasonCode: string,
	): FailureCategory => {
	if (!isDefiniteProtocolAnswer(error)) {
	return "transport";
	}
	if (reasonCode === "unknown") {
	const status = (error?.response as { status?: unknown } | undefined)
	?.status;
	if (typeof status === "number" && isNoVerdictHttpStatus(status)) {
	return "transport";
	}
	return "unknown";
	}
	return REASON_CODE_TO_FAILURE_CATEGORY[reasonCode] ?? "protocol_other";
	};

So every custom-network failure emits failure_category: "transport", reason_code: "unknown", regardless of what Horizon actually returned.

Why this is still low severity: buildCommonContext stamps network on every event, so these rows are identifiable and can be excluded from any query. They do not silently corrupt pubnet/testnet failure-category breakdowns — they just make custom-network failures uninformative.

Suggested fixes:

1. Un-revert: restore the single line from b4dee37 (response: (e as any)?.response in the custom-network catch). Restores correct classification with no other behaviour change.
2. Or close it deliberately: if custom-network failure telemetry genuinely isn't worth classifying, that's a fine call — but then it's worth a comment at the reject site saying so, since the current asymmetry between the two branches reads as an oversight rather than a decision.

[piyalbasu]

Five new union type aliases where code-style.md asks for enums

Style / guidance compliance — not a merge blocker.

TL;DR: The best-practices doc has an explicit rule preferring enums over union type aliases for finite sets of named string values, and it closes by naming the four union types still left in the codebase and saying "Don't add more." This PR adds five. They're all closed sets of string constants used as telemetry property values, which is exactly the shape the rule describes. Mechanical to convert, and worth doing before this lands so the count goes down over time rather than up.

---

Detailed explanation (for agents)

The rule, stated verbatim:

freighter/docs/skills/freighter-best-practices/references/code-style.md

Lines 100 to 125 in d486db9

	Never use `const enum` — all enums in the codebase are standard runtime enums.
	### Prefer Enums Over Union Type Aliases
	When you have a finite set of named string values, use an enum, not a union
	type:
	```typescript
	// BAD — union type for a finite set
	type SwapStatus = "pending" | "confirming" | "completed" | "failed";
	type InputType = "crypto" | "fiat";
	type AssetVisibility = "visible" | "hidden";
	type NotificationType = "warning" | "info";
	// GOOD — enum
	enum SwapStatus {
	Pending = "pending",
	Confirming = "confirming",
	Completed = "completed",
	Failed = "failed",
	}
	```
	The codebase has a few remaining union types that should be enums (`InputType`,
	`AssetVisibility`, `NotificationType`, `PillType`). Don't add more.

The codebase has a few remaining union types that should be enums (InputType, AssetVisibility, NotificationType, PillType). Don't add more.

The five added here:

extension/src/helpers/usdVolume.ts

• LegUsdStatus —

  freighter/extension/src/helpers/usdVolume.ts

  Lines 30 to 32 in d486db9

  	status: LegUsdStatus;
  	/** Rounded to 2dp. Present only when status === "ok". */
  	value?: number;

• AssetKind —

  freighter/extension/src/helpers/usdVolume.ts

  Lines 124 to 126 in d486db9

  	code: string;
  	/** `G…` classic issuer or `C…` Soroban-native contract. Omitted for native XLM. */
  	issuer?: string;

• FailureCategory —

  freighter/extension/src/helpers/usdVolume.ts

  Lines 201 to 212 in d486db9

  	// ---------------------------------------------------------------------------
  	// Failure classification
  	// ---------------------------------------------------------------------------
  	export type FailureCategory =
  	| "slippage"
  	| "fee"
  	| "balance"
  	| "trustline"
  	| "destination"
  	| "sequence"
  	| "auth"

extension/src/helpers/confirmationPriceSnapshot.ts

• PriceSource and PriceFreshness —

  freighter/extension/src/helpers/confirmationPriceSnapshot.ts

  Lines 4 to 7 in d486db9

  	export type PriceSource = "token_prices_v1" | "token_prices_v2";
  	export type PriceFreshness = "confirmation_fetch" | "cached_display";

Suggested fix: straight conversion, e.g.

export enum FailureCategory {
  Slippage = "slippage",
  Fee = "fee",
  // ...
}

REASON_CODE_TO_FAILURE_CATEGORY keeps working as Record<string, FailureCategory> with the enum members as values, and the emitted Amplitude strings are unchanged since the enum values are the same literals. Same for the other four.

One thing to weigh: FailureCategory and LegUsdStatus are consumed as Amplitude property values, so the string values are a wire contract — the conversion must preserve them exactly (it does, if written as above). If there's a deliberate reason these five were left as unions that the rule didn't anticipate, worth saying so in a comment so the next reader doesn't re-litigate it.

[piyalbasu]

Question: is dropping failure telemetry for pre-submission failures intended?

Question / scope check — not a merge blocker, but worth an answer before merge since it changes what the dashboards measure.

TL;DR: Today the three failure metrics fire whenever the user lands on the failure screen, whatever put them there — including failures that happen before the transaction is ever submitted, like a rejected signing or a failed simulation. This PR moves the emit to the submission-rejected path only, which fixes a real double-counting bug but also means those pre-submission failures now emit nothing at all. That's a narrower definition of "failed" than what's currently being recorded, so any existing failure-rate chart will step down when this ships for a reason unrelated to anything actually improving. Is the narrower definition what was wanted, or should pre-submission failures still emit somewhere?

---

Detailed explanation (for agents)

Before — SubmitFail emitted from an effect keyed on error, i.e. on any render of the failure screen regardless of how the flow got there:

freighter/extension/src/popup/components/InternalTransaction/SubmitFail/index.tsx

Lines 42 to 65 in db56fe0

	useEffect(() => {
	const resultCodes = getResultCodes(error);
	const reasonCode =
	resultCodes.operations?.[0] || resultCodes.transaction || "unknown";
	// A routed/path payment fails as a swap; a collectible send has its own
	// terminal event. `network` rides on the common context.
	if (isCollectible) {
	emitMetric(METRIC_NAMES.collectibleSendFailed, {
	reason_code: reasonCode,
	});
	} else if (isSwap) {
	emitMetric(METRIC_NAMES.swapFailed, {
	from_asset_code: getAssetFromCanonical(asset).code,
	to_asset_code: getAssetFromCanonical(destinationAsset).code,
	reason_code: reasonCode,
	});
	} else {
	emitMetric(METRIC_NAMES.paymentFailed, {
	payment_type: "payment",
	reason_code: reasonCode,
	});
	}
	}, [error, isSwap, isCollectible, asset, destinationAsset]);

After — the only emit site is the submitFreighterTransaction.rejected branch in useSubmitTxData:

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

Lines 395 to 412 in d486db9

	} else if (submitFreighterTransaction.rejected.match(submitResp)) {
	// Submission was attempted and we're reacting to its outcome — the
	// single, centralized failure-emit site (fixes the old effect-based
	// double-emit-on-remount bug in SubmitFail). A pre-submission failure
	// (signing, simulation) never reaches here, since nothing above this
	// point calls submitFreighterTransaction.
	const error = submitResp.payload;
	const resultCodes = getResultCodes(error);
	// A swap prepending a changeTrust operation reports one code per
	// operation (e.g. ["op_success", "op_under_dest_min"]) - the first
	// code that actually explains the failure isn't always index 0.
	const reasonCode =
	resultCodes.operations?.find(
	(code) => code !== "op_success" && code !== "op_not_attempted",
	) ||
	resultCodes.transaction ||
	"unknown";
	const failureCategory = getFailureCategory(error, reasonCode);

The move itself is right — the old effect re-fired on remount and double-counted, which is called out in the new file comment, and centralizing next to the price snapshot and transaction result is the correct shape. The question is only about the events that fall outside the new site's reach.

Concretely, what no longer emits: anything that reaches the failure screen without submitFreighterTransaction having been dispatched — a rejected/failed sign, a simulation error, and any pre-flight validation that routes to SubmitFail.

A related nit in the same area: the new comment at the rejected branch asserts

A pre-submission failure (signing, simulation) never reaches here, since nothing above this point calls submitFreighterTransaction.

That's true as a statement about this branch, but it reads as though pre-submission failures are handled elsewhere, when the change is that they're no longer counted at all. Worth rewording to say that explicitly — it's the kind of thing that will otherwise be re-derived by whoever next investigates a failure-rate discontinuity.

Possible resolutions:

1. Intended: keep as-is, and note in the PR description that *.failed now means "submitted and rejected" so the dashboard step-change is expected and explainable.
2. Not intended: add a separate emit for pre-submission failures — either a distinct event name, or the same event with a failure_category value that marks it as pre-submission (the category enum already has room for it).

[CassioMG]

Telemetry emission relies on ~30 non-null assertions instead of narrowing

Suggestion / robustness — correct today, but brittle against future edits; not a merge blocker.

TL;DR: The centralized telemetry emission uses roughly thirty non-null assertions on three values that are genuinely null for collectible sends. The assertions are safe today only because every branch that uses them happens to be unreachable for collectibles, but that invariant lives far from the use sites and nothing enforces it — a future edit that breaks it would throw mid-flow in a path that's supposed to degrade gracefully, rather than fail visibly at a type check. The project's anti-patterns guide calls this exact pattern out; narrowing once per branch would eliminate all the assertions without changing behavior.

---

Detailed explanation (for agents)

Root cause: sourceIdentity, destIdentity, and snapshotHandle are all typed X | null, conditionally initialized on !isCollectible:

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

Lines 141 to 161 in d486db9

	allBalancesCache[networkDetails.network]?.[publicKey]?.balances ?? null;
	const sourceIdentity = !isCollectible
	? classifyAssetIdentity(
	sourceAsset.code,
	sourceAsset.issuer,
	networkDetails.networkPassphrase,
	accountBalances,
	)
	: null;
	const destAssetParsed =
	isSwap && !isCollectible
	? getAssetFromCanonical(destinationAsset)
	: null;
	const destIdentity = destAssetParsed
	? classifyAssetIdentity(
	destAssetParsed.code,
	destAssetParsed.issuer,
	networkDetails.networkPassphrase,
	accountBalances,
	)
	: null;

They are then consumed with ! assertions across the swap/payment success and failure branches — e.g. the swap-completion block:

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

Lines 225 to 234 in d486db9

	const snapshot = snapshotHandle!.resolve();
	const sourceCanonical = getCanonicalFromAsset(
	sourceIdentity!.code,
	sourceIdentity!.issuer,
	);
	const destCanonical = getCanonicalFromAsset(
	destIdentity!.code,
	destIdentity!.issuer,
	);

and similarly at lines 283–290, 348–365, 419–445, and 454–470. Each site relies on the code-distant invariant "this branch only runs when isCollectible is false" (e.g. isSwap and isCollectible being mutually exclusive), which the type system cannot see and no runtime guard checks.

Guidance: docs/skills/freighter-best-practices/references/anti-patterns.md, "Non-null Assertions on Optional Data": "// WRONG: crashes silently if icons[asset] is undefined — const icon = icons[asset]!; ... Always handle the missing case explicitly."

Failure mode if the invariant ever breaks: a TypeError thrown from the emission block lands in the surrounding error handling and can misreport the outcome of an already-submitted transaction — the telemetry layer taking down the reporting of a successful send, which is the scenario the centralization in this PR was designed to avoid.

Suggested fixes (in increasing order of depth):

1. Narrow once per branch: at the top of each non-collectible emission block, guard and bind locals — if (!sourceIdentity || !snapshotHandle) return; const src = sourceIdentity; — so every subsequent use is assertion-free and a broken invariant degrades to "no telemetry emitted" instead of a throw.
2. Restructure: extract the swap/payment emission into a helper that takes non-null sourceIdentity/snapshotHandle (and destIdentity for swaps) as required parameters, so the null case is handled exactly once at the call site and the compiler enforces it everywhere else.

[CassioMG]

Partial price-fetch responses discard cached fallback prices

Low severity — telemetry-only gap on swaps whose destination token the price API doesn't cover; not a merge blocker.

TL;DR: When the confirmation-time price fetch succeeds but only covers some of the requested tokens, the snapshot keeps just the fresh response and throws away the cached display prices it was holding as a fallback — so the uncovered leg gets reported as unpriced even though a usable price was captured at confirmation. When the fetch fails outright, the code does fall back to those cached prices, so a partial success paradoxically yields worse coverage than a total failure. The code's own stated principle for the failure path is that coverage takes priority over freshness; merging fresh-first, cached-otherwise per token would apply that same principle to partial responses. (This differs from the earlier spot-price discussion — spot price is a different source; the cached display prices come from the same token-prices API, just captured earlier.)

Steps to reproduce:

1. Swap from a held token to a destination token the account doesn't hold and the token-prices endpoint doesn't cover, while the display cache has a price for it.
2. Let the swap succeed with the confirmation-time fetch resolving before submission finishes.
3. The emitted swap.completed event reports the destination leg with to_amount_usd_status: "no_price", despite a cached price having been available at confirmation.

---

Detailed explanation (for agents)

Root cause: in startConfirmationPriceSnapshot, resolve() returns pricesById: fetchedPrices wholesale when the fetch succeeded — no per-canonical-ID merge with cachedDisplayPrices for IDs the response omitted. The failure branch immediately below does use cachedDisplayPrices, making a partial success strictly worse than a total failure for the uncovered leg:

freighter/extension/src/helpers/confirmationPriceSnapshot.ts

Lines 74 to 99 in d486db9

	})
	.catch(() => {
	// Rejected (network error, non-2xx, or aborted): fall back to the
	// display-cache price at resolve() time rather than reporting the legs
	// unpriced — coverage takes priority over freshness.
	succeeded = false;
	});
	return {
	resolve: () => {
	if (succeeded) {
	return {
	pricesById: fetchedPrices,
	freshness: "confirmation_fetch",
	source,
	};
	}
	// Pending, rejected, or cancelled: abort so the request cannot outlive
	// the flow that needed it, and close on the display cache.
	controller.abort();
	return {
	pricesById: cachedDisplayPrices,
	freshness: "cached_display",
	source,
	};
	},

The catch block's comment states the intended priority — "coverage takes priority over freshness" — which the success branch doesn't honor for partially-covered responses.

Trigger path: getSwapDerivedData.ts shows /token-prices can lack entries for non-held destination tokens, so a swap to an unpopular token is a realistic partial-coverage case. The caller passes cachedDisplayPrices from tokenPricesSelector (same API source, older snapshot):

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

Lines 163 to 182 in d486db9

	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,
	})

Test gap: confirmationPriceSnapshot.test.ts covers full success and full failure, but not a partial success (request two IDs, receive one).

Suggested fixes (in increasing order of depth):

1. Per-ID merge: in the success branch, resolve each requested canonical ID as fresh value first, cached display value otherwise. Per-leg freshness provenance could then be reported instead of a single snapshot-level freshness if disambiguation matters downstream.
2. If intentional: keep the all-or-nothing behavior but document in resolve() why a partially-fresh snapshot must not mix freshness across legs, and add a partial-response test pinning it.

[Copilot]

Pull request overview

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

Suppressed comments (1)

extension/src/helpers/confirmationPriceSnapshot.ts:115

• On this fallback path, source describes the endpoint selected for the new confirmation request, not the endpoint that produced cachedDisplayPrices. Because the use_token_prices_v2 kill switch can change after the cache was populated, a cached v2 rate can be reported as token_prices_v1 (or vice versa). Carry the cached rate’s provenance with the cache, or omit/mark the source unknown when that provenance is unavailable.

      return {
        pricesById: cachedDisplayPrices,
        freshness: PriceFreshness.CachedDisplay,
        source,
      };

[Copilot]

Pull request overview

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

Suppressed comments (1)

@shared/api/internal.ts:768

• The new cancellation behavior is not exercised at this API boundary. The snapshot tests mock getTokenPrices, so they only verify that its signal is aborted—not that a pre-aborted v2 call skips fetchBackendV2, that an in-flight v2 result is rejected after abort, or that v1 forwards the signal to fetch. Please add those cases to @shared/api/__tests__/internal.test.ts so the cancellation guarantee cannot regress.

    if (signal?.aborted) {
      throw new DOMException("token-prices request aborted", "AbortError");

[Copilot]

Pull request overview

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

Suppressed comments (2)

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

• leg.value is already rounded to two decimals, so a real source value below $0.005 becomes 0 here and suppresses USD slippage even though the unrounded denominator is nonzero. Check unrounded.isZero() (or let computeUsdSlippagePct perform its existing zero check) so low-value swaps still report their percentage.

              sourceUsd.leg.value !== 0

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

• The confirmation price request starts even when data sharing is disabled, although every eventual emitMetric call is then guaranteed to no-op. This adds an unnecessary backend request for every opted-out payment/swap, and the v2 request cannot be cancelled once dispatched. Gate snapshot creation and its dependent telemetry branches on settingsDataSharingSelector.

      snapshotHandle = sourceIdentity
        ? startConfirmationPriceSnapshot({

[CassioMG]

New enums deviate from the repo's member-naming convention

Style / consistency — naming only, no behavior or telemetry impact; not a merge blocker.

TL;DR: The five new enums from the union-type conversion use PascalCase member names, while the project's style guide and every existing enum in the codebase use SCREAMING_SNAKE_CASE members. The string values are untouched, so nothing changes on the wire — this is purely about keeping the codebase's enum shape uniform, and it's cheapest to align now while the enums are brand new and only referenced within this PR's files.

---

Detailed explanation (for agents)

Convention: docs/skills/freighter-best-practices/references/code-style.md ("Enum Values") labels the pattern explicitly — "PascalCase enum with SCREAMING_SNAKE string values", e.g. export enum ActionStatus { IDLE = "IDLE", ... }. Every existing PascalCase-named enum in the repo conforms: ActionStatus, RequestState, AccountType, WalletType, NetworkCongestion all use SCREAMING_SNAKE members.

Deviating enums (all introduced by the union→enum conversion):

• LegUsdStatus (Ok, NoPrice, Error):

freighter/extension/src/helpers/usdVolume.ts

Lines 30 to 36 in 4dd8502

	export enum LegUsdStatus {
	Ok = "ok",
	NoPrice = "no_price",
	Error = "error",
	}

• AssetKind (Native, Classic, Soroban):

freighter/extension/src/helpers/usdVolume.ts

Lines 134 to 140 in 4dd8502

	export enum AssetKind {
	Native = "native",
	Classic = "classic",
	Soroban = "soroban",
	}

• FailureCategory (Slippage, Fee, Balance, Trustline, Destination, Sequence, Auth, Transport, ProtocolOther, Unknown):

freighter/extension/src/helpers/usdVolume.ts

Lines 218 to 231 in 4dd8502

	export enum FailureCategory {
	Slippage = "slippage",
	Fee = "fee",
	Balance = "balance",
	Trustline = "trustline",
	Destination = "destination",
	Sequence = "sequence",
	Auth = "auth",
	Transport = "transport",
	ProtocolOther = "protocol_other",
	Unknown = "unknown",
	}

• PriceSource (TokenPricesV1, TokenPricesV2) and PriceFreshness (ConfirmationFetch, CachedDisplay):

freighter/extension/src/helpers/confirmationPriceSnapshot.ts

Lines 4 to 14 in 4dd8502

	export enum PriceSource {
	TokenPricesV1 = "token_prices_v1",
	TokenPricesV2 = "token_prices_v2",
	}
	export enum PriceFreshness {
	ConfirmationFetch = "confirmation_fetch",
	CachedDisplay = "cached_display",
	}

Fix: rename members to SCREAMING_SNAKE (LegUsdStatus.NO_PRICE, FailureCategory.PROTOCOL_OTHER, PriceSource.TOKEN_PRICES_V1, …) keeping the string values exactly as they are. Mechanical rename across this PR's files only — the enums are new, so no call sites exist outside the PR.

[CassioMG]

This is looking good to me! Aside from the name convention nit

[JakeUrban]

Done in a9cd6a5 — confirmed the convention and renamed the members on all five enums.

Checked the guidance before applying: code-style.md's "Enum Values" section shows SCREAMING_SNAKE members under every naming style it lists, including the PascalCase-named ActionStatus example, and the existing enums all match (ActionStatus, RequestState, AccountType, WalletType, NetworkCongestion). So this was a straight deviation on my part, not a case worth documenting as deliberate.

LegUsdStatus, AssetKind, FailureCategory, PriceSource, PriceFreshness — members only; every string value is byte-for-byte unchanged, so the Amplitude wire contract is untouched. Worth noting the telemetry tests assert on those raw literals ("ok", "no_price", "confirmation_fetch", "protocol_other", …) and passed without modification, which pins the values independently of the enum members.

tsc clean, eslint clean, full suite green (219/219 suites, 1760 tests).

---

Generated by Claude Code

[Copilot]

Pull request overview

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

Suppressed comments (1)

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

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

• If signFreighterTransaction rejects, signedXDR remains "", but execution still dispatches submitFreighterTransaction below. This sends an invalid transaction, overwrites the original signing error with a submission error, and now emits a misleading payment/swap failure even though submission should never have been attempted. Return/throw immediately unless signing fulfilled with a non-empty payload before dispatching the submit thunk.

      let signedXDR = transactionSimulation.preparedTransaction ?? "";