feat(ui): PayBondInvoice popup for Mostro Phase 1.5 anti-abuse bond#68
Conversation
mostro-core 0.11.0 introduces `Action::PayBondInvoice` and `Status::WaitingTakerBond` so takers can be asked to lock an optional anti-abuse bond before the trade starts. Adds a dedicated bond popup mirroring PayInvoice but visually distinct (shield title, "Locked, not spent — refunded on normal completion" disclaimer) and gated on `WaitingTakerBond | None`. The take-order sync path and the DM listener both route `PayBondInvoice` through `OperationResult::PaymentRequestRequired`, which now carries the originating `Action` so `order_chat_static` covers either flavor. Additive only: daemons without bonds keep using `PayInvoice` and the no-bond flow is unchanged. Also updates docs (README, MESSAGE_FLOW_AND_PROTOCOL, TUI_INTERFACE, DM_LISTENER_FLOW, buy/sell order flow) to describe the new popup, status, and the Phase 1.5 taker phase 0. Co-authored-by: Cursor <cursoragent@cursor.com>
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds PayBondInvoice and WaitingTakerBond support: bumps mostro-core to 0.11.0, threads action through PaymentRequest result, updates DM upsert/extraction and notification gating, adds a bond invoice popup renderer and labels, extends key handlers, and updates protocol/docs. ChangesAnti-Abuse Bond Invoice Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ui/orders.rs (1)
596-598: ⚡ Quick winConsider adding test coverage for
WaitingTakerBondstatus.The timeline step logic correctly treats
Status::WaitingTakerBondlikePending(pre-trade state), but the existing test suite (lines 812-924) doesn't include test cases for this new status. While the logic is straightforward, adding tests would improve confidence and serve as documentation for the Phase 1.5+ bond flow.📋 Example test to add
#[test] fn buy_taker_waiting_taker_bond_maps_to_seller_payment_step() { let m = sample_order_message( Action::PayBondInvoice, Some(mostro_core::order::Kind::Buy), Some(false), Some(mostro_core::order::Status::WaitingTakerBond), ); assert_eq!( message_trade_timeline_step(&m), FlowStep::BuyFlowStep(StepLabelsBuy::StepSellerPayment) ); } #[test] fn sell_taker_waiting_taker_bond_maps_to_seller_payment_step() { let m = sample_order_message( Action::PayBondInvoice, Some(mostro_core::order::Kind::Sell), Some(false), Some(mostro_core::order::Status::WaitingTakerBond), ); assert_eq!( message_trade_timeline_step(&m), FlowStep::SellFlowStep(StepLabelsSell::StepSellerPayment) ); }Also applies to: 620-622
🤖 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/ui/orders.rs` around lines 596 - 598, Add unit tests that cover Status::WaitingTakerBond mapping in message_trade_timeline_step: create two tests using sample_order_message with Action::PayBondInvoice, Kind::Buy and Kind::Sell respectively, and Status::WaitingTakerBond, then assert the returned FlowStep equals FlowStep::BuyFlowStep(StepLabelsBuy::StepSellerPayment) for the buy case and FlowStep::SellFlowStep(StepLabelsSell::StepSellerPayment) for the sell case; place them alongside the existing tests that cover other statuses so the Phase 1.5+ bond flow is exercised.
🤖 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/ui/orders.rs`:
- Around line 596-598: Add unit tests that cover Status::WaitingTakerBond
mapping in message_trade_timeline_step: create two tests using
sample_order_message with Action::PayBondInvoice, Kind::Buy and Kind::Sell
respectively, and Status::WaitingTakerBond, then assert the returned FlowStep
equals FlowStep::BuyFlowStep(StepLabelsBuy::StepSellerPayment) for the buy case
and FlowStep::SellFlowStep(StepLabelsSell::StepSellerPayment) for the sell case;
place them alongside the existing tests that cover other statuses so the Phase
1.5+ bond flow is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b40990f-4611-4b90-bc82-830ee845de17
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
Cargo.tomldocs/DM_LISTENER_FLOW.mddocs/MESSAGE_FLOW_AND_PROTOCOL.mddocs/README.mddocs/TUI_INTERFACE.mddocs/buy order flow.mddocs/sell order flow.mdsrc/ui/app_state.rssrc/ui/key_handler/enter_handlers.rssrc/ui/key_handler/message_handlers.rssrc/ui/key_handler/mod.rssrc/ui/message_notification.rssrc/ui/orders.rssrc/util/dm_utils/mod.rssrc/util/dm_utils/notifications_ch_mng.rssrc/util/dm_utils/order_ch_mng.rssrc/util/order_utils/helper.rssrc/util/order_utils/take_order.rs
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this strictly and I found a blocker in the PayBondInvoice handling.
Blocker
In the DM listener path, the popup amount shown for PayBondInvoice / PayInvoice is derived from the embedded order amount instead of from the payment-request amount.
Specifically, in src/util/dm_utils/mod.rs:
Action::PayInvoice | Action::PayBondInvoice => match &inner_kind.payload {
Some(Payload::PaymentRequest(opt_order, invoice, _)) => {
(opt_order.as_ref().map(|o| o.amount), Some(invoice.clone()))
}
_ => (None, None),
},That uses opt_order.amount, which is the trade/order amount, not the invoice amount carried by Payload::PaymentRequest.
This is especially dangerous for PayBondInvoice, because the anti-abuse bond amount is not the same thing as the order amount. So the UI can end up showing the user a wrong sats figure in the bond popup even though the actual invoice string is different.
The take-order sync path already forwards sat_amount from the payment-request payload correctly (*opt_amount in src/util/order_utils/take_order.rs), but the DM listener path does not mirror that behavior.
Since this is a payment-facing popup and can display a misleading amount, I am marking this as request changes.
I think the fix is to source sat_amount from the PaymentRequest amount field in the DM path as well, and keep the two code paths consistent.
There was a problem hiding this comment.
Re-reviewed the latest commit.
The blocker I raised is resolved now.
The DM listener path now prefers the explicit PaymentRequest amount override for PayBondInvoice, and only falls back to the embedded order amount when needed for legacy PayInvoice behavior. The synchronous take_order path was updated consistently as well.
That was the important correctness fix here, because the payment-facing popup must not show a misleading sats amount for the anti-abuse bond flow.
I also checked the small follow-up timeline tests added in src/ui/orders.rs, and I do not see a new blocker in the updated version.
1 participant
Summary
mostro-core0.11.0 introducesAction::PayBondInvoiceandStatus::WaitingTakerBondso takers can be asked to lock an optional anti-abuse bond before the trade starts. This PR wires that new action end-to-end through the TUI.render_pay_bond_invoiceinsrc/ui/message_notification.rs) mirroringPayInvoicechrome but visually distinct: shield title, "Bond invoice to pay (… sats)" label, and a yellow "Locked, not spent — refunded on normal completion" disclaimer. Primary button is Acknowledge (payment happens in the user's wallet); Cancel Order still sendsAction::Cancel.order_status ∈ { WaitingTakerBond, None }(invoice_popup_allowed_for_order_statusinsrc/ui/orders.rs).Action:src/util/order_utils/take_order.rs) — branches oninner_message.actionand forwards throughOperationResult::PaymentRequestRequired { action, … }.src/util/dm_utils/mod.rs,order_ch_mng.rs,notifications_ch_mng.rs) — addedPayBondInvoicetoupsert_order_from_trade_dm, invoice/sat_amount extraction, actionability gate, and the auto-popup arm. The non-overwrite guard at L243 oforder_ch_mng.rsnow includesPayBondInvoice.inferred_status_from_trade_actionmapsPayBondInvoice → WaitingTakerBond; nullrequest_idallowlist extended to cover bond DMs.listing_step_from_statushandlesWaitingTakerBondfor bothKind::BuyandKind::Sellso the column doesn't jump while the taker is reviewing the bond popup.C, action-selection cycling, clear-copied-indicator, and Enter dispatch all widened to includePayBondInvoice.No-bond safety (important)
Mostro bonds are configurable in mostrod and not mandatory. All changes are additive:
PayInvoicefor hold invoices.Action::PayBondInvoice(or status reachesWaitingTakerBond).Docs
Updated
docs/README.md,docs/MESSAGE_FLOW_AND_PROTOCOL.md,docs/TUI_INTERFACE.md,docs/DM_LISTENER_FLOW.md,docs/buy order flow.md,docs/sell order flow.mdto describe:Action::PayBondInvoice/Status::WaitingTakerBondsemantics, and the configurable nature of bonds in mostrod;PayInvoice(pay-bond-invoicediscriminator frommostro-core0.11.0, replaces the Phase 1 hack of reusingPayInvoicefor bonds).Summary by CodeRabbit
New Features
Documentation
Chores