From b81d0c8fa589b510017aea4af857f27b6264420f Mon Sep 17 00:00:00 2001 From: Dev Date: Tue, 28 Jul 2026 03:29:40 +0300 Subject: [PATCH] fix: harden NDR lifecycle boundaries --- bitchat/Nostr/NostrIdentityBridge.swift | 53 +++- bitchat/Nostr/NostrRelayManager.swift | 82 +++-- bitchat/Services/NdrNostrService.swift | 5 +- .../NdrOutOfBandTransportTests.swift | 87 ++++++ .../NostrIdentityBridgeLifecycleTests.swift | 280 ++++++++++++++++++ .../Services/NostrRelayManagerTests.swift | 202 +++++++++++++ 6 files changed, 686 insertions(+), 23 deletions(-) create mode 100644 bitchatTests/Nostr/NostrIdentityBridgeLifecycleTests.swift diff --git a/bitchat/Nostr/NostrIdentityBridge.swift b/bitchat/Nostr/NostrIdentityBridge.swift index 2e18a235..f69d54ba 100644 --- a/bitchat/Nostr/NostrIdentityBridge.swift +++ b/bitchat/Nostr/NostrIdentityBridge.swift @@ -2,11 +2,22 @@ import BitFoundation import Foundation import CryptoKit +enum NostrIdentityBridgeError: Error, Equatable { + case identityReadUnavailable + case invalidStoredIdentity + case identityPersistenceFailed +} + /// Bridge between Noise and Nostr identities final class NostrIdentityBridge { private let keychainService = "chat.bitchat.nostr" private let currentIdentityKey = "nostr-current-identity" private let deviceSeedKey = "nostr-device-seed" + // Multiple production owners construct their own bridge. Creation and + // panic deletion of the shared current-identity keychain item must still + // be one process-wide lifecycle, or concurrent bridges can return + // different keys or an in-flight save can resurrect a wiped identity. + private static let identityLifecycleLock = NSLock() // In-memory cache to avoid transient keychain access issues private var deviceSeedCache: Data? // Cache derived identities to avoid repeated crypto during view rendering @@ -21,10 +32,26 @@ final class NostrIdentityBridge { /// Get or create the current Nostr identity func getCurrentNostrIdentity() throws -> NostrIdentity? { - // Check if we already have a Nostr identity - if let existingData = keychain.load(key: currentIdentityKey, service: keychainService), - let identity = try? JSONDecoder().decode(NostrIdentity.self, from: existingData) { + Self.identityLifecycleLock.lock() + defer { Self.identityLifecycleLock.unlock() } + + switch keychain.loadWithResult( + key: currentIdentityKey, + service: keychainService + ) { + case .success(let existingData): + guard let identity = try? JSONDecoder().decode( + NostrIdentity.self, + from: existingData + ) else { + throw NostrIdentityBridgeError.invalidStoredIdentity + } return identity + case .itemNotFound: + break + case .accessDenied, .deviceLocked, .authenticationFailed, + .otherError: + throw NostrIdentityBridgeError.identityReadUnavailable } // Generate new Nostr identity @@ -33,8 +60,21 @@ final class NostrIdentityBridge { // Store it let data = try JSONEncoder().encode(nostrIdentity) keychain.save(key: currentIdentityKey, data: data, service: keychainService, accessible: nil) - - return nostrIdentity + guard case .success(let persistedData) = keychain.loadWithResult( + key: currentIdentityKey, + service: keychainService + ), + persistedData == data, + let persistedIdentity = try? JSONDecoder().decode( + NostrIdentity.self, + from: persistedData + ), + persistedIdentity.publicKeyHex == nostrIdentity.publicKeyHex + else { + throw NostrIdentityBridgeError.identityPersistenceFailed + } + + return persistedIdentity } /// Get Nostr public key associated with a Noise public key @@ -49,6 +89,9 @@ final class NostrIdentityBridge { /// Clear all Nostr identity associations and current identity func clearAllAssociations() { + Self.identityLifecycleLock.lock() + defer { Self.identityLifecycleLock.unlock() } + // Must go through the injected keychain, not raw SecItem calls: // under test that keychain is in-memory, and a direct delete here // would wipe the developer's real Nostr identity on every test run. diff --git a/bitchat/Nostr/NostrRelayManager.swift b/bitchat/Nostr/NostrRelayManager.swift index 6013e5e6..98b81c3c 100644 --- a/bitchat/Nostr/NostrRelayManager.swift +++ b/bitchat/Nostr/NostrRelayManager.swift @@ -71,6 +71,7 @@ struct NostrRelayManagerDependencies { var torIsForeground: () -> Bool var awaitTorReady: (@escaping (Bool) -> Void) -> Void var makeSession: () -> NostrRelaySessionProtocol + var verifyEventSignature: @Sendable (NostrEvent) -> Bool var scheduleAfter: @Sendable (TimeInterval, @escaping @Sendable () -> Void) -> Void var now: () -> Date /// Uniform random value in [0, 1) used to jitter reconnect backoff. @@ -112,6 +113,7 @@ private extension NostrRelayManagerDependencies { } }, makeSession: { URLSessionAdapter(base: TorURLSession.shared.session) }, + verifyEventSignature: { $0.isValidSignature() }, scheduleAfter: { delay, action in DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: action) }, @@ -210,6 +212,12 @@ final class NostrRelayManager: ObservableObject { private var pendingSubscriptions: [String: [String: PendingSubscription]] = [:] // relay URL -> (subscription id -> pending REQ) private var pendingSubscriptionSequence: UInt64 = 0 private var messageHandlers: [String: (NostrEvent) -> Void] = [:] + // An EVENT is admitted before off-main signature verification and + // delivered later. The generation binds those two phases to the same + // logical subscription so unsubscribe/re-subscribe cannot retarget stale + // work to a replacement handler or poison its dedup state. + private var subscriptionGenerations: [String: UInt64] = [:] + private var nextSubscriptionGeneration: UInt64 = 0 private struct InboundEventKey: Hashable { let subscriptionID: String let eventID: String @@ -388,6 +396,7 @@ final class NostrRelayManager: ObservableObject { /// verification means a forged-signature copy can never poison the /// dedup cache and suppress the genuine event. private func ensureRelayInboundPipeline(for relayUrl: String) { + let verifyEventSignature = dependencies.verifyEventSignature let started = inboundRouter.startPipeline(for: relayUrl) { [weak self] stream in Task.detached(priority: .userInitiated) { for await frame in stream { @@ -395,21 +404,24 @@ final class NostrRelayManager: ObservableObject { guard let self else { return } switch parsed { case .event(let subId, let event): - guard await self.precheckInboundEvent( + guard let generation = await self.precheckInboundEvent( subscriptionID: subId, eventID: event.id, relayUrl: relayUrl - ) else { - continue - } - guard event.isValidSignature() else { + ) else { continue } + guard verifyEventSignature(event) else { SecureLogger.warning( "⚠️ Dropped invalid Nostr event id=\(event.id.prefix(16))… sub=\(subId) relay=\(relayUrl)", category: .session ) continue } - await self.deliverVerifiedInboundEvent(subscriptionID: subId, event: event, from: relayUrl) + await self.deliverVerifiedInboundEvent( + subscriptionID: subId, + subscriptionGeneration: generation, + event: event, + from: relayUrl + ) case .eose, .ok, .notice: await self.handleParsedMessage(parsed, from: relayUrl) } @@ -485,6 +497,7 @@ final class NostrRelayManager: ObservableObject { subscriptions.removeAll() pendingSubscriptions.removeAll() messageHandlers.removeAll() + subscriptionGenerations.removeAll() subscriptionRequestState.removeAll() eoseTrackers.removeAll() pendingEOSECallbacks.removeAll() @@ -764,8 +777,21 @@ final class NostrRelayManager: ObservableObject { return false } let requestState = SubscriptionRequestState(messageString: messageString, relayURLs: Set(urls)) + let previousRequestState = subscriptionRequestState[id] + if subscriptionGenerations[id] == nil + || previousRequestState != requestState + { + nextSubscriptionGeneration &+= 1 + subscriptionGenerations[id] = nextSubscriptionGeneration + removeRecentInboundEvents(forSubscriptionID: id) + duplicateInboundEventDropCountBySubscription.removeValue( + forKey: id + ) + } messageHandlers[id] = handler - if subscriptionRequestState[id] == requestState, subscriptionStateExists(id: id, requestState: requestState) { + if previousRequestState == requestState, + subscriptionStateExists(id: id, requestState: requestState) + { return true } @@ -874,6 +900,7 @@ final class NostrRelayManager: ObservableObject { /// Unsubscribe from a subscription func unsubscribe(id: String) { messageHandlers.removeValue(forKey: id) + subscriptionGenerations.removeValue(forKey: id) removeRecentInboundEvents(forSubscriptionID: id) duplicateInboundEventDropCountBySubscription.removeValue(forKey: id) subscriptionRequestState.removeValue(forKey: id) @@ -1287,24 +1314,49 @@ final class NostrRelayManager: ObservableObject { /// after the signature verifies (`deliverVerifiedInboundEvent`), so a /// forged-signature copy can never poison the dedup cache and suppress /// the genuine event. - private func precheckInboundEvent(subscriptionID: String, eventID: String, relayUrl: String) -> Bool { + private func precheckInboundEvent( + subscriptionID: String, + eventID: String, + relayUrl: String + ) -> UInt64? { if let index = relays.firstIndex(where: { $0.url == relayUrl }) { relays[index].messagesReceived += 1 } - guard !eventID.isEmpty else { return true } + guard messageHandlers[subscriptionID] != nil, + let generation = subscriptionGenerations[subscriptionID] + else { + // Relays are untrusted and can invent subscription IDs. Do not + // spend signature work or mutate dedup state for unsolicited + // events. + return nil + } + guard !eventID.isEmpty else { return generation } let key = InboundEventKey(subscriptionID: subscriptionID, eventID: eventID) if recentInboundEventKeys.contains(key) { recordDuplicateInboundEventDrop(subscriptionID: subscriptionID) - return false + return nil } - return true + return generation } /// Second main-actor hop, after off-main signature verification: /// authoritative check-and-record (the serial pipeline means the same /// event is never in flight twice, but the record must stay atomic with /// delivery) and handler dispatch. - private func deliverVerifiedInboundEvent(subscriptionID subId: String, event: NostrEvent, from relayUrl: String) { + private func deliverVerifiedInboundEvent( + subscriptionID subId: String, + subscriptionGeneration: UInt64, + event: NostrEvent, + from relayUrl: String + ) { + guard subscriptionGenerations[subId] == subscriptionGeneration, + let handler = messageHandlers[subId] + else { + // The event was admitted by a subscription that has since been + // retired. A replacement reusing the same wire ID is a different + // lifecycle and must neither receive nor dedup against this event. + return + } guard shouldDeliverInboundEvent(subscriptionID: subId, eventID: event.id) else { return } @@ -1315,11 +1367,7 @@ final class NostrRelayManager: ObservableObject { SecureLogger.debug("📥 Event #\(inboundEventLogCount) kind=\(event.kind) id=\(event.id.prefix(16))… relay=\(relayUrl)", category: .session) } } - if let handler = self.messageHandlers[subId] { - handler(event) - } else { - SecureLogger.warning("⚠️ No handler for subscription \(subId)", category: .session) - } + handler(event) } // Handle parsed non-EVENT messages on MainActor (state updates and handlers) diff --git a/bitchat/Services/NdrNostrService.swift b/bitchat/Services/NdrNostrService.swift index 41451081..777644ec 100644 --- a/bitchat/Services/NdrNostrService.swift +++ b/bitchat/Services/NdrNostrService.swift @@ -179,7 +179,10 @@ final class NdrNostrService { var onDecryptedMessage: NdrDecryptedMessageHandler? { didSet { - if oldValue == nil, onDecryptedMessage != nil { + // A retired lifecycle owner can leave a delivery deferred after + // returning `.retry`. Replacing that non-nil handler is the host's + // recovery boundary just as much as installing the first handler. + if onDecryptedMessage != nil { retryPendingDeliveries() } } diff --git a/bitchatTests/DoubleRatchet/NdrOutOfBandTransportTests.swift b/bitchatTests/DoubleRatchet/NdrOutOfBandTransportTests.swift index 091869dc..6f768c04 100644 --- a/bitchatTests/DoubleRatchet/NdrOutOfBandTransportTests.swift +++ b/bitchatTests/DoubleRatchet/NdrOutOfBandTransportTests.swift @@ -145,6 +145,8 @@ private final class ControllableNdrSessionMarkerStore: } } +private final class NdrDeliveryLifecycleOwner {} + @Suite(.serialized) struct NdrOutOfBandTransportTests { @Test("Double-ratchet bootstrap uses the coordinated wire value") @@ -1825,6 +1827,91 @@ struct NdrOutOfBandTransportTests { #expect(deliveries.count == 2) } + @Test("Replacing a retired delivery owner drains once across restart") + @MainActor + func replacingDeliveryOwnerWakesDeferredActionExactlyOnce() throws { + let alice = try NostrIdentity.generate() + let bob = try NostrIdentity.generate() + let aliceRelay = FakeRelayManager() + let aliceService = try makeService( + label: "delivery-owner-alice", + relay: aliceRelay + ) + let bobStorage = try makeTempDir(label: "delivery-owner-bob") + defer { try? FileManager.default.removeItem(at: bobStorage) } + var outer: NostrEvent? + + do { + let bobRelay = FakeRelayManager() + let bobService = NdrNostrService( + relayManager: bobRelay, + rolloutEnabled: true, + storageDirectoryProvider: { bobStorage } + ) + aliceService.configureIfNeeded(identity: alice) + bobService.configureIfNeeded(identity: bob) + try establishPairwiseSessions( + aliceService, + bobService, + firstIdentity: alice, + secondIdentity: bob, + firstRelay: aliceRelay, + secondRelay: bobRelay + ) + + var retiredOwner: NdrDeliveryLifecycleOwner? = + NdrDeliveryLifecycleOwner() + var retiredHandlerCalls = 0 + bobService.onDecryptedMessage = { [weak retiredOwner] _, completion in + retiredHandlerCalls += 1 + completion(retiredOwner == nil ? .retry : .consumed) + } + retiredOwner = nil + + aliceRelay.resetSentEvents() + guard case .sent = aliceService.send( + "bitchat1:lifecycle-owner", + to: bob.publicKeyHex + ) else { + Issue.record("Expected a pairwise send") + return + } + let sent = try #require( + aliceRelay.sentEvents.first(where: { $0.kind == 1060 }) + ) + outer = sent + bobService.processInboundRelayEvent(sent) + #expect(retiredHandlerCalls == 1) + + var replacementDeliveries = 0 + bobService.onDecryptedMessage = { message, completion in + #expect( + message.event.content == "bitchat1:lifecycle-owner" + ) + replacementDeliveries += 1 + completion(.consumed) + } + #expect(replacementDeliveries == 1) + } + + let restartedRelay = FakeRelayManager() + let restarted = NdrNostrService( + relayManager: restartedRelay, + rolloutEnabled: true, + storageDirectoryProvider: { bobStorage } + ) + var deliveriesAfterRestart = 0 + restarted.onDecryptedMessage = { _, completion in + deliveriesAfterRestart += 1 + completion(.consumed) + } + restarted.configureIfNeeded(identity: bob) + if let outer { + restarted.processInboundRelayEvent(outer) + } + #expect(deliveriesAfterRestart == 0) + } + @Test("Switching identities isolates invites and persisted ratchet state") @MainActor func identitySwitchIsolatesPersistedState() throws { diff --git a/bitchatTests/Nostr/NostrIdentityBridgeLifecycleTests.swift b/bitchatTests/Nostr/NostrIdentityBridgeLifecycleTests.swift new file mode 100644 index 00000000..b6d9a3b1 --- /dev/null +++ b/bitchatTests/Nostr/NostrIdentityBridgeLifecycleTests.swift @@ -0,0 +1,280 @@ +import BitFoundation +import Foundation +import Testing +@testable import bitchat + +@Suite("Nostr identity lifecycle", .serialized) +struct NostrIdentityBridgeLifecycleTests { + private static let identityKey = "nostr-current-identity" + private static let service = "chat.bitchat.nostr" + + @Test("A truly absent identity is created durably and reused") + func firstRunCreatesDurableIdentity() throws { + let keychain = MockKeychain() + let bridge = NostrIdentityBridge(keychain: keychain) + + let firstRead = try bridge.getCurrentNostrIdentity() + let secondRead = try bridge.getCurrentNostrIdentity() + let created = try #require(firstRead) + let restored = try #require(secondRead) + + #expect(restored.publicKeyHex == created.publicKeyHex) + guard case .success(let stored) = keychain.loadWithResult( + key: Self.identityKey, + service: Self.service + ) else { + Issue.record("Expected a durable identity after first-run creation") + return + } + #expect( + try JSONDecoder().decode( + NostrIdentity.self, + from: stored + ).publicKeyHex == created.publicKeyHex + ) + } + + @Test("Protected-data read failures never mint a replacement identity") + func protectedDataFailureFailsClosed() { + let keychain = MockKeychain() + keychain.simulatedGenericReadError = .deviceLocked + let bridge = NostrIdentityBridge(keychain: keychain) + + #expect(throws: (any Error).self) { + _ = try bridge.getCurrentNostrIdentity() + } + } + + @Test("Transient keychain read failures never mint a replacement identity") + func transientReadFailureFailsClosed() { + let keychain = MockKeychain() + keychain.simulatedGenericReadError = .otherError(-1) + let bridge = NostrIdentityBridge(keychain: keychain) + + #expect(throws: (any Error).self) { + _ = try bridge.getCurrentNostrIdentity() + } + } + + @Test("An undurable first-run identity is never returned") + func saveFailureFailsClosed() { + let keychain = MockKeychain() + keychain.simulatedGenericSaveFailureKeys.insert(Self.identityKey) + let bridge = NostrIdentityBridge(keychain: keychain) + + #expect(throws: (any Error).self) { + _ = try bridge.getCurrentNostrIdentity() + } + } + + @Test("Concurrent bridge instances return one durable identity") + func concurrentBridgeInstancesReturnSameIdentity() async throws { + let keychain = BlockingIdentityLifecycleKeychain() + let firstBridge = NostrIdentityBridge(keychain: keychain) + let secondBridge = NostrIdentityBridge(keychain: keychain) + + let firstTask = Task.detached { + try firstBridge.getCurrentNostrIdentity() + } + guard keychain.waitForFirstSave() else { + keychain.releaseFirstSave() + Issue.record("First identity creation never reached persistence") + return + } + + let secondTask = Task.detached { + try secondBridge.getCurrentNostrIdentity() + } + // With instance-local locking the second bridge can create and read a + // different identity while the first save is paused. With the shared + // lifecycle lock it remains outside the keychain until the first + // identity is durable. + _ = keychain.waitForSecondCreation() + keychain.releaseFirstSave() + + let first = try #require(try await firstTask.value) + let second = try #require(try await secondTask.value) + #expect(first.publicKeyHex == second.publicKeyHex) + #expect(keychain.identitySaveCount == 1) + } + + @Test("Panic clear cannot be overtaken by an in-flight identity save") + func panicClearSerializesWithInFlightCreate() async throws { + let keychain = BlockingIdentityLifecycleKeychain() + let creatingBridge = NostrIdentityBridge(keychain: keychain) + let clearingBridge = NostrIdentityBridge(keychain: keychain) + + let createTask = Task.detached { + try creatingBridge.getCurrentNostrIdentity() + } + guard keychain.waitForFirstSave() else { + keychain.releaseFirstSave() + Issue.record("Identity creation never reached persistence") + return + } + + let clearTask = Task.detached { + clearingBridge.clearAllAssociations() + } + // Before lifecycle serialization, panic deletion completes while the + // pre-panic save is paused and that save can resurrect the identity. + _ = keychain.waitForDeleteAll() + keychain.releaseFirstSave() + + _ = try await createTask.value + await clearTask.value + + guard case .itemNotFound = keychain.loadWithResult( + key: Self.identityKey, + service: Self.service + ) else { + Issue.record("A pre-panic identity was saved after panic clear") + return + } + } +} + +private final class BlockingIdentityLifecycleKeychain: + KeychainManagerProtocol, + @unchecked Sendable +{ + private let lock = NSLock() + private let firstSaveEntered = DispatchSemaphore(value: 0) + private let firstSaveRelease = DispatchSemaphore(value: 0) + private let secondCreationRead = DispatchSemaphore(value: 0) + private let deleteAllCompleted = DispatchSemaphore(value: 0) + private var serviceStorage: [String: [String: Data]] = [:] + private var saveCount = 0 + private var didSignalSecondCreation = false + + var identitySaveCount: Int { + lock.withLock { saveCount } + } + + func waitForFirstSave() -> Bool { + firstSaveEntered.wait(timeout: .now() + 1) == .success + } + + func waitForSecondCreation() -> Bool { + secondCreationRead.wait(timeout: .now() + 1) == .success + } + + func waitForDeleteAll() -> Bool { + deleteAllCompleted.wait(timeout: .now() + 1) == .success + } + + func releaseFirstSave() { + firstSaveRelease.signal() + } + + func saveIdentityKey(_ keyData: Data, forKey key: String) -> Bool { + save(key: key, data: keyData, service: "identity", accessible: nil) + return true + } + + func getIdentityKey(forKey key: String) -> Data? { + load(key: key, service: "identity") + } + + func deleteIdentityKey(forKey key: String) -> Bool { + delete(key: key, service: "identity") + return true + } + + func deleteAllKeychainData() -> Bool { + lock.withLock { + serviceStorage.removeAll() + } + return true + } + + func secureClear(_ data: inout Data) { + data = Data() + } + + func secureClear(_ string: inout String) { + string = "" + } + + func verifyIdentityKeyExists() -> Bool { + getIdentityKey(forKey: "identity_noiseStaticKey") != nil + } + + func getIdentityKeyWithResult(forKey key: String) -> KeychainReadResult { + guard let data = getIdentityKey(forKey: key) else { + return .itemNotFound + } + return .success(data) + } + + func saveIdentityKeyWithResult( + _ keyData: Data, + forKey key: String + ) -> KeychainSaveResult { + saveIdentityKey(keyData, forKey: key) ? .success : .otherError(-1) + } + + func save( + key: String, + data: Data, + service: String, + accessible _: CFString? + ) { + let isFirstSave = lock.withLock { + saveCount += 1 + return saveCount == 1 + } + if isFirstSave { + firstSaveEntered.signal() + firstSaveRelease.wait() + } + lock.withLock { + serviceStorage[service, default: [:]][key] = data + } + } + + func load(key: String, service: String) -> Data? { + guard case .success(let data) = loadWithResult( + key: key, + service: service + ) else { + return nil + } + return data + } + + func loadWithResult( + key: String, + service: String + ) -> KeychainReadResult { + let result: KeychainReadResult + let shouldSignalSecondCreation: Bool + (result, shouldSignalSecondCreation) = lock.withLock { + guard let data = serviceStorage[service]?[key] else { + return (.itemNotFound, false) + } + let shouldSignal = saveCount >= 2 && !didSignalSecondCreation + if shouldSignal { + didSignalSecondCreation = true + } + return (.success(data), shouldSignal) + } + if shouldSignalSecondCreation { + secondCreationRead.signal() + } + return result + } + + func delete(key: String, service: String) { + _ = lock.withLock { + serviceStorage[service]?.removeValue(forKey: key) + } + } + + func deleteAll(service: String) { + _ = lock.withLock { + serviceStorage.removeValue(forKey: service) + } + deleteAllCompleted.signal() + } +} diff --git a/bitchatTests/Services/NostrRelayManagerTests.swift b/bitchatTests/Services/NostrRelayManagerTests.swift index 379b3045..5d889113 100644 --- a/bitchatTests/Services/NostrRelayManagerTests.swift +++ b/bitchatTests/Services/NostrRelayManagerTests.swift @@ -1198,6 +1198,165 @@ final class NostrRelayManagerTests: XCTestCase { XCTAssertTrue(counted) } + func test_receiveEvent_withoutHandlerDoesNotPoisonFutureSubscriptionReplay() async throws { + let relayURL = "wss://future-subscription.example" + let context = makeContext(permission: .denied) + let event = try makeSignedEvent(content: "future subscription") + var barrierEOSECount = 0 + + context.manager.subscribe( + filter: makeFilter(), + id: "barrier", + relayUrls: [relayURL], + handler: { _ in }, + onEOSE: { barrierEOSECount += 1 } + ) + let barrierSubscribed = await waitUntil { + context.sessionFactory.latestConnection(for: relayURL)? + .sentStrings.count == 1 + } + XCTAssertTrue(barrierSubscribed) + + let connection = try XCTUnwrap( + context.sessionFactory.latestConnection(for: relayURL) + ) + try connection.emitEventMessage( + subscriptionID: "future", + event: event + ) + // The relay pipeline is serial. Observing this EOSE proves the + // unsolicited event ahead of it has finished verification and its + // delivery-side bookkeeping before the real subscription is created. + try connection.emitEOSE(subscriptionID: "barrier") + let unsolicitedSettled = await waitUntil { + barrierEOSECount == 1 + && context.manager.relays.first(where: { + $0.url == relayURL + })?.messagesReceived == 1 + } + XCTAssertTrue(unsolicitedSettled) + + var receivedIDs: [String] = [] + context.manager.subscribe( + filter: makeFilter(), + id: "future", + relayUrls: [relayURL] + ) { replayed in + receivedIDs.append(replayed.id) + } + let futureSubscribed = await waitUntil { + connection.sentStrings.count == 2 + } + XCTAssertTrue(futureSubscribed) + + try connection.emitEventMessage( + subscriptionID: "future", + event: event + ) + let replaySettled = await waitUntil { + receivedIDs == [event.id] + || context.manager + .debugDuplicateInboundEventDropCount( + forSubscriptionID: "future" + ) == 1 + } + XCTAssertTrue(replaySettled) + XCTAssertEqual(receivedIDs, [event.id]) + XCTAssertEqual( + context.manager.debugDuplicateInboundEventDropCount( + forSubscriptionID: "future" + ), + 0 + ) + } + + func test_receiveEvent_fromRetiredSubscriptionCannotReachReplacement() async throws { + let relayURL = "wss://retired-subscription.example" + let verifier = ControllableEventSignatureVerifier() + let context = makeContext( + permission: .denied, + verifyEventSignature: { verifier.verify($0) } + ) + let event = try makeSignedEvent(content: "retired generation") + var retiredReceivedIDs: [String] = [] + + context.manager.subscribe( + filter: makeFilter(), + id: "replaceable", + relayUrls: [relayURL] + ) { received in + retiredReceivedIDs.append(received.id) + } + let initialSubscriptionSent = await waitUntil { + context.sessionFactory.latestConnection(for: relayURL)? + .sentStrings.count == 1 + } + XCTAssertTrue(initialSubscriptionSent) + + let connection = try XCTUnwrap( + context.sessionFactory.latestConnection(for: relayURL) + ) + verifier.blockNextVerification() + try connection.emitEventMessage( + subscriptionID: "replaceable", + event: event + ) + let verificationBlocked = await waitUntil { + verifier.isVerificationBlocked + } + guard verificationBlocked else { + verifier.releaseBlockedVerification() + XCTFail("Event never reached signature verification") + return + } + + context.manager.unsubscribe(id: "replaceable") + var replacementReceivedIDs: [String] = [] + var replacementEOSECount = 0 + context.manager.subscribe( + filter: makeFilter(), + id: "replaceable", + relayUrls: [relayURL], + handler: { received in + replacementReceivedIDs.append(received.id) + }, + onEOSE: { replacementEOSECount += 1 } + ) + let replacementSubscriptionSent = await waitUntil { + connection.sentStrings.count == 3 + } + XCTAssertTrue(replacementSubscriptionSent) + + // The EOSE is queued behind the blocked event on the same serial relay + // pipeline, so its callback proves the stale event's delivery phase + // completed before assertions or replay. + try connection.emitEOSE(subscriptionID: "replaceable") + verifier.releaseBlockedVerification() + let staleEventSettled = await waitUntil { + replacementEOSECount == 1 + } + XCTAssertTrue(staleEventSettled) + XCTAssertTrue(retiredReceivedIDs.isEmpty) + XCTAssertTrue( + replacementReceivedIDs.isEmpty, + "An event admitted by the retired subscription reached its replacement" + ) + + try connection.emitEventMessage( + subscriptionID: "replaceable", + event: event + ) + let replaySettled = await waitUntil { + replacementReceivedIDs == [event.id] + || context.manager + .debugDuplicateInboundEventDropCount( + forSubscriptionID: "replaceable" + ) == 1 + } + XCTAssertTrue(replaySettled) + XCTAssertEqual(replacementReceivedIDs, [event.id]) + } + func test_noticeAndMalformedMessages_keepReceiveLoopAliveForLaterEvents() async throws { let relayURL = "wss://parser.example" let context = makeContext(permission: .denied) @@ -1990,6 +2149,9 @@ final class NostrRelayManagerTests: XCTestCase { torIsForeground: Bool = true, notificationCenter: NotificationCenter = NotificationCenter(), customRelays: MutableRelayList = MutableRelayList(urls: []), + verifyEventSignature: @escaping @Sendable (NostrEvent) -> Bool = { + $0.isValidSignature() + }, jitterUnit: @escaping () -> Double = { 0.5 } // 0.5 -> jitter factor 1.0 (no jitter) ) -> RelayManagerTestContext { let permissionSubject = CurrentValueSubject(permission) @@ -2013,6 +2175,7 @@ final class NostrRelayManagerTests: XCTestCase { torIsForeground: { torForeground.value }, awaitTorReady: torWaiter.await(completion:), makeSession: { sessionFactory }, + verifyEventSignature: verifyEventSignature, scheduleAfter: { delay, action in scheduler.schedule(delay: delay, action: action) }, @@ -2074,6 +2237,45 @@ final class NostrRelayManagerTests: XCTestCase { } } +private final class ControllableEventSignatureVerifier: @unchecked Sendable { + private let lock = NSLock() + private let verificationRelease = DispatchSemaphore(value: 0) + private var shouldBlockNext = false + private var blocked = false + + var isVerificationBlocked: Bool { + lock.withLock { blocked } + } + + func blockNextVerification() { + lock.withLock { + shouldBlockNext = true + } + } + + func releaseBlockedVerification() { + verificationRelease.signal() + } + + func verify(_ event: NostrEvent) -> Bool { + let shouldBlock = lock.withLock { + guard shouldBlockNext else { return false } + shouldBlockNext = false + return true + } + if shouldBlock { + lock.withLock { + blocked = true + } + verificationRelease.wait() + lock.withLock { + blocked = false + } + } + return event.isValidSignature() + } +} + @MainActor private struct RelayManagerTestContext { let manager: NostrRelayManager