feat(cashu): restore/monitor in-flight locked escrows — Track A TA-3 - #831
feat(cashu): restore/monitor in-flight locked escrows — Track A TA-3#831grunch wants to merge 6 commits into
Conversation
The integration PR of the Cashu foundation (docs/cashu/01-fundamentals.md §6). A node with `[cashu] enabled = true` now boots with NO Lightning node, connects its configured mint, and runs a Cashu event loop in which every trade action is rejected with `CantDo(InvalidAction)` — the feature tracks replace those arms one at a time. With Cashu disabled the daemon is behaviourally identical to `main`. - `main.rs`: branch on `Settings::is_cashu_enabled()`. Cashu mode skips `LndConnector::new()` and the LN status probe, connects the mint via `CashuClient::connect` (exit(1) if unreachable, mirroring LND-refusal), attaches it to the context, and returns `run_cashu(ctx)`. The spam-gate warm-up is extracted into `install_spam_gate()` and shared by both paths; the Lightning branch is otherwise unchanged. - `app.rs`: factor the transport/validation prologue and the post-dispatch error tail out of `run()` into `accept_event` / `finalize_dispatch`, shared VERBATIM by `run` and the new `run_cashu` so the two loops cannot drift. Add `dispatch_cashu` with the closed allow-list (`Orders`, `LastTradeIndex`, `RestoreSession`, `TradePubkey` → `handle_message_action_no_ln`; `AddCashuEscrow` → the TA-1 stub; everything else → `InvalidAction`). - `app/add_cashu_escrow.rs` (new): stub `add_cashu_escrow_action` returning `InvalidAction`; Track A fills the body in its own file (G-1). - `app/context.rs`: `AppContext` carries `Option<Arc<CashuClient>>` with a `with_cashu_client` setter and `cashu_client()` accessor (`None` in Lightning mode). The `escrow: Arc<dyn EscrowBackend>` field the spec sketches is deferred: `run()` still threads `&mut LndConnector` and an unused `Arc<dyn EscrowBackend>` would be dead weight — added when Track B needs it. - `scheduler.rs`: gate the LN-only jobs (cancel-orders, retry-payments, dev-fee, bond payouts, stranded-bond reconcile) behind `!is_cashu_enabled()` so they never call `LndConnector::new()` on a node with no LND; make `job_info_event_send` self-skip when `LN_STATUS` is absent instead of panicking. Tests: `dispatch_cashu` blocks every order-lifecycle action with `InvalidAction` and routes the allow-list through the no-LN handler; the CF-5 stub returns `InvalidAction`. Full suite green (1023) with Cashu off. Refs: docs/cashu/01-fundamentals.md (CF-5)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d39115ca60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue; | ||
| }; | ||
|
|
||
| match cashu_client.check_token_unspent(token).await { |
There was a problem hiding this comment.
When an operator changes [cashu].mint_url while there are locked rows from the previous mint, this calls NUT-07 on the newly configured client without comparing the stored order.cashu_mint_url (or the token's mint URL) to cashu_client.mint_url(). Since checkstate only receives Y values and the existing verifier notes that unknown secrets can be reported Unspent, the restore log can mark an escrow from another mint as live even though this daemon is no longer connected to the mint that issued/redeems it; guard the mint URL before treating Ok(true) as live.
Useful? React with 👍 / 👎.
|
|
||
| // Re-hydrate in-flight locked escrows after a restart (TA-3), the | ||
| // Cashu analogue of the Lightning `find_held_invoices` resubscribe. | ||
| cashu_restore::restore_cashu_escrows(get_db_pool().as_ref(), &cashu_client).await; |
There was a problem hiding this comment.
This call is awaited before the Cashu branch builds the AppContext, starts the scheduler, or enters run_cashu; inside restore_cashu_escrows each order is checked serially and check_state can wait up to MINT_REQUEST_TIMEOUT (10s). With a slow mint and many locked escrows, a best-effort monitor can therefore stall all event processing for N×10s on every restart, so run the checks in the background or cap the whole restore pass instead of blocking boot.
Useful? React with 👍 / 👎.
|
|
||
| /// Re-hydrate and re-check every in-flight locked Cashu escrow at boot. | ||
| pub async fn restore_cashu_escrows(pool: &SqlitePool, cashu_client: &CashuClient) { | ||
| let locked = match find_locked_cashu_orders(pool).await { |
There was a problem hiding this comment.
find_locked_cashu_orders is explicitly designed and tested to return terminal locked rows because cashu_escrow_locked_at is never cleared; by feeding every returned row into the spent/pending warning, a normally released escrow whose proofs are spent, or a canceled terminal order, will be logged as needing attention on every later boot. The restore path is described as monitoring in-flight escrows, so filter out terminal statuses here (or ensure close paths clear the lock) before calling the mint.
Useful? React with 👍 / 👎.
…ashu mode Two review findings from PR #828: - Bond-expiry paths in `job_expire_pending_older_orders` open `LndConnector::new()` via the bond-release helpers. Cashu mode has no LND, so gate all three release calls (WaitingMakerBond, taker-bond, range maker bond) behind `!Settings::is_cashu_enabled()`. Bonds are mutually exclusive with Cashu (CF-1), so a cashu node carries no bond rows by construction; this only matters defensively for a reused/mode-switched DB, where the order still expires but no doomed LND connection is attempted. - The Cashu boot branch early-returns before the RPC startup block, silently skipping the admin gRPC server. Warn when `[rpc].enabled = true` in Cashu mode so the missing API is visible instead of a surprise (the LN-independent RPC subset is a follow-up — the server currently requires an LND client). Skipped (with reasons posted on the PR): the `check_trade_index`-before-dispatch note (pre-existing shared-prologue behaviour, identical to the Lightning loop; the actions become valid in TA-2 where the check belongs) and the boot-init DRY hoist (it reorders the Lightning boot, violating the byte-identical guarantee). cargo fmt + clippy -D warnings + full suite (1023) green.
Fill in the CF-5 stub with the full escrow-lock algorithm
(docs/cashu/02-track-a-lock.md §4). A `cashu`-mode node now accepts a
seller's `AddCashuEscrow`, fully validates the 2-of-3 token against the mint
and the order's trade keys, atomically advances `WaitingPayment → Active`,
publishes the updated order event, and notifies the buyer to send fiat.
Validate-fully-then-commit discipline (same as `release_action`):
1. resolve the order;
2. authorise the sender == the order's seller trade key (else `InvalidPeer`);
3. require `WaitingPayment` status;
4. extract the `CashuLockProof` (absent ⇒ `InvalidCashuToken`);
5. bind the mint: `proof.mint_url` must equal the configured mint
(`InvalidMintUrl`) — a cheap pre-check; step 7 enforces the authoritative
token↔mint binding;
6. bind `{P_B, P_S, P_M}` to THIS order — reject a proof whose stated keys
disagree with the order, and derive the keys handed to the mint from the
order (never from the proof), so the 2-of-3 can only lock to keys Mostro
already holds;
7. `verify_escrow_token` (2-of-3 + seller-recovery locktime floor
`now + escrow_locktime_days` + mint + amount + DLEQ + unspent); mint
unreachable ⇒ `CashuMintUnavailable`, malformed ⇒ `InvalidCashuToken`;
8. `update_order_cashu_escrow` CAS (lock + status advance in one write);
zero rows ⇒ idempotent no-op (replay/concurrent), no notification;
9. publish the Active order event (best-effort, logged on failure);
10. notify buyer + seller with `CashuEscrowLocked`.
The escrow token locks `order.amount` exactly (Option 2 — the Mostro fee is a
separate token added in TA-1f).
Tests: the deterministic pre-mint rejection paths (wrong sender ⇒
`InvalidPeer`, wrong status, missing proof ⇒ `InvalidCashuToken`) and the
`cashu_reason` error mapping. The mint-backed happy-path + replay tests are a
follow-up: they need a 2-of-3 token builder in the CF-3 harness (which today
ships only connectivity helpers).
Depends on CF-5 (dispatch seam + cashu_client). Base: feat/cashu-cf5-boot-integration
Refs: docs/cashu/02-track-a-lock.md (TA-1)
TA-1 implements AddCashuEscrow, so dispatch_cashu no longer routes it to InvalidAction — it runs the real lock handler. Remove it from the 'blocks every order-lifecycle action' assertion list.
Add the Cashu branch to `take_sell_action`/`take_buy_action` and the `show_cashu_escrow_request` helper (docs/cashu/02-track-a-lock.md §5), and unblock `NewOrder`/`TakeBuy`/`TakeSell` in `dispatch_cashu`. A taken order on a cashu node now asks the SELLER to lock a 2-of-3 escrow instead of paying a hold invoice, leaving the order in `WaitingPayment` where the TA-1 CAS expects it. Creatable and takeable ship together so the book never fills with untakeable orders (the orphan-order hazard CF-5 warns about). - `util::show_cashu_escrow_request`: advance to `WaitingPayment`, record both trade pubkeys, publish the order event, and enqueue an escrow request to the seller (`Action::WaitingSellerToPay` + `Payload::Order` carrying `amount`/`buyer_trade_pubkey`/`seller_trade_pubkey`) plus a bare "waiting for the seller" notice to the buyer. The buyer redeems ecash directly, so there is no buyer payout invoice. The escrow locks `order.amount` exactly (the fee is a separate token in TA-1f); the mint URL + locktime floor are node policy the daemon enforces authoritatively at lock validation (TA-1 §5/§7), so they are not carried in the request (the 0.14.0 protocol has no field for them). - `take_sell`/`take_buy`: early-return through `show_cashu_escrow_request` when `is_cashu_enabled()`, skipping the hold-invoice / buyer-invoice path. A supplied buyer invoice is ignored in Cashu mode. Lightning path unchanged. - `dispatch_cashu`: route `NewOrder`/`TakeBuy`/`TakeSell` to `handle_message_action_no_ln` (their real handlers); the gate test drops them from the `InvalidAction` list accordingly. Test: `show_cashu_escrow_request` advances the status, records both pubkeys, and enqueues the seller request (with the trade pubkeys + bare amount) and the buyer notice. Depends on CF-5 (+ TA-1 for the full e2e lock). Base: feat/cashu-ta1-lock-handler Refs: docs/cashu/02-track-a-lock.md (TA-2)
On boot in Cashu mode, re-hydrate in-flight locked escrows from the DB (the Cashu analogue of the Lightning `find_held_invoices` resubscribe) so a restart never loses sight of a locked-but-not-released escrow (docs/cashu/02-track-a-lock.md §6/§10). - `CashuClient::check_token_unspent`: whether every proof of a stored escrow token is still unspent at the mint (NUT-07), fail-closed on a short state reply. Reuses the Y-derivation the lock path already uses; does not re-run the full acceptance check (the token is immutable and was validated at lock). - `cashu_restore::restore_cashu_escrows`: list `find_locked_cashu_orders`, and for each re-check the token against the mint, logging a warning for any escrow that is spent/pending (redeemed or double-spent while the daemon was down). Best-effort, log-only — it never mutates order state (that belongs to the release/cancel/dispute tracks) and never blocks boot. - Wired into the `main.rs` Cashu boot branch after the mint connects, before the scheduler starts. Test: restore over an empty pool finds nothing and never contacts the mint. The mint-backed spent/live detection is exercised by the CF-3 integration harness. Depends on CF-4 + CF-5. Base: feat/cashu-ta2-take-flow Refs: docs/cashu/02-track-a-lock.md (TA-3)
6652b70 to
a16c745
Compare
d39115c to
456c3f4
Compare
a16c745 to
f76d5fe
Compare
ToRyVand
left a comment
ToRyVand
left a comment
There was a problem hiding this comment.
Reviewed the TA-3 change itself. The monitor is well-scoped and the fail-closed NUT-07 check is right. Two things though: the branch as pushed can't be reviewed or merged as-is, and the restore query has a growth problem that will surface as soon as Track B lands.
1. The stack has diverged — this branch does not contain #830
#831's head (456c3f4) is not a descendant of #830's head (f76d5fe). The two branches carry different commits for the same work:
| Work | on #830 | on #831 |
|---|---|---|
| CF-5 | abd9f3a (#828, merged to main) |
0793689 (own copy) |
| TA-1 | ec4a046 (#829, merged to main) |
4c0780a (own copy) |
| TA-2 | 0baf13a + f76d5fe (the atomic-claim fix) |
a16c745 (own copy, no CAS fix) |
So #830 has been rebased onto main and picked up the merged CF-5/TA-1, while #831 still carries the pre-merge duplicates. Three consequences:
- The header's "The diff shown is TA-3 only" isn't what GitHub renders — the +1198/−178 across 10 files includes duplicated TA-1/TA-2 content, which is why
add_cashu_escrow.rs(+367),util.rs(+148) andtake_sell/take_buyappear in a PR whose Changes list mentions none of them. - #828 and #829 are already merged, so this would re-apply landed work.
- It's also missing
f76d5fe— the atomic-claim fix on #830 that closes the concurrent-take race. TA-3's own copy of TA-2 predates it.
The real TA-3 change is small and reviewable: git diff a16c745 456c3f4 is 117 lines across 3 files (cashu/mod.rs +30, cashu_restore.rs +82, main.rs +5) — exactly the Changes list. That's what I reviewed below. A rebase onto current #830 (or onto main once #830 lands) would make the PR show that and nothing else.
Since #832 is stacked on this branch, it inherits the same divergence.
2. The restore set only ever grows, and boot waits on it
find_locked_cashu_orders selects WHERE cashu_escrow_locked_at IS NOT NULL with no status filter and no LIMIT — the test is even named find_locked_cashu_orders_includes_locked_regardless_of_status. And nothing ever clears that column; a test comment in the same file states it outright: "nothing ever clears cashu_escrow_locked_at". The only writer is the lock CAS.
Today that's harmless — Track A has no release path, so every locked escrow genuinely is in flight. But once release lands (Track B), a completed trade keeps cashu_escrow_locked_at set with a legitimately spent token, so on every subsequent boot the monitor will:
- re-check every escrow the node has ever locked, for the life of the database, and
- log
escrow token is spent/pending at the mint … — needs attentionfor each completed trade — a warning that means "healthy" in that case.
An unbounded, monotonically growing set of false alarms is the failure mode that trains operators to ignore the warning, which costs exactly the signal this PR exists to provide.
It also has a cost at boot. restore_cashu_escrows(...).await is awaited inline in the Cashu boot branch (main.rs), the loop is sequential, and each check_token_unspent is bounded by MINT_REQUEST_TIMEOUT (10 s) but the aggregate is not. With an unreachable mint the boot delay is N × 10 s — so "never blocks boot on a mint hiccup" holds per call but not overall. After a few hundred lifetime trades that's a long wait before the scheduler starts.
Three directions, any of which would do:
- filter the query to non-terminal statuses (cheapest, and it's where the intent already is — "in-flight");
- clear
cashu_escrow_locked_at(or stamp acashu_escrow_released_at) when the escrow resolves, which Track B needs a hook for anyway; - run the checks concurrently (
futures::future::join_all, asPriceManager::update_allalready does) and/or spawn the restore instead of awaiting it, so a sick mint delays the monitor rather than the daemon.
The first is probably the right one to do here since it keeps TA-3 self-contained; the second is the real fix once release exists.
3. The TA-3 code itself reads well
check_token_unspentdeliberately skipping the fullverify_escrow_tokenre-validation is correct and the reasoning is stated (token is immutable and was validated at lock; this only answers "still live?").- Fail-closed on a short state reply (
states.states.len() != expected → false) is the right default and is the kind of thing that's easy to get backwards. - Log-only with no state mutation, and the explicit note that state transitions belong to release/cancel/dispute, keeps the boundary clean.
- The "locked but carries no escrow token" arm flagging a data anomaly rather than silently skipping is a good touch.
I reviewed the isolated a16c745..456c3f4 commit rather than running the suite on the branch as pushed, since a tree that re-applies already-merged commits isn't the one that would land — happy to re-check after a rebase.
Contributor, not a maintainer — technical review only, no merge signal implied.
f76d5fe to
d4e88f9
Compare
2 participants
What & why
On boot in Cashu mode, re-hydrate in-flight locked escrows from the DB — the Cashu analogue of the Lightning path's
find_held_invoicesresubscribe — so a restart never loses sight of an escrow that is locked but not yet released (docs/cashu/02-track-a-lock.md§6/§10). This is the optional, recommended monitor PR of Track A.Changes
CashuClient::check_token_unspent: whether every proof of a stored escrow token is still unspent at the mint (NUT-07), fail-closed on a short state reply. Reuses the Y-derivation the lock path already uses; it does not re-run the fullverify_escrow_tokenacceptance check (the token is immutable and was validated at lock — this only answers "is it still live?").cashu_restore::restore_cashu_escrows: listfind_locked_cashu_orders, and for each re-check the token, logging a warning for any escrow that is spent/pending (redeemed or double-spent while the daemon was down). Best-effort, log-only — it never mutates order state (that belongs to the release/cancel/dispute tracks) and never blocks boot on a mint hiccup.main.rsCashu boot branch after the mint connects, before the scheduler starts.Tests
Checklist
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test— 1028 passedDepends on #830 (TA-2), #829 (TA-1), #828 (CF-5). Refs:
docs/cashu/02-track-a-lock.md(TA-3).🧪 Manual testing — step by step
Prereqs: the Cashu-mode
mostrodsetup from CF-5 (#828) — mint up (docker compose -f docker-compose.cashu.yml up -d),[cashu] enabled = true. This branch is stacked on TA-2 (contains CF-5 + TA-1 + TA-2 + TA-3). Run withRUST_LOG=mostro=info.1. Clean start — nothing locked
mostrodin Cashu mode.2. Re-hydrate a locked escrow across a restart
AddCashuEscrow), or seed a row directly:mostrod. Expected at boot:3. Unit test
cargo test --bin mostrod cashu_restoreExpected: the empty-pool restore path is a clean no-op that never contacts the mint. ✔️