Skip to content

Commit 18cc7d6

Browse files
committed
fix(cashu): address TA-1 review — replay recovery + cross-order token guard
Two findings from the automated review of #829. Replay recovery (step 3b). The step-10 notifications are sent after the CAS commits, so a crash or a lost send queue in between left the escrow funded and the buyer never cued to send fiat — and the step-3 status check rejected every retry, making the documented idempotent retry path unreachable. An order this handler already locked (escrow stored, still Active) whose submitted token matches the stored one now replays the notifications and returns Ok(()): no mint round-trip, no second write. A different token on a locked order is a second funding the escrow can never honour, so it is rejected. The exception is scoped to Active, so a stale "escrow locked, send fiat" is never replayed onto a trade that has moved on. Cross-order token uniqueness (step 6b). The 2-of-3 condition commits to trade keys, not to an order id, and the mint reports proofs unspent until the first redeem — so two orders reusing the same trade keys could both validate the very same token and go Active against one redeemable escrow. The handler now rejects a token already escrowed elsewhere before the mint round-trip, and the CF-4 CAS carries a NOT EXISTS leg that closes the check-then-act race atomically. A CAS that matches zero rows because another order took the token is reported as InvalidCashuToken rather than a silent no-op. This is token-level uniqueness; proof-level (Y = hash_to_curve) uniqueness arrives with TA-1f. Track A §4 of the spec is updated to match, along with its Definition of Done.
1 parent 456e0fb commit 18cc7d6

3 files changed

Lines changed: 459 additions & 30 deletions

File tree

docs/cashu/02-track-a-lock.md

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,19 @@ to `release_action` (compute/verify first, persist second, notify last).
133133
`CantDo(InvalidPeer)`. (Same identity check shape as `release_action`.)
134134
3. **Check status.** The order must be in the "waiting for seller to fund" status
135135
(`WaitingPayment`); otherwise `CantDo(NotAllowedByStatus)`.
136+
3b. **Replay recovery (the one exception to step 3).** An order this handler
137+
already locked — `cashu_escrow_locked_at` set **and** status still `Active`
138+
is a *replay*, not a stray request: step 10's notifications are sent after
139+
the commit, so a crash (or a lost send queue) in between leaves the escrow
140+
funded and the buyer never cued to send fiat, while step 3 would reject every
141+
retry. If the submitted token equals the stored one, **re-send the step-10
142+
notifications and return `Ok(())`** — no mint round-trip, no second write. A
143+
*different* token on an already-locked order ⇒ `CantDo(InvalidCashuToken)`
144+
(a second funding the escrow can never honour). The exception is deliberately
145+
scoped to `Active`: a locked order that has moved on (`FiatSent`, `Success`,
146+
…) falls through to the step-3 rejection, so a stale "escrow locked, send
147+
fiat" is never replayed onto an advanced trade. Only the seller trade key
148+
(step 2) can reach this path.
136149
4. **Extract the proof.** `Payload::CashuLockProof(proof)`; absent ⇒
137150
`CantDo(InvalidCashuToken)`. (`MessageKind::verify()` already guarantees the
138151
payload shape, but the handler re-checks defensively.)
@@ -149,6 +162,21 @@ to `release_action` (compute/verify first, persist second, notify last).
149162
Any mismatch ⇒ `CantDo(InvalidCashuToken)`. This is the security core:
150163
the 2-of-3 must lock to the keys Mostro already holds for this order, never
151164
attacker-chosen keys.
165+
6b. **Reject a token already escrowed by another order.** The 2-of-3 commits to
166+
`{P_B, P_S, P_M}` — trade keys, not an order id — so step 6 cannot tell this
167+
order apart from another one that reuses the same trade keys, and the mint
168+
reports the proofs unspent until the first redeem: without a guard both
169+
orders validate the same token and go `Active` against a single redeemable
170+
escrow. `db::cashu_escrow_token_in_use(pool, &proof.token, order.id)`
171+
`CantDo(InvalidCashuToken)`, checked here so the seller gets a clear reason
172+
before the mint round-trip. The CF-4 CAS (step 8) repeats the predicate as a
173+
`NOT EXISTS` leg, which is what actually closes the check-then-act race
174+
between two concurrent submissions; a `false` CAS whose token turns out to be
175+
held elsewhere is reported as `InvalidCashuToken` rather than a silent no-op.
176+
This is *token-level* uniqueness — proof-level (`Y = hash_to_curve(secret)`)
177+
uniqueness, which also catches a token re-serialised around the same proofs,
178+
arrives with TA-1f's `cashu_fee_proofs` table (§4A) and should be extended to
179+
the escrow token there.
152180
7. **Validate the token against the mint.**
153181
`ctx.cashu_client()` → `verify_escrow_token(&proof.token, p_b, p_s, p_m,
154182
expected_amount, min_locktime)`. This composes: 2-of-3 condition (exactly 2 sigs
@@ -164,7 +192,9 @@ to `release_action` (compute/verify first, persist second, notify last).
164192
now, /*expected*/ WaitingPayment, /*new*/ Active)`. If it returns `false`
165193
(status changed concurrently, or escrow already locked — replay), log and
166194
return `Ok(())` without notifying (idempotent; same pattern as the
167-
`rows_affected() == 0` guard in `release_action`).
195+
`rows_affected() == 0` guard in `release_action`) — **unless** the token is
196+
meanwhile held by another order, the lost-race case of §6b, which is a
197+
rejection rather than a no-op.
168198
9. **Publish the updated order event** (kind 38383) via `update_order_event`, as
169199
the LN funding path does, so the order's public state stays consistent.
170200
10. **Notify the buyer.** Enqueue `Action::CashuEscrowLocked` to the buyer plus
@@ -642,8 +672,11 @@ Tracks B/C/D** — it only edits its own files plus the pre-wired CF-5 stub.
642672
unchanged (wrong sender, wrong status, wrong mint, pubkey mismatch, malformed
643673
token, mint unavailable, double-spent, replay, **missing/mis-valued
644674
fee_token when `mostro.fee > 0`**).
645-
3. The lock is atomic and idempotent (concurrent/replayed submission matches zero
646-
rows and is a safe no-op; **the fee token is never redeemed twice**).
675+
3. The lock is atomic and idempotent: a concurrent submission matches zero rows
676+
and is a safe no-op, a sequential retry of an already-locked escrow replays
677+
the notifications instead of being rejected (§4 step 3b), the same token is
678+
never escrowed by two orders (§4 step 6b), and **the fee token is never
679+
redeemed twice**.
647680
3b. The seller-funded fee token of `2 * order.fee` is validated against the mint
648681
and redeemed on success; Mostro's total revenue equals the Lightning-mode
649682
`2 * order.fee`, and dev-fee accounting is unchanged (Option 2, §4A). The

0 commit comments

Comments
 (0)