From 1c6512fb1319d6629ec0c11d2d65e9c6f5201123 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 19:49:45 -0300 Subject: [PATCH 01/11] feat: defer dedup store write until after event authentication to prevent relay censorship --- lib/services/mostro_service.dart | 67 ++++-- test/services/mostro_service_dedup_test.dart | 210 +++++++++++++++++++ 2 files changed, 260 insertions(+), 17 deletions(-) create mode 100644 test/services/mostro_service_dedup_test.dart diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index c1c2d250..a4ed7d8a 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -23,6 +23,15 @@ class MostroService { Settings _settings; StreamSubscription? _ordersSubscription; + /// Event ids currently being decrypted, claimed synchronously. + /// + /// The durable dedup store cannot serve this purpose: `hasItem` is async, so + /// two relays delivering the same event can both observe it as unseen and + /// process it twice. A `Set.add` in the event-loop turn closes that window + /// without committing anything durable about an event nobody has + /// authenticated yet. + final Set _inFlightEventIds = {}; + MostroService(this.ref) : _settings = ref.read(settingsProvider); void init() { @@ -106,31 +115,47 @@ class MostroService { return false; } + /// Handles one event from the orders subscription. + /// + /// The dedup store is written *after* the event has been authenticated, and + /// that ordering is the security property. An event id is not evidence of + /// anything: a relay that has seen a genuine message can copy its id onto a + /// tampered event and deliver that first. Recording the id up front let the + /// forgery claim the slot — it would fail decryption and be dropped, but the + /// genuine event arriving behind it would then hit `hasItem` and be silently + /// discarded. That is a censorship primitive available to any relay, for + /// free, with nothing forged that has to survive a signature check. Future _onData(NostrEvent event) async { - final eventStore = ref.read(eventStorageProvider); - - if (await eventStore.hasItem(event.id!)) return; - - // Reserve event ID immediately to prevent duplicate processing from multiple relays - await eventStore.putItem(event.id!, { - 'id': event.id, - 'created_at': event.createdAt!.millisecondsSinceEpoch ~/ 1000, - }); - - final sessions = ref.read(sessionNotifierProvider); - final matchingSession = sessions.firstWhereOrNull( - (s) => s.tradeKey.public == event.recipient, - ); - if (matchingSession == null) { - logger.w('No matching session found for recipient: ${event.recipient}'); + final eventId = event.id; + if (eventId == null) { + logger.w('Ignoring event with no id'); return; } - final privateKey = matchingSession.tradeKey.private; + + final eventStore = ref.read(eventStorageProvider); + if (await eventStore.hasItem(eventId)) return; + + // Claim the id for this turn only. Released in the finally below, so a + // rejected event leaves no trace and a later genuine event with the same + // id is still processed. + if (!_inFlightEventIds.add(eventId)) return; try { + final sessions = ref.read(sessionNotifierProvider); + final matchingSession = sessions.firstWhereOrNull( + (s) => s.tradeKey.public == event.recipient, + ); + if (matchingSession == null) { + logger.w('No matching session found for recipient: ${event.recipient}'); + return; + } + final privateKey = matchingSession.tradeKey.private; + // Transport branch (§5 Phase A): v1 gift wrap (kind 1059) yields an inner // rumor whose content is the message tuple; v2 NIP-44 direct (kind 14) // decrypts straight to the tuple. Both converge on jsonDecode below. + // Both paths pin the node as the author and verify a signature, so + // reaching the next line means the event is the node's. String? content; String? decryptedId; if (event.kind == 14) { @@ -150,6 +175,12 @@ class MostroService { if (content == null) return; + // Authenticated: only now does the id earn a durable slot. + await eventStore.putItem(eventId, { + 'id': eventId, + 'created_at': event.createdAt!.millisecondsSinceEpoch ~/ 1000, + }); + final result = jsonDecode(content); // Ensure result is a non-empty List before accessing elements @@ -189,6 +220,8 @@ class MostroService { await _maybeLinkChildOrder(msg, matchingSession); } catch (e) { logger.e('Error processing event', error: e); + } finally { + _inFlightEventIds.remove(eventId); } } diff --git a/test/services/mostro_service_dedup_test.dart b/test/services/mostro_service_dedup_test.dart new file mode 100644 index 00000000..4eb4d643 --- /dev/null +++ b/test/services/mostro_service_dedup_test.dart @@ -0,0 +1,210 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/session.dart'; +import 'package:mostro_mobile/data/repositories/event_storage.dart'; +import 'package:mostro_mobile/data/repositories/mostro_storage.dart'; +import 'package:mostro_mobile/features/settings/settings.dart'; +import 'package:mostro_mobile/features/settings/settings_notifier.dart'; +import 'package:mostro_mobile/features/settings/settings_provider.dart'; +import 'package:mostro_mobile/features/subscriptions/subscription_manager.dart'; +import 'package:mostro_mobile/features/subscriptions/subscription_manager_provider.dart'; +import 'package:mostro_mobile/shared/notifiers/session_notifier.dart'; +import 'package:mostro_mobile/shared/providers/mostro_database_provider.dart'; +import 'package:mostro_mobile/shared/providers/mostro_service_provider.dart'; +import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; +import 'package:sembast/sembast_memory.dart'; + +/// Feeds events into the orders stream MostroService listens on. +class _StubSubscriptionManager implements SubscriptionManager { + final _orders = StreamController.broadcast(); + + @override + Stream get orders => _orders.stream; + + void emit(NostrEvent event) => _orders.add(event); + + Future close() => _orders.close(); + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +/// Serves a fixed session list; MostroService only reads the list. +class _StubSessionNotifier extends StateNotifier> + implements SessionNotifier { + _StubSessionNotifier(super.sessions); + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +/// Serves fixed settings; MostroService only reads the value. +class _StubSettingsNotifier extends StateNotifier + implements SettingsNotifier { + _StubSettingsNotifier(super.settings); + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +/// A kind-14 Mostro message, signed by [node] and addressed to [recipient]. +Future _mostroMessage( + NostrKeyPairs node, + String recipient, +) async { + final payload = jsonEncode([ + { + 'order': {'version': 2, 'action': 'fiat-sent-ok', 'id': 'order-1'} + } + ]); + + return NostrEvent.fromPartialData( + kind: 14, + content: await NostrUtils.encryptNIP44(payload, node.private, recipient), + keyPairs: node, + tags: [ + ['p', recipient], + ], + ); +} + +/// The genuine event's id and signature lifted onto different content — free +/// for any relay that has seen the original, and caught by the signature check +/// because the id no longer matches what the event says. +NostrEvent _idCollidingForgery(NostrEvent genuine) { + return NostrEvent( + id: genuine.id, + sig: genuine.sig, + pubkey: genuine.pubkey, + kind: genuine.kind, + content: 'not the content this id was signed over', + createdAt: genuine.createdAt, + tags: genuine.tags, + ); +} + +void main() { + late Database eventDb; + late Database mostroDb; + late NostrKeyPairs nodeKeys; + late Session session; + late _StubSubscriptionManager subscriptions; + late ProviderContainer container; + + setUp(() async { + eventDb = await newDatabaseFactoryMemory().openDatabase('events'); + mostroDb = await newDatabaseFactoryMemory().openDatabase('mostro'); + + nodeKeys = NostrUtils.generateKeyPair(); + session = Session( + masterKey: NostrUtils.generateKeyPair(), + tradeKey: NostrUtils.generateKeyPair(), + keyIndex: 1, + fullPrivacy: false, + startTime: DateTime.now(), + orderId: 'order-1', + ); + + subscriptions = _StubSubscriptionManager(); + + container = ProviderContainer( + overrides: [ + eventDatabaseProvider.overrideWithValue(eventDb), + mostroDatabaseProvider.overrideWithValue(mostroDb), + subscriptionManagerProvider.overrideWithValue(subscriptions), + sessionNotifierProvider.overrideWith( + (ref) => _StubSessionNotifier([session]), + ), + settingsProvider.overrideWith( + (ref) => _StubSettingsNotifier( + Settings( + relays: const ['wss://relay.example'], + fullPrivacyMode: false, + mostroPublicKey: nodeKeys.public, + ), + ), + ), + ], + ); + + container.read(mostroServiceProvider).init(); + }); + + tearDown(() async { + container.dispose(); + await subscriptions.close(); + await eventDb.close(); + await mostroDb.close(); + }); + + Future> storedMessages() => + MostroStorage(db: mostroDb).getAllMessages(); + + // The dedup store is a censorship surface for as long as it is written + // before the event is authenticated. A relay that has seen a genuine message + // can paste its id onto anything; if that claims the slot, the genuine event + // behind it is dropped as a duplicate and the user never learns of it. + test('a rejected forgery does not censor the genuine event', () async { + final genuine = await _mostroMessage(nodeKeys, session.tradeKey.public); + final eventStore = EventStorage(db: eventDb); + + subscriptions.emit(_idCollidingForgery(genuine)); + await pumpEventQueue(); + + expect( + await eventStore.hasItem(genuine.id!), + isFalse, + reason: 'an unauthenticated event must not claim a durable dedup slot', + ); + + subscriptions.emit(genuine); + await pumpEventQueue(); + + expect(await eventStore.hasItem(genuine.id!), isTrue); + expect(await storedMessages(), hasLength(1)); + }); + + test('a genuine event is still deduplicated on re-delivery', () async { + final genuine = await _mostroMessage(nodeKeys, session.tradeKey.public); + + subscriptions.emit(genuine); + await pumpEventQueue(); + subscriptions.emit(genuine); + await pumpEventQueue(); + + expect(await storedMessages(), hasLength(1)); + }); + + // Two relays delivering the same event land in separate microtasks, and + // hasItem is async, so both can observe it as unseen. The in-flight claim is + // what closes that window now that the durable write happens later. + test('concurrent delivery of the same event is processed once', () async { + final genuine = await _mostroMessage(nodeKeys, session.tradeKey.public); + + subscriptions.emit(genuine); + subscriptions.emit(genuine); + await pumpEventQueue(); + + expect(await storedMessages(), hasLength(1)); + }); + + test('an event for an unknown trade key is not recorded', () async { + final stranger = NostrUtils.generateKeyPair(); + final event = await _mostroMessage(nodeKeys, stranger.public); + + subscriptions.emit(event); + await pumpEventQueue(); + + expect(await EventStorage(db: eventDb).hasItem(event.id!), isFalse); + expect(await storedMessages(), isEmpty); + }); +} From 201f22f6a6e160e0ca34c5168f984a411846c729 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 20:00:35 -0300 Subject: [PATCH 02/11] feat: claim event ids in flight so concurrent deliveries are processed once Introduce InFlightEvents to claim an event id synchronously for the duration of its processing. Now that the durable dedup write happens after the event is authenticated, the hasItem check is asynchronous: two relays delivering the same event can both observe it as unseen and process it twice. The claim is released once processing ends, so a rejected event leaves no trace and a genuine one arriving later is still handled. Applied in MostroService and in both chat notifiers. --- .../chat/notifiers/chat_room_notifier.dart | 11 +++ .../notifiers/dispute_chat_notifier.dart | 19 ++++- lib/services/mostro_service.dart | 26 +++---- lib/shared/utils/in_flight_events.dart | 34 ++++++++ test/shared/utils/in_flight_events_test.dart | 77 +++++++++++++++++++ 5 files changed, 149 insertions(+), 18 deletions(-) create mode 100644 lib/shared/utils/in_flight_events.dart create mode 100644 test/shared/utils/in_flight_events_test.dart diff --git a/lib/features/chat/notifiers/chat_room_notifier.dart b/lib/features/chat/notifiers/chat_room_notifier.dart index 43c38d38..b928bae4 100644 --- a/lib/features/chat/notifiers/chat_room_notifier.dart +++ b/lib/features/chat/notifiers/chat_room_notifier.dart @@ -9,6 +9,7 @@ import 'package:mostro_mobile/data/models/chat_room.dart'; import 'package:mostro_mobile/data/models/nostr_event.dart'; import 'package:mostro_mobile/data/models/session.dart'; import 'package:mostro_mobile/services/chat_cursor_store.dart'; +import 'package:mostro_mobile/shared/utils/in_flight_events.dart'; import 'package:mostro_mobile/services/encrypted_image_upload_service.dart'; import 'package:mostro_mobile/services/encrypted_file_upload_service.dart'; import 'package:sembast/sembast.dart'; @@ -27,6 +28,10 @@ import 'package:mostro_mobile/shared/utils/chat_keys.dart'; import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; class ChatRoomNotifier extends StateNotifier with MediaCacheMixin { + /// Guards against concurrent re-delivery now that the durable write happens + /// after the envelope is authenticated. See [InFlightEvents]. + final InFlightEvents _inFlight = InFlightEvents(); + static final EncryptedImageUploadService _imageUploadService = EncryptedImageUploadService(); static final EncryptedFileUploadService _fileUploadService = @@ -140,6 +145,12 @@ class ChatRoomNotifier extends StateNotifier with MediaCacheMixin { Future handleChatEvent(NostrEvent event) => _onChatEvent(event); Future _onChatEvent(NostrEvent event) async { + final eventId = event.id; + if (eventId == null) return; + await _inFlight.guard(eventId, () => _processChatEvent(event)); + } + + Future _processChatEvent(NostrEvent event) async { try { if (event.kind != 14) { return; diff --git a/lib/features/disputes/notifiers/dispute_chat_notifier.dart b/lib/features/disputes/notifiers/dispute_chat_notifier.dart index 21f2d850..00275946 100644 --- a/lib/features/disputes/notifiers/dispute_chat_notifier.dart +++ b/lib/features/disputes/notifiers/dispute_chat_notifier.dart @@ -5,6 +5,7 @@ import 'package:dart_nostr/dart_nostr.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:mostro_mobile/services/logger_service.dart'; import 'package:mostro_mobile/data/models/nostr_event.dart'; +import 'package:mostro_mobile/shared/utils/in_flight_events.dart'; import 'package:mostro_mobile/data/models/session.dart'; import 'package:mostro_mobile/features/chat/providers/active_chat_screens_provider.dart'; import 'package:mostro_mobile/features/notifications/providers/notifications_provider.dart'; @@ -80,6 +81,10 @@ class DisputeChatState { /// derived from the admin ECDH shared secret. /// Stores the encrypted outer events on disk, same pattern as P2P chat. class DisputeChatNotifier extends StateNotifier with MediaCacheMixin { + /// Guards against concurrent re-delivery now that the durable write happens + /// after the envelope is authenticated. See [InFlightEvents]. + final InFlightEvents _inFlight = InFlightEvents(); + static final EncryptedImageUploadService _imageUploadService = EncryptedImageUploadService(); static final EncryptedFileUploadService _fileUploadService = @@ -202,7 +207,16 @@ class DisputeChatNotifier extends StateNotifier with MediaCach /// Handle incoming chat events via chatUnwrap. /// Stores the outer event (encrypted) to disk, then unwraps for display. - void _onChatEvent(NostrEvent event) async { + Future _onChatEvent(NostrEvent event) async { + final eventId = event.id; + if (eventId == null) return; + await _inFlight.guard(eventId, () => _processChatEvent(event, eventId)); + } + + Future _processChatEvent( + NostrEvent event, + String wrapperEventId, + ) async { try { if (!mounted || event.kind != 14) return; @@ -213,9 +227,6 @@ class DisputeChatNotifier extends StateNotifier with MediaCach final chatKeys = _getChatKeys(session); if (event.pubkey != chatKeys.sign.public) return; - // Check for duplicate outer events (relay re-deliveries) - final wrapperEventId = event.id; - if (wrapperEventId == null) return; // Already on disk means a relay re-delivery, an own echo, or an event // the background service stored while the app slept. Keep processing: // state is keyed by inner id, so only the write is redundant. diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index a4ed7d8a..28118b9b 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -15,6 +15,8 @@ import 'package:mostro_mobile/features/settings/settings_provider.dart'; import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; import 'package:mostro_mobile/features/key_manager/key_manager_provider.dart'; import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; +import 'package:mostro_mobile/data/repositories/event_storage.dart'; +import 'package:mostro_mobile/shared/utils/in_flight_events.dart'; import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; class MostroService { @@ -23,14 +25,9 @@ class MostroService { Settings _settings; StreamSubscription? _ordersSubscription; - /// Event ids currently being decrypted, claimed synchronously. - /// - /// The durable dedup store cannot serve this purpose: `hasItem` is async, so - /// two relays delivering the same event can both observe it as unseen and - /// process it twice. A `Set.add` in the event-loop turn closes that window - /// without committing anything durable about an event nobody has - /// authenticated yet. - final Set _inFlightEventIds = {}; + /// Guards against concurrent re-delivery while the durable dedup write waits + /// for authentication. See [InFlightEvents]. + final InFlightEvents _inFlight = InFlightEvents(); MostroService(this.ref) : _settings = ref.read(settingsProvider); @@ -135,11 +132,14 @@ class MostroService { final eventStore = ref.read(eventStorageProvider); if (await eventStore.hasItem(eventId)) return; - // Claim the id for this turn only. Released in the finally below, so a - // rejected event leaves no trace and a later genuine event with the same - // id is still processed. - if (!_inFlightEventIds.add(eventId)) return; + await _inFlight.guard(eventId, () => _process(event, eventId, eventStore)); + } + Future _process( + NostrEvent event, + String eventId, + EventStorage eventStore, + ) async { try { final sessions = ref.read(sessionNotifierProvider); final matchingSession = sessions.firstWhereOrNull( @@ -220,8 +220,6 @@ class MostroService { await _maybeLinkChildOrder(msg, matchingSession); } catch (e) { logger.e('Error processing event', error: e); - } finally { - _inFlightEventIds.remove(eventId); } } diff --git a/lib/shared/utils/in_flight_events.dart b/lib/shared/utils/in_flight_events.dart new file mode 100644 index 00000000..fc2858ec --- /dev/null +++ b/lib/shared/utils/in_flight_events.dart @@ -0,0 +1,34 @@ +/// Claims event ids for the duration of their processing. +/// +/// Every path that consumes relay events deduplicates against a durable store, +/// but that store must only ever record events that have been authenticated — +/// an id is a relay's claim, not evidence, and writing one before the +/// signature is checked lets a forged copy take the slot and censor the +/// genuine event behind it. +/// +/// Moving the durable write after authentication reopens a smaller window: +/// `hasItem` is asynchronous, so two relays delivering the same event can both +/// observe it as unseen and process it twice. This closes that window without +/// persisting anything, by claiming the id synchronously for the turn and +/// releasing it once processing ends — so a rejected event leaves no trace and +/// a genuine one arriving later is still handled. +class InFlightEvents { + final Set _ids = {}; + + /// Runs [action] unless [eventId] is already in flight, in which case this + /// is a concurrent re-delivery and does nothing. + /// + /// The claim is released even if [action] throws, so a failure cannot leave + /// an id permanently blocked. + Future guard(String eventId, Future Function() action) async { + if (!_ids.add(eventId)) return; + try { + await action(); + } finally { + _ids.remove(eventId); + } + } + + /// Whether [eventId] is currently being processed. For tests and debugging. + bool isInFlight(String eventId) => _ids.contains(eventId); +} diff --git a/test/shared/utils/in_flight_events_test.dart b/test/shared/utils/in_flight_events_test.dart new file mode 100644 index 00000000..08514dd3 --- /dev/null +++ b/test/shared/utils/in_flight_events_test.dart @@ -0,0 +1,77 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/shared/utils/in_flight_events.dart'; + +void main() { + late InFlightEvents inFlight; + + setUp(() => inFlight = InFlightEvents()); + + test('runs the action for an id that is not in flight', () async { + var ran = 0; + await inFlight.guard('a', () async => ran++); + + expect(ran, 1); + }); + + // The window this exists to close: two relays deliver the same event, both + // start processing before either has written anything durable. + test('a concurrent claim on the same id does not run twice', () async { + var ran = 0; + final gate = Completer(); + + final first = inFlight.guard('a', () async { + ran++; + await gate.future; + }); + final second = inFlight.guard('a', () async => ran++); + + await second; + expect(ran, 1, reason: 'the second delivery must be dropped'); + + gate.complete(); + await first; + expect(ran, 1); + }); + + test('the claim is released once processing ends', () async { + await inFlight.guard('a', () async {}); + + expect(inFlight.isInFlight('a'), isFalse); + + var ran = 0; + await inFlight.guard('a', () async => ran++); + expect(ran, 1, reason: 'a later delivery of the same id still runs'); + }); + + // A rejected event must leave no trace, or one failure would block that id + // for the lifetime of the process — a censorship primitive of its own. + test('the claim is released when the action throws', () async { + await expectLater( + inFlight.guard('a', () async => throw StateError('rejected')), + throwsStateError, + ); + + expect(inFlight.isInFlight('a'), isFalse); + + var ran = 0; + await inFlight.guard('a', () async => ran++); + expect(ran, 1); + }); + + test('distinct ids do not block each other', () async { + final ran = []; + final gate = Completer(); + + final first = inFlight.guard('a', () async { + ran.add('a'); + await gate.future; + }); + await inFlight.guard('b', () async => ran.add('b')); + + expect(ran, ['a', 'b']); + gate.complete(); + await first; + }); +} From 64e3ed4f3375e1a782a5886cc56a390d5c14a1d6 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 20:50:17 -0300 Subject: [PATCH 03/11] fix: recompute event id before signature verification to prevent metadata impersonation Replace `isVerified()` with `NostrUtils.isValidEventSignature` in MostroNodesNotifier and DeepLinkService. The former checks the signature against the event's self-declared id without recomputing it from the serialized event, so a genuine (id, sig, pubkey) triple lifted onto attacker-chosen content still passes. A relay can serve this to impersonate a trusted node in the picker or inject arbitrary order parameters into a deep link. --- .../mostro/mostro_nodes_notifier.dart | 25 ++++- lib/services/deep_link_service.dart | 15 ++- .../mostro/mostro_nodes_notifier_test.dart | 43 +++++++- .../node_metadata_verification_test.dart | 103 ++++++++++++++++++ 4 files changed, 172 insertions(+), 14 deletions(-) create mode 100644 test/features/mostro/node_metadata_verification_test.dart diff --git a/lib/features/mostro/mostro_nodes_notifier.dart b/lib/features/mostro/mostro_nodes_notifier.dart index 3f3ec231..8f1b233e 100644 --- a/lib/features/mostro/mostro_nodes_notifier.dart +++ b/lib/features/mostro/mostro_nodes_notifier.dart @@ -6,6 +6,7 @@ import 'package:mostro_mobile/data/models/enums/storage_keys.dart'; import 'package:mostro_mobile/features/mostro/mostro_node.dart'; import 'package:mostro_mobile/features/settings/settings_provider.dart'; import 'package:mostro_mobile/services/logger_service.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; import 'package:mostro_mobile/shared/providers/nostr_service_provider.dart'; import 'package:shared_preferences/shared_preferences.dart'; @@ -270,12 +271,28 @@ class MostroNodesNotifier extends StateNotifier> { void _applyMetadataFromEvent(NostrEvent event) { try { - if (!event.isVerified()) { + // Fail closed. The previous rule applied metadata even when + // verification failed, reasoning that the author filter had already + // vouched for the event — but a filter is a request, not a guarantee: + // the relay decides what to answer with, and nothing stops it from + // returning an event it wrote itself. This metadata is the name and + // avatar the user reads when choosing which node to trade against, so + // an unverified one is a free impersonation of a trusted node. + // + // `NostrUtils.isValidEventSignature` rather than `isVerified()`: the + // latter checks the signature against the event's self-declared id and + // never recomputes it, so a genuine (id, sig, pubkey) triple lifted onto + // attacker-chosen content still passes. + if (!NostrUtils.isValidEventSignature(event)) { logger.w( - 'Kind 0 event for ${event.pubkey} failed signature verification ' - '(may be a dart_nostr limitation). Applying metadata anyway since ' - 'the event was fetched by author filter.', + 'Rejecting kind 0 metadata claiming to be from ${event.pubkey}: ' + 'signature verification failed', ); + return; + } + if (event.kind != 0) { + logger.w('Ignoring non-kind-0 event as node metadata: ${event.kind}'); + return; } final json = jsonDecode(event.content ?? '') as Map; updateNodeMetadata( diff --git a/lib/services/deep_link_service.dart b/lib/services/deep_link_service.dart index e9a8e690..cce354d1 100644 --- a/lib/services/deep_link_service.dart +++ b/lib/services/deep_link_service.dart @@ -155,16 +155,19 @@ class DeepLinkService { } // Helper to build the candidate list from a set of raw events. - // When mostroPubkey is present only events authored by that node are - // accepted. isVerified() failures are logged but not treated as hard - // rejections due to a known dart_nostr limitation (consistent with - // how mostro_nodes_notifier.dart handles kind-0 events). + // + // `NostrUtils.isValidEventSignature` rather than `isVerified()`: the + // latter checks the signature against the event's self-declared id and + // never recomputes it from the serialized event, so a genuine + // (id, sig, pubkey) triple lifted onto attacker-chosen tags still + // passes — and the order type this link opens is read straight out of + // those tags. List buildCandidates(List raw) { if (mostroPubkey == null) return raw; return raw.where((e) { - if (!e.isVerified()) { + if (!NostrUtils.isValidEventSignature(e)) { logger.w( - 'Event \${e.id} from pubkey \${e.pubkey} failed signature ' + 'Event ${e.id} from pubkey ${e.pubkey} failed signature ' 'verification — rejecting to prevent spoofing.', ); return false; diff --git a/test/features/mostro/mostro_nodes_notifier_test.dart b/test/features/mostro/mostro_nodes_notifier_test.dart index 30f08f8c..f97668db 100644 --- a/test/features/mostro/mostro_nodes_notifier_test.dart +++ b/test/features/mostro/mostro_nodes_notifier_test.dart @@ -571,9 +571,16 @@ void main() { expect(notifier.state.length, stateBefore.length); }); - test('fetchAllNodeMetadata applies unverified events with warning', () async { + // The author filter is a request, not a guarantee — the relay decides + // what it answers with. This metadata is the name and avatar shown when + // the user picks a node to trade against, so applying an unverified one + // hands out free impersonation of a trusted node. + test('fetchAllNodeMetadata rejects an unverified event', () async { final notifier = createNotifier(); await notifier.init(); + final nameBefore = notifier.state + .firstWhere((n) => n.pubkey == trustedPubkey) + .name; when(mockNostrService.fetchEvents(any, specificRelays: anyNamed('specificRelays'))) .thenAnswer((_) async => [ @@ -590,11 +597,39 @@ void main() { await notifier.fetchAllNodeMetadata(); - // Metadata is applied even if signature verification fails, - // since events are fetched by author filter final node = notifier.state.firstWhere((n) => n.pubkey == trustedPubkey); - expect(node.name, 'Unverified Name'); + expect(node.name, nameBefore); + }); + + // What `isVerified()` lets through: the signature and id are genuinely + // the node's, but the id was never recomputed over this content. + test('fetchAllNodeMetadata rejects a content swap under a real signature', + () async { + final notifier = await createNotifierWithTestNode(); + final genuine = makeKind0Event({'name': 'Real Node'}); + final nameBefore = notifier.state + .firstWhere((n) => n.pubkey == genuine.pubkey) + .name; + + when(mockNostrService.fetchEvents(any, specificRelays: anyNamed('specificRelays'))) + .thenAnswer((_) async => [ + NostrEvent( + id: genuine.id, + sig: genuine.sig, + pubkey: genuine.pubkey, + kind: genuine.kind, + content: jsonEncode({'name': 'Mostro Official'}), + createdAt: genuine.createdAt, + tags: genuine.tags, + ), + ]); + + await notifier.fetchAllNodeMetadata(); + + final node = + notifier.state.firstWhere((n) => n.pubkey == genuine.pubkey); + expect(node.name, nameBefore); }); test('fetchNodeMetadata deduplicates keeping latest when relays return multiple events', diff --git a/test/features/mostro/node_metadata_verification_test.dart b/test/features/mostro/node_metadata_verification_test.dart new file mode 100644 index 00000000..fe3a2315 --- /dev/null +++ b/test/features/mostro/node_metadata_verification_test.dart @@ -0,0 +1,103 @@ +import 'dart:convert'; + +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; + +/// A kind-0 metadata event as a node publishes it. +NostrEvent _metadata(NostrKeyPairs keys, Map profile) { + return NostrEvent.fromPartialData( + kind: 0, + content: jsonEncode(profile), + keyPairs: keys, + tags: const [], + ); +} + +/// Keeps the genuine (id, sig, pubkey) triple and swaps the content — exactly +/// what a relay can do for free, and exactly what `isVerified()` misses. +NostrEvent _contentSwapped(NostrEvent genuine, Map profile) { + return NostrEvent( + id: genuine.id, + sig: genuine.sig, + pubkey: genuine.pubkey, + kind: genuine.kind, + content: jsonEncode(profile), + createdAt: genuine.createdAt, + tags: genuine.tags, + ); +} + +void main() { + late NostrKeyPairs nodeKeys; + + setUp(() => nodeKeys = NostrUtils.generateKeyPair()); + + group('kind-0 node metadata verification', () { + test('a genuine metadata event verifies', () { + final event = _metadata(nodeKeys, { + 'name': 'mostro', + 'picture': 'https://example.test/a.png', + }); + + expect(NostrUtils.isValidEventSignature(event), isTrue); + }); + + // The gap MM-015 names. This is the event a relay serves to make a node it + // controls read as a trusted one in the node picker: the signature is real + // and belongs to the node, but it was never made over this content. + test('a content swap keeping the genuine signature is rejected', () { + final genuine = _metadata(nodeKeys, {'name': 'mostro'}); + final swapped = _contentSwapped(genuine, { + 'name': 'mostro (official)', + 'picture': 'https://attacker.test/logo.png', + }); + + expect( + swapped.isVerified(), + isTrue, + reason: 'isVerified only checks the sig against the declared id, ' + 'which is why it cannot be the gate here', + ); + expect( + NostrUtils.isValidEventSignature(swapped), + isFalse, + reason: 'recomputing the id binds the signature to the content', + ); + }); + + test('a tag swap keeping the genuine signature is rejected', () { + final genuine = _metadata(nodeKeys, {'name': 'mostro'}); + final retagged = NostrEvent( + id: genuine.id, + sig: genuine.sig, + pubkey: genuine.pubkey, + kind: genuine.kind, + content: genuine.content, + createdAt: genuine.createdAt, + tags: const [ + ['k', 'sell'], + ], + ); + + expect(retagged.isVerified(), isTrue); + expect(NostrUtils.isValidEventSignature(retagged), isFalse); + }); + + test('an event signed by someone else is rejected', () { + final attacker = NostrUtils.generateKeyPair(); + final genuine = _metadata(nodeKeys, {'name': 'mostro'}); + final forged = NostrEvent( + id: genuine.id, + sig: genuine.sig, + pubkey: attacker.public, + kind: genuine.kind, + content: genuine.content, + createdAt: genuine.createdAt, + tags: genuine.tags, + ); + + expect(NostrUtils.isValidEventSignature(forged), isFalse); + }); + }); +} From bddfadae273fd9b9404b659b77bb8cb4ee68c40b Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 20:55:27 -0300 Subject: [PATCH 04/11] fix: normalize all timestamps to milliseconds at model boundary to prevent dispute ordering corruption --- lib/data/models/mostro_message.dart | 23 ++++- lib/data/models/order.dart | 3 + lib/features/order/models/order_state.dart | 9 +- lib/features/restore/restore_manager.dart | 14 ++- .../models/mostro_message_timestamp_test.dart | 90 +++++++++++++++++++ 5 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 test/data/models/mostro_message_timestamp_test.dart diff --git a/lib/data/models/mostro_message.dart b/lib/data/models/mostro_message.dart index 6ffb0b1f..9b14fe3c 100644 --- a/lib/data/models/mostro_message.dart +++ b/lib/data/models/mostro_message.dart @@ -15,6 +15,15 @@ class MostroMessage { final Action action; int? tradeIndex; T? _payload; + + /// Milliseconds since the Unix epoch. + /// + /// One unit for this field, everywhere. It is read back from two sources + /// that do not agree natively — the local store writes milliseconds, while + /// anything arriving over the wire follows the Nostr convention of seconds — + /// and mixing them silently corrupted dispute ordering in both directions + /// (see [_toMillis]). Every producer normalises here rather than at each + /// consumer. int? timestamp; MostroMessage({ @@ -45,8 +54,20 @@ class MostroMessage { return json; } + /// Normalises an epoch value to milliseconds. + /// + /// `fromJson` deserialises both the local store (milliseconds) and wire + /// payloads (seconds), so the unit has to be inferred. The threshold is not + /// a guess: 1e12 milliseconds is 2001-09-09, and 1e12 seconds is far beyond + /// any date this app will see, so no real timestamp is ambiguous. + static int? _toMillis(dynamic raw) { + final value = raw is int ? raw : (raw is num ? raw.toInt() : null); + if (value == null) return null; + return value.abs() < 1000000000000 ? value * 1000 : value; + } + factory MostroMessage.fromJson(Map json) { - final timestamp = json['timestamp']; + final timestamp = _toMillis(json['timestamp']); // 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; diff --git a/lib/data/models/order.dart b/lib/data/models/order.dart index 0e88f113..54085e2a 100644 --- a/lib/data/models/order.dart +++ b/lib/data/models/order.dart @@ -20,6 +20,9 @@ class Order implements Payload { final String? buyerTradePubkey; final String? sellerTradePubkey; final String? buyerInvoice; + /// Seconds since the Unix epoch, as the protocol sends it (Nostr + /// convention). Convert before handing it to anything that expects + /// milliseconds, including [MostroMessage.timestamp]. final int? createdAt; final int? expiresAt; diff --git a/lib/features/order/models/order_state.dart b/lib/features/order/models/order_state.dart index 000f8dc2..7cd15733 100644 --- a/lib/features/order/models/order_state.dart +++ b/lib/features/order/models/order_state.dart @@ -289,10 +289,13 @@ 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 && payloadDisputeAccepted) { - // 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 + // MostroMessage.timestamp is already milliseconds (normalised at the + // model boundary). It used to be multiplied by 1000 here under a comment + // claiming it was seconds, which pushed every live dispute's createdAt + // tens of thousands of years into the future and pinned it to the top of + // the dispute list for good. if (message.timestamp != null) { - final tsMs = message.timestamp! * 1000; + final tsMs = message.timestamp!; if (updatedDispute.createdAt == null || updatedDispute.createdAt!.millisecondsSinceEpoch != tsMs) { updatedDispute = updatedDispute.copyWith( diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 2b0d10b8..f8423d51 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -756,8 +756,13 @@ class RestoreService { disputeId: restoredDispute.disputeId, orderId: restoredDispute.orderId, status: restoredDispute.status, + // Order.createdAt is the protocol's `created_at`: seconds, per + // the Nostr convention. Reading it as milliseconds dated every + // restored dispute to January 1970. createdAt: orderDetail.createdAt != null - ? DateTime.fromMillisecondsSinceEpoch(orderDetail.createdAt!) + ? DateTime.fromMillisecondsSinceEpoch( + orderDetail.createdAt! * 1000, + ) : DateTime.now(), action: userInitiated ? 'dispute-initiated-by-you' @@ -802,9 +807,10 @@ class RestoreService { id: orderDetail.id, action: action, payload: dispute, - timestamp: - orderDetail.createdAt ?? - DateTime.now().millisecondsSinceEpoch, + // Seconds on the wire, milliseconds in MostroMessage. + timestamp: orderDetail.createdAt != null + ? orderDetail.createdAt! * 1000 + : DateTime.now().millisecondsSinceEpoch, ); // Save dispute message to storage diff --git a/test/data/models/mostro_message_timestamp_test.dart b/test/data/models/mostro_message_timestamp_test.dart new file mode 100644 index 00000000..1edfc3ee --- /dev/null +++ b/test/data/models/mostro_message_timestamp_test.dart @@ -0,0 +1,90 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/dispute.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/enums/status.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/features/order/models/order_state.dart'; + +void main() { + // One unit, milliseconds, normalised where the value enters the model. + // Getting this wrong is not cosmetic: dispute ordering reads createdAt, so a + // corrupted value pins a dispute to the top of the list permanently. + group('MostroMessage.timestamp normalisation', () { + final millis = DateTime(2026, 3, 1).millisecondsSinceEpoch; + final seconds = millis ~/ 1000; + + Map payload(dynamic timestamp) => { + 'timestamp': timestamp, + 'action': 'fiat-sent-ok', + 'id': 'order-1', + }; + + test('a millisecond value from the local store is kept as-is', () { + expect(MostroMessage.fromJson(payload(millis)).timestamp, millis); + }); + + test('a second value from the wire is scaled to milliseconds', () { + expect(MostroMessage.fromJson(payload(seconds)).timestamp, millis); + }); + + test('an absent timestamp stays null', () { + expect(MostroMessage.fromJson(payload(null)).timestamp, isNull); + }); + + test('a non-numeric timestamp is dropped rather than throwing', () { + expect(MostroMessage.fromJson(payload('yesterday')).timestamp, isNull); + }); + + test('both representations of one instant agree', () { + expect( + MostroMessage.fromJson(payload(seconds)).timestamp, + MostroMessage.fromJson(payload(millis)).timestamp, + ); + }); + }); + + group('dispute createdAt derived from a message timestamp', () { + test('lands on the instant the message carries', () { + final at = DateTime(2026, 3, 1); + final state = OrderState( + status: Status.dispute, + action: Action.disputeInitiatedByPeer, + order: null, + dispute: Dispute(disputeId: 'd-1', orderId: 'order-1'), + ); + + final updated = state.updateWith( + MostroMessage( + id: 'order-1', + action: Action.disputeInitiatedByPeer, + payload: Dispute(disputeId: 'd-1', orderId: 'order-1'), + timestamp: at.millisecondsSinceEpoch, + ), + ); + + expect(updated.dispute!.createdAt, at); + }); + + // The regression MM-036 describes: multiplying an already-millisecond + // value by 1000 dated disputes tens of thousands of years out. + test('does not land in the far future', () { + final state = OrderState( + status: Status.dispute, + action: Action.disputeInitiatedByPeer, + order: null, + dispute: Dispute(disputeId: 'd-1', orderId: 'order-1'), + ); + + final updated = state.updateWith( + MostroMessage( + id: 'order-1', + action: Action.disputeInitiatedByPeer, + payload: Dispute(disputeId: 'd-1', orderId: 'order-1'), + timestamp: DateTime(2026, 3, 1).millisecondsSinceEpoch, + ), + ); + + expect(updated.dispute!.createdAt!.year, lessThan(2100)); + }); + }); +} From 5677b8b91110482c469ce4734540e290a6ca2da2 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 20:59:33 -0300 Subject: [PATCH 05/11] feat: extract message timestamp from node-signed created_at to prevent replay freshness forgery Capture the kind-14 `created_at` into `MostroMessage.timestamp` before storage. The outer event's timestamp is covered by the node's signature and recomputed id, so a relay can withhold a message but cannot make an old one look new. This prevents a replayed fiat-sent-ok from re-arming the Release button with a current receive time while the node-signed clock shows it is weeks stale. The v1 NIP-59 path continues to fall back to receive time because the wrap and seal deliberately randomise their timestamps. --- lib/data/repositories/mostro_storage.dart | 5 ++ lib/services/mostro_service.dart | 20 ++++++ test/services/mostro_service_dedup_test.dart | 66 +++++++++++++++++++- 3 files changed, 89 insertions(+), 2 deletions(-) diff --git a/lib/data/repositories/mostro_storage.dart b/lib/data/repositories/mostro_storage.dart index 659180da..ab91930c 100644 --- a/lib/data/repositories/mostro_storage.dart +++ b/lib/data/repositories/mostro_storage.dart @@ -17,6 +17,11 @@ class MostroStorage extends BaseStorage { if (await hasItem(id)) return; // Add metadata for easier querying final Map dbMap = message.toJson(); + // Receive time, and only as a last resort. This is not evidence of when + // the node spoke — a relay chooses when to deliver — so it is a + // placeholder for messages that carry no signed clock (the v1 gift-wrap + // path, and locally synthesised messages). v2 messages arrive with + // `timestamp` already set from the node-signed `created_at`. message.timestamp ??= DateTime.now().millisecondsSinceEpoch; dbMap['timestamp'] = message.timestamp; diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index 28118b9b..c6c2e6b3 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -204,6 +204,26 @@ class MostroService { final msg = MostroMessage.fromJson(result[0]); + // Freshness comes from the wire, not from when this device happened to + // hear about it. + // + // On v2 the kind-14 `created_at` is covered by the node's signature — + // the id is recomputed over it and the signature verified above — so it + // is the node stating when it said this, and a relay cannot move it. + // That makes it the only trustworthy clock in the protocol, and it + // exists only here: NIP-59 deliberately randomises the wrap and seal + // timestamps to avoid leaking timing, so the v1 path has nothing + // equivalent and keeps falling back to receive time in MostroStorage. + // + // It also outranks any `timestamp` inside the decrypted payload, which + // no signature covers independently. + if (event.kind == 14) { + final signedAt = event.createdAt; + if (signedAt != null) { + msg.timestamp = signedAt.millisecondsSinceEpoch; + } + } + final messageStorage = ref.read(mostroStorageProvider); // Use the inner rumor id if available (v1), otherwise fall back to the diff --git a/test/services/mostro_service_dedup_test.dart b/test/services/mostro_service_dedup_test.dart index 4eb4d643..23a69a61 100644 --- a/test/services/mostro_service_dedup_test.dart +++ b/test/services/mostro_service_dedup_test.dart @@ -59,8 +59,9 @@ class _StubSettingsNotifier extends StateNotifier /// A kind-14 Mostro message, signed by [node] and addressed to [recipient]. Future _mostroMessage( NostrKeyPairs node, - String recipient, -) async { + String recipient, { + DateTime? createdAt, +}) async { final payload = jsonEncode([ { 'order': {'version': 2, 'action': 'fiat-sent-ok', 'id': 'order-1'} @@ -71,6 +72,7 @@ Future _mostroMessage( kind: 14, content: await NostrUtils.encryptNIP44(payload, node.private, recipient), keyPairs: node, + createdAt: createdAt, tags: [ ['p', recipient], ], @@ -207,4 +209,64 @@ void main() { expect(await EventStorage(db: eventDb).hasItem(event.id!), isFalse); expect(await storedMessages(), isEmpty); }); + + // MM-021: freshness has to come from the wire. The kind-14 created_at is + // covered by the node's signature, so a relay can withhold a message but + // cannot make an old one look new — which is exactly what a replayed + // fiat-sent-ok needs in order to re-arm the seller's Release button. + group('message freshness', () { + test('the stored timestamp is the node-signed created_at', () async { + final signedAt = DateTime.now().subtract(const Duration(days: 3)); + final event = await _mostroMessage( + nodeKeys, + session.tradeKey.public, + createdAt: signedAt, + ); + + subscriptions.emit(event); + await pumpEventQueue(); + + final stored = await storedMessages(); + expect(stored, hasLength(1)); + // Tolerance of a second: Nostr serialises created_at in whole seconds, + // so an event that has crossed the wire loses the sub-second part while + // one built in-process keeps it. + expect( + stored.single.timestamp, + closeTo(signedAt.millisecondsSinceEpoch, 1000), + ); + }); + + test('a replayed old message does not read as fresh', () async { + final event = await _mostroMessage( + nodeKeys, + session.tradeKey.public, + createdAt: DateTime.now().subtract(const Duration(days: 30)), + ); + + subscriptions.emit(event); + await pumpEventQueue(); + + final stored = await storedMessages(); + final age = DateTime.now().millisecondsSinceEpoch - + stored.single.timestamp!; + expect( + age, + greaterThan(const Duration(days: 29).inMilliseconds), + reason: 'receive time would have made this look seconds old', + ); + }); + + test('a fresh message keeps a current timestamp', () async { + final event = await _mostroMessage(nodeKeys, session.tradeKey.public); + + subscriptions.emit(event); + await pumpEventQueue(); + + final stored = await storedMessages(); + final age = DateTime.now().millisecondsSinceEpoch - + stored.single.timestamp!; + expect(age, lessThan(const Duration(minutes: 1).inMilliseconds)); + }); + }); } From e44ca6314a2b84d70963dcd6a177d686ed8313b4 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 21:08:13 -0300 Subject: [PATCH 06/11] feat: bound kind-14 created_at to prevent future-dated and ancient messages from being accepted --- lib/shared/utils/nostr_utils.dart | 46 ++++++++ .../utils/nip59_authentication_test.dart | 108 ++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/lib/shared/utils/nostr_utils.dart b/lib/shared/utils/nostr_utils.dart index 3d55787d..d42f264a 100644 --- a/lib/shared/utils/nostr_utils.dart +++ b/lib/shared/utils/nostr_utils.dart @@ -8,6 +8,30 @@ import 'package:dart_nostr/dart_nostr.dart'; import 'package:elliptic/elliptic.dart'; import 'package:nip44/nip44.dart'; +/// How far into the future a node-signed `created_at` may sit before the +/// message is rejected. +/// +/// Only the node can produce a future-dated message — the timestamp is inside +/// the signature — so this is not a defence against relays. It bounds a +/// misconfigured or hostile node that dates messages forward so they never +/// age out. Deliberately loose: the cost of a false positive is a trade update +/// silently refused, and phone clocks do drift. +const Duration kMostroMessageMaxClockSkew = Duration(minutes: 15); + +/// How old a node-signed message may be before it is refused outright. +/// +/// A sanity bound, not the replay defence. Replayed messages are authentic and +/// carry their real (old) timestamp, so a relay serving a genuine message from +/// last week passes this easily — what stops it acting on stale state is the +/// per-session high-water mark, which refuses anything older than the newest +/// message already accepted for that session. +/// +/// The window has to stay wide because the orders subscription carries no +/// `since`: reconnecting replays a trade's history, and that history is how +/// state is rebuilt. Cutting it short would not block an attack, it would +/// erase the user's own trades. +const Duration kMostroMessageMaxAge = Duration(days: 90); + class NostrUtils { static final Nostr _instance = Nostr.instance; @@ -488,6 +512,9 @@ class NostrUtils { NostrEvent event, String privateKey, { required String expectedAuthor, + Duration maxAge = kMostroMessageMaxAge, + Duration maxClockSkew = kMostroMessageMaxClockSkew, + DateTime? now, }) async { if (event.kind != 14) { throw ArgumentError('Wrong kind: ${event.kind}'); @@ -507,6 +534,25 @@ class NostrUtils { throw ArgumentError('Invalid kind-14 event signature'); } + // The signature covers `created_at`, so past this point it is the node + // saying when it spoke, and a relay can no longer move it. Bound it in + // both directions before the payload is trusted. + final createdAt = event.createdAt; + if (createdAt == null) { + throw ArgumentError('Kind-14 event has no created_at'); + } + final clock = now ?? DateTime.now(); + if (createdAt.isAfter(clock.add(maxClockSkew))) { + throw ArgumentError( + 'Kind-14 event is dated in the future: $createdAt', + ); + } + if (clock.difference(createdAt) > maxAge) { + throw ArgumentError( + 'Kind-14 event is older than $maxAge: $createdAt', + ); + } + try { return await decryptNIP44( event.content!, diff --git a/test/shared/utils/nip59_authentication_test.dart b/test/shared/utils/nip59_authentication_test.dart index fbd5af2f..c4ef08f4 100644 --- a/test/shared/utils/nip59_authentication_test.dart +++ b/test/shared/utils/nip59_authentication_test.dart @@ -173,4 +173,112 @@ void main() { ); }); }); + + // MM-021: the kind-14 created_at is inside the signature, so past + // verification it is the node's own statement of when it spoke. Bounding it + // is cheap and rules out the impossible; it is not the replay defence, since + // a replayed message carries its real, in-range timestamp. + group('decryptNIP44DirectEvent temporal bounds', () { + Future directMessage({DateTime? createdAt}) async { + return NostrEvent.fromPartialData( + kind: 14, + content: await NostrUtils.encryptNIP44( + '[{"order":{"action":"fiat-sent-ok"}}]', + node.private, + recipient.public, + ), + keyPairs: node, + createdAt: createdAt, + tags: [ + ['p', recipient.public], + ], + ); + } + + test('accepts a message dated now', () async { + final event = await directMessage(); + + final content = await NostrUtils.decryptNIP44DirectEvent( + event, + recipient.private, + expectedAuthor: node.public, + ); + + expect(content, contains('fiat-sent-ok')); + }); + + test('rejects a message dated beyond the clock skew', () async { + final event = await directMessage( + createdAt: DateTime.now().add(const Duration(hours: 2)), + ); + + await expectLater( + NostrUtils.decryptNIP44DirectEvent( + event, + recipient.private, + expectedAuthor: node.public, + ), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('dated in the future'), + ), + ), + ); + }); + + test('tolerates a small clock difference', () async { + final event = await directMessage( + createdAt: DateTime.now().add(const Duration(minutes: 2)), + ); + + await expectLater( + NostrUtils.decryptNIP44DirectEvent( + event, + recipient.private, + expectedAuthor: node.public, + ), + completes, + ); + }); + + test('rejects a message older than the max age', () async { + final event = await directMessage( + createdAt: DateTime.now().subtract(const Duration(days: 120)), + ); + + await expectLater( + NostrUtils.decryptNIP44DirectEvent( + event, + recipient.private, + expectedAuthor: node.public, + ), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('older than'), + ), + ), + ); + }); + + // Trade history has to survive a reconnect: the orders subscription has no + // `since`, so refusing older messages would erase the user's own trades. + test('still accepts trade history from weeks ago', () async { + final event = await directMessage( + createdAt: DateTime.now().subtract(const Duration(days: 21)), + ); + + await expectLater( + NostrUtils.decryptNIP44DirectEvent( + event, + recipient.private, + expectedAuthor: node.public, + ), + completes, + ); + }); + }); } From 4dc86fd18fc512c21fe84c9b8499ecfe1c700886 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 21:13:39 -0300 Subject: [PATCH 07/11] feat: reject messages older than applied state to prevent replay attacks from modifying settled orders --- .../notifiers/abstract_mostro_notifier.dart | 39 ++++++ .../order/notifiers/order_notifier.dart | 8 ++ .../order/stale_message_guard_test.dart | 116 ++++++++++++++++++ 3 files changed, 163 insertions(+) create mode 100644 test/features/order/stale_message_guard_test.dart diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index 370b0910..81e12124 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -24,6 +24,34 @@ class AbstractMostroNotifier extends StateNotifier { late Session session; ProviderSubscription>? subscription; + + /// Signed timestamp of the newest message already folded into [state]. + /// + /// The high-water mark MM-021 calls for. Every message reaching this notifier + /// is authentic — a replayed one carries the node's real signature over its + /// real, old timestamp — so authentication alone cannot tell a fresh + /// instruction from an archived one. Ordering can: a message older than the + /// state it would modify is describing a past this order has already left. + /// + /// Rebuilt from storage on [sync], so it survives a restart without needing + /// its own persistence. Null until the first timestamped message arrives. + @protected + int? lastAppliedTimestamp; + + /// Whether [msg] may modify the current state. + /// + /// Fails open on a missing timestamp: the v1 gift-wrap path has no signed + /// clock (NIP-59 randomises those timestamps by design), so refusing + /// untimestamped messages would break v1 entirely rather than protect it. + /// Equal timestamps pass — a node can legitimately emit several messages in + /// one second, and exact re-deliveries are already stopped by event-id dedup. + @protected + bool supersedesAppliedState(MostroMessage msg) { + final incoming = msg.timestamp; + final applied = lastAppliedTimestamp; + if (incoming == null || applied == null) return true; + return incoming >= applied; + } final Set _processedEventIds = {}; // Timer storage for orphan session cleanup @@ -119,8 +147,19 @@ class AbstractMostroNotifier extends StateNotifier { final wasUserInitiatedCancel = msg.action == Action.canceled && _userInitiatedCancels.remove(orderId); + if (!supersedesAppliedState(msg)) { + logger.w( + 'Ignoring stale ${msg.action} for order $orderId: dated ' + '${msg.timestamp}, state already at $lastAppliedTimestamp', + ); + return; + } + if (mounted) { state = state.updateWith(msg); + if (msg.timestamp != null) { + lastAppliedTimestamp = msg.timestamp; + } } if (msg.timestamp != null && diff --git a/lib/features/order/notifiers/order_notifier.dart b/lib/features/order/notifiers/order_notifier.dart index a082d79d..947b09af 100644 --- a/lib/features/order/notifiers/order_notifier.dart +++ b/lib/features/order/notifiers/order_notifier.dart @@ -102,6 +102,14 @@ class OrderNotifier extends AbstractMostroNotifier { state = currentState; + // Re-arm the high-water mark from the folded history, so a restart does + // not reset it and let an archived message back in. The list is sorted + // by signed timestamp above, so the last entry is the newest. + final newest = messages.last.timestamp; + if (newest != null) { + lastAppliedTimestamp = newest; + } + logger.i( 'Synced order $orderId to state: ${state.status} - ${state.action}'); diff --git a/test/features/order/stale_message_guard_test.dart b/test/features/order/stale_message_guard_test.dart new file mode 100644 index 00000000..2e283657 --- /dev/null +++ b/test/features/order/stale_message_guard_test.dart @@ -0,0 +1,116 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/enums/status.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/session.dart'; +import 'package:mostro_mobile/features/order/models/order_state.dart'; +import 'package:mostro_mobile/features/order/notifiers/abstract_mostro_notifier.dart'; +import 'package:mostro_mobile/shared/notifiers/session_notifier.dart'; +import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; + +/// Exposes the guard under test. Nothing is overridden — the rule exercised +/// here is the production one. +class _Probe extends AbstractMostroNotifier { + _Probe(super.orderId, super.ref); + + bool check(MostroMessage msg) => supersedesAppliedState(msg); + + void setApplied(int? timestamp) => lastAppliedTimestamp = timestamp; +} + +class _StubSessionNotifier extends StateNotifier> + implements SessionNotifier { + _StubSessionNotifier() : super(const []); + + @override + Session? getSessionByOrderId(String orderId) => null; + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +void main() { + final at = DateTime(2026, 3, 1); + final now = at.millisecondsSinceEpoch; + final hourAgo = at.subtract(const Duration(hours: 1)).millisecondsSinceEpoch; + final weekAgo = at.subtract(const Duration(days: 7)).millisecondsSinceEpoch; + + late ProviderContainer container; + late Provider<_Probe> probeProvider; + + setUp(() { + probeProvider = Provider<_Probe>((ref) => _Probe('order-1', ref)); + container = ProviderContainer( + overrides: [ + sessionNotifierProvider.overrideWith((ref) => _StubSessionNotifier()), + ], + ); + }); + + tearDown(() => container.dispose()); + + _Probe probe() => container.read(probeProvider); + + MostroMessage message(int? timestamp) => MostroMessage( + action: Action.fiatSentOk, + id: 'order-1', + timestamp: timestamp, + ); + + // MM-021's payload: a replayed message is authentic and passes every + // signature check, because it *is* the node's message. What marks it as an + // attack is that it describes a past this order has already moved beyond. + group('supersedesAppliedState', () { + test('a newer message is applied', () { + final p = probe()..setApplied(hourAgo); + expect(p.check(message(now)), isTrue); + }); + + test('a message from a week ago cannot modify newer state', () { + final p = probe()..setApplied(now); + expect(p.check(message(weekAgo)), isFalse); + }); + + test('an equally dated message is applied', () { + // A node can emit several messages within one second; exact + // re-deliveries are stopped earlier by event-id dedup. + final p = probe()..setApplied(now); + expect(p.check(message(now)), isTrue); + }); + + test('the first message is always applied', () { + expect(probe().check(message(now)), isTrue); + }); + + // v1 gift wrap has no usable clock — NIP-59 randomises those timestamps + // by design — so refusing untimestamped messages would break v1 outright. + test('an untimestamped message fails open', () { + final p = probe()..setApplied(now); + expect(p.check(message(null)), isTrue); + }); + }); + + // The concrete scenario: a seller who has already settled sees a replayed + // fiat-sent-ok. Without the guard it re-arms Release. + group('replayed fiat-sent-ok on a settled order', () { + test('would move the state backwards if applied', () { + final settled = OrderState( + status: Status.settledHoldInvoice, + action: Action.released, + order: null, + ); + + // Proof the message is not inert: applied, it does change the state. + final ifApplied = settled.updateWith(message(weekAgo)); + expect(ifApplied.action, Action.fiatSentOk); + expect(ifApplied.status, isNot(settled.status)); + }); + + test('is refused once newer state exists', () { + final p = probe()..setApplied(now); + expect(p.check(message(weekAgo)), isFalse); + }); + }); +} From 6cda36aa36af3c9adbf3b5273b8be6c1be722a8e Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 21:21:26 -0300 Subject: [PATCH 08/11] feat: persist order freshness across restore to prevent replayed messages from re-arming settled orders --- lib/data/models/enums/storage_keys.dart | 3 +- .../notifiers/abstract_mostro_notifier.dart | 39 +++- lib/features/order/order_freshness_store.dart | 148 ++++++++++++++ lib/shared/providers/app_init_provider.dart | 5 + .../order/order_freshness_store_test.dart | 187 ++++++++++++++++++ .../order/stale_message_guard_test.dart | 75 +++++++ 6 files changed, 453 insertions(+), 4 deletions(-) create mode 100644 lib/features/order/order_freshness_store.dart create mode 100644 test/features/order/order_freshness_store_test.dart diff --git a/lib/data/models/enums/storage_keys.dart b/lib/data/models/enums/storage_keys.dart index 49ad1aa9..90118eb7 100644 --- a/lib/data/models/enums/storage_keys.dart +++ b/lib/data/models/enums/storage_keys.dart @@ -7,7 +7,8 @@ enum SharedPreferencesKeys { trustedNodeMetadata('trusted_node_metadata'), backgroundFilters('background_filters'), communitySelected('community_selected'), - nodeProtocolVersions('node_protocol_versions'); + nodeProtocolVersions('node_protocol_versions'), + orderFreshness('order_freshness'); final String value; diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index 81e12124..493e4a8a 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -5,6 +5,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:mostro_mobile/data/enums.dart'; import 'package:mostro_mobile/data/models.dart'; import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; +import 'package:mostro_mobile/features/order/order_freshness_store.dart'; import 'package:mostro_mobile/features/order/models/order_state.dart'; import 'package:mostro_mobile/features/restore/restore_mode_provider.dart'; import 'package:mostro_mobile/shared/providers.dart'; @@ -33,10 +34,42 @@ class AbstractMostroNotifier extends StateNotifier { /// instruction from an archived one. Ordering can: a message older than the /// state it would modify is describing a past this order has already left. /// - /// Rebuilt from storage on [sync], so it survives a restart without needing - /// its own persistence. Null until the first timestamped message arrives. + /// Rebuilt from storage on [sync] and mirrored into [OrderFreshnessStore], + /// which outlives the databases a restore clears. Null until the first + /// timestamped message arrives and nothing is remembered. + int? _lastAppliedTimestamp; + + @protected + int? get lastAppliedTimestamp { + final local = _lastAppliedTimestamp; + final remembered = _rememberedTimestamp(); + if (local == null) return remembered; + if (remembered == null) return local; + return local > remembered ? local : remembered; + } + @protected - int? lastAppliedTimestamp; + set lastAppliedTimestamp(int? value) { + _lastAppliedTimestamp = value; + if (value != null) { + try { + ref.read(orderFreshnessStoreProvider).record(orderId, value); + } catch (e) { + // Losing the durable mirror costs this order's memory across a + // restore; it can never produce a wrong (lower) mark, because the + // store only moves forward. + logger.w('Failed to persist freshness for order $orderId: $e'); + } + } + } + + int? _rememberedTimestamp() { + try { + return ref.read(orderFreshnessStoreProvider).timestampFor(orderId); + } catch (e) { + return null; + } + } /// Whether [msg] may modify the current state. /// diff --git a/lib/features/order/order_freshness_store.dart b/lib/features/order/order_freshness_store.dart new file mode 100644 index 00000000..bf1a9613 --- /dev/null +++ b/lib/features/order/order_freshness_store.dart @@ -0,0 +1,148 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:mostro_mobile/data/models/enums/storage_keys.dart'; +import 'package:mostro_mobile/services/logger_service.dart'; +import 'package:mostro_mobile/shared/providers/storage_providers.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +/// Remembers, per order, the signed timestamp of the newest message this +/// client has ever acted on. +/// +/// `AbstractMostroNotifier` already refuses a message older than the state it +/// would modify, and rebuilds that mark from stored history on sync. That is +/// enough while the history exists — but the history is exactly what a restore +/// deletes. `RestoreService._clearAll` drops sessions, messages and the event +/// dedup store before the node is even contacted, and the trade keys are then +/// re-derived identically from the same master key. For a window afterwards +/// the client has the same identity and no memory, which is the state a +/// replayed `fiat-sent-ok` needs in order to look like news. +/// +/// This store is the part that survives. It lives outside the databases the +/// restore clears, holds no trade content — an order id and a timestamp — and +/// only ever moves forward, so a wipe cannot be turned into a way to make old +/// instructions fresh again. +class OrderFreshnessStore { + /// Cap on remembered orders. Each entry is roughly 60 bytes, so this is well + /// under a hundred kilobytes; the oldest are dropped first because an order + /// whose last activity is ancient is not one a replay can usefully target. + static const int maxEntries = 2000; + + final SharedPreferencesAsync _prefs; + + Map _timestamps = {}; + bool _initialized = false; + + /// Tail of the write chain, so concurrent writes land in call order. + Future _writes = Future.value(); + + OrderFreshnessStore(this._prefs); + + bool get isInitialized => _initialized; + + /// Completes when every queued write has finished. + Future get pendingWrites => _writes; + + Future init() async { + _timestamps = await _load(); + _initialized = true; + } + + /// Newest applied timestamp for [orderId], or null if none is remembered. + int? timestampFor(String orderId) => _timestamps[orderId]; + + /// Records [timestamp] for [orderId], keeping the higher of the two. + /// + /// Returns true when the stored value moved. + bool record(String orderId, int timestamp) { + if (orderId.isEmpty) return false; + + final current = _timestamps[orderId]; + if (current != null && current >= timestamp) return false; + + _timestamps[orderId] = timestamp; + _prune(); + _persist(); + return true; + } + + /// Drops everything. For an explicit "forget this device's history" action + /// only — routine flows must not call it, since clearing reopens precisely + /// the window this store exists to close. + Future clear() { + _timestamps = {}; + return _enqueueWrite( + () => _prefs.remove(SharedPreferencesKeys.orderFreshness.value), + 'clear order freshness', + ); + } + + void _prune() { + if (_timestamps.length <= maxEntries) return; + + final byAge = _timestamps.entries.toList() + ..sort((a, b) => b.value.compareTo(a.value)); + _timestamps = { + for (final entry in byAge.take(maxEntries)) entry.key: entry.value, + }; + } + + void _persist() { + final snapshot = jsonEncode(_timestamps); + unawaited( + _enqueueWrite( + () => _prefs.setString( + SharedPreferencesKeys.orderFreshness.value, + snapshot, + ), + 'persist order freshness', + ), + ); + } + + Future _enqueueWrite( + Future Function() op, + String description, + ) { + final queued = _writes.then((_) => op()).catchError( + (Object e) => logger.e('Failed to $description: $e'), + ); + _writes = queued; + return queued; + } + + Future> _load() async { + try { + final json = + await _prefs.getString(SharedPreferencesKeys.orderFreshness.value); + if (json == null) return {}; + + final decoded = jsonDecode(json); + if (decoded is! Map) return {}; + + final result = {}; + decoded.forEach((key, value) { + // A malformed entry is dropped, never allowed to throw and take the + // whole store down with it. + if (key is! String || key.isEmpty) return; + final timestamp = value is int + ? value + : value is String + ? int.tryParse(value) + : null; + if (timestamp != null && timestamp > 0) { + result[key] = timestamp; + } + }); + return result; + } catch (e) { + logger.e('Failed to load order freshness: $e'); + return {}; + } + } +} + +final orderFreshnessStoreProvider = Provider((ref) { + return OrderFreshnessStore(ref.watch(sharedPreferencesProvider)); +}); diff --git a/lib/shared/providers/app_init_provider.dart b/lib/shared/providers/app_init_provider.dart index 3835f787..6bd0910d 100644 --- a/lib/shared/providers/app_init_provider.dart +++ b/lib/shared/providers/app_init_provider.dart @@ -5,6 +5,7 @@ import 'package:mostro_mobile/features/key_manager/key_manager_provider.dart'; import 'package:mostro_mobile/features/chat/providers/chat_room_providers.dart'; import 'package:mostro_mobile/features/mostro/mostro_nodes_provider.dart'; import 'package:mostro_mobile/features/mostro/protocol_version_store.dart'; +import 'package:mostro_mobile/features/order/order_freshness_store.dart'; import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart'; import 'package:mostro_mobile/features/relays/relay_health_monitor.dart'; import 'package:mostro_mobile/features/restore/restore_manager.dart'; @@ -39,6 +40,10 @@ final appInitializerProvider = FutureProvider((ref) async { // in memory when the transport is resolved. await ref.read(protocolVersionStoreProvider).init(); + // Must load before any order notifier folds a message: this is the only + // freshness memory that survives a restore wiping the message history. + await ref.read(orderFreshnessStoreProvider).init(); + final sessionManager = ref.read(sessionNotifierProvider.notifier); await sessionManager.init(); diff --git a/test/features/order/order_freshness_store_test.dart b/test/features/order/order_freshness_store_test.dart new file mode 100644 index 00000000..94d2a707 --- /dev/null +++ b/test/features/order/order_freshness_store_test.dart @@ -0,0 +1,187 @@ +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/enums/storage_keys.dart'; +import 'package:mostro_mobile/features/order/order_freshness_store.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +class _FakeSharedPreferencesAsync implements SharedPreferencesAsync { + final Map strings = {}; + final bool failWrites; + + _FakeSharedPreferencesAsync({this.failWrites = false}); + + @override + Future getString(String key) async => strings[key]; + + @override + Future setString(String key, String value) async { + if (failWrites) throw Exception('disk full'); + strings[key] = value; + } + + @override + Future remove(String key) async => strings.remove(key); + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +final _key = SharedPreferencesKeys.orderFreshness.value; +const _orderA = 'a3f1c2d4-1111-2222-3333-444455556666'; +const _orderB = 'b4e2d3c5-7777-8888-9999-aaaabbbbcccc'; + +void main() { + late _FakeSharedPreferencesAsync prefs; + late OrderFreshnessStore store; + + final now = DateTime(2026, 3, 1).millisecondsSinceEpoch; + final earlier = DateTime(2026, 2, 1).millisecondsSinceEpoch; + + setUp(() async { + prefs = _FakeSharedPreferencesAsync(); + store = OrderFreshnessStore(prefs); + await store.init(); + }); + + group('basics', () { + test('knows nothing about an unseen order', () { + expect(store.timestampFor(_orderA), isNull); + }); + + test('records and reports a timestamp', () { + expect(store.record(_orderA, now), isTrue); + expect(store.timestampFor(_orderA), now); + }); + + test('orders are tracked independently', () { + store.record(_orderA, now); + store.record(_orderB, earlier); + + expect(store.timestampFor(_orderA), now); + expect(store.timestampFor(_orderB), earlier); + }); + }); + + // Only ever forward. A store that could be walked back would hand an + // attacker the very thing it exists to deny. + group('monotonicity', () { + test('moves forward', () { + store.record(_orderA, earlier); + expect(store.record(_orderA, now), isTrue); + expect(store.timestampFor(_orderA), now); + }); + + test('refuses to move backward', () { + store.record(_orderA, now); + expect(store.record(_orderA, earlier), isFalse); + expect(store.timestampFor(_orderA), now); + }); + + test('an equal timestamp is a no-op', () { + store.record(_orderA, now); + expect(store.record(_orderA, now), isFalse); + }); + }); + + // The point of the whole store: a restore clears the message history, the + // trade keys are re-derived identically, and this is the only thing left + // that knows the order had already moved past that message. + group('surviving a wipe', () { + test('a reopened store still refuses a replayed message', () async { + store.record(_orderA, now); + await store.pendingWrites; + + final afterWipe = OrderFreshnessStore(prefs); + await afterWipe.init(); + + expect(afterWipe.timestampFor(_orderA), now); + expect(afterWipe.record(_orderA, earlier), isFalse); + }); + + test('an empty store knows nothing', () async { + final fresh = OrderFreshnessStore(_FakeSharedPreferencesAsync()); + await fresh.init(); + + expect(fresh.timestampFor(_orderA), isNull); + }); + }); + + group('durability', () { + test('memory stays authoritative when writes fail', () async { + final failing = OrderFreshnessStore( + _FakeSharedPreferencesAsync(failWrites: true), + ); + await failing.init(); + + expect(failing.record(_orderA, now), isTrue); + expect(failing.timestampFor(_orderA), now); + await failing.pendingWrites; + expect(failing.timestampFor(_orderA), now); + }); + + test('writes the whole snapshot', () async { + store.record(_orderA, now); + store.record(_orderB, earlier); + await store.pendingWrites; + + expect(jsonDecode(prefs.strings[_key]!), { + _orderA: now, + _orderB: earlier, + }); + }); + + test('clear empties both memory and disk', () async { + store.record(_orderA, now); + await store.clear(); + + expect(store.timestampFor(_orderA), isNull); + expect(prefs.strings[_key], isNull); + }); + }); + + group('corrupt storage', () { + Future storeWith(String raw) async { + final p = _FakeSharedPreferencesAsync()..strings[_key] = raw; + final s = OrderFreshnessStore(p); + await s.init(); + return s; + } + + test('unparseable json yields an empty store', () async { + final s = await storeWith('not json'); + expect(s.timestampFor(_orderA), isNull); + expect(s.isInitialized, isTrue); + }); + + test('malformed entries are dropped, good ones kept', () async { + final s = await storeWith(jsonEncode({ + _orderA: now, + _orderB: 'garbage', + 'zero': 0, + 'negative': -5, + })); + + expect(s.timestampFor(_orderA), now); + expect(s.timestampFor(_orderB), isNull); + expect(s.timestampFor('zero'), isNull); + expect(s.timestampFor('negative'), isNull); + }); + }); + + group('pruning', () { + test('keeps the newest entries when over the cap', () { + for (var i = 0; i < OrderFreshnessStore.maxEntries + 50; i++) { + store.record('order-$i', now + i); + } + + // The oldest were dropped; the newest survive. + expect(store.timestampFor('order-0'), isNull); + expect( + store.timestampFor('order-${OrderFreshnessStore.maxEntries + 49}'), + isNotNull, + ); + }); + }); +} diff --git a/test/features/order/stale_message_guard_test.dart b/test/features/order/stale_message_guard_test.dart index 2e283657..8beb7b09 100644 --- a/test/features/order/stale_message_guard_test.dart +++ b/test/features/order/stale_message_guard_test.dart @@ -4,7 +4,12 @@ import 'package:mostro_mobile/data/models/enums/action.dart'; import 'package:mostro_mobile/data/models/enums/status.dart'; import 'package:mostro_mobile/data/models/mostro_message.dart'; import 'package:mostro_mobile/data/models/session.dart'; +import 'package:mostro_mobile/data/models/enums/storage_keys.dart'; import 'package:mostro_mobile/features/order/models/order_state.dart'; +import 'package:mostro_mobile/features/order/order_freshness_store.dart'; +import 'package:mostro_mobile/shared/providers/storage_providers.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'dart:convert'; import 'package:mostro_mobile/features/order/notifiers/abstract_mostro_notifier.dart'; import 'package:mostro_mobile/shared/notifiers/session_notifier.dart'; import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; @@ -19,6 +24,25 @@ class _Probe extends AbstractMostroNotifier { void setApplied(int? timestamp) => lastAppliedTimestamp = timestamp; } +class _FakePrefs implements SharedPreferencesAsync { + final Map strings = {}; + + @override + Future getString(String key) async => strings[key]; + + @override + Future setString(String key, String value) async { + strings[key] = value; + } + + @override + Future remove(String key) async => strings.remove(key); + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + class _StubSessionNotifier extends StateNotifier> implements SessionNotifier { _StubSessionNotifier() : super(const []); @@ -39,11 +63,14 @@ void main() { late ProviderContainer container; late Provider<_Probe> probeProvider; + late _FakePrefs prefs; setUp(() { + prefs = _FakePrefs(); probeProvider = Provider<_Probe>((ref) => _Probe('order-1', ref)); container = ProviderContainer( overrides: [ + sharedPreferencesProvider.overrideWithValue(prefs), sessionNotifierProvider.overrideWith((ref) => _StubSessionNotifier()), ], ); @@ -113,4 +140,52 @@ void main() { expect(p.check(message(weekAgo)), isFalse); }); }); + + // MM-021's last mile. A restore deletes the message history and re-derives + // the same trade keys, leaving the notifier with no in-memory mark. The + // durable store is what keeps the order from accepting an archived message + // as news during that window. + group('freshness that survives a wiped history', () { + Future seedStore(int timestamp) async { + prefs.strings[SharedPreferencesKeys.orderFreshness.value] = + jsonEncode({'order-1': timestamp}); + await container.read(orderFreshnessStoreProvider).init(); + } + + test('a fresh notifier inherits the remembered mark', () async { + await seedStore(now); + + // No message has been folded in this process — the mark comes entirely + // from storage, exactly as it would right after a restore. + expect(probe().check(message(weekAgo)), isFalse); + }); + + test('and still accepts genuinely newer messages', () async { + await seedStore(weekAgo); + + expect(probe().check(message(now)), isTrue); + }); + + test('applying a message records it durably', () async { + await container.read(orderFreshnessStoreProvider).init(); + final p = probe()..setApplied(now); + await container.read(orderFreshnessStoreProvider).pendingWrites; + + expect( + container.read(orderFreshnessStoreProvider).timestampFor('order-1'), + now, + ); + expect(p.check(message(weekAgo)), isFalse); + }); + + test('the in-memory mark cannot lower the remembered one', () async { + await seedStore(now); + + final p = probe()..setApplied(weekAgo); + + // The store only moves forward, and the guard takes the higher of the + // two, so a stale local value cannot reopen the window. + expect(p.check(message(weekAgo)), isFalse); + }); + }); } From f844b6362d97d6e17fb62a726d995f0491cfd4ca Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 21:26:16 -0300 Subject: [PATCH 09/11] feat: enforce trade key index monotonicity to prevent key reuse from daemon replay or race conditions --- .../models/last_trade_index_response.dart | 12 +- lib/features/key_manager/key_manager.dart | 34 ++++++ lib/features/restore/restore_manager.dart | 12 +- .../trade_index_monotonicity_test.dart | 115 ++++++++++++++++++ 4 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 test/features/key_manager/trade_index_monotonicity_test.dart diff --git a/lib/data/models/last_trade_index_response.dart b/lib/data/models/last_trade_index_response.dart index cf914576..443c1529 100644 --- a/lib/data/models/last_trade_index_response.dart +++ b/lib/data/models/last_trade_index_response.dart @@ -13,9 +13,15 @@ class LastTradeIndexResponse implements Payload { String get type => 'last-trade-index'; factory LastTradeIndexResponse.fromJson(Map json) { - return LastTradeIndexResponse( - tradeIndex: json['trade_index'] as int, - ); + final raw = json['trade_index']; + final tradeIndex = raw is int ? raw : (raw is num ? raw.toInt() : null); + if (tradeIndex == null || tradeIndex < 0) { + // A trade index is a count of keys derived; negative or non-numeric is + // not a value the daemon can mean, and it feeds a key-derivation + // counter, so it is refused here rather than clamped downstream. + throw FormatException('Invalid trade_index: $raw'); + } + return LastTradeIndexResponse(tradeIndex: tradeIndex); } @override diff --git a/lib/features/key_manager/key_manager.dart b/lib/features/key_manager/key_manager.dart index 1c0f03c3..1ea9dbf2 100644 --- a/lib/features/key_manager/key_manager.dart +++ b/lib/features/key_manager/key_manager.dart @@ -1,6 +1,7 @@ import 'package:dart_nostr/dart_nostr.dart'; import 'package:mostro_mobile/features/key_manager/key_derivator.dart'; import 'package:mostro_mobile/features/key_manager/key_storage.dart'; +import 'package:mostro_mobile/services/logger_service.dart'; import 'package:mostro_mobile/features/key_manager/key_manager_errors.dart'; class KeyManager { @@ -127,4 +128,37 @@ class KeyManager { tradeKeyIndex = index; await _storage.storeTradeKeyIndex(index); } + + /// Moves the trade key index up to [index], and never down. + /// + /// Use this for any index derived from a value the daemon sent. The counter + /// records how many trade keys this device has already derived, so lowering + /// it hands the next trade a keypair that has been used before: past and + /// future trades become linkable by a shared pubkey, and a live session can + /// find its key reissued underneath it. + /// + /// Refusing to go down is also simply correct, attack or no attack. The + /// daemon only knows the indexes that reached an order, so a device that + /// derived keys without trading legitimately sits ahead of it, and a restore + /// must not undo that. + /// + /// Returns the index in effect afterwards. + Future raiseCurrentKeyIndexTo(int index) async { + if (index < 1) { + throw InvalidTradeKeyIndexException( + 'Trade key index must be greater than 0', + ); + } + + final current = await getCurrentKeyIndex(); + if (index <= current) { + logger.w( + 'Refusing to lower trade key index from $current to $index', + ); + return current; + } + + await setCurrentKeyIndex(index); + return index; + } } diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index f8423d51..05cf8753 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -605,7 +605,8 @@ class RestoreService { final settings = ref.read(settingsProvider); // Set the next trade key index - await keyManager.setCurrentKeyIndex(lastTradeIndex + 1); + // From the daemon: raise only. See KeyManager.raiseCurrentKeyIndexTo. + await keyManager.raiseCurrentKeyIndexTo(lastTradeIndex + 1); // Enable restore mode to block all old message processing ref.read(isRestoringProvider.notifier).state = true; @@ -971,7 +972,7 @@ class RestoreService { lastTradeIndexEvent, ); final lastTradeIndex = lastTradeIndexResponse.tradeIndex; - await keyManager.setCurrentKeyIndex(lastTradeIndex + 1); + await keyManager.raiseCurrentKeyIndexTo(lastTradeIndex + 1); noHistoryFound = lastTradeIndexResponse.noHistoryFound; success = true; return true; @@ -1107,10 +1108,9 @@ class RestoreService { ); final response = await _extractLastTradeIndex(event); - await keyManager.setCurrentKeyIndex(response.tradeIndex + 1); - logger.i( - 'syncTradeIndex: updated local trade index to ${response.tradeIndex + 1}', - ); + final effective = + await keyManager.raiseCurrentKeyIndexTo(response.tradeIndex + 1); + logger.i('syncTradeIndex: local trade index is now $effective'); } catch (e, stack) { logger.e('syncTradeIndex: failed', error: e, stackTrace: stack); } finally { diff --git a/test/features/key_manager/trade_index_monotonicity_test.dart b/test/features/key_manager/trade_index_monotonicity_test.dart new file mode 100644 index 00000000..168589a0 --- /dev/null +++ b/test/features/key_manager/trade_index_monotonicity_test.dart @@ -0,0 +1,115 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/last_trade_index_response.dart'; +import 'package:mostro_mobile/features/key_manager/key_derivator.dart'; +import 'package:mostro_mobile/features/key_manager/key_manager.dart'; +import 'package:mostro_mobile/features/key_manager/key_manager_errors.dart'; +import 'package:mostro_mobile/features/key_manager/key_storage.dart'; + +/// Only the two index methods are exercised here; anything else is a bug in +/// the test, not a case to stub out. +class _FakeKeyStorage implements KeyStorage { + int index = 1; + + @override + Future readTradeKeyIndex() async => index; + + @override + Future storeTradeKeyIndex(int value) async { + index = value; + } + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +void main() { + late _FakeKeyStorage storage; + late KeyManager keyManager; + + setUp(() { + storage = _FakeKeyStorage(); + keyManager = KeyManager(storage, KeyDerivator("m/44'/1237'/38383'/0")); + }); + + // MM-021 / MM-003: the index counts trade keys this device has derived. + // Lowering it reissues keys that have already been used — past and future + // trades become linkable by a shared pubkey, and a live session can find its + // key handed out again underneath it. + group('raiseCurrentKeyIndexTo', () { + test('raises to a higher index', () async { + storage.index = 10; + + expect(await keyManager.raiseCurrentKeyIndexTo(51), 51); + expect(await keyManager.getCurrentKeyIndex(), 51); + }); + + test('refuses to lower the index', () async { + storage.index = 50; + + expect(await keyManager.raiseCurrentKeyIndexTo(11), 50); + expect(await keyManager.getCurrentKeyIndex(), 50); + }); + + test('an equal index is a no-op', () async { + storage.index = 50; + + expect(await keyManager.raiseCurrentKeyIndexTo(50), 50); + expect(await keyManager.getCurrentKeyIndex(), 50); + }); + + test('rejects a non-positive index', () async { + await expectLater( + keyManager.raiseCurrentKeyIndexTo(0), + throwsA(isA()), + ); + }); + + // Not only an attack: the daemon only knows indexes that reached an order, + // so a device that derived keys without trading is legitimately ahead. + test('a device ahead of the daemon keeps its position', () async { + storage.index = 50; + + await keyManager.raiseCurrentKeyIndexTo(20 + 1); + + expect(await keyManager.getCurrentKeyIndex(), 50); + }); + }); + + group('LastTradeIndexResponse parsing', () { + test('accepts a valid index', () { + expect( + LastTradeIndexResponse.fromJson({'trade_index': 42}).tradeIndex, + 42, + ); + }); + + test('accepts zero, meaning no trades yet', () { + expect( + LastTradeIndexResponse.fromJson({'trade_index': 0}).tradeIndex, + 0, + ); + }); + + test('rejects a negative index', () { + expect( + () => LastTradeIndexResponse.fromJson({'trade_index': -5}), + throwsFormatException, + ); + }); + + test('rejects a missing index', () { + expect( + () => LastTradeIndexResponse.fromJson(const {}), + throwsFormatException, + ); + }); + + test('rejects a non-numeric index', () { + expect( + () => LastTradeIndexResponse.fromJson({'trade_index': 'many'}), + throwsFormatException, + ); + }); + }); +} From ce685dd1fa67a20ae4d8d2e6590f6440c9613528 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Thu, 20 Aug 2026 17:14:15 -0300 Subject: [PATCH 10/11] fix: check message freshness before consuming per-order state The stale-message guard ran after cancelSessionTimeoutCleanup and after the user-initiated cancel marker had been removed, so a superseded canceled message spent both before reaching the check that rejects it. The genuine response arriving afterwards then found no marker and was reported as a counterparty inactivity timeout: the wrong notification, and a bonded order's session deletion deferred by 60s instead of immediate. Move the check to the top of the handler, ahead of every message-driven side effect. Add notifier-level coverage driving the real subscribe() path. --- .../notifiers/abstract_mostro_notifier.dart | 22 ++- .../order/stale_cancel_marker_test.dart | 158 ++++++++++++++++++ 2 files changed, 172 insertions(+), 8 deletions(-) create mode 100644 test/features/order/stale_cancel_marker_test.dart diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index 493e4a8a..4ea513bf 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -154,6 +154,20 @@ class AbstractMostroNotifier extends StateNotifier { logger.i('Received message with action: ${msg?.action}'); } if (msg != null) { + // Freshness first, before anything is consumed. Everything below + // mutates state a stale message must not be able to spend: the + // orphan session timer, the user-initiated cancel marker — a + // superseded `canceled` that took that marker would leave a + // genuine cancel in flight to be read as a counterparty timeout — + // and the rejected-resolution recovery pass below. + if (!supersedesAppliedState(msg)) { + logger.w( + 'Ignoring stale ${msg.action} for order $orderId: dated ' + '${msg.timestamp}, state already at $lastAppliedTimestamp', + ); + return; + } + // Decided before anything treats this message as a response: // updateWith would drop it, so it must not count as one. Notably // it must not cancel the orphan-session timer below — a forged @@ -180,14 +194,6 @@ class AbstractMostroNotifier extends StateNotifier { final wasUserInitiatedCancel = msg.action == Action.canceled && _userInitiatedCancels.remove(orderId); - if (!supersedesAppliedState(msg)) { - logger.w( - 'Ignoring stale ${msg.action} for order $orderId: dated ' - '${msg.timestamp}, state already at $lastAppliedTimestamp', - ); - return; - } - if (mounted) { state = state.updateWith(msg); if (msg.timestamp != null) { diff --git a/test/features/order/stale_cancel_marker_test.dart b/test/features/order/stale_cancel_marker_test.dart new file mode 100644 index 00000000..c364f170 --- /dev/null +++ b/test/features/order/stale_cancel_marker_test.dart @@ -0,0 +1,158 @@ +import 'dart:async'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/enums/status.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/data/models/session.dart'; +import 'package:mostro_mobile/features/order/notifiers/abstract_mostro_notifier.dart'; +import 'package:mostro_mobile/shared/notifiers/session_notifier.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/shared/providers/storage_providers.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +/// Drives the real `subscribe()` path and records what reaches `handleEvent`. +class _Probe extends AbstractMostroNotifier { + _Probe(super.orderId, super.ref); + + final List userInitiatedFlags = []; + + void setApplied(int? timestamp) => lastAppliedTimestamp = timestamp; + + void markCancel() => AbstractMostroNotifier.markUserInitiatedCancel(orderId); + + @override + Future handleEvent( + MostroMessage event, { + bool bypassTimestampGate = false, + Status? previousStatus, + bool wasUserInitiatedCancel = false, + }) async { + userInitiatedFlags.add(wasUserInitiatedCancel); + } +} + +class _FakePrefs implements SharedPreferencesAsync { + final Map strings = {}; + + @override + Future getString(String key) async => strings[key]; + + @override + Future setString(String key, String value) async { + strings[key] = value; + } + + @override + Future remove(String key) async => strings.remove(key); + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +class _StubSessionNotifier extends StateNotifier> + implements SessionNotifier { + _StubSessionNotifier() : super(const []); + + @override + Session? getSessionByOrderId(String orderId) => null; + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + +void main() { + const orderId = 'order-1'; + + late ProviderContainer container; + late Provider<_Probe> probeProvider; + late StreamController messages; + + // Real wall-clock dates: subscribe() gates handleEvent on the message being + // within the last 60 seconds, so a fixed test date would never get through. + final nowMs = DateTime.now().millisecondsSinceEpoch; + final hourAgoMs = + DateTime.now().subtract(const Duration(hours: 1)).millisecondsSinceEpoch; + final weekAgoMs = + DateTime.now().subtract(const Duration(days: 7)).millisecondsSinceEpoch; + + setUp(() { + messages = StreamController.broadcast(); + probeProvider = Provider<_Probe>((ref) => _Probe(orderId, ref)); + container = ProviderContainer( + overrides: [ + sharedPreferencesProvider.overrideWithValue(_FakePrefs()), + sessionNotifierProvider.overrideWith((ref) => _StubSessionNotifier()), + mostroMessageStreamProvider.overrideWith((ref, id) => messages.stream), + ], + ); + AbstractMostroNotifier.unmarkUserInitiatedCancel(orderId); + }); + + tearDown(() { + AbstractMostroNotifier.unmarkUserInitiatedCancel(orderId); + messages.close(); + container.dispose(); + }); + + MostroMessage cancel(int timestamp) => MostroMessage( + action: Action.canceled, + id: orderId, + timestamp: timestamp, + ); + + /// Lets the stream deliver and the unawaited handleEvent settle. + Future settle() => Future.delayed(Duration.zero); + + // The marker is set by cancelOrder and consumed once, to tell a voluntary + // cancel apart from a counterparty inactivity timeout — both arrive as + // Action.canceled. A replayed cancel that spends it leaves the genuine + // response to be read as a timeout: wrong notification, and a bonded order's + // session deletion deferred instead of immediate. + group('a stale canceled does not consume the user-initiated marker', () { + test('the genuine response that follows is still user-initiated', + () async { + final probe = container.read(probeProvider) + ..setApplied(hourAgoMs) + ..markCancel(); + probe.subscribe(); + + messages.add(cancel(weekAgoMs)); + await settle(); + + // Refused outright: nothing reached handleEvent. + expect(probe.userInitiatedFlags, isEmpty); + + messages.add(cancel(nowMs)); + await settle(); + + expect(probe.userInitiatedFlags, [true]); + }); + + test('with no replay in front of it the marker still works', () async { + final probe = container.read(probeProvider) + ..setApplied(hourAgoMs) + ..markCancel(); + probe.subscribe(); + + messages.add(cancel(nowMs)); + await settle(); + + expect(probe.userInitiatedFlags, [true]); + }); + + test('an unmarked cancel is reported as not user-initiated', () async { + final probe = container.read(probeProvider)..setApplied(hourAgoMs); + probe.subscribe(); + + messages.add(cancel(nowMs)); + await settle(); + + expect(probe.userInitiatedFlags, [false]); + }); + }); +} From cc7e5f9b2d2dfb156ce4eb8ec2737c0dfff5d4cf Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Thu, 20 Aug 2026 17:15:38 -0300 Subject: [PATCH 11/11] fix(restore): anchor the freshness mark to the snapshot's signed time updateStateFromMessage applied the restored snapshot without moving the high-water mark, and the message it synthesizes is dated with the order's created_at rather than the moment the snapshot describes. On a device with no remembered mark, every archived lifecycle event still compared as newer than the state just restored and could move it backwards. Carry the orders-details event's signed created_at into restore() and record it as each snapshot is applied. Only the v2 kind-14 path carries a signed clock; NIP-59 randomises the wrap and seal timestamps, so v1 yields null and leaves the mark untouched. Move the forward-only update into AbstractMostroNotifier next to the rest of the freshness logic, and align the regular-order branch with the dispute branch, which already converts Order.createdAt from seconds to milliseconds. --- .../notifiers/abstract_mostro_notifier.dart | 14 +++++ .../order/notifiers/order_notifier.dart | 14 ++++- lib/features/restore/restore_manager.dart | 46 ++++++++++++--- .../order/stale_message_guard_test.dart | 56 +++++++++++++++++++ .../features/restore/restore_decode_test.dart | 46 +++++++++++++++ 5 files changed, 166 insertions(+), 10 deletions(-) diff --git a/lib/features/order/notifiers/abstract_mostro_notifier.dart b/lib/features/order/notifiers/abstract_mostro_notifier.dart index 4ea513bf..394ec519 100644 --- a/lib/features/order/notifiers/abstract_mostro_notifier.dart +++ b/lib/features/order/notifiers/abstract_mostro_notifier.dart @@ -85,6 +85,20 @@ class AbstractMostroNotifier extends StateNotifier { if (incoming == null || applied == null) return true; return incoming >= applied; } + + /// Moves the freshness mark forward to [appliedAt], never backwards. + /// + /// For state applied from something other than a streamed message — a + /// restored snapshot, whose own message is dated with the order's creation + /// time rather than the moment the state it carries describes. + @protected + void anchorAppliedTimestamp(int? appliedAt) { + if (appliedAt == null) return; + final applied = lastAppliedTimestamp; + if (applied == null || appliedAt > applied) { + lastAppliedTimestamp = appliedAt; + } + } final Set _processedEventIds = {}; // Timer storage for orphan session cleanup diff --git a/lib/features/order/notifiers/order_notifier.dart b/lib/features/order/notifiers/order_notifier.dart index 947b09af..18ee573e 100644 --- a/lib/features/order/notifiers/order_notifier.dart +++ b/lib/features/order/notifiers/order_notifier.dart @@ -281,11 +281,21 @@ class OrderNotifier extends AbstractMostroNotifier { ); } - /// Update state from MostroMessage (used during restore) - void updateStateFromMessage(MostroMessage message) { + /// Applies a restored snapshot to this order's state. + /// + /// [appliedAt] is the node's signed timestamp for the snapshot, in + /// milliseconds. It anchors the freshness high-water mark: the snapshot + /// already folds in every lifecycle event up to that moment, so anything + /// older must not be able to re-apply afterwards. Without it a restored + /// device has no mark at all, and every archived message looks like news. + /// + /// The message's own timestamp is deliberately not used — it carries the + /// order's creation time, which predates its whole history. + void updateStateFromMessage(MostroMessage message, {int? appliedAt}) { if (mounted) { state = state.updateWith(message); } + anchorAppliedTimestamp(appliedAt); } /// Set fiatWasSent flag (used during restore to provide context diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 05cf8753..a1094145 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -588,12 +588,16 @@ class RestoreService { } } + /// [snapshotAt] is the node's signed `created_at` for the orders-details + /// response, in milliseconds, or null when the transport carries no + /// trustworthy clock. See [signedSnapshotTimestamp]. Future restore( Map ordersIds, int lastTradeIndex, OrdersResponse ordersResponse, - List disputes, - ) async { + List disputes, { + int? snapshotAt, + }) async { try { if (_masterKey == null) { throw Exception('Master key not initialized'); @@ -820,7 +824,10 @@ class RestoreService { await storage.addMessage(disputeKey, disputeMessage); // Update state with dispute message - notifier.updateStateFromMessage(disputeMessage); + notifier.updateStateFromMessage( + disputeMessage, + appliedAt: snapshotAt, + ); logger.i( 'Restore: created dispute message for order ${orderDetail.id}', ); @@ -851,9 +858,10 @@ class RestoreService { id: orderDetail.id, action: action, payload: order, - timestamp: - orderDetail.createdAt ?? - DateTime.now().millisecondsSinceEpoch, + // Seconds on the wire, milliseconds in MostroMessage. + timestamp: orderDetail.createdAt != null + ? orderDetail.createdAt! * 1000 + : DateTime.now().millisecondsSinceEpoch, ); // Save order message to storage @@ -862,7 +870,10 @@ class RestoreService { await storage.addMessage(key, mostroMessage); // Update state with order message - notifier.updateStateFromMessage(mostroMessage); + notifier.updateStateFromMessage( + mostroMessage, + appliedAt: snapshotAt, + ); } } catch (e, stack) { logger.e( @@ -1007,7 +1018,13 @@ class RestoreService { // STAGE 4: Processing and restoring sessions progress.updateStep(RestoreStep.processingRoles); - await restore(ordersMap, lastTradeIndex, ordersResponse, disputes); + await restore( + ordersMap, + lastTradeIndex, + ordersResponse, + disputes, + snapshotAt: signedSnapshotTimestamp(ordersDetailsEvent), + ); // Navigate to home and clear notification tray final navProvider = ref.read(navigationProvider.notifier); @@ -1147,6 +1164,19 @@ class RestoreService { /// (kind 1059, gift wrap unwrapped to a rumor whose content is the tuple). Both /// converge on `tuple[0]`. /// +/// The node's signed timestamp for a restore response, in milliseconds, or +/// null when the transport carries no trustworthy clock. +/// +/// Only the v2 kind-14 `created_at` is covered by the node's signature and the +/// recomputed id, so only it can anchor freshness. NIP-59 randomises the wrap +/// and seal timestamps by design, so the v1 path has nothing equivalent and +/// deliberately yields null rather than a plausible-looking guess. +@visibleForTesting +int? signedSnapshotTimestamp(NostrEvent event) { + if (event.kind != 14) return null; + return event.createdAt?.millisecondsSinceEpoch; +} + /// Top-level (not a private method) so the transport branch can be /// regression-tested without the full [RestoreService] / Riverpod orchestration. @visibleForTesting diff --git a/test/features/order/stale_message_guard_test.dart b/test/features/order/stale_message_guard_test.dart index 8beb7b09..cbd7b4e4 100644 --- a/test/features/order/stale_message_guard_test.dart +++ b/test/features/order/stale_message_guard_test.dart @@ -22,6 +22,8 @@ class _Probe extends AbstractMostroNotifier { bool check(MostroMessage msg) => supersedesAppliedState(msg); void setApplied(int? timestamp) => lastAppliedTimestamp = timestamp; + + void anchor(int? timestamp) => anchorAppliedTimestamp(timestamp); } class _FakePrefs implements SharedPreferencesAsync { @@ -188,4 +190,58 @@ void main() { expect(p.check(message(weekAgo)), isFalse); }); }); + // A restore wipes the message history and then applies the node's snapshot. + // The snapshot's own message is dated with the order's creation time, which + // predates its entire lifecycle — so without a separate anchor the mark + // either stays null or lands far enough back that every archived message + // still reads as news. + group('freshness anchored to a restored snapshot', () { + setUp(() async { + await container.read(orderFreshnessStoreProvider).init(); + }); + + test('the snapshot time is what the mark records, not the message date', + () { + final p = probe()..anchor(now); + + // hourAgo is newer than the order's creation time but older than the + // snapshot: exactly the archived lifecycle event this closes off. + expect(p.check(message(hourAgo)), isFalse); + expect(p.check(message(weekAgo)), isFalse); + }); + + test('messages the snapshot could not have folded in are still applied', + () { + final p = probe()..anchor(hourAgo); + + expect(p.check(message(now)), isTrue); + }); + + test('a null snapshot time leaves the mark untouched', () { + // v1 gift wrap has no signed clock, so restore yields null rather than + // a guess. Nothing is anchored and the guard keeps failing open. + final p = probe()..anchor(null); + + expect(p.check(message(weekAgo)), isTrue); + }); + + test('anchoring never moves the mark backwards', () { + final p = probe() + ..setApplied(now) + ..anchor(weekAgo); + + expect(p.check(message(hourAgo)), isFalse); + }); + + test('the anchor survives into the durable store', () async { + probe().anchor(now); + await container.read(orderFreshnessStoreProvider).pendingWrites; + + expect( + container.read(orderFreshnessStoreProvider).timestampFor('order-1'), + now, + ); + }); + }); + } diff --git a/test/features/restore/restore_decode_test.dart b/test/features/restore/restore_decode_test.dart index bac75721..ddf2c2dc 100644 --- a/test/features/restore/restore_decode_test.dart +++ b/test/features/restore/restore_decode_test.dart @@ -117,4 +117,50 @@ void main() { ); }); }); + + // Restore applies the node's snapshot and must then refuse anything older, + // which needs a trustworthy date for the snapshot itself. Only v2 has one. + group('signedSnapshotTimestamp', () { + late NostrKeyPairs tempTradeKey; + late NostrKeyPairs mostroKey; + + setUp(() { + tempTradeKey = NostrUtils.generateKeyPair(); + mostroKey = NostrUtils.generateKeyPair(); + }); + + test('v2 (kind 14) yields the signed created_at in milliseconds', () { + final signedAt = DateTime.fromMillisecondsSinceEpoch( + DateTime(2026, 3, 1).millisecondsSinceEpoch, + ); + final event = NostrEvent.fromPartialData( + kind: 14, + content: 'irrelevant', + keyPairs: mostroKey, + tags: [ + ['p', tempTradeKey.public], + ], + createdAt: signedAt, + ); + + expect( + signedSnapshotTimestamp(event), + signedAt.millisecondsSinceEpoch, + ); + }); + + test('v1 gift wrap (kind 1059) yields null', () async { + // NIP-59 randomises the wrap timestamp by design, so it says nothing + // about when the node produced the snapshot. Null keeps the guard + // failing open rather than anchoring on a fabricated date. + final event = await NostrUtils.createNIP59Event( + '[{}]', + tempTradeKey.public, + mostroKey.private, + ); + + expect(event.kind, 1059); + expect(signedSnapshotTimestamp(event), isNull); + }); + }); }