From be57d8df3c53c49041aaa644edaf96a7ebe29038 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Mon, 13 Jul 2026 20:03:49 -0400 Subject: [PATCH 01/15] 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) --- .../specs/ANALYTICS_SCHEMA_FOUNDATION.md | 184 ++++++++++++++++++ .../src/popup/locales/en/translation.json | 2 + .../src/popup/locales/pt/translation.json | 2 + 3 files changed, 188 insertions(+) create mode 100644 extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md diff --git a/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md b/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md new file mode 100644 index 0000000000..6bd5de5182 --- /dev/null +++ b/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md @@ -0,0 +1,184 @@ +# Analytics Schema Alignment — Property-Model Foundation (Design Spec) + +**Status:** Approved design, pre-implementation +**Date:** 2026-07-13 +**Owner:** piyal +**Tracking issue:** [stellar/freighter#2883](https://github.com/stellar/freighter/issues/2883) — Analytics refactor (extension): cross-platform schema alignment (Epic) +**Canonical schema (RFC):** [stellar/wallet-eng-monorepo#10](https://github.com/stellar/wallet-eng-monorepo/pull/10) — `analytics-refactor-report.md` +**Identity primitive (dependency, landing separately):** [stellar/freighter#2876](https://github.com/stellar/freighter/pull/2876) — derives the unified `user_id` from the seed phrase + +--- + +## 1. Context + +Issue #2883 is a 7-stream Epic to align the extension's Amplitude events with freighter-mobile on a shared `domain.action_past` schema (defined in the RFC), clean up the property model, and retire redundant events. The Epic is too large for a single spec, so it is decomposed into independently shippable slices. **This spec covers the first slice only.** + +The RFC is the canonical, cross-platform schema catalog (~50 events, four-bucket property model, naming rules). We adopt it **doc-defined + mirrored**: each repo (extension, mobile) implements its own catalog matching the RFC; the doc is the source of truth and the drift guard. No shared package, no codegen. + +### Key facts about the current system + +- Emission is centralized: `emitMetric(name, body)` in `extension/src/helpers/metrics.ts` → `amplitude.track`. +- Event names live in a flat `METRIC_NAMES` const map (`extension/src/popup/constants/metricsNames.ts`). +- Screen-load events funnel through a Redux middleware: a `registerHandler` on the `navigate` action maps route → event name in `extension/src/popup/metrics/views.ts`. +- `buildCommonContext(state)` attaches shared context to **every** event. +- Identity: `amplitude.setUserId(...)` is already called in `initAmplitude`; the value it uses is owned by the unified-user-ID work (#2876), **not** by this refactor. + +The current system works and has drawn no complaints. This slice **evolves it in place** — it does **not** re-architect it. + +--- + +## 2. Scope + +### In scope (this slice) — global, name-agnostic property-model foundation + +The property-model changes live in the **central** `emitMetric` / `buildCommonContext` path, so they apply to all events at once and cannot be done per-event. That makes them a natural first slice, independent of event renaming: + +1. `schema_version` stamped on every event (permanent `"2"` marker — see §4). +2. `account_id_hash` replaces the truncated `publicKey` on events (§5). +3. Stop hand-sending SDK-supplied device/app fields; rely on the SDK's built-in context (§4, Bucket 1). +4. Reclassify durable traits → Amplitude **Identify** user properties; keep volatile context event-level (§4, §6). +5. New `getSurface()` helper + `app.opened` event carrying the one-time snapshot (§7). + +Event **names stay legacy** in this slice. + +### Out of scope (follow-on slices) + +- **Slice B** — collapse `views.ts` route map into the canonical `screen.viewed` event. +- **Slice C** — domain-event renames / consolidations / removals (`payment.completed`, `blockaid.scan_completed` + `scan_target`, trustline/asset consolidations, swap-as-first-class, missing events, redundant-event deletion). +- **Slice D** — Amplitude account-level taxonomy cleanup (dashboard hygiene, no app code). Independent; can run in parallel. +- **Identity / `user_id`** — owned by #2876. Not touched here. + +--- + +## 3. Key decisions + +| Decision | Choice | Rationale | +| -------------------------- | ----------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Migration strategy | **Hard cutover per event, no dual-write** | Dual-write (RFC Part 4) doubles event volume against an already-full Amplitude project and adds transitional branching in the emit path. Parity is validated in staging/QA + tests before each flip instead of via prod count-overlap. | +| Schema source of truth | **Doc-defined, mirrored per repo** | Matches the team's current cross-repo workflow; lowest infra cost. `schema_version` is the drift tripwire. | +| Emission architecture | **Evolve `emitMetric` in place** (no typed-catalog rewrite) | RFC is a naming/property cleanup, not a re-architecture. A typed `track` catalog would churn every call site for compile-time safety the team is not asking for; the spec guards drift. | +| `schema_version` placement | **Global, stamped now** | One central change; the marker distinguishes new vs old property model in dashboards even while names are still legacy. | +| `wallet_count` definition | **`allAccounts.length`** | Confirm mobile matches before shipping (mobile may count seed-phrase wallets). | +| `app.opened` cadence | **Per popup/fullpage/sidebar mount** | Closest extension analog to mobile's foreground event; matches RFC funnel-denominator intent. | + +--- + +## 4. Property model — the four buckets + +All centralized in `emitMetric` + `buildCommonContext` (`extension/src/helpers/metrics.ts`). + +### Bucket 1 — SDK-supplied device/app metadata (stop hand-sending) + +- **Delete** `platform`, `platformVersion`, `appVersion` from `buildCommonContext`. +- The Amplitude Browser SDK's context plugin already attaches `platform` / `os_name` / `os_version` / `device_model` to every event. Pass `appVersion` once in the `amplitude.init(...)` config so the SDK attaches `app_version`. +- **Do NOT enable behavioral `autocapture`** (page views, clicks, form/element interactions, sessions). For a wallet UI that risks capturing DOM/URL content. This slice adds **no** new data-collection surface — it removes duplicated fields only. + +### Bucket 2 — Event-level volatile context (stays, reshaped) + +`buildCommonContext` keeps/adds, per event: + +- `network` (kept) +- `surface` — from `getSurface()` (new) +- `account_type`, `account_funded`, `is_hardware_account`, `account_id_hash` — the reclassified active-account fields +- **Removed:** truncated `publicKey` (→ `account_id_hash`), `connection_type` / `effective_type` (→ `app.opened` only) + +### Bucket 3 — Durable Identify user traits (new; see §6) + +`wallet_count`, `has_hardware_wallet`, `has_imported_account`, `bundle_id` (existing). + +### Bucket 4 — One-time `app.opened` snapshot (see §7) + +`surface`, `network`, `connection_type`, `effective_type`, `schema_version`. + +### Reclassification of the current per-event `metricsData` props + +| Current per-event prop | New home | +| --------------------------- | -------------------------------------------------------------------------- | +| `hw_connected` | Identify `has_hardware_wallet` | +| `secret_key_account` | Identify `has_imported_account` + event `account_type=imported_secret_key` | +| `freighter_account_funded` | event `account_funded` | +| `secret_key_account_funded` | split → event `account_type` + `account_funded` | + +--- + +## 5. `account_id_hash` derivation + +- **Algorithm:** synchronous SHA-256 of the full G-address, hex-encoded, via the Stellar SDK's `hash()` (already used in `extension/src/background/messageListener/handlers/signAuthEntry.ts`). +- **Memoized per public key** in a module-level cache — no async in the emit hot path, no per-event recompute. +- **Cross-platform:** identical input (G-address string) + identical algorithm (SHA-256 hex) on mobile ⇒ identical hash ⇒ account-level joins work across platforms. A committed test vector locks this (same approach #2876 used for its cross-platform vectors). +- **No active key (pre-unlock):** `account_id_hash` is omitted. Never emit a raw or truncated public key. + +--- + +## 6. Identify wiring + +- Extend the existing `amplitude.identify()` block in `initAmplitude`. +- Traits derive from the same `allAccounts` that `storeAccountMetricsData(publicKey, allAccounts)` already walks (call sites: `popup/ducks/accountServices.ts:192`, `helpers/hooks/useGetAppData.tsx:74`, `views/RecoverAccount/hooks/useGetRecoverAccountData.ts`): + - `has_hardware_wallet` ← `hwExists` + - `has_imported_account` ← `importedExists` + - `wallet_count` ← `allAccounts.length` + - `bundle_id` ← existing +- Fire `identify()` **only when a trait value actually changes** (dirty-check against the last-sent set) — Identify is for durable traits, not per-action. +- **Consent:** no new gate — the existing `setOptOut` subscription already suppresses Identify uploads when data-sharing is off. + +--- + +## 7. `app.opened` & `getSurface()` + +### `getSurface()` → `popup | sidebar | fullpage` + +- `sidebar`: synchronous via `isSidebarMode()` (`mode=sidebar` query param). +- `popup` vs `fullpage`: `browser.tabs.getCurrent()` (undefined ⇒ popup, defined ⇒ fullpage) is async, so **resolve once at init and cache in a module variable**; `buildCommonContext` reads it synchronously thereafter. + +### `app.opened` + +- Fires once in the init path (where `initAmplitude` runs — `extension/src/popup/App.tsx:48`), i.e. **per popup/fullpage/sidebar mount**. +- Payload (one-time snapshot): `surface`, `network`, `connection_type`, `effective_type`, `schema_version`. +- `connection_type` / `effective_type` are **removed** from `buildCommonContext` at the same time, so they ride only on `app.opened`. + +--- + +## 8. Migration sequencing & interim state + +1. Land this slice: global property-model changes + Identify + `app.opened` + `account_id_hash` + `getSurface`. **Names remain legacy.** +2. After this slice, events carry the **new property model + `schema_version: "2"` under their legacy names** until slices B/C rename them. Given hard cutover (no dual-write) and that dashboards are being rebuilt regardless, this is acceptable; `schema_version` is exactly the flag that lets a dashboard distinguish the new property model from the old. +3. Slice B renames screen events → `screen.viewed`. Slice C renames/consolidates/removes domain events. Each rename is a hard cutover. + +--- + +## 9. Error handling & edge cases + +- **Pre-unlock / no active key:** `account_id_hash`, `account_type`, `account_funded`, `is_hardware_account` are omitted. Welcome/onboarding events carry none. No crash, no placeholder. +- **Hashing:** `hash()` is sync + deterministic on a valid G-address; wrap defensively and omit `account_id_hash` on error rather than throwing in the emit path. +- **Consent gate:** unchanged — `emitMetric` early-returns when data-sharing is off; Identify covered by `setOptOut`. +- **Init failure / missing `AMPLITUDE_KEY`:** unchanged — existing `try/catch` + console-log fallback preserved. +- **`browser.tabs.getCurrent()` unavailable / throws:** default `surface` to `popup`. + +--- + +## 10. Testing + +- **Unit (`extension/src/helpers/metrics.test.ts` — extend existing):** + - `buildCommonContext` no longer emits `platform` / `platformVersion` / `appVersion` / `publicKey` / `connection_type` / `effective_type`. + - `buildCommonContext` emits `account_id_hash`, `account_type`, `account_funded`, `is_hardware_account`, `surface`, `schema_version`. + - `account_id_hash` is deterministic and matches a committed cross-platform vector. + - Identify fires only on trait change. + - Data-sharing off ⇒ no `track`, no `identify`. +- **`app.opened`** fires once per init with the snapshot fields; those fields are absent from subsequent events. +- **Regression guard:** a test asserting **no** event payload contains a raw or truncated public key. +- Existing metric tests updated to the reshaped payload. + +--- + +## 11. Open items to confirm before shipping + +- [ ] **`wallet_count` parity:** confirm mobile also defines it as account count (not seed-phrase-wallet count). Resolved default here: `allAccounts.length`. +- [ ] Confirm the committed `account_id_hash` vector is mirrored by freighter-mobile (same G-address → same hash). + +--- + +## 12. Follow-on slices (not this spec) + +- **B** — `screen.viewed` consolidation (`views.ts` route map). +- **C** — domain-event renames/consolidations/removals + missing events (`swap.completed`/`.failed`, `transaction.submitted`, `collectible_send.*`, `asset_remove.responded`, transaction-details `screen.viewed` parity). +- **D** — Amplitude taxonomy cleanup (dashboard hygiene; runs independently, prioritized to free property headroom). diff --git a/extension/src/popup/locales/en/translation.json b/extension/src/popup/locales/en/translation.json index 52339ec4bb..0ddd30cdd0 100644 --- a/extension/src/popup/locales/en/translation.json +++ b/extension/src/popup/locales/en/translation.json @@ -77,6 +77,8 @@ "Authorizations": "Authorizations", "Authorize": "Authorize", "Authorized address": "Authorized address", + "Auto-lock timer": "Auto-lock timer", + "Auto-Lock Timer": "Auto-Lock Timer", "available": "available", "Back": "Back", "Balance": "Balance", diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index db2c569c1c..a0cb327877 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -77,6 +77,8 @@ "Authorizations": "Autorizações", "Authorize": "Autorizar", "Authorized address": "Endereço autorizado", + "Auto-lock timer": "Auto-lock timer", + "Auto-Lock Timer": "Auto-Lock Timer", "available": "disponível", "Back": "Voltar", "Balance": "Saldo", From b648480f2fe8b5d3e1c16b44745dbda78c10d7e2 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Mon, 13 Jul 2026 20:36:52 -0400 Subject: [PATCH 02/15] feat(analytics): add memoized account_id_hash helper --- extension/src/helpers/metrics.test.ts | 48 +++++++++++++++++++++++++++ extension/src/helpers/metrics.ts | 20 +++++++++++ 2 files changed, 68 insertions(+) create mode 100644 extension/src/helpers/metrics.test.ts diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts new file mode 100644 index 0000000000..5495a58183 --- /dev/null +++ b/extension/src/helpers/metrics.test.ts @@ -0,0 +1,48 @@ +// config/jest/setupTests.tsx globally auto-mocks "helpers/metrics" (so +// unrelated component/reducer tests don't have to deal with Amplitude etc.). +// This file tests the real implementation, so opt back out of that stub. +jest.unmock("helpers/metrics"); + +// Shared mocks — metrics.ts couples to the store, the SDK, and selectors at +// module load, so isolate all of them here. Individual tests set behavior. +jest.mock("@amplitude/analytics-browser"); +jest.mock("popup/App", () => ({ + store: { getState: jest.fn(() => ({})), subscribe: jest.fn() }, +})); +jest.mock("popup/ducks/accountServices", () => ({ + publicKeySelector: jest.fn(), +})); +jest.mock("popup/ducks/settings", () => ({ + settingsDataSharingSelector: jest.fn(() => true), + settingsNetworkDetailsSelector: jest.fn(() => ({ network: "TESTNET" })), +})); +jest.mock("helpers/experimentClient", () => ({ + initExperimentClient: jest.fn(), +})); +jest.mock("constants/env", () => ({ + AMPLITUDE_KEY: "test-key", + APP_VERSION: "9.9.9", + METRICS_PLATFORM: "WEB", +})); + +import { getAccountIdHash } from "helpers/metrics"; + +describe("getAccountIdHash", () => { + const PUBLIC_KEY = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; + const EXPECTED = + "f56f6f2c6cf1b9388e3495dfab96f0c55ec5d217f481b2ae45d11b46145c44ef"; + + it("returns the lowercase hex SHA-256 of the G-address (cross-platform vector)", () => { + expect(getAccountIdHash(PUBLIC_KEY)).toBe(EXPECTED); + }); + + it("is deterministic and 64 hex chars", () => { + const h = getAccountIdHash(PUBLIC_KEY); + expect(h).toBe(getAccountIdHash(PUBLIC_KEY)); + expect(h).toMatch(/^[0-9a-f]{64}$/); + }); + + it("differs for different keys", () => { + expect(getAccountIdHash("GABC")).not.toBe(getAccountIdHash("GXYZ")); + }); +}); diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index 1b7cfd7068..c7de8db8c7 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -2,6 +2,7 @@ import * as amplitude from "@amplitude/analytics-browser"; import { Action, Middleware } from "redux"; import { PayloadAction } from "@reduxjs/toolkit"; import { Location } from "react-router-dom"; +import { hash } from "stellar-sdk"; import browser from "webextension-polyfill"; @@ -274,6 +275,25 @@ const getCoarsenedUserAgent = (): string => { return "Unknown"; }; +/** + * Cross-platform account identifier: lowercase hex SHA-256 of the full + * G-address string. Never emit a raw/truncated public key. Memoized per key + * so the hot emit path stays synchronous and does no repeat work. Mobile must + * hash the same G-address string with SHA-256 to produce a matching value. + */ +const accountIdHashCache = new Map(); +export const getAccountIdHash = (publicKey: string): string => { + const cached = accountIdHashCache.get(publicKey); + if (cached) return cached; + try { + const digest = hash(Buffer.from(publicKey, "utf8")).toString("hex"); + accountIdHashCache.set(publicKey, digest); + return digest; + } catch { + return ""; + } +}; + /** * Builds common context data attached to every event. * Mirrors the mobile app's context properties for consistency across platforms. From 3c2cadfa70026591a25006b1a7826345d7badb1e Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 10:52:39 -0400 Subject: [PATCH 03/15] feat(analytics): add popup/sidebar/fullpage surface detection --- extension/src/helpers/metrics.test.ts | 42 ++++++++++++++++++++++++++- extension/src/helpers/metrics.ts | 25 ++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index 5495a58183..cf8705dffc 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -24,8 +24,17 @@ jest.mock("constants/env", () => ({ APP_VERSION: "9.9.9", METRICS_PLATFORM: "WEB", })); +jest.mock("popup/helpers/isSidebarMode", () => ({ + isSidebarMode: jest.fn(() => false), +})); +jest.mock("webextension-polyfill", () => ({ + tabs: { getCurrent: jest.fn() }, + runtime: { getManifest: jest.fn(() => ({ version: "9.9.9" })) }, +})); -import { getAccountIdHash } from "helpers/metrics"; +import { getAccountIdHash, getSurface, resolveSurface } from "helpers/metrics"; +import { isSidebarMode } from "popup/helpers/isSidebarMode"; +import browser from "webextension-polyfill"; describe("getAccountIdHash", () => { const PUBLIC_KEY = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; @@ -46,3 +55,34 @@ describe("getAccountIdHash", () => { expect(getAccountIdHash("GABC")).not.toBe(getAccountIdHash("GXYZ")); }); }); + +describe("getSurface", () => { + beforeEach(() => jest.clearAllMocks()); + + it("returns 'sidebar' when in sidebar mode", async () => { + (isSidebarMode as jest.Mock).mockReturnValue(true); + await resolveSurface(); + expect(getSurface()).toBe("sidebar"); + }); + + it("returns 'fullpage' when opened in a tab", async () => { + (isSidebarMode as jest.Mock).mockReturnValue(false); + (browser.tabs.getCurrent as jest.Mock).mockResolvedValue({ id: 1 }); + await resolveSurface(); + expect(getSurface()).toBe("fullpage"); + }); + + it("returns 'popup' when not a tab", async () => { + (isSidebarMode as jest.Mock).mockReturnValue(false); + (browser.tabs.getCurrent as jest.Mock).mockResolvedValue(undefined); + await resolveSurface(); + expect(getSurface()).toBe("popup"); + }); + + it("defaults to 'popup' if getCurrent throws", async () => { + (isSidebarMode as jest.Mock).mockReturnValue(false); + (browser.tabs.getCurrent as jest.Mock).mockRejectedValue(new Error("x")); + await resolveSurface(); + expect(getSurface()).toBe("popup"); + }); +}); diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index c7de8db8c7..1ae0f4b9e5 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -13,6 +13,7 @@ import { initExperimentClient } from "helpers/experimentClient"; import { BUNDLE_ID_USER_PROPERTY_KEY, getBundleId } from "helpers/analytics"; import { isDev } from "@shared/helpers/dev"; import { truncatedPublicKey } from "helpers/stellar"; +import { isSidebarMode } from "popup/helpers/isSidebarMode"; import { settingsDataSharingSelector, settingsNetworkDetailsSelector, @@ -233,6 +234,30 @@ export const initAmplitude = () => { // Common context (mirrors mobile's buildCommonContext) // --------------------------------------------------------------------------- +export type Surface = "popup" | "sidebar" | "fullpage"; + +// Resolved once at init: browser.tabs.getCurrent() is async, but the emit path +// must be synchronous, so cache the surface in a module variable. +let cachedSurface: Surface | null = null; + +/** Resolve and cache the surface. Call once during init, before app.opened. */ +export const resolveSurface = async (): Promise => { + if (isSidebarMode()) { + cachedSurface = "sidebar"; + return; + } + try { + const tab = await browser.tabs.getCurrent(); + cachedSurface = tab ? "fullpage" : "popup"; + } catch { + cachedSurface = "popup"; + } +}; + +/** Synchronous surface accessor for the emit path. */ +export const getSurface = (): Surface => + cachedSurface ?? (isSidebarMode() ? "sidebar" : "popup"); + /** * Returns the extension version. Prefers the build-time constant (always * available via DefinePlugin), falling back to the browser extension manifest. From 01750f6f94fec1874de2dd431cb57e3fe78fb278 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 11:57:12 -0400 Subject: [PATCH 04/15] refactor(analytics): reshape common context into RFC four-bucket model --- extension/src/helpers/metrics.test.ts | 90 ++++++++++++++++++++++- extension/src/helpers/metrics.ts | 100 ++++++++------------------ 2 files changed, 117 insertions(+), 73 deletions(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index cf8705dffc..9bdc388d3d 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -32,9 +32,18 @@ jest.mock("webextension-polyfill", () => ({ runtime: { getManifest: jest.fn(() => ({ version: "9.9.9" })) }, })); -import { getAccountIdHash, getSurface, resolveSurface } from "helpers/metrics"; +import { + getAccountIdHash, + getSurface, + resolveSurface, + buildCommonContext, +} from "helpers/metrics"; import { isSidebarMode } from "popup/helpers/isSidebarMode"; import browser from "webextension-polyfill"; +import { publicKeySelector } from "popup/ducks/accountServices"; +import { settingsNetworkDetailsSelector } from "popup/ducks/settings"; +import { METRICS_DATA } from "constants/localStorageTypes"; +import { AccountType } from "@shared/api/types"; describe("getAccountIdHash", () => { const PUBLIC_KEY = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; @@ -86,3 +95,82 @@ describe("getSurface", () => { expect(getSurface()).toBe("popup"); }); }); + +describe("buildCommonContext (four-bucket property model)", () => { + const PUBLIC_KEY = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; + + beforeEach(() => { + jest.clearAllMocks(); + localStorage.clear(); + (settingsNetworkDetailsSelector as unknown as jest.Mock).mockReturnValue({ + network: "TESTNET", + }); + (publicKeySelector as unknown as jest.Mock).mockReturnValue(PUBLIC_KEY); + localStorage.setItem( + METRICS_DATA, + JSON.stringify({ + accountType: AccountType.IMPORTED, + hwExists: false, + importedExists: true, + hwFunded: false, + importedFunded: true, + freighterFunded: false, + unfundedFreighterAccounts: [], + }), + ); + }); + + it("stamps schema_version '2'", () => { + expect(buildCommonContext({} as never).schema_version).toBe("2"); + }); + + it("emits the reshaped event-level bucket", () => { + const ctx = buildCommonContext({} as never); + expect(ctx).toMatchObject({ + network: "TESTNET", + account_type: "imported_secret_key", + account_funded: true, + is_hardware_account: false, + account_id_hash: + "f56f6f2c6cf1b9388e3495dfab96f0c55ec5d217f481b2ae45d11b46145c44ef", + }); + expect(ctx.surface).toBeDefined(); + }); + + it("drops SDK-supplied and legacy fields", () => { + const ctx = buildCommonContext({} as never); + expect(ctx).not.toHaveProperty("platform"); + expect(ctx).not.toHaveProperty("platformVersion"); + expect(ctx).not.toHaveProperty("appVersion"); + expect(ctx).not.toHaveProperty("publicKey"); + expect(ctx).not.toHaveProperty("connectionType"); + expect(ctx).not.toHaveProperty("effectiveType"); + }); + + it("omits account_id_hash when there is no active key (pre-unlock)", () => { + (publicKeySelector as unknown as jest.Mock).mockReturnValue(""); + expect(buildCommonContext({} as never)).not.toHaveProperty( + "account_id_hash", + ); + }); + + it("marks hardware active account", () => { + localStorage.setItem( + METRICS_DATA, + JSON.stringify({ + accountType: AccountType.HW, + hwExists: true, + importedExists: false, + hwFunded: true, + importedFunded: false, + freighterFunded: false, + unfundedFreighterAccounts: [], + }), + ); + expect(buildCommonContext({} as never)).toMatchObject({ + account_type: "hardware", + is_hardware_account: true, + account_funded: true, + }); + }); +}); diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index 1ae0f4b9e5..0993ebc1d7 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -8,7 +8,7 @@ import browser from "webextension-polyfill"; import { store } from "popup/App"; import { METRICS_DATA, METRICS_USER_ID } from "constants/localStorageTypes"; -import { AMPLITUDE_KEY, METRICS_PLATFORM, APP_VERSION } from "constants/env"; +import { AMPLITUDE_KEY } from "constants/env"; import { initExperimentClient } from "helpers/experimentClient"; import { BUNDLE_ID_USER_PROPERTY_KEY, getBundleId } from "helpers/analytics"; import { isDev } from "@shared/helpers/dev"; @@ -107,6 +107,16 @@ let hasInitialized = false; */ const AMPLITUDE_FLUSH_INTERVAL_MS = 500; +/** Schema generation marker for the new cross-platform property model. */ +export const SCHEMA_VERSION = "2"; + +/** Maps the internal account type to the RFC's wire value for `account_type`. */ +const ACCOUNT_TYPE_WIRE: Record = { + [AccountType.FREIGHTER]: "freighter", + [AccountType.HW]: "hardware", + [AccountType.IMPORTED]: "imported_secret_key", +}; + // --------------------------------------------------------------------------- // User identity (mirrors mobile's src/services/analytics/user.ts) // --------------------------------------------------------------------------- @@ -258,48 +268,6 @@ export const resolveSurface = async (): Promise => { export const getSurface = (): Surface => cachedSurface ?? (isSidebarMode() ? "sidebar" : "popup"); -/** - * Returns the extension version. Prefers the build-time constant (always - * available via DefinePlugin), falling back to the browser extension manifest. - */ -const getAppVersion = (): string => { - if (APP_VERSION) return APP_VERSION; - - try { - return browser.runtime.getManifest().version; - } catch { - return "unknown"; - } -}; - -/** - * Extracts a coarsened browser identifier from the user agent string. - * Returns "BrowserName/MajorVersion" (e.g. "Chrome/120", "Firefox/121"). - * Falls back to "Unknown" if parsing fails. - */ -const getCoarsenedUserAgent = (): string => { - const ua = navigator.userAgent; - - // Order matters: check more specific browsers first. - // Edge includes "Chrome" in its UA, so check Edge before Chrome. - const patterns: Array<[RegExp, string]> = [ - [/Edg(?:e|A|iOS)?\/(\d+)/, "Edge"], - [/OPR\/(\d+)/, "Opera"], - [/Firefox\/(\d+)/, "Firefox"], - [/(?:Chrome|CriOS)\/(\d+)/, "Chrome"], - [/Version\/(\d+).*Safari/, "Safari"], - ]; - - for (const [regex, name] of patterns) { - const match = ua.match(regex); - if (match) { - return `${name}/${match[1]}`; - } - } - - return "Unknown"; -}; - /** * Cross-platform account identifier: lowercase hex SHA-256 of the full * G-address string. Never emit a raw/truncated public key. Memoized per key @@ -320,41 +288,35 @@ export const getAccountIdHash = (publicKey: string): string => { }; /** - * Builds common context data attached to every event. - * Mirrors the mobile app's context properties for consistency across platforms. - * - * | Mobile property | Extension equivalent | - * |-------------------|----------------------------------------------| - * | Platform.OS | METRICS_PLATFORM ("WEB") | - * | Platform.Version | coarsened user agent (e.g. "Chrome/120") | - * | getVersion() | manifest version / APP_VERSION | - * | getBundleId() | "extension." | - * - * @param state Current Redux state (passed in to avoid a redundant getState() call). + * Builds the event-level "volatile context" bucket attached to every event, + * plus schema_version. Durable traits live in Identify (see syncIdentifyTraits); + * device/app metadata comes from the Amplitude SDK; connectivity is on app.opened. */ -const buildCommonContext = ( +export const buildCommonContext = ( state: ReturnType, ): Record => { const activePublicKey = publicKeySelector(state); const networkDetails = settingsNetworkDetailsSelector(state); + const metricsData = getMetricsData(); - // Navigator.connection is not available in all browsers - const nav = navigator as Navigator & { - connection?: { type?: string; effectiveType?: string }; + const accountFundedByType: Record = { + [AccountType.FREIGHTER]: metricsData.freighterFunded, + [AccountType.HW]: metricsData.hwFunded, + [AccountType.IMPORTED]: metricsData.importedFunded, }; const context: Record = { - publicKey: activePublicKey ? truncatedPublicKey(activePublicKey) : "N/A", - platform: METRICS_PLATFORM, - platformVersion: getCoarsenedUserAgent(), + schema_version: SCHEMA_VERSION, + surface: getSurface(), network: networkDetails?.network ?? "UNKNOWN", - connectionType: nav.connection?.type ?? "unknown", - appVersion: getAppVersion(), - bundleId: getBundleId(), + account_type: ACCOUNT_TYPE_WIRE[metricsData.accountType], + account_funded: accountFundedByType[metricsData.accountType], + is_hardware_account: metricsData.accountType === AccountType.HW, }; - if (nav.connection?.effectiveType) { - context.effectiveType = nav.connection.effectiveType; + if (activePublicKey) { + const idHash = getAccountIdHash(activePublicKey); + if (idHash) context.account_id_hash = idHash; } return context; @@ -402,15 +364,9 @@ const getMetricsData = (): MetricsData => { export const emitMetric = (name: string, body?: Record) => { const state = store.getState(); - const metricsData = getMetricsData(); - const eventProperties = { ...buildCommonContext(state), ...body, - freighter_account_funded: metricsData.freighterFunded, - hw_connected: metricsData.hwExists, - secret_key_account: metricsData.importedExists, - secret_key_account_funded: metricsData.importedFunded, }; const isDataSharingAllowed = settingsDataSharingSelector(state); From a6d4381a1a395bc90a6edf3da418e694c4c087f8 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 12:14:38 -0400 Subject: [PATCH 05/15] feat(analytics): supply appVersion to Amplitude init, keep autocapture off --- extension/src/helpers/metrics.test.ts | 23 +++++++++++++++++++++++ extension/src/helpers/metrics.ts | 3 ++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index 9bdc388d3d..898974dce1 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -32,6 +32,7 @@ jest.mock("webextension-polyfill", () => ({ runtime: { getManifest: jest.fn(() => ({ version: "9.9.9" })) }, })); +import * as amplitude from "@amplitude/analytics-browser"; import { getAccountIdHash, getSurface, @@ -174,3 +175,25 @@ describe("buildCommonContext (four-bucket property model)", () => { }); }); }); + +describe("initAmplitude SDK config", () => { + beforeEach(() => jest.clearAllMocks()); + + it("passes appVersion so the SDK attaches app_version, with autocapture off", async () => { + // initAmplitude guards on a module-level `hasInitialized` flag, so isolate + // the module here to ensure this test is independent of init having + // already run in another describe block. + let mod: typeof import("helpers/metrics"); + jest.isolateModules(() => { + mod = require("helpers/metrics"); + }); + await mod!.initAmplitude(); + expect( + (require("@amplitude/analytics-browser") as typeof amplitude).init, + ).toHaveBeenCalledWith( + "test-key", + undefined, + expect.objectContaining({ appVersion: "9.9.9", autocapture: false }), + ); + }); +}); diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index 0993ebc1d7..0dbf5c2c1b 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -8,7 +8,7 @@ import browser from "webextension-polyfill"; import { store } from "popup/App"; import { METRICS_DATA, METRICS_USER_ID } from "constants/localStorageTypes"; -import { AMPLITUDE_KEY } from "constants/env"; +import { AMPLITUDE_KEY, APP_VERSION } from "constants/env"; import { initExperimentClient } from "helpers/experimentClient"; import { BUNDLE_ID_USER_PROPERTY_KEY, getBundleId } from "helpers/analytics"; import { isDev } from "@shared/helpers/dev"; @@ -188,6 +188,7 @@ export const initAmplitude = () => { // generate a UUID deviceId and persist it across sessions. identityStorage: "localStorage", autocapture: false, + appVersion: APP_VERSION || undefined, // The extension popup can close at any time; reduce the flush interval // so queued events are sent promptly instead of waiting the default 1 s. flushIntervalMillis: AMPLITUDE_FLUSH_INTERVAL_MS, From c29ca1e240feb1a7b104645cbd4e546c8656bff4 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 12:27:57 -0400 Subject: [PATCH 06/15] 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) --- extension/src/helpers/metrics.test.ts | 70 +++++++++++++++++++++++++++ extension/src/helpers/metrics.ts | 41 +++++++++++++++- 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index 898974dce1..d537f84e92 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -38,6 +38,9 @@ import { getSurface, resolveSurface, buildCommonContext, + deriveIdentifyTraits, + storeBalanceMetricData, + initAmplitude, } from "helpers/metrics"; import { isSidebarMode } from "popup/helpers/isSidebarMode"; import browser from "webextension-polyfill"; @@ -45,6 +48,8 @@ import { publicKeySelector } from "popup/ducks/accountServices"; import { settingsNetworkDetailsSelector } from "popup/ducks/settings"; import { METRICS_DATA } from "constants/localStorageTypes"; import { AccountType } from "@shared/api/types"; +import { truncatedPublicKey } from "helpers/stellar"; +import { METRIC_NAMES } from "popup/constants/metricsNames"; describe("getAccountIdHash", () => { const PUBLIC_KEY = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; @@ -176,6 +181,71 @@ describe("buildCommonContext (four-bucket property model)", () => { }); }); +describe("deriveIdentifyTraits", () => { + it("counts accounts and detects hardware/imported presence", () => { + const accounts = [ + { publicKey: "G1", hardwareWalletType: "ledger", imported: false }, + { publicKey: "G2", hardwareWalletType: "", imported: true }, + { publicKey: "G3", hardwareWalletType: "", imported: false }, + ] as never; + expect(deriveIdentifyTraits(accounts)).toEqual({ + wallet_count: 3, + has_hardware_wallet: true, + has_imported_account: true, + }); + }); + + it("reports zero/false for an empty account list", () => { + expect(deriveIdentifyTraits([])).toEqual({ + wallet_count: 0, + has_hardware_wallet: false, + has_imported_account: false, + }); + }); +}); + +describe("storeBalanceMetricData (privacy)", () => { + const PUBLIC_KEY = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; + + beforeEach(() => { + jest.clearAllMocks(); + localStorage.clear(); + (settingsNetworkDetailsSelector as unknown as jest.Mock).mockReturnValue({ + network: "TESTNET", + }); + (publicKeySelector as unknown as jest.Mock).mockReturnValue(PUBLIC_KEY); + localStorage.setItem( + METRICS_DATA, + JSON.stringify({ + accountType: AccountType.FREIGHTER, + hwExists: false, + importedExists: false, + hwFunded: false, + importedFunded: false, + freighterFunded: false, + unfundedFreighterAccounts: [truncatedPublicKey(PUBLIC_KEY)], + }), + ); + }); + + it("emits freighterAccountFunded with account_id_hash and never a raw/truncated publicKey", () => { + // Ensure emitMetric's `!hasInitialized` guard doesn't short-circuit before + // the amplitude.track call this test inspects. + initAmplitude(); + storeBalanceMetricData(PUBLIC_KEY, true); + + expect(amplitude.track).toHaveBeenCalledWith( + METRIC_NAMES.freighterAccountFunded, + expect.objectContaining({ + account_id_hash: getAccountIdHash(PUBLIC_KEY), + }), + ); + + const [, body] = (amplitude.track as jest.Mock).mock.calls[0]; + expect(body).not.toHaveProperty("publicKey"); + }); +}); + describe("initAmplitude SDK config", () => { beforeEach(() => jest.clearAllMocks()); diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index 0dbf5c2c1b..b28682333e 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -241,6 +241,43 @@ export const initAmplitude = () => { } }; +/** Derives the durable Identify traits from the full account list. */ +export const deriveIdentifyTraits = (allAccounts: Account[]) => { + let hasHardware = false; + let hasImported = false; + allAccounts.forEach((acc) => { + if (acc.hardwareWalletType) { + hasHardware = true; + } else if (acc.imported) { + hasImported = true; + } + }); + return { + wallet_count: allAccounts.length, + has_hardware_wallet: hasHardware, + has_imported_account: hasImported, + }; +}; + +// Only re-send Identify when a durable trait actually changed. +let lastIdentifiedTraits: string | null = null; + +/** Sends durable wallet-composition traits to Amplitude Identify (dirty-checked). */ +export const syncIdentifyTraits = (allAccounts: Account[]): void => { + const traits = deriveIdentifyTraits(allAccounts); + const fingerprint = JSON.stringify(traits); + if (fingerprint === lastIdentifiedTraits) return; + lastIdentifiedTraits = fingerprint; + + if (!AMPLITUDE_KEY || !hasInitialized) return; + + const identify = new amplitude.Identify(); + identify.set("wallet_count", traits.wallet_count); + identify.set("has_hardware_wallet", traits.has_hardware_wallet); + identify.set("has_imported_account", traits.has_imported_account); + amplitude.identify(identify); +}; + // --------------------------------------------------------------------------- // Common context (mirrors mobile's buildCommonContext) // --------------------------------------------------------------------------- @@ -417,7 +454,7 @@ export const storeBalanceMetricData = ( metricsData.freighterFunded = true; if (idx !== -1) { emitMetric(METRIC_NAMES.freighterAccountFunded, { - publicKey: truncated, + account_id_hash: getAccountIdHash(publicKey), }); unfundedFreighterAccounts.splice(idx, 1); } @@ -464,4 +501,6 @@ export const storeAccountMetricsData = ( }); metricsData.accountType = accountType; localStorage.setItem(METRICS_DATA, JSON.stringify(metricsData)); + + syncIdentifyTraits(allAccounts); }; From 1f032d3d1c7c6d85e37b0250aaa7d6d264eb46f0 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 12:39:26 -0400 Subject: [PATCH 07/15] feat(analytics): emit app.opened with one-time connectivity snapshot --- extension/src/helpers/metrics.test.ts | 33 +++++++++++++++++++ extension/src/helpers/metrics.ts | 15 ++++++++- extension/src/popup/constants/metricsNames.ts | 1 + 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index d537f84e92..f37076dcc1 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -267,3 +267,36 @@ describe("initAmplitude SDK config", () => { ); }); }); + +describe("app.opened", () => { + it("exposes the appOpened event name", () => { + expect(METRIC_NAMES.appOpened).toBe("app.opened"); + }); + + it("emits app.opened once during init with the connectivity snapshot", async () => { + (Object.defineProperty as typeof Object.defineProperty)( + global.navigator, + "connection", + { value: { type: "wifi", effectiveType: "4g" }, configurable: true }, + ); + let mod: typeof import("helpers/metrics"); + jest.isolateModules(() => { + mod = require("helpers/metrics"); + }); + const track = ( + require("@amplitude/analytics-browser") as typeof import("@amplitude/analytics-browser") + ).track as jest.Mock; + track.mockClear(); + + await mod!.initAmplitude(); + + const call = track.mock.calls.find((c) => c[0] === "app.opened"); + expect(call).toBeDefined(); + expect(call![1]).toMatchObject({ + connection_type: "wifi", + effective_type: "4g", + schema_version: "2", + }); + expect(call![1].surface).toBeDefined(); + }); +}); diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index b28682333e..7d5a7a2c46 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -164,7 +164,7 @@ export const getUserId = (): string => { * Initializes the Amplitude SDK. Should be called once at app startup. * In development (no AMPLITUDE_KEY), events are logged to console only. */ -export const initAmplitude = () => { +export const initAmplitude = async () => { if (hasInitialized) return; if (!AMPLITUDE_KEY) { @@ -215,6 +215,19 @@ export const initAmplitude = () => { // initializeWithAmplitudeAnalytics requires the analytics SDK to be started first. initExperimentClient(); + // Resolve the surface once (async), then emit the one-time app.opened snapshot. + await resolveSurface(); + + const nav = navigator as Navigator & { + connection?: { type?: string; effectiveType?: string }; + }; + emitMetric(METRIC_NAMES.appOpened, { + connection_type: nav.connection?.type ?? "unknown", + ...(nav.connection?.effectiveType + ? { effective_type: nav.connection.effectiveType } + : {}), + }); + // Keep opt-out in sync whenever the data-sharing setting changes in Redux. // This is the authoritative source of truth; the initial call above may fire // before settings are loaded from the background script. diff --git a/extension/src/popup/constants/metricsNames.ts b/extension/src/popup/constants/metricsNames.ts index 35821e541d..8a11a8c421 100644 --- a/extension/src/popup/constants/metricsNames.ts +++ b/extension/src/popup/constants/metricsNames.ts @@ -191,4 +191,5 @@ export const METRIC_NAMES = { coinbaseOnrampOpened: "coinbase onramp: opened", wallets: "loaded screen: wallets", confirmSidebarRequest: "loaded screen: confirm sidebar request", + appOpened: "app.opened", }; From 2deec32981969c8b4a54421a55aeb04792ba10ca Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 12:54:11 -0400 Subject: [PATCH 08/15] test(analytics): guard against raw public keys in event payloads --- extension/src/helpers/metrics.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index f37076dcc1..870fd01a54 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -268,6 +268,18 @@ describe("initAmplitude SDK config", () => { }); }); +describe("privacy guard", () => { + it("buildCommonContext never includes a raw or truncated public key", () => { + (publicKeySelector as jest.Mock).mockReturnValue( + "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + ); + const ctx = buildCommonContext({} as never); + const serialized = JSON.stringify(ctx); + expect(serialized).not.toContain("GAAAAAAAAAAAAAAAAAAAAAAAAA"); + expect(ctx).not.toHaveProperty("publicKey"); + }); +}); + describe("app.opened", () => { it("exposes the appOpened event name", () => { expect(METRIC_NAMES.appOpened).toBe("app.opened"); From 2c7f63f536f63dcca10a2f4071c8278141745111 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 12:57:26 -0400 Subject: [PATCH 09/15] test(analytics): fix jest.Mock cast to satisfy typecheck in privacy guard test --- extension/src/helpers/metrics.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index 870fd01a54..a445a4e882 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -270,7 +270,7 @@ describe("initAmplitude SDK config", () => { describe("privacy guard", () => { it("buildCommonContext never includes a raw or truncated public key", () => { - (publicKeySelector as jest.Mock).mockReturnValue( + (publicKeySelector as unknown as jest.Mock).mockReturnValue( "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", ); const ctx = buildCommonContext({} as never); From 8919a81c9dad23210e0118a6d248cabd8e078f02 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 13:18:38 -0400 Subject: [PATCH 10/15] 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. --- extension/src/helpers/metrics.test.ts | 34 +++++++++++++++++++++++++++ extension/src/helpers/metrics.ts | 3 ++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index a445a4e882..a0d7fe989e 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -202,6 +202,40 @@ describe("deriveIdentifyTraits", () => { has_imported_account: false, }); }); + + it("does not cache the fingerprint when called before Amplitude init, so an identical call after init still sends Identify", async () => { + // syncIdentifyTraits guards on module-level `hasInitialized`/`AMPLITUDE_KEY` + // state, so isolate the module here to get a fresh, uninitialized instance + // independent of init having already run in another describe block. + const accounts = [ + { publicKey: "G1", hardwareWalletType: "ledger", imported: false }, + ] as never; + + let mod: typeof import("helpers/metrics"); + jest.isolateModules(() => { + mod = require("helpers/metrics"); + }); + const identify = ( + require("@amplitude/analytics-browser") as typeof amplitude + ).identify as jest.Mock; + identify.mockClear(); + + // Pre-init call: the init/consent guard short-circuits before any Identify + // is sent. The fingerprint must NOT be cached here (the hardening fix). + mod!.syncIdentifyTraits(accounts); + expect(identify).not.toHaveBeenCalled(); + + await mod!.initAmplitude(); + // initAmplitude sends its own Identify (bundle id property); clear that + // call so it doesn't get confused with the assertion below. + identify.mockClear(); + + // Same accounts as the pre-init call: if the fingerprint had been cached + // pre-init, this would be a no-op dirty-check short-circuit and Identify + // would never fire. Post-fix, it must fire because nothing was cached. + mod!.syncIdentifyTraits(accounts); + expect(identify).toHaveBeenCalled(); + }); }); describe("storeBalanceMetricData (privacy)", () => { diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index 7d5a7a2c46..2ed5767787 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -280,10 +280,11 @@ export const syncIdentifyTraits = (allAccounts: Account[]): void => { const traits = deriveIdentifyTraits(allAccounts); const fingerprint = JSON.stringify(traits); if (fingerprint === lastIdentifiedTraits) return; - lastIdentifiedTraits = fingerprint; if (!AMPLITUDE_KEY || !hasInitialized) return; + lastIdentifiedTraits = fingerprint; + const identify = new amplitude.Identify(); identify.set("wallet_count", traits.wallet_count); identify.set("has_hardware_wallet", traits.has_hardware_wallet); From 4beaf625646221d29c52ddd6e79f4f1907ad675f Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 13:19:53 -0400 Subject: [PATCH 11/15] =?UTF-8?q?docs(analytics):=20correct=20=C2=A76=20Id?= =?UTF-8?q?entify=20call-site=20list=20(accountServices=20duplicate)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md b/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md index 6bd5de5182..1ceb7af423 100644 --- a/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md +++ b/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md @@ -113,7 +113,7 @@ All centralized in `emitMetric` + `buildCommonContext` (`extension/src/helpers/m ## 6. Identify wiring - Extend the existing `amplitude.identify()` block in `initAmplitude`. -- Traits derive from the same `allAccounts` that `storeAccountMetricsData(publicKey, allAccounts)` already walks (call sites: `popup/ducks/accountServices.ts:192`, `helpers/hooks/useGetAppData.tsx:74`, `views/RecoverAccount/hooks/useGetRecoverAccountData.ts`): +- Traits derive from the same `allAccounts` that `storeAccountMetricsData(publicKey, allAccounts)` already walks. The effective sync sites are `helpers/hooks/useGetAppData.tsx:74` and `views/RecoverAccount/hooks/useGetRecoverAccountData.ts`, which call the shared `helpers/metrics` implementation. **Note:** `popup/ducks/accountServices.ts` has a _private duplicate_ `storeAccountMetricsData` (it imports only the `MetricsData` type from `helpers/metrics`), so wiring `syncIdentifyTraits` into the shared function does **not** cover the active-account-switch (`makeAccountActive`) path. That is acceptable — switching the active account does not change the account _set_ the durable traits describe, and any set change (add/import/create/recover) flows through `useGetAppData` and re-syncs. Follow-up: dedupe the two `storeAccountMetricsData` implementations. Traits: - `has_hardware_wallet` ← `hwExists` - `has_imported_account` ← `importedExists` - `wallet_count` ← `allAccounts.length` From f090439a14cb369a41e4d227dab1001674c0300d Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 14:04:58 -0400 Subject: [PATCH 12/15] 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) --- .../specs/ANALYTICS_SCHEMA_FOUNDATION.md | 184 ------------------ 1 file changed, 184 deletions(-) delete mode 100644 extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md diff --git a/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md b/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md deleted file mode 100644 index 1ceb7af423..0000000000 --- a/extension/specs/ANALYTICS_SCHEMA_FOUNDATION.md +++ /dev/null @@ -1,184 +0,0 @@ -# Analytics Schema Alignment — Property-Model Foundation (Design Spec) - -**Status:** Approved design, pre-implementation -**Date:** 2026-07-13 -**Owner:** piyal -**Tracking issue:** [stellar/freighter#2883](https://github.com/stellar/freighter/issues/2883) — Analytics refactor (extension): cross-platform schema alignment (Epic) -**Canonical schema (RFC):** [stellar/wallet-eng-monorepo#10](https://github.com/stellar/wallet-eng-monorepo/pull/10) — `analytics-refactor-report.md` -**Identity primitive (dependency, landing separately):** [stellar/freighter#2876](https://github.com/stellar/freighter/pull/2876) — derives the unified `user_id` from the seed phrase - ---- - -## 1. Context - -Issue #2883 is a 7-stream Epic to align the extension's Amplitude events with freighter-mobile on a shared `domain.action_past` schema (defined in the RFC), clean up the property model, and retire redundant events. The Epic is too large for a single spec, so it is decomposed into independently shippable slices. **This spec covers the first slice only.** - -The RFC is the canonical, cross-platform schema catalog (~50 events, four-bucket property model, naming rules). We adopt it **doc-defined + mirrored**: each repo (extension, mobile) implements its own catalog matching the RFC; the doc is the source of truth and the drift guard. No shared package, no codegen. - -### Key facts about the current system - -- Emission is centralized: `emitMetric(name, body)` in `extension/src/helpers/metrics.ts` → `amplitude.track`. -- Event names live in a flat `METRIC_NAMES` const map (`extension/src/popup/constants/metricsNames.ts`). -- Screen-load events funnel through a Redux middleware: a `registerHandler` on the `navigate` action maps route → event name in `extension/src/popup/metrics/views.ts`. -- `buildCommonContext(state)` attaches shared context to **every** event. -- Identity: `amplitude.setUserId(...)` is already called in `initAmplitude`; the value it uses is owned by the unified-user-ID work (#2876), **not** by this refactor. - -The current system works and has drawn no complaints. This slice **evolves it in place** — it does **not** re-architect it. - ---- - -## 2. Scope - -### In scope (this slice) — global, name-agnostic property-model foundation - -The property-model changes live in the **central** `emitMetric` / `buildCommonContext` path, so they apply to all events at once and cannot be done per-event. That makes them a natural first slice, independent of event renaming: - -1. `schema_version` stamped on every event (permanent `"2"` marker — see §4). -2. `account_id_hash` replaces the truncated `publicKey` on events (§5). -3. Stop hand-sending SDK-supplied device/app fields; rely on the SDK's built-in context (§4, Bucket 1). -4. Reclassify durable traits → Amplitude **Identify** user properties; keep volatile context event-level (§4, §6). -5. New `getSurface()` helper + `app.opened` event carrying the one-time snapshot (§7). - -Event **names stay legacy** in this slice. - -### Out of scope (follow-on slices) - -- **Slice B** — collapse `views.ts` route map into the canonical `screen.viewed` event. -- **Slice C** — domain-event renames / consolidations / removals (`payment.completed`, `blockaid.scan_completed` + `scan_target`, trustline/asset consolidations, swap-as-first-class, missing events, redundant-event deletion). -- **Slice D** — Amplitude account-level taxonomy cleanup (dashboard hygiene, no app code). Independent; can run in parallel. -- **Identity / `user_id`** — owned by #2876. Not touched here. - ---- - -## 3. Key decisions - -| Decision | Choice | Rationale | -| -------------------------- | ----------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Migration strategy | **Hard cutover per event, no dual-write** | Dual-write (RFC Part 4) doubles event volume against an already-full Amplitude project and adds transitional branching in the emit path. Parity is validated in staging/QA + tests before each flip instead of via prod count-overlap. | -| Schema source of truth | **Doc-defined, mirrored per repo** | Matches the team's current cross-repo workflow; lowest infra cost. `schema_version` is the drift tripwire. | -| Emission architecture | **Evolve `emitMetric` in place** (no typed-catalog rewrite) | RFC is a naming/property cleanup, not a re-architecture. A typed `track` catalog would churn every call site for compile-time safety the team is not asking for; the spec guards drift. | -| `schema_version` placement | **Global, stamped now** | One central change; the marker distinguishes new vs old property model in dashboards even while names are still legacy. | -| `wallet_count` definition | **`allAccounts.length`** | Confirm mobile matches before shipping (mobile may count seed-phrase wallets). | -| `app.opened` cadence | **Per popup/fullpage/sidebar mount** | Closest extension analog to mobile's foreground event; matches RFC funnel-denominator intent. | - ---- - -## 4. Property model — the four buckets - -All centralized in `emitMetric` + `buildCommonContext` (`extension/src/helpers/metrics.ts`). - -### Bucket 1 — SDK-supplied device/app metadata (stop hand-sending) - -- **Delete** `platform`, `platformVersion`, `appVersion` from `buildCommonContext`. -- The Amplitude Browser SDK's context plugin already attaches `platform` / `os_name` / `os_version` / `device_model` to every event. Pass `appVersion` once in the `amplitude.init(...)` config so the SDK attaches `app_version`. -- **Do NOT enable behavioral `autocapture`** (page views, clicks, form/element interactions, sessions). For a wallet UI that risks capturing DOM/URL content. This slice adds **no** new data-collection surface — it removes duplicated fields only. - -### Bucket 2 — Event-level volatile context (stays, reshaped) - -`buildCommonContext` keeps/adds, per event: - -- `network` (kept) -- `surface` — from `getSurface()` (new) -- `account_type`, `account_funded`, `is_hardware_account`, `account_id_hash` — the reclassified active-account fields -- **Removed:** truncated `publicKey` (→ `account_id_hash`), `connection_type` / `effective_type` (→ `app.opened` only) - -### Bucket 3 — Durable Identify user traits (new; see §6) - -`wallet_count`, `has_hardware_wallet`, `has_imported_account`, `bundle_id` (existing). - -### Bucket 4 — One-time `app.opened` snapshot (see §7) - -`surface`, `network`, `connection_type`, `effective_type`, `schema_version`. - -### Reclassification of the current per-event `metricsData` props - -| Current per-event prop | New home | -| --------------------------- | -------------------------------------------------------------------------- | -| `hw_connected` | Identify `has_hardware_wallet` | -| `secret_key_account` | Identify `has_imported_account` + event `account_type=imported_secret_key` | -| `freighter_account_funded` | event `account_funded` | -| `secret_key_account_funded` | split → event `account_type` + `account_funded` | - ---- - -## 5. `account_id_hash` derivation - -- **Algorithm:** synchronous SHA-256 of the full G-address, hex-encoded, via the Stellar SDK's `hash()` (already used in `extension/src/background/messageListener/handlers/signAuthEntry.ts`). -- **Memoized per public key** in a module-level cache — no async in the emit hot path, no per-event recompute. -- **Cross-platform:** identical input (G-address string) + identical algorithm (SHA-256 hex) on mobile ⇒ identical hash ⇒ account-level joins work across platforms. A committed test vector locks this (same approach #2876 used for its cross-platform vectors). -- **No active key (pre-unlock):** `account_id_hash` is omitted. Never emit a raw or truncated public key. - ---- - -## 6. Identify wiring - -- Extend the existing `amplitude.identify()` block in `initAmplitude`. -- Traits derive from the same `allAccounts` that `storeAccountMetricsData(publicKey, allAccounts)` already walks. The effective sync sites are `helpers/hooks/useGetAppData.tsx:74` and `views/RecoverAccount/hooks/useGetRecoverAccountData.ts`, which call the shared `helpers/metrics` implementation. **Note:** `popup/ducks/accountServices.ts` has a _private duplicate_ `storeAccountMetricsData` (it imports only the `MetricsData` type from `helpers/metrics`), so wiring `syncIdentifyTraits` into the shared function does **not** cover the active-account-switch (`makeAccountActive`) path. That is acceptable — switching the active account does not change the account _set_ the durable traits describe, and any set change (add/import/create/recover) flows through `useGetAppData` and re-syncs. Follow-up: dedupe the two `storeAccountMetricsData` implementations. Traits: - - `has_hardware_wallet` ← `hwExists` - - `has_imported_account` ← `importedExists` - - `wallet_count` ← `allAccounts.length` - - `bundle_id` ← existing -- Fire `identify()` **only when a trait value actually changes** (dirty-check against the last-sent set) — Identify is for durable traits, not per-action. -- **Consent:** no new gate — the existing `setOptOut` subscription already suppresses Identify uploads when data-sharing is off. - ---- - -## 7. `app.opened` & `getSurface()` - -### `getSurface()` → `popup | sidebar | fullpage` - -- `sidebar`: synchronous via `isSidebarMode()` (`mode=sidebar` query param). -- `popup` vs `fullpage`: `browser.tabs.getCurrent()` (undefined ⇒ popup, defined ⇒ fullpage) is async, so **resolve once at init and cache in a module variable**; `buildCommonContext` reads it synchronously thereafter. - -### `app.opened` - -- Fires once in the init path (where `initAmplitude` runs — `extension/src/popup/App.tsx:48`), i.e. **per popup/fullpage/sidebar mount**. -- Payload (one-time snapshot): `surface`, `network`, `connection_type`, `effective_type`, `schema_version`. -- `connection_type` / `effective_type` are **removed** from `buildCommonContext` at the same time, so they ride only on `app.opened`. - ---- - -## 8. Migration sequencing & interim state - -1. Land this slice: global property-model changes + Identify + `app.opened` + `account_id_hash` + `getSurface`. **Names remain legacy.** -2. After this slice, events carry the **new property model + `schema_version: "2"` under their legacy names** until slices B/C rename them. Given hard cutover (no dual-write) and that dashboards are being rebuilt regardless, this is acceptable; `schema_version` is exactly the flag that lets a dashboard distinguish the new property model from the old. -3. Slice B renames screen events → `screen.viewed`. Slice C renames/consolidates/removes domain events. Each rename is a hard cutover. - ---- - -## 9. Error handling & edge cases - -- **Pre-unlock / no active key:** `account_id_hash`, `account_type`, `account_funded`, `is_hardware_account` are omitted. Welcome/onboarding events carry none. No crash, no placeholder. -- **Hashing:** `hash()` is sync + deterministic on a valid G-address; wrap defensively and omit `account_id_hash` on error rather than throwing in the emit path. -- **Consent gate:** unchanged — `emitMetric` early-returns when data-sharing is off; Identify covered by `setOptOut`. -- **Init failure / missing `AMPLITUDE_KEY`:** unchanged — existing `try/catch` + console-log fallback preserved. -- **`browser.tabs.getCurrent()` unavailable / throws:** default `surface` to `popup`. - ---- - -## 10. Testing - -- **Unit (`extension/src/helpers/metrics.test.ts` — extend existing):** - - `buildCommonContext` no longer emits `platform` / `platformVersion` / `appVersion` / `publicKey` / `connection_type` / `effective_type`. - - `buildCommonContext` emits `account_id_hash`, `account_type`, `account_funded`, `is_hardware_account`, `surface`, `schema_version`. - - `account_id_hash` is deterministic and matches a committed cross-platform vector. - - Identify fires only on trait change. - - Data-sharing off ⇒ no `track`, no `identify`. -- **`app.opened`** fires once per init with the snapshot fields; those fields are absent from subsequent events. -- **Regression guard:** a test asserting **no** event payload contains a raw or truncated public key. -- Existing metric tests updated to the reshaped payload. - ---- - -## 11. Open items to confirm before shipping - -- [ ] **`wallet_count` parity:** confirm mobile also defines it as account count (not seed-phrase-wallet count). Resolved default here: `allAccounts.length`. -- [ ] Confirm the committed `account_id_hash` vector is mirrored by freighter-mobile (same G-address → same hash). - ---- - -## 12. Follow-on slices (not this spec) - -- **B** — `screen.viewed` consolidation (`views.ts` route map). -- **C** — domain-event renames/consolidations/removals + missing events (`swap.completed`/`.failed`, `transaction.submitted`, `collectible_send.*`, `asset_remove.responded`, transaction-details `screen.viewed` parity). -- **D** — Amplitude taxonomy cleanup (dashboard hygiene; runs independently, prioritized to free property headroom). From cf18cd4ee7ed99b22b4aea8f01ba188b0a337aa2 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Tue, 14 Jul 2026 16:54:15 -0400 Subject: [PATCH 13/15] =?UTF-8?q?fix(analytics):=20address=20PR=20review?= =?UTF-8?q?=20=E2=80=94=20consent-hydration=20timing=20+=20pre-unlock=20om?= =?UTF-8?q?ission?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- extension/src/helpers/metrics.test.ts | 87 ++++++++++++++++++- extension/src/helpers/metrics.ts | 65 +++++++++----- .../src/popup/locales/pt/translation.json | 4 +- 3 files changed, 127 insertions(+), 29 deletions(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index a0d7fe989e..76b1a2d870 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -153,11 +153,16 @@ describe("buildCommonContext (four-bucket property model)", () => { expect(ctx).not.toHaveProperty("effectiveType"); }); - it("omits account_id_hash when there is no active key (pre-unlock)", () => { + it("omits all account fields when there is no active key (pre-unlock)", () => { (publicKeySelector as unknown as jest.Mock).mockReturnValue(""); - expect(buildCommonContext({} as never)).not.toHaveProperty( - "account_id_hash", - ); + const ctx = buildCommonContext({} as never); + expect(ctx).not.toHaveProperty("account_id_hash"); + expect(ctx).not.toHaveProperty("account_type"); + expect(ctx).not.toHaveProperty("account_funded"); + expect(ctx).not.toHaveProperty("is_hardware_account"); + // non-account context is still present + expect(ctx).toMatchObject({ schema_version: "2", network: "TESTNET" }); + expect(ctx.surface).toBeDefined(); }); it("marks hardware active account", () => { @@ -314,6 +319,80 @@ describe("privacy guard", () => { }); }); +describe("consent hydration (startup)", () => { + it("defers app.opened while data-sharing is disallowed, then emits exactly once when it becomes allowed", async () => { + let mod: typeof import("helpers/metrics"); + let store: { subscribe: jest.Mock }; + let settingsMod: { settingsDataSharingSelector: jest.Mock }; + let track: jest.Mock; + jest.isolateModules(() => { + mod = require("helpers/metrics"); + store = require("popup/App").store; + settingsMod = require("popup/ducks/settings"); + track = (require("@amplitude/analytics-browser") as typeof amplitude) + .track as jest.Mock; + }); + + // Persisted preference hasn't hydrated yet → disallowed at init. + settingsMod!.settingsDataSharingSelector.mockReturnValue(false); + track!.mockClear(); + store!.subscribe.mockClear(); + + await mod!.initAmplitude(); + + // Nothing emitted while consent is disallowed. + expect( + track!.mock.calls.find((c) => c[0] === "app.opened"), + ).toBeUndefined(); + + // Consent now resolves to allowed; the store subscription fires. + settingsMod!.settingsDataSharingSelector.mockReturnValue(true); + const subCb = + store!.subscribe.mock.calls[store!.subscribe.mock.calls.length - 1][0]; + subCb(); + + expect(track!.mock.calls.filter((c) => c[0] === "app.opened")).toHaveLength( + 1, + ); + + // Idempotent: further store changes don't re-emit. + subCb(); + expect(track!.mock.calls.filter((c) => c[0] === "app.opened")).toHaveLength( + 1, + ); + }); + + it("does not send or cache Identify while data-sharing is disallowed, then sends once allowed", async () => { + let mod: typeof import("helpers/metrics"); + let settingsMod: { settingsDataSharingSelector: jest.Mock }; + let identify: jest.Mock; + jest.isolateModules(() => { + mod = require("helpers/metrics"); + settingsMod = require("popup/ducks/settings"); + identify = (require("@amplitude/analytics-browser") as typeof amplitude) + .identify as jest.Mock; + }); + const accounts = [ + { publicKey: "G1", hardwareWalletType: "", imported: false }, + ] as never; + + settingsMod!.settingsDataSharingSelector.mockReturnValue(true); + await mod!.initAmplitude(); + identify!.mockClear(); + + // Opted out: no Identify, and (critically) the fingerprint is NOT cached. + settingsMod!.settingsDataSharingSelector.mockReturnValue(false); + mod!.syncIdentifyTraits(accounts); + expect(identify!).not.toHaveBeenCalled(); + + // Opt in: the same traits must now reach Amplitude (proving nothing was + // cached while opted out — otherwise the dirty-check would suppress it). + settingsMod!.settingsDataSharingSelector.mockReturnValue(true); + mod!.syncIdentifyTraits(accounts); + expect(identify!).toHaveBeenCalled(); + }); +}); + describe("app.opened", () => { it("exposes the appOpened event name", () => { expect(METRIC_NAMES.appOpened).toBe("app.opened"); diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index 2ed5767787..f91adfa4b7 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -203,42 +203,51 @@ export const initAmplitude = async () => { identify.set(BUNDLE_ID_USER_PROPERTY_KEY, getBundleId()); amplitude.identify(identify); - // Apply initial opt-out state. Note: settings may not yet be loaded from the - // background at this point (they're fetched async). The store subscription - // below will correct this as soon as the real preference arrives. - const isDataSharingAllowed = settingsDataSharingSelector(store.getState()); - amplitude.setOptOut(!isDataSharingAllowed); - hasInitialized = true; // Initialize Experiment client now that analytics is ready. // initializeWithAmplitudeAnalytics requires the analytics SDK to be started first. initExperimentClient(); - // Resolve the surface once (async), then emit the one-time app.opened snapshot. + // Resolve the surface once (async) so getSurface() is synchronous afterward. await resolveSurface(); - const nav = navigator as Navigator & { - connection?: { type?: string; effectiveType?: string }; + // The persisted data-sharing preference has NOT hydrated yet at init — it + // defaults to `false` and the real value arrives asynchronously. Emitting + // app.opened here would be suppressed (emitMetric's consent gate) or dropped + // by the SDK (opt-out), yet lost forever. So defer it: emit exactly once, the + // moment data-sharing is (or becomes) allowed. + let hasEmittedAppOpened = false; + const emitAppOpenedOnce = () => { + if (hasEmittedAppOpened) return; + hasEmittedAppOpened = true; + + const nav = navigator as Navigator & { + connection?: { type?: string; effectiveType?: string }; + }; + emitMetric(METRIC_NAMES.appOpened, { + connection_type: nav.connection?.type ?? "unknown", + ...(nav.connection?.effectiveType + ? { effective_type: nav.connection.effectiveType } + : {}), + }); }; - emitMetric(METRIC_NAMES.appOpened, { - connection_type: nav.connection?.type ?? "unknown", - ...(nav.connection?.effectiveType - ? { effective_type: nav.connection.effectiveType } - : {}), - }); - // Keep opt-out in sync whenever the data-sharing setting changes in Redux. - // This is the authoritative source of truth; the initial call above may fire - // before settings are loaded from the background script. + // Keep Amplitude opt-out synced with the data-sharing preference (the + // authoritative source of truth). Primes immediately for the case where + // settings are already hydrated, and updates on every change. Fires the + // one-shot app.opened the first time consent resolves to allowed. let lastDataSharingAllowed: boolean | null = null; - store.subscribe(() => { + const syncDataSharing = () => { const allowed = settingsDataSharingSelector(store.getState()); if (allowed !== lastDataSharingAllowed) { lastDataSharingAllowed = allowed; amplitude.setOptOut(!allowed); + if (allowed) emitAppOpenedOnce(); } - }); + }; + store.subscribe(syncDataSharing); + syncDataSharing(); // Flush any queued events before the popup window closes so they aren't lost. if (typeof window !== "undefined") { @@ -283,6 +292,13 @@ export const syncIdentifyTraits = (allAccounts: Account[]): void => { if (!AMPLITUDE_KEY || !hasInitialized) return; + // Don't cache the fingerprint unless the Identify can actually be sent. During + // startup this runs before the data-sharing preference hydrates (SDK still + // opted out), so caching here would record traits as "sent" while the SDK + // drops them — and the unchanged traits would then be suppressed forever. Skip + // without caching so a later call (after consent hydrates) re-syncs. + if (!settingsDataSharingSelector(store.getState())) return; + lastIdentifiedTraits = fingerprint; const identify = new amplitude.Identify(); @@ -361,14 +377,17 @@ export const buildCommonContext = ( schema_version: SCHEMA_VERSION, surface: getSurface(), network: networkDetails?.network ?? "UNKNOWN", - account_type: ACCOUNT_TYPE_WIRE[metricsData.accountType], - account_funded: accountFundedByType[metricsData.accountType], - is_hardware_account: metricsData.accountType === AccountType.HW, }; + // Active-account fields are meaningful only when there is an active account. + // Pre-unlock (no active key) we omit them rather than emit a default + // "freighter/false" context that misrepresents state. if (activePublicKey) { const idHash = getAccountIdHash(activePublicKey); if (idHash) context.account_id_hash = idHash; + context.account_type = ACCOUNT_TYPE_WIRE[metricsData.accountType]; + context.account_funded = accountFundedByType[metricsData.accountType]; + context.is_hardware_account = metricsData.accountType === AccountType.HW; } return context; diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index a0cb327877..d80707df64 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -77,8 +77,8 @@ "Authorizations": "Autorizações", "Authorize": "Autorizar", "Authorized address": "Endereço autorizado", - "Auto-lock timer": "Auto-lock timer", - "Auto-Lock Timer": "Auto-Lock Timer", + "Auto-lock timer": "Temporizador de bloqueio automático", + "Auto-Lock Timer": "Temporizador de Bloqueio Automático", "available": "disponível", "Back": "Voltar", "Balance": "Saldo", From fcc801ec74d3829fa4296e3594f3a2c766a24882 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 15 Jul 2026 15:23:57 -0400 Subject: [PATCH 14/15] fix(analytics): derive account_funded from balances cache, not sticky flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- extension/src/helpers/metrics.test.ts | 42 +++++++++++++++++++++++++++ extension/src/helpers/metrics.ts | 22 +++++++++----- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index 76b1a2d870..86c6606a57 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -16,6 +16,9 @@ jest.mock("popup/ducks/settings", () => ({ settingsDataSharingSelector: jest.fn(() => true), settingsNetworkDetailsSelector: jest.fn(() => ({ network: "TESTNET" })), })); +jest.mock("popup/ducks/cache", () => ({ + balancesSelector: jest.fn(() => ({})), +})); jest.mock("helpers/experimentClient", () => ({ initExperimentClient: jest.fn(), })); @@ -46,6 +49,7 @@ import { isSidebarMode } from "popup/helpers/isSidebarMode"; import browser from "webextension-polyfill"; import { publicKeySelector } from "popup/ducks/accountServices"; import { settingsNetworkDetailsSelector } from "popup/ducks/settings"; +import { balancesSelector } from "popup/ducks/cache"; import { METRICS_DATA } from "constants/localStorageTypes"; import { AccountType } from "@shared/api/types"; import { truncatedPublicKey } from "helpers/stellar"; @@ -112,6 +116,9 @@ describe("buildCommonContext (four-bucket property model)", () => { network: "TESTNET", }); (publicKeySelector as unknown as jest.Mock).mockReturnValue(PUBLIC_KEY); + (balancesSelector as unknown as jest.Mock).mockReturnValue({ + TESTNET: { [PUBLIC_KEY]: { isFunded: true } }, + }); localStorage.setItem( METRICS_DATA, JSON.stringify({ @@ -143,6 +150,38 @@ describe("buildCommonContext (four-bucket property model)", () => { expect(ctx.surface).toBeDefined(); }); + it("derives account_funded from the active account's cached balance entry, not the sticky metricsData flag", () => { + // metricsData says this account type has never been funded, but the + // balances cache has a funded entry for the *active* key — the cache + // must win. This is the fix for the sticky-per-type inaccuracy. + localStorage.setItem( + METRICS_DATA, + JSON.stringify({ + accountType: AccountType.IMPORTED, + hwExists: false, + importedExists: true, + hwFunded: false, + importedFunded: false, + freighterFunded: false, + unfundedFreighterAccounts: [], + }), + ); + (balancesSelector as unknown as jest.Mock).mockReturnValue({ + TESTNET: { [PUBLIC_KEY]: { isFunded: true } }, + }); + expect(buildCommonContext({} as never)).toMatchObject({ + account_funded: true, + }); + }); + + it("omits account_funded when there is no cached balances entry for the active key", () => { + (balancesSelector as unknown as jest.Mock).mockReturnValue({}); + const ctx = buildCommonContext({} as never); + expect(ctx).not.toHaveProperty("account_funded"); + // Other active-account fields are still present. + expect(ctx).toMatchObject({ account_type: "imported_secret_key" }); + }); + it("drops SDK-supplied and legacy fields", () => { const ctx = buildCommonContext({} as never); expect(ctx).not.toHaveProperty("platform"); @@ -178,6 +217,9 @@ describe("buildCommonContext (four-bucket property model)", () => { unfundedFreighterAccounts: [], }), ); + (balancesSelector as unknown as jest.Mock).mockReturnValue({ + TESTNET: { [PUBLIC_KEY]: { isFunded: true } }, + }); expect(buildCommonContext({} as never)).toMatchObject({ account_type: "hardware", is_hardware_account: true, diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index f91adfa4b7..024c10e957 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -19,6 +19,7 @@ import { settingsNetworkDetailsSelector, } from "popup/ducks/settings"; import { publicKeySelector } from "popup/ducks/accountServices"; +import { balancesSelector } from "popup/ducks/cache"; import { Account, AccountType } from "@shared/api/types"; import { METRIC_NAMES } from "popup/constants/metricsNames"; @@ -367,12 +368,6 @@ export const buildCommonContext = ( const networkDetails = settingsNetworkDetailsSelector(state); const metricsData = getMetricsData(); - const accountFundedByType: Record = { - [AccountType.FREIGHTER]: metricsData.freighterFunded, - [AccountType.HW]: metricsData.hwFunded, - [AccountType.IMPORTED]: metricsData.importedFunded, - }; - const context: Record = { schema_version: SCHEMA_VERSION, surface: getSurface(), @@ -386,7 +381,20 @@ export const buildCommonContext = ( const idHash = getAccountIdHash(activePublicKey); if (idHash) context.account_id_hash = idHash; context.account_type = ACCOUNT_TYPE_WIRE[metricsData.accountType]; - context.account_funded = accountFundedByType[metricsData.accountType]; + + // account_funded reflects the *active account's* cached balance, not a + // sticky per-account-type flag — funding one Freighter account must not + // make every other (unfunded) Freighter account report funded. Balances + // are cached per network, per public key (see popup/ducks/cache + // balanceData); omit the property entirely (rather than defaulting to + // false) when there is no cached entry, or the cached fundedness is + // unknown (`isFunded === null`), for the active key. + const cachedBalances = + balancesSelector(state)[networkDetails?.network ?? ""]?.[activePublicKey]; + if (cachedBalances && cachedBalances.isFunded !== null) { + context.account_funded = cachedBalances.isFunded; + } + context.is_hardware_account = metricsData.accountType === AccountType.HW; } From 271d67c10acd126d079cd79c8bb06e7b7001b255 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 16 Jul 2026 11:28:27 -0400 Subject: [PATCH 15/15] fix(analytics): resolve account_type live from Redux, not the stale metricsData cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- extension/src/helpers/metrics.test.ts | 64 +++++++++++++++++++++------ extension/src/helpers/metrics.ts | 34 +++++++++++--- 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/extension/src/helpers/metrics.test.ts b/extension/src/helpers/metrics.test.ts index 86c6606a57..88899b27f4 100644 --- a/extension/src/helpers/metrics.test.ts +++ b/extension/src/helpers/metrics.test.ts @@ -11,6 +11,7 @@ jest.mock("popup/App", () => ({ })); jest.mock("popup/ducks/accountServices", () => ({ publicKeySelector: jest.fn(), + allAccountsSelector: jest.fn(() => []), })); jest.mock("popup/ducks/settings", () => ({ settingsDataSharingSelector: jest.fn(() => true), @@ -47,7 +48,10 @@ import { } from "helpers/metrics"; import { isSidebarMode } from "popup/helpers/isSidebarMode"; import browser from "webextension-polyfill"; -import { publicKeySelector } from "popup/ducks/accountServices"; +import { + publicKeySelector, + allAccountsSelector, +} from "popup/ducks/accountServices"; import { settingsNetworkDetailsSelector } from "popup/ducks/settings"; import { balancesSelector } from "popup/ducks/cache"; import { METRICS_DATA } from "constants/localStorageTypes"; @@ -116,13 +120,20 @@ describe("buildCommonContext (four-bucket property model)", () => { network: "TESTNET", }); (publicKeySelector as unknown as jest.Mock).mockReturnValue(PUBLIC_KEY); + // account_type/is_hardware_account are resolved LIVE from the Redux + // account list keyed on the active key — not from the localStorage cache. + (allAccountsSelector as unknown as jest.Mock).mockReturnValue([ + { publicKey: PUBLIC_KEY, name: "Imported", imported: true }, + ]); (balancesSelector as unknown as jest.Mock).mockReturnValue({ TESTNET: { [PUBLIC_KEY]: { isFunded: true } }, }); + // A deliberately *stale* metricsData cache (wrong type) — proves the + // account_type resolution no longer trusts it. localStorage.setItem( METRICS_DATA, JSON.stringify({ - accountType: AccountType.IMPORTED, + accountType: AccountType.FREIGHTER, hwExists: false, importedExists: true, hwFunded: false, @@ -205,18 +216,14 @@ describe("buildCommonContext (four-bucket property model)", () => { }); it("marks hardware active account", () => { - localStorage.setItem( - METRICS_DATA, - JSON.stringify({ - accountType: AccountType.HW, - hwExists: true, - importedExists: false, - hwFunded: true, - importedFunded: false, - freighterFunded: false, - unfundedFreighterAccounts: [], - }), - ); + (allAccountsSelector as unknown as jest.Mock).mockReturnValue([ + { + publicKey: PUBLIC_KEY, + name: "Ledger", + imported: false, + hardwareWalletType: "Ledger", + }, + ]); (balancesSelector as unknown as jest.Mock).mockReturnValue({ TESTNET: { [PUBLIC_KEY]: { isFunded: true } }, }); @@ -226,6 +233,35 @@ describe("buildCommonContext (four-bucket property model)", () => { account_funded: true, }); }); + + it("resolves account_type from the live account list, ignoring a stale metricsData cache", () => { + // The cache (set in beforeEach) says FREIGHTER, but the active account is + // resolvable as an imported secret-key account — the live list must win. + // This is the regression guard for the post-import mislabeling bug: an + // account-mutation thunk switches the active account without refreshing + // metricsData, and events emitted before the next full reload must still + // report the correct type. + const ctx = buildCommonContext({} as never); + expect(ctx).toMatchObject({ + account_type: "imported_secret_key", + is_hardware_account: false, + }); + }); + + it("omits account_type/is_hardware_account when the active key is not resolvable in allAccounts", () => { + // Auth-store update race: the active public key is set, but the account + // list hasn't caught up (or doesn't contain it). Fail safe by OMITTING the + // type fields rather than guessing "freighter" — parity with mobile. + (allAccountsSelector as unknown as jest.Mock).mockReturnValue([ + { publicKey: "GSOMEOTHERKEY", name: "Other", imported: false }, + ]); + const ctx = buildCommonContext({} as never); + expect(ctx).not.toHaveProperty("account_type"); + expect(ctx).not.toHaveProperty("is_hardware_account"); + // The active-key-derived fields are still emitted. + expect(ctx).toHaveProperty("account_id_hash"); + expect(ctx).toMatchObject({ account_funded: true }); + }); }); describe("deriveIdentifyTraits", () => { diff --git a/extension/src/helpers/metrics.ts b/extension/src/helpers/metrics.ts index 024c10e957..8a8f228e4a 100644 --- a/extension/src/helpers/metrics.ts +++ b/extension/src/helpers/metrics.ts @@ -18,7 +18,10 @@ import { settingsDataSharingSelector, settingsNetworkDetailsSelector, } from "popup/ducks/settings"; -import { publicKeySelector } from "popup/ducks/accountServices"; +import { + publicKeySelector, + allAccountsSelector, +} from "popup/ducks/accountServices"; import { balancesSelector } from "popup/ducks/cache"; import { Account, AccountType } from "@shared/api/types"; import { METRIC_NAMES } from "popup/constants/metricsNames"; @@ -118,6 +121,13 @@ const ACCOUNT_TYPE_WIRE: Record = { [AccountType.IMPORTED]: "imported_secret_key", }; +/** Resolves an account's type from its discriminating fields. */ +const resolveAccountType = (account: Account): AccountType => { + if (account.hardwareWalletType) return AccountType.HW; + if (account.imported) return AccountType.IMPORTED; + return AccountType.FREIGHTER; +}; + // --------------------------------------------------------------------------- // User identity (mirrors mobile's src/services/analytics/user.ts) // --------------------------------------------------------------------------- @@ -366,7 +376,6 @@ export const buildCommonContext = ( ): Record => { const activePublicKey = publicKeySelector(state); const networkDetails = settingsNetworkDetailsSelector(state); - const metricsData = getMetricsData(); const context: Record = { schema_version: SCHEMA_VERSION, @@ -380,7 +389,24 @@ export const buildCommonContext = ( if (activePublicKey) { const idHash = getAccountIdHash(activePublicKey); if (idHash) context.account_id_hash = idHash; - context.account_type = ACCOUNT_TYPE_WIRE[metricsData.accountType]; + + // Resolve account_type/is_hardware_account LIVE from the Redux account + // list, keyed on the active public key — not from the localStorage + // metricsData cache. Account-mutation thunks (importAccount, + // importHardwareWallet, addAccount, createAccount) switch the active + // account without refreshing metricsData, so the cache can lag a switch + // and mislabel e.g. a freshly-imported secret-key account as "freighter" + // until the next full app-data reload. If the active key isn't resolvable + // in allAccounts (an auth-store update race), OMIT these fields rather + // than guessing — matching mobile's fail-safe behavior. + const activeAccount = (allAccountsSelector(state) ?? []).find( + (acc: Account) => acc.publicKey === activePublicKey, + ); + if (activeAccount) { + const accountType = resolveAccountType(activeAccount); + context.account_type = ACCOUNT_TYPE_WIRE[accountType]; + context.is_hardware_account = accountType === AccountType.HW; + } // account_funded reflects the *active account's* cached balance, not a // sticky per-account-type flag — funding one Freighter account must not @@ -394,8 +420,6 @@ export const buildCommonContext = ( if (cachedBalances && cachedBalances.isFunded !== null) { context.account_funded = cachedBalances.isFunded; } - - context.is_hardware_account = metricsData.accountType === AccountType.HW; } return context;