diff --git a/bitchat/Nostr/NostrRelayManager.swift b/bitchat/Nostr/NostrRelayManager.swift index 53d794fa..765e6b53 100644 --- a/bitchat/Nostr/NostrRelayManager.swift +++ b/bitchat/Nostr/NostrRelayManager.swift @@ -212,9 +212,24 @@ final class NostrRelayManager: ObservableObject { // Bump generation to invalidate scheduled reconnects when we reset/disconnect private var connectionGeneration: Int = 0 - + + // Serial off-main inbound pipeline: raw socket frames are parsed and + // Schnorr-verified in arrival order OFF the main actor (this is the single + // signature verification for the whole inbound path — downstream handlers + // receive only verified events), then hop back to the main actor for dedup + // recording and handler dispatch. A single consumer task preserves + // per-subscription arrival order. + private struct InboundFrame { + let message: URLSessionWebSocketTask.Message + let relayUrl: String + } + private let inboundContinuation: AsyncStream.Continuation + private var inboundTask: Task? + init() { self.dependencies = .live() + let (inboundStream, inboundContinuation) = AsyncStream.makeStream() + self.inboundContinuation = inboundContinuation hasMutualFavorites = dependencies.hasMutualFavorites() hasLocationPermission = dependencies.hasLocationPermission() applyDefaultRelayPolicy(force: true) @@ -238,10 +253,13 @@ final class NostrRelayManager: ObservableObject { self.applyDefaultRelayPolicy() } .store(in: &cancellables) + startInboundPipeline(consuming: inboundStream) } internal init(dependencies: NostrRelayManagerDependencies) { self.dependencies = dependencies + let (inboundStream, inboundContinuation) = AsyncStream.makeStream() + self.inboundContinuation = inboundContinuation hasMutualFavorites = dependencies.hasMutualFavorites() hasLocationPermission = dependencies.hasLocationPermission() applyDefaultRelayPolicy(force: true) @@ -265,8 +283,56 @@ final class NostrRelayManager: ObservableObject { self.applyDefaultRelayPolicy() } .store(in: &cancellables) + startInboundPipeline(consuming: inboundStream) } - + + deinit { + inboundContinuation.finish() + inboundTask?.cancel() + } + + /// Starts the single consumer task behind `inboundContinuation`. + /// + /// Ordering is deliberate and security/performance-critical: + /// 1. `precheckInboundEvent` (main hop): per-relay stats plus a cheap + /// duplicate LOOKUP — duplicate fan-in from multiple relays dominates + /// real traffic and must never pay for Schnorr verification. + /// 2. `isValidSignature()` runs here, off the main actor — the ONLY + /// signature verification on the inbound path (JSON re-serialization + + /// SHA-256 + secp256k1 Schnorr per event). + /// 3. `deliverVerifiedInboundEvent` (main hop): authoritative + /// check-and-RECORD plus handler dispatch. Recording only after + /// verification means a forged-signature copy can never poison the + /// dedup cache and suppress the genuine event. + private func startInboundPipeline(consuming stream: AsyncStream) { + inboundTask = Task.detached(priority: .userInitiated) { [weak self] in + for await frame in stream { + guard let parsed = ParsedInbound(frame.message) else { continue } + guard let self else { return } + switch parsed { + case .event(let subId, let event): + guard await self.precheckInboundEvent( + subscriptionID: subId, + eventID: event.id, + relayUrl: frame.relayUrl + ) else { + continue + } + guard event.isValidSignature() else { + SecureLogger.warning( + "⚠️ Dropped invalid Nostr event id=\(event.id.prefix(16))… sub=\(subId) relay=\(frame.relayUrl)", + category: .session + ) + continue + } + await self.deliverVerifiedInboundEvent(subscriptionID: subId, event: event, from: frame.relayUrl) + case .eose, .ok, .notice: + await self.handleParsedMessage(parsed, from: frame.relayUrl) + } + } + } + } + /// Connect to all configured relays func connect() { // Global network policy gate @@ -958,14 +1024,10 @@ final class NostrRelayManager: ObservableObject { switch result { case .success(let message): - // Parse off-main to reduce UI jank, then hop back for state updates - Task.detached(priority: .utility) { - guard let parsed = ParsedInbound(message) else { return } - await MainActor.run { - self.handleParsedMessage(parsed, from: relayUrl) - } - } - + // Hand the raw frame to the serial inbound pipeline: parsing + // and signature verification run off-main, in arrival order. + self.inboundContinuation.yield(InboundFrame(message: message, relayUrl: relayUrl)) + // Continue receiving Task { @MainActor in self.receiveMessage(from: task, relayUrl: relayUrl) @@ -983,35 +1045,55 @@ final class NostrRelayManager: ObservableObject { // Note: declared at file scope below to avoid MainActor isolation inside this class // and keep parsing off the main actor. - // Handle parsed message on MainActor (state updates and handlers) + /// First main-actor hop for an inbound EVENT: per-relay stats plus a cheap + /// duplicate LOOKUP (no recording) so duplicate fan-in from multiple + /// relays never pays for Schnorr verification. Recording happens only + /// 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 { + if let index = relays.firstIndex(where: { $0.url == relayUrl }) { + relays[index].messagesReceived += 1 + } + guard !eventID.isEmpty else { return true } + let key = InboundEventKey(subscriptionID: subscriptionID, eventID: eventID) + if recentInboundEventKeys.contains(key) { + recordDuplicateInboundEventDrop(subscriptionID: subscriptionID) + return false + } + return true + } + + /// 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) { + guard shouldDeliverInboundEvent(subscriptionID: subId, eventID: event.id) else { + return + } + if event.kind != 1059 { + // Per-event logging floods dev builds in busy geohashes; sample it. + inboundEventLogCount += 1 + if inboundEventLogCount == 1 || inboundEventLogCount.isMultiple(of: TransportConfig.nostrInboundEventLogInterval) { + 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) + } + } + + // Handle parsed non-EVENT messages on MainActor (state updates and handlers) private func handleParsedMessage(_ parsed: ParsedInbound, from relayUrl: String) { switch parsed { - case .event(let subId, let event): - if let index = self.relays.firstIndex(where: { $0.url == relayUrl }) { - self.relays[index].messagesReceived += 1 - } - guard event.isValidSignature() else { - SecureLogger.warning( - "⚠️ Dropped invalid Nostr event id=\(event.id.prefix(16))… sub=\(subId) relay=\(relayUrl)", - category: .session - ) - return - } - guard shouldDeliverInboundEvent(subscriptionID: subId, eventID: event.id) else { - return - } - if event.kind != 1059 { - // Per-event logging floods dev builds in busy geohashes; sample it. - inboundEventLogCount += 1 - if inboundEventLogCount == 1 || inboundEventLogCount.isMultiple(of: TransportConfig.nostrInboundEventLogInterval) { - 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) - } + case .event: + // Events flow through the serial inbound pipeline (precheck → + // off-main signature verification → deliverVerifiedInboundEvent) + // and never reach this fallback. + assertionFailure("inbound EVENT bypassed the verified pipeline") case .eose(let subId): if var tracker = eoseTrackers[subId] { tracker.pendingRelays.remove(relayUrl) diff --git a/bitchat/ViewModels/GeoPresenceTracker.swift b/bitchat/ViewModels/GeoPresenceTracker.swift index ffe423b5..a31feb57 100644 --- a/bitchat/ViewModels/GeoPresenceTracker.swift +++ b/bitchat/ViewModels/GeoPresenceTracker.swift @@ -103,7 +103,8 @@ final class GeoPresenceTracker { else { return } - guard event.isValidSignature() else { return } + // The signature was already verified (exactly once, off the main + // actor) by NostrRelayManager before delivery. guard shouldProcessGeoSamplingEvent(event.id) else { return } let existingCount = context.geoParticipantCount(for: gh) diff --git a/bitchat/ViewModels/NostrInboundPipeline.swift b/bitchat/ViewModels/NostrInboundPipeline.swift index 95d9b234..e62e65f9 100644 --- a/bitchat/ViewModels/NostrInboundPipeline.swift +++ b/bitchat/ViewModels/NostrInboundPipeline.swift @@ -75,15 +75,17 @@ extension ChatViewModel: NostrInboundPipelineContext { } } -/// The inbound Nostr hot path: raw relay events in, chat messages / Noise -/// payloads out. Pure transformation plus dedup — no relay lifecycle. +/// The inbound Nostr hot path: verified relay events in, chat messages / +/// Noise payloads out. Pure transformation plus dedup — no relay lifecycle. /// -/// Ordering is deliberate and performance-critical: cheap rejects (kind, -/// dedup lookup) run BEFORE Schnorr signature verification because duplicates -/// dominate real relay traffic; events are recorded only AFTER verification so -/// a forged-signature copy can never poison the dedup set; gift-wrap -/// verification for the account mailbox runs off-main with an atomic -/// main-actor check-and-record. +/// Every event arriving here already had its Schnorr signature verified +/// exactly once, off the main actor, by `NostrRelayManager`'s serial inbound +/// pipeline (which records events into its own dedup cache only AFTER +/// verification, so forged copies can't suppress genuine events). This +/// pipeline therefore never re-verifies; it keeps its own event-ID dedup +/// (cheap main-actor lookups) and moves NIP-17 gift-wrap decryption — two +/// ECDH+ChaCha layers — off the main actor with an atomic main-actor +/// check-and-record. final class NostrInboundPipeline { private weak var context: (any NostrInboundPipelineContext)? private let presence: GeoPresenceTracker @@ -97,17 +99,15 @@ final class NostrInboundPipeline { @MainActor func subscribeNostrEvent(_ event: NostrEvent) { guard let context else { return } - // Cheap rejects (kind, dedup lookup) before Schnorr verification — - // duplicates dominate real traffic and must not pay for crypto. - // Only verified events are recorded, so a forged-signature copy can - // never poison the dedup set and suppress the genuine event. + // Cheap rejects (kind, dedup lookup) — duplicates dominate real + // traffic. The signature was already verified (exactly once, off the + // main actor) by NostrRelayManager before delivery. guard (event.kind == NostrProtocol.EventKind.ephemeralEvent.rawValue || event.kind == NostrProtocol.EventKind.geohashPresence.rawValue), !context.hasProcessedNostrEvent(event.id) else { return } - guard event.isValidSignature() else { return } context.recordProcessedNostrEvent(event.id) @@ -176,15 +176,14 @@ final class NostrInboundPipeline { @MainActor func handleNostrEvent(_ event: NostrEvent) { guard let context else { return } - // Cheap rejects (kind, dedup lookup) before Schnorr verification — - // duplicates dominate real traffic and must not pay for crypto. + // Cheap rejects (kind, dedup lookup) — the signature was already + // verified (exactly once, off the main actor) by NostrRelayManager. guard (event.kind == NostrProtocol.EventKind.ephemeralEvent.rawValue || event.kind == NostrProtocol.EventKind.geohashPresence.rawValue) else { return } if context.hasProcessedNostrEvent(event.id) { return } - guard event.isValidSignature() else { return } context.recordProcessedNostrEvent(event.id) // Sampled: fires for every geo event and floods dev logs in busy geohashes. @@ -264,104 +263,102 @@ final class NostrInboundPipeline { @MainActor func subscribeGiftWrap(_ giftWrap: NostrEvent, id: NostrIdentity) { guard let context else { return } - // Dedup lookup before Schnorr verification; record only after it passes. + // Cheap dedup pre-check only; processGeohashGiftWrap does the + // authoritative main-actor check-and-record before the off-main + // NIP-17 unwrap. The outer signature was already verified (exactly + // once, off the main actor) by NostrRelayManager. guard !context.hasProcessedNostrEvent(giftWrap.id) else { return } - guard giftWrap.isValidSignature() else { return } - context.recordProcessedNostrEvent(giftWrap.id) - guard let (content, senderPubkey, rumorTs) = try? NostrProtocol.decryptPrivateMessage( - giftWrap: giftWrap, - recipientIdentity: id - ), - let packet = Self.decodeEmbeddedBitChatPacket(from: content), - packet.type == MessageType.noiseEncrypted.rawValue, - let noisePayload = NoisePayload.decode(packet.payload) - else { - return - } - - let messageTimestamp = Date(timeIntervalSince1970: TimeInterval(rumorTs)) - let convKey = PeerID(nostr_: senderPubkey) - context.registerNostrKeyMapping(senderPubkey, for: convKey) - - switch noisePayload.type { - case .privateMessage: - context.handlePrivateMessage( - noisePayload, - senderPubkey: senderPubkey, - convKey: convKey, - id: id, - messageTimestamp: messageTimestamp - ) - case .delivered: - context.handleDelivered(noisePayload, senderPubkey: senderPubkey, convKey: convKey) - case .readReceipt: - context.handleReadReceipt(noisePayload, senderPubkey: senderPubkey, convKey: convKey) - case .verifyChallenge, .verifyResponse: - break + Task.detached(priority: .userInitiated) { [weak self] in + await self?.processGeohashGiftWrap(giftWrap, id: id, verbose: false) } } @MainActor func handleGiftWrap(_ giftWrap: NostrEvent, id: NostrIdentity) { guard let context else { return } - // Dedup lookup before Schnorr verification; record only after it passes. + // Cheap dedup pre-check only; see subscribeGiftWrap. if context.hasProcessedNostrEvent(giftWrap.id) { return } - guard giftWrap.isValidSignature() else { return } - context.recordProcessedNostrEvent(giftWrap.id) + + Task.detached(priority: .userInitiated) { [weak self] in + await self?.processGeohashGiftWrap(giftWrap, id: id, verbose: true) + } + } + + /// Geohash-DM gift wrap ingest. The NIP-17 unwrap (two ECDH+ChaCha + /// layers) runs off the main actor; results hop back for state updates. + /// `verbose` keeps `handleGiftWrap`'s decrypt logging without adding it + /// to the sampling path. + private func processGeohashGiftWrap(_ giftWrap: NostrEvent, id: NostrIdentity, verbose: Bool) async { + guard let context else { return } + // Authoritative check-and-record, atomic on the main actor so two + // concurrent detached tasks can't both process the same event. + let alreadyProcessed: Bool = await MainActor.run { + if context.hasProcessedNostrEvent(giftWrap.id) { return true } + context.recordProcessedNostrEvent(giftWrap.id) + return false + } + if alreadyProcessed { return } guard let (content, senderPubkey, rumorTs) = try? NostrProtocol.decryptPrivateMessage( giftWrap: giftWrap, recipientIdentity: id ) else { - SecureLogger.warning("GeoDM: failed decrypt giftWrap id=\(giftWrap.id.prefix(8))…", category: .session) + if verbose { + SecureLogger.warning("GeoDM: failed decrypt giftWrap id=\(giftWrap.id.prefix(8))…", category: .session) + } return } - SecureLogger.debug( - "GeoDM: decrypted gift-wrap id=\(giftWrap.id.prefix(16))... from=\(senderPubkey.prefix(8))...", - category: .session - ) - - guard let packet = Self.decodeEmbeddedBitChatPacket(from: content), - packet.type == MessageType.noiseEncrypted.rawValue, - let payload = NoisePayload.decode(packet.payload) - else { - return - } - - let convKey = PeerID(nostr_: senderPubkey) - context.registerNostrKeyMapping(senderPubkey, for: convKey) - - switch payload.type { - case .privateMessage: - let messageTimestamp = Date(timeIntervalSince1970: TimeInterval(rumorTs)) - context.handlePrivateMessage( - payload, - senderPubkey: senderPubkey, - convKey: convKey, - id: id, - messageTimestamp: messageTimestamp + if verbose { + SecureLogger.debug( + "GeoDM: decrypted gift-wrap id=\(giftWrap.id.prefix(16))... from=\(senderPubkey.prefix(8))...", + category: .session ) - case .delivered: - context.handleDelivered(payload, senderPubkey: senderPubkey, convKey: convKey) - case .readReceipt: - context.handleReadReceipt(payload, senderPubkey: senderPubkey, convKey: convKey) - case .verifyChallenge, .verifyResponse: - break + } + + await MainActor.run { + guard let packet = Self.decodeEmbeddedBitChatPacket(from: content), + packet.type == MessageType.noiseEncrypted.rawValue, + let payload = NoisePayload.decode(packet.payload) + else { + return + } + + let convKey = PeerID(nostr_: senderPubkey) + context.registerNostrKeyMapping(senderPubkey, for: convKey) + + switch payload.type { + case .privateMessage: + let messageTimestamp = Date(timeIntervalSince1970: TimeInterval(rumorTs)) + context.handlePrivateMessage( + payload, + senderPubkey: senderPubkey, + convKey: convKey, + id: id, + messageTimestamp: messageTimestamp + ) + case .delivered: + context.handleDelivered(payload, senderPubkey: senderPubkey, convKey: convKey) + case .readReceipt: + context.handleReadReceipt(payload, senderPubkey: senderPubkey, convKey: convKey) + case .verifyChallenge, .verifyResponse: + break + } } } @MainActor func handleNostrMessage(_ giftWrap: NostrEvent) { guard let context else { return } - // Cheap dedup pre-check only; Schnorr verification runs off-main in - // processNostrMessage, which then does the authoritative - // check-and-record. Recording stays after verification so a - // forged-signature copy can never poison the dedup set and suppress - // the genuine event. + // Cheap dedup pre-check only; processNostrMessage does the + // authoritative check-and-record before the off-main NIP-17 unwrap. + // The outer signature was already verified (exactly once, off the + // main actor) by NostrRelayManager, and only verified events are + // recorded, so a forged-signature copy can never poison the dedup + // set and suppress the genuine event. if context.hasProcessedNostrEvent(giftWrap.id) { return } Task.detached(priority: .userInitiated) { [weak self] in @@ -370,7 +367,6 @@ final class NostrInboundPipeline { } func processNostrMessage(_ giftWrap: NostrEvent) async { - guard giftWrap.isValidSignature() else { return } guard let context else { return } // Authoritative check-and-record, atomic on the main actor so two // concurrent detached tasks can't both process the same event. diff --git a/bitchatTests/ChatNostrCoordinatorContextTests.swift b/bitchatTests/ChatNostrCoordinatorContextTests.swift index 7244f366..deb3ccb9 100644 --- a/bitchatTests/ChatNostrCoordinatorContextTests.swift +++ b/bitchatTests/ChatNostrCoordinatorContextTests.swift @@ -382,10 +382,12 @@ struct ChatNostrCoordinatorContextTests { coordinator.inbound.handleGiftWrap(giftWrap, id: recipient) + // The NIP-17 unwrap runs off the main actor; wait for the hop back. let convKey = PeerID(nostr_: sender.publicKeyHex) + let routed = await TestHelpers.waitUntil({ context.handledPrivateMessages.count == 1 }) + #expect(routed) #expect(context.recordedNostrEventIDs == [giftWrap.id]) #expect(context.nostrKeyMapping[convKey] == sender.publicKeyHex) - #expect(context.handledPrivateMessages.count == 1) #expect(context.handledPrivateMessages.first?.senderPubkey == sender.publicKeyHex) #expect(context.handledPrivateMessages.first?.convKey == convKey) @@ -398,30 +400,37 @@ struct ChatNostrCoordinatorContextTests { // The same gift wrap is dropped on replay. coordinator.inbound.handleGiftWrap(giftWrap, id: recipient) + await drainMainQueue() #expect(context.recordedNostrEventIDs == [giftWrap.id]) #expect(context.handledPrivateMessages.count == 1) } + // NOTE: Inbound Schnorr signature verification (and the forged-copy + // dedup-poisoning invariant) is enforced once, off the main actor, at the + // relay boundary — see NostrRelayManagerTests + // `test_receiveEvent_invalidSignatureDoesNotPoisonDuplicateCache` and + // `test_receiveGiftWrap_tamperedSignatureIsDroppedAndDoesNotPoisonDedup`. + // The inbound pipeline only ever sees verified events. + @Test @MainActor - func processNostrMessage_invalidSignatureDoesNotPoisonDedup() async throws { + func processNostrMessage_duplicateDeliveryProcessesOnce() async throws { let context = MockChatNostrContext() let coordinator = ChatNostrCoordinator(context: context) let recipient = try NostrIdentity.generate() let sender = try NostrIdentity.generate() + context.nostrIdentity = recipient let giftWrap = try NostrProtocol.createPrivateMessage( content: "verify:noop", recipientPubkey: recipient.publicKeyHex, senderIdentity: sender ) - var invalidGiftWrap = giftWrap - invalidGiftWrap.sig = String(repeating: "0", count: 128) - // A forged-signature copy is rejected WITHOUT entering the dedup set... - await coordinator.inbound.processNostrMessage(invalidGiftWrap) - #expect(context.recordedNostrEventIDs.isEmpty) + // Fan-in of the same (already verified) gift wrap from several relays + // records and processes exactly once. + await coordinator.inbound.processNostrMessage(giftWrap) + #expect(context.recordedNostrEventIDs == [giftWrap.id]) - // ...so the genuine event with the same ID still processes and records. await coordinator.inbound.processNostrMessage(giftWrap) #expect(context.recordedNostrEventIDs == [giftWrap.id]) } diff --git a/bitchatTests/ChatViewModelExtensionsTests.swift b/bitchatTests/ChatViewModelExtensionsTests.swift index 3662f55a..9d6e0115 100644 --- a/bitchatTests/ChatViewModelExtensionsTests.swift +++ b/bitchatTests/ChatViewModelExtensionsTests.swift @@ -352,31 +352,11 @@ struct ChatViewModelNostrExtensionTests { #expect(!viewModel.messages.contains { $0.content == "Blocked" }) } - @Test @MainActor - func handleNostrEvent_rejectsInvalidSignature() async throws { - let (viewModel, _) = makeTestableViewModel() - let geohash = "u4pruydq" - let identity = try NostrIdentity.generate() - - viewModel.switchLocationChannel(to: .location(GeohashChannel(level: .city, geohash: geohash))) - - let event = NostrEvent( - pubkey: identity.publicKeyHex, - createdAt: Date(), - kind: .ephemeralEvent, - tags: [["g", geohash]], - content: "Valid" - ) - var signed = try event.sign(with: identity.schnorrSigningKey()) - signed.id = "deadbeef" - - viewModel.handleNostrEvent(signed) - - try? await Task.sleep(nanoseconds: 100_000_000) - viewModel.publicMessagePipeline.flushIfNeeded() - - #expect(!viewModel.messages.contains { $0.content == "Tampered" }) - } + // NOTE: Tampered-signature rejection is enforced once, off the main + // actor, at the relay boundary (events only reach the inbound pipeline + // after verification) — see NostrRelayManagerTests + // `test_receiveEvent_invalidSignatureDoesNotPoisonDuplicateCache` and + // `test_receiveGiftWrap_tamperedSignatureIsDroppedAndDoesNotPoisonDedup`. @Test @MainActor func subscribeGiftWrap_rejectsOversizedEmbeddedPacket() async throws { @@ -565,9 +545,14 @@ struct ChatViewModelNostrExtensionTests { viewModel.handleGiftWrap(giftWrap, id: recipient) - try? await Task.sleep(nanoseconds: 50_000_000) + // Gift-wrap decryption runs off the main actor; wait for the ack + // (sent even for blocked senders) to know processing finished. + let didAck = await TestHelpers.waitUntil( + { viewModel.sentGeoDeliveryAcks.contains(messageID) }, + timeout: 5.0 + ) + #expect(didAck) #expect(viewModel.privateChats[convKey] == nil) - #expect(viewModel.sentGeoDeliveryAcks.contains(messageID)) } @Test @MainActor diff --git a/bitchatTests/GeohashPresenceTests.swift b/bitchatTests/GeohashPresenceTests.swift index 96f19080..e0d43169 100644 --- a/bitchatTests/GeohashPresenceTests.swift +++ b/bitchatTests/GeohashPresenceTests.swift @@ -326,26 +326,11 @@ struct ChatViewModelPresenceHandlingTests { #expect(viewModel.geohashParticipantCount(for: activeGeohash) >= 1) } - @Test func subscribeNostrEvent_samplingInvalidSignatureDoesNotPoisonDedup() async throws { - let (viewModel, _) = makeTestableViewModel() - let sampleGeohash = "u4pru" - let identity = try NostrIdentity.generate() - let event = NostrEvent( - pubkey: identity.publicKeyHex, - createdAt: Date(), - kind: .geohashPresence, - tags: [["g", sampleGeohash]], - content: "" - ) - let signed = try event.sign(with: identity.schnorrSigningKey()) - var invalid = signed - invalid.sig = String(repeating: "0", count: 128) - - viewModel.subscribeNostrEvent(invalid, gh: sampleGeohash) - viewModel.subscribeNostrEvent(signed, gh: sampleGeohash) - - #expect(viewModel.geohashParticipantCount(for: sampleGeohash) == 1) - } + // NOTE: Tampered-signature rejection (and the forged-copy dedup-poisoning + // invariant) is enforced once, off the main actor, at the relay boundary — + // the sampling path only ever sees verified events. See + // NostrRelayManagerTests + // `test_receiveEvent_invalidSignatureDoesNotPoisonDuplicateCache`. // MARK: - Test Helper diff --git a/bitchatTests/Performance/PerformanceBaselineTests.swift b/bitchatTests/Performance/PerformanceBaselineTests.swift index 57714e45..9b82fc49 100644 --- a/bitchatTests/Performance/PerformanceBaselineTests.swift +++ b/bitchatTests/Performance/PerformanceBaselineTests.swift @@ -77,8 +77,10 @@ final class PerformanceBaselineTests: XCTestCase { // MARK: - 1a. Nostr inbound event handling (fresh events) /// `NostrInboundPipeline.handleNostrEvent` for never-seen geo events - /// (kind 20000): signature verification, dedup record, presence/nickname - /// bookkeeping, and public-message ingest scheduling. + /// (kind 20000): dedup record, presence/nickname bookkeeping, and + /// public-message ingest scheduling. Schnorr signature verification is + /// NOT part of this path anymore — it runs exactly once, off the main + /// actor, in `NostrRelayManager` before delivery. func testNostrInboundEventHandling_freshEvents() throws { let events = try Self.makeSignedGeohashEvents(count: 500) // A fresh context per measure pass so every event takes the @@ -106,8 +108,9 @@ final class PerformanceBaselineTests: XCTestCase { /// The dedup-hit path: identical events replayed. Duplicates dominate /// real relay traffic (the same event arrives from several relays), so - /// this path runs hundreds of times a minute in busy geohashes. Note it - /// still pays full Schnorr signature verification before the dedup check. + /// this path runs hundreds of times a minute in busy geohashes. It is a + /// pure dedup lookup: no crypto (verification happens upstream in + /// `NostrRelayManager`, and only for the first-seen copy). func testNostrInboundEventHandling_duplicateEvents() throws { let events = try Self.makeSignedGeohashEvents(count: 500) let context = PerfNostrContext() diff --git a/bitchatTests/Services/NostrRelayManagerTests.swift b/bitchatTests/Services/NostrRelayManagerTests.swift index fbb46060..2490e480 100644 --- a/bitchatTests/Services/NostrRelayManagerTests.swift +++ b/bitchatTests/Services/NostrRelayManagerTests.swift @@ -724,6 +724,77 @@ final class NostrRelayManagerTests: XCTestCase { XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount(forSubscriptionID: "events"), 0) } + /// The relay boundary is the single signature-verification point for the + /// whole inbound path (downstream pipelines no longer re-verify), so a + /// tampered gift wrap (kind 1059, the DM/mailbox path) must be dropped + /// here — and must not poison the dedup cache against the genuine copy. + func test_receiveGiftWrap_tamperedSignatureIsDroppedAndDoesNotPoisonDedup() async throws { + let firstRelayURL = "wss://giftwrap-one.example" + let secondRelayURL = "wss://giftwrap-two.example" + let context = makeContext(permission: .denied) + let sender = try NostrIdentity.generate() + let recipient = try NostrIdentity.generate() + let giftWrap = try NostrProtocol.createPrivateMessage( + content: "psst", + recipientPubkey: recipient.publicKeyHex, + senderIdentity: sender + ) + let tampered = invalidSignatureCopy(of: giftWrap) + var receivedIDs: [String] = [] + + context.manager.subscribe( + filter: makeFilter(), + id: "gift-wraps", + relayUrls: [firstRelayURL, secondRelayURL] + ) { event in + receivedIDs.append(event.id) + } + let subscriptionsSent = await waitUntil { + context.sessionFactory.latestConnection(for: firstRelayURL)?.sentStrings.count == 1 && + context.sessionFactory.latestConnection(for: secondRelayURL)?.sentStrings.count == 1 + } + XCTAssertTrue(subscriptionsSent) + + try context.sessionFactory.latestConnection(for: firstRelayURL)?.emitEventMessage(subscriptionID: "gift-wraps", event: tampered) + try context.sessionFactory.latestConnection(for: secondRelayURL)?.emitEventMessage(subscriptionID: "gift-wraps", event: giftWrap) + + let countedOnBothRelays = await waitUntil { + context.manager.relays.first(where: { $0.url == firstRelayURL })?.messagesReceived == 1 && + context.manager.relays.first(where: { $0.url == secondRelayURL })?.messagesReceived == 1 + } + XCTAssertTrue(countedOnBothRelays) + XCTAssertEqual(receivedIDs, [giftWrap.id]) + XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount, 0) + } + + /// Signature verification moved off-main into a single serial consumer; + /// several frames buffered on one socket must still be delivered to the + /// handler in arrival order. + func test_receiveEvent_deliversBackToBackEventsInArrivalOrder() async throws { + let relayURL = "wss://ordered.example" + let context = makeContext(permission: .denied) + let events = try (0..<12).map { try makeSignedEvent(content: "ordered-\($0)") } + var receivedIDs: [String] = [] + + context.manager.subscribe(filter: makeFilter(), id: "ordered", relayUrls: [relayURL]) { event in + receivedIDs.append(event.id) + } + let subscriptionSent = await waitUntil { + context.sessionFactory.latestConnection(for: relayURL)?.sentStrings.count == 1 + } + XCTAssertTrue(subscriptionSent) + + for event in events { + try context.sessionFactory.latestConnection(for: relayURL)?.emitEventMessage(subscriptionID: "ordered", event: event) + } + + let allDelivered = await waitUntil(timeout: 5.0) { + receivedIDs.count == events.count + } + XCTAssertTrue(allDelivered) + XCTAssertEqual(receivedIDs, events.map(\.id)) + } + func test_receiveEvent_withoutHandlerStillTracksReceivedCount() async throws { let relayURL = "wss://missing-handler.example" let context = makeContext(permission: .denied) @@ -1695,7 +1766,11 @@ private final class MockRelayConnection: NostrRelayConnectionProtocol { } func receive(completionHandler: @escaping (Result) -> Void) { - receiveHandler = completionHandler + if !pendingResults.isEmpty { + completionHandler(pendingResults.removeFirst()) + } else { + receiveHandler = completionHandler + } } func sendPing(pongReceiveHandler: @escaping (Error?) -> Void) { @@ -1728,15 +1803,24 @@ private final class MockRelayConnection: NostrRelayConnectionProtocol { } func emitRawString(_ string: String) throws { - let handler = receiveHandler - receiveHandler = nil - handler?(.success(.string(string))) + deliver(.success(.string(string))) } private func emit(jsonObject: Any) throws { let data = try JSONSerialization.data(withJSONObject: jsonObject) - let handler = receiveHandler - receiveHandler = nil - handler?(.success(.data(data))) + deliver(.success(.data(data))) + } + + // Frames emitted before the manager re-arms `receive` are queued so + // back-to-back emissions model a socket with several buffered frames. + private var pendingResults: [Result] = [] + + private func deliver(_ result: Result) { + if let handler = receiveHandler { + receiveHandler = nil + handler(result) + } else { + pendingResults.append(result) + } } }