Merge branch 'main' into fix/noise-dh-secret-clearing

bitchat/Services/MessageDeduplicationService.swift

@@ -12,6 +12,8 @@ import Foundation
 
 /// Generic LRU (Least Recently Used) cache for deduplication.
 /// Uses an efficient O(1) lookup with periodic compaction.
+/// Thread-safe via @MainActor - all callers are already on main actor.
+@MainActor
 final class LRUDeduplicationCache<Value> {
     private var map: [String: Value] = [:]
     private var order: [String] = []
@@ -157,6 +159,8 @@ enum ContentNormalizer {
 
 /// Service that manages message deduplication using LRU caches.
 /// Provides separate caches for content-based dedup and Nostr event ID dedup.
+/// Thread-safe via @MainActor - all callers are already on main actor.
+@MainActor
 final class MessageDeduplicationService {
 
     /// Cache for content-based near-duplicate detection

bitchat/Services/NostrTransport.swift

@@ -149,8 +149,11 @@ final class NostrTransport: Transport, @unchecked Sendable {
 
     func sendReadReceipt(_ receipt: ReadReceipt, to peerID: PeerID) {
         // Enqueue and process with throttling to avoid relay rate limits
-        readQueue.append(QueuedRead(receipt: receipt, peerID: peerID))
-        processReadQueueIfNeeded()
+        // Use barrier to synchronize access to readQueue
+        queue.async(flags: .barrier) { [weak self] in
+            self?.readQueue.append(QueuedRead(receipt: receipt, peerID: peerID))
+            self?.processReadQueueIfNeeded()
+        }
     }
 
     func sendFavoriteNotification(to peerID: PeerID, isFavorite: Bool) {
@@ -260,16 +263,17 @@ extension NostrTransport {
 // MARK: - Private Helpers
 
 extension NostrTransport {
+    /// Must be called within a barrier on `queue`
     private func processReadQueueIfNeeded() {
         guard !isSendingReadAcks else { return }
         guard !readQueue.isEmpty else { return }
         isSendingReadAcks = true
-        sendNextReadAck()
+        let item = readQueue.removeFirst()
+        sendReadAckItem(item)
     }
 
-    private func sendNextReadAck() {
-        guard !readQueue.isEmpty else { isSendingReadAcks = false; return }
-        let item = readQueue.removeFirst()
+    /// Sends a single read ack item (called after extraction from queue within barrier)
+    private func sendReadAckItem(_ item: QueuedRead) {
         Task { @MainActor in
             guard let recipientNpub = resolveRecipientNpub(for: item.peerID) else { scheduleNextReadAck(); return }
             guard let senderIdentity = try? idBridge.getCurrentNostrIdentity() else { scheduleNextReadAck(); return }
@@ -301,9 +305,10 @@ extension NostrTransport {
 
     private func scheduleNextReadAck() {
         DispatchQueue.main.asyncAfter(deadline: .now() + readAckInterval) { [weak self] in
-            guard let self = self else { return }
-            self.isSendingReadAcks = false
-            self.processReadQueueIfNeeded()
+            self?.queue.async(flags: .barrier) { [weak self] in
+                self?.isSendingReadAcks = false
+                self?.processReadQueueIfNeeded()
+            }
         }
     }
 

bitchatTests/MessageDeduplicationServiceTests.swift

@@ -12,6 +12,8 @@ import Foundation
 
 // MARK: - LRU Deduplication Cache Tests
 
+@Suite("LRU Deduplication Cache")
+@MainActor
 struct LRUDeduplicationCacheTests {
 
     // MARK: - Basic Operations
@@ -265,6 +267,8 @@ struct ContentNormalizerTests {
 
 // MARK: - Message Deduplication Service Tests
 
+@Suite("Message Deduplication Service")
+@MainActor
 struct MessageDeduplicationServiceTests {
 
     // MARK: - Content Deduplication
@@ -467,4 +471,134 @@ struct MessageDeduplicationServiceTests {
         #expect(service.contentTimestamp(for: "hello world") == now)
         #expect(service.contentTimestamp(for: "Hello World") == now)
     }
+
+    // MARK: - Thread Safety Tests (via @MainActor enforcement)
+
+    @Test("Concurrent content recording is safe via MainActor")
+    func concurrentContentRecording() async {
+        let service = MessageDeduplicationService(contentCapacity: 1000, nostrEventCapacity: 1000)
+        let iterations = 100
+
+        // All operations run on MainActor due to @MainActor annotation
+        // This test verifies the pattern works correctly
+        await withTaskGroup(of: Void.self) { group in
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordContent("Message \(i)", timestamp: Date())
+                }
+            }
+        }
+
+        // Verify some entries were recorded
+        #expect(service.contentTimestamp(for: "Message 0") != nil)
+        #expect(service.contentTimestamp(for: "Message 99") != nil)
+    }
+
+    @Test("Concurrent Nostr event recording is safe via MainActor")
+    func concurrentNostrEventRecording() async {
+        let service = MessageDeduplicationService(contentCapacity: 1000, nostrEventCapacity: 1000)
+        let iterations = 100
+
+        await withTaskGroup(of: Void.self) { group in
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordNostrEvent("event_\(i)")
+                }
+            }
+        }
+
+        // Verify events were recorded
+        #expect(service.hasProcessedNostrEvent("event_0"))
+        #expect(service.hasProcessedNostrEvent("event_99"))
+    }
+
+    @Test("Mixed concurrent operations are safe via MainActor")
+    func concurrentMixedOperations() async {
+        let service = MessageDeduplicationService(contentCapacity: 1000, nostrEventCapacity: 1000)
+        let iterations = 50
+
+        await withTaskGroup(of: Void.self) { group in
+            // Content recording tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordContent("Content \(i)", timestamp: Date())
+                }
+            }
+
+            // Event recording tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordNostrEvent("event_\(i)")
+                }
+            }
+
+            // ACK recording tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordNostrAck("ack_\(i)")
+                }
+            }
+
+            // Read tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    _ = service.contentTimestamp(for: "Content \(i)")
+                    _ = service.hasProcessedNostrEvent("event_\(i)")
+                    _ = service.hasProcessedNostrAck("ack_\(i)")
+                }
+            }
+        }
+
+        // If we reach here without crashes, the test passes
+    }
+}
+
+// MARK: - LRU Cache Thread Safety Tests
+
+@Suite("LRU Cache Thread Safety")
+@MainActor
+struct LRUCacheThreadSafetyTests {
+
+    @Test("Concurrent cache access is safe via MainActor")
+    func concurrentCacheAccess() async {
+        let cache = LRUDeduplicationCache<Int>(capacity: 500)
+        let iterations = 100
+
+        await withTaskGroup(of: Void.self) { group in
+            // Write tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    cache.record("key_\(i)", value: i)
+                }
+            }
+
+            // Read tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    _ = cache.contains("key_\(i)")
+                    _ = cache.value(for: "key_\(i)")
+                }
+            }
+        }
+
+        // Verify cache is in consistent state
+        #expect(cache.count <= 500) // Respects capacity
+    }
+
+    @Test("Cache eviction under concurrent load is safe")
+    func cacheEvictionUnderLoad() async {
+        let cache = LRUDeduplicationCache<Int>(capacity: 10)
+        let iterations = 100
+
+        await withTaskGroup(of: Void.self) { group in
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    cache.record("key_\(i)", value: i)
+                }
+            }
+        }
+
+        // Cache should maintain its capacity constraint
+        #expect(cache.count == 10)
+    }
 }

bitchatTests/Services/NostrTransportTests.swift

@@ -0,0 +1,108 @@
+//
+// NostrTransportTests.swift
+// bitchat
+//
+// This is free and unencumbered software released into the public domain.
+// For more information, see <https://unlicense.org>
+//
+
+import Foundation
+import Testing
+@testable import bitchat
+
+@Suite("NostrTransport Thread Safety Tests")
+struct NostrTransportTests {
+
+    @Test("Concurrent read receipt enqueue does not crash")
+    @MainActor
+    func concurrentReadReceiptEnqueue() async throws {
+        let keychain = MockKeychain()
+        let idBridge = NostrIdentityBridge(keychain: keychain)
+        let transport = NostrTransport(keychain: keychain, idBridge: idBridge)
+
+        // Create 100 concurrent read receipt submissions
+        let iterations = 100
+
+        await withTaskGroup(of: Void.self) { group in
+            for i in 0..<iterations {
+                group.addTask {
+                    let receipt = ReadReceipt(
+                        originalMessageID: UUID().uuidString,
+                        readerID: PeerID(str: String(format: "%016x", i)),
+                        readerNickname: "Reader\(i)"
+                    )
+                    let peerID = PeerID(str: String(format: "%016x", i))
+                    transport.sendReadReceipt(receipt, to: peerID)
+                }
+            }
+        }
+
+        // If we reach here without crashing, the test passes
+        // The concurrent enqueue operations completed without data races
+    }
+
+    @Test("Read queue processes under concurrent load")
+    @MainActor
+    func readQueueProcessingUnderLoad() async throws {
+        let keychain = MockKeychain()
+        let idBridge = NostrIdentityBridge(keychain: keychain)
+        let transport = NostrTransport(keychain: keychain, idBridge: idBridge)
+
+        // Rapidly enqueue many receipts from multiple concurrent sources
+        let iterations = 50
+
+        // First batch - rapid fire
+        for i in 0..<iterations {
+            let receipt = ReadReceipt(
+                originalMessageID: UUID().uuidString,
+                readerID: PeerID(str: String(format: "%016x", i)),
+                readerNickname: "Reader\(i)"
+            )
+            transport.sendReadReceipt(receipt, to: PeerID(str: String(format: "%016x", i)))
+        }
+
+        // Give some time for processing to start
+        try await Task.sleep(nanoseconds: 100_000_000) // 100ms
+
+        // Second batch - while first might be processing
+        await withTaskGroup(of: Void.self) { group in
+            for i in iterations..<(iterations * 2) {
+                group.addTask {
+                    let receipt = ReadReceipt(
+                        originalMessageID: UUID().uuidString,
+                        readerID: PeerID(str: String(format: "%016x", i)),
+                        readerNickname: "Reader\(i)"
+                    )
+                    transport.sendReadReceipt(receipt, to: PeerID(str: String(format: "%016x", i)))
+                }
+            }
+        }
+
+        // If we reach here without crashing or deadlocking, test passes
+    }
+
+    @Test("isPeerReachable is thread safe")
+    @MainActor
+    func isPeerReachableThreadSafety() async throws {
+        let keychain = MockKeychain()
+        let idBridge = NostrIdentityBridge(keychain: keychain)
+        let transport = NostrTransport(keychain: keychain, idBridge: idBridge)
+
+        let iterations = 100
+
+        // Concurrent reads on isPeerReachable
+        await withTaskGroup(of: Bool.self) { group in
+            for i in 0..<iterations {
+                group.addTask {
+                    let peerID = PeerID(str: String(format: "%016x", i))
+                    return transport.isPeerReachable(peerID)
+                }
+            }
+
+            // Collect results (all should be false since no favorites configured)
+            for await result in group {
+                #expect(result == false)
+            }
+        }
+    }
+}