From 7d8f21601976614083d966270926a0a2cd9bde28 Mon Sep 17 00:00:00 2001 From: wumibals Date: Mon, 27 Jul 2026 11:46:55 +0000 Subject: [PATCH] docs(product-audit): add knowledge-base entries for #274, #276, #279, #283 Adds 4 documentation files under bridgelet-product-audit/. Documentation-only; no changes to frontend/ or mobile/. --- .../freighter-api-version-pinning.md | 50 +++++++++++++++++++ .../three-repo-error-surface-consistency.md | 50 +++++++++++++++++++ .../runbooks/clear-stale-persisted-wallet.md | 49 ++++++++++++++++++ .../runbooks/onboard-mobile-app-to-ci.md | 50 +++++++++++++++++++ 4 files changed, 199 insertions(+) create mode 100644 bridgelet-product-audit/integration-notes/freighter-api-version-pinning.md create mode 100644 bridgelet-product-audit/integration-notes/three-repo-error-surface-consistency.md create mode 100644 bridgelet-product-audit/runbooks/clear-stale-persisted-wallet.md create mode 100644 bridgelet-product-audit/runbooks/onboard-mobile-app-to-ci.md diff --git a/bridgelet-product-audit/integration-notes/freighter-api-version-pinning.md b/bridgelet-product-audit/integration-notes/freighter-api-version-pinning.md new file mode 100644 index 0000000..120577e --- /dev/null +++ b/bridgelet-product-audit/integration-notes/freighter-api-version-pinning.md @@ -0,0 +1,50 @@ +# Freighter API Version Pinning + +## The factual record + +`frontend/package.json` currently declares: + +```json +"@stellar/freighter-api": "^6.0.1" +``` + +The caret means any 6.x release satisfies it, so the version actually installed is +whatever the lockfile resolved — not necessarily 6.0.1. + +## The coupling that makes this different + +Most dependencies are self-contained: the version you install is the code that +runs. `@stellar/freighter-api` is not. It is a **client for a browser extension +the user installs and updates independently.** + +The package version is fixed by this repo's lockfile; the extension version is +controlled entirely by the end user, changing whenever their browser updates it. +Nothing keeps the two aligned — a user can run an extension months newer or older +than the package this app was built against, and the app has no say in it. + +## The general risk + +When package and extension drift apart, failures surface at the boundary between +them — at runtime, on the user's machine, not at build time. A method the package +exposes may be missing from an older extension; a response shape it expects may +have changed in a newer one. Neither `tsc` nor CI catches this, because the +extension isn't present in either. + +There is direct evidence of exactly this in the codebase. +`signFreighterTransaction()` doesn't trust the response shape — it accepts +**three** different keys for the same value: + +```ts +signResult['signedTxXdr'] || signResult['signedTxXDR'] || signResult['xdr'] +``` + +and `isFreighterTransactionSigningAvailable()` feature-detects `signTransaction` +at runtime rather than assuming it exists — defensive coding that records real +variation across extension versions. + +## Implication + +Bumping the pinned version is not routine: it changes which extension versions the +app works with, and the fallbacks are load-bearing. Test against a real extension, +and treat removing any fallback as a compatibility decision. See the checklist +entry on dependency version review. diff --git a/bridgelet-product-audit/integration-notes/three-repo-error-surface-consistency.md b/bridgelet-product-audit/integration-notes/three-repo-error-surface-consistency.md new file mode 100644 index 0000000..3f3be3a --- /dev/null +++ b/bridgelet-product-audit/integration-notes/three-repo-error-surface-consistency.md @@ -0,0 +1,50 @@ +# Error Surface Consistency Across Three Repos + +Tracing one concrete error — `AlreadySwept` — from contract to user, to see +whether its identity survives the trip. + +## The chain + +| Stage | Repo | What holds the error | +|---|---|---| +| 1. Origin | `bridgelet-core` | `Error::AlreadySwept`, a typed contract variant | +| 2. Backend | `bridgelet-sdk` | String matching, per `error-string-matching.md` | +| 3. Frontend | `bridgelet` (this repo) | An API response rendered in the claim UI | + +**Stage 1** is the strongest link — a Rust enum variant is unambiguous. + +**Stage 2 is where identity is most at risk.** bridgelet-sdk-audit's +`error-string-matching.md` records that the SDK identifies contract errors by +**matching on strings** rather than structured codes. String matching is brittle: +reword the contract's error text and the match silently stops firing, degrading a +specific error into an unrecognised one. Nothing fails loudly. + +**Stage 3** is this repo. The claim path calls `client.redeemClaim(...)` and, on +failure, throws: + +```ts +throw new Error(result.error ?? 'Claim could not be completed. Please try again.'); +``` + +The specific reason rides in `result.error` — but the `??` fallback means **any +response lacking that field collapses to a generic message.** + +## Where identity can be lost + +1. **Contract → SDK:** a reworded contract message breaks string matching. +2. **SDK → API response:** if the error isn't propagated into a populated + `result.error`, stage 3 has nothing specific to show. +3. **Frontend fallback:** the `??` default fires and the user sees generic retry + text. + +Failure 3 is the most user-visible and most misleading. "Already swept" means *the +funds are gone, stop trying*; "please try again" invites the user to retry +something that can never succeed, then contact support when it doesn't. + +## Recommendation + +Confirm end-to-end that a genuine `AlreadySwept` reaches the claim UI as a +distinguishable message. Verify with an actual double-claim against a test account +rather than by reading code, since the weak link is runtime string matching. +**Backend counterpart:** bridgelet-sdk-audit's +`error-mapping-completeness-checklist.md` asks the same from the SDK side. diff --git a/bridgelet-product-audit/runbooks/clear-stale-persisted-wallet.md b/bridgelet-product-audit/runbooks/clear-stale-persisted-wallet.md new file mode 100644 index 0000000..d8d7e2b --- /dev/null +++ b/bridgelet-product-audit/runbooks/clear-stale-persisted-wallet.md @@ -0,0 +1,49 @@ +# Runbook: Clear a Stale or Corrupted Persisted Wallet + +For a user whose browser holds a bad `bridgelet_wallet` value — typically reported +as "it shows the wrong wallet" or "it thinks I'm connected when I'm not." + +## Background: what is actually stored + +`frontend/lib/wallet.ts` persists to `localStorage` under `bridgelet_wallet`, +holding `{ publicKey, type }` and no secret material. + +**Malformed JSON is already handled.** `loadPersistedWallet()` wraps its read in +`try`/`catch` and returns `null` on parse failure, so genuinely corrupted text +degrades silently to "no wallet connected" — it does not throw or wedge the UI. + +So this runbook is rarely about *corruption*. It is almost always a +**stale-but-valid** value: well-formed JSON pointing at the wrong wallet, which +parses cleanly and is trusted. The cast is also unchecked, so a well-formed value +of the *wrong shape* is returned as-is. + +## Steps + +**1. Confirm the symptom.** Ask what address the UI shows and whether the user +recognises it. A wrong-but-real address points here; a blank disconnected state +usually means the value was already discarded. + +**2. Prefer the in-app path.** If a disconnect control exists, have them click it +— it should call `clearPersistedWallet()`. Only fall back to dev tools otherwise. + +**3. Dev-tools fallback.** + +- Open dev tools (`F12`, or `Cmd+Option+I` on macOS). +- **Application** → **Local Storage** → the Bridgelet origin. +- Find `bridgelet_wallet`, note its value if useful, then delete just that key. + +Console equivalent: `localStorage.removeItem('bridgelet_wallet');` + +> Delete the single key. Don't tell users to "clear site data" — unnecessary here, +> and it discards unrelated state. + +## 4. Verify + +Reload. The wallet-connect UI should show a **clean, disconnected state** — no +address, connect option available. Have the user reconnect and confirm the address +is the one they expect. + +If a wrong address reappears after reconnecting, the problem is in the wallet +itself (wrong account selected in Freighter), not persisted state — stop here and +treat it as a wallet-selection issue. Background: +[`connected-wallet-persistence.md`](../glossary/connected-wallet-persistence.md). diff --git a/bridgelet-product-audit/runbooks/onboard-mobile-app-to-ci.md b/bridgelet-product-audit/runbooks/onboard-mobile-app-to-ci.md new file mode 100644 index 0000000..b5adbfe --- /dev/null +++ b/bridgelet-product-audit/runbooks/onboard-mobile-app-to-ci.md @@ -0,0 +1,50 @@ +# Runbook: Onboard the Mobile App to CI + +Steps for giving `mobile/` the lint / type-check / test coverage `frontend/` +already has via `frontend-ci.yml`. + +> This runbook describes the steps. It does **not** create the workflow file. + +## 1. Confirm what mobile can actually run + +Check `mobile/package.json` before writing anything. As of this writing: + +| Script | Command | Notes | +|---|---|---| +| `lint` | `eslint . --ext .ts,.tsx` | ✅ ready | +| `type-check` | `tsc --noEmit` | ⚠️ **note the hyphen** | +| `test` | `jest` | ✅ via `jest-expo` / `ts-jest` | +| `build` | — | ❌ **does not exist** | + +Two traps here: + +- **There is no `build` script.** `frontend-ci.yml` runs `npm run build` + unconditionally, so copying that step verbatim fails immediately. Mobile is + Expo-based (`expo start`), with no direct web-build equivalent — either omit the + step or decide deliberately what "build" means for mobile. +- **The type-check script is named differently.** Frontend has `typecheck`; mobile + has `type-check`. `frontend-ci.yml` detects scripts by exact name + (`"typecheck" in s`), so a copied detection block reports `has_typecheck=false` + and **silently skips type-checking** — a green run that checked nothing. + +## 2. Scope the workflow to `mobile/` + +Follow the existing pattern rather than inventing one: + +```yaml +defaults: + run: + working-directory: mobile +``` + +and point the Node cache at `cache-dependency-path: mobile/package-lock.json` — +that lockfile exists, so `npm ci` will work. Also decide whether to path-filter +the job to `mobile/**` so it skips docs-only changes; `lighthouse-ci.yml` shows +the pattern already in use here. + +## 3. Verify before merging + +Run each command locally from `mobile/` first (`npm ci`, then `lint`, +`type-check`, `test`). Then confirm on a throwaway PR that the job actually +**executes** the steps rather than skipping them — check the log for the +type-check step specifically, given the naming trap above.