feat: add optional fee_token to CashuLockProof#157
Conversation
Carries the seller-funded Mostro fee token (locked to P_M) alongside the 2-of-3 escrow token, for the Cashu escrow funder-pays-at-lock fee model. The field is Option<String>, defaults to None, and is omitted from the wire form when absent (serde default + skip_serializing_if), so proofs without a fee serialize exactly as before. new() is unchanged (sets None); a with_fee_token builder attaches it. verify() is intentionally untouched — fee presence/amount is a daemon-side, config-dependent check.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesCashuLockProof fee token support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: fb02078b5d
ℹ️ 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".
| /// fee. Omitted from the wire form when absent, so proofs without a fee | ||
| /// serialize exactly as before. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub fee_token: Option<String>, |
There was a problem hiding this comment.
Because CashuLockProof is a public struct re-exported through the crate prelude, adding this public field is not just a wire-compatible extension: any downstream crate that currently constructs CashuLockProof { token, mint_url, buyer_pubkey, seller_pubkey, mostro_pubkey } with a struct literal will fail to compile with a missing fee_token field, despite new() staying unchanged. If this is meant to be an additive patch-compatible change, avoid changing the public struct shape or coordinate it with a breaking release.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, and correct: adding a public field to CashuLockProof (re-exported in the prelude) is source-breaking for struct-literal construction, even though new() and the wire format stay compatible.
This is intentional and coordinated with the release, not a patch-compatible change: it will ship as a breaking 0.14.0 minor (pre-1.0, so the minor bump is the breaking-allowed one), never as 0.13.x. In practice nothing in the org breaks today — the struct is only built via new() here, and the daemon does not construct it yet — but you are right that external struct-literal users would need the new field, which the 0.14.0 bump accounts for.
If we want future field additions to be non-breaking we could mark the struct #[non_exhaustive], but that is itself breaking and would land in the same 0.14.0; leaving it out for now while the crate is young.
1 participant
What
Adds an optional
fee_tokenfield toCashuLockProof.In the Cashu 2-of-3 escrow flow, Mostro is removed from the money path, so it
cannot skim its fee from the escrow token the way it does with Lightning hold
invoices. The agreed model (Track A spec, "funder-pays-at-lock") has the seller
fund the Mostro fee as a separate Cashu token locked 1-of-1 to
P_M,submitted alongside the 2-of-3 escrow token in the same
AddCashuEscrowmessage. This field carries that token.
Changes (
src/message.rs)CashuLockProof.fee_token: Option<String>— the serialized fee token.Nonewhen the node charges no fee.#[serde(default, skip_serializing_if = "Option::is_none")]so the wireform is backward- and forward-compatible: an old client's proof (no
fee_tokenkey) deserializes toNone, and a fee-less proof serializesbyte-identically to before (key omitted, not
null).new()unchanged — still 5 args, now setsfee_token: None; no existingcaller breaks.
with_fee_token(self, String) -> Self— immutable builder to attach thetoken opt-in.
verify()intentionally untouched — it is a protocol-shape check with noaccess to
Settings, so it cannot know whether the node charges a fee.Presence/amount of
fee_tokenis validated daemon-side (config-dependent).Tests
test_cashu_lock_proof_fee_token_round_trip—with_fee_token→ JSONcontains
fee_token→ round-trips preservingSome.test_cashu_lock_proof_without_fee_token_is_omitted_and_defaults_to_none—fee-less proof omits the key and parses back to
None.CashuLockProof/verify()tests pass unmodified.cargo fmt --check,clippy --all-targets --all-features -D warnings, and thefull test suite (83 lib + 4 doc-tests) are green.
Notes
cargo releaseflow.escrow spec live in the
mostrorepo (docs/cashu/02-track-a-lock.md).Summary by CodeRabbit
New Features
Tests