From 948cf4d7156b1b11a0b8a98ce289e83d48e9b3c1 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Wed, 8 Jul 2026 18:49:26 -0600 Subject: [PATCH 01/13] fix: buffer live events during restore to prevent state corruption During the restore window, incoming events were dropped after being marked as seen for dedup, permanently losing genuinely live state changes (e.g. a counterparty payment arriving mid-restore). Buffer them instead and replay once restore ends, on both the success and error paths, clearing the dedup entry first so the replay isn't self-blocked. Also moves the restore-buffer check ahead of the session-match check in _onData, closing a gap where an event for a session not yet recreated mid-restore was still dropped instead of buffered. Synthetic restore snapshot messages now anchor their ordering timestamp to a single restore-start time instead of the daemon's per-order created_at (which reflects order creation, not current status, and can be null per protocol) or per-message "now", so live events during the window sort correctly relative to the snapshot. Closes #584. Supersedes the stale #589, whose reviewed buffer-and- replay design (credit: Catrya) this reincorporates against current main, extended to cover both v1 (gift wrap) and v2 (NIP-44) transport. --- .../SESSION_RECOVERY_ARCHITECTURE.md | 112 ++++++++++++++++-- integration_test/test_helpers.dart | 11 +- lib/features/restore/restore_manager.dart | 31 +++-- lib/services/mostro_service.dart | 67 +++++++++++ 4 files changed, 199 insertions(+), 22 deletions(-) diff --git a/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md b/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md index 4895c91f..25fde3e0 100644 --- a/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md +++ b/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md @@ -353,12 +353,14 @@ for (final orderDetail in ordersResponse.orders) { // ... other fields ); - // Build synthetic MostroMessage + // Build synthetic MostroMessage. The ordering timestamp is the single + // restore-start anchor (_restoreStartTime), NOT orderDetail.createdAt — + // see "Restore-Time Live Event Buffering and Replay" below for why. final mostroMessage = MostroMessage( id: orderDetail.id, action: action, payload: order, - timestamp: orderDetail.createdAt ?? DateTime.now().millisecondsSinceEpoch, + timestamp: _restoreStartTime, ); // Save message to storage and update state @@ -511,37 +513,121 @@ Action _getActionFromStatus(Status status, Role? userRole) { ### Restore Mode Protection -**File**: `lib/features/restore/restore_manager.dart:466-468` +**File**: `lib/features/restore/restore_manager.dart` -During recovery, a global flag prevents processing of old messages: +During recovery, a global flag (`isRestoringProvider`) marks the window in +which historical order/dispute state is being rebuilt from Mostro's restore +response: ```dart -// Enable restore mode to block all old message processing +// Enable restore mode to block synthetic/live processing races ref.read(isRestoringProvider.notifier).state = true; -_logger.i('Restore: enabled restore mode - blocking all old message processing'); +logger.i('Restore: enabled restore mode - blocking all old message processing'); ``` -**File**: `lib/services/mostro_service.dart:44-96` +`isRestoringProvider` is cleared on **both** the success path and the catch +block of `restore()`, so the transition back to `false` is outcome-agnostic +— see D3 below. + +**File**: `lib/services/mostro_service.dart` ```dart bool _isRestorePayload(Map json) { // Check if this is a restore-specific payload that should be ignored // during normal operation - + final wrapper = json['restore'] ?? json['order']; if (wrapper == null || wrapper is! Map) return false; - + final payload = wrapper['payload']; if (payload == null || payload is! Map) return false; - + // Check for restore-specific fields if (payload.containsKey('restore_data')) return true; if (payload.containsKey('trade_index')) return true; - + return false; } ``` +`_isRestorePayload` only recognizes the restore protocol's OWN response +payloads (restore data, orders list, trade index) arriving on the temporary +subscription — it has nothing to do with regular live order/dispute events +addressed to the user's real sessions. Those are handled by the +buffer-and-replay mechanism below. + +### Restore-Time Live Event Buffering and Replay + +**File**: `lib/services/mostro_service.dart` + +Fixes GitHub #584: previously, any live event (order update, dispute +message) that arrived on the user's real sessions while a restore was in +progress was either dropped outright or — in an earlier, buggy port of this +fix — buffered too late (after the session-match check), so events for a +session restore had not recreated yet were still lost. Live events are now +buffered unconditionally while `isRestoringProvider` is true and replayed, +in arrival order, once restore ends. + +**Architecture decisions**: + +- **D1 — Buffer check precedes session-match**: `_onData` checks + `isRestoringProvider` immediately after the dedup reserve, **before** the + `matchingSession == null` early return and before any decrypt. Restore + recreates sessions incrementally (one `saveSession()` call per order), so + an event addressed to a session that has not been recreated yet must still + be preserved, not dropped as "no matching session". +- **D2 — Keyed buffer**: `_restoreBuffer` is a `Map` + keyed by `event.id`, which gives dedup-by-id (re-buffering the same id + keeps a single entry in its original arrival position — Dart `Map` is + insertion-ordered) as a defensive backstop behind the top-level + `eventStore.hasItem`/`putItem` dedup check. +- **D3 — Outcome-agnostic flush trigger**: `MostroService.init()` registers + `ref.listen(isRestoringProvider, ...)` and flushes the buffer on any + `true → false` transition. This covers `RestoreService.restore()`'s + success path and its catch block identically — the flush does not need to + know why restore ended. +- **D4 — Dedup entry cleared before replay**: `_flushRestoreBuffer()` calls + `eventStore.deleteItem(event.id)` immediately before replaying each event + through `_onData`, because `_onData` reserved that id when the event was + first buffered. Without this, the dedup check at the top of `_onData` + would silently drop the replay. +- **D5 — Single restore-start anchor for synthetic messages**: synthetic + order/dispute messages built in `RestoreService.restore()` use one + timestamp, `_restoreStartTime` — captured once at the start of + `initRestoreProcess()`, before restore mode is enabled — instead of + `orderDetail.createdAt`. `createdAt` reflects when the order was + originally created, not when this snapshot was taken; for a long-lived + order, using it as the ordering timestamp would let intervening historical + replay outrank the current-state snapshot. The restore-start anchor sorts + newer than all pre-restore history yet older than any live event that + arrives during restore. The real creation time is still preserved in the + `Order`/`Dispute` payload for display; only the ordering timestamp + changes. + +**Target `_onData` control flow** (landmark-relative — decrypt, session-match, +DM/restore-payload skips, and the timestamp fallback already existed; only +the buffer check's *position*, relative to session-match, is new): + +``` +_onData(event): + 1. if eventStore.hasItem(id): return // dedup check + 2. eventStore.putItem(id, ...) // dedup reserve + 3. if isRestoringProvider: buffer[id] = event; return // <== reorder fix + 4. matchingSession = ...; if null: return // now AFTER buffer + 5. decrypt (v1 gift-wrap unWrap / v2 NIP-44 direct) + 6. jsonDecode; skip DM payloads; skip restore payloads + 7. msg = MostroMessage.fromJson(...) + 8. msg.timestamp ??= innerRumorCreatedAt ?? event.createdAt + 9. messageStorage.addMessage(...); link child order if applicable +``` + +On flush, `isRestoringProvider` is `false`, so replayed events flow past +step 3 straight to step 4 onward and receive their real protocol timestamp: +the inner rumor's `created_at` for v1 gift wrap (canonical per NIP-59 — the +outer wrap/seal timestamps are randomized for privacy), or the event's own +`created_at` for v2 NIP-44 direct (kind 14 has no seal/rumor layer, so its +own timestamp is already the real send time). + ### Session Validation The system validates that recreated sessions match the expected order data: @@ -718,8 +804,8 @@ sequenceDiagram --- -**Last Modified**: November 25, 2025 -**Version**: 1.0.0 +**Last Modified**: July 8, 2026 +**Version**: 1.1.0 **Author**: Architecture Documentation **Related Files**: - `lib/features/restore/restore_manager.dart` diff --git a/integration_test/test_helpers.dart b/integration_test/test_helpers.dart index ddabe916..773c30c6 100644 --- a/integration_test/test_helpers.dart +++ b/integration_test/test_helpers.dart @@ -315,11 +315,20 @@ class FakeMostroService implements MostroService { @override void updateSettings(Settings settings) {} - + @override void dispose() { // TODO: implement dispose } + + @override + Future onDataForTesting(NostrEvent event) async {} + + @override + Future flushRestoreBufferForTesting() async {} + + @override + Map get restoreBufferForTesting => {}; } Future pumpTestApp(WidgetTester tester) async { diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 4e8d1230..bc4d7e4b 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -62,6 +62,14 @@ class RestoreService { bool _operationInProgress = false; Completer? _operationCompleter; + // Single anchor timestamp for every synthetic message built by this restore + // run, captured once at the start of initRestoreProcess() (before restore + // mode is enabled). Ensures synthetic snapshots sort newer than pre-restore + // history yet older than any live event arriving during restore, regardless + // of the order's own (possibly stale) createdAt. Defaults to construction + // time as a safe fallback in case restore() is ever invoked directly. + int _restoreStartTime = DateTime.now().millisecondsSinceEpoch; + RestoreService(this.ref); Future importMnemonicAndRestore(String mnemonic) async { @@ -797,18 +805,20 @@ class RestoreService { } // Create dispute message with Dispute payload (per Mostro protocol) + // Timestamp is the restore-start anchor (not orderDetail.createdAt, + // which reflects the order's original creation time, not this + // snapshot's currency) so it outranks pre-restore history while + // still sorting behind any live event arriving during restore. final disputeMessage = MostroMessage( id: orderDetail.id, action: action, payload: dispute, - timestamp: - orderDetail.createdAt ?? - DateTime.now().millisecondsSinceEpoch, + timestamp: _restoreStartTime, ); // Save dispute message to storage final disputeKey = - '${orderDetail.id}_restore_${action.value}_${DateTime.now().millisecondsSinceEpoch}'; + '${orderDetail.id}_restore_${action.value}_$_restoreStartTime'; await storage.addMessage(disputeKey, disputeMessage); // Update state with dispute message @@ -839,18 +849,20 @@ class RestoreService { } // Create regular order message with Order payload + // Timestamp is the restore-start anchor (not orderDetail.createdAt, + // which reflects the order's original creation time, not this + // snapshot's currency) so it outranks pre-restore history while + // still sorting behind any live event arriving during restore. final mostroMessage = MostroMessage( id: orderDetail.id, action: action, payload: order, - timestamp: - orderDetail.createdAt ?? - DateTime.now().millisecondsSinceEpoch, + timestamp: _restoreStartTime, ); // Save order message to storage final key = - '${orderDetail.id}_restore_${action.value}_${DateTime.now().millisecondsSinceEpoch}'; + '${orderDetail.id}_restore_${action.value}_$_restoreStartTime'; await storage.addMessage(key, mostroMessage); // Update state with order message @@ -898,6 +910,9 @@ class RestoreService { _operationInProgress = true; _operationCompleter = Completer(); + // Snapshot the restore-start anchor before anything else, so every + // synthetic message this run produces shares one ordering timestamp. + _restoreStartTime = DateTime.now().millisecondsSinceEpoch; // Hold the shared session lock for the whole restore so order/take flows // cannot interleave with the session reset (and rebuild) below. final releaseSessionLock = diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index 7be83dbc..222db6d5 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -2,11 +2,13 @@ import 'dart:async'; import 'dart:convert'; import 'package:collection/collection.dart'; import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:mostro_mobile/services/logger_service.dart'; import 'package:mostro_mobile/data/enums.dart'; import 'package:mostro_mobile/data/models.dart'; +import 'package:mostro_mobile/features/restore/restore_mode_provider.dart'; import 'package:mostro_mobile/features/settings/settings.dart'; import 'package:mostro_mobile/features/subscriptions/subscription_manager_provider.dart'; import 'package:mostro_mobile/shared/providers.dart'; @@ -21,12 +23,19 @@ class MostroService { Settings _settings; StreamSubscription? _ordersSubscription; + ProviderSubscription? _restoreListener; + + // Live events received while a restore is in progress are buffered here + // (keyed by event id, so re-delivery of the same id keeps a single entry + // in its original arrival position) and replayed once restore ends. + final Map _restoreBuffer = {}; MostroService(this.ref) : _settings = ref.read(settingsProvider); void init() { // Cancel any existing subscription to prevent leaks on re-init _ordersSubscription?.cancel(); + _restoreListener?.close(); // Subscribe to the orders stream from SubscriptionManager // The SubscriptionManager will automatically manage subscriptions based on SessionNotifier changes @@ -44,10 +53,20 @@ class MostroService { }, cancelOnError: false, ); + + // Flush buffered live events once restore ends, regardless of outcome + // (RestoreService.restore() clears isRestoringProvider on both its + // success path and its catch block). + _restoreListener = ref.listen(isRestoringProvider, (previous, next) { + if (previous == true && next == false) { + unawaited(_flushRestoreBuffer()); + } + }); } void dispose() { _ordersSubscription?.cancel(); + _restoreListener?.close(); logger.i('MostroService disposed'); } @@ -116,6 +135,17 @@ class MostroService { 'created_at': event.createdAt!.millisecondsSinceEpoch ~/ 1000, }); + // Buffer live events while a restore is in progress. This runs BEFORE the + // session-match check below so events addressed to a session restore has + // not recreated yet are preserved instead of being dropped by "no + // matching session". Buffered events are replayed through _onData once + // restore completes (success or error path alike); see _flushRestoreBuffer. + if (ref.read(isRestoringProvider)) { + _restoreBuffer[event.id!] = event; + logger.i('Restore: buffered live event ${event.id}'); + return; + } + final sessions = ref.read(sessionNotifierProvider); final matchingSession = sessions.firstWhereOrNull( (s) => s.tradeKey.public == event.recipient, @@ -132,6 +162,9 @@ class MostroService { // decrypts straight to the tuple. Both converge on jsonDecode below. String? content; String? decryptedId; + // Inner rumor's created_at is the real send time (outer gift wrap is + // NIP-59 randomized for privacy); used for timestamp anchoring below. + DateTime? innerCreatedAt; if (event.kind == 14) { content = await NostrUtils.decryptNIP44DirectEvent( event, @@ -142,6 +175,7 @@ class MostroService { final decryptedEvent = await event.unWrap(privateKey); content = decryptedEvent.content; decryptedId = decryptedEvent.id; + innerCreatedAt = decryptedEvent.createdAt; } if (content == null) return; @@ -169,6 +203,14 @@ class MostroService { final msg = MostroMessage.fromJson(result[0]); + // For v1 gift-wrap (kind 1059) use the inner rumor's created_at (real + // send time; the outer wrap is NIP-59 randomized). For v2 NIP-44 direct + // (kind 14) innerCreatedAt is null (no rumor layer), so fall back to + // event.createdAt, which is already the real send time. + msg.timestamp ??= + innerCreatedAt?.millisecondsSinceEpoch ?? + event.createdAt?.millisecondsSinceEpoch; + final messageStorage = ref.read(mostroStorageProvider); // Use the inner rumor id if available (v1), otherwise fall back to the @@ -188,6 +230,31 @@ class MostroService { } } + /// Replays every event buffered during restore through [_onData], in + /// arrival order, once restore has ended. Clears each event's dedup entry + /// immediately before replaying it, since [_onData] reserved that entry + /// when the event was first buffered. + Future _flushRestoreBuffer() async { + if (_restoreBuffer.isEmpty) return; + final events = List.from(_restoreBuffer.values); + _restoreBuffer.clear(); + logger.i('Restore: flushing ${events.length} buffered live events'); + final eventStore = ref.read(eventStorageProvider); + for (final event in events) { + await eventStore.deleteItem(event.id!); + await _onData(event); + } + } + + @visibleForTesting + Future onDataForTesting(NostrEvent event) => _onData(event); + + @visibleForTesting + Future flushRestoreBufferForTesting() => _flushRestoreBuffer(); + + @visibleForTesting + Map get restoreBufferForTesting => _restoreBuffer; + Future _maybeLinkChildOrder( MostroMessage message, Session session, From 78a3f0053be5324a61e0f60ed4f31a3a72314f64 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Wed, 8 Jul 2026 19:35:56 -0600 Subject: [PATCH 02/13] Add lan --- docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md b/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md index 25fde3e0..c3653888 100644 --- a/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md +++ b/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md @@ -608,7 +608,7 @@ in arrival order, once restore ends. DM/restore-payload skips, and the timestamp fallback already existed; only the buffer check's *position*, relative to session-match, is new): -``` +```dart _onData(event): 1. if eventStore.hasItem(id): return // dedup check 2. eventStore.putItem(id, ...) // dedup reserve From 023dbe71edcc028729458045022388dfc95f4f57 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Wed, 8 Jul 2026 22:34:00 -0600 Subject: [PATCH 03/13] fix: isolate per-event failures in restore buffer replay _flushRestoreBuffer() cleared the buffer before processing and looped over events with no per-event error handling, so a single failure (e.g. deleteItem) would abort the whole replay, silently dropping any remaining buffered events for that restore. Wrap each event's deleteItem + _onData call in its own try/catch so one failure is logged and skipped without stopping the rest of the batch. --- lib/services/mostro_service.dart | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index 222db6d5..694d8e6d 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -241,8 +241,12 @@ class MostroService { logger.i('Restore: flushing ${events.length} buffered live events'); final eventStore = ref.read(eventStorageProvider); for (final event in events) { - await eventStore.deleteItem(event.id!); - await _onData(event); + try { + await eventStore.deleteItem(event.id!); + await _onData(event); + } catch (e) { + logger.e('Restore: failed to replay buffered event ${event.id}', error: e); + } } } From 6d2f390b49d4d3d826bb0bf30b4c29462d0b23f2 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 01:52:40 -0600 Subject: [PATCH 04/13] fix(restore): defer fiat-sent classification until buffer drains Codex flagged a race on PR #639: RestoreService checked local message history for fiatSent/fiatSentOk before the restore buffer had a chance to flush a same-order backfilled event, permanently misclassifying cooperatively-canceled/disputed orders as no-fiat-sent. A seller in that state loses the Release action even though the buyer already paid. Fix: track affected orders during the restore loop instead of guessing inline, then reconcile once in a finally block after MostroService's buffer is provably drained via a new public flushRestoreBuffer(). --- integration_test/test_helpers.dart | 3 + lib/features/restore/restore_manager.dart | 104 ++++++++++++++-------- lib/services/mostro_service.dart | 12 ++- 3 files changed, 82 insertions(+), 37 deletions(-) diff --git a/integration_test/test_helpers.dart b/integration_test/test_helpers.dart index 773c30c6..3b92d6c7 100644 --- a/integration_test/test_helpers.dart +++ b/integration_test/test_helpers.dart @@ -324,6 +324,9 @@ class FakeMostroService implements MostroService { @override Future onDataForTesting(NostrEvent event) async {} + @override + Future flushRestoreBuffer() async {} + @override Future flushRestoreBufferForTesting() async {} diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index bc4d7e4b..03fbb633 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -601,6 +601,10 @@ class RestoreService { OrdersResponse ordersResponse, List disputes, ) async { + // Orders needing a fiat-sent recheck once the restore buffer drains. + final pendingReconciliation = + <({String orderId, MostroMessage message})>[]; + try { if (_masterKey == null) { throw Exception('Master key not initialized'); @@ -787,22 +791,7 @@ class RestoreService { ); if (dispute != null) { - // For disputed orders, check if fiat was sent before the dispute - // so future cooperative cancel actions get remapped correctly - final disputeMessages = await storage.getAllMessagesForOrderId( - orderDetail.id, - ); - final hadFiatSent = disputeMessages.any( - (m) => - m.action == Action.fiatSent || - m.action == Action.fiatSentOk, - ); - if (hadFiatSent) { - notifier.setFiatWasSent(); - logger.i( - 'Restore: fiatWasSent=true for disputed order ${orderDetail.id}', - ); - } + // Fiat-sent status is checked later, once the buffer has flushed. // Create dispute message with Dispute payload (per Mostro protocol) // Timestamp is the restore-start anchor (not orderDetail.createdAt, @@ -823,30 +812,13 @@ class RestoreService { // Update state with dispute message notifier.updateStateFromMessage(disputeMessage); + pendingReconciliation + .add((orderId: orderDetail.id, message: disputeMessage)); logger.i( 'Restore: created dispute message for order ${orderDetail.id}', ); } else { - // For cooperativelyCanceled orders, check message history to - // determine if fiat was sent before the cancel was initiated. - // This sets fiatWasSent so updateWith can remap to the correct - // semantic action variant. - if (order.status == Status.cooperativelyCanceled) { - final messages = await storage.getAllMessagesForOrderId( - orderDetail.id, - ); - final hadFiatSent = messages.any( - (m) => - m.action == Action.fiatSent || - m.action == Action.fiatSentOk, - ); - if (hadFiatSent) { - notifier.setFiatWasSent(); - logger.i( - 'Restore: fiatWasSent=true for cooperativelyCanceled order ${orderDetail.id}', - ); - } - } + // Fiat-sent status is checked later, once the buffer has flushed. // Create regular order message with Order payload // Timestamp is the restore-start anchor (not orderDetail.createdAt, @@ -867,6 +839,10 @@ class RestoreService { // Update state with order message notifier.updateStateFromMessage(mostroMessage); + if (order.status == Status.cooperativelyCanceled) { + pendingReconciliation + .add((orderId: orderDetail.id, message: mostroMessage)); + } } } catch (e, stack) { logger.e( @@ -889,9 +865,65 @@ class RestoreService { ref.read(isRestoringProvider.notifier).state = false; logger.e('Restore: error during restore', error: e, stackTrace: stack); rethrow; + } finally { + // Restore mode is already off here, so the flush's replayed events + // reach storage instead of re-buffering. Never let this throw — it + // would mask a real restore error being rethrown above. + try { + await ref.read(mostroServiceProvider).flushRestoreBuffer(); + await _reconcileFiatSent(pendingReconciliation); + } catch (e, stack) { + logger.e( + 'Restore: post-restore flush/reconciliation failed', + error: e, + stackTrace: stack, + ); + } } } + // Rechecks fiat-sent status for orders deferred during the restore loop, + // now that the buffer has drained. Per-order failures are isolated, same + // as MostroService._flushRestoreBuffer. + Future _reconcileFiatSent( + List<({String orderId, MostroMessage message})> pending, + ) async { + if (pending.isEmpty) return; + final storage = ref.read(mostroStorageProvider); + logger.i('Restore: reconciling fiat-sent status for ${pending.length} orders'); + for (final entry in pending) { + try { + final messages = + await storage.getAllMessagesForOrderId(entry.orderId); + final hadFiatSent = messages.any( + (m) => + m.action == Action.fiatSent || m.action == Action.fiatSentOk, + ); + if (!hadFiatSent) continue; + + final notifier = + ref.read(orderNotifierProvider(entry.orderId).notifier); + notifier.setFiatWasSent(); + notifier.updateStateFromMessage(entry.message); + logger.i( + 'Restore: reconciled fiatWasSent=true for order ${entry.orderId}', + ); + } catch (e, stack) { + logger.e( + 'Restore: fiat reconciliation failed for order ${entry.orderId}', + error: e, + stackTrace: stack, + ); + } + } + } + + @visibleForTesting + Future reconcileFiatSentForTesting( + List<({String orderId, MostroMessage message})> pending, + ) => + _reconcileFiatSent(pending); + //Workflow: // 1. Clear existing data // 2. Create temporary subscription to key index 1 for restore notifications diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index 694d8e6d..df206d92 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -30,6 +30,10 @@ class MostroService { // in its original arrival position) and replayed once restore ends. final Map _restoreBuffer = {}; + // Shared by the reactive listener and flushRestoreBuffer() so both await + // the same drain instead of racing. + Future? _activeFlush; + MostroService(this.ref) : _settings = ref.read(settingsProvider); void init() { @@ -59,7 +63,7 @@ class MostroService { // success path and its catch block). _restoreListener = ref.listen(isRestoringProvider, (previous, next) { if (previous == true && next == false) { - unawaited(_flushRestoreBuffer()); + unawaited(flushRestoreBuffer()); } }); } @@ -250,6 +254,12 @@ class MostroService { } } + // Awaitable flush for production callers. Idempotent when the buffer is empty. + Future flushRestoreBuffer() => + _activeFlush ??= _flushRestoreBuffer().whenComplete(() { + _activeFlush = null; + }); + @visibleForTesting Future onDataForTesting(NostrEvent event) => _onData(event); From cbdb6b43ec3669bb9425c70530b3d2a763763edf Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 02:17:09 -0600 Subject: [PATCH 05/13] fix(order-state): stop double-converting dispute message timestamp message.timestamp is already milliseconds everywhere it's set in this codebase (mostro_service.dart, mostro_storage.dart, restore_manager.dart all use millisecondsSinceEpoch). This code treated it as seconds and multiplied by 1000 again, pushing Dispute.createdAt to roughly year 58488 for restored disputes. Pre-existing since 973b0174c (~9 months old), unrelated to the restore buffer/fiat-sent work in this branch. Real-world impact was low (createdAt is never rendered as text, only used to sort disputes when more than one exists) but surfaced again in review on this branch's restore path, so fixing it here. --- lib/features/order/models/order_state.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/features/order/models/order_state.dart b/lib/features/order/models/order_state.dart index 4f27f8f6..5699489a 100644 --- a/lib/features/order/models/order_state.dart +++ b/lib/features/order/models/order_state.dart @@ -161,10 +161,10 @@ class OrderState { // If we got a dispute from the message payload, ensure it has the message timestamp // This is critical for correct sorting in the dispute list if (updatedDispute != null && message.getPayload() != null) { - // Use message timestamp if dispute doesn't have a createdAt or if message has a timestamp - // Note: Nostr timestamps are in seconds, so convert to milliseconds + // message.timestamp is already milliseconds everywhere it's set in this + // codebase; a pre-existing bug here re-multiplied it by 1000 again. if (message.timestamp != null) { - final tsMs = message.timestamp! * 1000; + final tsMs = message.timestamp!; if (updatedDispute.createdAt == null || updatedDispute.createdAt!.millisecondsSinceEpoch != tsMs) { updatedDispute = updatedDispute.copyWith( From 541f0e4df16e698c641e2909e7d22aa1b1d99fec Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 08:55:41 -0600 Subject: [PATCH 06/13] fix(restore): address two Codex review findings on this branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. _restoreStartTime used raw millisecond precision, which could occasionally sort newer than a live event created in the same Nostr second (Nostr created_at is second-precision, never exact). Floored the anchor to just before the current second so it can never outrank a same-second live event. 2. _reconcileFiatSent replayed the tracked synthetic snapshot after the buffer flush, unconditionally overwriting anything newer the flush had just applied for the same order. An earlier version of this fix guarded the replay by comparing stored timestamps, but that left setFiatWasSent() running unconditionally while skipping the action remap it depends on — a live cooperativeCancelInitiated* event flushed before reconciliation would still get stuck on the NoFiat action variant. Replaced with a direct, idempotent correction (OrderNotifier.upgradeCooperativeCancelToFiatSent) that only touches the action field when it's still a stale NoFiat variant, regardless of what produced it. Documented both as D5/D6 in SESSION_RECOVERY_ARCHITECTURE.md. --- .../SESSION_RECOVERY_ARCHITECTURE.md | 16 ++++++- .../order/notifiers/order_notifier.dart | 12 +++++ lib/features/restore/restore_manager.dart | 46 ++++++++++--------- 3 files changed, 51 insertions(+), 23 deletions(-) diff --git a/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md b/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md index c3653888..72faba79 100644 --- a/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md +++ b/docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md @@ -602,7 +602,21 @@ in arrival order, once restore ends. newer than all pre-restore history yet older than any live event that arrives during restore. The real creation time is still preserved in the `Order`/`Dispute` payload for display; only the ordering timestamp - changes. + changes. Real Nostr timestamps are always second-precision, never exact + to the millisecond, so `_restoreStartTime` is floored to just before the + current second (`_floorToPreviousSecond`) rather than used raw — otherwise + a live event created in the same second could sort older than the anchor + despite happening after it. +- **D6 — Deferred fiat-sent classification, reconciled once after flush**: + `RestoreService.restore()` no longer checks `storage.getAllMessagesForOrderId()` + inline while building disputed/cooperativelyCanceled snapshots — the + confirming `fiatSent`/`fiatSentOk` message may still be sitting unflushed + in `MostroService._restoreBuffer` at that point. Affected orders are + tracked and rechecked once, in a `finally` block, after awaiting the new + public `MostroService.flushRestoreBuffer()` (single-flight guarded against + the existing reactive listener). The tracked snapshot is only replayed if + no stored message for that order is newer, so a buffered live update + flushed on the same pass is never overwritten by the stale snapshot. **Target `_onData` control flow** (landmark-relative — decrypt, session-match, DM/restore-payload skips, and the timestamp fallback already existed; only diff --git a/lib/features/order/notifiers/order_notifier.dart b/lib/features/order/notifiers/order_notifier.dart index e3046533..65f91610 100644 --- a/lib/features/order/notifiers/order_notifier.dart +++ b/lib/features/order/notifiers/order_notifier.dart @@ -230,6 +230,18 @@ class OrderNotifier extends AbstractMostroNotifier { state = state.copyWith(fiatWasSent: true); } + // Corrects a stale no-fiat cooperative-cancel action after the fact, + // regardless of what set it (a restore snapshot or a live event that + // ran before setFiatWasSent()). + void upgradeCooperativeCancelToFiatSent() { + if (!mounted) return; + if (state.action == Action.cooperativeCancelNoFiatByYou) { + state = state.copyWith(action: Action.cooperativeCancelFiatSentByYou); + } else if (state.action == Action.cooperativeCancelNoFiatByPeer) { + state = state.copyWith(action: Action.cooperativeCancelFiatSentByPeer); + } + } + /// Update dispute in state (used during restore) void updateDispute(Dispute dispute) { if (mounted) { diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 03fbb633..26acc6ba 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -68,7 +68,17 @@ class RestoreService { // history yet older than any live event arriving during restore, regardless // of the order's own (possibly stale) createdAt. Defaults to construction // time as a safe fallback in case restore() is ever invoked directly. - int _restoreStartTime = DateTime.now().millisecondsSinceEpoch; + int _restoreStartTime = + _floorToPreviousSecond(DateTime.now().millisecondsSinceEpoch); + + // Nostr timestamps are always whole seconds, never millisecond-precise; + // flooring the anchor to just before the current second guarantees it + // can never outrank a live event created in this same second. + static int _floorToPreviousSecond(int ms) => (ms ~/ 1000) * 1000 - 1; + + @visibleForTesting + static int floorToPreviousSecondForTesting(int ms) => + _floorToPreviousSecond(ms); RestoreService(this.ref); @@ -602,8 +612,7 @@ class RestoreService { List disputes, ) async { // Orders needing a fiat-sent recheck once the restore buffer drains. - final pendingReconciliation = - <({String orderId, MostroMessage message})>[]; + final pendingReconciliation = []; try { if (_masterKey == null) { @@ -812,8 +821,7 @@ class RestoreService { // Update state with dispute message notifier.updateStateFromMessage(disputeMessage); - pendingReconciliation - .add((orderId: orderDetail.id, message: disputeMessage)); + pendingReconciliation.add(orderDetail.id); logger.i( 'Restore: created dispute message for order ${orderDetail.id}', ); @@ -840,8 +848,7 @@ class RestoreService { // Update state with order message notifier.updateStateFromMessage(mostroMessage); if (order.status == Status.cooperativelyCanceled) { - pendingReconciliation - .add((orderId: orderDetail.id, message: mostroMessage)); + pendingReconciliation.add(orderDetail.id); } } } catch (e, stack) { @@ -885,32 +892,28 @@ class RestoreService { // Rechecks fiat-sent status for orders deferred during the restore loop, // now that the buffer has drained. Per-order failures are isolated, same // as MostroService._flushRestoreBuffer. - Future _reconcileFiatSent( - List<({String orderId, MostroMessage message})> pending, - ) async { + Future _reconcileFiatSent(List pending) async { if (pending.isEmpty) return; final storage = ref.read(mostroStorageProvider); logger.i('Restore: reconciling fiat-sent status for ${pending.length} orders'); - for (final entry in pending) { + for (final orderId in pending) { try { - final messages = - await storage.getAllMessagesForOrderId(entry.orderId); + final messages = await storage.getAllMessagesForOrderId(orderId); final hadFiatSent = messages.any( (m) => m.action == Action.fiatSent || m.action == Action.fiatSentOk, ); if (!hadFiatSent) continue; - final notifier = - ref.read(orderNotifierProvider(entry.orderId).notifier); + final notifier = ref.read(orderNotifierProvider(orderId).notifier); notifier.setFiatWasSent(); - notifier.updateStateFromMessage(entry.message); + notifier.upgradeCooperativeCancelToFiatSent(); logger.i( - 'Restore: reconciled fiatWasSent=true for order ${entry.orderId}', + 'Restore: reconciled fiatWasSent=true for order $orderId', ); } catch (e, stack) { logger.e( - 'Restore: fiat reconciliation failed for order ${entry.orderId}', + 'Restore: fiat reconciliation failed for order $orderId', error: e, stackTrace: stack, ); @@ -919,9 +922,7 @@ class RestoreService { } @visibleForTesting - Future reconcileFiatSentForTesting( - List<({String orderId, MostroMessage message})> pending, - ) => + Future reconcileFiatSentForTesting(List pending) => _reconcileFiatSent(pending); //Workflow: @@ -944,7 +945,8 @@ class RestoreService { _operationCompleter = Completer(); // Snapshot the restore-start anchor before anything else, so every // synthetic message this run produces shares one ordering timestamp. - _restoreStartTime = DateTime.now().millisecondsSinceEpoch; + _restoreStartTime = + _floorToPreviousSecond(DateTime.now().millisecondsSinceEpoch); // Hold the shared session lock for the whole restore so order/take flows // cannot interleave with the session reset (and rebuild) below. final releaseSessionLock = From 727758d05b2eecb8a3fce76c11b98de4f53294af Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 09:23:12 -0600 Subject: [PATCH 07/13] fix(order-state): stop re-stamping dispute createdAt on every update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updateWith() re-synced Dispute.createdAt to match the wrapping message's timestamp whenever they differed, not just when createdAt was missing. For restored disputes this clobbers the real creation time (sourced from orderDetail.createdAt when the synthetic message is built) with _restoreStartTime, the restore ordering anchor — making restored disputes sort as if freshly created. A dispute's creation time is fixed and both message-construction paths (restore and live) already resolve it properly before this code runs, so it should only ever fill a genuinely missing value, never overwrite an existing one. --- lib/features/order/models/order_state.dart | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/features/order/models/order_state.dart b/lib/features/order/models/order_state.dart index 5699489a..4686cba0 100644 --- a/lib/features/order/models/order_state.dart +++ b/lib/features/order/models/order_state.dart @@ -158,20 +158,16 @@ class OrderState { // Handle dispute status updates based on action Dispute? updatedDispute = message.getPayload() ?? dispute; - // If we got a dispute from the message payload, ensure it has the message timestamp - // This is critical for correct sorting in the dispute list if (updatedDispute != null && message.getPayload() != null) { - // message.timestamp is already milliseconds everywhere it's set in this - // codebase; a pre-existing bug here re-multiplied it by 1000 again. - if (message.timestamp != null) { - final tsMs = message.timestamp!; - if (updatedDispute.createdAt == null || - updatedDispute.createdAt!.millisecondsSinceEpoch != tsMs) { - updatedDispute = updatedDispute.copyWith( - createdAt: DateTime.fromMillisecondsSinceEpoch(tsMs), - ); - logger.i('Updated dispute ${updatedDispute.disputeId} createdAt from message timestamp: ${updatedDispute.createdAt}'); - } + // Only fill createdAt when the dispute doesn't already carry one — a + // dispute's creation time is fixed and must never be re-stamped by a + // later message's timestamp (which, during restore, is an ordering + // anchor, not a real creation time). + if (updatedDispute.createdAt == null && message.timestamp != null) { + updatedDispute = updatedDispute.copyWith( + createdAt: DateTime.fromMillisecondsSinceEpoch(message.timestamp!), + ); + logger.i('Set dispute ${updatedDispute.disputeId} createdAt from message timestamp: ${updatedDispute.createdAt}'); } } From 3feeda77125c792e1e1ed5910d0ca681e6c1a337 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 09:54:07 -0600 Subject: [PATCH 08/13] fix(restore): clear dedup reservation when dropping a session-less event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _onData reserves an event's dedup entry before checking for a matching session, then silently drops the event if none exists — without undoing that reservation. If restore() aborts before recreating every session (the finally block flushes the buffer regardless of outcome), any buffered event for a not-yet-recreated session gets dropped here during replay, but its dedup entry survives. A later retry, even after the session exists, would then see the event as already processed and silently skip it — permanent data loss. Delete the dedup entry before returning so a genuinely unprocessed event can still be picked up later. --- lib/services/mostro_service.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index df206d92..b81d5fd8 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -156,6 +156,10 @@ class MostroService { ); if (matchingSession == null) { logger.w('No matching session found for recipient: ${event.recipient}'); + // Undo the dedup reservation: this event was never actually + // processed, so a later retry (once the session exists) must not + // be silently dropped as "already seen". + await eventStore.deleteItem(event.id!); return; } final privateKey = matchingSession.tradeKey.private; From c3e9e1cfa3028ec83bc18968d7d9bb511b5ae4a0 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 18:22:47 -0600 Subject: [PATCH 09/13] fix(notifications): add receivedAt to fix stale-timestamp notification gate msg.timestamp now holds protocol send time (needed for ordering), which silently broke the 60s live-notification/navigation freshness gate for any message delivered late (offline reconnect, relay backlog). Adds a receivedAt field stamped with true client arrival time, persisted write-once via MostroStorage's existing dedup guard but excluded from toJson() so it never reaches the outbound wire path. Also fixes the restore buffer to preserve the original buffering instant across redeliveries instead of re-stamping it at flush time. Addresses grunch's BLOCKING review comment on PR #639. --- integration_test/test_helpers.dart | 8 +++- lib/data/models/mostro_message.dart | 8 ++++ lib/data/repositories/mostro_storage.dart | 1 + lib/services/mostro_service.dart | 57 ++++++++++++++++++----- 4 files changed, 61 insertions(+), 13 deletions(-) diff --git a/integration_test/test_helpers.dart b/integration_test/test_helpers.dart index 3b92d6c7..bec2f9aa 100644 --- a/integration_test/test_helpers.dart +++ b/integration_test/test_helpers.dart @@ -322,7 +322,10 @@ class FakeMostroService implements MostroService { } @override - Future onDataForTesting(NostrEvent event) async {} + Future onDataForTesting( + NostrEvent event, { + int? bufferedReceivedAt, + }) async {} @override Future flushRestoreBuffer() async {} @@ -331,7 +334,8 @@ class FakeMostroService implements MostroService { Future flushRestoreBufferForTesting() async {} @override - Map get restoreBufferForTesting => {}; + Map + get restoreBufferForTesting => {}; } Future pumpTestApp(WidgetTester tester) async { diff --git a/lib/data/models/mostro_message.dart b/lib/data/models/mostro_message.dart index 7d8a9323..79691af0 100644 --- a/lib/data/models/mostro_message.dart +++ b/lib/data/models/mostro_message.dart @@ -15,6 +15,7 @@ class MostroMessage { int? tradeIndex; T? _payload; int? timestamp; + int? receivedAt; MostroMessage({ required this.action, @@ -23,6 +24,7 @@ class MostroMessage { T? payload, this.tradeIndex, this.timestamp, + this.receivedAt, }) : _payload = payload; Map toJson({int? version}) { @@ -46,6 +48,11 @@ class MostroMessage { factory MostroMessage.fromJson(Map json) { final timestamp = json['timestamp']; + // receivedAt is only present when reconstructing from a stored Sembast + // record (injected by MostroStorage.addMessage); live wire payloads and + // restore's synthetic messages never carry this key, so it parses as + // null for those. + final receivedAt = json['receivedAt']; // IMPORTANT : Use 'order', 'restore' or 'cant-do' key as per protocol json = json['order'] ?? json['restore'] ?? json['cant-do'] ?? json; final num requestId = json['request_id'] ?? 0; @@ -59,6 +66,7 @@ class MostroMessage { ? Payload.fromJson(json['payload']) as T? : null, timestamp: timestamp, + receivedAt: receivedAt, ); } diff --git a/lib/data/repositories/mostro_storage.dart b/lib/data/repositories/mostro_storage.dart index 659180da..c4545710 100644 --- a/lib/data/repositories/mostro_storage.dart +++ b/lib/data/repositories/mostro_storage.dart @@ -19,6 +19,7 @@ class MostroStorage extends BaseStorage { final Map dbMap = message.toJson(); message.timestamp ??= DateTime.now().millisecondsSinceEpoch; dbMap['timestamp'] = message.timestamp; + dbMap['receivedAt'] = message.receivedAt; await store.record(id).put(db, dbMap); logger.i( diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index b81d5fd8..e82794a2 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -28,7 +28,12 @@ class MostroService { // Live events received while a restore is in progress are buffered here // (keyed by event id, so re-delivery of the same id keeps a single entry // in its original arrival position) and replayed once restore ends. - final Map _restoreBuffer = {}; + // `receivedAtMs` is the TRUE original buffering time: it is preserved + // (never re-stamped) if the same event id is buffered again, so the + // eventual replay stamps the message with when it was first seen, not + // when the buffer happens to flush. + final Map _restoreBuffer = + {}; // Shared by the reactive listener and flushRestoreBuffer() so both await // the same drain instead of racing. @@ -128,7 +133,7 @@ class MostroService { return false; } - Future _onData(NostrEvent event) async { + Future _onData(NostrEvent event, {int? bufferedReceivedAt}) async { final eventStore = ref.read(eventStorageProvider); if (await eventStore.hasItem(event.id!)) return; @@ -145,7 +150,21 @@ class MostroService { // matching session". Buffered events are replayed through _onData once // restore completes (success or error path alike); see _flushRestoreBuffer. if (ref.read(isRestoringProvider)) { - _restoreBuffer[event.id!] = event; + // Preserve-if-present: only a genuinely first-time buffering of this + // event id stamps `now()`. Any redelivery (the dedup reservation is + // released right below, specifically to allow redelivery) must keep + // the original buffering instant. + final existing = _restoreBuffer[event.id!]; + _restoreBuffer[event.id!] = ( + event: event, + receivedAtMs: + existing?.receivedAtMs ?? DateTime.now().millisecondsSinceEpoch, + ); + // Undo the dedup reservation: only the in-memory buffer holds this + // event now, and it does not survive process death. Releasing the + // reservation lets a later redelivery re-enter and flush normally + // instead of being dropped forever as "already seen". + await eventStore.deleteItem(event.id!); logger.i('Restore: buffered live event ${event.id}'); return; } @@ -219,6 +238,14 @@ class MostroService { innerCreatedAt?.millisecondsSinceEpoch ?? event.createdAt?.millisecondsSinceEpoch; + // True client arrival time: for a live event this is "now"; for a + // replayed restore-buffered event it is the original buffering + // instant, threaded back in by _flushRestoreBuffer. Always an + // explicit overwrite (never `??=`) so a peer-supplied value parsed by + // MostroMessage.fromJson can never survive — see INVARIANT R3-005. + msg.receivedAt = + bufferedReceivedAt ?? DateTime.now().millisecondsSinceEpoch; + final messageStorage = ref.read(mostroStorageProvider); // Use the inner rumor id if available (v1), otherwise fall back to the @@ -244,16 +271,22 @@ class MostroService { /// when the event was first buffered. Future _flushRestoreBuffer() async { if (_restoreBuffer.isEmpty) return; - final events = List.from(_restoreBuffer.values); + final entries = + List<({NostrEvent event, int receivedAtMs})>.from( + _restoreBuffer.values, + ); _restoreBuffer.clear(); - logger.i('Restore: flushing ${events.length} buffered live events'); + logger.i('Restore: flushing ${entries.length} buffered live events'); final eventStore = ref.read(eventStorageProvider); - for (final event in events) { + for (final entry in entries) { try { - await eventStore.deleteItem(event.id!); - await _onData(event); + await eventStore.deleteItem(entry.event.id!); + await _onData(entry.event, bufferedReceivedAt: entry.receivedAtMs); } catch (e) { - logger.e('Restore: failed to replay buffered event ${event.id}', error: e); + logger.e( + 'Restore: failed to replay buffered event ${entry.event.id}', + error: e, + ); } } } @@ -265,13 +298,15 @@ class MostroService { }); @visibleForTesting - Future onDataForTesting(NostrEvent event) => _onData(event); + Future onDataForTesting(NostrEvent event, {int? bufferedReceivedAt}) => + _onData(event, bufferedReceivedAt: bufferedReceivedAt); @visibleForTesting Future flushRestoreBufferForTesting() => _flushRestoreBuffer(); @visibleForTesting - Map get restoreBufferForTesting => _restoreBuffer; + Map + get restoreBufferForTesting => _restoreBuffer; Future _maybeLinkChildOrder( MostroMessage message, From e9aa8460cff7e48da77870306a3268a8909a313e Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 18:30:33 -0600 Subject: [PATCH 10/13] fix(notifications): swap freshness gate to compare receivedAt Both live-notification/navigation gate checks in AbstractMostroNotifier compared against msg.timestamp, which now holds protocol send time instead of arrival time. Swapped both to receivedAt so a message delivered late (offline reconnect) but genuinely new to this client still triggers a notification. --- .../order/notifiers/abstract_mostro_notifier.dart | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index 6be5d051..97ca8735 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -110,19 +110,19 @@ class AbstractMostroNotifier extends StateNotifier { if (mounted) { state = state.updateWith(msg); } - if (msg.timestamp != null && - msg.timestamp! > + if (msg.receivedAt != null && + msg.receivedAt! > DateTime.now() .subtract(const Duration(seconds: 60)) .millisecondsSinceEpoch) { logger.i( - 'Message timestamp check passed, calling handleEvent for ${msg.action}'); + 'Message freshness check passed, calling handleEvent for ${msg.action}'); unawaited(handleEvent(msg, previousStatus: previousStatus, wasUserInitiatedCancel: wasUserInitiatedCancel)); } else { logger.w( - 'Message timestamp check failed for ${msg.action}. Timestamp: ${msg.timestamp}, Current: ${DateTime.now().millisecondsSinceEpoch}, Threshold: ${DateTime.now().subtract(const Duration(seconds: 60)).millisecondsSinceEpoch}'); + 'Message freshness check failed for ${msg.action}. ReceivedAt: ${msg.receivedAt}, Current: ${DateTime.now().millisecondsSinceEpoch}, Threshold: ${DateTime.now().subtract(const Duration(seconds: 60)).millisecondsSinceEpoch}'); // Handle dispute actions even if timestamp is old, since they're critical for UI state // but bypass navigation/notification side effects @@ -180,8 +180,8 @@ class AbstractMostroNotifier extends StateNotifier { final navProvider = ref.read(navigationProvider.notifier); // Check if this is a recent event for notification/navigation purposes - final isRecent = event.timestamp != null && - event.timestamp! > + final isRecent = event.receivedAt != null && + event.receivedAt! > DateTime.now() .subtract(const Duration(seconds: 60)) .millisecondsSinceEpoch; @@ -203,7 +203,7 @@ class AbstractMostroNotifier extends StateNotifier { ); } else if (notificationData != null && bypassTimestampGate) { logger.i( - 'Skipping notification for old event: ${event.action} (timestamp: ${event.timestamp})'); + 'Skipping notification for old event: ${event.action} (receivedAt: ${event.receivedAt})'); } /// Handle incoming events and update state accordingly From 5f2789fd4634fd456a83535c5c86ad76e7c75d90 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Mon, 13 Jul 2026 18:59:07 -0600 Subject: [PATCH 11/13] test(notifications): add coverage for restore buffer and freshness gate Adds the test coverage grunch's review requested for the restore-buffer work: buffering during isRestoring (with and without a matching session), dedup clearing on flush so replay isn't swallowed, the v1 inner-rumor timestamp anchor, and _reconcileFiatSent upgrading a stale cooperativeCancelNoFiatByPeer once a confirming fiat-sent message lands in the flush. Also adds the core regression coverage for this session's receivedAt fix: the freshness gate must key off receivedAt (not protocol timestamp) so a message with an old send time but a fresh arrival time still notifies, MostroStorage's write-once guard must not let a redelivery overwrite the original receivedAt, the restore buffer must preserve the first-buffered receivedAtMs across redeliveries, and the client's explicit receivedAt assignment must always win over any wire-supplied value. test/mocks.dart gains a generated mock for EventStorage so the ported buffer tests can assert dedup-clearing order with verifyInOrder, matching the mocking style already used for the sibling storage mocks. --- .../repositories/mostro_storage_test.dart | 38 ++ ...t_mostro_notifier_freshness_gate_test.dart | 200 +++++++++ .../restore_manager_reconciliation_test.dart | 189 ++++++++ test/mocks.dart | 2 + test/services/mostro_service_test.dart | 419 ++++++++++++++++++ 5 files changed, 848 insertions(+) create mode 100644 test/data/repositories/mostro_storage_test.dart create mode 100644 test/features/order/notifiers/abstract_mostro_notifier_freshness_gate_test.dart create mode 100644 test/features/restore/restore_manager_reconciliation_test.dart diff --git a/test/data/repositories/mostro_storage_test.dart b/test/data/repositories/mostro_storage_test.dart new file mode 100644 index 00000000..058261b3 --- /dev/null +++ b/test/data/repositories/mostro_storage_test.dart @@ -0,0 +1,38 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:sembast/sembast_memory.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/repositories/mostro_storage.dart'; + +void main() { + late MostroStorage storage; + + setUp(() async { + final db = await databaseFactoryMemory.openDatabase('mostro_storage_test.db'); + storage = MostroStorage(db: db); + }); + + group('MostroStorage.addMessage receivedAt persistence', () { + test('write-once: a second addMessage call for the same key does not ' + 'overwrite the original receivedAt', () async { + final first = MostroMessage( + action: Action.fiatSent, + id: 'order-2', + receivedAt: 1000, + ); + final second = MostroMessage( + action: Action.fiatSent, + id: 'order-2', + receivedAt: 9999, + ); + + await storage.addMessage('key-2', first); + await storage.addMessage('key-2', second); + + final reloaded = await storage.getLatestMessageById('order-2'); + + expect(reloaded, isNotNull); + expect(reloaded!.receivedAt, 1000); + }); + }); +} diff --git a/test/features/order/notifiers/abstract_mostro_notifier_freshness_gate_test.dart b/test/features/order/notifiers/abstract_mostro_notifier_freshness_gate_test.dart new file mode 100644 index 00000000..57e125f6 --- /dev/null +++ b/test/features/order/notifiers/abstract_mostro_notifier_freshness_gate_test.dart @@ -0,0 +1,200 @@ +import 'dart:async'; + +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:sembast/sembast_memory.dart'; + +import 'package:mostro_mobile/data/enums.dart'; +import 'package:mostro_mobile/data/models.dart'; +import 'package:mostro_mobile/features/notifications/providers/notifications_provider.dart'; +import 'package:mostro_mobile/features/order/notifiers/abstract_mostro_notifier.dart'; +import 'package:mostro_mobile/shared/providers/mostro_database_provider.dart'; +import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; +import 'package:mostro_mobile/shared/providers/navigation_notifier_provider.dart'; +import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; + +import '../../../mocks.dart'; +import '../../../mocks.mocks.dart'; + +/// Regression coverage for the notification freshness gate swap: both gate +/// call sites in `AbstractMostroNotifier` (subscribe()'s check and +/// handleEvent()'s `isRecent`) must key off `receivedAt` (true client arrival +/// time) instead of `timestamp` (protocol/send time). The single most +/// important scenario here is the reconnect-after-offline case: an old +/// protocol `timestamp` must NOT suppress a message that is genuinely new to +/// this client. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + const orderId = 'freshness-gate-order'; + var dbCounter = 0; + + late ProviderContainer container; + late StreamController messageController; + late AbstractMostroNotifier notifier; + + Session buildSession() { + return Session( + masterKey: NostrKeyPairs( + private: + '1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'), + tradeKey: NostrKeyPairs( + private: + 'abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890'), + keyIndex: 0, + fullPrivacy: false, + startTime: DateTime.now(), + orderId: orderId, + role: Role.buyer, + ); + } + + Future flushAsyncOperations() async { + for (var i = 0; i < 10; i++) { + await Future.delayed(Duration.zero); + } + } + + setUp(() async { + dbCounter++; + final db = await databaseFactoryMemory + .openDatabase('freshness_gate_test_$dbCounter.db'); + messageController = StreamController.broadcast(); + + final mockKeyManager = MockKeyManager(); + final mockSessionStorage = MockSessionStorage(); + final mockSettings = MockSettings(); + final mockRef = MockRef(); + final mockSessionNotifier = MockSessionNotifier( + mockRef, mockKeyManager, mockSessionStorage, mockSettings); + mockSessionNotifier.setMockSession(buildSession()); + + container = ProviderContainer(overrides: [ + mostroDatabaseProvider.overrideWithValue(db), + mostroMessageStreamProvider + .overrideWith((ref, id) => messageController.stream), + sessionNotifierProvider.overrideWith((ref) => mockSessionNotifier), + ]); + + final ref = container.read(Provider((ref) => ref)); + notifier = AbstractMostroNotifier(orderId, ref); + notifier.subscribe(); + }); + + tearDown(() async { + notifier.dispose(); + container.dispose(); + await messageController.close(); + }); + + group('AbstractMostroNotifier freshness gate (receivedAt-based)', () { + test('recent receivedAt passes the gate and fires a notification', + () async { + final now = DateTime.now().millisecondsSinceEpoch; + final message = MostroMessage( + id: orderId, + action: Action.paymentFailed, + payload: + PaymentFailed(paymentAttempts: 1, paymentRetriesInterval: 60), + timestamp: now, + receivedAt: now, + ); + + messageController.add(message); + await flushAsyncOperations(); + + final temporary = container.read(currentTemporaryNotificationProvider); + expect(temporary.show, isTrue); + expect(temporary.action, Action.paymentFailed); + }); + + test( + 'old protocol timestamp with a recent receivedAt still passes the ' + 'gate (reconnect-after-offline regression)', () async { + final oldTimestamp = DateTime.now() + .subtract(const Duration(minutes: 10)) + .millisecondsSinceEpoch; + final recentReceivedAt = DateTime.now().millisecondsSinceEpoch; + final message = MostroMessage( + id: orderId, + action: Action.paymentFailed, + payload: + PaymentFailed(paymentAttempts: 1, paymentRetriesInterval: 60), + timestamp: oldTimestamp, + receivedAt: recentReceivedAt, + ); + + messageController.add(message); + await flushAsyncOperations(); + + final temporary = container.read(currentTemporaryNotificationProvider); + expect(temporary.show, isTrue, + reason: 'A message with an old protocol timestamp but a fresh ' + 'receivedAt must still be notified — this is the exact bug ' + 'this change fixes.'); + expect(temporary.action, Action.paymentFailed); + }); + + test( + 'old receivedAt fails the gate: no notification, but state still ' + 'updates unconditionally', () async { + final oldReceivedAt = DateTime.now() + .subtract(const Duration(minutes: 10)) + .millisecondsSinceEpoch; + final message = MostroMessage( + id: orderId, + action: Action.paymentFailed, + payload: + PaymentFailed(paymentAttempts: 2, paymentRetriesInterval: 30), + timestamp: DateTime.now().millisecondsSinceEpoch, + receivedAt: oldReceivedAt, + ); + + messageController.add(message); + await flushAsyncOperations(); + + final temporary = container.read(currentTemporaryNotificationProvider); + expect(temporary.show, isFalse); + + expect(notifier.state.action, Action.paymentFailed); + expect(notifier.state.status, Status.paymentFailed); + }); + + test( + 'dispute action bypasses the gate for state processing despite an ' + 'old receivedAt, but still suppresses notification and navigation', + () async { + final oldReceivedAt = DateTime.now() + .subtract(const Duration(minutes: 10)) + .millisecondsSinceEpoch; + final dispute = Dispute(disputeId: 'dispute-1', status: 'initiated'); + final message = MostroMessage( + id: orderId, + action: Action.disputeInitiatedByPeer, + payload: dispute, + timestamp: oldReceivedAt, + receivedAt: oldReceivedAt, + ); + + final pathBefore = container.read(navigationProvider).path; + + messageController.add(message); + await flushAsyncOperations(); + + // Notification is suppressed for the old event. + expect(container.read(currentTemporaryNotificationProvider).show, + isFalse); + // Navigation is suppressed too (isRecent is false, bypassTimestampGate + // is true). + expect(container.read(navigationProvider).path, pathBefore); + // Yet handleEvent still processed the dispute action (state-only): + // orderId/action are only stamped by handleEvent's dispute branch, not + // by the unconditional updateWith() merge, so this proves the bypass + // actually ran despite the old receivedAt. + expect(notifier.state.dispute, isNotNull); + expect(notifier.state.dispute!.orderId, orderId); + expect(notifier.state.dispute!.action, 'dispute-initiated-by-peer'); + }); + }); +} diff --git a/test/features/restore/restore_manager_reconciliation_test.dart b/test/features/restore/restore_manager_reconciliation_test.dart new file mode 100644 index 00000000..bda2d728 --- /dev/null +++ b/test/features/restore/restore_manager_reconciliation_test.dart @@ -0,0 +1,189 @@ +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.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/role.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/order.dart'; +import 'package:mostro_mobile/features/key_manager/key_manager_provider.dart'; +import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; +import 'package:mostro_mobile/features/restore/restore_manager.dart'; +import 'package:mostro_mobile/features/settings/settings.dart'; +import 'package:mostro_mobile/features/settings/settings_provider.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/order_repository_provider.dart'; +import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; +import 'package:mostro_mobile/shared/providers/session_storage_provider.dart'; +import 'package:mostro_mobile/shared/providers/storage_providers.dart'; + +import '../../mocks.dart'; +import '../../mocks.mocks.dart'; + +/// Regression coverage requested by review: _reconcileFiatSent must upgrade a +/// stale cooperativeCancelNoFiatByPeer/generic snapshot to +/// cooperativeCancelFiatSentByPeer once a confirming fiat-sent message is +/// found in storage after the restore buffer has drained. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('RestoreService - post-flush fiat-sent reconciliation', () { + late ProviderContainer container; + late MockMostroService mockMostroService; + late MockSharedPreferencesAsync mockPreferences; + late MockOpenOrdersRepository mockOrdersRepository; + late MockDatabase mockDatabase; + late MockSessionStorage mockSessionStorage; + late MockKeyManager mockKeyManager; + late MockSessionNotifier mockSessionNotifier; + late MockMostroStorage mockMostroStorage; + late MockRef ref; + + const testOrderId = 'reconciliation-order-id'; + + Order buildOrder() => const Order( + id: testOrderId, + kind: OrderType.sell, + fiatCode: 'USD', + fiatAmount: 100, + paymentMethod: 'Lightning', + amount: 100, + ); + + setUp(() { + mockMostroService = MockMostroService(); + mockPreferences = MockSharedPreferencesAsync(); + mockOrdersRepository = MockOpenOrdersRepository(); + mockDatabase = MockDatabase(); + mockSessionStorage = MockSessionStorage(); + mockKeyManager = MockKeyManager(); + mockMostroStorage = MockMostroStorage(); + ref = MockRef(); + + final testSettings = MockSettings(); + + mockSessionNotifier = MockSessionNotifier( + ref, mockKeyManager, mockSessionStorage, testSettings); + + when(mockKeyManager.masterKeyPair).thenReturn( + NostrKeyPairs( + private: + '1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'), + ); + when(mockKeyManager.getCurrentKeyIndex()).thenAnswer((_) async => 0); + + // Default: no confirming fiat-sent message on file. Overridden per-test. + when(mockMostroStorage.getAllMessagesForOrderId(any)) + .thenAnswer((_) async => []); + + container = ProviderContainer(overrides: [ + mostroServiceProvider.overrideWithValue(mockMostroService), + orderRepositoryProvider.overrideWithValue(mockOrdersRepository), + sharedPreferencesProvider.overrideWithValue(mockPreferences), + mostroDatabaseProvider.overrideWithValue(mockDatabase), + eventDatabaseProvider.overrideWithValue(mockDatabase), + sessionStorageProvider.overrideWithValue(mockSessionStorage), + keyManagerProvider.overrideWithValue(mockKeyManager), + sessionNotifierProvider.overrideWith((ref) => mockSessionNotifier), + settingsProvider.overrideWith((ref) { + final mockSettings = MockSettingsNotifier(); + mockSettings.state = Settings( + relays: ['wss://relay.damus.io'], + fullPrivacyMode: false, + mostroPublicKey: + '9d9d0455a96871f2dc4289b8312429db2e925f167b37c77bf7b28014be235980', + defaultFiatCode: 'USD', + ); + return mockSettings; + }), + mostroStorageProvider.overrideWithValue(mockMostroStorage), + ]); + }); + + tearDown(() { + container.dispose(); + }); + + test( + 'order regains the release action once a confirming fiat-sent message is found after flush', + () async { + // Arrange: apply the base cooperativelyCanceled snapshot exactly like + // restore()'s per-order loop does (generic action, fiatWasSent still + // false at this point). + final coopCancelMessage = MostroMessage( + id: testOrderId, + action: Action.cooperativeCancelInitiatedByPeer, + payload: buildOrder(), + ); + final notifier = + container.read(orderNotifierProvider(testOrderId).notifier); + notifier.updateStateFromMessage(coopCancelMessage); + + final beforeState = container.read(orderNotifierProvider(testOrderId)); + expect(beforeState.fiatWasSent, isFalse); + expect( + beforeState.getActions(Role.seller), + isNot(contains(Action.release)), + ); + + // Simulate the confirming message becoming visible only after the + // restore buffer is drained (the race this feature fixes). + when(mockMostroStorage.getAllMessagesForOrderId(testOrderId)) + .thenAnswer((_) async => [ + MostroMessage( + id: testOrderId, + action: Action.fiatSent, + ), + ]); + + final restoreService = container.read(restoreServiceProvider); + + // Act + await restoreService.reconcileFiatSentForTesting([testOrderId]); + + // Assert + final afterState = container.read(orderNotifierProvider(testOrderId)); + expect(afterState.fiatWasSent, isTrue); + expect(afterState.action, equals(Action.cooperativeCancelFiatSentByPeer)); + expect(afterState.getActions(Role.seller), contains(Action.release)); + }); + + test( + 'corrects a stale NoFiat action even when a live event (not our own snapshot) produced it before reconciliation ran', + () async { + // Simulates the Codex-flagged race: a buffered live cooperativeCancel + // event is flushed and processed by _onData/updateWith BEFORE + // _reconcileFiatSent runs, so it gets remapped with fiatWasSent still + // false — same NoFiat outcome as the synthetic snapshot case above, + // just from a different source message. + final liveCoopCancelMessage = MostroMessage( + id: testOrderId, + action: Action.cooperativeCancelInitiatedByPeer, + payload: buildOrder(), + ); + final notifier = + container.read(orderNotifierProvider(testOrderId).notifier); + notifier.updateStateFromMessage(liveCoopCancelMessage); + expect( + container.read(orderNotifierProvider(testOrderId)).action, + equals(Action.cooperativeCancelNoFiatByPeer), + ); + + when(mockMostroStorage.getAllMessagesForOrderId(testOrderId)) + .thenAnswer((_) async => [ + MostroMessage(id: testOrderId, action: Action.fiatSent), + ]); + + final restoreService = container.read(restoreServiceProvider); + await restoreService.reconcileFiatSentForTesting([testOrderId]); + + final afterState = container.read(orderNotifierProvider(testOrderId)); + expect(afterState.fiatWasSent, isTrue); + expect(afterState.action, equals(Action.cooperativeCancelFiatSentByPeer)); + expect(afterState.getActions(Role.seller), contains(Action.release)); + }); + }); +} diff --git a/test/mocks.dart b/test/mocks.dart index 0778bb9d..3c6911b4 100644 --- a/test/mocks.dart +++ b/test/mocks.dart @@ -4,6 +4,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:mockito/annotations.dart'; import 'package:mostro_mobile/data/models/session.dart'; import 'package:mostro_mobile/data/models/enums/role.dart'; +import 'package:mostro_mobile/data/repositories/event_storage.dart'; import 'package:mostro_mobile/data/repositories/open_orders_repository.dart'; import 'package:mostro_mobile/data/repositories/session_storage.dart'; import 'package:mostro_mobile/data/repositories/mostro_storage.dart'; @@ -38,6 +39,7 @@ import 'mocks.mocks.dart'; SessionStorage, KeyManager, MostroStorage, + EventStorage, Settings, Ref, ProviderSubscription, diff --git a/test/services/mostro_service_test.dart b/test/services/mostro_service_test.dart index 6a72045f..d63ac5bf 100644 --- a/test/services/mostro_service_test.dart +++ b/test/services/mostro_service_test.dart @@ -19,6 +19,7 @@ import 'package:mostro_mobile/features/settings/settings_provider.dart'; import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/nostr_event.dart'; import 'package:mostro_mobile/data/models/order.dart'; import 'package:mostro_mobile/features/order/models/order_state.dart'; import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; @@ -28,11 +29,63 @@ import 'package:mostro_mobile/data/models/enums/order_type.dart'; import 'package:mostro_mobile/data/models/enums/action.dart'; import 'package:mostro_mobile/data/repositories/open_orders_repository.dart'; import 'package:mostro_mobile/shared/providers/order_repository_provider.dart'; +import 'package:mostro_mobile/data/repositories/event_storage.dart'; +import 'package:mostro_mobile/features/restore/restore_mode_provider.dart'; +import 'package:mostro_mobile/shared/providers/mostro_service_provider.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; +import 'package:sembast/sembast_memory.dart'; import '../mocks.dart'; import '../mocks.mocks.dart'; import 'mostro_service_helper_functions.dart'; +/// Builds a real, correctly-signed kind-14 (NIP-44 direct) event simulating a +/// live message sent FROM the Mostro daemon TO the client's trade key. Reuses +/// [MostroMessage.wrapNip44] as-is (full-privacy mode, no master key), which +/// is the exact production wire format `_onData` expects to decrypt. +Future buildLiveDaemonEvent({ + required NostrKeyPairs mostroKeyPair, + required NostrKeyPairs recipientTradeKey, + required MostroMessage message, +}) { + return message.wrapNip44( + tradeKey: mostroKeyPair, + recipientPubKey: recipientTradeKey.public, + ); +} + +/// Builds a kind-14 event whose decrypted wire payload carries a spoofed +/// top-level `receivedAt` key alongside the real `order` wrapper — the shape +/// `_onData` would see if a peer ever supplied that client-local field. Used +/// to prove the client's own explicit assignment always overwrites it. +Future buildSpoofedReceivedAtEvent({ + required NostrKeyPairs mostroKeyPair, + required NostrKeyPairs recipientTradeKey, + required MostroMessage message, + required int spoofedReceivedAt, +}) async { + final messageMap = { + 'order': message.toJson(version: 2), + 'receivedAt': spoofedReceivedAt, + }; + final tuple = jsonEncode([messageMap, null, null]); + final encrypted = await NostrUtils.encryptNIP44( + tuple, + mostroKeyPair.private, + recipientTradeKey.public, + ); + + return NostrEvent.fromPartialData( + kind: 14, + content: encrypted, + keyPairs: mostroKeyPair, + tags: [ + ['p', recipientTradeKey.public], + ], + createdAt: DateTime.now(), + ); +} + void main() { // Provide dummy values for Mockito provideDummy(Settings( @@ -49,6 +102,10 @@ void main() { // Add dummy for MostroStorage provideDummy(MockMostroStorage()); + // Add dummy for EventStorage (never invoked directly — its constructor only + // stores the Database reference, so a mock db suffices as a placeholder). + provideDummy(EventStorage(db: MockDatabase())); + // Add dummy for NostrService provideDummy(MockNostrService()); @@ -564,6 +621,368 @@ void main() { expect(payload, isNull); }); }); + + group('MostroService._onData receivedAt stamping', () { + late NostrKeyPairs mostroKeyPair; + late NostrKeyPairs clientTradeKey; + late MockSettings liveSettings; + late MockMostroStorage capturingStorage; + late Database eventDb; + late EventStorage realEventStorage; + + setUp(() async { + mostroKeyPair = NostrUtils.generateKeyPair(); + clientTradeKey = NostrUtils.generateKeyPair(); + + liveSettings = MockSettings(); + when(liveSettings.mostroPublicKey).thenReturn(mostroKeyPair.public); + when(mockRef.read(settingsProvider)).thenReturn(liveSettings); + + capturingStorage = MockMostroStorage(); + when(capturingStorage.addMessage(any, any)) + .thenAnswer((_) async {}); + when(mockRef.read(mostroStorageProvider)).thenReturn(capturingStorage); + + eventDb = await databaseFactoryMemory.openDatabase( + 'mostro_service_test_events_${DateTime.now().microsecondsSinceEpoch}.db', + ); + realEventStorage = EventStorage(db: eventDb); + when(mockRef.read(eventStorageProvider)).thenReturn(realEventStorage); + + when(mockRef.read(isRestoringProvider)).thenReturn(false); + + final session = Session( + startTime: DateTime.now(), + masterKey: clientTradeKey, + keyIndex: 1, + tradeKey: clientTradeKey, + fullPrivacy: true, + ); + when(mockRef.read(sessionNotifierProvider)).thenReturn([session]); + + // Reconstruct the service so its internal `_settings` field (captured + // at construction) reflects `liveSettings`, not the outer setUp's. + mostroService = MostroService(mockRef); + }); + + test( + 'a wire payload carrying a receivedAt-shaped key is overwritten by ' + 'the client explicit assignment', () async { + final message = MostroMessage(action: Action.fiatSent, id: 'order-c'); + final spoofed = 1; // implausibly old, would fail any freshness check + final event = await buildSpoofedReceivedAtEvent( + mostroKeyPair: mostroKeyPair, + recipientTradeKey: clientTradeKey, + message: message, + spoofedReceivedAt: spoofed, + ); + + final before = DateTime.now().millisecondsSinceEpoch; + await mostroService.onDataForTesting(event); + final after = DateTime.now().millisecondsSinceEpoch; + + final captured = verify( + capturingStorage.addMessage(any, captureAny), + ).captured; + final stamped = captured.single as MostroMessage; + expect(stamped.receivedAt, isNot(spoofed)); + expect(stamped.receivedAt!, greaterThanOrEqualTo(before)); + expect(stamped.receivedAt!, lessThanOrEqualTo(after)); + }); + }); + + group('MostroService restore buffer preserve-if-present and flush timing', + () { + late NostrKeyPairs mostroKeyPair; + late NostrKeyPairs clientTradeKey; + late MockSettings liveSettings; + late MockMostroStorage capturingStorage; + late Database eventDb; + late EventStorage realEventStorage; + + setUp(() async { + mostroKeyPair = NostrUtils.generateKeyPair(); + clientTradeKey = NostrUtils.generateKeyPair(); + + liveSettings = MockSettings(); + when(liveSettings.mostroPublicKey).thenReturn(mostroKeyPair.public); + when(mockRef.read(settingsProvider)).thenReturn(liveSettings); + + capturingStorage = MockMostroStorage(); + when(capturingStorage.addMessage(any, any)) + .thenAnswer((_) async {}); + when(mockRef.read(mostroStorageProvider)).thenReturn(capturingStorage); + + eventDb = await databaseFactoryMemory.openDatabase( + 'mostro_service_test_restore_${DateTime.now().microsecondsSinceEpoch}.db', + ); + realEventStorage = EventStorage(db: eventDb); + when(mockRef.read(eventStorageProvider)).thenReturn(realEventStorage); + + final session = Session( + startTime: DateTime.now(), + masterKey: clientTradeKey, + keyIndex: 1, + tradeKey: clientTradeKey, + fullPrivacy: true, + ); + when(mockRef.read(sessionNotifierProvider)).thenReturn([session]); + + mostroService = MostroService(mockRef); + }); + + test( + 'redelivering the same event id while restoring preserves the ' + 'first-buffered receivedAtMs', () async { + when(mockRef.read(isRestoringProvider)).thenReturn(true); + + final message = MostroMessage(action: Action.fiatSent, id: 'order-d'); + final event = await buildLiveDaemonEvent( + mostroKeyPair: mostroKeyPair, + recipientTradeKey: clientTradeKey, + message: message, + ); + + final beforeFirst = DateTime.now().millisecondsSinceEpoch; + await mostroService.onDataForTesting(event); + final firstStamp = + mostroService.restoreBufferForTesting[event.id]!.receivedAtMs; + expect(firstStamp, greaterThanOrEqualTo(beforeFirst)); + + await Future.delayed(const Duration(milliseconds: 5)); + final beforeSecond = DateTime.now().millisecondsSinceEpoch; + + // Redeliver the same event id: production code releases the dedup + // reservation after buffering, so this re-enters _onData. + await mostroService.onDataForTesting(event); + final secondStamp = + mostroService.restoreBufferForTesting[event.id]!.receivedAtMs; + + expect(secondStamp, equals(firstStamp)); + expect(secondStamp, lessThan(beforeSecond)); + }); + }); + + group('MostroService restore buffer and reorder', () { + late MockRef restoreRef; + late MockSettings restoreSettings; + late MockEventStorage mockEventStorage; + late MockMostroStorage mockMostroStorage; + late MockSubscriptionManager restoreSubscriptionManager; + late MostroService restoreService; + late NostrKeyPairs mostroRestoreKeyPair; + late NostrKeyPairs userTradeKeyPair; + late List> addedMessages; + late Set reservedEventIds; + + setUp(() { + restoreRef = MockRef(); + restoreSettings = MockSettings(); + mockEventStorage = MockEventStorage(); + mockMostroStorage = MockMostroStorage(); + addedMessages = []; + reservedEventIds = {}; + + mostroRestoreKeyPair = NostrUtils.generateKeyPair(); + userTradeKeyPair = NostrUtils.generateKeyPair(); + + when(restoreSettings.mostroPublicKey) + .thenReturn(mostroRestoreKeyPair.public); + when(restoreRef.read(settingsProvider)).thenReturn(restoreSettings); + + // Real in-memory dedup semantics, so the buffer/flush/replay tests don't + // need per-test re-stubbing of hasItem/putItem/deleteItem. + when(restoreRef.read(eventStorageProvider)).thenReturn(mockEventStorage); + when(mockEventStorage.hasItem(any)).thenAnswer((invocation) async { + final id = invocation.positionalArguments[0] as String; + return reservedEventIds.contains(id); + }); + when(mockEventStorage.putItem(any, any)).thenAnswer((invocation) async { + final id = invocation.positionalArguments[0] as String; + reservedEventIds.add(id); + }); + when(mockEventStorage.deleteItem(any)).thenAnswer((invocation) async { + final id = invocation.positionalArguments[0] as String; + reservedEventIds.remove(id); + }); + + when(restoreRef.read(mostroStorageProvider)) + .thenReturn(mockMostroStorage); + when(mockMostroStorage.addMessage(any, any)) + .thenAnswer((invocation) async { + addedMessages.add(invocation.positionalArguments); + }); + + when(restoreRef.read(sessionNotifierProvider)).thenReturn([]); + when(restoreRef.read(isRestoringProvider)).thenReturn(false); + + // SubscriptionManager's constructor (which MockSubscriptionManager + // inherits unchanged) registers a session listener eagerly and reads + // the connected node's info stream. + when(restoreRef.listen>( + any, + any, + onError: anyNamed('onError'), + fireImmediately: anyNamed('fireImmediately'), + )).thenReturn(MockProviderSubscription>()); + + final restoreOrderRepo = MockOpenOrdersRepository(); + when(restoreOrderRepo.mostroInstanceStream) + .thenAnswer((_) => const Stream.empty()); + when(restoreRef.read(orderRepositoryProvider)) + .thenReturn(restoreOrderRepo); + + restoreSubscriptionManager = MockSubscriptionManager(restoreRef); + when(restoreRef.read(subscriptionManagerProvider)) + .thenReturn(restoreSubscriptionManager); + + // init() registers a ref.listen(isRestoringProvider, ...) callback; + // it must be stubbed for init() to succeed, even though these tests + // drive the flush directly via flushRestoreBufferForTesting() rather + // than through the reactive listener. + when(restoreRef.listen(isRestoringProvider, any)) + .thenReturn(MockProviderSubscription()); + + restoreService = MostroService(restoreRef); + restoreService.init(); + + // SubscriptionManager's constructor makes its own unrelated reads of + // sessionNotifierProvider (e.g. _initializeExistingSessions); clear that + // interaction history so verifyNever assertions below only observe + // calls made by the test body itself. + clearInteractions(restoreRef); + }); + + tearDown(() { + restoreService.dispose(); + restoreSubscriptionManager.dispose(); + }); + + void setMatchingSession() { + final session = Session( + startTime: DateTime.now(), + masterKey: NostrUtils.generateKeyPair(), + keyIndex: 1, + tradeKey: userTradeKeyPair, + fullPrivacy: false, + ); + when(restoreRef.read(sessionNotifierProvider)).thenReturn([session]); + } + + NostrEvent bareEvent({required int kind, required String id}) { + return NostrEvent( + id: id, + kind: kind, + content: 'irrelevant-not-decrypted-while-buffered', + pubkey: mostroRestoreKeyPair.public, + sig: '', + createdAt: DateTime.now(), + tags: [ + ['p', userTradeKeyPair.public], + ], + ); + } + + test('buffers a v1 kind-1059 event during restore without processing it', + () async { + when(restoreRef.read(isRestoringProvider)).thenReturn(true); + final event = bareEvent(kind: 1059, id: 'evt-v1-buffer'); + + await restoreService.onDataForTesting(event); + + expect( + restoreService.restoreBufferForTesting.keys, + contains('evt-v1-buffer'), + ); + expect(addedMessages, isEmpty); + verifyNever(restoreRef.read(sessionNotifierProvider)); + }); + + test( + 'buffers an event before the session-match check, even when no session exists yet', + () async { + when(restoreRef.read(isRestoringProvider)).thenReturn(true); + when(restoreRef.read(sessionNotifierProvider)).thenReturn([]); + final event = bareEvent(kind: 1059, id: 'evt-no-session'); + + await restoreService.onDataForTesting(event); + + expect( + restoreService.restoreBufferForTesting.keys, + contains('evt-no-session'), + reason: + 'reorder fix: buffering must precede the matchingSession==null early return', + ); + }); + + test( + 'replays buffered events in arrival order, clearing dedup before each replay', + () async { + setMatchingSession(); + when(restoreRef.read(isRestoringProvider)).thenReturn(true); + + final eventA = await MostroMessage(action: Action.newOrder, id: 'order-a') + .wrap(tradeKey: mostroRestoreKeyPair, recipientPubKey: userTradeKeyPair.public); + final eventB = await MostroMessage(action: Action.newOrder, id: 'order-b') + .wrap(tradeKey: mostroRestoreKeyPair, recipientPubKey: userTradeKeyPair.public); + final eventC = await MostroMessage(action: Action.newOrder, id: 'order-c') + .wrap(tradeKey: mostroRestoreKeyPair, recipientPubKey: userTradeKeyPair.public); + + await restoreService.onDataForTesting(eventA); + await restoreService.onDataForTesting(eventB); + await restoreService.onDataForTesting(eventC); + expect( + restoreService.restoreBufferForTesting.keys.toList(), + [eventA.id, eventB.id, eventC.id], + ); + + when(restoreRef.read(isRestoringProvider)).thenReturn(false); + await restoreService.flushRestoreBufferForTesting(); + + verifyInOrder([ + mockEventStorage.deleteItem(eventA.id!), + mockEventStorage.deleteItem(eventB.id!), + mockEventStorage.deleteItem(eventC.id!), + ]); + expect(restoreService.restoreBufferForTesting, isEmpty); + expect( + addedMessages.map((args) => (args[1] as MostroMessage).id).toList(), + ['order-a', 'order-b', 'order-c'], + ); + }); + + test( + "v1 replayed event uses the inner rumor's real timestamp, never the gift-wrap outer time", + () async { + setMatchingSession(); + when(restoreRef.read(isRestoringProvider)).thenReturn(true); + + final message = MostroMessage(action: Action.newOrder, id: 'order-ts-v1'); + final event = await message.wrap( + tradeKey: mostroRestoreKeyPair, + recipientPubKey: userTradeKeyPair.public, + ); + final expectedInnerCreatedAt = + (await event.unWrap(userTradeKeyPair.private)) + .createdAt! + .millisecondsSinceEpoch; + + await restoreService.onDataForTesting(event); + + when(restoreRef.read(isRestoringProvider)).thenReturn(false); + await restoreService.flushRestoreBufferForTesting(); + + expect(addedMessages, hasLength(1)); + final replayedMessage = addedMessages.first[1] as MostroMessage; + expect(replayedMessage.timestamp, expectedInnerCreatedAt); + expect( + replayedMessage.timestamp, + isNot(event.createdAt!.millisecondsSinceEpoch), + reason: + 'the outer gift-wrap timestamp is NIP-59 randomized and must not be used for ordering', + ); + }); + }); } // Testable service that captures publishOrder calls From ab51d7c27ee033ce5d89e5ec64705d824ed64e92 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Tue, 14 Jul 2026 13:49:04 -0600 Subject: [PATCH 12/13] fix(storage): make addMessage's write-once check atomic hasItem and put were separate awaited calls, leaving a window where two concurrent addMessage calls for the same key (e.g. the same event redelivered by a second relay) could both pass the existence check before either write landed, breaking the first-write-wins guarantee receivedAt depends on. Wraps both inside a single db.transaction so the check and write can't be interleaved by another call. --- lib/data/repositories/mostro_storage.dart | 31 ++++++++++++------- .../repositories/mostro_storage_test.dart | 27 ++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/lib/data/repositories/mostro_storage.dart b/lib/data/repositories/mostro_storage.dart index c4545710..85752fe2 100644 --- a/lib/data/repositories/mostro_storage.dart +++ b/lib/data/repositories/mostro_storage.dart @@ -14,17 +14,26 @@ class MostroStorage extends BaseStorage { Future addMessage(String key, MostroMessage message) async { final id = key; try { - if (await hasItem(id)) return; - // Add metadata for easier querying - final Map dbMap = message.toJson(); - message.timestamp ??= DateTime.now().millisecondsSinceEpoch; - dbMap['timestamp'] = message.timestamp; - dbMap['receivedAt'] = message.receivedAt; - - await store.record(id).put(db, dbMap); - logger.i( - 'Saved message of type ${message.action} with order id ${message.id}', - ); + // The existence check and the write happen inside the same + // transaction so a concurrent addMessage for the same key (e.g. the + // same event redelivered by a second relay) can't slip past the + // check before the first call finishes writing. + final wrote = await db.transaction((txn) async { + if (await store.record(id).exists(txn)) return false; + // Add metadata for easier querying + final Map dbMap = message.toJson(); + message.timestamp ??= DateTime.now().millisecondsSinceEpoch; + dbMap['timestamp'] = message.timestamp; + dbMap['receivedAt'] = message.receivedAt; + + await store.record(id).put(txn, dbMap); + return true; + }); + if (wrote) { + logger.i( + 'Saved message of type ${message.action} with order id ${message.id}', + ); + } } catch (e, stack) { logger.e( 'addMessage failed for $id', diff --git a/test/data/repositories/mostro_storage_test.dart b/test/data/repositories/mostro_storage_test.dart index 058261b3..d69dd1bc 100644 --- a/test/data/repositories/mostro_storage_test.dart +++ b/test/data/repositories/mostro_storage_test.dart @@ -34,5 +34,32 @@ void main() { expect(reloaded, isNotNull); expect(reloaded!.receivedAt, 1000); }); + + test( + 'concurrent addMessage calls for the same key: only the first-started ' + 'write is retained, the second is a no-op', () async { + final first = MostroMessage( + action: Action.fiatSent, + id: 'order-3', + receivedAt: 1111, + ); + final second = MostroMessage( + action: Action.fiatSent, + id: 'order-3', + receivedAt: 2222, + ); + + // Neither call is awaited before the other starts, so their + // exists-check-then-write sequences genuinely overlap; the + // transaction wrapping addMessage is what keeps this deterministic. + final firstCall = storage.addMessage('key-3', first); + final secondCall = storage.addMessage('key-3', second); + await Future.wait([firstCall, secondCall]); + + final reloaded = await storage.getLatestMessageById('order-3'); + + expect(reloaded, isNotNull); + expect(reloaded!.receivedAt, 1111); + }); }); } From 67d95b89c5ed36100c079b7cb0c94532bb81c761 Mon Sep 17 00:00:00 2001 From: bracr10 Date: Tue, 14 Jul 2026 16:13:23 -0600 Subject: [PATCH 13/13] fix(notifications): clarify freshness-gate log wording and level "Message freshness check failed" logged at warning level read like an error to a reviewer, when skipping a notification for a non-live (e.g. restore-reconstructed) message is the expected, common case. Reworded to "Notification skipped/eligible" and dropped to info level. --- lib/features/order/notifiers/abstract_mostro_notifier.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index 97ca8735..12ff73ec 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -116,13 +116,13 @@ class AbstractMostroNotifier extends StateNotifier { .subtract(const Duration(seconds: 60)) .millisecondsSinceEpoch) { logger.i( - 'Message freshness check passed, calling handleEvent for ${msg.action}'); + 'Notification eligible for ${msg.action}: live event, calling handleEvent'); unawaited(handleEvent(msg, previousStatus: previousStatus, wasUserInitiatedCancel: wasUserInitiatedCancel)); } else { - logger.w( - 'Message freshness check failed for ${msg.action}. ReceivedAt: ${msg.receivedAt}, Current: ${DateTime.now().millisecondsSinceEpoch}, Threshold: ${DateTime.now().subtract(const Duration(seconds: 60)).millisecondsSinceEpoch}'); + logger.i( + 'Notification skipped for ${msg.action}: not a live event (receivedAt: ${msg.receivedAt})'); // Handle dispute actions even if timestamp is old, since they're critical for UI state // but bypass navigation/notification side effects