feat(cashu): implement Track B - release happy path#763
Conversation
This implements the Cashu 2-of-3 escrow release happy path as per docs/CASHU_ESCROW_ARCHITECTURE.md (Track B). It bypasses Lightning invoice settlement and directly transitions the order to Success status.
Walkthrough
ChangesCashu Escrow Release Path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Generates and sends the CashuPmSignature message to the buyer during the happy path release, as a fallback in case the seller fails to send their signature out-of-band via Nostr DM. Ensures the buyer always receives a valid 2nd signature (P_M) if the seller confirms the release to Mostro.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/release.rs (1)
333-343:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
HoldInvoicePaymentSettledis sent to seller even in Cashu mode where no hold invoice exists.This message block runs after the
if is_cashu { ... } else { ... }branches, meaning the seller receivesAction::HoldInvoicePaymentSettledfor Cashu orders despite there being no Lightning hold invoice. This is semantically incorrect and may confuse clients.Consider guarding this with
!is_cashuor introducing a Cashu-specific action.Proposed fix
- // We send a HoldInvoicePaymentSettled message to seller, the client should - // indicate *funds released* message to seller - enqueue_order_msg( - request_id, - Some(order.id), - Action::HoldInvoicePaymentSettled, - None, - seller_pubkey, - None, - ) - .await; + // We send a HoldInvoicePaymentSettled message to seller, the client should + // indicate *funds released* message to seller + if !is_cashu { + enqueue_order_msg( + request_id, + Some(order.id), + Action::HoldInvoicePaymentSettled, + None, + seller_pubkey, + None, + ) + .await; + }🤖 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/app/release.rs` around lines 333 - 343, The code enqueues Action::HoldInvoicePaymentSettled unconditionally even for Cashu orders where no hold invoice exists; update the call sites around enqueue_order_msg in release.rs so that Action::HoldInvoicePaymentSettled is only enqueued when !is_cashu (or replace with a Cashu-specific action), i.e., guard the enqueue_order_msg(...) call with a check on is_cashu (or branch to a Cashu action) to avoid sending HoldInvoicePaymentSettled for Cashu orders.
🤖 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 `@src/app/release.rs`:
- Around line 234-260: The token parsing / key conversion / signing currently
swallows errors; update the block that builds pm_signatures so each fallible
operation logs failures (include the offending token_str/order.id and request_id
for context): log errors from
cdk::nuts::Token::from_str(order.cashu_escrow_token), from
cdk::nuts::nut01::SecretKey::from_str(my_keys.secret_key().to_secret_hex()), and
from p_m_secret.sign(&msg) before continuing, and still push any successful
CashuProofSignature::new entries into pm_signatures; use the existing logging
framework (e.g., tracing::error! or the module's logger) and ensure logs mention
functions/symbols like Token::from_str, SecretKey::from_str, p_m_secret.sign,
and CashuProofSignature so failures are diagnosable.
---
Outside diff comments:
In `@src/app/release.rs`:
- Around line 333-343: The code enqueues Action::HoldInvoicePaymentSettled
unconditionally even for Cashu orders where no hold invoice exists; update the
call sites around enqueue_order_msg in release.rs so that
Action::HoldInvoicePaymentSettled is only enqueued when !is_cashu (or replace
with a Cashu-specific action), i.e., guard the enqueue_order_msg(...) call with
a check on is_cashu (or branch to a Cashu action) to avoid sending
HoldInvoicePaymentSettled for Cashu orders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| let mut pm_signatures = Vec::new(); | ||
| let token_str = order.cashu_escrow_token.as_ref().unwrap(); | ||
| if let Ok(token) = cdk::nuts::Token::from_str(token_str) { | ||
| let secrets = token.token_secrets(); | ||
| if let Ok(p_m_secret) = cdk::nuts::nut01::SecretKey::from_str(&my_keys.secret_key().to_secret_hex()) { | ||
| for secret in secrets { | ||
| let msg = secret.to_bytes(); | ||
| if let Ok(sig) = p_m_secret.sign(&msg) { | ||
| pm_signatures.push(mostro_core::message::CashuProofSignature::new( | ||
| secret.to_string(), | ||
| sig.to_string(), | ||
| )); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !pm_signatures.is_empty() { | ||
| enqueue_order_msg( | ||
| request_id, | ||
| Some(order.id), | ||
| Action::CashuPmSignature, | ||
| Some(Payload::CashuSignatures(pm_signatures)), | ||
| buyer_pubkey, | ||
| None, | ||
| ).await; | ||
| } |
There was a problem hiding this comment.
Log errors during Cashu signature generation to aid debugging.
The nested if let Ok(...) chains silently discard errors from token parsing, key conversion, and signing. If any step fails, the buyer may receive no CashuPmSignature (or partial signatures), and there's no diagnostic trail. Since P_M signatures are critical for the buyer to redeem the Cashu escrow, failures here should be logged.
Proposed fix with error logging
let token_str = order.cashu_escrow_token.as_ref().unwrap();
- if let Ok(token) = cdk::nuts::Token::from_str(token_str) {
- let secrets = token.token_secrets();
- if let Ok(p_m_secret) = cdk::nuts::nut01::SecretKey::from_str(&my_keys.secret_key().to_secret_hex()) {
- for secret in secrets {
- let msg = secret.to_bytes();
- if let Ok(sig) = p_m_secret.sign(&msg) {
- pm_signatures.push(mostro_core::message::CashuProofSignature::new(
- secret.to_string(),
- sig.to_string(),
- ));
+ match cdk::nuts::Token::from_str(token_str) {
+ Ok(token) => {
+ let secrets = token.token_secrets();
+ match cdk::nuts::nut01::SecretKey::from_str(&my_keys.secret_key().to_secret_hex()) {
+ Ok(p_m_secret) => {
+ for secret in secrets {
+ let msg = secret.to_bytes();
+ match p_m_secret.sign(&msg) {
+ Ok(sig) => {
+ pm_signatures.push(mostro_core::message::CashuProofSignature::new(
+ secret.to_string(),
+ sig.to_string(),
+ ));
+ }
+ Err(e) => {
+ tracing::warn!("Order {}: failed to sign Cashu secret: {}", order.id, e);
+ }
+ }
+ }
}
+ Err(e) => {
+ tracing::warn!("Order {}: failed to parse P_M secret key: {}", order.id, e);
+ }
}
}
+ Err(e) => {
+ tracing::warn!("Order {}: failed to parse Cashu escrow token: {}", order.id, e);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut pm_signatures = Vec::new(); | |
| let token_str = order.cashu_escrow_token.as_ref().unwrap(); | |
| if let Ok(token) = cdk::nuts::Token::from_str(token_str) { | |
| let secrets = token.token_secrets(); | |
| if let Ok(p_m_secret) = cdk::nuts::nut01::SecretKey::from_str(&my_keys.secret_key().to_secret_hex()) { | |
| for secret in secrets { | |
| let msg = secret.to_bytes(); | |
| if let Ok(sig) = p_m_secret.sign(&msg) { | |
| pm_signatures.push(mostro_core::message::CashuProofSignature::new( | |
| secret.to_string(), | |
| sig.to_string(), | |
| )); | |
| } | |
| } | |
| } | |
| } | |
| if !pm_signatures.is_empty() { | |
| enqueue_order_msg( | |
| request_id, | |
| Some(order.id), | |
| Action::CashuPmSignature, | |
| Some(Payload::CashuSignatures(pm_signatures)), | |
| buyer_pubkey, | |
| None, | |
| ).await; | |
| } | |
| let mut pm_signatures = Vec::new(); | |
| let token_str = order.cashu_escrow_token.as_ref().unwrap(); | |
| match cdk::nuts::Token::from_str(token_str) { | |
| Ok(token) => { | |
| let secrets = token.token_secrets(); | |
| match cdk::nuts::nut01::SecretKey::from_str(&my_keys.secret_key().to_secret_hex()) { | |
| Ok(p_m_secret) => { | |
| for secret in secrets { | |
| let msg = secret.to_bytes(); | |
| match p_m_secret.sign(&msg) { | |
| Ok(sig) => { | |
| pm_signatures.push(mostro_core::message::CashuProofSignature::new( | |
| secret.to_string(), | |
| sig.to_string(), | |
| )); | |
| } | |
| Err(e) => { | |
| tracing::warn!("Order {}: failed to sign Cashu secret: {}", order.id, e); | |
| } | |
| } | |
| } | |
| } | |
| Err(e) => { | |
| tracing::warn!("Order {}: failed to parse P_M secret key: {}", order.id, e); | |
| } | |
| } | |
| } | |
| Err(e) => { | |
| tracing::warn!("Order {}: failed to parse Cashu escrow token: {}", order.id, e); | |
| } | |
| } | |
| if !pm_signatures.is_empty() { | |
| enqueue_order_msg( | |
| request_id, | |
| Some(order.id), | |
| Action::CashuPmSignature, | |
| Some(Payload::CashuSignatures(pm_signatures)), | |
| buyer_pubkey, | |
| None, | |
| ).await; | |
| } |
🤖 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/app/release.rs` around lines 234 - 260, The token parsing / key
conversion / signing currently swallows errors; update the block that builds
pm_signatures so each fallible operation logs failures (include the offending
token_str/order.id and request_id for context): log errors from
cdk::nuts::Token::from_str(order.cashu_escrow_token), from
cdk::nuts::nut01::SecretKey::from_str(my_keys.secret_key().to_secret_hex()), and
from p_m_secret.sign(&msg) before continuing, and still push any successful
CashuProofSignature::new entries into pm_signatures; use the existing logging
framework (e.g., tracing::error! or the module's logger) and ensure logs mention
functions/symbols like Token::from_str, SecretKey::from_str, p_m_secret.sign,
and CashuProofSignature so failures are diagnosable.
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed the current head of this PR and I am requesting changes.
There are two blocking issues.
- The Cashu mode event loop is explicitly not wiring the trading actions yet.
run_cashu() rejects the core trade actions (NewOrder, TakeSell, TakeBuy, AddInvoice, Release, Cancel, AdminCancel, AdminSettle) with CantDoReason::InvalidAction instead of actually driving the escrow flow.
That means this PR does not just introduce an incomplete backend seam, it starts a Cashu-mode daemon that cannot process the core trading flow at all. That is too incomplete to merge behind an operational mode switch.
- The code still presents the same mixed-mode problem around the "LND not required" claim.
Examples in this head:
src/main.rslogsStarting in Cashu escrow mode (LND not required)- but important Lightning-specific paths still exist directly in
src/util.rs(show_hold_invoice,invoice_subscribe) andsrc/app/bond/flow.rs
So the mode split is still not consistently enforced across the runtime surface.
I also re-checked the Cashu helper code and this branch still carries the same src/cashu/mod.rs clippy issues that were already visible in the sibling work (dead code / useless same-type conversions), which is another sign the branch is not merge-ready.
2 participants
This PR implements Track B of the Cashu 2-of-3 escrow architecture described in
docs/CASHU_ESCROW_ARCHITECTURE.md.In Cashu mode, the Mostro daemon does not hold custody of funds. Therefore, in the happy path release flow, Mostro simply needs to transition the order state to
Successand bypass the Lightning hold invoice settlement.The cryptographic signatures for the release are sent out-of-band directly from the seller to the buyer via Nostr DM.
Successinstead ofSettledHoldInvoice.PurchaseCompletedmessage to the buyer immediately.Summary by CodeRabbit
Release Notes