diff --git a/lib/data/repositories/dispute_repository.dart b/lib/data/repositories/dispute_repository.dart index c2611f60..d33acc37 100644 --- a/lib/data/repositories/dispute_repository.dart +++ b/lib/data/repositories/dispute_repository.dart @@ -47,7 +47,8 @@ class DisputeRepository { // (v1 gift wrap kind 1059 / v2 NIP-44 direct kind 14), with PoW from the // Mostro instance. In reputation mode the master key and key index bind // the identity proof; full privacy omits both. - final mostroInstance = _ref.read(orderRepositoryProvider).mostroInstance; + final mostroInstance = + await _ref.read(orderRepositoryProvider).awaitMostroInstance(); if (mostroInstance == null) { logger.w( 'Mostro instance info unavailable, sending dispute with PoW 0 — ' diff --git a/lib/data/repositories/open_orders_repository.dart b/lib/data/repositories/open_orders_repository.dart index 64ea9de2..252e3061 100644 --- a/lib/data/repositories/open_orders_repository.dart +++ b/lib/data/repositories/open_orders_repository.dart @@ -12,6 +12,12 @@ const orderEventKind = 38383; const infoEventKind = 38385; const orderFilterDurationHours = 48; +/// How long a send path waits for the node's kind-38385 info event before +/// falling back to defaults. The info event normally arrives with the initial +/// order subscription; this only bounds a cold start against a slow relay and +/// stays well under the 10s orphan-session cleanup timer. +const mostroInstanceWaitTimeout = Duration(seconds: 3); + class OpenOrdersRepository implements OrderRepository { final NostrService _nostrService; NostrEvent? _mostroInstance; @@ -46,6 +52,36 @@ class OpenOrdersRepository implements OrderRepository { Stream get mostroInstanceStream => _mostroInstanceController.stream; + /// Returns the node's kind-38385 info event, waiting up to [timeout] for it + /// if it has not arrived yet. + /// + /// The info event carries both the PoW difficulty and the `protocol_version` + /// that selects the outbound transport, so a send issued before it lands + /// would have to guess both. Guessing the transport wrong is unrecoverable: + /// the node ignores the envelope it does not speak and nothing retries the + /// action. Send paths await this instead; a timeout still falls back to the + /// caller's defaults (PoW 0, [resolveTransport]'s v2 default) so an + /// unreachable node degrades rather than blocks the UI. + Future awaitMostroInstance({ + Duration timeout = mostroInstanceWaitTimeout, + }) async { + final cached = _mostroInstance; + if (cached != null) return cached; + if (_mostroInstanceController.isClosed) return null; + try { + return await _mostroInstanceController.stream.first.timeout(timeout); + } on TimeoutException { + logger.w( + 'Mostro instance info did not arrive within ' + '${timeout.inSeconds}s; proceeding with defaults', + ); + return null; + } catch (e) { + logger.w('Failed while waiting for Mostro instance info: $e'); + return null; + } + } + OpenOrdersRepository(this._nostrService, this._settings) { // Subscribe to orders and initialize data _subscribeToOrders(); diff --git a/lib/features/mostro/mostro_instance.dart b/lib/features/mostro/mostro_instance.dart index 8c5ebb2c..b511c437 100644 --- a/lib/features/mostro/mostro_instance.dart +++ b/lib/features/mostro/mostro_instance.dart @@ -39,9 +39,13 @@ class MostroInstance { final int maxOrdersPerResponse; /// Wire transport advertised via the `protocol_version` tag (§2 of the - /// transport v2 migration). Defaults to `1` (NIP-59 gift wrap) when the tag - /// is absent or unparseable, matching the legacy-daemon behaviour. - final int protocolVersion; + /// transport v2 migration), or `null` when the node does not advertise it. + /// + /// Kept nullable so "not advertised" stays distinguishable from an explicit + /// version: that is the distinction [resolveTransport] negotiates on, and + /// collapsing it to `1` here would also make the About screen report v1 for + /// a node the app is actually speaking v2 to. + final int? protocolVersion; /// Bond policy state. See [BondPolicy] for the three-state semantics. final BondPolicy bondPolicy; @@ -78,7 +82,7 @@ class MostroInstance { this.lndNodeUri, this.fiatCurrenciesAccepted, this.maxOrdersPerResponse, { - this.protocolVersion = 1, + this.protocolVersion, this.bondPolicy = BondPolicy.unsupported, this.bondApplyTo, this.bondSlashOnWaitingTimeout, @@ -111,7 +115,7 @@ class MostroInstance { event.lndNodeUri, event.fiatCurrenciesAccepted, event.maxOrdersPerResponse, - protocolVersion: event.protocolVersion ?? 1, + protocolVersion: event.protocolVersion, bondPolicy: event.bondPolicy, bondApplyTo: event.bondApplyTo, bondSlashOnWaitingTimeout: event.bondSlashOnWaitingTimeout, @@ -179,10 +183,10 @@ 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 or unparseable. [resolveTransport] + /// treats `null` as v2 (NIP-44), the live transport; the nullable form is + /// preserved so it can distinguish "not advertised" from an explicit + /// `protocol_version=1`, the one value that still selects gift wrap. int? get protocolVersion { final raw = _getOptionalTagValue('protocol_version'); return raw == null ? null : int.tryParse(raw); diff --git a/lib/features/mostro/transport.dart b/lib/features/mostro/transport.dart index 76c13efe..0ea27de2 100644 --- a/lib/features/mostro/transport.dart +++ b/lib/features/mostro/transport.dart @@ -16,25 +16,35 @@ enum Transport { giftWrap, nip44 } /// `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. +/// - `1` → [Transport.giftWrap] (v1, explicitly advertised). This is the only +/// input that selects the legacy gift wrap path. +/// - `null` → [Transport.nip44]. The tag is absent or the node info has not +/// been fetched yet. Advertising `protocol_version` is mandatory and gift +/// wrap is obsolete in the protocol, so "unknown" has exactly one sensible +/// answer: the live transport. Defaulting to v1 here used to cost a +/// kind-1059 REQ on every relay at every cold start, CLOSEd and re-REQd as +/// kind 14 the moment the info event landed. +/// - any other value → [Transport.nip44], logged at `warn`. The old rule +/// degraded to v1 as a version-skew guard; that guard was worth its cost +/// only while v1 was the live transport. A node advertising a version we do +/// not know (3, say) is far likelier to speak v2 than the obsolete v1, so +/// the safer guess is v2 — the `warn` still surfaces the skew. Transport resolveTransport(int? protocolVersion) { switch (protocolVersion) { - case 2: - return Transport.nip44; case 1: - case null: + // Legacy nodes only: the gift wrap transport is obsolete in the + // protocol and its code paths are scheduled for removal. return Transport.giftWrap; + case 2: + case null: + // v2 is the live transport. Defaulting to it when the node info has + // not arrived yet avoids a useless kind-1059 REQ at every cold start + // followed by a CLOSE + re-REQ once protocol_version resolves. + return Transport.nip44; default: logger.w( - 'Unsupported protocol_version $protocolVersion; ' - 'degrading to v1 gift wrap', + 'Unknown protocol_version $protocolVersion; assuming v2 (NIP-44)', ); - return Transport.giftWrap; + return Transport.nip44; } } diff --git a/lib/features/restore/restore_manager.dart b/lib/features/restore/restore_manager.dart index 4e8d1230..bb1463d3 100644 --- a/lib/features/restore/restore_manager.dart +++ b/lib/features/restore/restore_manager.dart @@ -36,6 +36,7 @@ import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; import 'package:mostro_mobile/shared/providers/navigation_notifier_provider.dart'; import 'package:mostro_mobile/shared/providers/nostr_service_provider.dart'; import 'package:mostro_mobile/shared/providers/notifications_history_repository_provider.dart'; +import 'package:mostro_mobile/features/mostro/transport.dart'; import 'package:mostro_mobile/shared/providers/order_repository_provider.dart'; import 'package:mostro_mobile/shared/providers/session_lifecycle_lock_provider.dart'; import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; @@ -219,25 +220,39 @@ class RestoreService { throw Exception('Temp trade key not initialized'); } - // Listen on both transports. The node info (kind 38385) that advertises - // protocol_version may not have loaded yet when restore starts, so we - // subscribe to v1 gift wrap (kind 1059) and v2 NIP-44 direct (kind 14) - // simultaneously rather than resolving a single transport — the node - // answers on whichever it speaks. (limit 0: only new events, no history.) + // Resolve the node's transport and open a single filter for it. + // _waitForNodeConnectivity() has already polled for the kind-38385 info + // event by this point, so protocol_version is normally known here. + // (limit 0: only new events, no history.) final mostroPubkey = ref.read(settingsProvider).mostroPublicKey; - final v1Filter = NostrFilter( - kinds: [1059], - p: [_tempTradeKey!.public], - limit: 0, - ); + final advertisedVersion = + ref.read(orderRepositoryProvider).mostroInstance?.protocolVersion; + final v2Filter = NostrFilter( kinds: [14], authors: [mostroPubkey], p: [_tempTradeKey!.public], limit: 0, ); + // No `authors` pin: NIP-59 signs the outer gift wrap with a per-event + // ephemeral key, so the node's pubkey never appears there. The sender is + // verified after decryption instead (decodeRestoreMessage). + final v1Filter = NostrFilter( + kinds: [1059], + p: [_tempTradeKey!.public], + limit: 0, + ); + + // Only when the info event never arrived do we fall back to listening on + // both transports, since the node then answers on whichever it speaks. + final filters = advertisedVersion == null + ? [v1Filter, v2Filter] + : switch (resolveTransport(advertisedVersion)) { + Transport.nip44 => [v2Filter], + Transport.giftWrap => [v1Filter], + }; - final request = NostrRequest(filters: [v1Filter, v2Filter]); + final request = NostrRequest(filters: filters); final stream = ref.read(nostrServiceProvider).subscribeToEvents(request); final subscription = stream.listen( @@ -272,7 +287,8 @@ class RestoreService { ); // Respect full privacy mode: if enabled, don't pass master key, wrap will be done just with trade key - final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance; + final mostroInstance = + await ref.read(orderRepositoryProvider).awaitMostroInstance(); final mostroPow = mostroInstance?.pow ?? 0; if (mostroInstance == null) { logger.w( @@ -369,7 +385,8 @@ class RestoreService { ); // Respect full privacy mode: if enabled, don't pass master key, wrap will be done just with trade key - final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance; + final mostroInstance = + await ref.read(orderRepositoryProvider).awaitMostroInstance(); final mostroPow = mostroInstance?.pow ?? 0; if (mostroInstance == null) { logger.w( @@ -450,7 +467,8 @@ class RestoreService { ); // Respect full privacy mode: if enabled, don't pass master key, wrap will be done just with trade key - final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance; + final mostroInstance = + await ref.read(orderRepositoryProvider).awaitMostroInstance(); final mostroPow = mostroInstance?.pow ?? 0; if (mostroInstance == null) { logger.w( @@ -1075,10 +1093,10 @@ class RestoreService { _masterKey = keyManager.masterKeyPair; _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. + // Wait for the node info event (kind 38385) before sending: it carries + // the PoW difficulty and the protocol_version that selects the outbound + // transport, and it also lets _createTempSubscription() open a single + // filter instead of listening on both transports. await _waitForNodeConnectivity(ref.read(settingsProvider).mostroPublicKey); _tempSubscription = await _createTempSubscription(); diff --git a/lib/features/settings/about_screen.dart b/lib/features/settings/about_screen.dart index df5ff5ca..b835314e 100644 --- a/lib/features/settings/about_screen.dart +++ b/lib/features/settings/about_screen.dart @@ -338,7 +338,7 @@ class AboutScreen extends ConsumerWidget { _buildInfoRowWithDialog( context, S.of(context)!.protocolVersion, - instance.protocolVersion.toString(), + instance.protocolVersion?.toString() ?? '—', S.of(context)!.protocolVersionExplanation, ), const SizedBox(height: 16), diff --git a/lib/features/subscriptions/subscription_manager.dart b/lib/features/subscriptions/subscription_manager.dart index 2eba1715..cb2fcfa8 100644 --- a/lib/features/subscriptions/subscription_manager.dart +++ b/lib/features/subscriptions/subscription_manager.dart @@ -98,8 +98,10 @@ 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 (v2 by default, see + /// [resolveTransport]) and would never fall back for a node that actually + /// advertises `protocol_version=1`. Re-subscribes only when the resolved + /// transport actually changes. void _initMostroInstanceListener() { try { _mostroInstanceListener = @@ -124,15 +126,17 @@ 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. + /// node's advertised `protocol_version` (§2, §4.1). Defaults to v2 NIP-44 + /// when the node info is not yet available or unreadable, matching + /// [resolveTransport]: gift wrap is obsolete, so falling back to it would + /// pin the session to a kind-1059 REQ the node never answers. Transport _resolveOrdersTransport() { try { final infoEvent = ref.read(orderRepositoryProvider).mostroInstance; return resolveTransport(infoEvent?.protocolVersion); } catch (e) { - logger.w('Failed to resolve orders transport, defaulting to v1: $e'); - return Transport.giftWrap; + logger.w('Failed to resolve orders transport, defaulting to v2: $e'); + return Transport.nip44; } } diff --git a/lib/services/mostro_service.dart b/lib/services/mostro_service.dart index 1b8b4711..3df7c3bc 100644 --- a/lib/services/mostro_service.dart +++ b/lib/services/mostro_service.dart @@ -393,8 +393,12 @@ class MostroService { Future publishOrder(MostroMessage order) async { final session = await _getSession(order); - // Read PoW difficulty from the connected Mostro instance (kind 38385) - final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance; + // Read PoW difficulty and protocol version from the connected Mostro + // instance (kind 38385), waiting for it if the info event has not landed + // yet: an outbound envelope sent on the transport the node does not speak + // is dropped silently and never retried. + final mostroInstance = + await ref.read(orderRepositoryProvider).awaitMostroInstance(); final difficulty = mostroInstance?.pow ?? 0; if (mostroInstance == null) { logger.w( @@ -404,7 +408,8 @@ 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. + // B). Nodes that advertise protocol_version 1 keep the gift-wrap path + // byte-for-byte. final event = await order.wrapForTransport( protocolVersion: mostroInstance?.protocolVersion, tradeKey: session.tradeKey, diff --git a/test/data/repositories/open_orders_repository_await_instance_test.dart b/test/data/repositories/open_orders_repository_await_instance_test.dart new file mode 100644 index 00000000..cbf3c793 --- /dev/null +++ b/test/data/repositories/open_orders_repository_await_instance_test.dart @@ -0,0 +1,93 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:dart_nostr/dart_nostr.dart'; +import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; +import 'package:mostro_mobile/data/repositories/open_orders_repository.dart'; +import 'package:mostro_mobile/features/settings/settings.dart'; + +import '../../mocks.mocks.dart'; + +/// Regression tests for [OpenOrdersRepository.awaitMostroInstance]. +/// +/// Send paths take the outbound transport from the node's kind-38385 info +/// event. Sending before it arrives means guessing, and a wrong guess is +/// unrecoverable: the node ignores the envelope it does not speak and nothing +/// retries the action. These pin the three outcomes callers depend on. +void main() { + const mostroPubkey = + '0000000000000000000000000000000000000000000000000000000000000001'; + + late MockNostrService nostrService; + late StreamController eventController; + late OpenOrdersRepository repository; + + final settings = Settings( + relays: const [], + fullPrivacyMode: false, + mostroPublicKey: mostroPubkey, + defaultFiatCode: 'USD', + selectedLanguage: 'en', + ); + + NostrEvent infoEvent() => NostrEvent( + id: 'info-1', + kind: infoEventKind, + pubkey: mostroPubkey, + content: '', + sig: '', + createdAt: DateTime.now(), + tags: const [ + ['d', mostroPubkey], + ['z', 'info'], + ['protocol_version', '2'], + ], + ); + + setUp(() { + nostrService = MockNostrService(); + eventController = StreamController.broadcast(); + when(nostrService.isInitialized).thenReturn(true); + when(nostrService.subscribeToEvents(any)) + .thenAnswer((_) => eventController.stream); + repository = OpenOrdersRepository(nostrService, settings); + }); + + tearDown(() { + repository.dispose(); + eventController.close(); + }); + + test('returns the cached info event without waiting', () async { + eventController.add(infoEvent()); + await Future.delayed(Duration.zero); + + final resolved = await repository + .awaitMostroInstance(timeout: const Duration(seconds: 30)); + + expect(resolved, isNotNull); + expect(resolved!.protocolVersion, 2); + }); + + test('resolves once the info event arrives', () async { + final pending = repository.awaitMostroInstance( + timeout: const Duration(seconds: 5), + ); + + await Future.delayed(Duration.zero); + eventController.add(infoEvent()); + + final resolved = await pending; + expect(resolved, isNotNull); + expect(resolved!.protocolVersion, 2); + }); + + test('returns null on timeout instead of blocking the send', () async { + final resolved = await repository.awaitMostroInstance( + timeout: const Duration(milliseconds: 50), + ); + + expect(resolved, isNull); + }); +} diff --git a/test/features/mostro/mostro_instance_test.dart b/test/features/mostro/mostro_instance_test.dart index 23fbc849..9e1fd507 100644 --- a/test/features/mostro/mostro_instance_test.dart +++ b/test/features/mostro/mostro_instance_test.dart @@ -326,10 +326,10 @@ void main() { ); } - test('tag absent → getter null, model defaults to v1', () { + test('tag absent → getter null, model keeps null (not advertised)', () { final event = buildEvent(const []); expect(event.protocolVersion, isNull); - expect(MostroInstance.fromEvent(event).protocolVersion, 1); + expect(MostroInstance.fromEvent(event).protocolVersion, isNull); }); test('protocol_version="2" → v2', () { @@ -348,12 +348,12 @@ void main() { expect(MostroInstance.fromEvent(event).protocolVersion, 1); }); - test('unparseable value → getter null, model defaults to v1', () { + test('unparseable value → getter null, model keeps null', () { final event = buildEvent(const [ ['protocol_version', 'abc'], ]); expect(event.protocolVersion, isNull); - expect(MostroInstance.fromEvent(event).protocolVersion, 1); + expect(MostroInstance.fromEvent(event).protocolVersion, isNull); }); }); } diff --git a/test/features/mostro/transport_test.dart b/test/features/mostro/transport_test.dart index ebc51825..9a7a4a36 100644 --- a/test/features/mostro/transport_test.dart +++ b/test/features/mostro/transport_test.dart @@ -11,13 +11,15 @@ void main() { expect(resolveTransport(1), Transport.giftWrap); }); - test('null (tag absent / node info not yet fetched) → giftWrap', () { - expect(resolveTransport(null), Transport.giftWrap); + test('null (tag absent / node info not yet fetched) → nip44', () { + // Gift wrap is obsolete; defaulting to v2 avoids a useless kind-1059 + // REQ + resubscribe at every cold start while the node info loads. + expect(resolveTransport(null), Transport.nip44); }); - test('unsupported version → degrades to giftWrap', () { - expect(resolveTransport(3), Transport.giftWrap); - expect(resolveTransport(0), Transport.giftWrap); + test('unknown versions assume the live transport (nip44)', () { + expect(resolveTransport(3), Transport.nip44); + expect(resolveTransport(0), Transport.nip44); }); }); } diff --git a/test/features/subscriptions/subscription_filter_diff_test.dart b/test/features/subscriptions/subscription_filter_diff_test.dart index 717874d7..c0adb987 100644 --- a/test/features/subscriptions/subscription_filter_diff_test.dart +++ b/test/features/subscriptions/subscription_filter_diff_test.dart @@ -24,6 +24,8 @@ import '../../mocks.mocks.dart'; /// times per protocol step. Since the orders filter carries no `since`, each /// re-issue replayed the full gift-wrap history. The manager now skips the /// resubscribe when the filter identity (keys, transport, node) is unchanged. +const _mostroPubkey = 'mostro-pubkey'; + void main() { late MockNostrService nostrService; late MockOpenOrdersRepository orderRepository; @@ -196,8 +198,12 @@ void main() { // Deterministic on slow CI machines: wait for the first chat REQ to be // issued instead of assuming a fixed delay is enough, then allow a settle // window in which a duplicated REQ would land. - bool isChatRequest(NostrRequest r) => - r.filters.any((f) => (f.kinds ?? []).contains(14)); + // The orders REQ is kind 14 too since the transport defaults to v2, so + // match on the author instead of the kind alone: chat filters are keyed + // on the peer-derived K_sign pubkeys, never on the node's pubkey. + bool isChatRequest(NostrRequest r) => r.filters.any((f) => + (f.kinds ?? []).contains(14) && + !(f.authors ?? const []).contains(_mostroPubkey)); final deadline = DateTime.now().add(const Duration(seconds: 5)); while (!issuedRequests.any(isChatRequest) && DateTime.now().isBefore(deadline)) { @@ -228,7 +234,7 @@ class _FixedSettingsNotifier extends SettingsNotifier { state = Settings( relays: const [], fullPrivacyMode: false, - mostroPublicKey: 'mostro-pubkey', + mostroPublicKey: _mostroPubkey, ); } } diff --git a/test/services/mostro_service_test.dart b/test/services/mostro_service_test.dart index 6a72045f..1a55a99f 100644 --- a/test/services/mostro_service_test.dart +++ b/test/services/mostro_service_test.dart @@ -138,6 +138,9 @@ void main() { // Stub orderRepositoryProvider so publishOrder can read PoW difficulty final mockOrderRepo = MockOpenOrdersRepository(); when(mockOrderRepo.mostroInstance).thenReturn(null); + when(mockOrderRepo.awaitMostroInstance(timeout: anyNamed('timeout'))) + .thenAnswer((_) async => null); + when(mockOrderRepo.awaitMostroInstance()).thenAnswer((_) async => null); when(mockRef.read(orderRepositoryProvider)).thenReturn(mockOrderRepo); // Create mockSubscriptionManager with the stubbed mockRef