Skip to content

Commit bdec8f7

Browse files
piyalbasuclaude
andauthored
feat(analytics): property-model foundation for cross-platform schema alignment (#2883) (#2903)
* docs(analytics): design spec for property-model foundation (#2883) First slice of the cross-platform analytics schema-alignment epic: global property-model cleanup (schema_version, account_id_hash, Identify traits, drop SDK-duplicated fields, app.opened snapshot). Evolves the existing emitMetric path in place; hard cutover, no dual-write. Event renames deferred to follow-on slices B/C. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(analytics): add memoized account_id_hash helper * feat(analytics): add popup/sidebar/fullpage surface detection * refactor(analytics): reshape common context into RFC four-bucket model * feat(analytics): supply appVersion to Amplitude init, keep autocapture off * feat(analytics): send durable wallet traits via Amplitude Identify Adds deriveIdentifyTraits (pure wallet_count/has_hardware_wallet/ has_imported_account) and syncIdentifyTraits (dirty-checked Amplitude Identify call), wired into storeAccountMetricsData. Also fixes a privacy violation found in review: storeBalanceMetricData emitted a truncated public key in the freighterAccountFunded event body. Replaced with account_id_hash of the full public key, matching the schema's "never emit a raw or truncated public key" constraint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(analytics): emit app.opened with one-time connectivity snapshot * test(analytics): guard against raw public keys in event payloads * test(analytics): fix jest.Mock cast to satisfy typecheck in privacy guard test * fix(analytics): record Identify fingerprint only after send guard syncIdentifyTraits cached the dirty-check fingerprint before the AMPLITUDE_KEY/hasInitialized guard, so a pre-init call would poison the cache without ever sending an Identify, causing a later identical call to silently short-circuit and never sync the user's traits. Move the cache write to after the guard so it's only recorded once an Identify actually fires. * docs(analytics): correct §6 Identify call-site list (accountServices duplicate) Final review found accountServices.ts uses a private duplicate storeAccountMetricsData that doesn't call syncIdentifyTraits; the effective sync sites are useGetAppData + the recover hook. Non-functional gap (traits self-heal via useGetAppData). Note added + dedupe follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(analytics): drop process-oriented design doc from repo The canonical analytics schema lives in the cross-platform RFC (stellar/wallet-eng-monorepo#10); this repo doc was a brainstorming artifact (slice refs, PR numbers, migration-decision narrative) that would rot as a second source of truth. Design context lives in the PR description; cross-platform contracts go to the RFC. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analytics): address PR review — consent-hydration timing + pre-unlock omission - app.opened: defer emit until the data-sharing preference resolves to allowed (it defaults false and hydrates async, so emitting at init was suppressed and lost). Emit once, when consent becomes allowed. - syncIdentifyTraits: gate on consent and don't cache the fingerprint while opted out, so traits re-sync after consent hydrates. - buildCommonContext: omit account_type/account_funded/is_hardware_account pre-unlock (no active key), per the spec's omission rule. - pt locale: real Portuguese for the auto-generated Auto-lock timer keys. - Tests: 22/22; adds consent-hydration regressions for app.opened + Identify. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analytics): derive account_funded from balances cache, not sticky flag account_funded was read from a per-account-type sticky map (accountFundedByType[metricsData.accountType], sourced from localStorage freighterFunded/hwFunded/importedFunded). Funding any one account of a given type latched that type "funded" for every subsequent event, even events for a different, still-unfunded account of the same type. Read it instead from the active account's cached balance (balancesSelector from popup/ducks/cache, keyed by network then public key), matching the cross-platform contract mobile already implements. Omit account_funded (tri-state, same as account_id_hash) when there is no cached entry for the active key, or when the cached isFunded is null (unknown), rather than defaulting to false. storeBalanceMetricData and the one-time freighterAccountFunded milestone event are untouched — they still use metricsData for that purpose. * fix(analytics): resolve account_type live from Redux, not the stale metricsData cache buildCommonContext derived account_type/is_hardware_account from the localStorage metricsData cache, which is only refreshed by useGetAppData (on a cache miss), makeAccountActive, and recoverAccount. Account-mutation thunks (importAccount, importHardwareWallet, addAccount, createAccount) switch the active account without refreshing it, so events emitted before the next full app-data reload mislabel the active account's type — e.g. a freshly-imported secret-key account reports account_type "freighter" (accountScreenImportAccount + the follow-on account view event both fire in this window). Resolve account_type/is_hardware_account live from the Redux account list keyed on the active public key, and omit them when the active key isn't resolvable in allAccounts (auth-store update race) rather than guessing — matching mobile's fail-safe behavior. account_id_hash and account_funded are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analytics): cache Identify fingerprint only after a successful send syncIdentifyTraits cached lastIdentifiedTraits before dispatching the Identify. If amplitude.identify() ever throws, the traits were already marked "sent", so every later call with the same traits short-circuits on the dirty-check and never retries — the same self-poisoning failure the pre-init/consent guards were hardened against. Reorder so the fingerprint is cached only after a successful dispatch, inside try/catch. On a throw, lastIdentifiedTraits stays unset so the next sync retries. Mirrors the mobile implementation (freighter-mobile#936). Adds a regression test asserting a one-off identify() throw does not suppress the next sync of the same traits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(analytics): screen.viewed consolidation (#2883) (#2907) * feat(analytics): add screen.viewed event and emitScreenViewed helper Introduce the canonical, consolidated screen-view event `screen.viewed` and the machinery to emit it, ahead of cutting every screen-load event over to it (Slice B of #2883). - Add METRIC_NAMES.screenViewed = "screen.viewed". - Add helpers/metrics#emitScreenViewed, which emits screen.viewed with a screen_name plus optional flow/step and any preserved extra props (surface and the rest of the common context come from Slice A's buildCommonContext). Undefined flow/step are dropped from the payload. - Add helpers/metrics#toScreenName: the deterministic canonicalization of a legacy "loaded screen: X" string into a snake_case screen_name, and the Flow union type. - Register emitScreenViewed in the global helpers/metrics test mock so component tests that render screens keep working after the cutover. - Cover toScreenName and emitScreenViewed in metrics.test.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1GhBXnrhTgwhopiq46ceq * feat(analytics): consolidate all screen-load events into screen.viewed Collapse every "loaded screen: X" event into the single canonical `screen.viewed` event (Slice B of #2883). Screen identity now lives in the `screen_name` property (derived deterministically from the legacy string), grouped by `flow`, with `step` on terminal completion/success screens; `surface` comes from the Slice-A common context. - popup/metrics/views.ts: refactor routeToEventName into a route -> { screen_name, flow, step? } map (single source of truth) and emit screen.viewed via emitScreenViewed, preserving the extra props on grant-access / add-token / sign-transaction / sign-auth-entry / sign-message. The modify-asset-list route keeps its non-screen (action) event untouched. - Cut the remaining screen-load emit sites over to emitScreenViewed: Send + Swap step maps, SwapAmount set-max, DisplayBackupPhrase, TrustlineError, and discover's trackDiscoverViewed. - Remove now-dead "loaded screen: X" name constants from metricsNames.ts (all unreferenced after the cutover); leave non-screen names intact. - Add popup/metrics/views.test.ts covering the route -> screen mapping, preserved props, completion-screen steps, and the no-legacy-name guarantee. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1GhBXnrhTgwhopiq46ceq * docs(analytics): remove internal slice-name references from comments * chore(analytics): drop unrelated files accidentally swept into this branch Removes files that were included by an errant 'git add -A' during an earlier commit (local .claude settings, .github parity/runbook infra, addtoken-sac e2e tests + pr-evidence, a pr-review note) and reverts a prettier-only reformat of the v3 manifest. Untracked here only — the files remain on disk for the branches they belong to. * refactor(analytics): remove unused toScreenName helper The route->screen map holds literal screen_name values as the single source of truth, so the toScreenName transform had no production caller (only its own test + a stale comment). Remove it and its test; reword the ScreenDef comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(analytics): correct stale legacy-string comments in screen maps The extension already identifies screens by declared screen_name literals (routeToScreen / per-step maps) with no 'loaded screen: X' plumbing. Fix the comments that described screen_name as 'derived from the legacy string' — the names are declared literals, not derived. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics(screen.viewed): reconcile cross-platform drift with mobile (D2–D6, D8) From the cross-platform drift review of RFC #2883 slice B: - D2: type `step` as the canonical Step enum {confirm, processing, success}; tag send_payment_confirm / swap_confirm step:"confirm" to match mobile. - D3: add flow:"assets" to the shared `account` and `view_public_key_generator` screens (align with mobile). - D4: canonicalize the reveal-phrase screen to `show_recovery_phrase` (+ `unlock_recovery_phrase` for the extension-only locked-state gate). - D5: reclassify the swap percentage/set-max button emit as an action event (`swap: amount percentage set`) instead of an inflating screen.viewed. - D6: skip + report to Sentry for an uncatalogued route instead of throwing inside the navigate handler (removes a popup-crash risk). - D8: drop the `send_payment` container screen (intentional non-emit); the send flow's step effect owns the per-step screens, matching mobile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(swap): stub emitScreenViewed in Swap.selectionType test The Swap view emits screen.viewed on mount; the real emitScreenViewed runs buildCommonContext, which reads the Redux auth slice this test's minimal store doesn't provide, throwing "Cannot read properties of undefined (reading 'publicKey')". The suite overrides the global metrics mock with requireActual + emitMetric only, so the real emit ran. Stub emitScreenViewed too — this suite covers picker-selection wiring, not screen-view analytics. Pre-existing since the screen.viewed cutover (fails identically at the parent commit); surfaced when CI ran on this branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics(swap): reuse shared set-max event for cross-platform parity The swap amount screen coined a new "swap: amount percentage set" event that fired on every percentage tap. Mobile has no such event — it emits the shared "send payment: set max" on the Max tap only, reused across send and swap. The extension Send handler already matched that; only Swap diverged. Point the swap Max tap at METRIC_NAMES.sendPaymentSetMax (gated on 100%) and drop the now-dead swapAmountPercentageSet constant, so all four call sites (ext send/swap, mobile send/swap) emit one consistent event. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(analytics): rename routeToScreen -> SCREEN_BY_ROUTE Naming consistency with the SEND_SCREEN_BY_STEP / SWAP_SCREEN_BY_STEP step maps. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(analytics): emit send_payment_processing for the in-flight send stage The extension's in-flight submission is an internal state of the confirm screen (submitStatus === PENDING → SendingTransaction), not a distinct step/route, so it never fired a screen.viewed. Mobile emits send_payment_processing (flow:send, step:processing) for this stage, so a cross-platform "processing" funnel had no extension data. Emit send_payment_processing (flow:"send", step:"processing") from a submitStatus effect in Send/index when a submission enters PENDING, once per submission (reset when the status clears). Swap is intentionally left alone — mobile emits no swap-processing event, so adding one would create new single-platform drift; deferred to the action/step follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * analytics(screen.viewed): emit send_payment_success on submission success Mirror the existing processing effect: when the send submission settles into ActionStatus.SUCCESS, emit send_payment_success (flow:"send", step:"success"), guarded to fire once per submission and reset on IDLE. Pairs with mobile (stellar/freighter-mobile#937), which emits the same send_payment_success from its processing screen's SENT state, so the send funnel confirm -> processing -> success is symmetric across both platforms rather than success being single-platform drift. Adds a matching test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(analytics): domain-event consolidation (#2883) (#2909) * feat(analytics): add screen.viewed event and emitScreenViewed helper Introduce the canonical, consolidated screen-view event `screen.viewed` and the machinery to emit it, ahead of cutting every screen-load event over to it (Slice B of #2883). - Add METRIC_NAMES.screenViewed = "screen.viewed". - Add helpers/metrics#emitScreenViewed, which emits screen.viewed with a screen_name plus optional flow/step and any preserved extra props (surface and the rest of the common context come from Slice A's buildCommonContext). Undefined flow/step are dropped from the payload. - Add helpers/metrics#toScreenName: the deterministic canonicalization of a legacy "loaded screen: X" string into a snake_case screen_name, and the Flow union type. - Register emitScreenViewed in the global helpers/metrics test mock so component tests that render screens keep working after the cutover. - Cover toScreenName and emitScreenViewed in metrics.test.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1GhBXnrhTgwhopiq46ceq * feat(analytics): consolidate all screen-load events into screen.viewed Collapse every "loaded screen: X" event into the single canonical `screen.viewed` event (Slice B of #2883). Screen identity now lives in the `screen_name` property (derived deterministically from the legacy string), grouped by `flow`, with `step` on terminal completion/success screens; `surface` comes from the Slice-A common context. - popup/metrics/views.ts: refactor routeToEventName into a route -> { screen_name, flow, step? } map (single source of truth) and emit screen.viewed via emitScreenViewed, preserving the extra props on grant-access / add-token / sign-transaction / sign-auth-entry / sign-message. The modify-asset-list route keeps its non-screen (action) event untouched. - Cut the remaining screen-load emit sites over to emitScreenViewed: Send + Swap step maps, SwapAmount set-max, DisplayBackupPhrase, TrustlineError, and discover's trackDiscoverViewed. - Remove now-dead "loaded screen: X" name constants from metricsNames.ts (all unreferenced after the cutover); leave non-screen names intact. - Add popup/metrics/views.test.ts covering the route -> screen mapping, preserved props, completion-screen steps, and the no-legacy-name guarantee. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1GhBXnrhTgwhopiq46ceq * docs(analytics): remove internal slice-name references from comments * feat(analytics): consolidate domain event catalog to shared grammar (#2883) Rewrite the domain (action/outcome) event names to the cross-platform `domain.action_past` grammar: a dotted domain prefix followed by a snake_case past-tense action. Outcomes get their own terminal events (completed/failed/rejected/blocked/submitted) and a `result` property is reserved for cases where the attempt itself is the analytical unit (Blockaid scans). Several legacy names collapse into single events keyed by a discriminator property (Blockaid scans, add-token responses, trustline removal failures, payment type selection). Adds constants for newly instrumented and split events and drops redundant ones. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VTPk4zPZ2GVdmTrmQCCckM * feat(analytics): map onboarding, account & recovery events (#2883) Point account creation, recovery-phrase, password, re-auth, recovery and import events at the new names, carrying `reason_code` on failures. Drop the redundant recover-account-finished and vague backup-phrase success/error events (completion screens are already covered by screen.viewed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VTPk4zPZ2GVdmTrmQCCckM * feat(analytics): map payment & swap outcomes, add collectible + submission events (#2883) Direct (non-routed) payment outcomes emit payment.completed/payment.failed; routed/path-payment outcomes settle as swap.completed/swap.failed, matching how the send flow already routes a destination-asset send through the swap path. Swap completion now carries from_asset_code/to_asset_code. Collectible sends get their own collectible_send.completed/failed terminal events, and a transaction.submitted event fires when a signed transaction is actually broadcast to the network (distinct from the signing approval). Failure events carry a reason_code derived from the operation/transaction result codes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VTPk4zPZ2GVdmTrmQCCckM * feat(analytics): rework signing & dApp-access events, split rejects from failures (#2883) Rename grant-access and signing approvals/rejections to the dapp_access.* and signing.* names, and separate user rejection from runtime failure: per-type reject handlers now emit signing.message_rejected and signing.auth_entry_rejected instead of a single generic event. The generic "user signed transaction" / "user cancelled signing flow" events are removed as duplicates of the per-type approve/reject handlers. The memo-required case emits signing.transaction_blocked with reason_code=memo_required. Add-token prompt responses collapse into asset_add.responded keyed by decision; the injected-API token events move to the asset_add_api.* names. Redundant per-handler account_type reads are dropped now that it rides on the shared common context. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VTPk4zPZ2GVdmTrmQCCckM * feat(analytics): consolidate asset, trustline & blockaid events (#2883) Trustline add/remove emit asset.added/asset.removed with asset_code; manage-asset errors emit asset.operation_failed with operation + reason_code; the three trustline-removal blockers collapse into trustline_remove.failed keyed by reason_code; the modify-asset-list event becomes asset_list.modified. The five Blockaid scan events consolidate into blockaid.scan_completed / blockaid.scan_failed, keyed by scan_target (domain | transaction | asset) with a coarse result. The remove-token prompt now emits asset_remove.responded (decision confirm | reject). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VTPk4zPZ2GVdmTrmQCCckM * feat(analytics): rename discovery, history, account-view & on-ramp events (#2883) Point discovery, history-item, account rename / public-key-copy / StellarExpert, first-funded and Coinbase on-ramp events at the new names. Discovery events carry protocol_id; history and account-rename events carry a source; the first-funded event moves to account.first_funded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VTPk4zPZ2GVdmTrmQCCckM * test(analytics): update metric-name assertions for the new grammar (#2883) Point the affected unit tests at the renamed constants and event names. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VTPk4zPZ2GVdmTrmQCCckM * chore(analytics): drop unrelated files accidentally swept into this branch Removes files that were included by an errant 'git add -A' during an earlier commit (local .claude settings, .github parity/runbook infra, addtoken-sac e2e tests + pr-evidence, a pr-review note) and reverts a prettier-only reformat of the v3 manifest. Untracked here only — the files remain on disk for the branches they belong to. * refactor(analytics): remove unused toScreenName helper The route->screen map holds literal screen_name values as the single source of truth, so the toScreenName transform had no production caller (only its own test + a stale comment). Remove it and its test; reword the ScreenDef comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(analytics): correct stale legacy-string comments in screen maps The extension already identifies screens by declared screen_name literals (routeToScreen / per-step maps) with no 'loaded screen: X' plumbing. Fix the comments that described screen_name as 'derived from the legacy string' — the names are declared literals, not derived. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analytics): narrow Blockaid scan result by status before reading is_malicious Slice C's blockaidScanCompleted metric read response.data.is_malicious without narrowing the SiteScanResponse union, which only carries that field on the status==='hit' variant (TS2339). Mirror the existing guard used elsewhere (status === 'hit' && is_malicious). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: cross-platform property-shape parity + deferred fixes Reconcile the extension analytics catalog with freighter-mobile against the shared cross-platform analytics schema, plus the deferred signing / history / dApp-access items. - Property shapes: asset_code+asset_issuer (drop legacy code/issuer); payment. completed asset_code; swap.* {from,to}_asset_code only; reason_code-only failures (drop raw error); import_method on account.imported/import_failed; recovery_method on account_recovery.completed; snake_case discover (protocol_name/is_known_protocol). - Blockaid result normalized to safe|warn|block|unknown; asset_bulk scan_target. - Remove transaction.submitted (extension has no dApp sign-and-submit path; internal broadcasts already covered by payment/swap/collectible_send.completed). - Wire runtime signing-failure events (signing.message_failed [new] + signing.auth_entry_failed) off the sign thunks' .rejected. - Signing origin: thread the dApp url through useSetupSigningFlow into the metric handlers; attach origin to dapp_access + signing events. - Wire account.public_key_copied (ViewPublicKey) and history.full_history_opened (AccountHeader nav). Contract-breaking wire/payload changes (hard cutover per #2883) — notify dashboard owners. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: address cross-platform drift review (blockaid coverage, collectible props, origin) - Emit blockaid.scan_failed from the transaction/asset/asset_bulk thrown-error catch blocks (previously only the response.error branch emitted), matching mobile's full failure coverage; skip AbortError cancellations. - asset_bulk scan_completed now carries an aggregate worst-case `result` + `address_count` (was scan_target only). - collectible_send.completed carries {collection_address, token_id}. - Normalize signing / dapp_access `origin` to the bare hostname (was the raw URL) so it matches mobile's dappDomain-based origin. - Document that transaction.submitted is reserved / never emitted on the extension (dApp API signs-and-returns; internal broadcasts are covered by the payment/swap/collectible_send .completed events). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: wire one-sided events + distinct report event + source discriminator - payment.simulation_failed now emitted from the simulation catch (was mobile-only); carries {reason_code, network}. - onboarding.recovery_phrase_viewed (recovery-phrase screen mount) and onboarding.completed (mnemonic confirm) now emitted, matching mobile. - blockaid.warning_reported: new distinct event for reportAssetWarning / reportTransactionWarning (were mislabeled as blockaid.scan_completed). - asset_add.responded carries source:"dapp_api" (extension emits it only for the dApp injected-API prompt) to distinguish from mobile's manual add. - Docs: onboarding.recovery_phrase_back_clicked not emitted (no Back affordance on the recovery-phrase screens); account.first_funded is extension-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: emit asset.operation_failed + trustline_remove.failed on the live fail path Both events were emitted only by the orphaned TrustlineError component (never rendered in production), so they were effectively dead on the extension while mobile emits them. Emit them from the live ChangeTrust submit fail branch (SubmitTx isFail), which knows add-vs-remove directly — so `operation` is never mislabeled (fixes the old envelope-inference "remove" default). trustline_remove .failed maps op_low_reserve->low_reserve and op_invalid_limit->has_balance (extension does not split buying_liabilities; mobile does — documented). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: complete trustline-failure parity; remove orphaned TrustlineError - trustline_remove.failed now splits op_invalid_limit into buying_liabilities vs has_balance using the asset's buying liabilities from the balance cache — full parity with mobile (was reporting has_balance only). - Delete the orphaned TrustlineError component (+ styles) and its two stale, skipped ManageAssets test cases: it was never rendered in production, and its asset.operation_failed / trustline_remove.failed emits are now live on the ChangeTrust submit fail path (SubmitTx). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: drift-review clear fixes (scrub, domain result, asset dims, source) - StrKey-scrub `reason_code` on the import / recovery / create-password / reauth and runtime signing-failure paths (secret-material hardening, matching mobile; new helpers/stellarStrKey.ts ported from mobile). - payment.simulation_failed: drop the hand-added `network` (it rides on buildCommonContext; hand-adding violates the catalog invariant). - asset_remove.responded (manual manage-assets prompt) carries source:"manage_assets". - trustline_remove.failed carries asset_code / asset_issuer. - blockaid.scan_completed{scan_target:"domain"} uses the full safe|warn|block|unknown mapping (was block/safe only), mirroring mobile's assessSiteSecurity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: drift-review decisions (asset_code on responded, token_code on scan) - asset_remove.responded (manual manage-assets prompt) carries asset_code (N3). - blockaid.scan_completed{scan_target:"asset"} carries token_code (N5), matching mobile; derived from the CODE-ISSUER scan address. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: round-4 drift fixes (blockaid abort guards, asset_code on asset_add.responded) - blockaid.scan_failed: skip emit on AbortError in the domain and transaction scan catches (matches scanAsset's guard) so cancelled scans don't report as failures. - asset_add.responded: thread asset_code from the add-token view through the add/reject dispatch (analytics-only arg on the thunks; read off action.meta.arg in the metrics handler), mirroring mobile's asset_add.responded { asset_code }. - Includes catalog/annotation cleanup carried in this round (dapp_access.blocked assertion, reserved-event notes, duplicate describe-block removal). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: scrub StrKeys on the remaining free-text reason_code paths Defense-in-depth to match the scrubbing the PR already applies on the auth/onboarding/import paths — Amplitude is a third-party sink not covered by Sentry's beforeSend, so a G…/S… StrKey embedded in an error string must be redacted before it leaves the client. - payment.simulation_failed (useSimulateTxData): scrub error.message. - asset_add_api.failed (useSetupAddTokenFlow): scrub the thunk-rejection message and the caught error message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: snake_case swap.trustline_added props (D5) Align swap.trustline_added to asset_code/asset_issuer (was tokenCode/tokenIssuer), matching the sibling asset.added and the shared snake_case property convention. Kept in lockstep with the identical mobile rename so the wire values stay aligned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: address PR review — scan_failed scrub, StrKey C/M, swap key parity Review comments from #2909 (paired with the mobile #938 changes): - blockaid.scan_failed reason_code now scrubbed at all six emit sites (four err.message catches + two response.error branches) — the last raw error paths. - scrubStrKeys now covers contract (C…, 56) and muxed (M…, 69) StrKeys, not just G…/S…; widened to tolerate null. (Muxed is 69 chars, so it's an alternation.) - swap.source_selected / destination_selected use snake_case asset_code / asset_issuer / requires_trustline; swap.quote_expired drops the raw amounts (privacy parity with completed/failed) and emits from_asset_code / to_asset_code as bare codes (getAssetFromCanonical) + result_code. Removed the now-dead amount params from useSwapQuoteExpiry. Kept in lockstep with mobile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * analytics: scrub the two remaining scan_failed catches (scanAsset / scanAssetBulk) Follow-up to the scan_failed scrub: the previous pass only caught the two hook catches (useScanSite/useScanTx) because the replace matched their deeper indentation; the scanAsset and scanAssetBulk catch blocks (shallower indent) still passed err.message raw. These asset paths are the likeliest to carry a C…/G… address in a thrown error, so scrub them too — all six scan_failed emit sites now route reason_code through scrubStrKeys. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ca05603 commit bdec8f7

52 files changed

Lines changed: 2529 additions & 1171 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

config/jest/setupTests.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ jest.mock("@amplitude/analytics-browser", () => ({
8686
jest.mock("helpers/metrics", () => ({
8787
registerHandler: () => {},
8888
emitMetric: () => {},
89+
emitScreenViewed: () => {},
8990
initAmplitude: () => {},
9091
getUserId: () => "test-user-id",
9192
metricsMiddleware: jest.fn(

extension/src/helpers/hooks/useGetOnrampToken.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ function useGetOnrampToken({ asset }: UseGetOnrampTokenParams) {
4545

4646
setTokenError("");
4747
const coinbaseUrl = getCoinbaseUrl({ token, asset });
48-
emitMetric(METRIC_NAMES.coinbaseOnrampOpened, { asset });
48+
emitMetric(METRIC_NAMES.onrampCoinbaseOpened, { asset });
4949

5050
openTab(coinbaseUrl);
5151
}
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
// config/jest/setupTests.tsx globally stubs "helpers/metrics" (and
2+
// "popup/App") for every test file so unrelated suites don't have to deal
3+
// with Amplitude/store internals. This file resolves to the same module
4+
// (by absolute path) via the relative "./metrics" import below, so we must
5+
// un-mock it here to exercise the real implementation.
6+
jest.unmock("helpers/metrics");
7+
8+
import * as amplitude from "@amplitude/analytics-browser";
9+
import * as Sentry from "@sentry/browser";
10+
11+
import { getAnalyticsUserId } from "@shared/api/internal";
12+
import { METRICS_USER_ID } from "constants/localStorageTypes";
13+
import {
14+
initAmplitude,
15+
reconcileAnalyticsUserId,
16+
resetAnalyticsUserIdReconciliation,
17+
} from "./metrics";
18+
19+
jest.mock("@amplitude/analytics-browser", () => ({
20+
init: jest.fn(),
21+
setUserId: jest.fn(),
22+
identify: jest.fn(),
23+
Identify: jest.fn().mockImplementation(() => ({ set: jest.fn() })),
24+
setOptOut: jest.fn(),
25+
track: jest.fn(),
26+
flush: jest.fn(),
27+
}));
28+
29+
jest.mock("@sentry/browser", () => ({
30+
setUser: jest.fn(),
31+
}));
32+
33+
jest.mock("@shared/api/internal", () => ({
34+
getAnalyticsUserId: jest.fn(),
35+
}));
36+
37+
jest.mock("popup/App", () => ({
38+
store: {
39+
getState: jest.fn().mockReturnValue({}),
40+
subscribe: jest.fn(),
41+
},
42+
}));
43+
44+
jest.mock("helpers/experimentClient", () => ({
45+
initExperimentClient: jest.fn(),
46+
}));
47+
48+
jest.mock("popup/ducks/settings", () => ({
49+
settingsDataSharingSelector: jest.fn().mockReturnValue(true),
50+
settingsNetworkDetailsSelector: jest.fn().mockReturnValue({
51+
network: "TESTNET",
52+
}),
53+
}));
54+
55+
jest.mock("popup/ducks/accountServices", () => ({
56+
publicKeySelector: jest.fn().mockReturnValue(""),
57+
}));
58+
59+
const mockGetAnalyticsUserId = getAnalyticsUserId as jest.Mock;
60+
61+
describe("reconcileAnalyticsUserId (auth id migration)", () => {
62+
beforeAll(() => {
63+
// Flip the module-level `hasInitialized` flag once so the
64+
// `hasInitialized && AMPLITUDE_KEY` guard in reconcileAnalyticsUserId
65+
// can be exercised. AMPLITUDE_KEY is stubbed truthy in the jest env
66+
// (see config/jest/setupTests.tsx).
67+
initAmplitude();
68+
});
69+
70+
beforeEach(() => {
71+
jest.clearAllMocks();
72+
localStorage.clear();
73+
// Clear the once-per-session guard so each case starts from an
74+
// un-reconciled session (mirrors a fresh unlock).
75+
resetAnalyticsUserIdReconciliation();
76+
});
77+
78+
it("overwrites a random persisted id with the auth id and re-identifies", async () => {
79+
localStorage.setItem(METRICS_USER_ID, "4873921"); // existing random id
80+
mockGetAnalyticsUserId.mockResolvedValue({
81+
analyticsUserId: "a".repeat(64),
82+
});
83+
84+
await reconcileAnalyticsUserId();
85+
86+
expect(localStorage.getItem(METRICS_USER_ID)).toBe("a".repeat(64));
87+
expect(amplitude.setUserId).toHaveBeenCalledWith("a".repeat(64));
88+
expect(Sentry.setUser).toHaveBeenCalledWith({ id: "a".repeat(64) });
89+
});
90+
91+
it("is a no-op when the persisted id already equals the auth id", async () => {
92+
localStorage.setItem(METRICS_USER_ID, "a".repeat(64));
93+
mockGetAnalyticsUserId.mockResolvedValue({
94+
analyticsUserId: "a".repeat(64),
95+
});
96+
97+
await reconcileAnalyticsUserId();
98+
99+
expect(amplitude.setUserId).not.toHaveBeenCalled();
100+
expect(Sentry.setUser).not.toHaveBeenCalled();
101+
});
102+
103+
it("is a no-op when locked (null auth id) — keeps the bootstrap id", async () => {
104+
localStorage.setItem(METRICS_USER_ID, "4873921");
105+
mockGetAnalyticsUserId.mockResolvedValue({ analyticsUserId: null });
106+
107+
await reconcileAnalyticsUserId();
108+
109+
expect(localStorage.getItem(METRICS_USER_ID)).toBe("4873921");
110+
expect(amplitude.setUserId).not.toHaveBeenCalled();
111+
expect(Sentry.setUser).not.toHaveBeenCalled();
112+
});
113+
114+
it("never throws into callers when the background message fails", async () => {
115+
localStorage.setItem(METRICS_USER_ID, "4873921");
116+
mockGetAnalyticsUserId.mockRejectedValue(new Error("no background"));
117+
118+
await expect(reconcileAnalyticsUserId()).resolves.toBeUndefined();
119+
expect(localStorage.getItem(METRICS_USER_ID)).toBe("4873921");
120+
expect(amplitude.setUserId).not.toHaveBeenCalled();
121+
});
122+
123+
it("does NOT call Sentry.setUser when data-sharing is off, but still persists + re-identifies amplitude", async () => {
124+
const { settingsDataSharingSelector } = jest.requireMock(
125+
"popup/ducks/settings",
126+
) as {
127+
settingsDataSharingSelector: jest.Mock;
128+
};
129+
settingsDataSharingSelector.mockReturnValue(false);
130+
131+
localStorage.setItem(METRICS_USER_ID, "4873921");
132+
mockGetAnalyticsUserId.mockResolvedValue({
133+
analyticsUserId: "b".repeat(64),
134+
});
135+
136+
await reconcileAnalyticsUserId();
137+
138+
expect(localStorage.getItem(METRICS_USER_ID)).toBe("b".repeat(64));
139+
expect(amplitude.setUserId).toHaveBeenCalledWith("b".repeat(64));
140+
expect(Sentry.setUser).not.toHaveBeenCalled();
141+
});
142+
143+
it("calls Sentry.setUser when data-sharing is on", async () => {
144+
const { settingsDataSharingSelector } = jest.requireMock(
145+
"popup/ducks/settings",
146+
) as {
147+
settingsDataSharingSelector: jest.Mock;
148+
};
149+
settingsDataSharingSelector.mockReturnValue(true);
150+
151+
localStorage.setItem(METRICS_USER_ID, "4873921");
152+
mockGetAnalyticsUserId.mockResolvedValue({
153+
analyticsUserId: "c".repeat(64),
154+
});
155+
156+
await reconcileAnalyticsUserId();
157+
158+
expect(Sentry.setUser).toHaveBeenCalledWith({ id: "c".repeat(64) });
159+
});
160+
161+
it("reconciles once per session: skips the background round-trip on repeat calls", async () => {
162+
localStorage.setItem(METRICS_USER_ID, "4873921");
163+
mockGetAnalyticsUserId.mockResolvedValue({
164+
analyticsUserId: "d".repeat(64),
165+
});
166+
167+
await reconcileAnalyticsUserId();
168+
await reconcileAnalyticsUserId();
169+
await reconcileAnalyticsUserId();
170+
171+
// Only the first call hits the (expensive) background handler.
172+
expect(mockGetAnalyticsUserId).toHaveBeenCalledTimes(1);
173+
expect(localStorage.getItem(METRICS_USER_ID)).toBe("d".repeat(64));
174+
});
175+
176+
it("does not latch the guard while locked, so a later unlock still reconciles", async () => {
177+
localStorage.setItem(METRICS_USER_ID, "4873921");
178+
// Locked: background returns a null auth id.
179+
mockGetAnalyticsUserId.mockResolvedValueOnce({ analyticsUserId: null });
180+
await reconcileAnalyticsUserId();
181+
expect(localStorage.getItem(METRICS_USER_ID)).toBe("4873921");
182+
183+
// Now unlocked: the auth id resolves and reconciliation proceeds.
184+
mockGetAnalyticsUserId.mockResolvedValue({
185+
analyticsUserId: "e".repeat(64),
186+
});
187+
await reconcileAnalyticsUserId();
188+
189+
expect(mockGetAnalyticsUserId).toHaveBeenCalledTimes(2);
190+
expect(localStorage.getItem(METRICS_USER_ID)).toBe("e".repeat(64));
191+
});
192+
193+
it("re-reconciles after the guard is reset on lock", async () => {
194+
mockGetAnalyticsUserId.mockResolvedValue({
195+
analyticsUserId: "f".repeat(64),
196+
});
197+
198+
await reconcileAnalyticsUserId();
199+
expect(mockGetAnalyticsUserId).toHaveBeenCalledTimes(1);
200+
201+
// Simulate a lock transition (SessionLockListener → SESSION_LOCKED).
202+
resetAnalyticsUserIdReconciliation();
203+
204+
await reconcileAnalyticsUserId();
205+
expect(mockGetAnalyticsUserId).toHaveBeenCalledTimes(2);
206+
});
207+
});

0 commit comments

Comments
 (0)