From 02d3a6f18367294a6e70ca241a71cc2280b9f336 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 00:26:19 -0300 Subject: [PATCH 01/12] test: add comprehensive coverage for NostrUtils.isValidEventSignature --- lib/shared/utils/nostr_utils.dart | 11 +- test/shared/utils/event_signature_test.dart | 175 ++++++++++++++++++++ 2 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 test/shared/utils/event_signature_test.dart diff --git a/lib/shared/utils/nostr_utils.dart b/lib/shared/utils/nostr_utils.dart index 36951c9c..aa4ab0b3 100644 --- a/lib/shared/utils/nostr_utils.dart +++ b/lib/shared/utils/nostr_utils.dart @@ -475,7 +475,7 @@ class NostrUtils { 'Unexpected author: expected $expectedAuthor, got ${event.pubkey}', ); } - if (!_isValidEventSignature(event)) { + if (!isValidEventSignature(event)) { throw ArgumentError('Invalid kind-14 event signature'); } @@ -492,7 +492,14 @@ class NostrUtils { /// Verifies a Nostr event's id and Schnorr signature (NIP-01): recomputes the /// id from the serialized event and checks the signature over it. - static bool _isValidEventSignature(NostrEvent event) { + /// + /// Prefer this over `NostrEvent.isVerified()` for any event whose *content* + /// is trusted. `isVerified()` only checks the Schnorr signature against the + /// event's self-declared `id`, so a genuine `(id, sig, pubkey)` triple lifted + /// from one event and pasted onto arbitrary content and tags still passes it. + /// Recomputing the id from the serialized event is what binds the signature + /// to what the event actually says. + static bool isValidEventSignature(NostrEvent event) { final id = event.id; final sig = event.sig; final createdAt = event.createdAt; diff --git a/test/shared/utils/event_signature_test.dart b/test/shared/utils/event_signature_test.dart new file mode 100644 index 00000000..376989e6 --- /dev/null +++ b/test/shared/utils/event_signature_test.dart @@ -0,0 +1,175 @@ +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; + +/// Builds a kind-38385 info event of the shape the daemon publishes +/// (empty content, everything in tags), signed by [keyPair]. +NostrEvent _infoEvent(NostrKeyPairs keyPair, {String protocolVersion = '2'}) { + return NostrEvent.fromPartialData( + kind: 38385, + content: '', + keyPairs: keyPair, + tags: [ + ['d', 'info'], + ['y', 'mostro'], + ['protocol_version', protocolVersion], + ], + ); +} + +void main() { + group('NostrUtils.isValidEventSignature', () { + test('accepts a genuinely signed event', () { + final event = _infoEvent(NostrUtils.generateKeyPair()); + + expect(NostrUtils.isValidEventSignature(event), isTrue); + }); + + test('accepts a signed event with non-empty content and no tags', () { + final event = NostrEvent.fromPartialData( + kind: 1, + content: 'plain note', + keyPairs: NostrUtils.generateKeyPair(), + tags: const [], + ); + + expect(NostrUtils.isValidEventSignature(event), isTrue); + }); + + test('rejects tampered tags — the id no longer matches the content', () { + final signed = _infoEvent(NostrUtils.generateKeyPair()); + + // Flip protocol_version 2 -> 1 while keeping the original id/sig. + final tampered = NostrEvent( + id: signed.id, + sig: signed.sig, + pubkey: signed.pubkey, + kind: signed.kind, + content: signed.content, + createdAt: signed.createdAt, + tags: const [ + ['d', 'info'], + ['y', 'mostro'], + ['protocol_version', '1'], + ], + ); + + expect(NostrUtils.isValidEventSignature(tampered), isFalse); + }); + + test('rejects tampered content', () { + final signed = NostrEvent.fromPartialData( + kind: 1, + content: 'original', + keyPairs: NostrUtils.generateKeyPair(), + tags: const [], + ); + + final tampered = NostrEvent( + id: signed.id, + sig: signed.sig, + pubkey: signed.pubkey, + kind: signed.kind, + content: 'spoofed', + createdAt: signed.createdAt, + tags: signed.tags, + ); + + expect(NostrUtils.isValidEventSignature(tampered), isFalse); + }); + + // This is the case NostrEvent.isVerified() lets through: it only checks + // the Schnorr signature against the event's self-declared id, so a genuine + // (id, sig, pubkey) triple lifted from one event and pasted onto a + // different payload passes it. Recomputing the id is what catches it. + test('rejects a genuine signature triple pasted onto a different payload', + () { + final keyPair = NostrUtils.generateKeyPair(); + final genuine = _infoEvent(keyPair); + + final forged = NostrEvent( + id: genuine.id, + sig: genuine.sig, + pubkey: genuine.pubkey, + kind: 38385, + content: '', + createdAt: genuine.createdAt, + tags: const [ + ['d', 'info'], + ['y', 'mostro'], + ['protocol_version', '1'], + ], + ); + + // The weaker check the rest of the codebase reaches for is fooled... + expect(forged.isVerified(), isTrue); + // ...while recomputing the id is not. + expect(NostrUtils.isValidEventSignature(forged), isFalse); + }); + + test('rejects an event signed by a different key', () { + final signed = _infoEvent(NostrUtils.generateKeyPair()); + final impostor = NostrUtils.generateKeyPair(); + + final swapped = NostrEvent( + id: signed.id, + sig: signed.sig, + pubkey: impostor.public, + kind: signed.kind, + content: signed.content, + createdAt: signed.createdAt, + tags: signed.tags, + ); + + expect(NostrUtils.isValidEventSignature(swapped), isFalse); + }); + + test('rejects an event with a malformed signature', () { + final signed = _infoEvent(NostrUtils.generateKeyPair()); + + final broken = NostrEvent( + id: signed.id, + sig: 'not-a-signature', + pubkey: signed.pubkey, + kind: signed.kind, + content: signed.content, + createdAt: signed.createdAt, + tags: signed.tags, + ); + + expect(NostrUtils.isValidEventSignature(broken), isFalse); + }); + + test('rejects an unsigned event', () { + final signed = _infoEvent(NostrUtils.generateKeyPair()); + + final unsigned = NostrEvent( + id: signed.id, + sig: null, + pubkey: signed.pubkey, + kind: signed.kind, + content: signed.content, + createdAt: signed.createdAt, + tags: signed.tags, + ); + + expect(NostrUtils.isValidEventSignature(unsigned), isFalse); + }); + + test('rejects an event with no id', () { + final signed = _infoEvent(NostrUtils.generateKeyPair()); + + final idless = NostrEvent( + id: null, + sig: signed.sig, + pubkey: signed.pubkey, + kind: signed.kind, + content: signed.content, + createdAt: signed.createdAt, + tags: signed.tags, + ); + + expect(NostrUtils.isValidEventSignature(idless), isFalse); + }); + }); +} From 47a6333b9fe40de2352afb482aa5b43e4afabe9e Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 00:31:49 -0300 Subject: [PATCH 02/12] test: verify kind-38385 info event signature before accepting Add signature verification to OpenOrdersRepository's info event intake to prevent downgrade attacks. A relay can re-tag a genuine event with a forged protocol_version while keeping the node's real pubkey and signature triple; accepting this would pin the client to the v1 gift-wrap transport, whose intake authenticates nothing. --- .../repositories/open_orders_repository.dart | 12 ++ .../open_orders_info_event_test.dart | 174 ++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 test/data/repositories/open_orders_info_event_test.dart diff --git a/lib/data/repositories/open_orders_repository.dart b/lib/data/repositories/open_orders_repository.dart index d271b043..809cd038 100644 --- a/lib/data/repositories/open_orders_repository.dart +++ b/lib/data/repositories/open_orders_repository.dart @@ -7,6 +7,7 @@ import 'package:mostro_mobile/data/repositories/order_repository_interface.dart' import 'package:mostro_mobile/features/settings/settings.dart'; import 'package:mostro_mobile/services/logger_service.dart'; import 'package:mostro_mobile/services/nostr_service.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; const orderEventKind = 38383; const infoEventKind = 38385; @@ -82,6 +83,17 @@ class OpenOrdersRepository implements OrderRepository { _eventStreamController.add(_events.values.toList()); } else if (event.kind == infoEventKind && event.pubkey == _settings.mostroPublicKey) { + // The author field alone proves nothing: any relay can hand us an + // event that merely *claims* the node's pubkey. The info event + // configures the wire transport (`protocol_version`), the bond policy + // and the PoW target, so an unsigned forgery is a downgrade primitive. + if (!NostrUtils.isValidEventSignature(event)) { + logger.w( + 'Rejecting kind-$infoEventKind info event claiming to be from ' + '${event.pubkey}: signature verification failed', + ); + return; + } logger.i('Mostro instance info loaded: $event'); _mostroInstance = event; if (!_mostroInstanceController.isClosed) { diff --git a/test/data/repositories/open_orders_info_event_test.dart b/test/data/repositories/open_orders_info_event_test.dart new file mode 100644 index 00000000..2c888c51 --- /dev/null +++ b/test/data/repositories/open_orders_info_event_test.dart @@ -0,0 +1,174 @@ +import 'dart:async'; + +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:mostro_mobile/data/repositories/open_orders_repository.dart'; +import 'package:mostro_mobile/features/settings/settings.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; + +import '../../mocks.mocks.dart'; + +/// Builds a kind-38385 info event of the shape mostrod publishes: empty +/// content, everything carried in tags. +NostrEvent _signedInfoEvent( + NostrKeyPairs keyPair, { + String protocolVersion = '2', +}) { + return NostrEvent.fromPartialData( + kind: 38385, + content: '', + keyPairs: keyPair, + tags: [ + ['d', 'info'], + ['y', 'mostro'], + ['z', 'info'], + ['protocol_version', protocolVersion], + ], + ); +} + +/// Re-tags [source] while keeping its original id/sig/pubkey — what a relay +/// can do for free, and what `NostrEvent.isVerified()` fails to catch. +NostrEvent _reTagged(NostrEvent source, List> tags) { + return NostrEvent( + id: source.id, + sig: source.sig, + pubkey: source.pubkey, + kind: source.kind, + content: source.content, + createdAt: source.createdAt, + tags: tags, + ); +} + +void main() { + late MockNostrService mockNostrService; + late StreamController eventController; + late NostrKeyPairs nodeKeys; + late Settings settings; + + setUp(() { + nodeKeys = NostrUtils.generateKeyPair(); + mockNostrService = MockNostrService(); + eventController = StreamController.broadcast(); + + settings = Settings( + relays: const ['wss://relay.example'], + fullPrivacyMode: false, + mostroPublicKey: nodeKeys.public, + ); + + when(mockNostrService.isInitialized).thenReturn(true); + when(mockNostrService.subscribeToEvents(any)) + .thenAnswer((_) => eventController.stream); + }); + + tearDown(() async { + await eventController.close(); + }); + + OpenOrdersRepository buildRepository() => + OpenOrdersRepository(mockNostrService, settings); + + group('OpenOrdersRepository info event (kind 38385) verification', () { + test('accepts a genuinely signed info event from the configured node', + () async { + final repository = buildRepository(); + final event = _signedInfoEvent(nodeKeys); + + eventController.add(event); + await pumpEventQueue(); + + expect(repository.mostroInstance, isNotNull); + expect(repository.mostroInstance!.id, event.id); + }); + + test('emits the accepted info event on mostroInstanceStream', () async { + final repository = buildRepository(); + final emitted = repository.mostroInstanceStream.first; + + eventController.add(_signedInfoEvent(nodeKeys)); + + expect((await emitted).kind, 38385); + }); + + test('rejects an info event whose signature does not verify', () async { + final repository = buildRepository(); + final genuine = _signedInfoEvent(nodeKeys); + + final forged = NostrEvent( + id: genuine.id, + sig: 'f' * 128, + pubkey: nodeKeys.public, + kind: genuine.kind, + content: genuine.content, + createdAt: genuine.createdAt, + tags: genuine.tags, + ); + + eventController.add(forged); + await pumpEventQueue(); + + expect(repository.mostroInstance, isNull); + }); + + // The downgrade primitive this guard exists for: flip protocol_version + // 2 -> 1 on a genuine event, keeping the node's real pubkey and a real + // signature triple. Accepting this pins the client to the v1 gift-wrap + // transport, whose intake authenticates nothing. + test('rejects a protocol_version downgrade re-tagged onto a real signature', + () async { + final repository = buildRepository(); + final genuine = _signedInfoEvent(nodeKeys, protocolVersion: '2'); + + final downgraded = _reTagged(genuine, [ + ['d', 'info'], + ['y', 'mostro'], + ['z', 'info'], + ['protocol_version', '1'], + ]); + + // The weak check the codebase reaches for elsewhere would allow this. + expect(downgraded.isVerified(), isTrue); + + eventController.add(downgraded); + await pumpEventQueue(); + + expect(repository.mostroInstance, isNull); + }); + + test('rejects an info event authored by a different key', () async { + final repository = buildRepository(); + final impostor = NostrUtils.generateKeyPair(); + + eventController.add(_signedInfoEvent(impostor, protocolVersion: '1')); + await pumpEventQueue(); + + expect(repository.mostroInstance, isNull); + }); + + test('a rejected event does not evict an already accepted one', () async { + final repository = buildRepository(); + final genuine = _signedInfoEvent(nodeKeys); + + eventController.add(genuine); + await pumpEventQueue(); + expect(repository.mostroInstance, isNotNull); + + eventController.add(_reTagged(genuine, [ + ['d', 'info'], + ['y', 'mostro'], + ['z', 'info'], + ['protocol_version', '1'], + ])); + await pumpEventQueue(); + + expect(repository.mostroInstance!.id, genuine.id); + expect( + repository.mostroInstance!.tags, + contains(equals(['protocol_version', '2'])), + ); + }); + }); +} From 217f1f743ad19448b6b5c6b5382f256a852d0869 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 00:34:22 -0300 Subject: [PATCH 03/12] fix: reject superseded kind-38385 info events to prevent config rollback Add monotonic timestamp enforcement to OpenOrdersRepository's info event intake: only accept events newer than the current mostroInstance.createdAt. A relay can replay a genuinely signed but superseded info event to roll the advertised protocol_version back; signature verification alone cannot prevent this downgrade path. The timestamp check resets to null on instance switch (via updateSettings), so the newly selected node's own info event is never blocked by the previous node's timestamp. --- .../repositories/open_orders_repository.dart | 20 ++++ .../open_orders_info_event_test.dart | 93 +++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/lib/data/repositories/open_orders_repository.dart b/lib/data/repositories/open_orders_repository.dart index 809cd038..b14aa2d1 100644 --- a/lib/data/repositories/open_orders_repository.dart +++ b/lib/data/repositories/open_orders_repository.dart @@ -94,6 +94,26 @@ class OpenOrdersRepository implements OrderRepository { ); return; } + // A valid signature says the node authored this event, not that it + // still reflects the node's configuration. Relays pick which events + // they serve and in what order, so without a monotonicity check the + // last one to arrive wins — letting a relay replay a genuinely signed + // but superseded info event to roll the advertised config back (most + // importantly `protocol_version` 2 -> 1). Only move forward in time. + // + // Reset to null on instance switch (see updateSettings), so this never + // blocks the newly selected node's own info event. + final currentCreatedAt = _mostroInstance?.createdAt; + final incomingCreatedAt = event.createdAt; + if (currentCreatedAt != null && + (incomingCreatedAt == null || + !incomingCreatedAt.isAfter(currentCreatedAt))) { + logger.d( + 'Ignoring kind-$infoEventKind info event from ${event.pubkey} ' + 'dated $incomingCreatedAt: not newer than $currentCreatedAt', + ); + return; + } logger.i('Mostro instance info loaded: $event'); _mostroInstance = event; if (!_mostroInstanceController.isClosed) { diff --git a/test/data/repositories/open_orders_info_event_test.dart b/test/data/repositories/open_orders_info_event_test.dart index 2c888c51..bc803b89 100644 --- a/test/data/repositories/open_orders_info_event_test.dart +++ b/test/data/repositories/open_orders_info_event_test.dart @@ -14,11 +14,13 @@ import '../../mocks.mocks.dart'; NostrEvent _signedInfoEvent( NostrKeyPairs keyPair, { String protocolVersion = '2', + DateTime? createdAt, }) { return NostrEvent.fromPartialData( kind: 38385, content: '', keyPairs: keyPair, + createdAt: createdAt, tags: [ ['d', 'info'], ['y', 'mostro'], @@ -171,4 +173,95 @@ void main() { ); }); }); + + group('OpenOrdersRepository info event freshness', () { + // Signature validity says the node authored the event, not that it is + // current. A relay that holds on to a superseded info event can replay it + // to roll the node's advertised config back — the downgrade path that + // survives signature verification. + test('ignores a signed but superseded info event', () async { + final repository = buildRepository(); + final now = DateTime.now(); + + final current = _signedInfoEvent( + nodeKeys, + protocolVersion: '2', + createdAt: now, + ); + final superseded = _signedInfoEvent( + nodeKeys, + protocolVersion: '1', + createdAt: now.subtract(const Duration(days: 30)), + ); + + eventController.add(current); + await pumpEventQueue(); + eventController.add(superseded); + await pumpEventQueue(); + + // Both are genuinely signed; only recency separates them. + expect(NostrUtils.isValidEventSignature(superseded), isTrue); + expect(repository.mostroInstance!.id, current.id); + }); + + test('applies a newer info event', () async { + final repository = buildRepository(); + final now = DateTime.now(); + + final older = _signedInfoEvent( + nodeKeys, + createdAt: now.subtract(const Duration(hours: 1)), + ); + final newer = _signedInfoEvent(nodeKeys, createdAt: now); + + eventController.add(older); + await pumpEventQueue(); + eventController.add(newer); + await pumpEventQueue(); + + expect(repository.mostroInstance!.id, newer.id); + }); + + test('ignores a re-delivery of the already accepted event', () async { + final repository = buildRepository(); + final event = _signedInfoEvent(nodeKeys, createdAt: DateTime.now()); + + final emissions = []; + final sub = repository.mostroInstanceStream.listen(emissions.add); + + // The same event arriving from several relays must not re-emit. + eventController.add(event); + await pumpEventQueue(); + eventController.add(event); + await pumpEventQueue(); + + expect(emissions, hasLength(1)); + await sub.cancel(); + }); + + test('accepts the newly selected node info after an instance switch', + () async { + final repository = buildRepository(); + final now = DateTime.now(); + + eventController.add(_signedInfoEvent(nodeKeys, createdAt: now)); + await pumpEventQueue(); + + // The next node's info event is older in wall-clock terms; the switch + // must not let the previous node's timestamp shut it out. + final nextNodeKeys = NostrUtils.generateKeyPair(); + repository.updateSettings( + settings.copyWith(mostroPublicKey: nextNodeKeys.public), + ); + + final nextInfo = _signedInfoEvent( + nextNodeKeys, + createdAt: now.subtract(const Duration(days: 7)), + ); + eventController.add(nextInfo); + await pumpEventQueue(); + + expect(repository.mostroInstance!.id, nextInfo.id); + }); + }); } From 2af5eff6b4ea2239a1befb1d61077fb041fd8393 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 00:39:14 -0300 Subject: [PATCH 04/12] feat: add monotonic protocol version store to prevent transport downgrade attacks Introduce ProtocolVersionStore, which remembers the highest verified protocol_version each Mostro node has ever advertised. A relay can replay a genuinely signed but superseded kind-38385 info event to downgrade the client's transport; the existing signature and timestamp checks reset on restart, so a cold start accepts the first event with nothing to compare it against. --- lib/data/models/enums/storage_keys.dart | 3 +- .../mostro/protocol_version_store.dart | 146 +++++++++++++ lib/shared/providers/app_init_provider.dart | 8 +- .../mostro/protocol_version_store_test.dart | 205 ++++++++++++++++++ 4 files changed, 360 insertions(+), 2 deletions(-) create mode 100644 lib/features/mostro/protocol_version_store.dart create mode 100644 test/features/mostro/protocol_version_store_test.dart diff --git a/lib/data/models/enums/storage_keys.dart b/lib/data/models/enums/storage_keys.dart index f2980c0b..49ad1aa9 100644 --- a/lib/data/models/enums/storage_keys.dart +++ b/lib/data/models/enums/storage_keys.dart @@ -6,7 +6,8 @@ enum SharedPreferencesKeys { mostroCustomNodes('mostro_custom_nodes'), trustedNodeMetadata('trusted_node_metadata'), backgroundFilters('background_filters'), - communitySelected('community_selected'); + communitySelected('community_selected'), + nodeProtocolVersions('node_protocol_versions'); final String value; diff --git a/lib/features/mostro/protocol_version_store.dart b/lib/features/mostro/protocol_version_store.dart new file mode 100644 index 00000000..7268d579 --- /dev/null +++ b/lib/features/mostro/protocol_version_store.dart @@ -0,0 +1,146 @@ +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 the highest `protocol_version` each Mostro node has ever been +/// *verified* to advertise, so a relay cannot walk a client back to an older +/// wire transport. +/// +/// The client learns a node's transport from its kind-38385 info event. Two +/// guards already sit in front of that event: the signature must verify, and +/// the event must be newer than the one in use. Neither survives a restart — +/// `OpenOrdersRepository` holds the info event in memory only, so on a cold +/// start the first info event to arrive is accepted with nothing to compare +/// it against. A relay that withholds the current event, or replays a +/// genuinely signed one from before the operator migrated to v2, downgrades +/// the client for that whole session. +/// +/// This store is the part that persists. It is deliberately monotonic: a +/// recorded version is never lowered, so once a node has been seen speaking +/// v2 no later event can make this client speak v1 to it again. +/// +/// Keyed by node pubkey, so switching between Mostro instances keeps each +/// node's history separate. +class ProtocolVersionStore { + final SharedPreferencesAsync _prefs; + + /// Node pubkey -> highest verified `protocol_version`. Authoritative once + /// [init] has run; persistence is a write-behind mirror of it. + Map _versions = {}; + + bool _initialized = false; + + ProtocolVersionStore(this._prefs); + + /// Loads persisted versions into memory. Must complete before the first + /// [versionFor] call for the ratchet to apply on a cold start; a store that + /// failed to load simply knows nothing and reports null. + Future init() async { + _versions = await _load(); + _initialized = true; + } + + bool get isInitialized => _initialized; + + /// Highest verified protocol version seen for [pubkey], or null if this + /// client has never verified an info event from that node. + int? versionFor(String pubkey) => _versions[pubkey]; + + /// Records [version] for [pubkey], keeping the higher of the two. + /// + /// Call this only for a version parsed from an info event whose signature + /// has been verified — an unverified event is a relay's claim, not the + /// node's, and recording it would poison the ratchet permanently. + /// + /// Returns true when the stored value changed. + bool record(String pubkey, int version) { + if (pubkey.isEmpty) return false; + // Guard against a malformed tag ratcheting the store to a value no + // resolver would honour anyway. + if (version < 1) { + logger.w('Ignoring non-positive protocol_version $version for $pubkey'); + return false; + } + + final current = _versions[pubkey]; + if (current != null && current >= version) { + if (current > version) { + logger.w( + 'Node $pubkey advertised protocol_version $version but has been ' + 'verified at $current before; keeping $current', + ); + } + return false; + } + + _versions[pubkey] = version; + logger.i('Recorded protocol_version $version for node $pubkey'); + _persist(); + return true; + } + + /// Drops everything this store knows. Intended for an explicit "forget this + /// device's history" action, not for routine flows — clearing it reopens the + /// downgrade window the ratchet exists to close. + Future clear() async { + _versions = {}; + try { + await _prefs.remove(SharedPreferencesKeys.nodeProtocolVersions.value); + } catch (e) { + logger.e('Failed to clear protocol versions: $e'); + } + } + + /// Memory is authoritative, so a failed write costs at most the ratchet's + /// memory of this node across a restart — never a wrong value. + void _persist() { + _prefs + .setString( + SharedPreferencesKeys.nodeProtocolVersions.value, + jsonEncode(_versions), + ) + .catchError( + (e) => logger.e('Failed to persist protocol versions: $e'), + ); + } + + Future> _load() async { + try { + final json = await _prefs.getString( + SharedPreferencesKeys.nodeProtocolVersions.value, + ); + if (json == null) return {}; + + final decoded = jsonDecode(json); + if (decoded is! Map) return {}; + + final result = {}; + decoded.forEach((key, value) { + if (key is! String || key.isEmpty) return; + // Tolerate anything a previous version (or a corrupted write) left + // behind: a malformed entry is dropped, never allowed to throw and + // take the whole ratchet down with it. + final version = value is int + ? value + : value is String + ? int.tryParse(value) + : null; + if (version != null && version >= 1) { + result[key] = version; + } + }); + return result; + } catch (e) { + logger.e('Failed to load protocol versions: $e'); + return {}; + } + } +} + +final protocolVersionStoreProvider = Provider((ref) { + return ProtocolVersionStore(ref.watch(sharedPreferencesProvider)); +}); diff --git a/lib/shared/providers/app_init_provider.dart b/lib/shared/providers/app_init_provider.dart index 1e59a517..3835f787 100644 --- a/lib/shared/providers/app_init_provider.dart +++ b/lib/shared/providers/app_init_provider.dart @@ -4,6 +4,7 @@ import 'package:mostro_mobile/core/config.dart'; 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/providers/order_notifier_provider.dart'; import 'package:mostro_mobile/features/relays/relay_health_monitor.dart'; import 'package:mostro_mobile/features/restore/restore_manager.dart'; @@ -33,9 +34,14 @@ final appInitializerProvider = FutureProvider((ref) async { await mostroNodes.init(); unawaited(mostroNodes.fetchAllNodeMetadata()); + // Must load before SubscriptionManager builds its first orders filter: the + // ratchet only protects a cold start if the persisted versions are already + // in memory when the transport is resolved. + await ref.read(protocolVersionStoreProvider).init(); + final sessionManager = ref.read(sessionNotifierProvider.notifier); await sessionManager.init(); - + ref.read(subscriptionManagerProvider); // Start the relay health watchdog: re-engages bootstrap relays and diff --git a/test/features/mostro/protocol_version_store_test.dart b/test/features/mostro/protocol_version_store_test.dart new file mode 100644 index 00000000..633bf90e --- /dev/null +++ b/test/features/mostro/protocol_version_store_test.dart @@ -0,0 +1,205 @@ +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/mostro/protocol_version_store.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +/// Minimal in-memory double for the three methods the store uses. +class _FakeSharedPreferencesAsync implements SharedPreferencesAsync { + final Map strings = {}; + + /// When set, every write fails — used to prove memory stays authoritative. + 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}'); +} + +const _nodeA = + '9d9d0455a96871f2dc4289b8312429db2e925f167b37c77bf7b28014be235980'; +const _nodeB = + '0000000000000000000000000000000000000000000000000000000000000001'; + +final _key = SharedPreferencesKeys.nodeProtocolVersions.value; + +void main() { + late _FakeSharedPreferencesAsync prefs; + late ProtocolVersionStore store; + + setUp(() async { + prefs = _FakeSharedPreferencesAsync(); + store = ProtocolVersionStore(prefs); + await store.init(); + }); + + group('ProtocolVersionStore basics', () { + test('knows nothing about a node it has never seen', () { + expect(store.versionFor(_nodeA), isNull); + }); + + test('records a version and reports it back', () { + expect(store.record(_nodeA, 2), isTrue); + expect(store.versionFor(_nodeA), 2); + }); + + test('keeps each node separate', () { + store.record(_nodeA, 2); + store.record(_nodeB, 1); + + expect(store.versionFor(_nodeA), 2); + expect(store.versionFor(_nodeB), 1); + }); + + test('ignores an empty pubkey', () { + expect(store.record('', 2), isFalse); + }); + + test('ignores a non-positive version', () { + expect(store.record(_nodeA, 0), isFalse); + expect(store.record(_nodeA, -1), isFalse); + expect(store.versionFor(_nodeA), isNull); + }); + }); + + group('ProtocolVersionStore ratchet', () { + // The whole point of the store: once a node has been verified speaking v2, + // nothing can make this client speak v1 to it again. + test('never lowers a recorded version', () { + store.record(_nodeA, 2); + + expect(store.record(_nodeA, 1), isFalse); + expect(store.versionFor(_nodeA), 2); + }); + + test('raises a recorded version', () { + store.record(_nodeA, 1); + + expect(store.record(_nodeA, 2), isTrue); + expect(store.versionFor(_nodeA), 2); + }); + + test('re-recording the same version is a no-op', () { + store.record(_nodeA, 2); + + expect(store.record(_nodeA, 2), isFalse); + expect(store.versionFor(_nodeA), 2); + }); + + test('a downgrade attempt on one node does not touch another', () { + store.record(_nodeA, 2); + store.record(_nodeB, 2); + + store.record(_nodeA, 1); + + expect(store.versionFor(_nodeA), 2); + expect(store.versionFor(_nodeB), 2); + }); + }); + + group('ProtocolVersionStore persistence', () { + test('survives a restart', () async { + store.record(_nodeA, 2); + + final reopened = ProtocolVersionStore(prefs); + await reopened.init(); + + expect(reopened.versionFor(_nodeA), 2); + }); + + test('the ratchet holds across a restart', () async { + store.record(_nodeA, 2); + + final reopened = ProtocolVersionStore(prefs); + await reopened.init(); + + expect(reopened.record(_nodeA, 1), isFalse); + expect(reopened.versionFor(_nodeA), 2); + }); + + test('writes the whole snapshot, not just the last change', () async { + store.record(_nodeA, 2); + store.record(_nodeB, 1); + + expect( + jsonDecode(prefs.strings[_key]!), + {_nodeA: 2, _nodeB: 1}, + ); + }); + + test('a failed write leaves memory authoritative', () async { + final failing = _FakeSharedPreferencesAsync(failWrites: true); + final s = ProtocolVersionStore(failing); + await s.init(); + + expect(s.record(_nodeA, 2), isTrue); + expect(s.versionFor(_nodeA), 2); + }); + + test('clear forgets everything', () async { + store.record(_nodeA, 2); + await store.clear(); + + expect(store.versionFor(_nodeA), isNull); + expect(prefs.strings[_key], isNull); + }); + }); + + group('ProtocolVersionStore corrupt storage', () { + Future storeWith(String raw) async { + prefs.strings[_key] = raw; + final s = ProtocolVersionStore(prefs); + await s.init(); + return s; + } + + test('starts empty on unparseable JSON', () async { + final s = await storeWith('{not json'); + expect(s.versionFor(_nodeA), isNull); + expect(s.isInitialized, isTrue); + }); + + test('starts empty when the payload is not a map', () async { + final s = await storeWith('[1, 2, 3]'); + expect(s.versionFor(_nodeA), isNull); + }); + + // A single bad entry must not cost the ratchet its memory of every other + // node — that would be a downgrade window opened by a storage bug. + test('drops malformed entries but keeps the good ones', () async { + final s = await storeWith(jsonEncode({ + _nodeA: 2, + _nodeB: 'garbage', + 'another': null, + 'negative': -5, + })); + + expect(s.versionFor(_nodeA), 2); + expect(s.versionFor(_nodeB), isNull); + expect(s.versionFor('another'), isNull); + expect(s.versionFor('negative'), isNull); + }); + + test('accepts a numeric string version', () async { + final s = await storeWith(jsonEncode({_nodeA: '2'})); + expect(s.versionFor(_nodeA), 2); + }); + }); +} From da1ecd825c3c3913999906a80119d57be97659ee Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 00:43:27 -0300 Subject: [PATCH 05/12] feat: anchor transport resolution to the highest verified protocol version Introduce anchoredProtocolVersion and resolveAnchoredTransport, which combine a node's current advertisement with the highest version it has previously been verified to speak, taking the maximum of the two. A relay can replay a genuinely signed but superseded info event to walk the client back to v1; the ratchet holds by refusing to accept any version claim lower than what the node has already proven. --- lib/features/mostro/transport.dart | 67 ++++++++++++--- .../subscriptions/subscription_manager.dart | 56 ++++++++++-- test/features/mostro/transport_test.dart | 85 +++++++++++++++++-- 3 files changed, 181 insertions(+), 27 deletions(-) diff --git a/lib/features/mostro/transport.dart b/lib/features/mostro/transport.dart index 76c13efe..566ccc0b 100644 --- a/lib/features/mostro/transport.dart +++ b/lib/features/mostro/transport.dart @@ -12,29 +12,74 @@ import 'package:mostro_mobile/services/logger_service.dart'; /// `docs/architecture/TRANSPORT_V2_MIGRATION.md` (§4.1). enum Transport { giftWrap, nip44 } +/// Transport assumed when nothing is known about a node. +/// +/// v2 (kind 14), matching mostrod's own default: since v0.18.0 the daemon +/// subscribes to exactly one kind and picks nip44 unless an operator opts into +/// gift-wrap explicitly, and v0.19.0 removes the choice entirely (issue #786). +/// +/// The direction of this default is a security property, not a guess about +/// what most nodes run. "Unknown" used to mean v1, whose intake authenticates +/// nothing — so a relay could pin a client to the forgeable transport just by +/// *withholding* the node's info event, no forgery required. Defaulting the +/// other way makes the failure mode a brief deafness against a genuine v1 node +/// (self-healing the moment its signed info event arrives) instead of a silent +/// downgrade. +const Transport kDefaultTransport = Transport.nip44; + /// Resolves the wire transport for a node from its advertised /// `protocol_version` (§2, §4.1). /// -/// - `2` → [Transport.nip44] (v2). /// - `1` → [Transport.giftWrap] (v1, explicitly advertised). -/// - `null` → [Transport.giftWrap]. The tag is absent or the node info has not -/// been fetched yet; during the migration window this is the common legacy -/// case, so it resolves to v1 without noise. -/// - any other value → [Transport.giftWrap], logged at `warn`. We do not speak -/// that protocol, so we degrade to v1 (version-skew guard) and surface the -/// degraded state so a misconfigured node is not silently mis-paired. +/// - `2` → [Transport.nip44] (v2). +/// - `null` → [kDefaultTransport]. The tag is absent, the node info has not +/// been fetched yet, or a relay is withholding it. +/// - any other value → [kDefaultTransport], logged at `warn`. We do not speak +/// that protocol; falling back to v1 here would hand any party who can put a +/// number in that tag a downgrade primitive, so an unrecognised version +/// degrades *upwards* to the safe default instead. +/// +/// Callers that can reach a [ProtocolVersionStore] should prefer +/// [resolveAnchoredTransport], which additionally refuses to walk a node back +/// to a transport older than one it has already been verified to speak. Transport resolveTransport(int? protocolVersion) { switch (protocolVersion) { + case 1: + return Transport.giftWrap; case 2: return Transport.nip44; - case 1: case null: - return Transport.giftWrap; + return kDefaultTransport; default: logger.w( 'Unsupported protocol_version $protocolVersion; ' - 'degrading to v1 gift wrap', + 'falling back to $kDefaultTransport', ); - return Transport.giftWrap; + return kDefaultTransport; } } + +/// Combines what a node advertises *now* with the highest version it has +/// previously been verified to advertise, taking the higher of the two. +/// +/// [advertised] comes from the node's current kind-38385 info event, and +/// [remembered] from [ProtocolVersionStore]. Both may be null: null +/// [advertised] means no info event is in hand, null [remembered] means this +/// client has never verified one from that node. +/// +/// Taking the maximum is what makes the ratchet hold. Upgrades pass straight +/// through, while a claim that a node speaks something *older* than it has +/// already been proven to speak is ignored — that claim is only ever reachable +/// by a relay replaying or suppressing events, never by the node itself under +/// a migration that only moves forward. +int? anchoredProtocolVersion(int? advertised, int? remembered) { + if (remembered == null) return advertised; + if (advertised == null) return remembered; + return advertised > remembered ? advertised : remembered; +} + +/// [resolveTransport] applied to [anchoredProtocolVersion] — the resolution +/// every caller should use once a [ProtocolVersionStore] is reachable. +Transport resolveAnchoredTransport(int? advertised, int? remembered) { + return resolveTransport(anchoredProtocolVersion(advertised, remembered)); +} diff --git a/lib/features/subscriptions/subscription_manager.dart b/lib/features/subscriptions/subscription_manager.dart index 73f4b29d..1489bbf9 100644 --- a/lib/features/subscriptions/subscription_manager.dart +++ b/lib/features/subscriptions/subscription_manager.dart @@ -8,6 +8,7 @@ import 'package:mostro_mobile/core/models/relay_list_event.dart'; import 'package:mostro_mobile/data/models/nostr_event.dart'; import 'package:mostro_mobile/data/models/session.dart'; import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; +import 'package:mostro_mobile/features/mostro/protocol_version_store.dart'; import 'package:mostro_mobile/features/mostro/transport.dart'; import 'package:mostro_mobile/features/settings/settings_provider.dart'; import 'package:mostro_mobile/features/subscriptions/subscription.dart'; @@ -59,13 +60,23 @@ class SubscriptionManager { /// switch to the v2 (kind 14) transport once the node advertises /// `protocol_version=2`. The info event arrives asynchronously after the /// initial subscription, so without this the orders filter would stay pinned - /// to the transport resolved at subscription time (typically v1 at cold - /// start). Re-subscribes only when the resolved transport actually changes. + /// to the transport resolved at subscription time. Re-subscribes only when + /// the resolved transport actually changes. + /// + /// Doubles as the feed for the downgrade ratchet: every event on this stream + /// has already had its signature verified and been checked for being newer + /// than the one in use (see [OpenOrdersRepository]), which is exactly the + /// precondition [ProtocolVersionStore.record] requires. void _initMostroInstanceListener() { try { _mostroInstanceListener = ref.read(orderRepositoryProvider).mostroInstanceStream.listen( - (_) { + (event) { + // Before the early returns below: a client with no open sessions + // still needs to learn the node's version, or the ratchet would only + // ever arm for users who happen to be mid-trade. + _recordAdvertisedProtocolVersion(event); + final newTransport = _resolveOrdersTransport(); if (newTransport == _appliedOrdersTransport) return; final sessions = ref.read(sessionNotifierProvider); @@ -84,16 +95,43 @@ class SubscriptionManager { } } - /// Resolves the transport for the orders subscription from the connected - /// node's advertised `protocol_version` (§2, §4.1). Defaults to v1 gift wrap - /// when the node info is not yet available or unreadable. + /// Records the version carried by a verified info [event] against its + /// author, so a later attempt to walk this node back to an older transport + /// has something to be measured against. + /// + /// Keyed by `event.pubkey` rather than the configured node pubkey: the + /// repository only emits events it has already matched to the connected + /// node, and using the event's own author keeps the store correct if that + /// ever changes. + void _recordAdvertisedProtocolVersion(NostrEvent event) { + try { + final version = event.protocolVersion; + if (version == null) return; + ref.read(protocolVersionStoreProvider).record(event.pubkey, version); + } catch (e) { + logger.w('Failed to record advertised protocol version: $e'); + } + } + + /// Resolves the transport for the orders subscription, combining the node's + /// currently advertised `protocol_version` (§2, §4.1) with the highest + /// version it has previously been verified to speak. + /// + /// Falls back to [kDefaultTransport] rather than v1 when the node info is + /// unavailable or unreadable: a relay can produce that state at will by + /// simply not serving the info event, and v1's intake authenticates nothing. Transport _resolveOrdersTransport() { try { final infoEvent = ref.read(orderRepositoryProvider).mostroInstance; - return resolveTransport(infoEvent?.protocolVersion); + final mostroPubkey = ref.read(settingsProvider).mostroPublicKey; + final remembered = + ref.read(protocolVersionStoreProvider).versionFor(mostroPubkey); + return resolveAnchoredTransport(infoEvent?.protocolVersion, remembered); } catch (e) { - logger.w('Failed to resolve orders transport, defaulting to v1: $e'); - return Transport.giftWrap; + logger.w( + 'Failed to resolve orders transport, using $kDefaultTransport: $e', + ); + return kDefaultTransport; } } diff --git a/test/features/mostro/transport_test.dart b/test/features/mostro/transport_test.dart index ebc51825..02424210 100644 --- a/test/features/mostro/transport_test.dart +++ b/test/features/mostro/transport_test.dart @@ -3,21 +3,92 @@ import 'package:mostro_mobile/features/mostro/transport.dart'; void main() { group('resolveTransport', () { + test('protocol_version 1 → giftWrap', () { + expect(resolveTransport(1), Transport.giftWrap); + }); + test('protocol_version 2 → nip44', () { expect(resolveTransport(2), Transport.nip44); }); - test('protocol_version 1 → giftWrap', () { - expect(resolveTransport(1), Transport.giftWrap); + // "Unknown" is a state any relay can create for free by not serving the + // node's info event. Resolving it to v1 — whose intake authenticates + // nothing — made omission a downgrade primitive, so it resolves to the + // safe default instead. + test('null (tag absent, info not fetched, or withheld) → default', () { + expect(resolveTransport(null), kDefaultTransport); + expect(kDefaultTransport, Transport.nip44); + }); + + // Same reasoning: degrading an unrecognised version to v1 would let any + // party who can put a number in that tag pick the forgeable transport. + test('unsupported version degrades upwards to the default, not to v1', () { + expect(resolveTransport(3), kDefaultTransport); + expect(resolveTransport(99), kDefaultTransport); + expect(resolveTransport(0), kDefaultTransport); + expect(resolveTransport(-1), kDefaultTransport); + }); + }); + + group('anchoredProtocolVersion', () { + test('knows nothing when neither source knows anything', () { + expect(anchoredProtocolVersion(null, null), isNull); + }); + + test('uses the advertised version when nothing is remembered', () { + expect(anchoredProtocolVersion(1, null), 1); + expect(anchoredProtocolVersion(2, null), 2); + }); + + test('uses the remembered version when nothing is advertised', () { + expect(anchoredProtocolVersion(null, 1), 1); + expect(anchoredProtocolVersion(null, 2), 2); + }); + + test('lets a node upgrade', () { + expect(anchoredProtocolVersion(2, 1), 2); + }); + + test('refuses to walk a node back below what it has proven', () { + expect(anchoredProtocolVersion(1, 2), 2); + }); + + test('agreeing sources pass straight through', () { + expect(anchoredProtocolVersion(2, 2), 2); + expect(anchoredProtocolVersion(1, 1), 1); + }); + }); + + group('resolveAnchoredTransport', () { + test('first contact with a legacy v1 node still reaches v1', () { + expect(resolveAnchoredTransport(1, null), Transport.giftWrap); + }); + + test('first contact with a v2 node reaches v2', () { + expect(resolveAnchoredTransport(2, null), Transport.nip44); + }); + + // The attack this whole chain exists to stop: the node is known to speak + // v2, and a relay tries to put the client back on the forgeable transport. + test('a replayed v1 advertisement cannot downgrade a known v2 node', () { + expect(resolveAnchoredTransport(1, 2), Transport.nip44); + }); + + test('withholding the info event cannot downgrade a known v2 node', () { + expect(resolveAnchoredTransport(null, 2), Transport.nip44); + }); + + test('withholding the info event on first contact reaches the default', + () { + expect(resolveAnchoredTransport(null, null), kDefaultTransport); }); - test('null (tag absent / node info not yet fetched) → giftWrap', () { - expect(resolveTransport(null), Transport.giftWrap); + test('a node that has only ever been seen at v1 stays reachable', () { + expect(resolveAnchoredTransport(1, 1), Transport.giftWrap); }); - test('unsupported version → degrades to giftWrap', () { - expect(resolveTransport(3), Transport.giftWrap); - expect(resolveTransport(0), Transport.giftWrap); + test('a v1 node that upgrades is followed to v2', () { + expect(resolveAnchoredTransport(2, 1), Transport.nip44); }); }); } From ed437c3c803d0ffffe89519cf30830814c1999de Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 00:47:43 -0300 Subject: [PATCH 06/12] feat: unify transport resolution through anchoredProtocolVersionFor Introduce anchoredProtocolVersionFor as the single resolution point for all send and receive paths. The dispute repository, restore manager, mostro service and subscription manager now call this instead of reading mostroInstance?.protocolVersion directly, ensuring the orders subscription and every outbound message always agree on which transport is in play. --- lib/data/repositories/dispute_repository.dart | 3 +- .../mostro/protocol_version_store.dart | 32 ++++++++++ lib/features/restore/restore_manager.dart | 14 +++-- .../subscriptions/subscription_manager.dart | 13 +--- lib/services/mostro_service.dart | 8 ++- .../mostro/transport_consistency_test.dart | 63 +++++++++++++++++++ 6 files changed, 111 insertions(+), 22 deletions(-) create mode 100644 test/features/mostro/transport_consistency_test.dart diff --git a/lib/data/repositories/dispute_repository.dart b/lib/data/repositories/dispute_repository.dart index c2611f60..336357f6 100644 --- a/lib/data/repositories/dispute_repository.dart +++ b/lib/data/repositories/dispute_repository.dart @@ -1,5 +1,6 @@ import 'package:collection/collection.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:mostro_mobile/features/mostro/protocol_version_store.dart'; import 'package:mostro_mobile/data/models/dispute.dart'; import 'package:mostro_mobile/data/models/mostro_message.dart'; import 'package:mostro_mobile/data/models/enums/action.dart'; @@ -56,7 +57,7 @@ class DisputeRepository { } final mostroPow = mostroInstance?.pow ?? 0; final event = await disputeMessage.wrapForTransport( - protocolVersion: mostroInstance?.protocolVersion, + protocolVersion: anchoredProtocolVersionFor(_ref), tradeKey: session.tradeKey, recipientPubKey: _mostroPubkey, masterKey: session.fullPrivacy ? null : session.masterKey, diff --git a/lib/features/mostro/protocol_version_store.dart b/lib/features/mostro/protocol_version_store.dart index 7268d579..53d023d2 100644 --- a/lib/features/mostro/protocol_version_store.dart +++ b/lib/features/mostro/protocol_version_store.dart @@ -2,7 +2,11 @@ import 'dart:convert'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:mostro_mobile/data/models/enums/storage_keys.dart'; +import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; +import 'package:mostro_mobile/features/mostro/transport.dart'; +import 'package:mostro_mobile/features/settings/settings_provider.dart'; import 'package:mostro_mobile/services/logger_service.dart'; +import 'package:mostro_mobile/shared/providers/order_repository_provider.dart'; import 'package:mostro_mobile/shared/providers/storage_providers.dart'; import 'package:shared_preferences/shared_preferences.dart'; @@ -144,3 +148,31 @@ class ProtocolVersionStore { final protocolVersionStoreProvider = Provider((ref) { return ProtocolVersionStore(ref.watch(sharedPreferencesProvider)); }); + +/// The protocol version to use when talking to the currently connected node: +/// what it advertises now, anchored against the highest version it has ever +/// been verified to advertise. +/// +/// Single resolution point on purpose. The send path and the receive +/// subscription must never disagree about which transport is in play — a +/// client listening on kind 14 while publishing kind 1059 is partitioned from +/// the node, and the attacker-controlled half of that split is the forgeable +/// one. Every caller reads this instead of reaching for +/// `mostroInstance?.protocolVersion` directly. +/// +/// Returns null only when the node has never been heard from and nothing is +/// remembered; [resolveTransport] maps that to [kDefaultTransport]. +int? anchoredProtocolVersionFor(Ref ref) { + try { + final infoEvent = ref.read(orderRepositoryProvider).mostroInstance; + final mostroPubkey = ref.read(settingsProvider).mostroPublicKey; + final remembered = + ref.read(protocolVersionStoreProvider).versionFor(mostroPubkey); + return anchoredProtocolVersion(infoEvent?.protocolVersion, remembered); + } catch (e) { + // Null resolves to the safe default rather than to v1, so failing to read + // the node's state cannot be turned into a downgrade. + logger.w('Failed to resolve anchored protocol version: $e'); + return null; + } +} diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 4e8d1230..21f95e09 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -6,6 +6,7 @@ import 'package:dart_nostr/nostr/model/event/event.dart'; import 'package:dart_nostr/nostr/model/request/filter.dart'; import 'package:dart_nostr/nostr/model/request/request.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:mostro_mobile/features/mostro/protocol_version_store.dart'; import 'package:mostro_mobile/core/config.dart'; import 'package:mostro_mobile/services/logger_service.dart'; import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; @@ -281,7 +282,7 @@ class RestoreService { ); } final wrappedEvent = await mostroMessage.wrapForTransport( - protocolVersion: mostroInstance?.protocolVersion, + protocolVersion: anchoredProtocolVersionFor(ref), tradeKey: _tempTradeKey!, recipientPubKey: settings.mostroPublicKey, masterKey: settings.fullPrivacyMode ? null : _masterKey, @@ -378,7 +379,7 @@ class RestoreService { ); } final wrappedEvent = await mostroMessage.wrapForTransport( - protocolVersion: mostroInstance?.protocolVersion, + protocolVersion: anchoredProtocolVersionFor(ref), tradeKey: _tempTradeKey!, recipientPubKey: settings.mostroPublicKey, masterKey: settings.fullPrivacyMode ? null : _masterKey, @@ -459,7 +460,7 @@ class RestoreService { ); } final wrappedEvent = await mostroMessage.wrapForTransport( - protocolVersion: mostroInstance?.protocolVersion, + protocolVersion: anchoredProtocolVersionFor(ref), tradeKey: _tempTradeKey!, recipientPubKey: settings.mostroPublicKey, masterKey: settings.fullPrivacyMode ? null : _masterKey, @@ -1076,9 +1077,10 @@ class RestoreService { _tempTradeKey = await keyManager.deriveTradeKeyFromIndex(1); // Wait for the node info event (kind 38385) before sending: at app init - // mostroInstance is still null, which makes wrapForTransport default to v1 - // gift wrap. On a protocol v2 node the request would then go out as kind - // 1059 and never be answered, leaving the local key index stale. + // nothing is known about the node yet, so the transport resolves to the + // default. Against a node that speaks the other protocol the request + // would go out on the wrong kind and never be answered, leaving the + // local key index stale. await _waitForNodeConnectivity(ref.read(settingsProvider).mostroPublicKey); _tempSubscription = await _createTempSubscription(); diff --git a/lib/features/subscriptions/subscription_manager.dart b/lib/features/subscriptions/subscription_manager.dart index 1489bbf9..5295db92 100644 --- a/lib/features/subscriptions/subscription_manager.dart +++ b/lib/features/subscriptions/subscription_manager.dart @@ -121,18 +121,7 @@ class SubscriptionManager { /// unavailable or unreadable: a relay can produce that state at will by /// simply not serving the info event, and v1's intake authenticates nothing. Transport _resolveOrdersTransport() { - try { - final infoEvent = ref.read(orderRepositoryProvider).mostroInstance; - final mostroPubkey = ref.read(settingsProvider).mostroPublicKey; - final remembered = - ref.read(protocolVersionStoreProvider).versionFor(mostroPubkey); - return resolveAnchoredTransport(infoEvent?.protocolVersion, remembered); - } catch (e) { - logger.w( - 'Failed to resolve orders transport, using $kDefaultTransport: $e', - ); - return kDefaultTransport; - } + return resolveTransport(anchoredProtocolVersionFor(ref)); } void _initSessionListener() { diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index 7be83dbc..e31478f2 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -4,6 +4,7 @@ import 'package:collection/collection.dart'; import 'package:dart_nostr/dart_nostr.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:mostro_mobile/features/mostro/protocol_version_store.dart'; import 'package:mostro_mobile/services/logger_service.dart'; import 'package:mostro_mobile/data/enums.dart'; import 'package:mostro_mobile/data/models.dart'; @@ -363,10 +364,11 @@ class MostroService { ); } - // Route through the transport advertised by the connected node (§5 Phase - // B). v1 nodes (default) keep the gift-wrap path byte-for-byte. + // Route through the transport the connected node speaks (§5 Phase B), + // anchored so a relay cannot steer the send path onto v1 independently of + // what the orders subscription is listening on. final event = await order.wrapForTransport( - protocolVersion: mostroInstance?.protocolVersion, + protocolVersion: anchoredProtocolVersionFor(ref), tradeKey: session.tradeKey, recipientPubKey: _settings.mostroPublicKey, masterKey: session.fullPrivacy ? null : session.masterKey, diff --git a/test/features/mostro/transport_consistency_test.dart b/test/features/mostro/transport_consistency_test.dart new file mode 100644 index 00000000..cfd7b153 --- /dev/null +++ b/test/features/mostro/transport_consistency_test.dart @@ -0,0 +1,63 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/enums/action.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/features/mostro/transport.dart'; +import 'package:mostro_mobile/features/subscriptions/subscription_manager.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; + +/// The kind the orders subscription would listen on for [version]. +int _listeningKind(int? version) { + final filter = buildOrdersFilter( + resolveTransport(version), + ['a' * 64], + 'b' * 64, + ); + return filter.kinds!.single; +} + +/// The kind an outbound message would actually be published as for [version]. +Future _publishingKind(int? version) async { + final tradeKey = NostrUtils.generateKeyPair(); + final message = MostroMessage(action: Action.fiatSent, id: 'order-1'); + + final event = await message.wrapForTransport( + protocolVersion: version, + tradeKey: tradeKey, + recipientPubKey: NostrUtils.generateKeyPair().public, + ); + return event.kind!; +} + +void main() { + // Send and receive derive their kind from the same resolution. If they ever + // disagree the client is partitioned from the node — listening on one kind + // while publishing on another — and the half an attacker controls is the + // forgeable one. These lock the two together at the only values that reach + // the wire. + group('send and receive resolve to the same kind', () { + for (final version in [null, 1, 2, 3, 99]) { + test('protocol_version $version', () async { + expect(await _publishingKind(version), _listeningKind(version)); + }); + } + }); + + group('resolved kinds are the protocol kinds', () { + test('v1 is gift wrap (1059)', () async { + expect(_listeningKind(1), 1059); + expect(await _publishingKind(1), 1059); + }); + + test('v2 is NIP-44 direct (14)', () async { + expect(_listeningKind(2), 14); + expect(await _publishingKind(2), 14); + }); + + // An unknown node must not land on the forgeable transport, on either + // side of the connection. + test('an unknown node lands on kind 14, not 1059', () async { + expect(_listeningKind(null), 14); + expect(await _publishingKind(null), 14); + }); + }); +} From d83ddec153f8eed084ee5a8ce613923295ecccce Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 01:03:03 -0300 Subject: [PATCH 07/12] feat: authenticate NIP-59 seal signer to prevent message forgery Add expectedAuthor parameter to decryptNIP59Event, unWrap and mostroUnWrap, which verifies the seal's pubkey and signature before trusting its content. The outer wrap is signed by a throwaway ephemeral key and the rumor is unsigned by design, so the seal is the only layer that names the real sender; without this check any party able to reach a trade key could inject arbitrary Mostro messages. --- lib/data/models/nostr_event.dart | 26 ++- .../background_notification_service.dart | 20 ++- lib/features/restore/restore_manager.dart | 5 +- lib/services/mostro_service.dart | 5 +- lib/services/nostr_service.dart | 11 +- lib/shared/utils/nostr_utils.dart | 40 ++++- .../models/nostr_event_extensions_test.dart | 10 +- .../utils/nip59_authentication_test.dart | 151 ++++++++++++++++++ 8 files changed, 248 insertions(+), 20 deletions(-) create mode 100644 test/shared/utils/nip59_authentication_test.dart diff --git a/lib/data/models/nostr_event.dart b/lib/data/models/nostr_event.dart index 743fdecb..77173a9f 100644 --- a/lib/data/models/nostr_event.dart +++ b/lib/data/models/nostr_event.dart @@ -68,10 +68,14 @@ extension NostrEventExtensions on NostrEvent { return timeago.format(createdAt!, allowFromNow: true, locale: locale); } - Future unWrap(String privateKey) async { + Future unWrap( + String privateKey, { + required String expectedAuthor, + }) async { return await NostrUtils.decryptNIP59Event( this, privateKey, + expectedAuthor: expectedAuthor, ); } @@ -104,7 +108,10 @@ extension NostrEventExtensions on NostrEvent { } } - Future mostroUnWrap(NostrKeyPairs receiver) async { + Future mostroUnWrap( + NostrKeyPairs receiver, { + required String expectedAuthor, + }) async { if (kind != 1059) { throw ArgumentError('Expected kind 1059 (Gift Wrap), got: $kind'); } @@ -139,6 +146,21 @@ extension NostrEventExtensions on NostrEvent { throw Exception('SEAL content is empty'); } + // STEP 2b: Authenticate the sender. The seal is the only signed + // layer that names them: the outer wrap is signed by a throwaway + // ephemeral key, and the rumor is unsigned by design, so its pubkey + // field is a claim. Pin the author and verify the signature before + // trusting anything inside. + if (sealEvent.pubkey != expectedAuthor) { + throw Exception( + 'Unexpected seal author: expected $expectedAuthor, ' + 'got ${sealEvent.pubkey}', + ); + } + if (!NostrUtils.isValidEventSignature(sealEvent)) { + throw Exception('Invalid seal signature'); + } + // STEP 3: Decrypt SEAL with sender's pubkey (from SEAL) // The SEAL pubkey identifies the actual sender (admin or user) final senderPubkey = sealEvent.pubkey; diff --git a/lib/features/notifications/services/background_notification_service.dart b/lib/features/notifications/services/background_notification_service.dart index 6754655c..16056901 100644 --- a/lib/features/notifications/services/background_notification_service.dart +++ b/lib/features/notifications/services/background_notification_service.dart @@ -301,20 +301,28 @@ Future _handleTradeKeyEvent(NostrEvent event, Session session) a // 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. + // Needed by both transports: v2 pins the kind-14 author, v1 pins the seal + // signer. Without it neither path can tell the node apart from any relay + // that can reach this trade key. + final mostroPubkey = await _loadMostroPubkey(); + if (mostroPubkey == null) { + logger.w('No Mostro pubkey available, cannot decrypt event'); + return null; + } + final String? content; if (event.kind == 14) { - final mostroPubkey = await _loadMostroPubkey(); - if (mostroPubkey == null) { - logger.w('No Mostro pubkey available, cannot decrypt kind-14 event'); - return null; - } content = await NostrUtils.decryptNIP44DirectEvent( event, session.tradeKey.private, expectedAuthor: mostroPubkey, ); } else { - content = (await event.unWrap(session.tradeKey.private)).content; + content = (await event.unWrap( + session.tradeKey.private, + expectedAuthor: mostroPubkey, + )) + .content; } if (content == null) { return null; diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 21f95e09..2b0d10b8 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -1157,7 +1157,10 @@ Future> decodeRestoreMessage( expectedAuthor: mostroPubkey, ); } else { - final rumor = await event.mostroUnWrap(tempTradeKey); + final rumor = await event.mostroUnWrap( + tempTradeKey, + expectedAuthor: mostroPubkey, + ); content = rumor.content ?? ''; } diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index e31478f2..c1c2d250 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -140,7 +140,10 @@ class MostroService { expectedAuthor: _settings.mostroPublicKey, ); } else { - final decryptedEvent = await event.unWrap(privateKey); + final decryptedEvent = await event.unWrap( + privateKey, + expectedAuthor: _settings.mostroPublicKey, + ); content = decryptedEvent.content; decryptedId = decryptedEvent.id; } diff --git a/lib/services/nostr_service.dart b/lib/services/nostr_service.dart index 354b53cb..d330132e 100644 --- a/lib/services/nostr_service.dart +++ b/lib/services/nostr_service.dart @@ -271,13 +271,18 @@ class NostrService { Future decryptNIP59Event( NostrEvent event, - String privateKey, - ) async { + String privateKey, { + String? expectedAuthor, + }) async { if (!_isInitialized) { throw Exception('Nostr is not initialized. Call init() first.'); } - return NostrUtils.decryptNIP59Event(event, privateKey); + return NostrUtils.decryptNIP59Event( + event, + privateKey, + expectedAuthor: expectedAuthor ?? settings.mostroPublicKey, + ); } Future createRumor( diff --git a/lib/shared/utils/nostr_utils.dart b/lib/shared/utils/nostr_utils.dart index aa4ab0b3..9e8b6add 100644 --- a/lib/shared/utils/nostr_utils.dart +++ b/lib/shared/utils/nostr_utils.dart @@ -380,10 +380,28 @@ class NostrUtils { return wrapEvent; } + /// Decrypts a protocol-v1 (NIP-59 gift wrap) Mostro event and authenticates + /// its sender. + /// + /// The outer wrap (kind 1059) is signed by a throwaway ephemeral key and + /// proves nothing — anyone can encrypt to a public trade key. The rumor + /// inside carries a `pubkey` field but is unsigned by design, so that field + /// is a claim, not evidence. The authentication in NIP-59 lives in the + /// middle layer: the seal (kind 13) is signed by the sender's real identity + /// key. mostrod builds it exactly that way — `EventBuilder::seal(...) + /// .sign(identity_keys)` in mostro-core's `nip59.rs` — and the node uses a + /// single keypair for identity and trade, so the seal's author is the + /// configured node pubkey. + /// + /// [expectedAuthor] is checked against the seal's author and the seal's + /// signature is verified, which is what makes this path forgery-resistant. + /// Without both, any party able to reach the victim's trade key could inject + /// arbitrary Mostro messages. static Future decryptNIP59Event( NostrEvent event, - String privateKey, - ) async { + String privateKey, { + required String expectedAuthor, + }) async { if (event.kind != 1059) { throw ArgumentError('Wrong kind: ${event.kind}'); } @@ -402,14 +420,26 @@ class NostrUtils { event.pubkey, ); - final rumorEvent = NostrEvent.deserialized( + // This is the seal (kind 13), not the rumor: the rumor is one layer + // further in, inside the seal's encrypted content. + final sealEvent = NostrEvent.deserialized( '["EVENT", "", $decryptedContent]', ); + if (sealEvent.pubkey != expectedAuthor) { + throw ArgumentError( + 'Unexpected seal author: expected $expectedAuthor, ' + 'got ${sealEvent.pubkey}', + ); + } + if (!isValidEventSignature(sealEvent)) { + throw ArgumentError('Invalid seal signature'); + } + final finalDecryptedContent = await decryptNIP44( - rumorEvent.content!, + sealEvent.content!, privateKey, - rumorEvent.pubkey, + sealEvent.pubkey, ); final wrap = jsonDecode(finalDecryptedContent) as Map; diff --git a/test/data/models/nostr_event_extensions_test.dart b/test/data/models/nostr_event_extensions_test.dart index f7153a1d..3dcbb548 100644 --- a/test/data/models/nostr_event_extensions_test.dart +++ b/test/data/models/nostr_event_extensions_test.dart @@ -192,7 +192,10 @@ void main() { group('NostrEventExtensions.mostroUnWrap', () { test('rejects an event that is not a gift wrap', () async { await expectLater( - orderEvent().mostroUnWrap(NostrKeyPairs(private: '1' * 64)), + orderEvent().mostroUnWrap( + NostrKeyPairs(private: '1' * 64), + expectedAuthor: 'b' * 64, + ), throwsArgumentError, ); }); @@ -209,7 +212,10 @@ void main() { ); await expectLater( - event.mostroUnWrap(NostrKeyPairs(private: '1' * 64)), + event.mostroUnWrap( + NostrKeyPairs(private: '1' * 64), + expectedAuthor: 'b' * 64, + ), throwsArgumentError, ); }); diff --git a/test/shared/utils/nip59_authentication_test.dart b/test/shared/utils/nip59_authentication_test.dart new file mode 100644 index 00000000..0a25deff --- /dev/null +++ b/test/shared/utils/nip59_authentication_test.dart @@ -0,0 +1,151 @@ +import 'dart:convert'; + +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mostro_mobile/data/models/nostr_event.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; + +/// Wraps [content] as mostrod does on the v1 transport: rumor -> seal signed +/// by the sender's identity key -> gift wrap under a throwaway ephemeral key. +Future _giftWrap({ + required NostrKeyPairs sender, + required String recipientPubKey, + String content = '[{"order":{"action":"pay-invoice"}}]', +}) { + return NostrUtils.createNIP59Event(content, recipientPubKey, sender.private); +} + +void main() { + late NostrKeyPairs node; + late NostrKeyPairs recipient; + + setUp(() { + node = NostrUtils.generateKeyPair(); + recipient = NostrUtils.generateKeyPair(); + }); + + group('decryptNIP59Event sender authentication', () { + test('accepts a gift wrap sealed by the expected author', () async { + final wrap = await _giftWrap( + sender: node, + recipientPubKey: recipient.public, + ); + + final rumor = await NostrUtils.decryptNIP59Event( + wrap, + recipient.private, + expectedAuthor: node.public, + ); + + expect(rumor.content, '[{"order":{"action":"pay-invoice"}}]'); + }); + + // The core of MM-001: the outer wrap is signed by a throwaway key and the + // recipient's trade pubkey is public (it rides in the `p` tag of every + // event addressed to them), so anyone who can see the relay traffic can + // encrypt a well-formed gift wrap to a victim. Only the seal names the + // real sender. + test('rejects a gift wrap sealed by anyone else', () async { + final attacker = NostrUtils.generateKeyPair(); + final forged = await _giftWrap( + sender: attacker, + recipientPubKey: recipient.public, + ); + + // It decrypts perfectly — nothing about the encryption is broken. + await expectLater( + NostrUtils.decryptNIP59Event( + forged, + recipient.private, + expectedAuthor: node.public, + ), + throwsA(isA()), + ); + }); + + test('rejects a seal whose signature does not verify', () async { + final wrap = await _giftWrap( + sender: node, + recipientPubKey: recipient.public, + ); + + // Rebuild the wrap around a seal carrying the node's pubkey but a + // signature that was never produced for it. + final sealJson = jsonDecode( + await NostrUtils.decryptNIP44( + wrap.content!, + recipient.private, + wrap.pubkey, + ), + ) as Map; + sealJson['sig'] = 'f' * 128; + + final tamperedWrap = await NostrUtils.createWrap( + NostrUtils.generateKeyPair(), + await NostrUtils.encryptNIP44( + jsonEncode(sealJson), + NostrUtils.generateKeyPair().private, + recipient.public, + ), + recipient.public, + ); + + await expectLater( + NostrUtils.decryptNIP59Event( + tamperedWrap, + recipient.private, + expectedAuthor: node.public, + ), + throwsA(isA()), + ); + }); + + test('still rejects a non-gift-wrap kind', () async { + final notAWrap = NostrEvent.fromPartialData( + kind: 1, + content: 'hello', + keyPairs: node, + tags: const [], + ); + + await expectLater( + NostrUtils.decryptNIP59Event( + notAWrap, + recipient.private, + expectedAuthor: node.public, + ), + throwsArgumentError, + ); + }); + }); + + group('NostrEventExtensions.unWrap', () { + test('round-trips a message from the expected author', () async { + final wrap = await _giftWrap( + sender: node, + recipientPubKey: recipient.public, + content: '[{"order":{"action":"fiat-sent-ok"}}]', + ); + + final rumor = await wrap.unWrap( + recipient.private, + expectedAuthor: node.public, + ); + + expect(rumor.content, contains('fiat-sent-ok')); + }); + + test('refuses a message from an impostor', () async { + final attacker = NostrUtils.generateKeyPair(); + final forged = await _giftWrap( + sender: attacker, + recipientPubKey: recipient.public, + ); + + await expectLater( + forged.unWrap(recipient.private, expectedAuthor: node.public), + throwsA(isA()), + ); + }); + }); +} From c5508f63d5e1b118b9e5b1a4a914ba0443d64257 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 18:31:31 -0300 Subject: [PATCH 08/12] fix: prevent protocol version store write reordering and clarify legacy version semantics Introduce a write queue in ProtocolVersionStore to serialize all mutations to SharedPreferencesAsync, preventing concurrent setString/remove calls from landing out of order and resurrecting cleared state or overwriting newer snapshots with older ones. Add pendingWrites to expose flush points and _enqueueWrite to chain operations while swallowing individual failures. --- lib/data/models/nostr_event.dart | 18 +- .../mostro/protocol_version_store.dart | 70 ++++++-- lib/features/mostro/transport.dart | 12 ++ .../background_notification_service.dart | 2 +- .../subscriptions/subscription_manager.dart | 8 + lib/services/nostr_service.dart | 14 +- lib/shared/utils/nostr_utils.dart | 41 ++++- .../anchored_transport_resolution_test.dart | 169 ++++++++++++++++++ .../mostro/protocol_version_store_test.dart | 76 ++++++++ .../utils/nip59_authentication_test.dart | 37 +++- 10 files changed, 402 insertions(+), 45 deletions(-) create mode 100644 test/features/mostro/anchored_transport_resolution_test.dart diff --git a/lib/data/models/nostr_event.dart b/lib/data/models/nostr_event.dart index 77173a9f..0a5fb715 100644 --- a/lib/data/models/nostr_event.dart +++ b/lib/data/models/nostr_event.dart @@ -146,20 +146,10 @@ extension NostrEventExtensions on NostrEvent { throw Exception('SEAL content is empty'); } - // STEP 2b: Authenticate the sender. The seal is the only signed - // layer that names them: the outer wrap is signed by a throwaway - // ephemeral key, and the rumor is unsigned by design, so its pubkey - // field is a claim. Pin the author and verify the signature before - // trusting anything inside. - if (sealEvent.pubkey != expectedAuthor) { - throw Exception( - 'Unexpected seal author: expected $expectedAuthor, ' - 'got ${sealEvent.pubkey}', - ); - } - if (!NostrUtils.isValidEventSignature(sealEvent)) { - throw Exception('Invalid seal signature'); - } + // STEP 2b: Authenticate the sender before trusting anything inside. + // See NostrUtils.authenticateSeal for why the seal is the layer that + // carries this evidence. + NostrUtils.authenticateSeal(sealEvent, expectedAuthor); // STEP 3: Decrypt SEAL with sender's pubkey (from SEAL) // The SEAL pubkey identifies the actual sender (admin or user) diff --git a/lib/features/mostro/protocol_version_store.dart b/lib/features/mostro/protocol_version_store.dart index 53d023d2..6a22847b 100644 --- a/lib/features/mostro/protocol_version_store.dart +++ b/lib/features/mostro/protocol_version_store.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -38,8 +39,37 @@ class ProtocolVersionStore { bool _initialized = false; + /// Tail of the write chain. Every mutation of the persisted key is appended + /// here so writes land in call order. + /// + /// `SharedPreferencesAsync` holds no Dart-side cache: each call goes + /// straight to platform storage and concurrent calls complete in whatever + /// order the platform picks. Without this chain a `setString` issued before + /// a `clear()` could land after it and resurrect the cleared map, or an + /// older snapshot could overwrite a newer one — the ratchet is monotonic in + /// memory, but its disk mirror would not be. + Future _writes = Future.value(); + ProtocolVersionStore(this._prefs); + /// Completes when every write queued so far has finished. Useful to flush + /// before shutdown, and the only way a caller can observe durability — + /// [record] returns as soon as memory is updated. + Future get pendingWrites => _writes; + + /// Appends [op] to the write chain and returns when it has run. Failures are + /// logged and swallowed so one bad write cannot poison every later one. + 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; + } + /// Loads persisted versions into memory. Must complete before the first /// [versionFor] call for the ratchet to apply on a cold start; a store that /// failed to load simply knows nothing and reports null. @@ -90,26 +120,31 @@ class ProtocolVersionStore { /// Drops everything this store knows. Intended for an explicit "forget this /// device's history" action, not for routine flows — clearing it reopens the /// downgrade window the ratchet exists to close. - Future clear() async { + Future clear() { _versions = {}; - try { - await _prefs.remove(SharedPreferencesKeys.nodeProtocolVersions.value); - } catch (e) { - logger.e('Failed to clear protocol versions: $e'); - } + return _enqueueWrite( + () => _prefs.remove(SharedPreferencesKeys.nodeProtocolVersions.value), + 'clear protocol versions', + ); } /// Memory is authoritative, so a failed write costs at most the ratchet's /// memory of this node across a restart — never a wrong value. + /// + /// The snapshot is encoded here, at call time, and the write is queued: what + /// reaches disk is the map as it stood when the caller asked, applied in the + /// order the callers asked. void _persist() { - _prefs - .setString( + final snapshot = jsonEncode(_versions); + unawaited( + _enqueueWrite( + () => _prefs.setString( SharedPreferencesKeys.nodeProtocolVersions.value, - jsonEncode(_versions), - ) - .catchError( - (e) => logger.e('Failed to persist protocol versions: $e'), - ); + snapshot, + ), + 'persist protocol versions', + ), + ); } Future> _load() async { @@ -162,13 +197,20 @@ final protocolVersionStoreProvider = Provider((ref) { /// /// Returns null only when the node has never been heard from and nothing is /// remembered; [resolveTransport] maps that to [kDefaultTransport]. +/// +/// A verified info event that carries no `protocol_version` tag reads as +/// [kLegacyProtocolVersion], not as "unknown" — see that constant for why the +/// two cases must stay distinct. int? anchoredProtocolVersionFor(Ref ref) { try { final infoEvent = ref.read(orderRepositoryProvider).mostroInstance; final mostroPubkey = ref.read(settingsProvider).mostroPublicKey; final remembered = ref.read(protocolVersionStoreProvider).versionFor(mostroPubkey); - return anchoredProtocolVersion(infoEvent?.protocolVersion, remembered); + final advertised = infoEvent == null + ? null + : (infoEvent.protocolVersion ?? kLegacyProtocolVersion); + return anchoredProtocolVersion(advertised, remembered); } catch (e) { // Null resolves to the safe default rather than to v1, so failing to read // the node's state cannot be turned into a downgrade. diff --git a/lib/features/mostro/transport.dart b/lib/features/mostro/transport.dart index 566ccc0b..bbeee7e9 100644 --- a/lib/features/mostro/transport.dart +++ b/lib/features/mostro/transport.dart @@ -27,6 +27,18 @@ enum Transport { giftWrap, nip44 } /// downgrade. const Transport kDefaultTransport = Transport.nip44; +/// The version a node states by *omitting* the `protocol_version` tag from an +/// otherwise valid info event. +/// +/// Legacy daemons (pre-v0.18.0) never emit the tag, so on a verified, +/// non-superseded info event an absent tag is v1 asserted by omission. That is +/// a different fact from having no info event at all, and the two must not +/// collapse into the same `null`: the first is evidence about the node, the +/// second is the absence of evidence. Reading a legacy node's silence as +/// [kDefaultTransport] would pin the client to kind 14 against a node that only +/// ever listens on kind 1059 — a permanent, self-inflicted partition. +const int kLegacyProtocolVersion = 1; + /// Resolves the wire transport for a node from its advertised /// `protocol_version` (§2, §4.1). /// diff --git a/lib/features/notifications/services/background_notification_service.dart b/lib/features/notifications/services/background_notification_service.dart index 16056901..6796a014 100644 --- a/lib/features/notifications/services/background_notification_service.dart +++ b/lib/features/notifications/services/background_notification_service.dart @@ -305,7 +305,7 @@ Future _handleTradeKeyEvent(NostrEvent event, Session session) a // signer. Without it neither path can tell the node apart from any relay // that can reach this trade key. final mostroPubkey = await _loadMostroPubkey(); - if (mostroPubkey == null) { + if (mostroPubkey == null || mostroPubkey.isEmpty) { logger.w('No Mostro pubkey available, cannot decrypt event'); return null; } diff --git a/lib/features/subscriptions/subscription_manager.dart b/lib/features/subscriptions/subscription_manager.dart index 5295db92..696a5d15 100644 --- a/lib/features/subscriptions/subscription_manager.dart +++ b/lib/features/subscriptions/subscription_manager.dart @@ -103,6 +103,14 @@ class SubscriptionManager { /// repository only emits events it has already matched to the connected /// node, and using the event's own author keeps the store correct if that /// ever changes. + /// + /// A missing tag is deliberately *not* recorded as [kLegacyProtocolVersion], + /// even though [anchoredProtocolVersionFor] reads it that way when resolving + /// a transport. Resolution can see that the info event is in hand; the store + /// outlives it. Persisting 1 would leave `remembered == 1` with no info + /// event after a restart, and a relay that then simply withholds the event + /// would resolve v1 off remembered state alone — turning the ratchet, whose + /// whole purpose is to block downgrades, into a durable downgrade primitive. void _recordAdvertisedProtocolVersion(NostrEvent event) { try { final version = event.protocolVersion; diff --git a/lib/services/nostr_service.dart b/lib/services/nostr_service.dart index d330132e..c556fcfa 100644 --- a/lib/services/nostr_service.dart +++ b/lib/services/nostr_service.dart @@ -269,6 +269,11 @@ class NostrService { ); } + /// [expectedAuthor] defaults to the configured node, which is the sender for + /// every Mostro message on this path. It is resolved and checked here rather + /// than passed through: an empty `mostroPublicKey` would otherwise reach the + /// author pin as a valid-looking argument, fail the comparison, and report + /// "unexpected seal author" for what is really unconfigured settings. Future decryptNIP59Event( NostrEvent event, String privateKey, { @@ -278,10 +283,17 @@ class NostrService { throw Exception('Nostr is not initialized. Call init() first.'); } + final author = expectedAuthor ?? settings.mostroPublicKey; + if (author.isEmpty) { + throw StateError( + 'Cannot authenticate the sender: no Mostro public key configured', + ); + } + return NostrUtils.decryptNIP59Event( event, privateKey, - expectedAuthor: expectedAuthor ?? settings.mostroPublicKey, + expectedAuthor: author, ); } diff --git a/lib/shared/utils/nostr_utils.dart b/lib/shared/utils/nostr_utils.dart index 9e8b6add..3d55787d 100644 --- a/lib/shared/utils/nostr_utils.dart +++ b/lib/shared/utils/nostr_utils.dart @@ -426,15 +426,7 @@ class NostrUtils { '["EVENT", "", $decryptedContent]', ); - if (sealEvent.pubkey != expectedAuthor) { - throw ArgumentError( - 'Unexpected seal author: expected $expectedAuthor, ' - 'got ${sealEvent.pubkey}', - ); - } - if (!isValidEventSignature(sealEvent)) { - throw ArgumentError('Invalid seal signature'); - } + authenticateSeal(sealEvent, expectedAuthor); final finalDecryptedContent = await decryptNIP44( sealEvent.content!, @@ -467,6 +459,12 @@ class NostrUtils { ), subscriptionId: '', ); + } on ArgumentError { + // Authentication verdicts propagate unwrapped. Folding them into the + // generic Exception below would make "this message is not from the node" + // indistinguishable from "the payload was malformed", both to callers + // and to anyone reading a log line. + rethrow; } catch (e) { throw Exception('Failed to decrypt NIP-59 event: $e'); } @@ -520,6 +518,31 @@ class NostrUtils { } } + /// Authenticates a NIP-59 seal (kind 13) as having been written by + /// [expectedAuthor]. + /// + /// The seal is the only layer of a gift wrap that names its sender under a + /// signature: the outer wrap (kind 1059) is signed by a throwaway ephemeral + /// key and proves nothing, and the rumor inside is unsigned by design, so + /// its `pubkey` field is a claim rather than evidence. mostrod builds the + /// seal as `EventBuilder::seal(...).sign(identity_keys)` (mostro-core + /// `nip59.rs`), and the node uses one keypair for identity and trade, so the + /// seal's author is the configured node pubkey. + /// + /// Single implementation on purpose: both gift-wrap unwrapping paths call + /// this, so a change to what "authenticated" means cannot reach one and miss + /// the other. Throws when the seal fails either check. + static void authenticateSeal(NostrEvent seal, String expectedAuthor) { + if (seal.pubkey != expectedAuthor) { + throw ArgumentError( + 'Unexpected seal author: expected $expectedAuthor, got ${seal.pubkey}', + ); + } + if (!isValidEventSignature(seal)) { + throw ArgumentError('Invalid seal signature'); + } + } + /// Verifies a Nostr event's id and Schnorr signature (NIP-01): recomputes the /// id from the serialized event and checks the signature over it. /// diff --git a/test/features/mostro/anchored_transport_resolution_test.dart b/test/features/mostro/anchored_transport_resolution_test.dart new file mode 100644 index 00000000..444baa9c --- /dev/null +++ b/test/features/mostro/anchored_transport_resolution_test.dart @@ -0,0 +1,169 @@ +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:mockito/mockito.dart'; +import 'package:mostro_mobile/data/models/enums/storage_keys.dart'; +import 'package:mostro_mobile/data/repositories/open_orders_repository.dart'; +import 'package:mostro_mobile/features/mostro/protocol_version_store.dart'; +import 'package:mostro_mobile/features/mostro/transport.dart'; +import 'package:mostro_mobile/features/settings/settings.dart'; +import 'package:mostro_mobile/features/settings/settings_provider.dart'; +import 'package:mostro_mobile/shared/providers/order_repository_provider.dart'; +import 'package:mostro_mobile/shared/providers/storage_providers.dart'; +import 'package:mostro_mobile/shared/utils/nostr_utils.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import '../../mocks.mocks.dart'; + +/// A kind-38385 info event shaped like mostrod's: empty content, everything in +/// tags. Passing null for [protocolVersion] omits the tag entirely, which is +/// what every daemon before v0.18.0 publishes. +NostrEvent _infoEvent( + NostrKeyPairs keyPair, { + required String? protocolVersion, +}) { + return NostrEvent.fromPartialData( + kind: 38385, + content: '', + keyPairs: keyPair, + tags: [ + ['d', 'info'], + ['y', 'mostro'], + ['z', 'info'], + if (protocolVersion != null) ['protocol_version', protocolVersion], + ], + ); +} + +class _FakeSharedPreferencesAsync 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}'); +} + +/// Reads [anchoredProtocolVersionFor] from inside the container. It takes a +/// `Ref`, which only a provider has; refresh this to re-evaluate it. +final _anchored = Provider((ref) => anchoredProtocolVersionFor(ref)); + +void main() { + late MockNostrService mockNostrService; + late StreamController eventController; + late NostrKeyPairs nodeKeys; + late _FakeSharedPreferencesAsync prefs; + late ProviderContainer container; + + setUp(() async { + nodeKeys = NostrUtils.generateKeyPair(); + mockNostrService = MockNostrService(); + eventController = StreamController.broadcast(); + + final settings = Settings( + relays: const ['wss://relay.example'], + fullPrivacyMode: false, + mostroPublicKey: nodeKeys.public, + ); + + prefs = _FakeSharedPreferencesAsync(); + prefs.strings[SharedPreferencesKeys.appSettings.value] = + jsonEncode(settings.toJson()); + + when(mockNostrService.isInitialized).thenReturn(true); + when(mockNostrService.subscribeToEvents(any)) + .thenAnswer((_) => eventController.stream); + + container = ProviderContainer( + overrides: [ + sharedPreferencesProvider.overrideWithValue(prefs), + orderRepositoryProvider.overrideWithValue( + OpenOrdersRepository(mockNostrService, settings), + ), + ], + ); + + await container.read(settingsProvider.notifier).init(); + expect(container.read(settingsProvider).mostroPublicKey, nodeKeys.public); + }); + + tearDown(() async { + container.dispose(); + await eventController.close(); + }); + + int? anchored() => container.refresh(_anchored); + + /// Feeds [event] through the repository and waits for it to be applied. + Future deliver(NostrEvent event) async { + final applied = container + .read(orderRepositoryProvider) + .mostroInstanceStream + .first + .timeout(const Duration(seconds: 2)); + eventController.add(event); + await applied; + } + + // The `null` returned by NostrEventExtensions.protocolVersion means two + // different things, and collapsing them is a self-inflicted partition: a + // legacy node that publishes a perfectly valid info event without the tag + // would resolve to kind 14 and never be heard from again, because it only + // ever listens on kind 1059. + group('a verified info event with no protocol_version tag', () { + test('resolves to gift wrap, not to the safe default', () async { + await deliver(_infoEvent(nodeKeys, protocolVersion: null)); + + expect(anchored(), kLegacyProtocolVersion); + expect(resolveTransport(anchored()), Transport.giftWrap); + }); + + test('is still distinct from having no info event at all', () { + // Nothing delivered: absence of evidence, so the safe default stands. + expect(anchored(), isNull); + expect(resolveTransport(anchored()), kDefaultTransport); + }); + + test('does not walk back a node already verified at v2', () async { + container.read(protocolVersionStoreProvider).record(nodeKeys.public, 2); + + await deliver(_infoEvent(nodeKeys, protocolVersion: null)); + + // The ratchet outranks a tag-less event: on a node that has already + // migrated, only a relay replaying an old event produces this state. + expect(anchored(), 2); + expect(resolveTransport(anchored()), Transport.nip44); + }); + }); + + group('a verified info event advertising a version', () { + test('v2 resolves to nip44', () async { + await deliver(_infoEvent(nodeKeys, protocolVersion: '2')); + + expect(anchored(), 2); + expect(resolveTransport(anchored()), Transport.nip44); + }); + + test('an explicit v1 resolves to gift wrap', () async { + await deliver(_infoEvent(nodeKeys, protocolVersion: '1')); + + expect(anchored(), 1); + expect(resolveTransport(anchored()), Transport.giftWrap); + }); + }); +} diff --git a/test/features/mostro/protocol_version_store_test.dart b/test/features/mostro/protocol_version_store_test.dart index 633bf90e..ab082f33 100644 --- a/test/features/mostro/protocol_version_store_test.dart +++ b/test/features/mostro/protocol_version_store_test.dart @@ -33,6 +33,50 @@ class _FakeSharedPreferencesAsync implements SharedPreferencesAsync { throw UnimplementedError('${invocation.memberName}'); } + +class _Countdown { + int _micros; + _Countdown(this._micros); + + Duration next() { + final current = Duration(microseconds: _micros); + _micros = _micros > 500 ? _micros - 500 : 0; + return current; + } +} + +/// Completes its writes in reverse call order, so a store that fires them +/// concurrently loses the race and a store that queues them does not. +class _ReorderingSharedPreferencesAsync implements SharedPreferencesAsync { + final Map strings = {}; + + /// Longest delay first: each successive call waits less than the one before + /// it, so without serialisation the last call lands first. Held in a box + /// because `SharedPreferencesAsync` is `@immutable`. + final _Countdown _delay = _Countdown(5000); + + @override + Future getString(String key) async => strings[key]; + + @override + Future setString(String key, String value) async { + await _stagger(); + strings[key] = value; + } + + @override + Future remove(String key) async { + await _stagger(); + strings.remove(key); + } + + Future _stagger() => Future.delayed(_delay.next()); + + @override + dynamic noSuchMethod(Invocation invocation) => + throw UnimplementedError('${invocation.memberName}'); +} + const _nodeA = '9d9d0455a96871f2dc4289b8312429db2e925f167b37c77bf7b28014be235980'; const _nodeB = @@ -117,6 +161,7 @@ void main() { group('ProtocolVersionStore persistence', () { test('survives a restart', () async { store.record(_nodeA, 2); + await store.pendingWrites; final reopened = ProtocolVersionStore(prefs); await reopened.init(); @@ -126,6 +171,7 @@ void main() { test('the ratchet holds across a restart', () async { store.record(_nodeA, 2); + await store.pendingWrites; final reopened = ProtocolVersionStore(prefs); await reopened.init(); @@ -137,6 +183,7 @@ void main() { test('writes the whole snapshot, not just the last change', () async { store.record(_nodeA, 2); store.record(_nodeB, 1); + await store.pendingWrites; expect( jsonDecode(prefs.strings[_key]!), @@ -202,4 +249,33 @@ void main() { expect(s.versionFor(_nodeA), 2); }); }); + + group('ProtocolVersionStore write ordering', () { + late _ReorderingSharedPreferencesAsync slowPrefs; + late ProtocolVersionStore orderedStore; + + setUp(() async { + slowPrefs = _ReorderingSharedPreferencesAsync(); + orderedStore = ProtocolVersionStore(slowPrefs); + await orderedStore.init(); + }); + + test('the newest snapshot is the one that lands', () async { + orderedStore.record(_nodeA, 1); + orderedStore.record(_nodeA, 2); + await orderedStore.pendingWrites; + + // Unserialised, the second (faster) write would land first and the first + // would overwrite it with the stale {_nodeA: 1}. + expect(jsonDecode(slowPrefs.strings[_key]!), {_nodeA: 2}); + }); + + test('a write issued before clear() does not resurrect the map', () async { + orderedStore.record(_nodeA, 2); + await orderedStore.clear(); + + expect(slowPrefs.strings[_key], isNull); + expect(orderedStore.versionFor(_nodeA), isNull); + }); + }); } diff --git a/test/shared/utils/nip59_authentication_test.dart b/test/shared/utils/nip59_authentication_test.dart index 0a25deff..fbd5af2f 100644 --- a/test/shared/utils/nip59_authentication_test.dart +++ b/test/shared/utils/nip59_authentication_test.dart @@ -52,14 +52,21 @@ void main() { recipientPubKey: recipient.public, ); - // It decrypts perfectly — nothing about the encryption is broken. + // It decrypts perfectly — nothing about the encryption is broken, so the + // rejection must come from the author pin and nowhere else. await expectLater( NostrUtils.decryptNIP59Event( forged, recipient.private, expectedAuthor: node.public, ), - throwsA(isA()), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Unexpected seal author'), + ), + ), ); }); @@ -80,11 +87,17 @@ void main() { ) as Map; sealJson['sig'] = 'f' * 128; + // One ephemeral keypair for both the encryption and the wrap author: + // NIP-44 derives its key by ECDH, so the recipient decrypts using + // `tamperedWrap.pubkey`. Two different keypairs here would make the + // *outer* decryption fail, and the test would pass without ever reaching + // the seal-signature check it exists to cover. + final wrapperKeys = NostrUtils.generateKeyPair(); final tamperedWrap = await NostrUtils.createWrap( - NostrUtils.generateKeyPair(), + wrapperKeys, await NostrUtils.encryptNIP44( jsonEncode(sealJson), - NostrUtils.generateKeyPair().private, + wrapperKeys.private, recipient.public, ), recipient.public, @@ -96,7 +109,13 @@ void main() { recipient.private, expectedAuthor: node.public, ), - throwsA(isA()), + throwsA( + isA().having( + (e) => e.message, + 'message', + 'Invalid seal signature', + ), + ), ); }); @@ -144,7 +163,13 @@ void main() { await expectLater( forged.unWrap(recipient.private, expectedAuthor: node.public), - throwsA(isA()), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Unexpected seal author'), + ), + ), ); }); }); From 63041b754006e73c7c10e0548ccb56b0c690e790 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 19:08:44 -0300 Subject: [PATCH 09/12] fix: request info event without order history cutoff to prevent protocol version loss Split the subscription into two filters: one for orders with the existing time bound, one for kind-38385 info events without `since`. Info events are addressable, so a relay holds exactly one copy per node; a combined filter would hide it once the node has been up longer than the window, leaving `protocol_version` unknown for the whole session and stranding the client on kind 14 against a v1 node now that unknown resolves to v2. --- .../repositories/open_orders_repository.dart | 28 +++++++++++---- .../open_orders_info_event_test.dart | 36 +++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/lib/data/repositories/open_orders_repository.dart b/lib/data/repositories/open_orders_repository.dart index b14aa2d1..db793b05 100644 --- a/lib/data/repositories/open_orders_repository.dart +++ b/lib/data/repositories/open_orders_repository.dart @@ -67,14 +67,28 @@ class OpenOrdersRepository implements OrderRepository { final filterTime = DateTime.now().subtract(Duration(hours: orderFilterDurationHours)); - final filter = NostrFilter( - kinds: [orderEventKind, infoEventKind], - since: filterTime, - authors: [_settings.mostroPublicKey], - ); - + // Two filters, not one. The order history is deliberately bounded to a + // recent window, but the info event must not inherit that bound: kind + // 38385 is addressable, so the node publishes it once at startup and the + // relay keeps only that copy. A node that has been up longer than the + // window has an info event older than `since`, and a combined filter would + // make the relay withhold it — leaving `protocol_version` unknown for the + // whole session. That used to be harmless because unknown meant v1; now + // that unknown resolves to v2 it would strand the client on kind 14 + // against a node that only listens on kind 1059. final request = NostrRequest( - filters: [filter], + filters: [ + NostrFilter( + kinds: [orderEventKind], + since: filterTime, + authors: [_settings.mostroPublicKey], + ), + NostrFilter( + kinds: [infoEventKind], + authors: [_settings.mostroPublicKey], + limit: 1, + ), + ], ); _subscription = _nostrService.subscribeToEvents(request).listen((event) { diff --git a/test/data/repositories/open_orders_info_event_test.dart b/test/data/repositories/open_orders_info_event_test.dart index bc803b89..053b35d9 100644 --- a/test/data/repositories/open_orders_info_event_test.dart +++ b/test/data/repositories/open_orders_info_event_test.dart @@ -264,4 +264,40 @@ void main() { expect(repository.mostroInstance!.id, nextInfo.id); }); }); + + group('OpenOrdersRepository subscription filters', () { + // The order history window is a UI concern; the info event is a protocol + // one. Kind 38385 is addressable, so a relay holds exactly one copy per + // node and a `since` bound simply hides it once the node has been up + // longer than the window. With the safe default now resolving to v2, an + // unseen info event strands the client on kind 14 against a v1 node. + test('requests the info event without the order history cutoff', () { + buildRepository(); + + final request = + verify(mockNostrService.subscribeToEvents(captureAny)).captured.single + as NostrRequest; + + final infoFilter = request.filters.singleWhere( + (f) => f.kinds!.contains(infoEventKind), + ); + expect(infoFilter.since, isNull); + expect(infoFilter.authors, [nodeKeys.public]); + expect(infoFilter.kinds, [infoEventKind]); + }); + + test('still bounds the order history', () { + buildRepository(); + + final request = + verify(mockNostrService.subscribeToEvents(captureAny)).captured.single + as NostrRequest; + + final orderFilter = request.filters.singleWhere( + (f) => f.kinds!.contains(orderEventKind), + ); + expect(orderFilter.since, isNotNull); + expect(orderFilter.kinds, [orderEventKind]); + }); + }); } From 841cdc90a2bb523c926d79fbcf4dce71e35ca9a1 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 19:15:57 -0300 Subject: [PATCH 10/12] feat: distinguish absent protocol_version from malformed to prevent misinterpreting unknown as legacy --- lib/features/mostro/mostro_instance.dart | 19 +++++++--- .../mostro/protocol_version_store.dart | 34 +++++++++++++++--- .../anchored_transport_resolution_test.dart | 36 +++++++++++++++++++ 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/lib/features/mostro/mostro_instance.dart b/lib/features/mostro/mostro_instance.dart index 8c5ebb2c..dcbb86ef 100644 --- a/lib/features/mostro/mostro_instance.dart +++ b/lib/features/mostro/mostro_instance.dart @@ -179,15 +179,26 @@ extension MostroInstanceExtensions on NostrEvent { /// Parses the wire transport version from the `protocol_version` tag (§2). /// - /// Returns `null` when the tag is absent or unparseable. Callers treat - /// `null` as legacy v1 (NIP-59 gift wrap); the nullable form is preserved so - /// the transport resolver can distinguish "not advertised" from an explicit - /// version when deciding whether to log a version-skew downgrade. + /// Returns `null` when the tag is absent, empty or unparseable. Those are + /// not the same fact, and callers deciding a transport must not treat them + /// alike — pair this with [advertisesProtocolVersion] to tell them apart. int? get protocolVersion { final raw = _getOptionalTagValue('protocol_version'); return raw == null ? null : int.tryParse(raw); } + /// Whether the event carries a `protocol_version` tag at all, regardless of + /// whether its value parses. + /// + /// This is the half of the story [protocolVersion] cannot tell. A daemon + /// before v0.18.0 emits no tag, and on a verified event that silence *is* an + /// assertion of v1. A tag holding `""` or `abc` asserts nothing: the node + /// meant to state a version and the value is unusable, so the client has no + /// evidence and must fall back to its safe default rather than read the + /// malformed value as legacy and pair itself with gift wrap. + bool get advertisesProtocolVersion => + tags?.any((t) => t.isNotEmpty && t[0] == 'protocol_version') ?? false; + /// Parses the anti-abuse bond policy from the `bond_enabled` tag. /// /// - Tag absent → [BondPolicy.unsupported] (legacy daemon). diff --git a/lib/features/mostro/protocol_version_store.dart b/lib/features/mostro/protocol_version_store.dart index 6a22847b..eb7d66f7 100644 --- a/lib/features/mostro/protocol_version_store.dart +++ b/lib/features/mostro/protocol_version_store.dart @@ -1,6 +1,7 @@ import 'dart:async'; import 'dart:convert'; +import 'package:dart_nostr/dart_nostr.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:mostro_mobile/data/models/enums/storage_keys.dart'; import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; @@ -207,10 +208,7 @@ int? anchoredProtocolVersionFor(Ref ref) { final mostroPubkey = ref.read(settingsProvider).mostroPublicKey; final remembered = ref.read(protocolVersionStoreProvider).versionFor(mostroPubkey); - final advertised = infoEvent == null - ? null - : (infoEvent.protocolVersion ?? kLegacyProtocolVersion); - return anchoredProtocolVersion(advertised, remembered); + return anchoredProtocolVersion(_advertisedBy(infoEvent), remembered); } catch (e) { // Null resolves to the safe default rather than to v1, so failing to read // the node's state cannot be turned into a downgrade. @@ -218,3 +216,31 @@ int? anchoredProtocolVersionFor(Ref ref) { return null; } } + +/// What [infoEvent] actually asserts about its transport, in the three states +/// the tag can be in. +/// +/// - No info event, or a tag that is present but unusable → null. Both are an +/// absence of evidence, and [resolveTransport] maps null to the safe +/// default. A malformed value must land here rather than in the legacy +/// branch: reading `protocol_version=abc` as v1 would pair the client with +/// gift wrap against a node that may well be speaking NIP-44. +/// - Tag absent entirely → [kLegacyProtocolVersion]. Silence from a verified +/// event is a legacy daemon asserting v1. +/// - Tag present and parseable → that version, whatever it is; +/// [resolveTransport] decides what to do with one it does not speak. +int? _advertisedBy(NostrEvent? infoEvent) { + if (infoEvent == null) return null; + + final parsed = infoEvent.protocolVersion; + if (parsed != null) return parsed; + + if (infoEvent.advertisesProtocolVersion) { + logger.w( + 'Node ${infoEvent.pubkey} advertises an unparseable protocol_version; ' + 'treating the transport as unknown rather than legacy', + ); + return null; + } + return kLegacyProtocolVersion; +} diff --git a/test/features/mostro/anchored_transport_resolution_test.dart b/test/features/mostro/anchored_transport_resolution_test.dart index 444baa9c..3792715f 100644 --- a/test/features/mostro/anchored_transport_resolution_test.dart +++ b/test/features/mostro/anchored_transport_resolution_test.dart @@ -166,4 +166,40 @@ void main() { expect(resolveTransport(anchored()), Transport.giftWrap); }); }); + + // Three states, not two. `protocolVersion` returns null for an absent tag + // and for an unusable one, and only the first is the node asserting v1. + group('a malformed protocol_version tag', () { + test('an unparseable value falls back to the safe default', () async { + await deliver(_infoEvent(nodeKeys, protocolVersion: 'abc')); + + expect(anchored(), isNull); + expect(resolveTransport(anchored()), kDefaultTransport); + }); + + test('an empty value falls back to the safe default', () async { + await deliver(_infoEvent(nodeKeys, protocolVersion: '')); + + expect(anchored(), isNull); + expect(resolveTransport(anchored()), kDefaultTransport); + }); + + test('does not lower a node already verified at v2', () async { + container.read(protocolVersionStoreProvider).record(nodeKeys.public, 2); + + await deliver(_infoEvent(nodeKeys, protocolVersion: 'abc')); + + expect(anchored(), 2); + expect(resolveTransport(anchored()), Transport.nip44); + }); + + test('a version this client does not speak is not read as legacy', + () async { + await deliver(_infoEvent(nodeKeys, protocolVersion: '99')); + + // Parseable, so it reaches resolveTransport, which degrades upwards. + expect(anchored(), 99); + expect(resolveTransport(anchored()), kDefaultTransport); + }); + }); } From 8a62ab0b745afd1a37e85fb1fdeb44d0d2cd49de Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Wed, 19 Aug 2026 19:18:45 -0300 Subject: [PATCH 11/12] feat: apply NIP-01 addressable event ordering to info event replacement Introduce _supersedesCurrentInfo, which implements NIP-01's replacement rule for addressable events: higher created_at wins, and a tie goes to the lower id. The tie-break ensures all clients converge on the same copy when a node publishes multiple events within the same second, preventing relay race conditions from pinning different configs across sessions while still rejecting exact re-deliveries. --- .../repositories/open_orders_repository.dart | 48 ++++++++++--- .../open_orders_info_event_test.dart | 67 +++++++++++++++++++ 2 files changed, 104 insertions(+), 11 deletions(-) diff --git a/lib/data/repositories/open_orders_repository.dart b/lib/data/repositories/open_orders_repository.dart index db793b05..61e9d8eb 100644 --- a/lib/data/repositories/open_orders_repository.dart +++ b/lib/data/repositories/open_orders_repository.dart @@ -110,21 +110,18 @@ class OpenOrdersRepository implements OrderRepository { } // A valid signature says the node authored this event, not that it // still reflects the node's configuration. Relays pick which events - // they serve and in what order, so without a monotonicity check the - // last one to arrive wins — letting a relay replay a genuinely signed - // but superseded info event to roll the advertised config back (most - // importantly `protocol_version` 2 -> 1). Only move forward in time. + // they serve and in what order, so without a replacement rule the last + // one to arrive wins — letting a relay replay a genuinely signed but + // superseded info event to roll the advertised config back (most + // importantly `protocol_version` 2 -> 1). // // Reset to null on instance switch (see updateSettings), so this never // blocks the newly selected node's own info event. - final currentCreatedAt = _mostroInstance?.createdAt; - final incomingCreatedAt = event.createdAt; - if (currentCreatedAt != null && - (incomingCreatedAt == null || - !incomingCreatedAt.isAfter(currentCreatedAt))) { + if (!_supersedesCurrentInfo(event)) { logger.d( - 'Ignoring kind-$infoEventKind info event from ${event.pubkey} ' - 'dated $incomingCreatedAt: not newer than $currentCreatedAt', + 'Ignoring kind-$infoEventKind info event ${event.id} from ' + '${event.pubkey} dated ${event.createdAt}: does not supersede ' + '${_mostroInstance?.id} dated ${_mostroInstance?.createdAt}', ); return; } @@ -164,6 +161,35 @@ class OpenOrdersRepository implements OrderRepository { }); } + /// Whether [candidate] replaces the info event currently in use, under + /// NIP-01's ordering for addressable events: the higher `created_at` wins, + /// and a tie goes to the lower id. + /// + /// The tie-break is what makes this converge. Two events sharing a second + /// are both genuinely the node's — they cleared the signature check — but + /// only one of them is the copy every other client will settle on, and a + /// pure "strictly newer" rule silently keeps whichever the fastest relay + /// happened to deliver. An exact re-delivery compares equal on both fields + /// and is still rejected, so this does not reopen the replay window the + /// check exists to close. + bool _supersedesCurrentInfo(NostrEvent candidate) { + final current = _mostroInstance; + if (current == null) return true; + + final candidateAt = candidate.createdAt; + if (candidateAt == null) return false; + final currentAt = current.createdAt; + if (currentAt == null) return true; + + if (candidateAt.isAfter(currentAt)) return true; + if (currentAt.isAfter(candidateAt)) return false; + + final candidateId = candidate.id; + final currentId = current.id; + if (candidateId == null || currentId == null) return false; + return candidateId.compareTo(currentId) < 0; + } + void _emitEvents() { if (!_eventStreamController.isClosed) { _eventStreamController.add(_events.values.toList()); diff --git a/test/data/repositories/open_orders_info_event_test.dart b/test/data/repositories/open_orders_info_event_test.dart index 053b35d9..86b810f7 100644 --- a/test/data/repositories/open_orders_info_event_test.dart +++ b/test/data/repositories/open_orders_info_event_test.dart @@ -300,4 +300,71 @@ void main() { expect(orderFilter.kinds, [orderEventKind]); }); }); + + // NIP-01 orders addressable events by created_at, and settles a tie on the + // lower id. Without the tie-break, whichever relay answers first pins the + // config for the session and two clients can disagree about what the node + // said. + group('OpenOrdersRepository info event created_at ties', () { + /// Two distinct info events sharing a timestamp, returned lower id first. + List tiedPair(DateTime at) { + final pair = []; + var protocolVersion = 2; + while (pair.length < 2) { + final candidate = _signedInfoEvent( + nodeKeys, + createdAt: at, + protocolVersion: '${protocolVersion++}', + ); + if (!pair.any((e) => e.id == candidate.id)) pair.add(candidate); + } + pair.sort((a, b) => a.id!.compareTo(b.id!)); + return pair; + } + + test('a tie is won by the lower id, whichever arrives first', () async { + final repository = buildRepository(); + final pair = tiedPair(DateTime.now()); + final lower = pair.first; + final higher = pair.last; + + eventController.add(higher); + await pumpEventQueue(); + expect(repository.mostroInstance!.id, higher.id); + + eventController.add(lower); + await pumpEventQueue(); + expect(repository.mostroInstance!.id, lower.id); + }); + + test('the higher id does not displace the lower one', () async { + final repository = buildRepository(); + final pair = tiedPair(DateTime.now()); + final lower = pair.first; + final higher = pair.last; + + eventController.add(lower); + await pumpEventQueue(); + + eventController.add(higher); + await pumpEventQueue(); + expect(repository.mostroInstance!.id, lower.id); + }); + + test('an exact re-delivery is still ignored', () async { + final repository = buildRepository(); + final info = _signedInfoEvent(nodeKeys, createdAt: DateTime.now()); + + final emitted = []; + final sub = repository.mostroInstanceStream.listen(emitted.add); + + eventController.add(info); + await pumpEventQueue(); + eventController.add(info); + await pumpEventQueue(); + + expect(emitted.length, 1); + await sub.cancel(); + }); + }); } From 14f83f263343bf6eb172f88c9058d6d3d4305185 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Thu, 20 Aug 2026 17:56:55 -0300 Subject: [PATCH 12/12] fix: defer protocol version persistence until init merges early records to prevent session loss --- .../mostro/protocol_version_store.dart | 31 ++++- .../mostro/protocol_version_store_test.dart | 106 ++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/lib/features/mostro/protocol_version_store.dart b/lib/features/mostro/protocol_version_store.dart index eb7d66f7..28cc47c2 100644 --- a/lib/features/mostro/protocol_version_store.dart +++ b/lib/features/mostro/protocol_version_store.dart @@ -74,9 +74,32 @@ class ProtocolVersionStore { /// Loads persisted versions into memory. Must complete before the first /// [versionFor] call for the ratchet to apply on a cold start; a store that /// failed to load simply knows nothing and reports null. + /// + /// Merges rather than replaces. A `SubscriptionManager` can exist before this + /// runs — `RelaysNotifier` builds one of its own during bootstrap, and every + /// instance feeds the info-event stream into [record] — so entries may + /// already have been ratcheted into memory. Overwriting them would drop the + /// higher version, and the next snapshot would carry that loss to disk: a + /// ratchet that forgets is the one thing this store must not be. Future init() async { - _versions = await _load(); + final loaded = await _load(); + final early = _versions; + _versions = loaded; _initialized = true; + + var merged = false; + early.forEach((pubkey, version) { + final current = _versions[pubkey]; + if (current == null || version > current) { + _versions[pubkey] = version; + merged = true; + } + }); + + // Only when something survived the load, so a normal cold start still + // costs no write. This is also the first write allowed through, so what + // lands on disk is the union rather than either half. + if (merged) _persist(); } bool get isInitialized => _initialized; @@ -114,7 +137,11 @@ class ProtocolVersionStore { _versions[pubkey] = version; logger.i('Recorded protocol_version $version for node $pubkey'); - _persist(); + // Nothing reaches disk before [init] has merged what is already there. + // A snapshot taken now would be of a map that has not seen storage yet, + // and writing it would erase every node this device had verified in an + // earlier session — the load that was going to rescue them has not run. + if (_initialized) _persist(); return true; } diff --git a/test/features/mostro/protocol_version_store_test.dart b/test/features/mostro/protocol_version_store_test.dart index ab082f33..bae7221e 100644 --- a/test/features/mostro/protocol_version_store_test.dart +++ b/test/features/mostro/protocol_version_store_test.dart @@ -77,6 +77,34 @@ class _ReorderingSharedPreferencesAsync implements SharedPreferencesAsync { throw UnimplementedError('${invocation.memberName}'); } +/// Delays only the read, so `init()` is still awaiting `_load()` while a +/// `record()` lands. Reproduces bootstrap order: `RelaysNotifier` builds a +/// `SubscriptionManager` — and with it the info-event feed into `record()` — +/// before `appInitializerProvider` gets to `init()`. +class _SlowReadSharedPreferencesAsync implements SharedPreferencesAsync { + final Map strings = {}; + + @override + Future getString(String key) async { + await Future.delayed(const Duration(milliseconds: 20)); + return 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}'); +} + const _nodeA = '9d9d0455a96871f2dc4289b8312429db2e925f167b37c77bf7b28014be235980'; const _nodeB = @@ -278,4 +306,82 @@ void main() { expect(orderedStore.versionFor(_nodeA), isNull); }); }); + group('ProtocolVersionStore records arriving before init', () { + late _SlowReadSharedPreferencesAsync slowRead; + late ProtocolVersionStore earlyStore; + + setUp(() { + slowRead = _SlowReadSharedPreferencesAsync(); + earlyStore = ProtocolVersionStore(slowRead); + }); + + test('a version recorded while loading is not dropped', () async { + final loading = earlyStore.init(); + earlyStore.record(_nodeA, 2); + await loading; + + expect(earlyStore.versionFor(_nodeA), 2); + }); + + test('and reaches disk instead of being erased by the next snapshot', + () async { + final loading = earlyStore.init(); + earlyStore.record(_nodeA, 2); + await loading; + + // A later record for a different node snapshots the whole map. If init() + // had dropped _nodeA, this write is what would carry the loss to disk. + earlyStore.record(_nodeB, 2); + await earlyStore.pendingWrites; + + expect(jsonDecode(slowRead.strings[_key]!), {_nodeA: 2, _nodeB: 2}); + }); + + test('the persisted value wins when it is the higher one', () async { + slowRead.strings[_key] = jsonEncode({_nodeA: 2}); + + final loading = earlyStore.init(); + // A legacy info event racing the load must not walk the ratchet back. + earlyStore.record(_nodeA, 1); + await loading; + + expect(earlyStore.versionFor(_nodeA), 2); + }); + + test('an early record for an unrelated node keeps the loaded ones', + () async { + slowRead.strings[_key] = jsonEncode({_nodeB: 2}); + + final loading = earlyStore.init(); + earlyStore.record(_nodeA, 2); + await loading; + + expect(earlyStore.versionFor(_nodeA), 2); + expect(earlyStore.versionFor(_nodeB), 2); + }); + + test('a record before init does not erase an earlier session', () async { + slowRead.strings[_key] = jsonEncode({_nodeB: 2}); + + // Bootstrap at its worst: the info event lands before init() is even + // called. A snapshot of the still-empty map would overwrite _nodeB. + earlyStore.record(_nodeA, 2); + await earlyStore.pendingWrites; + expect(jsonDecode(slowRead.strings[_key]!), {_nodeB: 2}); + + await earlyStore.init(); + await earlyStore.pendingWrites; + + expect(jsonDecode(slowRead.strings[_key]!), {_nodeB: 2, _nodeA: 2}); + expect(earlyStore.versionFor(_nodeB), 2); + }); + + test('a plain cold start writes nothing', () async { + await earlyStore.init(); + await earlyStore.pendingWrites; + + expect(slowRead.strings[_key], isNull); + }); + }); + }