Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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.
49 changes: 49 additions & 0 deletions bridgelet-product-audit/runbooks/clear-stale-persisted-wallet.md
Original file line number Diff line number Diff line change
@@ -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).
50 changes: 50 additions & 0 deletions bridgelet-product-audit/runbooks/onboard-mobile-app-to-ci.md
Original file line number Diff line number Diff line change
@@ -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.
Loading