From 729d81e60b26a919123078a9e5df10ff1f3efab6 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Thu, 18 Jun 2026 00:32:40 -0300 Subject: [PATCH 1/2] fix(restore): detect and restore failed payout substate for settled-hold-invoice buyers --- lib/features/restore/restore_manager.dart | 103 +++++++++++++--- .../restore/restore_failed_payout_test.dart | 115 ++++++++++++++++++ 2 files changed, 203 insertions(+), 15 deletions(-) create mode 100644 test/features/restore/restore_failed_payout_test.dart diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 4e8d1230..eaf84777 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -838,23 +838,57 @@ class RestoreService { } } - // Create regular order message with Order payload - final mostroMessage = MostroMessage( - id: orderDetail.id, - action: action, - payload: order, - timestamp: - orderDetail.createdAt ?? - DateTime.now().millisecondsSinceEpoch, - ); + // `settled-hold-invoice` is overloaded for the buyer (see issue + // #615): it covers both "hold settled, sats in flight" and "payout + // failed, awaiting a new invoice". The protocol distinguishes them + // only via the action (`payment-failed` / `add-invoice`), so a + // status-based rebuild always lands on the "paying sats" screen. + // On restore the daemon re-sends `add-invoice` to the buyer's trade + // key when the payout failed (mostro#754). Detect that substate and + // replay `payment-failed` then `add-invoice` so the order lands on + // `payment-failed` + `add-invoice` (the new-invoice prompt) instead. + List actionsToApply = [action]; + final orderSession = ref + .read(sessionNotifierProvider.notifier) + .getSessionByOrderId(orderDetail.id); + if (order.status == Status.settledHoldInvoice && + orderSession?.role == Role.buyer) { + final messages = await storage.getAllMessagesForOrderId( + orderDetail.id, + ); + if (restoreHasFailedPayoutSignal(messages)) { + actionsToApply = [Action.paymentFailed, Action.addInvoice]; + logger.i( + 'Restore: detected failed payout for order ${orderDetail.id}, ' + 'restoring payment-failed substate instead of "paying sats"', + ); + } + } - // Save order message to storage - final key = - '${orderDetail.id}_restore_${action.value}_${DateTime.now().millisecondsSinceEpoch}'; - await storage.addMessage(key, mostroMessage); + // Create and apply the regular order message(s) with Order payload. + // When more than one action is replayed, stagger their timestamps so + // a later sync() replays them in the same order and converges to the + // same final state. + final baseTimestamp = + orderDetail.createdAt ?? DateTime.now().millisecondsSinceEpoch; + for (var i = 0; i < actionsToApply.length; i++) { + final replayAction = actionsToApply[i]; + final mostroMessage = MostroMessage( + id: orderDetail.id, + action: replayAction, + payload: order, + timestamp: baseTimestamp + i, + ); + + // Save order message to storage + final key = + '${orderDetail.id}_restore_${replayAction.value}_' + '${DateTime.now().millisecondsSinceEpoch}_$i'; + await storage.addMessage(key, mostroMessage); - // Update state with order message - notifier.updateStateFromMessage(mostroMessage); + // Update state with order message + notifier.updateStateFromMessage(mostroMessage); + } } } catch (e, stack) { logger.e( @@ -1170,6 +1204,45 @@ Future> decodeRestoreMessage( return contentList[0] as Map; } +/// Detects the "payout failed, awaiting a new invoice" substate of an order +/// that Mostro reports as `settled-hold-invoice`. See issue #615. +/// +/// `settled-hold-invoice` is overloaded for the buyer: it covers both "hold +/// settled, sats in flight" and "payout failed". The protocol distinguishes +/// them only via the action, so the snapshot status alone cannot recover the +/// failed substate. On restore the daemon re-sends `add-invoice` (and may also +/// re-send `payment-failed`) to the buyer's trade key when the payout failed +/// (mostro#754). +/// +/// `payment-failed` only ever occurs after a failed payout, so it is a +/// definitive signal on its own. `add-invoice` also appears early in the happy +/// flow (waiting-buyer-invoice), so it is only treated as a failed-payout signal +/// when it arrives after the hold was released/settled. Restore clears storage +/// before re-subscribing, so in practice only freshly re-sent messages are +/// present, but the ordering check keeps this correct even if the daemon +/// re-sends the full message history. +bool restoreHasFailedPayoutSignal(List messages) { + final sorted = [...messages] + ..sort((a, b) => (a.timestamp ?? 0).compareTo(b.timestamp ?? 0)); + + int releaseIndex = -1; + for (var i = 0; i < sorted.length; i++) { + final action = sorted[i].action; + if (action == Action.release || + action == Action.released || + action == Action.holdInvoicePaymentSettled) { + releaseIndex = i; + } + } + + for (var i = 0; i < sorted.length; i++) { + final action = sorted[i].action; + if (action == Action.paymentFailed) return true; + if (action == Action.addInvoice && i > releaseIndex) return true; + } + return false; +} + /// Thrown when Mostro responds with cant-do: invalid_trade_index to /// Action.lastTradeIndex during the restore flow. class RestoreInvalidTradeIndexException implements Exception { diff --git a/test/features/restore/restore_failed_payout_test.dart b/test/features/restore/restore_failed_payout_test.dart new file mode 100644 index 00000000..d562f6f3 --- /dev/null +++ b/test/features/restore/restore_failed_payout_test.dart @@ -0,0 +1,115 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models.dart'; +import 'package:mostro_mobile/data/enums.dart'; +import 'package:mostro_mobile/features/order/models/order_state.dart'; +import 'package:mostro_mobile/features/restore/restore_manager.dart'; + +/// Regression coverage for issue #615: restoring an order that Mostro reports +/// as `settled-hold-invoice` after a failed Lightning payout must land on the +/// new-invoice prompt (`add-invoice` + `payment-failed`), not "paying sats" +/// (`released` + `settled-hold-invoice`). + +MostroMessage _msg(Action action, {int? timestamp}) => + MostroMessage( + action: action, + id: 'order-1', + timestamp: timestamp, + payload: const Order( + id: 'order-1', + kind: OrderType.buy, + status: Status.settledHoldInvoice, + amount: 500, + fiatCode: 'USD', + fiatAmount: 50, + paymentMethod: 'Cash', + ), + ); + +/// Replays a sequence of actions onto a fresh order state, mirroring how +/// RestoreService.restore() applies snapshot-derived messages via +/// OrderNotifier.updateStateFromMessage(). +OrderState _replay(List actions) { + OrderState state = OrderState( + action: Action.newOrder, + status: Status.pending, + order: null, + ); + for (final action in actions) { + state = state.updateWith(_msg(action)); + } + return state; +} + +void main() { + group('restoreHasFailedPayoutSignal', () { + test('returns false for empty history', () { + expect(restoreHasFailedPayoutSignal([]), isFalse); + }); + + test('returns true when payment-failed is present', () { + expect( + restoreHasFailedPayoutSignal([_msg(Action.paymentFailed, timestamp: 1)]), + isTrue, + ); + }); + + test('returns true for a re-sent add-invoice (storage cleared on restore)', + () { + expect( + restoreHasFailedPayoutSignal([_msg(Action.addInvoice, timestamp: 1)]), + isTrue, + ); + }); + + test('returns true when add-invoice arrives after the hold was released', + () { + expect( + restoreHasFailedPayoutSignal([ + _msg(Action.released, timestamp: 1), + _msg(Action.addInvoice, timestamp: 2), + ]), + isTrue, + ); + }); + + test('returns false for an early add-invoice that precedes the release', () { + // Happy path where the daemon re-sends full history: the only add-invoice + // is the early waiting-buyer-invoice one, before the release. + expect( + restoreHasFailedPayoutSignal([ + _msg(Action.addInvoice, timestamp: 1), + _msg(Action.released, timestamp: 2), + ]), + isFalse, + ); + }); + + test('returns false for a settled order with no failed-payout signal', () { + expect( + restoreHasFailedPayoutSignal([ + _msg(Action.holdInvoicePaymentSettled, timestamp: 1), + ]), + isFalse, + ); + }); + }); + + group('restore state rebuild for settled-hold-invoice + buyer', () { + test( + 'failed payout replays to payment-failed + add-invoice (new-invoice prompt)', + () { + final state = _replay([Action.paymentFailed, Action.addInvoice]); + + expect(state.status, equals(Status.paymentFailed)); + expect(state.action, equals(Action.addInvoice)); + }); + + test('happy path replays to settled-hold-invoice + released (paying sats)', + () { + final state = _replay([Action.released]); + + expect(state.status, equals(Status.settledHoldInvoice)); + expect(state.action, equals(Action.released)); + }); + }); +} From 9ce5e31308c541fac3542157896648243e1c2797 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 15 Jul 2026 22:59:19 -0300 Subject: [PATCH 2/2] fix(restore): seed failed-payout replay timestamps from latest stored message to prevent state regression - Fold over existing messages to find max timestamp instead of using order createdAt - Stamp replayed payment-failed/add-invoice pair after all stored messages - Prevents older re-delivered released/settled from replaying after the fix and dragging buyer back to "paying sats" - Update doc comments to clarify mostro#754 invariant (daemon re-sends only add-invoice for failed payouts) - Explain --- lib/features/restore/restore_manager.dart | 50 +++++++++++++------ .../restore/restore_failed_payout_test.dart | 8 ++- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index eaf84777..4895eb6a 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -847,7 +847,11 @@ class RestoreService { // key when the payout failed (mostro#754). Detect that substate and // replay `payment-failed` then `add-invoice` so the order lands on // `payment-failed` + `add-invoice` (the new-invoice prompt) instead. + // Seed replay timestamps from the order creation time so the + // reconstructed happy-path message keeps its historical ordering. List actionsToApply = [action]; + int baseTimestamp = + orderDetail.createdAt ?? DateTime.now().millisecondsSinceEpoch; final orderSession = ref .read(sessionNotifierProvider.notifier) .getSessionByOrderId(orderDetail.id); @@ -858,6 +862,18 @@ class RestoreService { ); if (restoreHasFailedPayoutSignal(messages)) { actionsToApply = [Action.paymentFailed, Action.addInvoice]; + // Stamp the replayed pair after every message already in + // storage so a later OrderNotifier.sync() (which replays storage + // sorted by timestamp) applies them last and converges to + // payment-failed. Seeding from createdAt instead would let an + // older re-delivered `released`/`settled` replay after the pair + // and drag the buyer back to "paying sats", making the fix + // non-persistent across provider recreation or app restart. + baseTimestamp = messages.fold( + baseTimestamp, + (max, m) => (m.timestamp ?? 0) > max ? m.timestamp! : max, + ) + + 1; logger.i( 'Restore: detected failed payout for order ${orderDetail.id}, ' 'restoring payment-failed substate instead of "paying sats"', @@ -869,8 +885,6 @@ class RestoreService { // When more than one action is replayed, stagger their timestamps so // a later sync() replays them in the same order and converges to the // same final state. - final baseTimestamp = - orderDetail.createdAt ?? DateTime.now().millisecondsSinceEpoch; for (var i = 0; i < actionsToApply.length; i++) { final replayAction = actionsToApply[i]; final mostroMessage = MostroMessage( @@ -1208,19 +1222,27 @@ Future> decodeRestoreMessage( /// that Mostro reports as `settled-hold-invoice`. See issue #615. /// /// `settled-hold-invoice` is overloaded for the buyer: it covers both "hold -/// settled, sats in flight" and "payout failed". The protocol distinguishes -/// them only via the action, so the snapshot status alone cannot recover the -/// failed substate. On restore the daemon re-sends `add-invoice` (and may also -/// re-send `payment-failed`) to the buyer's trade key when the payout failed -/// (mostro#754). +/// settled, sats in flight" and "payout failed, awaiting a new invoice". The +/// protocol distinguishes them only via the action, and `payment-failed` is a +/// client-synthesized status that never travels on the wire, so the snapshot +/// status alone cannot recover the failed substate. +/// +/// Invariant this relies on (mostro#754): on `restore-session` the daemon +/// re-enqueues ONLY `add-invoice`, and only for orders flagged +/// `failed_payment = true` in `settled-hold-invoice`. It does NOT re-send +/// `payment-failed`, `released` or `settled`. Restore also clears local storage +/// before re-subscribing. A bare `add-invoice` sitting in freshly cleared +/// storage is therefore the daemon's targeted failed-payout prompt, which is +/// why a lone `add-invoice` counts as a signal here. Requiring a preceding +/// `payment-failed`/release for the single-message case would miss this re-send +/// and reintroduce the money-at-risk regression of #615. /// -/// `payment-failed` only ever occurs after a failed payout, so it is a -/// definitive signal on its own. `add-invoice` also appears early in the happy -/// flow (waiting-buyer-invoice), so it is only treated as a failed-payout signal -/// when it arrives after the hold was released/settled. Restore clears storage -/// before re-subscribing, so in practice only freshly re-sent messages are -/// present, but the ordering check keeps this correct even if the daemon -/// re-sends the full message history. +/// The ordering check keeps this correct if a relay additionally re-delivers +/// the full historical stream: `add-invoice` also appears early in the happy +/// flow (waiting-buyer-invoice), so it is trusted as a failed-payout signal +/// only when no release/settle precedes it (the daemon's fresh re-send) or when +/// it arrives after one. `payment-failed`, if ever present, is definitive on +/// its own. bool restoreHasFailedPayoutSignal(List messages) { final sorted = [...messages] ..sort((a, b) => (a.timestamp ?? 0).compareTo(b.timestamp ?? 0)); diff --git a/test/features/restore/restore_failed_payout_test.dart b/test/features/restore/restore_failed_payout_test.dart index d562f6f3..f4f2149f 100644 --- a/test/features/restore/restore_failed_payout_test.dart +++ b/test/features/restore/restore_failed_payout_test.dart @@ -53,8 +53,12 @@ void main() { ); }); - test('returns true for a re-sent add-invoice (storage cleared on restore)', - () { + // Per mostro#754 the daemon re-sends ONLY a bare `add-invoice` (no + // `payment-failed`, no `released`) for a failed payout, and restore clears + // storage first, so a lone post-clear `add-invoice` is the failed-payout + // prompt and must be honored. Weakening this to require `payment-failed` or + // a preceding release would reintroduce the money-at-risk bug in #615. + test('lone re-sent add-invoice is a failed-payout signal (mostro#754)', () { expect( restoreHasFailedPayoutSignal([_msg(Action.addInvoice, timestamp: 1)]), isTrue,