feat(bond): support Phase 1.5 anti-abuse bond invoice flow#592
Conversation
- New PayBondInvoiceScreen at /pay_bond/:orderId with explanatory text, invoice QR, copy/share buttons and cancel confirmation dialog - Route handling in AbstractMostroNotifier for the new action - Status label Awaiting deposit payment in trades list - New l10n keys in en/es/it (de/fr use English placeholders) - Notification mapper and restore manager updated to satisfy exhaustive-switch requirements
- actions map: Status.waitingTakerBond exposes payBondInvoice, cancel, for both buyer and seller roles - MostroMessageDetail: localized message and status label for the bond waiting state - Trade detail switch: render Pay deposit button navigating to pay_bond, and use the bond-specific confirmer dialog when cancelling from the bond waiting state
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThis PR implements a complete anti-abuse bond payment flow for Mostro P2P trades. It introduces new ChangesBond Payment Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/features/notifications/utils/notification_message_mapper.dart (1)
20-21: ⚡ Quick winUse bond-specific notification keys for
payBondInvoice.Right now bond deposit prompts reuse the generic “payment required” copy, which can blur the distinction between normal invoice payment and anti-abuse deposit payment. A dedicated title/message pair would make the notification intent clearer.
💡 Suggested mapping refinement
- case mostro.Action.payInvoice: - case mostro.Action.payBondInvoice: + case mostro.Action.payInvoice: return 'notification_payment_required_title'; + case mostro.Action.payBondInvoice: + return 'notification_bond_payment_required_title';- case mostro.Action.payInvoice: - case mostro.Action.payBondInvoice: + case mostro.Action.payInvoice: return 'notification_payment_required_message'; + case mostro.Action.payBondInvoice: + return 'notification_bond_payment_required_message';Also add corresponding ARB keys for all supported locales.
Also applies to: 123-124
🤖 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 `@lib/features/notifications/utils/notification_message_mapper.dart` around lines 20 - 21, The mapping for mostro.Action.payBondInvoice currently returns the generic 'notification_payment_required_title'; change this to a bond-specific key (e.g., 'notification_bond_payment_required_title') in the notification mapping function (where mostro.Action.payBondInvoice is handled) and update any other payBondInvoice-related returns in the same mapper (the other occurrence mentioned) to use corresponding bond-specific title/message keys (e.g., 'notification_bond_payment_required_message'); then add the new ARB keys and translations for all supported locales so the new bond-specific title/message are available in the app.
🤖 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 `@lib/features/order/screens/pay_bond_invoice_screen.dart`:
- Around line 118-123: The QR widget is rendered even when lnInvoice is empty,
producing a confusing blank QR; update the build logic around QrImageView to
conditionally render a placeholder/loading state (e.g., a spinner, message, or
empty-state widget) whenever lnInvoice is null or an empty string instead of
passing it into QrImageView, and only create QrImageView with data: lnInvoice
when lnInvoice.isNotEmpty; adjust related errorStateBuilder if needed to handle
real QR errors separately.
- Around line 57-59: The navigation happens before the cancellation completes;
change the sequence so you call and await orderNotifier.cancelOrder() first and
only call context.go('/') after it succeeds; wrap the await
orderNotifier.cancelOrder() in a try/catch (or handle the returned result) to
detect failure and show an error (e.g., via ScaffoldMessenger or the notifier's
error state) instead of navigating; update the code around
orderNotifierProvider(orderId).notifier, cancelOrder(), and context.go('/')
accordingly.
---
Nitpick comments:
In `@lib/features/notifications/utils/notification_message_mapper.dart`:
- Around line 20-21: The mapping for mostro.Action.payBondInvoice currently
returns the generic 'notification_payment_required_title'; change this to a
bond-specific key (e.g., 'notification_bond_payment_required_title') in the
notification mapping function (where mostro.Action.payBondInvoice is handled)
and update any other payBondInvoice-related returns in the same mapper (the
other occurrence mentioned) to use corresponding bond-specific title/message
keys (e.g., 'notification_bond_payment_required_message'); then add the new ARB
keys and translations for all supported locales so the new bond-specific
title/message are available in the app.
🪄 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
Run ID: 713ad311-f306-4fb3-80e4-b710762aa494
📒 Files selected for processing (17)
lib/core/app_routes.dartlib/data/models/enums/action.dartlib/data/models/enums/status.dartlib/features/notifications/utils/notification_message_mapper.dartlib/features/notifications/widgets/notification_item.dartlib/features/order/models/order_state.dartlib/features/order/notifiers/abstract_mostro_notifier.dartlib/features/order/screens/pay_bond_invoice_screen.dartlib/features/restore/restore_manager.dartlib/features/trades/screens/trade_detail_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dartlib/features/trades/widgets/trades_list_item.dartlib/l10n/intl_de.arblib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_fr.arblib/l10n/intl_it.arb
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed the PR and also checked the existing review discussion before deciding. The previously raised concern about optimistic navigation on cancel is not a blocker here, because Catrya already clarified that this mirrors the existing app-wide cancellation pattern and failures are surfaced asynchronously through the existing cant-do flow. That is a broader architectural cleanup, not something this PR should be forced to solve in isolation.
On the actual change set, the implementation is coherent and matches the Phase 1.5 bond flow well:
Action.payBondInvoiceandStatus.waitingTakerBondare integrated consistently through routing, notification handling, restore, trade detail, and trades list presentation.- The new
/pay_bond/:orderIdscreen covers the key UX actions: explanation, QR, copy, share, and cancel confirmation. OrderStateexposes the correct action set forwaitingTakerBondfor both roles, which keeps the FSM explicit instead of relying on ad-hoc UI conditionals.- Restore support is present, so reopened sessions can land back in the bond flow correctly.
- CI is green.
I do not see a correctness issue here that should block merge. The remaining CodeRabbit points are either copy-level refinements or follow-up polish, not merge blockers for this phase. Approved.
1 participant
Adds the full mobile UX for the anti-abuse deposit (bond) that the taker must pay before continuing with a trade. The taker locks a Lightning hold invoice as a deposit; it is released when the trade finishes correctly.
What's included
/pay_bond/<orderId>screen shown when the deposit invoice arrives, with:Test plan
/pay_bond/
Summary by CodeRabbit
New Features
Localization