From c164148f4e84716fa4b883ef85471ce99b2f0372 Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sun, 13 Sep 2026 23:25:39 +0200 Subject: [PATCH 01/12] fix(order): drop an invoice when its take cycle ends Taking an order, paying the bond and the escrow, cancelling as the taker and then taking the same order again showed the *previous* cycle's escrow invoice on the anti-abuse bond screen. Mostro cancels that hold invoice when the take is cancelled, so paying it fails with INCORRECT_PAYMENT_DETAILS and the order cannot be taken any more. OrderState is keyed by order id and kept paymentRequest across cycles, so the escrow bolt11 outlived the take that produced it. On top of that a cancelled order outranks every waiting phase, so the stale-transition guard dropped the whole next cycle during a replay, leaving the old invoice as the newest thing on screen. - OrderState.updateWith clears paymentRequest on the statuses that end a take cycle (endsTradeCycle), the way it already clears the taker reputation on a republish; copyWith gains clearPaymentRequest. - OrderNotifier resets its state when the user takes an order again, and sync() restarts the replay on the first message of a new cycle instead of dropping it as stale (wouldRejectAsStale). - PayBondInvoiceScreen only renders an invoice while the order is in the bond phase, and shows an explanatory empty state otherwise, so no entry point (notification card, trade detail) can surface a dead bolt11. Fixes #731 --- lib/features/order/models/order_state.dart | 43 +++- .../order/notifiers/order_notifier.dart | 41 +++- .../screens/pay_bond_invoice_screen.dart | 51 ++++- lib/l10n/intl_de.arb | 1 + lib/l10n/intl_en.arb | 1 + lib/l10n/intl_es.arb | 1 + lib/l10n/intl_fr.arb | 1 + lib/l10n/intl_it.arb | 1 + lib/l10n/intl_pt.arb | 1 + .../order/models/order_state_retake_test.dart | 185 ++++++++++++++++++ 10 files changed, 320 insertions(+), 6 deletions(-) create mode 100644 test/features/order/models/order_state_retake_test.dart diff --git a/lib/features/order/models/order_state.dart b/lib/features/order/models/order_state.dart index 0482cf2d1..6bcd4577a 100644 --- a/lib/features/order/models/order_state.dart +++ b/lib/features/order/models/order_state.dart @@ -92,12 +92,15 @@ class OrderState { bool? fiatWasSent, UserInfo? peerReputation, bool clearPeerReputation = false, + bool clearPaymentRequest = false, }) { return OrderState( status: status ?? this.status, action: action ?? this.action, order: order ?? this.order, - paymentRequest: paymentRequest ?? this.paymentRequest, + paymentRequest: clearPaymentRequest + ? null + : paymentRequest ?? this.paymentRequest, cantDo: cantDo ?? this.cantDo, dispute: dispute ?? this.dispute, peer: peer ?? this.peer, @@ -109,6 +112,19 @@ class OrderState { ); } + /// Statuses that close the current take cycle: the order either went back + /// to the book or ended for good. + /// + /// Everything Mostro issued for that cycle — bond and escrow hold invoices + /// above all — is cancelled node-side when this happens, so no payload from + /// it may survive into the next take of the same order id. + static bool endsTradeCycle(Status status) => + status == Status.pending || + status == Status.canceled || + status == Status.canceledByAdmin || + status == Status.cooperativelyCanceled || + status == Status.expired; + /// Dispute statuses in which the dispute is over: a resolution has already /// been applied and no further admin action is expected for it. static const _terminalDisputeStatuses = { @@ -256,11 +272,20 @@ class OrderState { logger.d('Order returned to pending: dropping the taker reputation'); } - // Preserve PaymentRequest correctly + // Preserve PaymentRequest correctly — but only within the take cycle it + // belongs to. Mostro cancels the bond and escrow hold invoices when the + // cycle ends, so carrying one into the next take renders a bolt11 that + // can no longer be paid (INCORRECT_PAYMENT_DETAILS). + final bool cycleEnded = endsTradeCycle(newStatus); PaymentRequest? newPaymentRequest; if (message.payload is PaymentRequest) { newPaymentRequest = message.getPayload(); logger.d('New PaymentRequest found in message'); + } else if (cycleEnded) { + newPaymentRequest = null; + if (paymentRequest != null) { + logger.d('Take cycle ended ($newStatus): dropping the stale invoice'); + } } else { newPaymentRequest = paymentRequest; // Preserve existing } @@ -400,6 +425,7 @@ class OrderState { ? message.getPayload()!.order : order, paymentRequest: newPaymentRequest, + clearPaymentRequest: newPaymentRequest == null, cantDo: message.getPayload() ?? cantDo, dispute: updatedDispute, peer: newPeer, @@ -460,6 +486,19 @@ class OrderState { return !isRepublish; } + /// Whether [message] would be dropped by the stale-transition guard. + /// + /// Lets callers replaying persisted history tell a genuinely late copy from + /// the first message of a *new* take cycle: after a cancel, every later + /// message looks backwards to the guard, because a cancelled order outranks + /// every waiting phase. Combined with [endsTradeCycle] on the current + /// status, that is the signal to start the replay over instead of dropping + /// the rest of the history (#731). + bool wouldRejectAsStale(MostroMessage message) => isStaleTransition( + message.action, + _getStatusFromAction(message.action, message.getPayload()?.status), + ); + /// Maps actions to their corresponding statuses based on mostrod DM messages Status _getStatusFromAction(Action action, Status? payloadStatus) { switch (action) { diff --git a/lib/features/order/notifiers/order_notifier.dart b/lib/features/order/notifiers/order_notifier.dart index bce7b2f49..774ff9427 100644 --- a/lib/features/order/notifiers/order_notifier.dart +++ b/lib/features/order/notifiers/order_notifier.dart @@ -92,9 +92,25 @@ class OrderNotifier extends AbstractMostroNotifier { OrderState currentState = state; for (final message in messages) { - if (message.action != Action.cantDo) { - currentState = currentState.updateWith(message); + if (message.action == Action.cantDo) continue; + + // A cancelled order outranks every waiting phase, so to the + // stale-transition guard the first message of the *next* take of the + // same order id looks like a late copy and the whole new cycle would + // be dropped — leaving the previous cycle's invoice on screen (#731). + // Replay it from a clean slate instead. + if (OrderState.endsTradeCycle(currentState.status) && + currentState.wouldRejectAsStale(message)) { + logger.i( + 'Order $orderId was retaken: replaying ${message.action} as a new cycle'); + currentState = OrderState( + status: Status.pending, + action: Action.newOrder, + order: currentState.order, + ); } + + currentState = currentState.updateWith(message); } // A replay that lands on the same values notifies nobody: @@ -140,6 +156,21 @@ class OrderNotifier extends AbstractMostroNotifier { } } + /// Drops every payload the previous take cycle left on this order id. + /// + /// Notifiers are keyed by order id, so retaking an order the user had + /// already taken and cancelled resumes the very same state object. Its + /// invoices were cancelled node-side when the previous cycle ended, and + /// rendering one again is how the stale bond screen happened (#731). The + /// order snapshot survives: it describes the listing, not the take. + void _resetForNewTakeCycle() { + state = OrderState( + status: Status.pending, + action: Action.newOrder, + order: state.order, + ); + } + Future takeSellOrder( String orderId, int? amount, String? lnAddress) async { // Serialize session creation + publish with the restore reset behind the @@ -156,6 +187,9 @@ class OrderNotifier extends AbstractMostroNotifier { // it can't delete the session we just created (retake within 60s). AbstractMostroNotifier.clearBondCancelDeletion(orderId); + // Same reason, for the state this notifier still holds from that cycle. + _resetForNewTakeCycle(); + // Start 10s timeout cleanup timer for orphan session prevention AbstractMostroNotifier.startSessionTimeoutCleanup(orderId, ref); @@ -182,6 +216,9 @@ class OrderNotifier extends AbstractMostroNotifier { // it can't delete the session we just created (retake within 60s). AbstractMostroNotifier.clearBondCancelDeletion(orderId); + // Same reason, for the state this notifier still holds from that cycle. + _resetForNewTakeCycle(); + // Start 10s timeout cleanup timer for orphan session prevention AbstractMostroNotifier.startSessionTimeoutCleanup(orderId, ref); diff --git a/lib/features/order/screens/pay_bond_invoice_screen.dart b/lib/features/order/screens/pay_bond_invoice_screen.dart index 29358f085..05c5de372 100644 --- a/lib/features/order/screens/pay_bond_invoice_screen.dart +++ b/lib/features/order/screens/pay_bond_invoice_screen.dart @@ -6,6 +6,7 @@ import 'package:qr_flutter/qr_flutter.dart'; import 'package:share_plus/share_plus.dart'; import 'package:url_launcher/url_launcher.dart'; import 'package:mostro_mobile/core/app_theme.dart'; +import 'package:mostro_mobile/data/enums.dart' as enums; import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; import 'package:mostro_mobile/features/order/widgets/order_app_bar.dart'; import 'package:mostro_mobile/generated/l10n.dart'; @@ -123,8 +124,16 @@ class PayBondInvoiceScreen extends ConsumerWidget { Widget build(BuildContext context, WidgetRef ref) { final s = S.of(context)!; final orderState = ref.watch(orderNotifierProvider(orderId)); - final lnInvoice = orderState.paymentRequest?.lnInvoice ?? ''; - final bondAmount = orderState.paymentRequest?.order?.amount; + // Only the invoice of the bond phase belongs on this screen. The order + // notifier is keyed by order id, so an escrow invoice from an earlier + // take of the same order could otherwise be rendered here as a bond — + // long after Mostro cancelled it node-side (#731). + final isBondPhase = orderState.action == enums.Action.payBondInvoice || + orderState.status == enums.Status.waitingTakerBond; + final lnInvoice = + isBondPhase ? orderState.paymentRequest?.lnInvoice ?? '' : ''; + final bondAmount = + isBondPhase ? orderState.paymentRequest?.order?.amount : null; // A maker creating an order pays the bond before it is published, so the // copy must warn them to keep the screen open or the order won't be created. final isMakerBond = ref @@ -134,6 +143,44 @@ class PayBondInvoiceScreen extends ConsumerWidget { false; final explanation = isMakerBond ? s.bondExplanationMaker : s.bondExplanation; + if (lnInvoice.isEmpty) { + return Scaffold( + backgroundColor: AppTheme.dark1, + appBar: OrderAppBar(title: s.bondScreenTitle), + body: Padding( + padding: const EdgeInsets.all(24), + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + const Icon( + Icons.hourglass_disabled, + color: AppTheme.textSecondary, + size: 48, + ), + const SizedBox(height: 16), + Text( + s.bondInvoiceUnavailable, + textAlign: TextAlign.center, + style: const TextStyle( + color: AppTheme.cream1, + fontSize: 15, + height: 1.4, + ), + ), + const SizedBox(height: 24), + ElevatedButton( + onPressed: () => context.go('/'), + style: ElevatedButton.styleFrom( + backgroundColor: AppTheme.mostroGreen, + ), + child: Text(s.close), + ), + ], + ), + ), + ); + } + return Scaffold( backgroundColor: AppTheme.dark1, appBar: OrderAppBar(title: s.bondScreenTitle), diff --git a/lib/l10n/intl_de.arb b/lib/l10n/intl_de.arb index f7625814e..6e90e8640 100644 --- a/lib/l10n/intl_de.arb +++ b/lib/l10n/intl_de.arb @@ -843,6 +843,7 @@ "copy": "Kopieren", "share": "Teilen", "failedToShareInvoice": "Teilen der Rechnung fehlgeschlagen. Bitte versuche stattdessen, sie zu kopieren.", + "bondInvoiceUnavailable": "Diese Kautions-Rechnung ist nicht mehr gültig. Gehe zurück und nimm die Order erneut an, um eine neue zu erhalten.", "bondScreenTitle": "Anti-Missbrauchs-Kaution", "bondExplanation": "Um diese Order anzunehmen, musst du eine Anti-Missbrauchs-Kaution zahlen. So funktioniert es:\n\n• Deine Sats werden in deiner Wallet gehalten, sie werden nicht ausgegeben.\n• Wenn der Tausch erfolgreich abgeschlossen wird, erhältst du deine Kaution automatisch zurück.\n• Wenn du bei dieser Order einen Streit hast und ihn verlierst, verlierst du die Kaution.\n• Dieser Mechanismus schützt alle Nutzer vor Betrügern.", "bondExplanationMaker": "Bevor deine Order erstellt werden kann, musst du eine Anti-Missbrauchs-Kaution zahlen. Lass diesen Bildschirm geöffnet, bis die Zahlung abgeschlossen ist – wenn du ihn schließt oder nicht zahlst, wird die Order nicht erstellt. So funktioniert es:\n\n• Deine Sats werden in deiner Wallet gehalten, sie werden nicht ausgegeben.\n• Wenn der Tausch erfolgreich abgeschlossen wird, erhältst du deine Kaution automatisch zurück.\n• Wenn du bei dieser Order einen Streit hast und ihn verlierst, verlierst du die Kaution.\n• Dieser Mechanismus schützt alle Nutzer vor Betrügern.", diff --git a/lib/l10n/intl_en.arb b/lib/l10n/intl_en.arb index 413f07e31..16f481afe 100644 --- a/lib/l10n/intl_en.arb +++ b/lib/l10n/intl_en.arb @@ -843,6 +843,7 @@ "copy": "Copy", "share": "Share", "failedToShareInvoice": "Failed to share invoice. Please try copying instead.", + "bondInvoiceUnavailable": "This bond invoice is no longer valid. Go back and take the order again to get a new one.", "bondScreenTitle": "Anti-abuse deposit", "bondExplanation": "To take this order you must pay an anti-abuse deposit. Here's how it works:\n\n• Your sats are held in your wallet, they are not spent.\n• If the exchange completes successfully, you get your deposit back automatically.\n• If you have a dispute on this order and lose it, you will lose the deposit.\n• This mechanism protects all users against scammers.", "bondExplanationMaker": "Before your order can be created, you must pay an anti-abuse deposit. Keep this screen open until the payment is done — if you close it or don't pay, the order won't be created. Here's how it works:\n\n• Your sats are held in your wallet, they are not spent.\n• If the exchange completes successfully, you get your deposit back automatically.\n• If you have a dispute on this order and lose it, you will lose the deposit.\n• This mechanism protects all users against scammers.", diff --git a/lib/l10n/intl_es.arb b/lib/l10n/intl_es.arb index 4002d018e..eadabb53b 100644 --- a/lib/l10n/intl_es.arb +++ b/lib/l10n/intl_es.arb @@ -693,6 +693,7 @@ "copy": "Copiar", "share": "Compartir", "failedToShareInvoice": "Error al compartir factura. Por favor intenta copiarla en su lugar.", + "bondInvoiceUnavailable": "Esta factura de depósito ya no es válida. Vuelve atrás y toma la orden de nuevo para obtener una nueva.", "bondScreenTitle": "Depósito anti-abuso", "bondExplanation": "Para tomar esta orden debes pagar un depósito anti-abuso. Funciona así:\n\n• Tus sats quedan retenidos en tu wallet, no se gastan.\n• Si el intercambio finaliza correctamente, recuperas el depósito automáticamente.\n• Si tienes una disputa en esta orden y la pierdes, perderás el depósito.\n• Este mecanismo protege a todos los usuarios contra estafadores.", "bondExplanationMaker": "Para crear tu orden primero debes pagar un depósito anti-abuso. Mantén esta pantalla abierta hasta completar el pago: si la cierras o no pagas, la orden no se creará. Funciona así:\n\n• Tus sats quedan retenidos en tu wallet, no se gastan.\n• Si el intercambio finaliza correctamente, recuperas el depósito automáticamente.\n• Si tienes una disputa en esta orden y la pierdes, perderás el depósito.\n• Este mecanismo protege a todos los usuarios contra estafadores.", diff --git a/lib/l10n/intl_fr.arb b/lib/l10n/intl_fr.arb index 4cb398b64..22a9c30a5 100644 --- a/lib/l10n/intl_fr.arb +++ b/lib/l10n/intl_fr.arb @@ -843,6 +843,7 @@ "copy": "Copier", "share": "Partager", "failedToShareInvoice": "Échec du partage de la facture. Veuillez essayer de copier à la place.", + "bondInvoiceUnavailable": "Cette facture de dépôt n'est plus valide. Reviens en arrière et reprends l'ordre pour en obtenir une nouvelle.", "bondScreenTitle": "Dépôt anti-abus", "bondExplanation": "Pour prendre cette commande, vous devez payer un dépôt anti-abus. Voici comment cela fonctionne :\n\n• Vos sats sont conservés dans votre portefeuille, ils ne sont pas dépensés.\n• Si l'échange se termine avec succès, vous récupérez votre dépôt automatiquement.\n• Si vous avez un litige sur cette commande et que vous le perdez, vous perdrez le dépôt.\n• Ce mécanisme protège tous les utilisateurs contre les escrocs.", "bondExplanationMaker": "Avant que votre commande puisse être créée, vous devez payer un dépôt anti-abus. Gardez cet écran ouvert jusqu'à ce que le paiement soit effectué — si vous le fermez ou ne payez pas, la commande ne sera pas créée. Voici comment cela fonctionne :\n\n• Vos sats sont conservés dans votre portefeuille, ils ne sont pas dépensés.\n• Si l'échange se termine avec succès, vous récupérez votre dépôt automatiquement.\n• Si vous avez un litige sur cette commande et que vous le perdez, vous perdrez le dépôt.\n• Ce mécanisme protège tous les utilisateurs contre les escrocs.", diff --git a/lib/l10n/intl_it.arb b/lib/l10n/intl_it.arb index 1a0c3c3a5..3c389bbc3 100644 --- a/lib/l10n/intl_it.arb +++ b/lib/l10n/intl_it.arb @@ -723,6 +723,7 @@ "copy": "Copia", "share": "Condividi", "failedToShareInvoice": "Errore nel condividere la fattura. Per favore prova a copiarla invece.", + "bondInvoiceUnavailable": "Questa fattura di deposito non è più valida. Torna indietro e prendi di nuovo l'ordine per ottenerne una nuova.", "bondScreenTitle": "Deposito anti-abuso", "bondExplanation": "Per prendere questo ordine devi pagare un deposito anti-abuso. Funziona così:\n\n• I tuoi sats vengono trattenuti nel tuo wallet, non vengono spesi.\n• Se lo scambio si conclude correttamente, recuperi il deposito automaticamente.\n• Se hai una disputa su questo ordine e la perdi, perderai il deposito.\n• Questo meccanismo protegge tutti gli utenti dai truffatori.", "bondExplanationMaker": "Prima che il tuo ordine possa essere creato, devi pagare un deposito anti-abuso. Tieni aperta questa schermata fino al completamento del pagamento: se la chiudi o non paghi, l'ordine non verrà creato. Funziona così:\n\n• I tuoi sats vengono trattenuti nel tuo wallet, non vengono spesi.\n• Se lo scambio si conclude correttamente, recuperi il deposito automaticamente.\n• Se hai una disputa su questo ordine e la perdi, perderai il deposito.\n• Questo meccanismo protegge tutti gli utenti dai truffatori.", diff --git a/lib/l10n/intl_pt.arb b/lib/l10n/intl_pt.arb index c5f12e052..2ba125aea 100644 --- a/lib/l10n/intl_pt.arb +++ b/lib/l10n/intl_pt.arb @@ -843,6 +843,7 @@ "copy": "Copiar", "share": "Compartilhar", "failedToShareInvoice": "Falha ao compartilhar a fatura. Por favor, tente copiar em vez disso.", + "bondInvoiceUnavailable": "Esta fatura de depósito já não é válida. Volta atrás e aceita a ordem novamente para obteres uma nova.", "bondScreenTitle": "Depósito antiabuso", "bondExplanation": "Para aceitar esta ordem, você deve pagar um depósito antiabuso. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", "bondExplanationMaker": "Antes que sua ordem possa ser criada, você deve pagar um depósito antiabuso. Mantenha esta tela aberta até que o pagamento seja concluído — se você fechá-la ou não pagar, a ordem não será criada. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", diff --git a/test/features/order/models/order_state_retake_test.dart b/test/features/order/models/order_state_retake_test.dart new file mode 100644 index 000000000..a0d01c220 --- /dev/null +++ b/test/features/order/models/order_state_retake_test.dart @@ -0,0 +1,185 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/enums.dart'; +import 'package:mostro_mobile/data/models.dart'; +import 'package:mostro_mobile/features/order/models/order_state.dart'; + +/// An invoice belongs to the take cycle that produced it. Mostro cancels the +/// bond and escrow hold invoices node-side when a cycle ends, so a bolt11 kept +/// across a cancel is unpayable (INCORRECT_PAYMENT_DETAILS) — and the bond +/// screen rendering the previous cycle's escrow invoice is what made an order +/// untakeable in the first place. +void main() { + Order orderPayload({Status status = Status.pending}) => Order( + id: 'order-1', + kind: OrderType.buy, + status: status, + amount: 16558, + fiatCode: 'EUR', + fiatAmount: 10, + paymentMethod: 'prueba', + ); + + MostroMessage invoiceMessage( + Action action, + String bolt11, { + int amount = 16558, + }) => + MostroMessage( + id: 'order-1', + action: action, + payload: PaymentRequest( + order: orderPayload(), + lnInvoice: bolt11, + ), + timestamp: amount, + ); + + MostroMessage orderMessage(Action action, Status status) => + MostroMessage( + id: 'order-1', + action: action, + payload: orderPayload(status: status), + ); + + OrderState initial() => OrderState( + action: Action.newOrder, + status: Status.pending, + order: null, + ); + + group('take cycle boundaries', () { + test('the bond invoice survives inside its own cycle', () { + final state = + initial().updateWith(invoiceMessage(Action.payBondInvoice, 'lnbcbond')); + + expect(state.status, Status.waitingTakerBond); + expect(state.paymentRequest?.lnInvoice, 'lnbcbond'); + }); + + test('cancelling the take drops the escrow invoice', () { + final state = initial() + .updateWith(invoiceMessage(Action.payBondInvoice, 'lnbcbond')) + .updateWith(invoiceMessage(Action.payInvoice, 'lnbcescrow')) + .updateWith(orderMessage(Action.canceled, Status.canceled)); + + expect(state.status, Status.canceled); + expect(state.paymentRequest, isNull); + }); + + test('a republish back to pending drops the escrow invoice', () { + final state = initial() + .updateWith(invoiceMessage(Action.payInvoice, 'lnbcescrow')) + .updateWith(orderMessage(Action.newOrder, Status.pending)); + + expect(state.status, Status.pending); + expect(state.paymentRequest, isNull); + }); + + test('the invoice of the next cycle is not swallowed by the cleanup', () { + // Republished back to the book, then taken again: the bond invoice of + // the new cycle arrives on a state the cleanup just emptied. + final state = initial() + .updateWith(invoiceMessage(Action.payInvoice, 'lnbcescrow')) + .updateWith(orderMessage(Action.newOrder, Status.pending)) + .updateWith(invoiceMessage(Action.payBondInvoice, 'lnbcbond2')); + + expect(state.status, Status.waitingTakerBond); + expect(state.paymentRequest?.lnInvoice, 'lnbcbond2'); + }); + }); + + group('endsTradeCycle', () { + test('covers every status in which Mostro voids the cycle invoices', () { + for (final status in [ + Status.pending, + Status.canceled, + Status.canceledByAdmin, + Status.cooperativelyCanceled, + Status.expired, + ]) { + expect(OrderState.endsTradeCycle(status), isTrue, reason: '$status'); + } + + for (final status in [ + Status.waitingTakerBond, + Status.waitingPayment, + Status.waitingBuyerInvoice, + Status.active, + Status.fiatSent, + Status.success, + ]) { + expect(OrderState.endsTradeCycle(status), isFalse, reason: '$status'); + } + }); + }); + + group('wouldRejectAsStale', () { + test('flags the first message of the take that follows a cancel', () { + final canceled = + initial().updateWith(orderMessage(Action.canceled, Status.canceled)); + + // A cancelled order outranks every waiting phase, so without the + // new-cycle reset in OrderNotifier.sync the retake would be dropped. + expect( + canceled.wouldRejectAsStale( + invoiceMessage(Action.payBondInvoice, 'lnbcbond2')), + isTrue, + ); + }); + + test('does not flag a forward move inside the cycle', () { + final bond = + initial().updateWith(invoiceMessage(Action.payBondInvoice, 'lnbcbond')); + + expect( + bond.wouldRejectAsStale(invoiceMessage(Action.payInvoice, 'lnbcescrow')), + isFalse, + ); + }); + }); + + group('replaying the persisted history of a retaken order', () { + // Mirrors the loop in OrderNotifier.sync: a message that only looks stale + // because the previous cycle ended restarts the replay. + OrderState replay(List messages) { + var current = initial(); + for (final message in messages) { + if (OrderState.endsTradeCycle(current.status) && + current.wouldRejectAsStale(message)) { + current = OrderState( + status: Status.pending, + action: Action.newOrder, + order: current.order, + ); + } + current = current.updateWith(message); + } + return current; + } + + test('ends on the new bond invoice, never the cancelled escrow one', () { + final state = replay([ + invoiceMessage(Action.payBondInvoice, 'lnbcbond'), + invoiceMessage(Action.payInvoice, 'lnbcescrow'), + orderMessage(Action.waitingBuyerInvoice, Status.waitingBuyerInvoice), + orderMessage(Action.canceled, Status.canceled), + invoiceMessage(Action.payBondInvoice, 'lnbcbond2'), + ]); + + expect(state.action, Action.payBondInvoice); + expect(state.status, Status.waitingTakerBond); + expect(state.paymentRequest?.lnInvoice, 'lnbcbond2'); + }); + + test('a history that ends on the cancel keeps no invoice at all', () { + final state = replay([ + invoiceMessage(Action.payBondInvoice, 'lnbcbond'), + invoiceMessage(Action.payInvoice, 'lnbcescrow'), + orderMessage(Action.canceled, Status.canceled), + ]); + + expect(state.status, Status.canceled); + expect(state.paymentRequest, isNull); + }); + }); +} From ec918fe00c9d18e0b96265e2c208f86ce5c3de02 Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sun, 13 Sep 2026 23:39:11 +0200 Subject: [PATCH 02/12] i18n: match the French and Portuguese register of the surrounding UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The French UI addresses the user as vous and the Portuguese file is pt-BR (você); the new bond-invoice message used the informal French forms and European Portuguese ones. --- lib/l10n/intl_fr.arb | 2 +- lib/l10n/intl_pt.arb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/l10n/intl_fr.arb b/lib/l10n/intl_fr.arb index 22a9c30a5..3d1fe3439 100644 --- a/lib/l10n/intl_fr.arb +++ b/lib/l10n/intl_fr.arb @@ -843,7 +843,7 @@ "copy": "Copier", "share": "Partager", "failedToShareInvoice": "Échec du partage de la facture. Veuillez essayer de copier à la place.", - "bondInvoiceUnavailable": "Cette facture de dépôt n'est plus valide. Reviens en arrière et reprends l'ordre pour en obtenir une nouvelle.", + "bondInvoiceUnavailable": "Cette facture de dépôt n'est plus valide. Revenez en arrière et reprenez l'ordre pour en obtenir une nouvelle.", "bondScreenTitle": "Dépôt anti-abus", "bondExplanation": "Pour prendre cette commande, vous devez payer un dépôt anti-abus. Voici comment cela fonctionne :\n\n• Vos sats sont conservés dans votre portefeuille, ils ne sont pas dépensés.\n• Si l'échange se termine avec succès, vous récupérez votre dépôt automatiquement.\n• Si vous avez un litige sur cette commande et que vous le perdez, vous perdrez le dépôt.\n• Ce mécanisme protège tous les utilisateurs contre les escrocs.", "bondExplanationMaker": "Avant que votre commande puisse être créée, vous devez payer un dépôt anti-abus. Gardez cet écran ouvert jusqu'à ce que le paiement soit effectué — si vous le fermez ou ne payez pas, la commande ne sera pas créée. Voici comment cela fonctionne :\n\n• Vos sats sont conservés dans votre portefeuille, ils ne sont pas dépensés.\n• Si l'échange se termine avec succès, vous récupérez votre dépôt automatiquement.\n• Si vous avez un litige sur cette commande et que vous le perdez, vous perdrez le dépôt.\n• Ce mécanisme protège tous les utilisateurs contre les escrocs.", diff --git a/lib/l10n/intl_pt.arb b/lib/l10n/intl_pt.arb index 2ba125aea..b46fe811f 100644 --- a/lib/l10n/intl_pt.arb +++ b/lib/l10n/intl_pt.arb @@ -843,7 +843,7 @@ "copy": "Copiar", "share": "Compartilhar", "failedToShareInvoice": "Falha ao compartilhar a fatura. Por favor, tente copiar em vez disso.", - "bondInvoiceUnavailable": "Esta fatura de depósito já não é válida. Volta atrás e aceita a ordem novamente para obteres uma nova.", + "bondInvoiceUnavailable": "Esta fatura de depósito já não é válida. Volte atrás e aceite a ordem novamente para obter uma nova.", "bondScreenTitle": "Depósito antiabuso", "bondExplanation": "Para aceitar esta ordem, você deve pagar um depósito antiabuso. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", "bondExplanationMaker": "Antes que sua ordem possa ser criada, você deve pagar um depósito antiabuso. Mantenha esta tela aberta até que o pagamento seja concluído — se você fechá-la ou não pagar, a ordem não será criada. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", From a03042d002efee6391c7d2138f0da15e00b8610b Mon Sep 17 00:00:00 2001 From: 21Mill Date: Tue, 15 Sep 2026 21:45:17 +0200 Subject: [PATCH 03/12] fix(order): restart a take cycle only on evidence of a new take MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous version restarted the cycle for any message the stale-transition guard rejected. Once an order is canceled, canceled by admin or expired, that is *every* later message the guard exists to drop: `created_at` has one-second resolution and relays replay newest-first, so a duplicate from the same second as the cancel sorts after it and reopened a terminal trade — the hole #723 closed. A restart now needs both halves of the evidence, in one place shared by the replay and the live stream (`AbstractMostroNotifier.applyToCycle`): - the action can only open a cycle (`OrderState.opensTakeCycle`), and - its event time is strictly later than the message that ended the previous cycle. Mostro cannot cancel a take and accept the next one within the same second, so a tie is a late copy. `cooperativelyCanceled` also stops counting as a cycle end: here it is the *pending* cooperative cancel (every `cooperative-cancel-initiated-*` maps to it) and the trade can still reach fiat-sent. Counting it dropped the invoice mid-trade and, with the restart, wiped `fiatWasSent`, `peer` and `dispute` — and without a tracked dispute every later `admin-settled` is refused by `rejectsAdminDisputeMessage`. With the rule on the live path too, `_resetForNewTakeCycle` is gone: the state no longer changes before the take is published, so a `cant-do` response or a failed publish can't leave it out of step with storage, and it can't race an in-flight `sync()`. `wouldRejectAsStale` now derives its transition through the same `_transitionFor` as `updateWith`, cooperative-cancel remap included, so the predicate cannot drift from what `updateWith` does. Tests move to where the logic lives: the replay scenarios run the real `sync()` in test/features/order/notifiers/order_notifier_retake_cycle_test.dart, including both late-copy regressions from the review. --- lib/features/order/models/order_state.dart | 94 +++-- .../notifiers/abstract_mostro_notifier.dart | 49 ++- .../order/notifiers/order_notifier.dart | 42 +-- .../order/models/order_state_retake_test.dart | 84 ++--- .../order_notifier_retake_cycle_test.dart | 330 ++++++++++++++++++ 5 files changed, 483 insertions(+), 116 deletions(-) create mode 100644 test/features/order/notifiers/order_notifier_retake_cycle_test.dart diff --git a/lib/features/order/models/order_state.dart b/lib/features/order/models/order_state.dart index 6bcd4577a..0cd5e4cb1 100644 --- a/lib/features/order/models/order_state.dart +++ b/lib/features/order/models/order_state.dart @@ -118,13 +118,35 @@ class OrderState { /// Everything Mostro issued for that cycle — bond and escrow hold invoices /// above all — is cancelled node-side when this happens, so no payload from /// it may survive into the next take of the same order id. + /// + /// [Status.cooperativelyCanceled] is deliberately absent: here it is the + /// *pending* cooperative cancel (every `cooperative-cancel-initiated-*` + /// action maps to it) and the trade can still reach fiat-sent. The cancel + /// only lands with `cooperative-cancel-accepted`, which maps to + /// [Status.canceled]. static bool endsTradeCycle(Status status) => status == Status.pending || status == Status.canceled || status == Status.canceledByAdmin || - status == Status.cooperativelyCanceled || status == Status.expired; + /// Actions that can only be the first message of a take cycle. + /// + /// A message that ends up rejected by the stale-transition guard is either a + /// late copy or the start of the *next* take of the same order id; the action + /// is half of what tells them apart (the other half is its event time, see + /// `AbstractMostroNotifier.applyToCycle`). `add-invoice` also appears in the + /// payout-retry flow, but that runs from `payment-failed`, never from a + /// status that ends a cycle, so it cannot be confused with a take here. + static bool opensTakeCycle(Action action) => + action == Action.takeBuy || + action == Action.takeSell || + action == Action.payBondInvoice || + action == Action.payInvoice || + action == Action.addInvoice || + action == Action.waitingSellerToPay || + action == Action.waitingBuyerInvoice; + /// Dispute statuses in which the dispute is over: a resolution has already /// been applied and no further admin action is expected for it. static const _terminalDisputeStatuses = { @@ -226,25 +248,16 @@ class OrderState { message.action == Action.fiatSent || message.action == Action.fiatSentOk; - // Remap generic cooperative cancel actions to semantic variants - // based on whether fiat was sent before the cancel was initiated - Action effectiveAction = message.action; - if (message.action == Action.cooperativeCancelInitiatedByYou) { - effectiveAction = newFiatWasSent - ? Action.cooperativeCancelFiatSentByYou - : Action.cooperativeCancelNoFiatByYou; - logger.d('Remapped ${message.action} → $effectiveAction (fiatWasSent: $newFiatWasSent)'); - } else if (message.action == Action.cooperativeCancelInitiatedByPeer) { - effectiveAction = newFiatWasSent - ? Action.cooperativeCancelFiatSentByPeer - : Action.cooperativeCancelNoFiatByPeer; - logger.d('Remapped ${message.action} → $effectiveAction (fiatWasSent: $newFiatWasSent)'); + // Remap generic cooperative cancel actions to semantic variants, then + // derive the status — the same derivation `wouldRejectAsStale` consults. + final transition = _transitionFor(message, fiatSent: newFiatWasSent); + final Action effectiveAction = transition.action; + final Status newStatus = transition.status; + if (effectiveAction != message.action) { + logger.d( + 'Remapped ${message.action} → $effectiveAction (fiatWasSent: $newFiatWasSent)'); } - // Determine the new status based on the action received - Status newStatus = _getStatusFromAction( - effectiveAction, message.getPayload()?.status); - // DEBUG: Log status mapping logger.d('Status mapping: $effectiveAction → $newStatus'); @@ -486,18 +499,49 @@ class OrderState { return !isRepublish; } + /// The action and status [updateWith] would apply for [message]. + /// + /// The cooperative-cancel remap depends on whether fiat was sent, so it is + /// computed from this state unless the caller already knows the updated + /// value. Kept in one place so [wouldRejectAsStale] can never disagree with + /// what [updateWith] actually does. + ({Action action, Status status}) _transitionFor( + MostroMessage message, { + bool? fiatSent, + }) { + final sent = fiatSent ?? + (fiatWasSent || + message.action == Action.fiatSent || + message.action == Action.fiatSentOk); + + var action = message.action; + if (action == Action.cooperativeCancelInitiatedByYou) { + action = sent + ? Action.cooperativeCancelFiatSentByYou + : Action.cooperativeCancelNoFiatByYou; + } else if (action == Action.cooperativeCancelInitiatedByPeer) { + action = sent + ? Action.cooperativeCancelFiatSentByPeer + : Action.cooperativeCancelNoFiatByPeer; + } + + return ( + action: action, + status: _getStatusFromAction(action, message.getPayload()?.status), + ); + } + /// Whether [message] would be dropped by the stale-transition guard. /// /// Lets callers replaying persisted history tell a genuinely late copy from /// the first message of a *new* take cycle: after a cancel, every later /// message looks backwards to the guard, because a cancelled order outranks - /// every waiting phase. Combined with [endsTradeCycle] on the current - /// status, that is the signal to start the replay over instead of dropping - /// the rest of the history (#731). - bool wouldRejectAsStale(MostroMessage message) => isStaleTransition( - message.action, - _getStatusFromAction(message.action, message.getPayload()?.status), - ); + /// every waiting phase. It is only half of the answer — see + /// `AbstractMostroNotifier.applyToCycle` for the rest (#731). + bool wouldRejectAsStale(MostroMessage message) { + final transition = _transitionFor(message); + return isStaleTransition(transition.action, transition.status); + } /// Maps actions to their corresponding statuses based on mostrod DM messages Status _getStatusFromAction(Action action, Status? payloadStatus) { diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index bb7e13011..ca6104464 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -45,6 +45,53 @@ class AbstractMostroNotifier extends StateNotifier { /// response is treated as a voluntary cancel (immediate session deletion) /// and notified as user-initiated rather than a counterparty timeout. @protected + /// Event time of the message that left this order in a cycle-ending status. + /// + /// `null` while the current cycle is still running. Maintained by + /// [applyToCycle], which is the only place that may restart a cycle. + int? cycleEndedAt; + + /// Applies [message] to [current], restarting the take cycle first when the + /// message opens a *new* one rather than being a late copy of the old. + /// + /// After a cancel every later message looks backwards to the stale guard — + /// a cancelled order outranks every waiting phase — so the guard alone can + /// no longer tell "the order was taken again" (#731) from "a duplicate + /// arrived late" (#723). Restarting therefore needs positive evidence: + /// + /// 1. the action can only open a cycle ([OrderState.opensTakeCycle]), and + /// 2. its event time is *strictly* later than the message that ended the + /// previous cycle. `created_at` has one-second resolution and relays + /// replay newest-first, so a copy from the same second as the cancel is + /// a late copy: Mostro cannot cancel a take and accept the next one + /// within the same second. + OrderState applyToCycle(OrderState current, MostroMessage message) { + final eventTime = message.eventCreatedAt ?? message.timestamp ?? 0; + final endedAt = cycleEndedAt; + + if (endedAt != null && + eventTime > endedAt && + OrderState.endsTradeCycle(current.status) && + OrderState.opensTakeCycle(message.action) && + current.wouldRejectAsStale(message)) { + logger.i( + 'Order $orderId was taken again: replaying ${message.action} as a new cycle'); + current = OrderState( + status: Status.pending, + action: Action.newOrder, + order: current.order, + ); + } + + final next = current.updateWith(message); + + // Recorded from the resulting status rather than from the transition: a + // replay starts from the current state, so the message that ended the + // cycle can be applied onto an already-ended one. + cycleEndedAt = OrderState.endsTradeCycle(next.status) ? eventTime : null; + return next; + } + static void markUserInitiatedCancel(String orderId) { _userInitiatedCancels.add(orderId); } @@ -134,7 +181,7 @@ class AbstractMostroNotifier extends StateNotifier { _userInitiatedCancels.remove(orderId); if (mounted) { - state = state.updateWith(msg); + state = applyToCycle(state, msg); } if (msg.timestamp != null && diff --git a/lib/features/order/notifiers/order_notifier.dart b/lib/features/order/notifiers/order_notifier.dart index 774ff9427..e0972ea13 100644 --- a/lib/features/order/notifiers/order_notifier.dart +++ b/lib/features/order/notifiers/order_notifier.dart @@ -91,26 +91,11 @@ class OrderNotifier extends AbstractMostroNotifier { OrderState currentState = state; + // The replay rebuilds the cycle bookkeeping from the history itself. + cycleEndedAt = null; for (final message in messages) { if (message.action == Action.cantDo) continue; - - // A cancelled order outranks every waiting phase, so to the - // stale-transition guard the first message of the *next* take of the - // same order id looks like a late copy and the whole new cycle would - // be dropped — leaving the previous cycle's invoice on screen (#731). - // Replay it from a clean slate instead. - if (OrderState.endsTradeCycle(currentState.status) && - currentState.wouldRejectAsStale(message)) { - logger.i( - 'Order $orderId was retaken: replaying ${message.action} as a new cycle'); - currentState = OrderState( - status: Status.pending, - action: Action.newOrder, - order: currentState.order, - ); - } - - currentState = currentState.updateWith(message); + currentState = applyToCycle(currentState, message); } // A replay that lands on the same values notifies nobody: @@ -156,21 +141,6 @@ class OrderNotifier extends AbstractMostroNotifier { } } - /// Drops every payload the previous take cycle left on this order id. - /// - /// Notifiers are keyed by order id, so retaking an order the user had - /// already taken and cancelled resumes the very same state object. Its - /// invoices were cancelled node-side when the previous cycle ended, and - /// rendering one again is how the stale bond screen happened (#731). The - /// order snapshot survives: it describes the listing, not the take. - void _resetForNewTakeCycle() { - state = OrderState( - status: Status.pending, - action: Action.newOrder, - order: state.order, - ); - } - Future takeSellOrder( String orderId, int? amount, String? lnAddress) async { // Serialize session creation + publish with the restore reset behind the @@ -187,9 +157,6 @@ class OrderNotifier extends AbstractMostroNotifier { // it can't delete the session we just created (retake within 60s). AbstractMostroNotifier.clearBondCancelDeletion(orderId); - // Same reason, for the state this notifier still holds from that cycle. - _resetForNewTakeCycle(); - // Start 10s timeout cleanup timer for orphan session prevention AbstractMostroNotifier.startSessionTimeoutCleanup(orderId, ref); @@ -216,9 +183,6 @@ class OrderNotifier extends AbstractMostroNotifier { // it can't delete the session we just created (retake within 60s). AbstractMostroNotifier.clearBondCancelDeletion(orderId); - // Same reason, for the state this notifier still holds from that cycle. - _resetForNewTakeCycle(); - // Start 10s timeout cleanup timer for orphan session prevention AbstractMostroNotifier.startSessionTimeoutCleanup(orderId, ref); diff --git a/test/features/order/models/order_state_retake_test.dart b/test/features/order/models/order_state_retake_test.dart index a0d01c220..8814bec02 100644 --- a/test/features/order/models/order_state_retake_test.dart +++ b/test/features/order/models/order_state_retake_test.dart @@ -19,11 +19,7 @@ void main() { paymentMethod: 'prueba', ); - MostroMessage invoiceMessage( - Action action, - String bolt11, { - int amount = 16558, - }) => + MostroMessage invoiceMessage(Action action, String bolt11) => MostroMessage( id: 'order-1', action: action, @@ -31,7 +27,6 @@ void main() { order: orderPayload(), lnInvoice: bolt11, ), - timestamp: amount, ); MostroMessage orderMessage(Action action, Status status) => @@ -94,13 +89,15 @@ void main() { Status.pending, Status.canceled, Status.canceledByAdmin, - Status.cooperativelyCanceled, Status.expired, ]) { expect(OrderState.endsTradeCycle(status), isTrue, reason: '$status'); } for (final status in [ + // The *pending* cooperative cancel: the trade can still reach + // fiat-sent, so nothing of the cycle may be dropped yet. + Status.cooperativelyCanceled, Status.waitingTakerBond, Status.waitingPayment, Status.waitingBuyerInvoice, @@ -113,6 +110,35 @@ void main() { }); }); + group('opensTakeCycle', () { + test('accepts only the actions that can start a take', () { + for (final action in [ + Action.takeBuy, + Action.takeSell, + Action.payBondInvoice, + Action.payInvoice, + Action.addInvoice, + Action.waitingSellerToPay, + Action.waitingBuyerInvoice, + ]) { + expect(OrderState.opensTakeCycle(action), isTrue, reason: '$action'); + } + + // Mid-trade and terminal actions: a copy of one of these arriving after + // a cancel is a late duplicate, never a new take. + for (final action in [ + Action.holdInvoicePaymentAccepted, + Action.buyerTookOrder, + Action.fiatSentOk, + Action.released, + Action.canceled, + Action.newOrder, + ]) { + expect(OrderState.opensTakeCycle(action), isFalse, reason: '$action'); + } + }); + }); + group('wouldRejectAsStale', () { test('flags the first message of the take that follows a cancel', () { final canceled = @@ -138,48 +164,4 @@ void main() { }); }); - group('replaying the persisted history of a retaken order', () { - // Mirrors the loop in OrderNotifier.sync: a message that only looks stale - // because the previous cycle ended restarts the replay. - OrderState replay(List messages) { - var current = initial(); - for (final message in messages) { - if (OrderState.endsTradeCycle(current.status) && - current.wouldRejectAsStale(message)) { - current = OrderState( - status: Status.pending, - action: Action.newOrder, - order: current.order, - ); - } - current = current.updateWith(message); - } - return current; - } - - test('ends on the new bond invoice, never the cancelled escrow one', () { - final state = replay([ - invoiceMessage(Action.payBondInvoice, 'lnbcbond'), - invoiceMessage(Action.payInvoice, 'lnbcescrow'), - orderMessage(Action.waitingBuyerInvoice, Status.waitingBuyerInvoice), - orderMessage(Action.canceled, Status.canceled), - invoiceMessage(Action.payBondInvoice, 'lnbcbond2'), - ]); - - expect(state.action, Action.payBondInvoice); - expect(state.status, Status.waitingTakerBond); - expect(state.paymentRequest?.lnInvoice, 'lnbcbond2'); - }); - - test('a history that ends on the cancel keeps no invoice at all', () { - final state = replay([ - invoiceMessage(Action.payBondInvoice, 'lnbcbond'), - invoiceMessage(Action.payInvoice, 'lnbcescrow'), - orderMessage(Action.canceled, Status.canceled), - ]); - - expect(state.status, Status.canceled); - expect(state.paymentRequest, isNull); - }); - }); } diff --git a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart new file mode 100644 index 000000000..60c56b7eb --- /dev/null +++ b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart @@ -0,0 +1,330 @@ +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/enums/order_type.dart'; +import 'package:mostro_mobile/data/models/enums/status.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/order.dart'; +import 'package:mostro_mobile/data/models/payment_request.dart'; +import 'package:mostro_mobile/data/repositories/session_storage.dart'; +import 'package:mostro_mobile/features/order/notifiers/order_notifier.dart'; +import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; +import 'package:mostro_mobile/features/settings/settings.dart'; +import 'package:mostro_mobile/services/mostro_service.dart'; +import 'package:mostro_mobile/services/nostr_service.dart'; +import 'package:mostro_mobile/shared/notifiers/session_notifier.dart'; +import 'package:mostro_mobile/shared/providers/mostro_database_provider.dart'; +import 'package:mostro_mobile/shared/providers/mostro_service_provider.dart'; +import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; +import 'package:mostro_mobile/shared/providers/nostr_service_provider.dart'; +import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; +import 'package:mostro_mobile/shared/providers/storage_providers.dart'; +import 'package:sembast/sembast_memory.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:shared_preferences_platform_interface/in_memory_shared_preferences_async.dart'; +import 'package:shared_preferences_platform_interface/shared_preferences_async_platform_interface.dart'; + +/// Sessions play no part in these scenarios; this keeps the harness free of +/// the generated mocks. +class _NoopSessionStorage implements SessionStorage { + @override + dynamic noSuchMethod(Invocation invocation) => Future.value(); +} + +/// Retaking an order reuses its notifier, so the replay has to tell the first +/// message of the *new* take from a late copy of the old one. Getting that +/// wrong either leaves the previous cycle's invoice on the bond screen (#731) +/// or lets a duplicate reopen a terminal trade (#723). Both directions are +/// exercised here against the real `sync()`. +class _SilentNostrService extends NostrService { + @override + bool get isInitialized => true; + + @override + Stream subscribeToEvents( + NostrRequest request, { + void Function(String)? onEose, + }) => + const Stream.empty(); +} + +class _IdleMostroService extends MostroService { + _IdleMostroService(super.ref); +} + +class _FixedSessionNotifier extends SessionNotifier { + _FixedSessionNotifier(Ref ref) + : super( + ref, + _NoopSessionStorage(), + Settings( + relays: [], + fullPrivacyMode: false, + mostroPublicKey: 'test', + ), + ) { + state = []; + } +} + +/// Real `sync()`, no live stream. +class _SyncOnlyOrderNotifier extends OrderNotifier { + _SyncOnlyOrderNotifier(super.orderId, super.ref); + + @override + void subscribe() {} +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + const orderId = 'test-order-id'; + + late Database db; + late ProviderContainer container; + + setUp(() async { + SharedPreferencesAsyncPlatform.instance = + InMemorySharedPreferencesAsync.empty(); + db = await newDatabaseFactoryMemory().openDatabase('retake_cycle.db'); + + container = ProviderContainer( + overrides: [ + sharedPreferencesProvider.overrideWithValue(SharedPreferencesAsync()), + mostroDatabaseProvider.overrideWithValue(db), + nostrServiceProvider.overrideWithValue(_SilentNostrService()), + mostroServiceProvider.overrideWith((ref) => _IdleMostroService(ref)), + sessionNotifierProvider + .overrideWith((ref) => _FixedSessionNotifier(ref)), + orderNotifierProvider.overrideWith( + (ref, id) => _SyncOnlyOrderNotifier(id, ref), + ), + ], + ); + }); + + tearDown(() async { + container.dispose(); + await db.close(); + }); + + Order orderPayload(Status status) => Order( + id: orderId, + kind: OrderType.buy, + status: status, + amount: 16558, + fiatCode: 'EUR', + fiatAmount: 10, + paymentMethod: 'face to face', + ); + + MostroMessage message( + Action action, + Status status, { + required int eventCreatedAt, + required int timestamp, + }) => + MostroMessage( + action: action, + id: orderId, + eventCreatedAt: eventCreatedAt, + timestamp: timestamp, + payload: orderPayload(status), + ); + + MostroMessage invoice( + Action action, + String bolt11, { + required int eventCreatedAt, + required int timestamp, + }) => + MostroMessage( + action: action, + id: orderId, + eventCreatedAt: eventCreatedAt, + timestamp: timestamp, + payload: PaymentRequest( + order: orderPayload(Status.pending), + lnInvoice: bolt11, + ), + ); + + Future persist(List<(String, MostroMessage)> history) async { + final storage = container.read(mostroStorageProvider); + for (final (key, msg) in history) { + await storage.addMessage(key, msg); + } + } + + Future syncedState() async { + await container.read(orderNotifierProvider(orderId).notifier).sync(); + final state = container.read(orderNotifierProvider(orderId)); + return OrderStateSnapshot( + status: state.status, + action: state.action, + invoice: state.paymentRequest?.lnInvoice, + fiatWasSent: state.fiatWasSent, + ); + } + + group('a genuine retake', () { + test('ends on the new bond invoice, never the cancelled escrow one', + () async { + await persist([ + ('a', invoice(Action.payBondInvoice, 'lnbcbond', + eventCreatedAt: 1000, timestamp: 1)), + ('b', invoice(Action.payInvoice, 'lnbcescrow', + eventCreatedAt: 2000, timestamp: 2)), + ( + 'c', + message(Action.waitingBuyerInvoice, Status.waitingBuyerInvoice, + eventCreatedAt: 2500, timestamp: 3) + ), + ( + 'd', + message(Action.canceled, Status.canceled, + eventCreatedAt: 3000, timestamp: 4) + ), + ('e', invoice(Action.payBondInvoice, 'lnbcbond2', + eventCreatedAt: 4000, timestamp: 5)), + ]); + + final state = await syncedState(); + + expect(state.status, Status.waitingTakerBond); + expect(state.action, Action.payBondInvoice); + expect(state.invoice, 'lnbcbond2'); + }); + + test('a history that ends on the cancel keeps no invoice at all', () async { + await persist([ + ('a', invoice(Action.payBondInvoice, 'lnbcbond', + eventCreatedAt: 1000, timestamp: 1)), + ('b', invoice(Action.payInvoice, 'lnbcescrow', + eventCreatedAt: 2000, timestamp: 2)), + ( + 'c', + message(Action.canceled, Status.canceled, + eventCreatedAt: 3000, timestamp: 3) + ), + ]); + + final state = await syncedState(); + + expect(state.status, Status.canceled); + expect(state.invoice, isNull); + }); + }); + + group('late copies must not reopen a terminal order', () { + // `created_at` has one-second resolution and relays replay newest-first, + // so a duplicate from the same second as the cancel sorts after it. + test('same-second copy of a setup message after canceled', () async { + await persist([ + ( + 'a', + message(Action.waitingSellerToPay, Status.waitingPayment, + eventCreatedAt: 1000, timestamp: 1) + ), + ( + 'b', + message(Action.holdInvoicePaymentAccepted, Status.active, + eventCreatedAt: 2000, timestamp: 2) + ), + ( + 'c', + message(Action.canceled, Status.canceled, + eventCreatedAt: 3000, timestamp: 3) + ), + ( + 'd', + message(Action.waitingSellerToPay, Status.waitingPayment, + eventCreatedAt: 3000, timestamp: 4) + ), + ]); + + expect((await syncedState()).status, Status.canceled); + }); + + test('same-second copy of a mid-trade message after canceled', () async { + await persist([ + ( + 'a', + message(Action.waitingSellerToPay, Status.waitingPayment, + eventCreatedAt: 1000, timestamp: 1) + ), + ( + 'b', + message(Action.holdInvoicePaymentAccepted, Status.active, + eventCreatedAt: 2000, timestamp: 2) + ), + ( + 'c', + message(Action.canceled, Status.canceled, + eventCreatedAt: 3000, timestamp: 3) + ), + ( + 'd', + message(Action.holdInvoicePaymentAccepted, Status.active, + eventCreatedAt: 3000, timestamp: 4) + ), + ]); + + expect((await syncedState()).status, Status.canceled); + }); + + // `canceled-by-admin` and `expired` take the same path: they are just + // more statuses `endsTradeCycle` covers. They are not driven from here + // because an `admin-*` message is itself dropped without a tracked + // dispute (`rejectsAdminDisputeMessage`), which would prove nothing + // about the cycle rule. + }); + + group('a pending cooperative cancel is not a cycle end', () { + test('a late setup copy keeps fiatWasSent', () async { + await persist([ + ( + 'a', + message(Action.buyerTookOrder, Status.active, + eventCreatedAt: 1000, timestamp: 1) + ), + ( + 'b', + message(Action.fiatSentOk, Status.fiatSent, + eventCreatedAt: 2000, timestamp: 2) + ), + ( + 'c', + message(Action.cooperativeCancelInitiatedByPeer, Status.active, + eventCreatedAt: 3000, timestamp: 3) + ), + ( + 'd', + message(Action.buyerTookOrder, Status.active, + eventCreatedAt: 3000, timestamp: 4) + ), + ]); + + final state = await syncedState(); + + expect(state.status, Status.cooperativelyCanceled); + expect(state.fiatWasSent, isTrue); + }); + }); +} + +/// The few fields these scenarios assert on. +class OrderStateSnapshot { + final Status status; + final Action action; + final String? invoice; + final bool fiatWasSent; + + OrderStateSnapshot({ + required this.status, + required this.action, + required this.invoice, + required this.fiatWasSent, + }); +} From 87dea305a05144af0d17ae49cf453ebc23495e0b Mon Sep 17 00:00:00 2001 From: 21Mill Date: Tue, 15 Sep 2026 21:45:25 +0200 Subject: [PATCH 04/12] fix(order): tell the user why the bond screen has no invoice `lnInvoice.isEmpty` covered three different situations and described all of them as "this invoice is no longer valid, take the order again": - the invoice has not loaded yet. The maker create flow navigates here from AddOrderNotifier, so the order notifier is still at its initial state until its own sync() reads the message. The copy told a maker mid-bond to leave, and `bondExplanationMaker` warns that leaving means the order is never created; - the bond is already paid and the trade moved on, which the user sees when they come back from the wallet late or reopen the screen; - the take cycle really ended, the only case the copy described. Each branch now has its own state: a spinner with no destructive action while it loads, a pointer to the trade once the bond is paid, and the expired message only when the cycle ended. That message says "go back", so its button pops when there is a stack to pop instead of always going to the order book. Adds the first tests for this screen, one per branch. The Portuguese "bond invoice unavailable" string also moves to pt-BR phrasing. --- .../screens/pay_bond_invoice_screen.dart | 133 +++++++++--- lib/l10n/intl_de.arb | 3 + lib/l10n/intl_en.arb | 3 + lib/l10n/intl_es.arb | 3 + lib/l10n/intl_fr.arb | 3 + lib/l10n/intl_it.arb | 3 + lib/l10n/intl_pt.arb | 5 +- .../screens/pay_bond_invoice_screen_test.dart | 199 ++++++++++++++++++ 8 files changed, 317 insertions(+), 35 deletions(-) create mode 100644 test/features/order/screens/pay_bond_invoice_screen_test.dart diff --git a/lib/features/order/screens/pay_bond_invoice_screen.dart b/lib/features/order/screens/pay_bond_invoice_screen.dart index 05c5de372..a6dc52603 100644 --- a/lib/features/order/screens/pay_bond_invoice_screen.dart +++ b/lib/features/order/screens/pay_bond_invoice_screen.dart @@ -7,6 +7,7 @@ import 'package:share_plus/share_plus.dart'; import 'package:url_launcher/url_launcher.dart'; import 'package:mostro_mobile/core/app_theme.dart'; import 'package:mostro_mobile/data/enums.dart' as enums; +import 'package:mostro_mobile/features/order/models/order_state.dart'; import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; import 'package:mostro_mobile/features/order/widgets/order_app_bar.dart'; import 'package:mostro_mobile/generated/l10n.dart'; @@ -144,40 +145,47 @@ class PayBondInvoiceScreen extends ConsumerWidget { final explanation = isMakerBond ? s.bondExplanationMaker : s.bondExplanation; if (lnInvoice.isEmpty) { - return Scaffold( - backgroundColor: AppTheme.dark1, - appBar: OrderAppBar(title: s.bondScreenTitle), - body: Padding( - padding: const EdgeInsets.all(24), - child: Column( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - const Icon( - Icons.hourglass_disabled, - color: AppTheme.textSecondary, - size: 48, - ), - const SizedBox(height: 16), - Text( - s.bondInvoiceUnavailable, - textAlign: TextAlign.center, - style: const TextStyle( - color: AppTheme.cream1, - fontSize: 15, - height: 1.4, - ), - ), - const SizedBox(height: 24), - ElevatedButton( - onPressed: () => context.go('/'), - style: ElevatedButton.styleFrom( - backgroundColor: AppTheme.mostroGreen, - ), - child: Text(s.close), - ), - ], - ), - ), + // No invoice to show — and the reason decides what to tell the user. + // Saying "expired, take the order again" in all three cases is what + // could make a maker abandon a bond that was merely still loading. + final cycleEnded = + OrderState.endsTradeCycle(orderState.status) && + orderState.status != enums.Status.pending; + // `pending` here is the notifier's initial state as well: the maker bond + // arrives on AddOrderNotifier and only reaches this provider once its + // sync() has read the message, so the first frames legitimately have + // nothing to render. + final stillLoading = !cycleEnded && + (orderState.status == enums.Status.pending || + orderState.status == enums.Status.waitingTakerBond); + + if (stillLoading) { + return _EmptyBondState( + title: s.bondScreenTitle, + icon: null, + message: s.bondInvoicePending, + ); + } + + if (cycleEnded) { + return _EmptyBondState( + title: s.bondScreenTitle, + icon: Icons.hourglass_disabled, + message: s.bondInvoiceUnavailable, + // The copy says "go back", so go back when there is a stack to pop. + actionLabel: s.close, + onAction: (context) => + context.canPop() ? context.pop() : context.go('/'), + ); + } + + // Past the bond phase: the bond is paid and the trade moved on. + return _EmptyBondState( + title: s.bondScreenTitle, + icon: Icons.check_circle_outline, + message: s.bondAlreadyPaid, + actionLabel: s.goToTrade, + onAction: (context) => context.go('/trade_detail/$orderId'), ); } @@ -289,3 +297,60 @@ class PayBondInvoiceScreen extends ConsumerWidget { ); } } + +/// The bond screen with no invoice to show: one message, at most one action. +class _EmptyBondState extends StatelessWidget { + final String title; + final IconData? icon; + final String message; + final String? actionLabel; + final void Function(BuildContext context)? onAction; + + const _EmptyBondState({ + required this.title, + required this.icon, + required this.message, + this.actionLabel, + this.onAction, + }); + + @override + Widget build(BuildContext context) { + return Scaffold( + backgroundColor: AppTheme.dark1, + appBar: OrderAppBar(title: title), + body: Padding( + padding: const EdgeInsets.all(24), + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + if (icon != null) + Icon(icon, color: AppTheme.textSecondary, size: 48) + else + const CircularProgressIndicator(color: AppTheme.mostroGreen), + const SizedBox(height: 16), + Text( + message, + textAlign: TextAlign.center, + style: const TextStyle( + color: AppTheme.cream1, + fontSize: 15, + height: 1.4, + ), + ), + if (actionLabel != null && onAction != null) ...[ + const SizedBox(height: 24), + ElevatedButton( + onPressed: () => onAction!(context), + style: ElevatedButton.styleFrom( + backgroundColor: AppTheme.mostroGreen, + ), + child: Text(actionLabel!), + ), + ], + ], + ), + ), + ); + } +} diff --git a/lib/l10n/intl_de.arb b/lib/l10n/intl_de.arb index 6e90e8640..435ba909a 100644 --- a/lib/l10n/intl_de.arb +++ b/lib/l10n/intl_de.arb @@ -843,6 +843,9 @@ "copy": "Kopieren", "share": "Teilen", "failedToShareInvoice": "Teilen der Rechnung fehlgeschlagen. Bitte versuche stattdessen, sie zu kopieren.", + "bondInvoicePending": "Warte auf die Kautions-Rechnung …", + "bondAlreadyPaid": "Diese Kaution ist bereits bezahlt. Dein Handel läuft schon.", + "goToTrade": "ZUM HANDEL", "bondInvoiceUnavailable": "Diese Kautions-Rechnung ist nicht mehr gültig. Gehe zurück und nimm die Order erneut an, um eine neue zu erhalten.", "bondScreenTitle": "Anti-Missbrauchs-Kaution", "bondExplanation": "Um diese Order anzunehmen, musst du eine Anti-Missbrauchs-Kaution zahlen. So funktioniert es:\n\n• Deine Sats werden in deiner Wallet gehalten, sie werden nicht ausgegeben.\n• Wenn der Tausch erfolgreich abgeschlossen wird, erhältst du deine Kaution automatisch zurück.\n• Wenn du bei dieser Order einen Streit hast und ihn verlierst, verlierst du die Kaution.\n• Dieser Mechanismus schützt alle Nutzer vor Betrügern.", diff --git a/lib/l10n/intl_en.arb b/lib/l10n/intl_en.arb index 16f481afe..9c13afbd2 100644 --- a/lib/l10n/intl_en.arb +++ b/lib/l10n/intl_en.arb @@ -843,6 +843,9 @@ "copy": "Copy", "share": "Share", "failedToShareInvoice": "Failed to share invoice. Please try copying instead.", + "bondInvoicePending": "Waiting for the deposit invoice…", + "bondAlreadyPaid": "This deposit has already been paid. Your trade is already in progress.", + "goToTrade": "GO TO TRADE", "bondInvoiceUnavailable": "This bond invoice is no longer valid. Go back and take the order again to get a new one.", "bondScreenTitle": "Anti-abuse deposit", "bondExplanation": "To take this order you must pay an anti-abuse deposit. Here's how it works:\n\n• Your sats are held in your wallet, they are not spent.\n• If the exchange completes successfully, you get your deposit back automatically.\n• If you have a dispute on this order and lose it, you will lose the deposit.\n• This mechanism protects all users against scammers.", diff --git a/lib/l10n/intl_es.arb b/lib/l10n/intl_es.arb index eadabb53b..ad1f3da66 100644 --- a/lib/l10n/intl_es.arb +++ b/lib/l10n/intl_es.arb @@ -693,6 +693,9 @@ "copy": "Copiar", "share": "Compartir", "failedToShareInvoice": "Error al compartir factura. Por favor intenta copiarla en su lugar.", + "bondInvoicePending": "Esperando la factura del depósito…", + "bondAlreadyPaid": "Este depósito ya está pagado. Tu intercambio ya está en marcha.", + "goToTrade": "IR AL INTERCAMBIO", "bondInvoiceUnavailable": "Esta factura de depósito ya no es válida. Vuelve atrás y toma la orden de nuevo para obtener una nueva.", "bondScreenTitle": "Depósito anti-abuso", "bondExplanation": "Para tomar esta orden debes pagar un depósito anti-abuso. Funciona así:\n\n• Tus sats quedan retenidos en tu wallet, no se gastan.\n• Si el intercambio finaliza correctamente, recuperas el depósito automáticamente.\n• Si tienes una disputa en esta orden y la pierdes, perderás el depósito.\n• Este mecanismo protege a todos los usuarios contra estafadores.", diff --git a/lib/l10n/intl_fr.arb b/lib/l10n/intl_fr.arb index 3d1fe3439..258e5cedb 100644 --- a/lib/l10n/intl_fr.arb +++ b/lib/l10n/intl_fr.arb @@ -843,6 +843,9 @@ "copy": "Copier", "share": "Partager", "failedToShareInvoice": "Échec du partage de la facture. Veuillez essayer de copier à la place.", + "bondInvoicePending": "En attente de la facture de dépôt…", + "bondAlreadyPaid": "Ce dépôt est déjà payé. Votre échange est déjà en cours.", + "goToTrade": "ALLER À L'ÉCHANGE", "bondInvoiceUnavailable": "Cette facture de dépôt n'est plus valide. Revenez en arrière et reprenez l'ordre pour en obtenir une nouvelle.", "bondScreenTitle": "Dépôt anti-abus", "bondExplanation": "Pour prendre cette commande, vous devez payer un dépôt anti-abus. Voici comment cela fonctionne :\n\n• Vos sats sont conservés dans votre portefeuille, ils ne sont pas dépensés.\n• Si l'échange se termine avec succès, vous récupérez votre dépôt automatiquement.\n• Si vous avez un litige sur cette commande et que vous le perdez, vous perdrez le dépôt.\n• Ce mécanisme protège tous les utilisateurs contre les escrocs.", diff --git a/lib/l10n/intl_it.arb b/lib/l10n/intl_it.arb index 3c389bbc3..92ecb1e23 100644 --- a/lib/l10n/intl_it.arb +++ b/lib/l10n/intl_it.arb @@ -723,6 +723,9 @@ "copy": "Copia", "share": "Condividi", "failedToShareInvoice": "Errore nel condividere la fattura. Per favore prova a copiarla invece.", + "bondInvoicePending": "In attesa della fattura del deposito…", + "bondAlreadyPaid": "Questo deposito è già stato pagato. Il tuo scambio è già in corso.", + "goToTrade": "VAI ALLO SCAMBIO", "bondInvoiceUnavailable": "Questa fattura di deposito non è più valida. Torna indietro e prendi di nuovo l'ordine per ottenerne una nuova.", "bondScreenTitle": "Deposito anti-abuso", "bondExplanation": "Per prendere questo ordine devi pagare un deposito anti-abuso. Funziona così:\n\n• I tuoi sats vengono trattenuti nel tuo wallet, non vengono spesi.\n• Se lo scambio si conclude correttamente, recuperi il deposito automaticamente.\n• Se hai una disputa su questo ordine e la perdi, perderai il deposito.\n• Questo meccanismo protegge tutti gli utenti dai truffatori.", diff --git a/lib/l10n/intl_pt.arb b/lib/l10n/intl_pt.arb index b46fe811f..b6517af06 100644 --- a/lib/l10n/intl_pt.arb +++ b/lib/l10n/intl_pt.arb @@ -843,7 +843,10 @@ "copy": "Copiar", "share": "Compartilhar", "failedToShareInvoice": "Falha ao compartilhar a fatura. Por favor, tente copiar em vez disso.", - "bondInvoiceUnavailable": "Esta fatura de depósito já não é válida. Volte atrás e aceite a ordem novamente para obter uma nova.", + "bondInvoicePending": "Aguardando a fatura do depósito…", + "bondAlreadyPaid": "Este depósito já foi pago. Sua troca já está em andamento.", + "goToTrade": "IR PARA A TROCA", + "bondInvoiceUnavailable": "Esta fatura de depósito não é mais válida. Volte atrás e aceite a ordem novamente para obter uma nova.", "bondScreenTitle": "Depósito antiabuso", "bondExplanation": "Para aceitar esta ordem, você deve pagar um depósito antiabuso. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", "bondExplanationMaker": "Antes que sua ordem possa ser criada, você deve pagar um depósito antiabuso. Mantenha esta tela aberta até que o pagamento seja concluído — se você fechá-la ou não pagar, a ordem não será criada. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", diff --git a/test/features/order/screens/pay_bond_invoice_screen_test.dart b/test/features/order/screens/pay_bond_invoice_screen_test.dart new file mode 100644 index 000000000..cdf99da97 --- /dev/null +++ b/test/features/order/screens/pay_bond_invoice_screen_test.dart @@ -0,0 +1,199 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; +import 'package:mostro_mobile/data/enums.dart' as enums; +import 'package:mostro_mobile/data/models.dart'; +import 'package:mostro_mobile/features/order/models/order_state.dart'; +import 'package:mostro_mobile/features/order/notifiers/order_notifier.dart'; +import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; +import 'package:mostro_mobile/features/order/screens/pay_bond_invoice_screen.dart'; +import 'package:mostro_mobile/generated/l10n.dart'; +import 'package:mostro_mobile/services/mostro_service.dart'; +import 'package:mostro_mobile/shared/providers/mostro_service_provider.dart'; +import 'package:mostro_mobile/shared/providers/order_repository_provider.dart'; +import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; +import 'package:mostro_mobile/shared/providers/storage_providers.dart'; +import 'package:mostro_mobile/shared/notifiers/session_notifier.dart'; +import 'package:mostro_mobile/features/settings/settings.dart'; +import 'package:mostro_mobile/data/repositories/session_storage.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:shared_preferences_platform_interface/in_memory_shared_preferences_async.dart'; +import 'package:shared_preferences_platform_interface/shared_preferences_async_platform_interface.dart'; +import 'package:qr_flutter/qr_flutter.dart'; + +/// The screen is reachable from a notification card, from trade detail and +/// from the maker create flow, so it can be opened at any point of an order's +/// life. "No invoice to show" then means three different things, and telling +/// a maker mid-bond that the invoice expired would make them abandon a valid +/// order (#732 review). +class _NoopSessionStorage implements SessionStorage { + @override + dynamic noSuchMethod(Invocation invocation) => Future.value(); +} + +/// No bond session: the screen only asks for one to pick the maker copy. +class _FixedSessionNotifier extends SessionNotifier { + _FixedSessionNotifier(Ref ref) + : super( + ref, + _NoopSessionStorage(), + Settings( + relays: [], + fullPrivacyMode: false, + mostroPublicKey: 'test', + ), + ) { + state = []; + } +} + +class _IdleMostroService extends MostroService { + _IdleMostroService(super.ref); +} + +class _FixedOrderNotifier extends OrderNotifier { + _FixedOrderNotifier(super.orderId, super.ref, OrderState initial) { + state = initial; + } + + @override + void subscribe() {} + + @override + Future sync() async {} +} + +void main() { + const orderId = 'order-1'; + + setUp(() { + SharedPreferencesAsyncPlatform.instance = + InMemorySharedPreferencesAsync.empty(); + }); + + Order order(enums.Status status) => Order( + id: orderId, + kind: enums.OrderType.buy, + status: status, + amount: 16558, + fiatCode: 'EUR', + fiatAmount: 10, + paymentMethod: 'face to face', + ); + + OrderState stateWith({ + required enums.Status status, + required enums.Action action, + String? invoice, + }) => + OrderState( + status: status, + action: action, + order: order(status), + paymentRequest: invoice == null + ? null + : PaymentRequest(order: order(status), lnInvoice: invoice), + ); + + Future pumpScreen(WidgetTester tester, OrderState state) async { + final router = GoRouter( + initialLocation: '/pay_bond/$orderId', + routes: [ + GoRoute(path: '/', builder: (_, __) => const Scaffold()), + GoRoute( + path: '/trade_detail/:id', builder: (_, __) => const Scaffold()), + GoRoute( + path: '/pay_bond/:orderId', + builder: (_, __) => const PayBondInvoiceScreen(orderId: orderId), + ), + ], + ); + + await tester.pumpWidget( + ProviderScope( + overrides: [ + sharedPreferencesProvider.overrideWithValue(SharedPreferencesAsync()), + sessionNotifierProvider + .overrideWith((ref) => _FixedSessionNotifier(ref)), + mostroServiceProvider.overrideWith((ref) => _IdleMostroService(ref)), + eventProvider(orderId).overrideWithValue(null), + orderNotifierProvider.overrideWith( + (ref, id) => _FixedOrderNotifier(id, ref, state), + ), + ], + child: MaterialApp.router( + routerConfig: router, + localizationsDelegates: S.localizationsDelegates, + supportedLocales: S.supportedLocales, + ), + ), + ); + await tester.pump(); + } + + testWidgets('renders the QR while the bond invoice is the current one', + (tester) async { + await pumpScreen( + tester, + stateWith( + status: enums.Status.waitingTakerBond, + action: enums.Action.payBondInvoice, + invoice: 'lnbcbond', + ), + ); + + expect(find.byType(QrImageView), findsOneWidget); + }); + + testWidgets('waits, without a destructive action, before the invoice arrives', + (tester) async { + await pumpScreen( + tester, + // The maker create flow lands here while this provider is still at its + // initial state: the bond message reaches it only after its own sync(). + stateWith( + status: enums.Status.pending, + action: enums.Action.newOrder, + ), + ); + + expect(find.byType(CircularProgressIndicator), findsOneWidget); + expect(find.byType(QrImageView), findsNothing); + expect(find.byType(ElevatedButton), findsNothing); + }); + + testWidgets('sends the user to the trade once the bond has been paid', + (tester) async { + await pumpScreen( + tester, + stateWith( + status: enums.Status.waitingBuyerInvoice, + action: enums.Action.waitingBuyerInvoice, + ), + ); + + expect(find.byType(QrImageView), findsNothing); + expect(find.byType(CircularProgressIndicator), findsNothing); + + await tester.tap(find.byType(ElevatedButton)); + await tester.pumpAndSettle(); + + expect(find.byType(PayBondInvoiceScreen), findsNothing); + }); + + testWidgets('says the invoice is gone once the take cycle ended', + (tester) async { + await pumpScreen( + tester, + stateWith( + status: enums.Status.canceled, + action: enums.Action.canceled, + ), + ); + + expect(find.byType(QrImageView), findsNothing); + expect(find.byType(CircularProgressIndicator), findsNothing); + expect(find.byType(ElevatedButton), findsOneWidget); + }); +} From 10400ee589df3f1fdc3631df1678d58dae03bef2 Mon Sep 17 00:00:00 2001 From: 21Mill Date: Tue, 15 Sep 2026 22:00:31 +0200 Subject: [PATCH 05/12] fix(order): ignore messages from a take cycle the order has left MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live delivery is not ordered. Once a retake opens a new cycle, a `canceled` from the previous one can still reach the stream, and the stale guard waves it through because a cancelled order outranks every waiting phase: it voided the bond invoice the user was looking at, and downstream it deleted the session and navigated out of a running trade. `applyToCycle` now remembers where the current cycle opened (`cycleStartedAt`) as well as where the previous one ended, and drops anything older. `subscribe()` consults the same predicate before touching the orphan-session timer, the cancel flag, notifications or navigation — the gate `rejectsAdminDisputeMessage` already established for messages `updateWith` refuses. Also fixes the test helper: the OrderNotifier constructor starts a sync() it does not await, so a second sync() while that one runs only requested a replay and the assertions could read the pre-hydration state. --- .../notifiers/abstract_mostro_notifier.dart | 63 +++++++++++++++++-- .../order/notifiers/order_notifier.dart | 1 + .../order_notifier_retake_cycle_test.dart | 46 +++++++++++++- 3 files changed, 103 insertions(+), 7 deletions(-) diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index ca6104464..5110d7486 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -41,16 +41,35 @@ class AbstractMostroNotifier extends StateNotifier { // uses the same Action.canceled. Consumed once per cancel in subscribe(). static final Set _userInitiatedCancels = {}; - /// Marks an order as cancelled by the user, so the matching `canceled` - /// response is treated as a voluntary cancel (immediate session deletion) - /// and notified as user-initiated rather than a counterparty timeout. - @protected /// Event time of the message that left this order in a cycle-ending status. /// /// `null` while the current cycle is still running. Maintained by /// [applyToCycle], which is the only place that may restart a cycle. int? cycleEndedAt; + /// Event time of the message that opened the cycle this order is in. + /// + /// `null` until a cycle has been seen to open. Everything older than it + /// belongs to a cycle that is over — see [precedesActiveCycle]. + int? cycleStartedAt; + + static int _eventTimeOf(MostroMessage message) => + message.eventCreatedAt ?? message.timestamp ?? 0; + + /// Whether [message] belongs to a take cycle this order has already left. + /// + /// The replay is sorted by event time, but live delivery is not: a `canceled` + /// from the previous cycle can reach the stream *after* the new cycle's bond + /// invoice. The stale guard waves it through — a cancelled order outranks + /// every waiting phase — and it would then void the invoice the user is + /// looking at, delete the session and navigate away. Callers use this to + /// drop the message and its side effects, the way + /// [OrderState.rejectsAdminDisputeMessage] already does. + bool precedesActiveCycle(MostroMessage message) { + final startedAt = cycleStartedAt; + return startedAt != null && _eventTimeOf(message) < startedAt; + } + /// Applies [message] to [current], restarting the take cycle first when the /// message opens a *new* one rather than being a late copy of the old. /// @@ -65,9 +84,20 @@ class AbstractMostroNotifier extends StateNotifier { /// replay newest-first, so a copy from the same second as the cancel is /// a late copy: Mostro cannot cancel a take and accept the next one /// within the same second. + /// + /// Messages from before the current cycle opened are dropped outright + /// ([precedesActiveCycle]). OrderState applyToCycle(OrderState current, MostroMessage message) { - final eventTime = message.eventCreatedAt ?? message.timestamp ?? 0; + if (precedesActiveCycle(message)) { + logger.w( + 'Ignoring ${message.action} for order $orderId: it belongs to a take ' + 'cycle that ended at $cycleStartedAt'); + return current; + } + + final eventTime = _eventTimeOf(message); final endedAt = cycleEndedAt; + var restarted = false; if (endedAt != null && eventTime > endedAt && @@ -81,17 +111,27 @@ class AbstractMostroNotifier extends StateNotifier { action: Action.newOrder, order: current.order, ); + restarted = true; } + final cameFromEndedCycle = OrderState.endsTradeCycle(current.status); final next = current.updateWith(message); // Recorded from the resulting status rather than from the transition: a // replay starts from the current state, so the message that ended the // cycle can be applied onto an already-ended one. - cycleEndedAt = OrderState.endsTradeCycle(next.status) ? eventTime : null; + final ended = OrderState.endsTradeCycle(next.status); + cycleEndedAt = ended ? eventTime : null; + if (!ended && (restarted || cameFromEndedCycle)) { + cycleStartedAt = eventTime; + } return next; } + /// Marks an order as cancelled by the user, so the matching `canceled` + /// response is treated as a voluntary cancel (immediate session deletion) + /// and notified as user-initiated rather than a counterparty timeout. + @protected static void markUserInitiatedCancel(String orderId) { _userInitiatedCancels.add(orderId); } @@ -166,6 +206,17 @@ class AbstractMostroNotifier extends StateNotifier { return; } + // Same reasoning for a message the current cycle has outlived: + // live delivery is not ordered, so an old cycle's `canceled` can + // arrive after the new cycle started. Applying it would void the + // new bond invoice; notifying and navigating on it would send + // the user out of a trade that is running (#731). + if (precedesActiveCycle(msg)) { + logger.w( + 'Dropping ${msg.action} for order $orderId: it predates the current take cycle'); + return; + } + // Cancel timer on ANY response from Mostro for this order cancelSessionTimeoutCleanup(orderId); diff --git a/lib/features/order/notifiers/order_notifier.dart b/lib/features/order/notifiers/order_notifier.dart index e0972ea13..66183c37b 100644 --- a/lib/features/order/notifiers/order_notifier.dart +++ b/lib/features/order/notifiers/order_notifier.dart @@ -93,6 +93,7 @@ class OrderNotifier extends AbstractMostroNotifier { // The replay rebuilds the cycle bookkeeping from the history itself. cycleEndedAt = null; + cycleStartedAt = null; for (final message in messages) { if (message.action == Action.cantDo) continue; currentState = applyToCycle(currentState, message); diff --git a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart index 60c56b7eb..9ae726177 100644 --- a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart +++ b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart @@ -158,7 +158,15 @@ void main() { } Future syncedState() async { - await container.read(orderNotifierProvider(orderId).notifier).sync(); + // The OrderNotifier constructor starts a sync() it does not await, and a + // second sync() while that one runs only requests a replay. Let the + // constructor's pass drain first, then run one of our own on a quiet + // notifier, so the state read below is always the hydrated one. + final notifier = container.read(orderNotifierProvider(orderId).notifier); + for (var i = 0; i < 10; i++) { + await Future.delayed(Duration.zero); + } + await notifier.sync(); final state = container.read(orderNotifierProvider(orderId)); return OrderStateSnapshot( status: state.status, @@ -281,6 +289,42 @@ void main() { // about the cycle rule. }); + group('live delivery is not ordered', () { + test('a cancel from the previous cycle cannot void the new invoice', + () async { + await persist([ + ('a', invoice(Action.payBondInvoice, 'lnbcbond', + eventCreatedAt: 1000, timestamp: 1)), + ( + 'b', + message(Action.canceled, Status.canceled, + eventCreatedAt: 3000, timestamp: 2) + ), + ('c', invoice(Action.payBondInvoice, 'lnbcbond2', + eventCreatedAt: 4000, timestamp: 3)), + ]); + + // Replay leaves the notifier inside the new cycle… + expect((await syncedState()).invoice, 'lnbcbond2'); + + // …and the previous cycle's cancel, delivered late by the live stream, + // must not reach the state: it would void the invoice on screen, and + // downstream it deletes the session and navigates away. + final notifier = + container.read(orderNotifierProvider(orderId).notifier); + final lateCancel = message(Action.canceled, Status.canceled, + eventCreatedAt: 3000, timestamp: 9); + + expect(notifier.precedesActiveCycle(lateCancel), isTrue); + final after = + notifier.applyToCycle(container.read(orderNotifierProvider(orderId)), + lateCancel); + + expect(after.status, Status.waitingTakerBond); + expect(after.paymentRequest?.lnInvoice, 'lnbcbond2'); + }); + }); + group('a pending cooperative cancel is not a cycle end', () { test('a late setup copy keeps fiatWasSent', () async { await persist([ From dda95d72838830dc2a3c9af1dd32ce57e858c78b Mon Sep 17 00:00:00 2001 From: 21Mill Date: Tue, 15 Sep 2026 22:03:57 +0200 Subject: [PATCH 06/12] test(order): make the retake replay tests wait on a real sync() The OrderNotifier constructor starts a sync() nothing can await, and a second sync() while that one runs only sets _resyncRequested and returns, so the assertions could read pre-hydration state. Draining event-loop turns first only narrowed the window: the storage read can still be pending. The test notifier now swallows the constructor's pass, leaving the sync() the helper awaits as the only one that runs. --- .../order_notifier_retake_cycle_test.dart | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart index 9ae726177..85213488d 100644 --- a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart +++ b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart @@ -68,12 +68,28 @@ class _FixedSessionNotifier extends SessionNotifier { } } -/// Real `sync()`, no live stream. +/// Real `sync()`, no live stream — and no unawaited pass either. +/// +/// The OrderNotifier constructor starts a `sync()` nothing can await, and a +/// second `sync()` while that one runs only sets `_resyncRequested` and +/// returns. Swallowing the constructor's call leaves the pass the test awaits +/// as the only one, so the assertions never race hydration. class _SyncOnlyOrderNotifier extends OrderNotifier { _SyncOnlyOrderNotifier(super.orderId, super.ref); + bool _constructorSyncSkipped = false; + @override void subscribe() {} + + @override + Future sync() async { + if (!_constructorSyncSkipped) { + _constructorSyncSkipped = true; + return; + } + return super.sync(); + } } void main() { @@ -158,15 +174,9 @@ void main() { } Future syncedState() async { - // The OrderNotifier constructor starts a sync() it does not await, and a - // second sync() while that one runs only requests a replay. Let the - // constructor's pass drain first, then run one of our own on a quiet - // notifier, so the state read below is always the hydrated one. - final notifier = container.read(orderNotifierProvider(orderId).notifier); - for (var i = 0; i < 10; i++) { - await Future.delayed(Duration.zero); - } - await notifier.sync(); + // _SyncOnlyOrderNotifier swallows the constructor's unawaited sync(), so + // this is the only pass and awaiting it is enough. + await container.read(orderNotifierProvider(orderId).notifier).sync(); final state = container.read(orderNotifierProvider(orderId)); return OrderStateSnapshot( status: state.status, From 73d7011010cb4ca3434ef1ba6291e1361d41b98f Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sat, 3 Oct 2026 02:22:29 +0200 Subject: [PATCH 07/12] fix(order): stop the bond screen lying to a maker in its empty states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pending` meant two opposite things to `stillLoading`: the taker's bond invoice has not arrived yet, and the maker's bond is paid and the order is live in the book. The pay-bond notification stays in the history and `notification_item.dart` pushes this screen, so a maker who tapped it after paying got "Waiting for the deposit invoice…" and a spinner that could never resolve, with no action and only the back arrow as an exit. On main that state rendered an empty QR, so the spinner came from this PR. `Session.bondPending` already separates the two, and the screen already reads it as `isMakerBond`. Loading now requires a bond to actually be awaited; a published maker order falls through to the paid branch, which gets copy that fits it — the order is in the book, not "your trade is already in progress". The cycle-ended branch had the same problem: it told a maker whose own order expired to "take the order again", which a maker cannot do. Two notes on the signal. `bondPending` is transient and never persisted, so after a restart a maker in bond limbo sees the paid branch instead of the spinner; that window is seconds long and the paid branch is at least actionable. And it is read with `ref.read` on a mutable field, so it is evaluated at build time and nothing here depends on it changing live. The locale set grew to seven: `intl_nl.arb` arrived on main after this branch and had none of this PR's four bond keys, which `flutter gen-l10n` reported as untranslated. It now carries them and the two new ones. --- .../screens/pay_bond_invoice_screen.dart | 27 ++++-- lib/l10n/intl_de.arb | 2 + lib/l10n/intl_en.arb | 2 + lib/l10n/intl_es.arb | 2 + lib/l10n/intl_fr.arb | 2 + lib/l10n/intl_it.arb | 2 + lib/l10n/intl_nl.arb | 6 ++ lib/l10n/intl_pt.arb | 2 + .../screens/pay_bond_invoice_screen_test.dart | 93 +++++++++++++++++-- 9 files changed, 123 insertions(+), 15 deletions(-) diff --git a/lib/features/order/screens/pay_bond_invoice_screen.dart b/lib/features/order/screens/pay_bond_invoice_screen.dart index a6dc52603..cbdfdbaa7 100644 --- a/lib/features/order/screens/pay_bond_invoice_screen.dart +++ b/lib/features/order/screens/pay_bond_invoice_screen.dart @@ -151,13 +151,15 @@ class PayBondInvoiceScreen extends ConsumerWidget { final cycleEnded = OrderState.endsTradeCycle(orderState.status) && orderState.status != enums.Status.pending; - // `pending` here is the notifier's initial state as well: the maker bond - // arrives on AddOrderNotifier and only reaches this provider once its - // sync() has read the message, so the first frames legitimately have - // nothing to render. + // `pending` is not enough on its own: it is the notifier's initial state + // while a maker bond is still loading, *and* the state of a maker's + // order that is paid and live in the book. The pay-bond notification + // stays in the history and pushes this screen, so a maker who taps it + // after paying used to get a spinner that could never resolve (#732 + // review). `bondPending` is the marker that separates the two. final stillLoading = !cycleEnded && - (orderState.status == enums.Status.pending || - orderState.status == enums.Status.waitingTakerBond); + (orderState.status == enums.Status.waitingTakerBond || + (orderState.status == enums.Status.pending && isMakerBond)); if (stillLoading) { return _EmptyBondState( @@ -171,7 +173,11 @@ class PayBondInvoiceScreen extends ConsumerWidget { return _EmptyBondState( title: s.bondScreenTitle, icon: Icons.hourglass_disabled, - message: s.bondInvoiceUnavailable, + // A maker cannot take their own order, so "take the order again" + // only makes sense to a taker. + message: isMakerBond + ? s.bondOrderNoLongerActive + : s.bondInvoiceUnavailable, // The copy says "go back", so go back when there is a stack to pop. actionLabel: s.close, onAction: (context) => @@ -179,11 +185,14 @@ class PayBondInvoiceScreen extends ConsumerWidget { ); } - // Past the bond phase: the bond is paid and the trade moved on. + // Past the bond phase: the bond is paid. For a taker that means the + // trade moved on; for the maker of a pending order it means the order + // is in the book, with no trade to go to yet. + final orderIsPublished = orderState.status == enums.Status.pending; return _EmptyBondState( title: s.bondScreenTitle, icon: Icons.check_circle_outline, - message: s.bondAlreadyPaid, + message: orderIsPublished ? s.bondOrderPublished : s.bondAlreadyPaid, actionLabel: s.goToTrade, onAction: (context) => context.go('/trade_detail/$orderId'), ); diff --git a/lib/l10n/intl_de.arb b/lib/l10n/intl_de.arb index 435ba909a..1c65383f0 100644 --- a/lib/l10n/intl_de.arb +++ b/lib/l10n/intl_de.arb @@ -847,6 +847,8 @@ "bondAlreadyPaid": "Diese Kaution ist bereits bezahlt. Dein Handel läuft schon.", "goToTrade": "ZUM HANDEL", "bondInvoiceUnavailable": "Diese Kautions-Rechnung ist nicht mehr gültig. Gehe zurück und nimm die Order erneut an, um eine neue zu erhalten.", + "bondOrderPublished": "Deine Order ist im Orderbuch veröffentlicht und ihre Kaution ist bereits bezahlt.", + "bondOrderNoLongerActive": "Diese Order ist nicht mehr aktiv, daher ist ihre Kautions-Rechnung nicht mehr gültig.", "bondScreenTitle": "Anti-Missbrauchs-Kaution", "bondExplanation": "Um diese Order anzunehmen, musst du eine Anti-Missbrauchs-Kaution zahlen. So funktioniert es:\n\n• Deine Sats werden in deiner Wallet gehalten, sie werden nicht ausgegeben.\n• Wenn der Tausch erfolgreich abgeschlossen wird, erhältst du deine Kaution automatisch zurück.\n• Wenn du bei dieser Order einen Streit hast und ihn verlierst, verlierst du die Kaution.\n• Dieser Mechanismus schützt alle Nutzer vor Betrügern.", "bondExplanationMaker": "Bevor deine Order erstellt werden kann, musst du eine Anti-Missbrauchs-Kaution zahlen. Lass diesen Bildschirm geöffnet, bis die Zahlung abgeschlossen ist – wenn du ihn schließt oder nicht zahlst, wird die Order nicht erstellt. So funktioniert es:\n\n• Deine Sats werden in deiner Wallet gehalten, sie werden nicht ausgegeben.\n• Wenn der Tausch erfolgreich abgeschlossen wird, erhältst du deine Kaution automatisch zurück.\n• Wenn du bei dieser Order einen Streit hast und ihn verlierst, verlierst du die Kaution.\n• Dieser Mechanismus schützt alle Nutzer vor Betrügern.", diff --git a/lib/l10n/intl_en.arb b/lib/l10n/intl_en.arb index 9c13afbd2..252fd4a2e 100644 --- a/lib/l10n/intl_en.arb +++ b/lib/l10n/intl_en.arb @@ -847,6 +847,8 @@ "bondAlreadyPaid": "This deposit has already been paid. Your trade is already in progress.", "goToTrade": "GO TO TRADE", "bondInvoiceUnavailable": "This bond invoice is no longer valid. Go back and take the order again to get a new one.", + "bondOrderPublished": "Your order is published in the order book and its deposit has already been paid.", + "bondOrderNoLongerActive": "This order is no longer active, so its deposit invoice is no longer valid.", "bondScreenTitle": "Anti-abuse deposit", "bondExplanation": "To take this order you must pay an anti-abuse deposit. Here's how it works:\n\n• Your sats are held in your wallet, they are not spent.\n• If the exchange completes successfully, you get your deposit back automatically.\n• If you have a dispute on this order and lose it, you will lose the deposit.\n• This mechanism protects all users against scammers.", "bondExplanationMaker": "Before your order can be created, you must pay an anti-abuse deposit. Keep this screen open until the payment is done — if you close it or don't pay, the order won't be created. Here's how it works:\n\n• Your sats are held in your wallet, they are not spent.\n• If the exchange completes successfully, you get your deposit back automatically.\n• If you have a dispute on this order and lose it, you will lose the deposit.\n• This mechanism protects all users against scammers.", diff --git a/lib/l10n/intl_es.arb b/lib/l10n/intl_es.arb index ad1f3da66..02ebaae37 100644 --- a/lib/l10n/intl_es.arb +++ b/lib/l10n/intl_es.arb @@ -697,6 +697,8 @@ "bondAlreadyPaid": "Este depósito ya está pagado. Tu intercambio ya está en marcha.", "goToTrade": "IR AL INTERCAMBIO", "bondInvoiceUnavailable": "Esta factura de depósito ya no es válida. Vuelve atrás y toma la orden de nuevo para obtener una nueva.", + "bondOrderPublished": "Tu orden está publicada en el libro de órdenes y su depósito ya está pagado.", + "bondOrderNoLongerActive": "Esta orden ya no está activa, así que su factura de depósito ya no es válida.", "bondScreenTitle": "Depósito anti-abuso", "bondExplanation": "Para tomar esta orden debes pagar un depósito anti-abuso. Funciona así:\n\n• Tus sats quedan retenidos en tu wallet, no se gastan.\n• Si el intercambio finaliza correctamente, recuperas el depósito automáticamente.\n• Si tienes una disputa en esta orden y la pierdes, perderás el depósito.\n• Este mecanismo protege a todos los usuarios contra estafadores.", "bondExplanationMaker": "Para crear tu orden primero debes pagar un depósito anti-abuso. Mantén esta pantalla abierta hasta completar el pago: si la cierras o no pagas, la orden no se creará. Funciona así:\n\n• Tus sats quedan retenidos en tu wallet, no se gastan.\n• Si el intercambio finaliza correctamente, recuperas el depósito automáticamente.\n• Si tienes una disputa en esta orden y la pierdes, perderás el depósito.\n• Este mecanismo protege a todos los usuarios contra estafadores.", diff --git a/lib/l10n/intl_fr.arb b/lib/l10n/intl_fr.arb index 258e5cedb..eec767af7 100644 --- a/lib/l10n/intl_fr.arb +++ b/lib/l10n/intl_fr.arb @@ -847,6 +847,8 @@ "bondAlreadyPaid": "Ce dépôt est déjà payé. Votre échange est déjà en cours.", "goToTrade": "ALLER À L'ÉCHANGE", "bondInvoiceUnavailable": "Cette facture de dépôt n'est plus valide. Revenez en arrière et reprenez l'ordre pour en obtenir une nouvelle.", + "bondOrderPublished": "Votre ordre est publié dans le carnet d'ordres et son dépôt est déjà payé.", + "bondOrderNoLongerActive": "Cet ordre n'est plus actif, sa facture de dépôt n'est donc plus valide.", "bondScreenTitle": "Dépôt anti-abus", "bondExplanation": "Pour prendre cette commande, vous devez payer un dépôt anti-abus. Voici comment cela fonctionne :\n\n• Vos sats sont conservés dans votre portefeuille, ils ne sont pas dépensés.\n• Si l'échange se termine avec succès, vous récupérez votre dépôt automatiquement.\n• Si vous avez un litige sur cette commande et que vous le perdez, vous perdrez le dépôt.\n• Ce mécanisme protège tous les utilisateurs contre les escrocs.", "bondExplanationMaker": "Avant que votre commande puisse être créée, vous devez payer un dépôt anti-abus. Gardez cet écran ouvert jusqu'à ce que le paiement soit effectué — si vous le fermez ou ne payez pas, la commande ne sera pas créée. Voici comment cela fonctionne :\n\n• Vos sats sont conservés dans votre portefeuille, ils ne sont pas dépensés.\n• Si l'échange se termine avec succès, vous récupérez votre dépôt automatiquement.\n• Si vous avez un litige sur cette commande et que vous le perdez, vous perdrez le dépôt.\n• Ce mécanisme protège tous les utilisateurs contre les escrocs.", diff --git a/lib/l10n/intl_it.arb b/lib/l10n/intl_it.arb index 92ecb1e23..daf56ad34 100644 --- a/lib/l10n/intl_it.arb +++ b/lib/l10n/intl_it.arb @@ -727,6 +727,8 @@ "bondAlreadyPaid": "Questo deposito è già stato pagato. Il tuo scambio è già in corso.", "goToTrade": "VAI ALLO SCAMBIO", "bondInvoiceUnavailable": "Questa fattura di deposito non è più valida. Torna indietro e prendi di nuovo l'ordine per ottenerne una nuova.", + "bondOrderPublished": "Il tuo ordine è pubblicato nel libro ordini e il suo deposito è già stato pagato.", + "bondOrderNoLongerActive": "Questo ordine non è più attivo, quindi la sua fattura di deposito non è più valida.", "bondScreenTitle": "Deposito anti-abuso", "bondExplanation": "Per prendere questo ordine devi pagare un deposito anti-abuso. Funziona così:\n\n• I tuoi sats vengono trattenuti nel tuo wallet, non vengono spesi.\n• Se lo scambio si conclude correttamente, recuperi il deposito automaticamente.\n• Se hai una disputa su questo ordine e la perdi, perderai il deposito.\n• Questo meccanismo protegge tutti gli utenti dai truffatori.", "bondExplanationMaker": "Prima che il tuo ordine possa essere creato, devi pagare un deposito anti-abuso. Tieni aperta questa schermata fino al completamento del pagamento: se la chiudi o non paghi, l'ordine non verrà creato. Funziona così:\n\n• I tuoi sats vengono trattenuti nel tuo wallet, non vengono spesi.\n• Se lo scambio si conclude correttamente, recuperi il deposito automaticamente.\n• Se hai una disputa su questo ordine e la perdi, perderai il deposito.\n• Questo meccanismo protegge tutti gli utenti dai truffatori.", diff --git a/lib/l10n/intl_nl.arb b/lib/l10n/intl_nl.arb index 3e82ff944..1484ae34d 100644 --- a/lib/l10n/intl_nl.arb +++ b/lib/l10n/intl_nl.arb @@ -846,6 +846,12 @@ "bondScreenTitle": "Borg tegen misbruik", "bondExplanation": "Om deze order te accepteren moet je een borg tegen misbruik betalen. Zo werkt het:\n\n• Je sats worden in je wallet vastgezet, niet uitgegeven.\n• Verloopt de ruil goed, dan krijg je je borg automatisch terug.\n• Krijg je een dispuut over deze order en verlies je dat, dan ben je de borg kwijt.\n• Dit beschermt alle gebruikers tegen oplichters.", "bondExplanationMaker": "Voordat je order kan worden aangemaakt, moet je een borg tegen misbruik betalen. Houd dit scherm open tot de betaling rond is; sluit je het of betaal je niet, dan wordt de order niet aangemaakt. Zo werkt het:\n\n• Je sats worden in je wallet vastgezet, niet uitgegeven.\n• Verloopt de ruil goed, dan krijg je je borg automatisch terug.\n• Krijg je een dispuut over deze order en verlies je dat, dan ben je de borg kwijt.\n• Dit beschermt alle gebruikers tegen oplichters.", + "bondInvoicePending": "Wachten op de borg-invoice…", + "bondAlreadyPaid": "Deze borg is al betaald. Je ruil is al onderweg.", + "goToTrade": "NAAR DE RUIL", + "bondInvoiceUnavailable": "Deze borg-invoice is niet meer geldig. Ga terug en accepteer de order opnieuw voor een nieuwe.", + "bondOrderPublished": "Je order staat in het orderboek en de borg is al betaald.", + "bondOrderNoLongerActive": "Deze order is niet meer actief, dus de borg-invoice is niet meer geldig.", "bondPayInvoicePrompt": "Betaal de invoice hieronder van {amount} sats om door te gaan, of annuleer als je niet akkoord gaat.", "statusWaitingTakerBond": "Wacht op borg", "payBondMessage": "Betaal de borg tegen misbruik om door te gaan met deze order.", diff --git a/lib/l10n/intl_pt.arb b/lib/l10n/intl_pt.arb index b6517af06..f1de6899c 100644 --- a/lib/l10n/intl_pt.arb +++ b/lib/l10n/intl_pt.arb @@ -847,6 +847,8 @@ "bondAlreadyPaid": "Este depósito já foi pago. Sua troca já está em andamento.", "goToTrade": "IR PARA A TROCA", "bondInvoiceUnavailable": "Esta fatura de depósito não é mais válida. Volte atrás e aceite a ordem novamente para obter uma nova.", + "bondOrderPublished": "Sua ordem está publicada no livro de ordens e o depósito dela já foi pago.", + "bondOrderNoLongerActive": "Esta ordem não está mais ativa, então a fatura de depósito dela não é mais válida.", "bondScreenTitle": "Depósito antiabuso", "bondExplanation": "Para aceitar esta ordem, você deve pagar um depósito antiabuso. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", "bondExplanationMaker": "Antes que sua ordem possa ser criada, você deve pagar um depósito antiabuso. Mantenha esta tela aberta até que o pagamento seja concluído — se você fechá-la ou não pagar, a ordem não será criada. Veja como funciona:\n\n• Seus sats ficam retidos na sua carteira, eles não são gastos.\n• Se a troca for concluída com sucesso, você recebe seu depósito de volta automaticamente.\n• Se você tiver uma disputa nesta ordem e perdê-la, você perderá o depósito.\n• Este mecanismo protege todos os usuários contra golpistas.", diff --git a/test/features/order/screens/pay_bond_invoice_screen_test.dart b/test/features/order/screens/pay_bond_invoice_screen_test.dart index cdf99da97..dbf5bd2fe 100644 --- a/test/features/order/screens/pay_bond_invoice_screen_test.dart +++ b/test/features/order/screens/pay_bond_invoice_screen_test.dart @@ -17,6 +17,7 @@ import 'package:mostro_mobile/shared/providers/storage_providers.dart'; import 'package:mostro_mobile/shared/notifiers/session_notifier.dart'; import 'package:mostro_mobile/features/settings/settings.dart'; import 'package:mostro_mobile/data/repositories/session_storage.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; import 'package:shared_preferences/shared_preferences.dart'; import 'package:shared_preferences_platform_interface/in_memory_shared_preferences_async.dart'; import 'package:shared_preferences_platform_interface/shared_preferences_async_platform_interface.dart'; @@ -32,9 +33,11 @@ class _NoopSessionStorage implements SessionStorage { dynamic noSuchMethod(Invocation invocation) => Future.value(); } -/// No bond session: the screen only asks for one to pick the maker copy. +/// The screen asks the session whether a bond is actually awaited, which is +/// what separates "the invoice has not arrived" from "the bond is paid and the +/// order is live in the book". class _FixedSessionNotifier extends SessionNotifier { - _FixedSessionNotifier(Ref ref) + _FixedSessionNotifier(Ref ref, this._sessions) : super( ref, _NoopSessionStorage(), @@ -44,10 +47,36 @@ class _FixedSessionNotifier extends SessionNotifier { mostroPublicKey: 'test', ), ) { - state = []; + state = _sessions; + } + + final List _sessions; + + // The real lookup reads a private map that only its own writes populate, so + // seeding `state` is not enough for a double. + @override + Session? getSessionByOrderId(String orderId) { + for (final session in _sessions) { + if (session.orderId == orderId) return session; + } + return null; } } +/// A maker session for [orderId], in bond limbo or past it. +Session _makerSession(String orderId, {required bool bondPending}) { + final session = Session( + masterKey: NostrUtils.generateKeyPair(), + tradeKey: NostrUtils.generateKeyPair(), + keyIndex: 1, + fullPrivacy: false, + startTime: DateTime.now(), + orderId: orderId, + ); + session.bondPending = bondPending; + return session; +} + class _IdleMostroService extends MostroService { _IdleMostroService(super.ref); } @@ -96,7 +125,14 @@ void main() { : PaymentRequest(order: order(status), lnInvoice: invoice), ); - Future pumpScreen(WidgetTester tester, OrderState state) async { + /// [bondPending] mirrors `Session.bondPending`: true while a maker-created + /// order sits in bond limbo, false once the bond is paid and the order is + /// published — and false too when there is no session at all (a taker). + Future pumpScreen( + WidgetTester tester, + OrderState state, { + bool? bondPending, + }) async { final router = GoRouter( initialLocation: '/pay_bond/$orderId', routes: [ @@ -114,8 +150,14 @@ void main() { ProviderScope( overrides: [ sharedPreferencesProvider.overrideWithValue(SharedPreferencesAsync()), - sessionNotifierProvider - .overrideWith((ref) => _FixedSessionNotifier(ref)), + sessionNotifierProvider.overrideWith( + (ref) => _FixedSessionNotifier( + ref, + bondPending == null + ? [] + : [_makerSession(orderId, bondPending: bondPending)], + ), + ), mostroServiceProvider.overrideWith((ref) => _IdleMostroService(ref)), eventProvider(orderId).overrideWithValue(null), orderNotifierProvider.overrideWith( @@ -156,6 +198,7 @@ void main() { status: enums.Status.pending, action: enums.Action.newOrder, ), + bondPending: true, ); expect(find.byType(CircularProgressIndicator), findsOneWidget); @@ -163,6 +206,25 @@ void main() { expect(find.byType(ElevatedButton), findsNothing); }); + // The pay-bond notification stays in the history, and tapping it pushes this + // screen. A maker who taps it after paying is on a published order: no + // invoice is coming, so a spinner would never resolve (#732 review). + testWidgets('does not spin on a published maker order', (tester) async { + await pumpScreen( + tester, + stateWith( + status: enums.Status.pending, + action: enums.Action.newOrder, + ), + bondPending: false, + ); + await tester.pump(const Duration(seconds: 5)); + + expect(find.byType(CircularProgressIndicator), findsNothing); + expect(find.byType(QrImageView), findsNothing); + expect(find.byType(ElevatedButton), findsOneWidget); + }); + testWidgets('sends the user to the trade once the bond has been paid', (tester) async { await pumpScreen( @@ -182,6 +244,25 @@ void main() { expect(find.byType(PayBondInvoiceScreen), findsNothing); }); + // A maker cannot take their own order, so "take the order again" is wrong + // for the maker of an expired one (#732 review). + testWidgets('tells a maker their own order is over, not to retake it', + (tester) async { + await pumpScreen( + tester, + stateWith( + status: enums.Status.expired, + action: enums.Action.canceled, + ), + bondPending: true, + ); + + final context = tester.element(find.byType(PayBondInvoiceScreen)); + final s = S.of(context)!; + expect(find.text(s.bondOrderNoLongerActive), findsOneWidget); + expect(find.text(s.bondInvoiceUnavailable), findsNothing); + }); + testWidgets('says the invoice is gone once the take cycle ended', (tester) async { await pumpScreen( From 2b165c4e804d0c4167d2f3d83d15e3817dcb5e84 Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sat, 3 Oct 2026 02:22:39 +0200 Subject: [PATCH 08/12] fix(order): route the restore state write through the cycle rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `updateStateFromMessage` wrote `state.updateWith(message)` directly, so it was the one state write in OrderNotifier that skipped `applyToCycle`. RestoreManager calls it for every restored order — cancelled and expired included — and for the restored dispute, so `cycleEndedAt` stayed null, the restart condition could not fire, and the next take of that order had its bond invoice dropped as a stale transition: #731 again, through the one door that bypassed the new rule. It lasted until something ran `sync()`. Routing it through `applyToCycle` exposed a unit mismatch that had to be fixed with it. The restored messages carry only a `timestamp`, taken straight from `orderDetail.createdAt`, and mostrod reports that in seconds (`Timestamp::now().as_secs()` in `mostro/src/flow.rs`) while `MostroMessage.timestamp` and `eventCreatedAt` are both milliseconds. Once restored messages drive the cycle bookkeeping, a `cycleStartedAt` set by a live message is ~1000x larger than any restored event time, so `precedesActiveCycle` would have silently dropped every restored message as predating the current cycle. `restoreCreatedAtMillis` now carries that conversion and its contract. It also fixes a pre-existing bug at the same root: the restored dispute was built with `DateTime.fromMillisecondsSinceEpoch(createdAt)`, which dated every restored dispute to January 1970. The `Order` payload keeps the raw seconds value, which is the scale the protocol uses for that field. --- .../order/notifiers/order_notifier.dart | 9 +++++- lib/features/restore/restore_manager.dart | 27 ++++++++++------ .../order_notifier_retake_cycle_test.dart | 31 ++++++++++++++++++- .../features/restore/restore_decode_test.dart | 24 ++++++++++++++ 4 files changed, 80 insertions(+), 11 deletions(-) diff --git a/lib/features/order/notifiers/order_notifier.dart b/lib/features/order/notifiers/order_notifier.dart index 66183c37b..a747cb067 100644 --- a/lib/features/order/notifiers/order_notifier.dart +++ b/lib/features/order/notifiers/order_notifier.dart @@ -275,9 +275,16 @@ class OrderNotifier extends AbstractMostroNotifier { } /// Update state from MostroMessage (used during restore) + /// + /// Goes through [applyToCycle] like `sync()` and the live stream do. + /// RestoreManager calls this for every restored order, cancelled and expired + /// included; writing the state directly left `cycleEndedAt` null, so the + /// next take of that order could not restart the cycle and its bond invoice + /// was dropped as stale — #731 again, through the one write that skipped the + /// rule (#732 review). void updateStateFromMessage(MostroMessage message) { if (mounted) { - state = state.updateWith(message); + state = applyToCycle(state, message); } } diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index bb1463d33..3ee29fdad 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -773,9 +773,8 @@ class RestoreService { disputeId: restoredDispute.disputeId, orderId: restoredDispute.orderId, status: restoredDispute.status, - createdAt: orderDetail.createdAt != null - ? DateTime.fromMillisecondsSinceEpoch(orderDetail.createdAt!) - : DateTime.now(), + createdAt: DateTime.fromMillisecondsSinceEpoch( + restoreCreatedAtMillis(orderDetail.createdAt)), action: userInitiated ? 'dispute-initiated-by-you' : 'dispute-initiated-by-peer', @@ -819,9 +818,7 @@ class RestoreService { id: orderDetail.id, action: action, payload: dispute, - timestamp: - orderDetail.createdAt ?? - DateTime.now().millisecondsSinceEpoch, + timestamp: restoreCreatedAtMillis(orderDetail.createdAt), ); // Save dispute message to storage @@ -861,9 +858,7 @@ class RestoreService { id: orderDetail.id, action: action, payload: order, - timestamp: - orderDetail.createdAt ?? - DateTime.now().millisecondsSinceEpoch, + timestamp: restoreCreatedAtMillis(orderDetail.createdAt), ); // Save order message to storage @@ -1160,6 +1155,20 @@ class RestoreService { /// Top-level (not a private method) so the transport branch can be /// regression-tested without the full [RestoreService] / Riverpod orchestration. @visibleForTesting +/// Converts mostrod's `created_at` to the millisecond scale this app stores. +/// +/// The daemon reports it in seconds (`Timestamp::now().as_secs()` in +/// `mostro/src/flow.rs`), while [MostroMessage.timestamp] and +/// [MostroMessage.eventCreatedAt] are both milliseconds. Feeding the raw value +/// in put restored messages ~1000x in the past, which mattered once restored +/// messages started driving the take-cycle bookkeeping (#732 review), and it +/// also dated restored disputes to January 1970. +@visibleForTesting +int restoreCreatedAtMillis(int? createdAtSeconds) => + createdAtSeconds != null + ? createdAtSeconds * Duration.millisecondsPerSecond + : DateTime.now().millisecondsSinceEpoch; + Future> decodeRestoreMessage( NostrEvent event, NostrKeyPairs tempTradeKey, diff --git a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart index 85213488d..1abc6f1ba 100644 --- a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart +++ b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart @@ -138,7 +138,7 @@ void main() { MostroMessage message( Action action, Status status, { - required int eventCreatedAt, + required int? eventCreatedAt, required int timestamp, }) => MostroMessage( @@ -366,6 +366,35 @@ void main() { expect(state.fiatWasSent, isTrue); }); }); + + // RestoreManager writes state through updateStateFromMessage, not sync(). If + // that path skips the cycle bookkeeping, cycleEndedAt stays null, the + // restart condition cannot fire and the next take's bond invoice is dropped + // as stale — #731 again, through the one write that bypassed the rule + // (#732 review). + group('the restore path', () { + test('a restored cancel still lets the next take open a cycle', () async { + final notifier = + container.read(orderNotifierProvider(orderId).notifier); + + // What RestoreManager builds: a locally synthesized message, so no + // eventCreatedAt — only a timestamp, in milliseconds. + notifier.updateStateFromMessage( + message(Action.canceled, Status.canceled, + eventCreatedAt: null, timestamp: 3000), + ); + + final next = notifier.applyToCycle( + container.read(orderNotifierProvider(orderId)), + invoice(Action.payBondInvoice, 'lnbcnewbond', + eventCreatedAt: 5000, timestamp: 5000), + ); + + expect(next.status, Status.waitingTakerBond); + expect(next.paymentRequest?.lnInvoice, 'lnbcnewbond'); + }); + + }); } /// The few fields these scenarios assert on. diff --git a/test/features/restore/restore_decode_test.dart b/test/features/restore/restore_decode_test.dart index bac757217..7b0d7b71e 100644 --- a/test/features/restore/restore_decode_test.dart +++ b/test/features/restore/restore_decode_test.dart @@ -117,4 +117,28 @@ void main() { ); }); }); + + // Mostrod reports created_at in seconds; MostroMessage timestamps are + // milliseconds. Restored messages now drive the take-cycle bookkeeping, so + // the scale is load-bearing (#732 review). + group('restoreCreatedAtMillis', () { + test('scales the daemon seconds to milliseconds', () { + // 2026-10-02T23:23:29Z, as the node's orders table stores it. + expect(restoreCreatedAtMillis(1790983409), 1790983409000); + expect( + DateTime.fromMillisecondsSinceEpoch( + restoreCreatedAtMillis(1790983409), isUtc: true) + .year, + 2026, + ); + }); + + test('falls back to now, already in milliseconds', () { + final before = DateTime.now().millisecondsSinceEpoch; + final value = restoreCreatedAtMillis(null); + expect(value, greaterThanOrEqualTo(before)); + expect(DateTime.fromMillisecondsSinceEpoch(value).year, + DateTime.now().year); + }); + }); } From 1f145d2196a7b3d08bbe367ffbb8f2ba76afce55 Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sat, 3 Oct 2026 02:22:45 +0200 Subject: [PATCH 09/12] docs(order): the cycle guard is a defence, not an observed delivery Both comments on `precedesActiveCycle` said live delivery can hand over an older cycle's message. It cannot: the orders stream is `watchLatestMessage`, which emits only the newest stored message by `compareByEventTime`, so a previous cycle's `canceled` stored after the new bond invoice is never the latest and never reaches it. The guard is worth keeping, but the next reader should not build on a delivery that the stream cannot make. --- .../notifiers/abstract_mostro_notifier.dart | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index 5110d7486..2cdafeb74 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -58,13 +58,18 @@ class AbstractMostroNotifier extends StateNotifier { /// Whether [message] belongs to a take cycle this order has already left. /// - /// The replay is sorted by event time, but live delivery is not: a `canceled` - /// from the previous cycle can reach the stream *after* the new cycle's bond - /// invoice. The stale guard waves it through — a cancelled order outranks - /// every waiting phase — and it would then void the invoice the user is - /// looking at, delete the session and navigate away. Callers use this to - /// drop the message and its side effects, the way + /// Such a message would be destructive: the stale guard waves a previous + /// cycle's `canceled` through — a cancelled order outranks every waiting + /// phase — so applying it would void the invoice the user is looking at, + /// delete the session and navigate away. Callers use this to drop the + /// message and its side effects, the way /// [OrderState.rejectsAdminDisputeMessage] already does. + /// + /// This is a defence, not a case observed on the live stream: the orders + /// stream is `watchLatestMessage`, which emits only the newest stored + /// message by `compareByEventTime`, so an older cycle's `canceled` stored + /// after the new bond is never the latest and never reaches it (#732 + /// review). It still guards `sync()` and any future caller. bool precedesActiveCycle(MostroMessage message) { final startedAt = cycleStartedAt; return startedAt != null && _eventTimeOf(message) < startedAt; @@ -206,11 +211,12 @@ class AbstractMostroNotifier extends StateNotifier { return; } - // Same reasoning for a message the current cycle has outlived: - // live delivery is not ordered, so an old cycle's `canceled` can - // arrive after the new cycle started. Applying it would void the - // new bond invoice; notifying and navigating on it would send - // the user out of a trade that is running (#731). + // Same reasoning for a message the current cycle has + // outlived: applying it would void the new bond invoice, and + // notifying or navigating on it would send the user out of a + // trade that is running (#731). Kept as a defence — the stream + // emits only the latest stored message, so it should not be + // able to hand one over. if (precedesActiveCycle(msg)) { logger.w( 'Dropping ${msg.action} for order $orderId: it predates the current take cycle'); From a22693c8485e9ccaf79a6abe96df39d25c23ea2f Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sat, 3 Oct 2026 02:30:30 +0200 Subject: [PATCH 10/12] fix(order): do not tell a taker their order is published MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing the maker spinner made `pending` with no invoice fall through to the paid branch, which now claims "your order is published in the order book". A taker reaches that state two ways: their take timed out and Mostro put the order back in the book, or they opened the screen from a notification before this order's notifier had synced. Neither owns an order in the book, and the first had their bond returned, so the copy was a false statement with a GO TO TRADE button under it. `pending` needs three signals, not one. An absent order payload is the notifier's unhydrated initial state, so waiting is the honest answer there. `bondPending` means a maker bond is actually awaited. And a session for this order means the order is the user's own: Mostro deletes the taker's session when it republishes, so a pending order with no session belongs to someone else and the taker copy — go back and take it again — is the correct one. Tests cover a taker on a republished order and the unhydrated state, which had no coverage: every pending case went through a maker session. --- .../screens/pay_bond_invoice_screen.dart | 59 +++++++++++++------ .../screens/pay_bond_invoice_screen_test.dart | 46 ++++++++++++++- 2 files changed, 86 insertions(+), 19 deletions(-) diff --git a/lib/features/order/screens/pay_bond_invoice_screen.dart b/lib/features/order/screens/pay_bond_invoice_screen.dart index cbdfdbaa7..bd440cfa0 100644 --- a/lib/features/order/screens/pay_bond_invoice_screen.dart +++ b/lib/features/order/screens/pay_bond_invoice_screen.dart @@ -137,28 +137,32 @@ class PayBondInvoiceScreen extends ConsumerWidget { isBondPhase ? orderState.paymentRequest?.order?.amount : null; // A maker creating an order pays the bond before it is published, so the // copy must warn them to keep the screen open or the order won't be created. - final isMakerBond = ref - .read(sessionNotifierProvider.notifier) - .getSessionByOrderId(orderId) - ?.bondPending ?? - false; + final session = + ref.read(sessionNotifierProvider.notifier).getSessionByOrderId(orderId); + final isMakerBond = session?.bondPending ?? false; final explanation = isMakerBond ? s.bondExplanationMaker : s.bondExplanation; if (lnInvoice.isEmpty) { // No invoice to show — and the reason decides what to tell the user. - // Saying "expired, take the order again" in all three cases is what - // could make a maker abandon a bond that was merely still loading. + // Saying "expired, take the order again" in every case is what could + // make a maker abandon a bond that was merely still loading. final cycleEnded = OrderState.endsTradeCycle(orderState.status) && orderState.status != enums.Status.pending; - // `pending` is not enough on its own: it is the notifier's initial state - // while a maker bond is still loading, *and* the state of a maker's - // order that is paid and live in the book. The pay-bond notification - // stays in the history and pushes this screen, so a maker who taps it - // after paying used to get a spinner that could never resolve (#732 - // review). `bondPending` is the marker that separates the two. + // `pending` is not enough on its own. It is the notifier's unhydrated + // initial state, the state of a maker awaiting their bond invoice, the + // state of a maker's order that is paid and live in the book, and the + // state a taker finds after their take timed out and Mostro republished + // the order. This screen is reachable from a notification card at any + // time, so any of them can show up here (#732 review). Three signals + // separate them: an absent order payload means nothing has hydrated + // yet, `bondPending` means a maker bond is actually awaited, and a + // session for this order means the order is the user's own — Mostro + // deletes the taker's session when it republishes. + final notHydrated = orderState.order == null; final stillLoading = !cycleEnded && (orderState.status == enums.Status.waitingTakerBond || + notHydrated || (orderState.status == enums.Status.pending && isMakerBond)); if (stillLoading) { @@ -185,14 +189,33 @@ class PayBondInvoiceScreen extends ConsumerWidget { ); } - // Past the bond phase: the bond is paid. For a taker that means the - // trade moved on; for the maker of a pending order it means the order - // is in the book, with no trade to go to yet. - final orderIsPublished = orderState.status == enums.Status.pending; + // Hydrated and still pending, with no bond awaited: either the maker's + // own order, published and paid, or a taker whose take ended and whose + // session Mostro deleted when it put the order back in the book. + if (orderState.status == enums.Status.pending) { + return session != null + ? _EmptyBondState( + title: s.bondScreenTitle, + icon: Icons.check_circle_outline, + message: s.bondOrderPublished, + actionLabel: s.goToTrade, + onAction: (context) => context.go('/trade_detail/$orderId'), + ) + : _EmptyBondState( + title: s.bondScreenTitle, + icon: Icons.hourglass_disabled, + message: s.bondInvoiceUnavailable, + actionLabel: s.close, + onAction: (context) => + context.canPop() ? context.pop() : context.go('/'), + ); + } + + // Past the bond phase: the bond is paid and the trade moved on. return _EmptyBondState( title: s.bondScreenTitle, icon: Icons.check_circle_outline, - message: orderIsPublished ? s.bondOrderPublished : s.bondAlreadyPaid, + message: s.bondAlreadyPaid, actionLabel: s.goToTrade, onAction: (context) => context.go('/trade_detail/$orderId'), ); diff --git a/test/features/order/screens/pay_bond_invoice_screen_test.dart b/test/features/order/screens/pay_bond_invoice_screen_test.dart index dbf5bd2fe..c20cfa99b 100644 --- a/test/features/order/screens/pay_bond_invoice_screen_test.dart +++ b/test/features/order/screens/pay_bond_invoice_screen_test.dart @@ -115,11 +115,15 @@ void main() { required enums.Status status, required enums.Action action, String? invoice, + bool hydrated = true, }) => OrderState( status: status, action: action, - order: order(status), + // The notifier's initial state carries no payload until a sync() has + // read the history, which is how the screen tells "nothing has loaded + // yet" from a real pending order. + order: hydrated ? order(status) : null, paymentRequest: invoice == null ? null : PaymentRequest(order: order(status), lnInvoice: invoice), @@ -220,8 +224,11 @@ void main() { ); await tester.pump(const Duration(seconds: 5)); + final context = tester.element(find.byType(PayBondInvoiceScreen)); + final s = S.of(context)!; expect(find.byType(CircularProgressIndicator), findsNothing); expect(find.byType(QrImageView), findsNothing); + expect(find.text(s.bondOrderPublished), findsOneWidget); expect(find.byType(ElevatedButton), findsOneWidget); }); @@ -244,6 +251,43 @@ void main() { expect(find.byType(PayBondInvoiceScreen), findsNothing); }); + // A taker whose take timed out finds the order back in the book, with their + // bond returned and no session: telling them "your order is published" would + // be a statement about an order they do not own (#732 review). + testWidgets('tells a taker to retake a republished order', (tester) async { + await pumpScreen( + tester, + stateWith( + status: enums.Status.pending, + action: enums.Action.newOrder, + ), + // No session: Mostro deletes the taker's when it republishes. + ); + await tester.pump(const Duration(seconds: 5)); + + final context = tester.element(find.byType(PayBondInvoiceScreen)); + final s = S.of(context)!; + expect(find.text(s.bondInvoiceUnavailable), findsOneWidget); + expect(find.text(s.bondOrderPublished), findsNothing); + expect(find.byType(CircularProgressIndicator), findsNothing); + }); + + // Opened from a notification before this order's notifier has synced: the + // state is the unhydrated initial one, so waiting is the honest answer. + testWidgets('waits while the order has not hydrated yet', (tester) async { + await pumpScreen( + tester, + stateWith( + status: enums.Status.pending, + action: enums.Action.newOrder, + hydrated: false, + ), + ); + + expect(find.byType(CircularProgressIndicator), findsOneWidget); + expect(find.byType(ElevatedButton), findsNothing); + }); + // A maker cannot take their own order, so "take the order again" is wrong // for the maker of an expired one (#732 review). testWidgets('tells a maker their own order is over, not to retake it', From 94646b6540dce518f33e7830e898630606a388a9 Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sat, 3 Oct 2026 02:30:30 +0200 Subject: [PATCH 11/12] docs(restore): restore each helper's own doc and annotation restoreCreatedAtMillis was inserted between decodeRestoreMessage's doc comment and @visibleForTesting and its declaration, so it ended up with two of each while decodeRestoreMessage lost both. No runtime effect, but the test-only lint stopped applying to decodeRestoreMessage. --- lib/features/restore/restore_manager.dart | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 3ee29fdad..b03577370 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -1147,14 +1147,6 @@ class RestoreService { } } -/// Decodes a restore response into its message map, handling both transports: -/// v2 (kind 14, NIP-44 direct, decrypted straight to the tuple) and v1 -/// (kind 1059, gift wrap unwrapped to a rumor whose content is the tuple). Both -/// converge on `tuple[0]`. -/// -/// Top-level (not a private method) so the transport branch can be -/// regression-tested without the full [RestoreService] / Riverpod orchestration. -@visibleForTesting /// Converts mostrod's `created_at` to the millisecond scale this app stores. /// /// The daemon reports it in seconds (`Timestamp::now().as_secs()` in @@ -1169,6 +1161,14 @@ int restoreCreatedAtMillis(int? createdAtSeconds) => ? createdAtSeconds * Duration.millisecondsPerSecond : DateTime.now().millisecondsSinceEpoch; +/// Decodes a restore response into its message map, handling both transports: +/// v2 (kind 14, NIP-44 direct, decrypted straight to the tuple) and v1 +/// (kind 1059, gift wrap unwrapped to a rumor whose content is the tuple). Both +/// converge on `tuple[0]`. +/// +/// Top-level (not a private method) so the transport branch can be +/// regression-tested without the full [RestoreService] / Riverpod orchestration. +@visibleForTesting Future> decodeRestoreMessage( NostrEvent event, NostrKeyPairs tempTradeKey, From 3c7d9f010299fd6d6a9c55e2dd6b649dd79048d6 Mon Sep 17 00:00:00 2001 From: 21Mill Date: Sat, 3 Oct 2026 02:35:56 +0200 Subject: [PATCH 12/12] test(order): cover the silent drop of a message with no event time `_eventTimeOf` falls back to 0, so a message carrying neither `eventCreatedAt` nor `timestamp` counts as older than any started cycle and both `applyToCycle` and `subscribe()` drop it without telling anyone. That behaviour had no test; this one documents it. --- .../order_notifier_retake_cycle_test.dart | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart index 1abc6f1ba..5ceae6cc7 100644 --- a/test/features/order/notifiers/order_notifier_retake_cycle_test.dart +++ b/test/features/order/notifiers/order_notifier_retake_cycle_test.dart @@ -367,6 +367,34 @@ void main() { }); }); + group('the cycle guard', () { + test('a message with no time at all counts as preceding the cycle', + () async { + final notifier = + container.read(orderNotifierProvider(orderId).notifier); + + notifier.state = notifier.applyToCycle( + container.read(orderNotifierProvider(orderId)), + invoice(Action.payBondInvoice, 'lnbcbond', + eventCreatedAt: 4000, timestamp: 4), + ); + + // _eventTimeOf falls back to 0, so a message carrying neither clock is + // treated as older than any started cycle and dropped. Documented here + // because the drop is silent. + expect( + notifier.precedesActiveCycle( + MostroMessage( + action: Action.canceled, + id: orderId, + payload: orderPayload(Status.canceled), + ), + ), + isTrue, + ); + }); + }); + // RestoreManager writes state through updateStateFromMessage, not sync(). If // that path skips the cycle bookkeeping, cycleEndedAt stays null, the // restart condition cannot fire and the next take's bond invoice is dropped