diff --git a/bitchat/Services/BLE/BLELinkStateStore.swift b/bitchat/Services/BLE/BLELinkStateStore.swift index ebf39114..79440a64 100644 --- a/bitchat/Services/BLE/BLELinkStateStore.swift +++ b/bitchat/Services/BLE/BLELinkStateStore.swift @@ -8,6 +8,12 @@ struct BLEPeripheralLinkState { var isConnecting: Bool var isConnected: Bool var lastConnectionAttempt: Date? + /// When didConnect last fired for this link. Nil for links restored + /// already-connected (their connect predates this process), which is + /// exactly the signal redundant-link consolidation needs: a restored + /// link lives on an old BLE address the peer no longer advertises, + /// so it must never be kept over a freshly connected duplicate. + var lastConnectedAt: Date? = nil var assembler: NotificationStreamAssembler } @@ -112,11 +118,12 @@ final class BLELinkStateStore { ) } - func markConnected(_ peripheral: CBPeripheral) { + func markConnected(_ peripheral: CBPeripheral, at now: Date = Date()) { let peripheralID = peripheral.identifier.uuidString if updatePeripheral(peripheralID, { $0.isConnecting = false $0.isConnected = true + $0.lastConnectedAt = now }) == nil { setPeripheralState( BLEPeripheralLinkState( @@ -125,6 +132,7 @@ final class BLELinkStateStore { isConnecting: false, isConnected: true, lastConnectionAttempt: nil, + lastConnectedAt: now, assembler: NotificationStreamAssembler() ), for: peripheralID diff --git a/bitchat/Services/BLE/BLERedundantLinkPolicy.swift b/bitchat/Services/BLE/BLERedundantLinkPolicy.swift index 6a053b5a..7bf5c3f4 100644 --- a/bitchat/Services/BLE/BLERedundantLinkPolicy.swift +++ b/bitchat/Services/BLE/BLERedundantLinkPolicy.swift @@ -21,24 +21,53 @@ enum BLERedundantLinkPolicy { /// A link mid-service-rediscovery (didModifyServices cleared it) /// must never be kept over a writable duplicate. let hasCharacteristic: Bool + /// When didConnect last fired for this link in this process. Nil + /// for restored links, whose connect predates the relaunch. + let lastConnectedAt: Date? - init(uuid: String, peerID: PeerID?, isConnected: Bool, hasCharacteristic: Bool) { + init( + uuid: String, + peerID: PeerID?, + isConnected: Bool, + hasCharacteristic: Bool, + lastConnectedAt: Date? = nil + ) { self.uuid = uuid self.peerID = peerID self.isConnected = isConnected self.hasCharacteristic = hasCharacteristic + self.lastConnectedAt = lastConnectedAt } } /// The link to keep when a peer has several connected bound peripheral - /// links, or nil when there is nothing to consolidate. Prefers the - /// ingress link of the verified direct announce that triggered the check - /// (the strongest liveness proof available), falling back to the peer's - /// most recently bound link — but only among writable links while any - /// exist: keeping a characteristic-less link and cancelling the writable + /// links, or nil when there is nothing to consolidate. + /// + /// Prefers the most recently CONNECTED candidate. Duplicates arise when + /// the peer reappears under a fresh BLE address (privacy address + /// rotation) while an older connection — typically state-restored — + /// lives on: only the newest connection sits on the address the peer + /// still advertises. Cancelling that one instead just gets it + /// rediscovered and reconnected, a retire↔reconnect oscillation at the + /// retirement cooldown (field-observed July 31); the older-address link + /// cannot return once cancelled, so consolidation converges immediately. + /// Physical connect recency is also a signal an announce replay cannot + /// nominate, unlike the previous ingress-link preference — announce + /// anchors (ingress, then most recently bound) now only break ties and + /// serve links with no connect timestamp at all. Link "health" signals + /// like RSSI are deliberately not inputs: they are transient and the + /// stale-address link often reads stronger; connect recency is the only + /// signal that tracks address currency. + /// + /// The survivor must be writable while any writable candidate exists: + /// keeping a characteristic-less link and cancelling the writable /// duplicate would strand outbound traffic on the central link until - /// rediscovery finishes. When neither anchor is a viable candidate, - /// consolidation waits for a later announce rather than guessing. + /// rediscovery finishes. But when the physically NEWEST connection is + /// the one that is not writable yet (service discovery still running), + /// consolidation defers entirely — selecting an older writable link + /// would cancel the freshly advertised connection and recreate the + /// oscillation. When no candidate is identifiable, consolidation waits + /// for a later announce rather than guessing. static func keptPeripheralUUID( ingressPeripheralUUID: String?, mostRecentlyBoundUUID: String?, @@ -51,6 +80,42 @@ enum BLERedundantLinkPolicy { let writable = bound.filter(\.hasCharacteristic) let candidates = writable.isEmpty ? bound : writable + // The newest connection is still mid-service-discovery while a + // writable (typically restored, stale-address) duplicate exists: + // defer to a later announce instead of keeping the older link and + // cancelling the one connection on the currently advertised address. + if !writable.isEmpty, + let newestBoundDate = bound.compactMap(\.lastConnectedAt).max(), + !writable.contains(where: { $0.lastConnectedAt == newestBoundDate }) { + return nil + } + + if let newestDate = candidates.compactMap(\.lastConnectedAt).max() { + let newest = candidates.filter { $0.lastConnectedAt == newestDate } + if newest.count == 1 { + return newest[0].uuid + } + return anchoredChoice( + among: newest, + ingressPeripheralUUID: ingressPeripheralUUID, + mostRecentlyBoundUUID: mostRecentlyBoundUUID + ) ?? newest.map(\.uuid).min() + } + + return anchoredChoice( + among: candidates, + ingressPeripheralUUID: ingressPeripheralUUID, + mostRecentlyBoundUUID: mostRecentlyBoundUUID + ) + } + + /// The pre-timestamp anchors: the verified announce's ingress link, + /// then the peer's most recently bound link. + private static func anchoredChoice( + among candidates: [PeripheralLink], + ingressPeripheralUUID: String?, + mostRecentlyBoundUUID: String? + ) -> String? { if let ingressPeripheralUUID, candidates.contains(where: { $0.uuid == ingressPeripheralUUID }) { return ingressPeripheralUUID } diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index 5f29ade3..8891b40e 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -6376,7 +6376,8 @@ extension BLEService { store.peripheralStates.map { (uuid: $0.peripheral.identifier.uuidString, isConnected: $0.isConnected, - hasCharacteristic: $0.characteristic != nil) + hasCharacteristic: $0.characteristic != nil, + lastConnectedAt: $0.lastConnectedAt) } } return physical.map { @@ -6384,7 +6385,8 @@ extension BLEService { uuid: $0.uuid, peerID: linkBindings.peer(forPeripheralID: $0.uuid), isConnected: $0.isConnected, - hasCharacteristic: $0.hasCharacteristic + hasCharacteristic: $0.hasCharacteristic, + lastConnectedAt: $0.lastConnectedAt ) } } diff --git a/bitchatTests/Services/BLERedundantLinkPolicyTests.swift b/bitchatTests/Services/BLERedundantLinkPolicyTests.swift index db23a5e8..d544535b 100644 --- a/bitchatTests/Services/BLERedundantLinkPolicyTests.swift +++ b/bitchatTests/Services/BLERedundantLinkPolicyTests.swift @@ -7,8 +7,8 @@ struct BLERedundantLinkPolicyTests { private let peer = PeerID(str: "1122334455667788") private let otherPeer = PeerID(str: "8877665544332211") - private func link(_ uuid: String, _ peerID: PeerID?, connected: Bool = true, writable: Bool = true) -> BLERedundantLinkPolicy.PeripheralLink { - BLERedundantLinkPolicy.PeripheralLink(uuid: uuid, peerID: peerID, isConnected: connected, hasCharacteristic: writable) + private func link(_ uuid: String, _ peerID: PeerID?, connected: Bool = true, writable: Bool = true, connectedAt: Date? = nil) -> BLERedundantLinkPolicy.PeripheralLink { + BLERedundantLinkPolicy.PeripheralLink(uuid: uuid, peerID: peerID, isConnected: connected, hasCharacteristic: writable, lastConnectedAt: connectedAt) } @Test @@ -131,4 +131,144 @@ struct BLERedundantLinkPolicyTests { ) #expect(Set(retiring) == Set(["p-stale-1", "p-stale-2"])) } + + // MARK: Connect-recency preference (the July 31 retire↔reconnect fix) + + @Test + func newestConnectionWinsOverIngressAndBindingAnchors() { + // Field oscillation: the restored old-address link (no connect + // timestamp) carried the announce ingress AND the binding, so it + // kept winning — and the cancelled fresh-address link kept getting + // rediscovered and reconnected. Physical connect recency must beat + // both announce anchors. + let now = Date() + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-restored", + mostRecentlyBoundUUID: "p-restored", + links: [ + link("p-restored", peer), + link("p-fresh", peer, connectedAt: now) + ], + peerID: peer + ) + #expect(kept == "p-fresh") + } + + @Test + func amongTimestampedLinksTheNewestWins() { + let now = Date() + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-older", + mostRecentlyBoundUUID: "p-older", + links: [ + link("p-older", peer, connectedAt: now.addingTimeInterval(-30)), + link("p-newer", peer, connectedAt: now) + ], + peerID: peer + ) + #expect(kept == "p-newer") + } + + @Test + func newestLinkMidDiscoveryDefersInsteadOfKeepingOlderWritable() { + // The fresh connection hasn't finished service discovery, so it is + // not writable yet. Keeping the older writable (restored) link now + // would cancel the one connection on the currently advertised + // address and recreate the oscillation — defer to a later announce. + let now = Date() + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-writable", + mostRecentlyBoundUUID: "p-writable", + links: [ + link("p-writable", peer, connectedAt: now.addingTimeInterval(-30)), + link("p-fresh-bare", peer, writable: false, connectedAt: now) + ], + peerID: peer + ) + #expect(kept == nil) + } + + @Test + func restoredWritableAnchorAlsoDefersToFreshUnwritableLink() { + // Same discovery window as above, but the writable duplicate is a + // restored link with no connect timestamp at all — the exact field + // topology. It must not win just because the fresh link is bare. + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-restored", + mostRecentlyBoundUUID: "p-restored", + links: [ + link("p-restored", peer), + link("p-fresh-bare", peer, writable: false, connectedAt: Date()) + ], + peerID: peer + ) + #expect(kept == nil) + } + + @Test + func coNewestWritableLinkStillWinsOverBareTwin() { + // Two links share the newest timestamp and one is writable: no + // discovery window to wait out — the writable co-newest survives. + let now = Date() + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: nil, + mostRecentlyBoundUUID: nil, + links: [ + link("p-bare", peer, writable: false, connectedAt: now), + link("p-writable", peer, connectedAt: now) + ], + peerID: peer + ) + #expect(kept == "p-writable") + } + + @Test + func allUnwritableDuplicatesConsolidateByConnectRecency() { + // No writable link exists at all: nothing can be stranded, so the + // newest connection consolidates immediately. + let now = Date() + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-older", + mostRecentlyBoundUUID: "p-older", + links: [ + link("p-older", peer, writable: false, connectedAt: now.addingTimeInterval(-30)), + link("p-newer", peer, writable: false, connectedAt: now) + ], + peerID: peer + ) + #expect(kept == "p-newer") + } + + @Test + func allRestoredLinksFallBackToAnnounceAnchors() { + // No connect timestamps at all (every link restored): the legacy + // ingress-then-binding preference still decides. + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-ingress", + mostRecentlyBoundUUID: "p-bound", + links: [link("p-ingress", peer), link("p-bound", peer)], + peerID: peer + ) + #expect(kept == "p-ingress") + } + + @Test + func timestampTiesBreakByAnchorsThenDeterministically() { + let now = Date() + let anchored = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-b", + mostRecentlyBoundUUID: nil, + links: [link("p-a", peer, connectedAt: now), link("p-b", peer, connectedAt: now)], + peerID: peer + ) + #expect(anchored == "p-b") + + let unanchored = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: nil, + mostRecentlyBoundUUID: nil, + links: [link("p-b", peer, connectedAt: now), link("p-a", peer, connectedAt: now)], + peerID: peer + ) + #expect(unanchored == "p-a") + } }