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,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.
Original file line number Diff line number Diff line change
@@ -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).
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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).
Loading