From 8db362c8a022e396640a0a12512f27ae1e0575d3 Mon Sep 17 00:00:00 2001 From: AbdulmujibOladayo Date: Mon, 27 Jul 2026 11:47:17 +0000 Subject: [PATCH] docs(product-audit): add knowledge-base entries for #269, #270, #271, #272 Adds 4 documentation files under bridgelet-product-audit/. Documentation-only; no changes to frontend/ or mobile/. --- .../analytics-events-vs-onchain-outcomes.md | 48 ++++++++++++++++++ .../claim-flow-wallet-requirement.md | 50 +++++++++++++++++++ .../generated-wallet-key-custody.md | 50 +++++++++++++++++++ .../mobile-app-contract-awareness.md | 50 +++++++++++++++++++ 4 files changed, 198 insertions(+) create mode 100644 bridgelet-product-audit/integration-notes/analytics-events-vs-onchain-outcomes.md create mode 100644 bridgelet-product-audit/integration-notes/claim-flow-wallet-requirement.md create mode 100644 bridgelet-product-audit/integration-notes/generated-wallet-key-custody.md create mode 100644 bridgelet-product-audit/integration-notes/mobile-app-contract-awareness.md diff --git a/bridgelet-product-audit/integration-notes/analytics-events-vs-onchain-outcomes.md b/bridgelet-product-audit/integration-notes/analytics-events-vs-onchain-outcomes.md new file mode 100644 index 0000000..65cc583 --- /dev/null +++ b/bridgelet-product-audit/integration-notes/analytics-events-vs-onchain-outcomes.md @@ -0,0 +1,48 @@ +# Analytics Events vs On-Chain Outcomes + +Do the events in `docs/analytics-spec.md` reflect *confirmed on-chain* outcomes, +or optimistic client-side state? The answer differs per event — and the spec is +commendably explicit about it. + +## Completion-related events + +| Event | Fires when | On-chain confirmed? | +|---|---|---| +| `Payment Confirmed` | Sender clicks "Confirm & Pay" and signs | **No** — intent only | +| `Payment Created` | Transaction succeeds on-chain | **Yes** | +| `Payment Creation Failed` | Blockchain transaction fails after signing | **Yes** (negative) | +| `Claim Submitted` | Recipient submits the claim | **No** | +| `Claim Succeeded` | Sweep transaction confirmed on-chain | **Yes** | +| `Claim Failed` | Sweep fails after recipient confirmed | **Yes** (negative) | + +## The naming trap + +`Payment Confirmed` is the one to watch. The spec states plainly that it +"indicates user intent; it fires *before* blockchain confirmation." The word +*Confirmed* refers to **the user confirming**, not the **chain** confirming. + +Anyone reading a dashboard without the spec in hand will almost certainly read +"Payment Confirmed" as settlement. It isn't — `Payment Created` is. The funnel +section makes the distinction load-bearing: step 5 measures "% who sign the +transaction" and step 6 measures "% confirmed → on-chain success". The gap between +those two steps *is* the on-chain failure rate. + +So the spec's own model is sound; the risk is entirely in how the event name reads +out of context. + +## Structural consequence + +Because signature and settlement are separate events, a payment that is signed and +then fails on-chain produces `Payment Confirmed` with no `Payment Created`. Any +metric counting `Payment Confirmed` as a success will **overcount**. The same shape +applies on the recipient side between `Claim Submitted` and `Claim Succeeded`, where +`sweep_duration_ms` explicitly measures the interval to on-chain confirmation. + +## Backend counterpart + +This is the frontend-side instance of a question the backend faces too. See +bridgelet-sdk-audit's `webhook-delivery-vs-chain-event-consistency.md`, which asks +whether a delivered webhook reliably corresponds to a settled chain event. Both +reduce to: *does this signal mean "someone asked" or "the ledger agreed"?* Answers +should stay consistent across the two surfaces, or a dashboard and a webhook +consumer will disagree about the same payment. diff --git a/bridgelet-product-audit/integration-notes/claim-flow-wallet-requirement.md b/bridgelet-product-audit/integration-notes/claim-flow-wallet-requirement.md new file mode 100644 index 0000000..2641ea0 --- /dev/null +++ b/bridgelet-product-audit/integration-notes/claim-flow-wallet-requirement.md @@ -0,0 +1,50 @@ +# Claim Flow — Is a Wallet Required? + +What the frontend actually requires from a recipient today. + +## What the code does + +The claim route is `frontend/app/claim/[token]/`, with the work done in +`claim-page-client.tsx`. The claim itself is a single call: + +```ts +async function handleClaim(destinationAddress: string) { + const result = await client.redeemClaim(token, destinationAddress); + ... +} +``` + +The two inputs are the **claim token** (from the URL path) and a **destination +address string**. That is all. + +Searching `claim-page-client.tsx` for wallet-connection references returns +**nothing** — no `ConnectedWallet`, no `connectFreighter()`, no import from +`lib/wallet.ts`. None of the three `WalletType` paths is invoked on the claim side. + +**So no, a recipient does not connect a wallet to claim.** They supply an address, +validated by format only — `accessible-claim-form.tsx` enforces `^G[A-Z2-7]{55}$`, +the Stellar public-key shape — which is not ownership proof. This is coherent with +the product's premise: the recipient is assumed *not* to have a wallet, so +requiring one would defeat the purpose. + +## Which on-chain path this maps to + +bridgelet-audit documents two shapes for `bridgelet-core`: +`sweep-controller-claim-flow.md` and `sweep-controller-signature-flow.md`. The +frontend's behaviour — token plus destination address, no recipient signature — +points at the **claim flow**, not the signature flow. Bridgelet's own authority +sweeps to the supplied address; the recipient never signs. + +## Mismatch worth flagging + +The gap is **not** UI-implies-more-than-needed. It runs the other way. A UI that +asks only for an address may under-signal how consequential that address is. With +no ownership proof and no connected wallet to source it from, a mistyped-but-valid +`G...` address is a plausible way to send funds somewhere unrecoverable. The +`^G[A-Z2-7]{55}$` check catches malformed input, not *wrong* input — and the sweep +is irreversible once confirmed. + +Whether the intended design is "any valid address, recipient's responsibility" or +"eventually require a connected wallet" is worth confirming against +`docs/bridgelet-frd-ui-ux.md`. See also +[`../glossary/sender-vs-recipient-auth-models.md`](../glossary/sender-vs-recipient-auth-models.md). diff --git a/bridgelet-product-audit/integration-notes/generated-wallet-key-custody.md b/bridgelet-product-audit/integration-notes/generated-wallet-key-custody.md new file mode 100644 index 0000000..5f50e6e --- /dev/null +++ b/bridgelet-product-audit/integration-notes/generated-wallet-key-custody.md @@ -0,0 +1,50 @@ +# Generated Wallet — Key Custody + +An open design question, recorded before the code in question acquires its first +caller. + +## Current state + +`generateNewWallet()` in `frontend/lib/wallet.ts` mints a fresh Stellar keypair +client-side and returns the secret in plaintext: + +```ts +const { Keypair } = await import('@stellar/stellar-sdk'); +const keypair = Keypair.random(); +return { + wallet: { publicKey: keypair.publicKey(), type: 'generated' }, + secretKey: keypair.secret(), +}; +``` + +A search of `frontend/app` and `frontend/components` finds **no caller**. The only +other reference in the repo is documentation: `docs/security-model.mdx` logs it as +threat **T-15** ("Private key leak from generated wallet"), noting the secret is +returned directly to the caller, that the flow is not presented in the UI by +default, and rating it **⚠️ Partial — secret handling requires audit**. + +So the function is written, typed, exported, and unused. + +## What a safe design must specify first + +Because the secret is returned as a plain string to browser JavaScript, whoever +writes the first caller inherits every one of these decisions: + +1. **Storage.** Where does the secret live? Note that the existing persistence + path is *not* an answer — `persistWallet()` stores only `{ publicKey, type }` + and has no field for a secret. Anything else means a new mechanism, and + `localStorage` is readable by any script on the origin. +2. **Transmission.** Is the secret ever sent anywhere? It must never reach the + backend, or the "non-custodial" framing of the `generated` path collapses. +3. **Recovery.** How does the user get it back? A keypair generated in a browser + tab and never displayed or exported is lost when that tab closes — along with + the funds. Some deliberate export/backup step is mandatory, not optional. +4. **Lifetime in memory.** How long does the string sit in JS memory, and what + else on the page could read it? + +## Framing + +This is **not currently exploitable** — no caller means no live path producing a +secret. That is exactly why it is worth writing down now: specifying custody is +cheap while the answer is still "nobody uses this", and expensive once a UI ships +that hands users keys. Treat T-15 as blocking on the first caller, not on today. diff --git a/bridgelet-product-audit/integration-notes/mobile-app-contract-awareness.md b/bridgelet-product-audit/integration-notes/mobile-app-contract-awareness.md new file mode 100644 index 0000000..829b6c3 --- /dev/null +++ b/bridgelet-product-audit/integration-notes/mobile-app-contract-awareness.md @@ -0,0 +1,50 @@ +# Mobile App — Contract Awareness + +Does the mobile app talk to Stellar/Soroban directly, or is it purely a client of +the bridgelet-sdk API? + +## Finding: no direct chain awareness + +A search across `mobile/` for `@stellar/`, `stellar-sdk`, `freighter`, `soroban` +and `horizon` returns **no genuine matches**. The only hits are incidental +substring collisions in React Native styling — `paddingHorizontal`, `horizontal`, +`showsHorizontalScrollIndicator` — which are unrelated to Horizon the Stellar API. + +`mobile/services/` contains exactly one module, `logger/index.ts`, a +console-backed logger with no network transport. There is no wallet service, no +signing code, and no chain client under `mobile/app` or `mobile/services`. + +## Contrast with the frontend + +The web frontend is the opposite: + +| | `frontend/` | `mobile/` | +|---|---|---| +| Stellar SDK | `@stellar/stellar-sdk` (`Keypair.random()`) | none | +| Wallet integration | `@stellar/freighter-api`, `connectFreighter()` | none | +| Transaction signing | `signFreighterTransaction()`, network passphrases | none | +| Chain concepts in code | `WalletType`, XDR, `G...` addresses | none | + +`frontend/lib/wallet.ts` handles extension connection, XDR signing and network +passphrase resolution. Nothing equivalent exists on mobile. + +## Implication for the audit + +If mobile is purely API-driven, then **its contract-consumption story is entirely +inherited from bridgelet-sdk's behaviour** — there is no independent code path in +this repo to audit for mobile. Any question about how mobile handles a contract +error, a sweep outcome, or a network mismatch resolves to a question about the +SDK's API responses, not about mobile code. + +That is a meaningful reduction in audit surface, and worth stating explicitly so +reviewers don't go looking for mobile-side chain handling that doesn't exist. + +## Caveat + +"No direct chain awareness" is a statement about **imports found during this +review**, not a guarantee. Mobile could still receive chain-derived data as plain +JSON from the API and render it — consumption without awareness. If mobile later +gains wallet or signing support, this note is invalidated and should be rewritten. + +See also [`../glossary/mobile-app-services-logger.md`](../glossary/mobile-app-services-logger.md) +and [`../runbooks/onboard-mobile-app-to-ci.md`](../runbooks/onboard-mobile-app-to-ci.md).