feat(cashu): C0 — upgrade mostro-core to 0.14 and pin the Cashu wire form - #229
Conversation
…form
First phase of docs/cashu/README.md. Pure dependency PR, as specified: no
features, no behaviour change, nothing Cashu-related is reachable at runtime.
0.13.1 -> 0.14.1 compiles with zero source changes on native and wasm; the
suite passes unmodified. What 0.14 adds is the frozen Cashu protocol surface:
Action::{AddCashuEscrow, CashuEscrowLocked, CashuPmSignature},
Payload::{CashuLockProof, CashuSignatures} and five CantDoReason variants.
The phase's other deliverable was to verify the wire form rather than trust
§2's illustrative JSON. Done, and pinned as tests (rust/src/mostro/cashu_wire.rs)
so an upstream rename fails our suite instead of a live trade: Action is
kebab-case, Payload snake_case (so the discriminator is `cashu_lock_proof`),
CashuLockProof's field names are as documented, and fee_token is
skip_serializing_if=Option::is_none — a node charging no fee produces the
pre-0.14 form byte-for-byte. §2's example turned out to be accurate.
One finding changes the plan. The escrow request (Mostro -> seller after a
take) is not merely undocumented, it is absent from mostro-core 0.14.1: the
Cashu fields live on the daemon-internal Order and NOT on SmallOrder, which is
what payloads actually carry, and there is no payload variant for the request.
So C5's seller side cannot be implemented without inventing a wire format.
A test asserts this, so the day upstream adds the carrier the suite goes red
and points at the decision. Risk #1 updated from "not yet published" to
"confirmed absent", with the blast radius scoped: only C5's seller side.
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThe PR upgrades ChangesCashu wire contracts
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
…ter all My first pass concluded the escrow request had no wire form and that C5's seller side was blocked. That was wrong, and the correction matters because it unblocks a phase. It is pinned — in the daemon branch feat/cashu-ta2-take-flow, not in mostro-core, because it needs no new types: show_cashu_escrow_request (src/util.rs) sends the seller Action::WaitingSellerToPay carrying Payload::Order(SmallOrder) with status WaitingPayment, both trade pubkeys and buyer_invoice = None, and sends the buyer the same action with no payload. mint_url, P_M and locktime are absent from the request by design: the client reads them from the 38385 info tags (C1) and the known Mostro pubkey. Absence from mostro-core was the observation; "therefore unpinned" was the faulty inference — I had not read the daemon source the spec pointed at. So C5 classifies by payload shape exactly as §4.4 planned: Lightning sends the seller PayInvoice + PaymentRequest, Cashu sends WaitingSellerToPay + Order. The test survives with a truer name — SmallOrder carrying no Cashu fields is now the pinned *assumption* behind that classification rather than evidence of a gap, and it still fails loudly if upstream changes shape.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@rust/Cargo.toml`:
- Line 19: Update the mostro-core dependency declaration in Cargo.toml from
version 0.14 to 0.14.1, preserving the existing dependency configuration while
enforcing the tested minimum version.
In `@rust/src/mostro/cashu_wire.rs`:
- Around line 228-265: Update the test
escrow_request_shape_is_still_unpinned_upstream to state and detect only Cashu
fields added to SmallOrder; remove any claim that it detects a new Payload
variant. In docs/cashu/README.md at line 572, require an explicit Payload API
review when upgrading dependencies instead of claiming the test suite detects
new request variants.
- Around line 125-157: Update rust/src/mostro/cashu_wire.rs lines 125-157 in
add_cashu_escrow_message_matches_the_documented_example to serialize an Order
containing the MessageKind, trade index, and version, then assert the complete
order envelope including order.version; update docs/cashu/README.md lines 96-102
only if needed to keep its documented field-for-field example aligned with the
test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 70033c53-47e7-4ae7-89c5-6e750a831e67
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
docs/cashu/README.mdrust/Cargo.tomlrust/src/mostro/cashu_wire.rsrust/src/mostro/mod.rs
The wire-form test claimed to match docs/cashu/README.md §2 "field for field" but serialized a bare MessageKind, so it pinned neither the `order` envelope Message adds nor the `version` stamp MessageKind::new applies — both of which the daemon parses. Asserting them surfaced a stale example: PROTOCOL_VER is 2 since the transport-v2 migration, not the 1 the doc carried over from protocol v1. - Cargo.toml: 0.14 → 0.14.1, the version the wire form was confirmed against and the one docs/cashu/README.md §2 names. The releases differ only in transport.rs (protocol-v1 deprecation), so Cargo.lock is unchanged; this aligns the declaration with the tested minimum. - cashu_wire.rs: serialize Message::Order(..) and assert the whole envelope, version included. - README §2: "version": 1 → 2. Verified: cargo test --lib (118 passed), cargo clippy -- -D warnings clean.
1 participant
Phase C0 of
docs/cashu/README.md. First of the Cashu series.Depends on: nothing. Blocks: C4, C5. Parallel with: C1, C2.
What this is
The spec's smallest, most boring phase on purpose: "No new features. Pure dependency PR — trivially reviewable, and it de-risks every later phase." Nothing Cashu-related is reachable at runtime after this merges.
mostro-core0.13.1 → 0.14.1 compiles with zero source changes, on native andwasm32. The suite passes unmodified (118 tests, +8 new).What 0.14 brings is the frozen protocol surface:
Action::{AddCashuEscrow, CashuEscrowLocked, CashuPmSignature},Payload::{CashuLockProof, CashuSignatures}, and theCantDoReasonvariants (InvalidCashuToken,CashuMintUnavailable,InvalidMintUrl,CashuEscrowNotLocked,CashuSignatureMissing).Wire form: verified, not assumed
C0's other deliverable was to "verify and document the exact serde wire form" — §2's JSON was explicitly marked "casing to be confirmed". It is now confirmed, and pinned as tests in
rust/src/mostro/cashu_wire.rsrather than left as prose, so an upstream rename fails our suite instead of a live trade:Actioncasingrename_all = "kebab-case"→add-cashu-escrow,cashu-escrow-locked,cashu-pm-signaturePayloaddiscriminatorrename_all = "snake_case"→cashu_lock_proof,cashu_signaturesCashuLockProoffieldstoken,mint_url,buyer_pubkey,seller_pubkey,mostro_pubkey,fee_tokenfee_tokenabsentskip_serializing_if = "Option::is_none"— omitted, notnull, so a no-fee node produces the pre-0.14 form byte-for-byteMessageKind::verifyAddCashuEscrowneeds an order id and aCashuLockProof;CashuPmSignaturerejects an empty signature vector§2's example turned out to be accurate. The doc is updated to say so and to point at the tests.
Finding: the escrow request, resolved
Risk #1 asked how the "Mostro → seller: escrow request" reaches the client. It is not in
mostro-core0.14.1 — no payload variant, and the Cashu fields (cashu_mint_url,cashu_escrow_token,cashu_escrow_locked_at) sit on the daemon-internalOrder, not onSmallOrder, which is what payloads carry.I first concluded from that it was unpinned and C5's seller side was blocked. That was wrong — I had not yet read the daemon source the spec points at. It reuses existing types, which is exactly why nothing was added to
mostro-core. Per daemon branchfeat/cashu-ta2-take-flow(show_cashu_escrow_request,src/util.rs):WaitingSellerToPayPayload::Order(SmallOrder)—status = WaitingPayment, both trade pubkeys set,buyer_invoice = NoneWaitingSellerToPaymint_url,P_Mand locktime are deliberately absent from the request: the client takes them from the 38385 info tags (C1) and the known Mostro pubkey.So C5 is not blocked, and it classifies by payload shape exactly as §4.4 planned — Lightning sends the seller
PayInvoice+PaymentRequest, Cashu sendsWaitingSellerToPay+Order. Risk #1 is marked resolved in the doc, with one caveat recorded: that daemon branch has diverged from itsmain, so the shape should be re-confirmed when Track A merges.The test survives under a truer name (
escrow_request_rides_on_an_unmodified_small_order):SmallOrderstaying free of Cashu fields is the assumption that makes the classification safe, and it still fails loudly if upstream changes shape.Test plan
cargo build— native, cleancargo check --target wasm32-unknown-unknown— cleancargo test— 118 pass, 0 fail (110 pre-existing, unmodified, + 8 new wire tests)cargo clippy -- -D warnings— cleanrust/src/api/change, so nofrb-generate.shrun is requiredSeries plan
C0 ✅ · C1 mode detection · C2 wallet core (cdk 0.17.3) · C3 wallet UI · C4 escrow primitives · C5 Track A · C6–C8 Tracks B/C/D · C9 wasm · C10 polish
Waves 0–1 (C0–C4) are implementable today. C5+ is gated on the finding above.
Summary by CodeRabbit
Documentation
Compatibility
mostro-core0.14 release.Validation