feat(cashu): F3 — EscrowBackend trait + Lightning impl + Cashu stub

[grunch]

F3 · EscrowBackend trait + Lightning impl + Cashu stub

Foundation milestone F3 from docs/CASHU_ESCROW_ARCHITECTURE.md. Adds the escrow-backend seam so the order-action handlers stop calling LND directly, and stubs the Cashu backend for the parallel feature tracks. Opt-in feature is still off — Lightning behaviour is unchanged.

Design (as agreed)

• Thin primitive wrapper. EscrowBackend keeps today's hold-invoice primitives (create_hold_invoice, settle_hold_invoice, cancel_hold_invoice) rather than semantic lock/release/... names.
• Threaded param, not AppContext. Handlers take &mut dyn EscrowBackend; the dispatcher keeps its concrete LndConnector and relies on unsized coercion at the call site. AppContext-carried backend is deferred.

Changes

• src/escrow.rs (new): EscrowBackend trait; impl EscrowBackend for LndConnector (behaviour-preserving pass-through to the inherent methods); CashuBackend stub with unimplemented!() bodies for the lock/release/cancel tracks.
• src/util.rs: settle_seller_hold_invoice now takes &mut dyn EscrowBackend.
• src/app/release.rs: release_action now takes &mut dyn EscrowBackend (keeps LndConnector for the buyer-payout send_payment, which is not an escrow primitive).
• src/main.rs: register mod escrow.

Scope notes / deliberate deviations

• take_buy/take_sell/add_invoice don't thread a connector — they build their own inside show_hold_invoice, so the lock seam belongs to Track A.
• admin_settle/admin_cancel stay on concrete LndConnector because they feed the LN-only bond subsystem (bond::apply_bond_resolution), which is mutually exclusive with Cashu (design decision Remove 11th parameter #5). Dispute routing is Track D.
• cancel.rs retains its existing CancelLightning trait; folding it (and bond's SettleLightning) into EscrowBackend is left as future cleanup.

Verification

• cargo fmt --all — clean
• cargo clippy --bin mostrod --all-targets — clean
• cargo test --bin mostrod — 373 passed, 0 failed (behaviour-preserving)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Updated internal escrow infrastructure to support multiple payment backend implementations, improving platform flexibility for future payment processing options.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

• develop

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 73cad468-7417-46b6-8680-8578b8aa2978

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR introduces an EscrowBackend trait to abstract escrow operations (create hold invoice, settle, cancel) from concrete Lightning implementations. It implements the trait for LndConnector and stubs a CashuBackend for future work, then updates release_action and settle_seller_hold_invoice to accept trait objects instead of concrete types.

Changes

Escrow Backend Abstraction

Layer / File(s)	Summary
EscrowBackend trait definition and documentation src/escrow.rs	Module-level documentation and the new EscrowBackend trait define three async methods (create_hold_invoice, settle_hold_invoice, cancel_hold_invoice) with their signatures and result types.
LndConnector implementation src/escrow.rs	EscrowBackend trait is implemented for LndConnector as a thin pass-through that wires the three escrow methods to the connector's inherent hold-invoice methods and maps return types.
CashuBackend implementation (stubbed) src/escrow.rs	EscrowBackend trait is implemented for CashuBackend with all escrow methods stubbed via unimplemented!() and descriptive error messages indicating logic resides in separate Cashu feature tracks.
Module export and function signature updates src/main.rs, src/app/release.rs, src/util.rs	Module is exported in crate root; release_action and settle_seller_hold_invoice parameter types change from &mut LndConnector to &mut dyn EscrowBackend, with matching import additions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• MostroP2P/mostro#491: Both PRs modify src/app/release.rs's release_action function signature and parameters.

Suggested reviewers

• arkanoider
• Catrya
• AndreaDiazCorreia

Poem

🐰 A backend abstraction hops into the light,
Lightning and Cashu can both fit just right!
Traits dance where concrete once stood so tall,
Escrow now flexible—ready for all! 🌙✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main change: introducing an EscrowBackend trait with Lightning and Cashu implementations, which is the core objective of this PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/cashu-f3-escrow-backend

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[grunch]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

ℹ️ 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".

[grunch]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

src/escrow.rs (1)

39-43: 🏗️ Heavy lift

Consider abstracting the Lightning-specific return type in future iterations.

The create_hold_invoice method returns AddHoldInvoiceResp from fedimint-tonic-lnd, which is Lightning-specific. This means any alternative backend (like Cashu) would need to produce Lightning invoice responses, creating a leaky abstraction. For milestone F3 with stubbed Cashu methods this is acceptable, but Track A (Cashu lock) will likely need to revisit this design—consider introducing a backend-agnostic envelope type or splitting the trait into backend-specific variants.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/escrow.rs` around lines 39 - 43, The trait method create_hold_invoice
currently returns the Lightning-specific type AddHoldInvoiceResp, which leaks
LND into the trait; refactor by introducing a backend-agnostic envelope/result
type (e.g., HoldInvoiceResult or CreateHoldInvoiceResponse) used as the return
for create_hold_invoice, or split the trait into backend-specific variants
(e.g., LightningEscrow with create_hold_invoice returning AddHoldInvoiceResp and
CashuEscrow with a Cashu-specific response); update signatures that reference
create_hold_invoice to use the new abstract type and implement adapters that
convert AddHoldInvoiceResp into the envelope so non-Lightning backends can
implement the trait without depending on fedimint-tonic-lnd.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/escrow.rs`:
- Around line 39-43: The trait method create_hold_invoice currently returns the
Lightning-specific type AddHoldInvoiceResp, which leaks LND into the trait;
refactor by introducing a backend-agnostic envelope/result type (e.g.,
HoldInvoiceResult or CreateHoldInvoiceResponse) used as the return for
create_hold_invoice, or split the trait into backend-specific variants (e.g.,
LightningEscrow with create_hold_invoice returning AddHoldInvoiceResp and
CashuEscrow with a Cashu-specific response); update signatures that reference
create_hold_invoice to use the new abstract type and implement adapters that
convert AddHoldInvoiceResp into the envelope so non-Lightning backends can
implement the trait without depending on fedimint-tonic-lnd.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 322c62c4-8c18-49e5-bcb4-9c232876c0cf

📥 Commits

Reviewing files that changed from the base of the PR and between 1e0ef08 and 068cf73.

📒 Files selected for processing (4)

• src/app/release.rs
• src/escrow.rs
• src/main.rs
• src/util.rs

[ermeme]

Code Review Summary

Verdict: Approved

Strict pass on the current F3 scope. The diff is small and behavior-preserving: release_action and settle_seller_hold_invoice now accept the escrow trait, the LndConnector implementation delegates to the existing inherent methods, and no active runtime path instantiates the Cashu stub.

Looks Good

• The PR keeps Lightning behavior unchanged for the touched release/admin-settle path.
• The trait implementation explicitly delegates via LndConnector::..., avoiding accidental recursion through the trait method names.
• settle_hold_invoice / cancel_hold_invoice discard the LND response only after ?, preserving error propagation.
• cargo fmt --all -- --check passes locally.

Non-blocking notes

• The current EscrowBackend is still Lightning-shaped (AddHoldInvoiceResp, preimage/hash, hold-invoice method names). That is acceptable for the declared thin-primitive F3 scope, but the next Cashu tracks should not treat this as the final semantic backend API.
• The CashuBackend unimplemented!() methods are fine only because nothing wires it into runtime yet. The config-mode PR must fail closed until real implementations replace these stubs.

I could not run the full cargo test / cargo clippy suite in this runner because the filesystem is still effectively full, but source inspection plus cargo fmt --check found no blocking issue.

[mostronatorcoder]

I reviewed the current head commit 723e4b83849ffd585958cc51d73d991aed349cfe.

The EscrowBackend trait is the right direction, but this PR still lands the abstraction only partially applied.

Right now:

• release_action and settle_seller_hold_invoice were moved to &mut dyn EscrowBackend, which is good,
• but other escrow-opening / escrow-driving paths still instantiate LndConnector directly (src/app/bond/flow.rs, helper paths in src/util.rs, scheduler/runtime paths, etc.).

That leaves the seam inconsistent: some code goes through the backend abstraction, other escrow-relevant code still hardcodes Lightning directly. For a foundation PR, that is exactly the kind of half-switched state that makes later Cashu work harder to review and easier to miss in follow-up patches.

Requested changes

Please either:

1. finish threading the escrow abstraction through the remaining escrow-related paths that still construct / call LndConnector directly, or
2. narrow the stated scope so this PR only introduces the trait and updates the specific call sites that are explicitly in scope, without also carrying the broader Cashu schema preparation.

As written, the abstraction boundary is not yet consistently enforced, so I don't think it is ready to merge as the foundation seam.

[grunch]

Thanks for the review. I want to clarify the intended scope, because two of the three flagged paths are deliberate, documented carve-outs rather than half-switched state.

src/util.rs invoice_subscribe and src/app/bond/flow.rs (bond hold invoice) are intentionally not part of the escrow seam. This is stated explicitly in the src/escrow.rs module docs:

Payout (send_payment) and invoice subscription are not part of the escrow seam and remain on the concrete LndConnector.

The anti-abuse bond is LN-only and not part of the escrow seam; the admin handlers call bond::apply_bond_resolution with a raw connector.

The bond is a separate primitive from trade escrow (LN-only, not subject to the Cashu 2-of-3 multisig design), and invoice subscription / payout are Lightning transport concerns, not escrow operations. Threading them through EscrowBackend would be incorrect, not just out of scope — it would force the Cashu backend to implement Lightning-specific subscription/bond semantics that do not apply.

You are right about one path, though: src/scheduler.rs job_cancel_orders cancels a hold invoice to return locked trade funds to the seller on timeout — that is semantically cooperative_cancel and currently bypasses the seam. Good catch.

I am keeping this PR scoped as the behavior-preserving F3 foundation (trait + LightningBackend over the user-facing take_*/add_invoice/release/cancel/admin_* call sites). The daemon-driven scheduler timeout-cancel is a distinct path (not the user-initiated cancel.rs branch that Track C owns), so rather than widen this PR I have captured it explicitly as a tracked follow-up in the spec under Integration & Hardening in docs/CASHU_ESCROW_ARCHITECTURE.md, so it cannot be silently missed:

Daemon-driven timeout cancel through the escrow seam. scheduler.rs's job_cancel_orders currently returns locked trade funds to the seller by calling LndConnector::cancel_hold_invoice directly. [...] Route this path through EscrowBackend::cooperative_cancel so daemon-initiated timeout refunds work for either backend.

So: keep the foundation PR scoped to the trait plus the genuinely-escrow call sites, leave the documented LN-only carve-outs as-is, and close the one real gap in its own follow-up.