Fix BLE identity state races

bitchat/Services/BLE/BLEAnnounceThrottle.swift

@@ -1,6 +1,13 @@
 import Foundation
 
-struct BLEAnnounceThrottle {
+/// Thread-safe announce admission state.
+///
+/// Announce requests originate from the Bluetooth delegate queue, the
+/// concurrent message queue, and the maintenance timer. Keeping the timestamp
+/// behind a lock makes admission and maintenance snapshots atomic when those
+/// request sources race.
+final class BLEAnnounceThrottle: @unchecked Sendable {
+    private let lock = NSLock()
     private var lastSent: Date
     private let normalMinimumInterval: TimeInterval
     private let forcedMinimumInterval: TimeInterval
@@ -16,16 +23,18 @@ struct BLEAnnounceThrottle {
     }
 
     func elapsed(since now: Date) -> TimeInterval {
-        now.timeIntervalSince(lastSent)
+        lock.withLock { now.timeIntervalSince(lastSent) }
     }
 
-    mutating func shouldSend(force: Bool, now: Date) -> Bool {
-        let minimumInterval = force ? forcedMinimumInterval : normalMinimumInterval
-        guard elapsed(since: now) >= minimumInterval else {
-            return false
-        }
+    func shouldSend(force: Bool, now: Date) -> Bool {
+        lock.withLock {
+            let minimumInterval = force ? forcedMinimumInterval : normalMinimumInterval
+            guard now.timeIntervalSince(lastSent) >= minimumInterval else {
+                return false
+            }
 
-        lastSent = now
-        return true
+            lastSent = now
+            return true
+        }
     }
 }

bitchat/Services/BLE/BLELocalIdentityStateStore.swift

@@ -0,0 +1,55 @@
+import BitFoundation
+import Foundation
+
+struct BLELocalIdentitySnapshot: Equatable, Sendable {
+    let peerID: PeerID
+    let peerIDData: Data
+    let nickname: String
+}
+
+/// Lock-backed local identity state shared by the transport's message,
+/// Bluetooth, maintenance, and main-actor entry points.
+///
+/// `peerID` and its binary wire representation must change as one unit during
+/// panic rotation. A snapshot also gives announce construction one consistent
+/// view of the nickname and identity instead of reading three independently
+/// mutable properties across queues.
+final class BLELocalIdentityStateStore: @unchecked Sendable {
+    private let lock = NSLock()
+    private var state: BLELocalIdentitySnapshot
+
+    init(
+        peerID: PeerID = PeerID(str: ""),
+        nickname: String = "anon"
+    ) {
+        state = BLELocalIdentitySnapshot(
+            peerID: peerID,
+            peerIDData: Data(hexString: peerID.id) ?? Data(),
+            nickname: nickname
+        )
+    }
+
+    func snapshot() -> BLELocalIdentitySnapshot {
+        lock.withLock { state }
+    }
+
+    func setNickname(_ nickname: String) {
+        lock.withLock {
+            state = BLELocalIdentitySnapshot(
+                peerID: state.peerID,
+                peerIDData: state.peerIDData,
+                nickname: nickname
+            )
+        }
+    }
+
+    func replacePeerIdentity(with peerID: PeerID) {
+        lock.withLock {
+            state = BLELocalIdentitySnapshot(
+                peerID: peerID,
+                peerIDData: Data(hexString: peerID.id) ?? Data(),
+                nickname: state.nickname
+            )
+        }
+    }
+}

bitchat/Services/BLE/BLEService.swift

@@ -134,7 +134,7 @@ final class BLEService: NSObject {
     private let incomingFileStore = BLEIncomingFileStore()
     
     // Simple announce throttling
-    private var announceThrottle = BLEAnnounceThrottle()
+    private let announceThrottle = BLEAnnounceThrottle()
     
     // Application state tracking (thread-safe)
     #if os(iOS)
@@ -162,9 +162,7 @@ final class BLEService: NSObject {
     private let identityManager: SecureIdentityStateManagerProtocol
     private let keychain: KeychainManagerProtocol
     private let idBridge: NostrIdentityBridge
-    /// Binary form of `myPeerID`; same contract — mutated only inside a
-    /// `messageQueue` barrier via `refreshPeerIdentity()`.
-    private var myPeerIDData: Data = Data()
+    private let localIdentityState = BLELocalIdentityStateStore()
 
     // MARK: - Advertising Privacy
     // No Local Name by default for maximum privacy. No rotating alias.
@@ -417,7 +415,7 @@ final class BLEService: NSObject {
     }
 
     func resetIdentityForPanic(currentNickname: String) {
-        messageQueue.sync(flags: .barrier) {
+        collectionsQueue.sync(flags: .barrier) {
             pendingNoiseSessionQueues.removeAll()
         }
 
@@ -537,20 +535,17 @@ final class BLEService: NSObject {
     
     // MARK: Identity
 
-    /// Derived from the Noise identity fingerprint; rotated only via
-    /// `refreshPeerIdentity()` (e.g. panic reset), which performs the swap
-    /// inside a `messageQueue` barrier so concurrent queue work never sees a
-    /// half-updated identity. Externally read-only — no out-of-band mutation
-    /// may bypass that derivation.
-    private(set) var myPeerID = PeerID(str: "")
-    /// Externally read-only; mutate via `setNickname(_:)`, which also
-    /// broadcasts the change to peers.
-    private(set) var myNickname: String = "anon"
+    /// Derived from the Noise identity fingerprint. Reads can originate from
+    /// the main actor, message queue, Bluetooth queue, and maintenance timer,
+    /// so all three local identity fields live in one lock-backed snapshot.
+    var myPeerID: PeerID { localIdentityState.snapshot().peerID }
+    var myNickname: String { localIdentityState.snapshot().nickname }
+    private var myPeerIDData: Data { localIdentityState.snapshot().peerIDData }
 
     /// Sole mutator for `myNickname`: updates the stored value and force-sends
     /// an announce so peers learn the new name.
     func setNickname(_ nickname: String) {
-        self.myNickname = nickname
+        localIdentityState.setNickname(nickname)
         // Send announce to notify peers of nickname change (force send)
         sendAnnounce(forceSend: true)
     }
@@ -596,10 +591,11 @@ final class BLEService: NSObject {
     }
     
     func stopServices() {
+        let localIdentity = localIdentityState.snapshot()
         // Send leave message synchronously to ensure delivery
         var leavePacket = BitchatPacket(
             type: MessageType.leave.rawValue,
-            senderID: myPeerIDData,
+            senderID: localIdentity.peerIDData,
             recipientID: nil,
             timestamp: UInt64(Date().timeIntervalSince1970 * 1000),
             payload: Data(),
@@ -1637,6 +1633,16 @@ final class BLEService: NSObject {
         }
     }
     private func sendAnnounce(forceSend: Bool = false) {
+        // Announce construction reads the replaceable Noise service and several
+        // related state snapshots. Serialize the whole operation with identity
+        // rotation instead of letting CoreBluetooth and maintenance callbacks
+        // execute it directly on their own queues.
+        messageQueue.async(flags: .barrier) { [weak self] in
+            self?.sendAnnounceNow(forceSend: forceSend)
+        }
+    }
+
+    private func sendAnnounceNow(forceSend: Bool) {
         // Throttle announces to prevent flooding
         if !announceThrottle.shouldSend(force: forceSend, now: Date()) {
             return
@@ -1656,8 +1662,9 @@ final class BLEService: NSObject {
             )
         }
 
+        let localIdentity = localIdentityState.snapshot()
         let announcement = AnnouncementPacket(
-            nickname: myNickname,
+            nickname: localIdentity.nickname,
             noisePublicKey: noisePub,
             signingPublicKey: signingPub,
             directNeighbors: connectedPeerIDs,
@@ -1673,7 +1680,7 @@ final class BLEService: NSObject {
         // Create packet with signature using the noise private key
         let packet = BitchatPacket(
             type: MessageType.announce.rawValue,
-            senderID: myPeerIDData,
+            senderID: localIdentity.peerIDData,
             recipientID: nil,
             timestamp: UInt64(Date().timeIntervalSince1970 * 1000),
             payload: payload,
@@ -1687,14 +1694,7 @@ final class BLEService: NSObject {
             return
         }
         
-        // Call directly if on messageQueue, otherwise dispatch
-        if DispatchQueue.getSpecific(key: messageQueueKey) != nil {
-            broadcastPacket(signedPacket)
-        } else {
-            messageQueue.async { [weak self] in
-                self?.broadcastPacket(signedPacket)
-            }
-        }
+        broadcastPacket(signedPacket)
         // Ensure our own announce is included in sync state
         gossipSyncManager?.onPublicPacketSeen(signedPacket)
 
@@ -3358,8 +3358,9 @@ extension BLEService {
     private func refreshPeerIdentity() {
         let swap = {
             let fingerprint = self.noiseService.getIdentityFingerprint()
-            self.myPeerID = PeerID(str: fingerprint.prefix(16))
-            self.myPeerIDData = Data(hexString: self.myPeerID.id) ?? Data()
+            self.localIdentityState.replacePeerIdentity(
+                with: PeerID(str: fingerprint.prefix(16))
+            )
             self.meshTopology.reset()
         }
         if DispatchQueue.getSpecific(key: messageQueueKey) != nil {

bitchatTests/Services/BLEAnnounceThrottleTests.swift

@@ -5,7 +5,7 @@ import Testing
 struct BLEAnnounceThrottleTests {
     @Test
     func firstAnnounceIsAllowed() {
-        var throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
+        let throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
 
         let shouldSend = throttle.shouldSend(force: false, now: Date(timeIntervalSince1970: 100))
 
@@ -15,7 +15,7 @@ struct BLEAnnounceThrottleTests {
     @Test
     func regularAnnounceUsesNormalMinimumInterval() {
         let now = Date(timeIntervalSince1970: 100)
-        var throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
+        let throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
 
         let first = throttle.shouldSend(force: false, now: now)
         let suppressed = throttle.shouldSend(force: false, now: now.addingTimeInterval(9.9))
@@ -29,7 +29,7 @@ struct BLEAnnounceThrottleTests {
     @Test
     func forcedAnnounceUsesShorterMinimumInterval() {
         let now = Date(timeIntervalSince1970: 100)
-        var throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
+        let throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
 
         let first = throttle.shouldSend(force: false, now: now)
         let suppressed = throttle.shouldSend(force: true, now: now.addingTimeInterval(1.9))
@@ -43,10 +43,40 @@ struct BLEAnnounceThrottleTests {
     @Test
     func elapsedReportsTimeSinceAcceptedSend() {
         let now = Date(timeIntervalSince1970: 100)
-        var throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
+        let throttle = BLEAnnounceThrottle(normalMinimumInterval: 10, forcedMinimumInterval: 2)
 
         _ = throttle.shouldSend(force: false, now: now)
 
         #expect(throttle.elapsed(since: now.addingTimeInterval(3)) == 3)
     }
+
+    @Test
+    func concurrentRequestsAdmitOnlyOneAnnounce() {
+        let now = Date(timeIntervalSince1970: 100)
+        let throttle = BLEAnnounceThrottle(
+            normalMinimumInterval: 10,
+            forcedMinimumInterval: 2
+        )
+        let accepted = LockedCounter()
+
+        DispatchQueue.concurrentPerform(iterations: 1_000) { _ in
+            if throttle.shouldSend(force: false, now: now) {
+                accepted.increment()
+            }
+        }
+
+        #expect(accepted.value == 1)
+        #expect(throttle.elapsed(since: now.addingTimeInterval(3)) == 3)
+    }
+}
+
+private final class LockedCounter: @unchecked Sendable {
+    private let lock = NSLock()
+    private var count = 0
+
+    var value: Int { lock.withLock { count } }
+
+    func increment() {
+        lock.withLock { count += 1 }
+    }
 }

bitchatTests/Services/BLELocalIdentityStateStoreTests.swift

@@ -0,0 +1,57 @@
+import BitFoundation
+import Foundation
+import Testing
+@testable import bitchat
+
+struct BLELocalIdentityStateStoreTests {
+    @Test
+    func identityReplacementUpdatesWireBytesAtomically() throws {
+        let initial = PeerID(str: "0011223344556677")
+        let replacement = PeerID(str: "8899aabbccddeeff")
+        let store = BLELocalIdentityStateStore(peerID: initial, nickname: "alice")
+
+        store.replacePeerIdentity(with: replacement)
+
+        let snapshot = store.snapshot()
+        #expect(snapshot.peerID == replacement)
+        #expect(snapshot.peerIDData == Data(hexString: replacement.id))
+        #expect(snapshot.nickname == "alice")
+    }
+
+    @Test
+    func concurrentReadsNeverObserveSplitIdentityState() {
+        let peerIDs = [
+            PeerID(str: "0011223344556677"),
+            PeerID(str: "8899aabbccddeeff")
+        ]
+        let store = BLELocalIdentityStateStore(peerID: peerIDs[0], nickname: "alice")
+        let failures = LockedFailureRecorder()
+
+        DispatchQueue.concurrentPerform(iterations: 2_000) { index in
+            if index.isMultiple(of: 2) {
+                store.replacePeerIdentity(with: peerIDs[index % peerIDs.count])
+            } else {
+                store.setNickname(index.isMultiple(of: 3) ? "alice" : "bob")
+            }
+
+            let snapshot = store.snapshot()
+            let expectedWireID = Data(hexString: snapshot.peerID.id) ?? Data()
+            if snapshot.peerIDData != expectedWireID {
+                failures.record()
+            }
+        }
+
+        #expect(!failures.hasFailure)
+    }
+}
+
+private final class LockedFailureRecorder: @unchecked Sendable {
+    private let lock = NSLock()
+    private var failed = false
+
+    var hasFailure: Bool { lock.withLock { failed } }
+
+    func record() {
+        lock.withLock { failed = true }
+    }
+}