diff --git a/bitchat/Services/BLE/BLEFanoutSelector.swift b/bitchat/Services/BLE/BLEFanoutSelector.swift index 6cabcdd2..dadb9df1 100644 --- a/bitchat/Services/BLE/BLEFanoutSelector.swift +++ b/bitchat/Services/BLE/BLEFanoutSelector.swift @@ -15,6 +15,8 @@ enum BLEFanoutSelector { excludedLinks: Set = [], peripheralPeerBindings: [String: PeerID] = [:], centralPeerBindings: [String: PeerID] = [:], + preferredPeripheralPerPeer: [PeerID: String] = [:], + collapseDuplicatePeerLinks: Bool = true, directedPeerHint: PeerID?, packetType: UInt8, messageID: String @@ -31,7 +33,8 @@ enum BLEFanoutSelector { to: directedPeerHint, links: rawAllowed, peripheralPeerBindings: peripheralPeerBindings, - centralPeerBindings: centralPeerBindings + centralPeerBindings: centralPeerBindings, + preferredPeripheralPerPeer: preferredPeripheralPerPeer ) { return directedSelection } @@ -46,11 +49,20 @@ enum BLEFanoutSelector { return BLEFanoutSelection(peripheralIDs: [], centralIDs: []) } - let allowed = collapseDuplicateLinksPerPeer( - rawAllowed, - peripheralPeerBindings: peripheralPeerBindings, - centralPeerBindings: centralPeerBindings - ) + // Direct announces are the packet that binds a link to its peer + // (BLEService's raw bind and verified rebind). Collapsing them per + // peer starves duplicate same-peer links of the announce they need to + // become bound — the duplicates then look "pre-announce" forever and + // every broadcast sprays down all of them. Announces are small and + // throttled, so they go on every live link. + let allowed = collapseDuplicatePeerLinks + ? collapseDuplicateLinksPerPeer( + rawAllowed, + peripheralPeerBindings: peripheralPeerBindings, + centralPeerBindings: centralPeerBindings, + preferredPeripheralPerPeer: preferredPeripheralPerPeer + ) + : rawAllowed guard shouldSubset(packetType: packetType, directedPeerHint: directedPeerHint) else { return BLEFanoutSelection( @@ -97,7 +109,8 @@ enum BLEFanoutSelector { to peerID: PeerID, links: (peripheralIDs: [String], centralIDs: [String]), peripheralPeerBindings: [String: PeerID], - centralPeerBindings: [String: PeerID] + centralPeerBindings: [String: PeerID], + preferredPeripheralPerPeer: [PeerID: String] ) -> BLEFanoutSelection? { let directLinks = collapseDuplicateLinksPerPeer( ( @@ -105,7 +118,8 @@ enum BLEFanoutSelector { centralIDs: links.centralIDs.filter { centralPeerBindings[$0] == peerID } ), peripheralPeerBindings: peripheralPeerBindings, - centralPeerBindings: centralPeerBindings + centralPeerBindings: centralPeerBindings, + preferredPeripheralPerPeer: preferredPeripheralPerPeer ) guard !directLinks.peripheralIDs.isEmpty || !directLinks.centralIDs.isEmpty else { @@ -140,7 +154,8 @@ enum BLEFanoutSelector { private static func collapseDuplicateLinksPerPeer( _ links: (peripheralIDs: [String], centralIDs: [String]), peripheralPeerBindings: [String: PeerID], - centralPeerBindings: [String: PeerID] + centralPeerBindings: [String: PeerID], + preferredPeripheralPerPeer: [PeerID: String] ) -> (peripheralIDs: [String], centralIDs: [String]) { guard !peripheralPeerBindings.isEmpty || !centralPeerBindings.isEmpty else { return links @@ -148,13 +163,30 @@ enum BLEFanoutSelector { var seenPeers = Set() var keptPeripheralIDs: [String] = [] + // When a peer has several bound peripheral links (duplicate + // connections after a restore), collapse onto its preferred one (the + // most recently bound) instead of dictionary order — an arbitrary + // pick could route a peer's single collapsed copy down a stale link. for id in links.peripheralIDs { - if let peer = peripheralPeerBindings[id], !seenPeers.insert(peer).inserted { - continue + guard let peer = peripheralPeerBindings[id], + preferredPeripheralPerPeer[peer] == id, + seenPeers.insert(peer).inserted else { continue } + keptPeripheralIDs.append(id) + } + for id in links.peripheralIDs { + if let peer = peripheralPeerBindings[id] { + if preferredPeripheralPerPeer[peer] == id { continue } + if !seenPeers.insert(peer).inserted { continue } } keptPeripheralIDs.append(id) } + // Known limitation: centrals collapse in subscription order (oldest + // first) — there is no recency signal like the peripheral reverse + // map. A central-only peer with duplicate subscriptions rides the + // oldest one until the remote side (which owns those connections) + // consolidates on its next verified announce (bounded by its + // retirement cooldown). var keptCentralIDs: [String] = [] for id in links.centralIDs { if let peer = centralPeerBindings[id], !seenPeers.insert(peer).inserted { diff --git a/bitchat/Services/BLE/BLELinkStateStore.swift b/bitchat/Services/BLE/BLELinkStateStore.swift index cbb75fa2..31914092 100644 --- a/bitchat/Services/BLE/BLELinkStateStore.swift +++ b/bitchat/Services/BLE/BLELinkStateStore.swift @@ -164,7 +164,11 @@ final class BLELinkStateStore { guard let peerID else { return [] } var links: Set = [] - if let peripheralUUID = peerToPeripheralUUID[peerID] { + // Scan all states rather than the 1:1 reverse map: after a state + // restoration the same device can hold several live peripheral links + // bound to one peer (it reappears under a fresh UUID while the + // restored connection lives on). + for (peripheralUUID, state) in peripherals where state.peerID == peerID { links.insert(.peripheral(peripheralUUID)) } for (centralUUID, mappedPeerID) in centralToPeerID where mappedPeerID == peerID { @@ -173,6 +177,13 @@ final class BLELinkStateStore { return links } + /// The peer's most recently bound peripheral link, per peer. Used to keep + /// duplicate-link fanout collapse deterministic (see BLEFanoutSelector). + var preferredPeripheralBindings: [PeerID: String] { + assertOwned() + return peerToPeripheralUUID + } + func peerID(forPeripheralID peripheralID: String) -> PeerID? { assertOwned() return peripherals[peripheralID]?.peerID @@ -221,8 +232,19 @@ final class BLELinkStateStore { func removePeripheral(_ peripheralID: String) -> PeerID? { assertOwned() let peerID = peripherals.removeValue(forKey: peripheralID)?.peerID - if let peerID { - peerToPeripheralUUID.removeValue(forKey: peerID) + // Only clear (or repair) the reverse map when it points at the removed + // link: with duplicate links to one peer, removing a stale duplicate + // must not strand the peer's surviving bound link. + if let peerID, peerToPeripheralUUID[peerID] == peripheralID { + // Prefer a writable survivor: repairing onto a link that is + // mid-service-rediscovery would strand directed sends until the + // characteristic comes back. + let survivors = peripherals.filter { $0.value.peerID == peerID && $0.value.isConnected } + if let survivorUUID = survivors.first(where: { $0.value.characteristic != nil })?.key ?? survivors.first?.key { + peerToPeripheralUUID[peerID] = survivorUUID + } else { + peerToPeripheralUUID.removeValue(forKey: peerID) + } } return peerID } diff --git a/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift b/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift index 3e9d7fe7..4284cfb9 100644 --- a/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift +++ b/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift @@ -12,6 +12,14 @@ struct BLEOutboundFragmentTransferRequest { guard packet.type == MessageType.fileTransfer.rawValue else { return nil } return transferId ?? packet.payload.sha256Hex() } + + /// Content identity independent of the caller-chosen transfer ID: the + /// same file resent through another path (gossip-sync replay, retry) + /// arrives with a different explicit transferId but identical payload. + var contentKey: String? { + guard packet.type == MessageType.fileTransfer.rawValue else { return nil } + return packet.payload.sha256Hex() + } } struct BLEOutboundFragmentTransferScheduler { @@ -23,6 +31,11 @@ struct BLEOutboundFragmentTransferScheduler { enum SubmitResult { case start(request: BLEOutboundFragmentTransferRequest, reservedTransferId: String?) case queued(request: BLEOutboundFragmentTransferRequest, transferId: String?, position: QueuePosition) + /// The same file is already being (or waiting to be) fragmented out + /// to an audience covering this request; sending it again would just + /// double the airtime (field-verified: one 41KB voice file went out + /// as two complete fragment streams). + case droppedDuplicate(request: BLEOutboundFragmentTransferRequest, activeTransferId: String?) } enum CancelResult { @@ -38,14 +51,34 @@ struct BLEOutboundFragmentTransferScheduler { } private struct ActiveTransferState { - let totalFragments: Int + var totalFragments: Int var sentFragments: Int var workItems: [DispatchWorkItem] + var contentKey: String? + var directedPeer: PeerID? } private var activeTransfers: [String: ActiveTransferState] = [:] private var pendingTransfers: [BLEOutboundFragmentTransferRequest] = [] + /// A transfer of the same content whose audience covers `directedPeer`: + /// a broadcast covers every peer; a directed transfer covers only its + /// recipient. A directed resend to a peer NOT covered by what's in + /// flight (different recipient of a private file) is never a duplicate. + private func coveringDuplicate(contentKey: String, directedPeer: PeerID?) -> String? { + for (transferId, state) in activeTransfers where state.contentKey == contentKey { + if state.directedPeer == nil || state.directedPeer == directedPeer { + return transferId + } + } + for request in pendingTransfers where request.contentKey == contentKey { + if request.directedPeer == nil || request.directedPeer == directedPeer { + return request.resolvedTransferId + } + } + return nil + } + var activeCount: Int { activeTransfers.count } @@ -69,6 +102,16 @@ struct BLEOutboundFragmentTransferScheduler { return .start(request: request, reservedTransferId: nil) } + // Only requests without an explicit transferId are dropped as + // duplicates: those are resend paths (gossip-sync replay, directed + // spool) with no UI waiting on them. An app-initiated send carries a + // transferId whose progress events the UI tracks, so it always runs. + if request.transferId == nil, + let contentKey = request.contentKey, + let coveringId = coveringDuplicate(contentKey: contentKey, directedPeer: request.directedPeer) { + return .droppedDuplicate(request: request, activeTransferId: coveringId) + } + guard activeTransfers.count < maxConcurrentTransfers else { pendingTransfers.append(request) return .queued(request: request, transferId: transferId, position: .back) @@ -79,7 +122,13 @@ struct BLEOutboundFragmentTransferScheduler { return .queued(request: request, transferId: transferId, position: .front) } - activeTransfers[transferId] = ActiveTransferState(totalFragments: 0, sentFragments: 0, workItems: []) + activeTransfers[transferId] = ActiveTransferState( + totalFragments: 0, + sentFragments: 0, + workItems: [], + contentKey: request.contentKey, + directedPeer: request.directedPeer + ) return .start(request: request, reservedTransferId: transferId) } @@ -88,12 +137,11 @@ struct BLEOutboundFragmentTransferScheduler { totalFragments: Int, workItems: [DispatchWorkItem] ) -> Bool { - guard activeTransfers[transferId] != nil else { return false } - activeTransfers[transferId] = ActiveTransferState( - totalFragments: totalFragments, - sentFragments: 0, - workItems: workItems - ) + guard var state = activeTransfers[transferId] else { return false } + state.totalFragments = totalFragments + state.sentFragments = 0 + state.workItems = workItems + activeTransfers[transferId] = state return true } @@ -149,13 +197,25 @@ struct BLEOutboundFragmentTransferScheduler { while availableSlots > 0, !pendingTransfers.isEmpty { let request = pendingTransfers.removeFirst() - availableSlots -= 1 guard let transferId = request.resolvedTransferId else { + availableSlots -= 1 results.append(.start(request: request, reservedTransferId: nil)) continue } + // A queued duplicate of content that started while it waited + // must not resend the whole file once the slot frees up (same + // explicit-transferId exemption as submit). + if request.transferId == nil, + let contentKey = request.contentKey, + let coveringId = coveringDuplicate(contentKey: contentKey, directedPeer: request.directedPeer) { + results.append(.droppedDuplicate(request: request, activeTransferId: coveringId)) + continue + } + + availableSlots -= 1 + guard activeTransfers.count < maxConcurrentTransfers else { pendingTransfers.insert(request, at: 0) results.append(.queued(request: request, transferId: transferId, position: .front)) @@ -168,7 +228,13 @@ struct BLEOutboundFragmentTransferScheduler { continue } - activeTransfers[transferId] = ActiveTransferState(totalFragments: 0, sentFragments: 0, workItems: []) + activeTransfers[transferId] = ActiveTransferState( + totalFragments: 0, + sentFragments: 0, + workItems: [], + contentKey: request.contentKey, + directedPeer: request.directedPeer + ) results.append(.start(request: request, reservedTransferId: transferId)) } diff --git a/bitchat/Services/BLE/BLEOutboundLinkPlanner.swift b/bitchat/Services/BLE/BLEOutboundLinkPlanner.swift index 4462ecc8..5f86ebea 100644 --- a/bitchat/Services/BLE/BLEOutboundLinkPlanner.swift +++ b/bitchat/Services/BLE/BLEOutboundLinkPlanner.swift @@ -20,6 +20,8 @@ enum BLEOutboundLinkPlanner { excludedLinks: Set, peripheralPeerBindings: [String: PeerID] = [:], centralPeerBindings: [String: PeerID] = [:], + preferredPeripheralPerPeer: [PeerID: String] = [:], + directAnnounceTTL: UInt8 = TransportConfig.messageTTLDefault, directedOnlyPeer: PeerID? ) -> BLEOutboundLinkPlan { if let minLimit = minimumLinkLimit( @@ -36,6 +38,9 @@ enum BLEOutboundLinkPlanner { } let directedPeerHint = directedPeerHint(for: packet, explicitPeer: directedOnlyPeer) + // Direct announces bypass the per-peer duplicate-link collapse so + // every live link gets bound (see BLEFanoutSelector.selectLinks). + let isDirectAnnounce = packet.type == MessageType.announce.rawValue && packet.ttl == directAnnounceTTL let selectedLinks = BLEFanoutSelector.selectLinks( peripheralIDs: peripheralIDs, centralIDs: centralIDs, @@ -43,6 +48,8 @@ enum BLEOutboundLinkPlanner { excludedLinks: excludedLinks, peripheralPeerBindings: peripheralPeerBindings, centralPeerBindings: centralPeerBindings, + preferredPeripheralPerPeer: preferredPeripheralPerPeer, + collapseDuplicatePeerLinks: !isDirectAnnounce, directedPeerHint: directedPeerHint, packetType: packet.type, messageID: BLEOutboundPacketPolicy.messageID(for: packet) diff --git a/bitchat/Services/BLE/BLERedundantLinkPolicy.swift b/bitchat/Services/BLE/BLERedundantLinkPolicy.swift new file mode 100644 index 00000000..6a053b5a --- /dev/null +++ b/bitchat/Services/BLE/BLERedundantLinkPolicy.swift @@ -0,0 +1,73 @@ +import BitFoundation +import Foundation + +/// Decides which central-role connections (peripheral links we own) are +/// redundant duplicates of a peer's live link. +/// +/// One connection per role per peer is the normal dual-role topology (each +/// device is both central and peripheral). After a BLE state-restoration +/// relaunch, though, the same phone can reappear under a fresh peripheral +/// UUID while the restored connection lives on — leaving several live +/// central-role connections to one peer, each carrying every packet +/// (field-verified: 2-3x airtime on all traffic). Only same-role duplicates +/// are retired; the peer's central-role subscription on our peripheral +/// manager is its own connection to manage, and it runs the same policy. +enum BLERedundantLinkPolicy { + struct PeripheralLink: Equatable { + let uuid: String + let peerID: PeerID? + let isConnected: Bool + /// Whether the link has a discovered characteristic (is writable). + /// A link mid-service-rediscovery (didModifyServices cleared it) + /// must never be kept over a writable duplicate. + let hasCharacteristic: Bool + + init(uuid: String, peerID: PeerID?, isConnected: Bool, hasCharacteristic: Bool) { + self.uuid = uuid + self.peerID = peerID + self.isConnected = isConnected + self.hasCharacteristic = hasCharacteristic + } + } + + /// 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 + /// 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. + static func keptPeripheralUUID( + ingressPeripheralUUID: String?, + mostRecentlyBoundUUID: String?, + links: [PeripheralLink], + peerID: PeerID + ) -> String? { + let bound = links.filter { $0.peerID == peerID && $0.isConnected } + guard bound.count > 1 else { return nil } + + let writable = bound.filter(\.hasCharacteristic) + let candidates = writable.isEmpty ? bound : writable + + if let ingressPeripheralUUID, candidates.contains(where: { $0.uuid == ingressPeripheralUUID }) { + return ingressPeripheralUUID + } + if let mostRecentlyBoundUUID, candidates.contains(where: { $0.uuid == mostRecentlyBoundUUID }) { + return mostRecentlyBoundUUID + } + return nil + } + + /// Connected peripheral links bound to the peer other than the kept one. + static func peripheralUUIDsToRetire( + links: [PeripheralLink], + peerID: PeerID, + keeping keptUUID: String + ) -> [String] { + links + .filter { $0.peerID == peerID && $0.isConnected && $0.uuid != keptUUID } + .map(\.uuid) + } +} diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index 56fee0b6..2ef8aba6 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -41,6 +41,10 @@ final class BLEService: NSObject { // store): entries older than the cooldown are pruned on insert. private var lastLinkRebindAt: [String: Date] = [:] + // Redundant-link retirement cooldown per peer (bleQueue-owned): bounds + // how often a replayed announce could flip which duplicate link survives. + private var lastRedundantLinkRetirementAt: [PeerID: Date] = [:] + // BCH-01-004: Rate-limiting for subscription-triggered announces. private var subscriptionAnnounceLimiter = BLESubscriptionAnnounceLimiter() @@ -1190,8 +1194,13 @@ final class BLEService: NSObject { let outboundPriority = BLEOutboundPacketPolicy.priority(for: packet, data: data) let states = snapshotPeripheralStates() - let connectedStates = states.filter { $0.isConnected } - let subscribedCentrals = characteristic == nil ? [] : snapshotSubscribedCentrals().centrals + // A link without a discovered characteristic cannot be written to + // (the write loop below skips it); offering it to the planner only + // wastes fanout slots — and a peer's single collapsed copy would be + // silently dropped if its bound link is still mid-rediscovery. + let connectedStates = states.filter { $0.isConnected && $0.characteristic != nil } + let centralSnapshot = snapshotSubscribedCentrals() + let subscribedCentrals = characteristic == nil ? [] : centralSnapshot.centrals let connectedPeripheralIDs = connectedStates.map { $0.peripheral.identifier.uuidString } let centralIDs = subscribedCentrals.map { $0.identifier.uuidString } let peripheralPeerBindings = Dictionary(uniqueKeysWithValues: connectedStates.compactMap { state in @@ -1207,7 +1216,12 @@ final class BLEService: NSObject { ingressRecord: ingressRecord, excludedLinks: excludedPeerLinks, peripheralPeerBindings: peripheralPeerBindings, - centralPeerBindings: snapshotSubscribedCentrals().peerIDsByCentralUUID, + centralPeerBindings: centralSnapshot.peerIDsByCentralUUID, + // Perf note: this is a third bleQueue hop per send; if send-path + // profiling ever flags it, fold it into snapshotPeripheralStates + // as a combined snapshot. + preferredPeripheralPerPeer: readLinkState { $0.preferredPeripheralBindings }, + directAnnounceTTL: messageTTL, directedOnlyPeer: directedOnlyPeer ) @@ -1925,7 +1939,17 @@ extension BLEService: CBCentralManagerDelegate { // Clean up references and peer mappings _ = linkStateStore.removePeripheral(peripheralID) - if let peerID { + // A duplicate link can drop while the peer stays live on another + // (the dual-role central link, or a second bound link after a + // restore): peer-disconnect bookkeeping only runs once the peer's + // last live link is gone. removePeripheral just repaired the reverse + // map onto a connected survivor, so directLinkState is accurate + // here. The scan restart and connect-slot refill below stay + // unguarded — they respond to the physical drop regardless of + // remaining logical links. + let remainingLinks = peerID.map { linkStateStore.directLinkState(for: $0) } + let peerStillLinked = (remainingLinks?.hasPeripheral ?? false) || (remainingLinks?.hasCentral ?? false) + if let peerID, !peerStillLinked { // Do not remove peer; mark as not connected but retain for reachability collectionsQueue.sync(flags: .barrier) { peerRegistry.markDisconnected(peerID) @@ -1933,7 +1957,7 @@ extension BLEService: CBCentralManagerDelegate { refreshLocalTopology() } - + // Restart scanning with allow duplicates for faster rediscovery if centralManager?.state == .poweredOn { // Stop and restart scanning to ensure we get fresh discovery events @@ -1944,15 +1968,15 @@ extension BLEService: CBCentralManagerDelegate { } // Attempt to fill freed slot from queue bleQueue.async { [weak self] in self?.tryConnectFromQueue() } - + // Notify delegate about disconnection on main thread (direct link dropped) notifyUI { [weak self] in guard let self = self else { return } - + // Get current peer list (after removal) let currentPeerIDs = self.collectionsQueue.sync { self.peerRegistry.peerIDs } - - if let peerID { + + if let peerID, !peerStillLinked { self.notifyPeerDisconnectedDebounced(peerID) } self.requestPeerDataPublish() @@ -2565,6 +2589,13 @@ extension BLEService: CBPeripheralManagerDelegate { // Find and disconnect the peer associated with this central if let peerID = removedPeerID { + // The remote side retiring a redundant duplicate connection + // arrives here as an unsubscribe while the peer stays live on + // its other links; only the peer's last link disconnecting + // counts. If every link truly dropped, the surviving-link + // callbacks (didDisconnectPeripheral, or this one again) run + // the bookkeeping. + guard linkStateStore.links(to: peerID).isEmpty else { return } // Mark peer as not connected; retain for reachability collectionsQueue.sync(flags: .barrier) { peerRegistry.markDisconnected(peerID) @@ -4114,6 +4145,12 @@ extension BLEService { } else { SecureLogger.debug("🚦 Queued fragment transfer waiting for slot", category: .session) } + + case let .droppedDuplicate(_, activeTransferId): + SecureLogger.debug( + "🔁 Skipping duplicate outbound transfer — same content already in flight as \(activeTransferId?.prefix(8) ?? "?")…", + category: .session + ) } } @@ -4446,9 +4483,11 @@ extension BLEService { } // A verified direct announce proves the sender owns the link it came - // in on: heal any stale binding left by a peer-ID rotation. + // in on: heal any stale binding left by a peer-ID rotation, and + // consolidate duplicate same-role connections onto that link. if let result, result.isVerified, result.isDirectAnnounce { rebindLinkAfterVerifiedDirectAnnounce(packet, to: result.peerID) + retireRedundantPeripheralLinks(packet, to: result.peerID) } // Bridge courier watch: a verified announce may add a peer whose @@ -4538,6 +4577,12 @@ extension BLEService { self.messageQueue.async { [weak self] in self?.promoteReboundPeerToConnected(peerID) } + // Any other peripheral links still bound to the rotated-away ID + // are stale duplicates of the same physical device (its restored + // connections outlived the relaunch that rotated the ID): cancel + // them now instead of leaving ghost links that spray duplicate + // traffic until the inactivity timeout. + self.cancelBoundPeripheralLinks(to: previousPeerID, keeping: linkUUID) // Retire the rotated-away ID only once its last link is gone; a // remaining stale link heals the same way or ages out. guard self.linkStateStore.links(to: previousPeerID).isEmpty else { return } @@ -4547,6 +4592,87 @@ extension BLEService { } } + /// After a restore relaunch the same phone can reappear under a fresh + /// peripheral UUID while its restored connection lives on, leaving + /// several live central-role connections to one peer that each carry + /// every packet (field-verified: every voice frame arrived 2-3x). A + /// verified direct announce is the consolidation point: keep the link it + /// proves live (or the peer's most recently bound one) and cancel the + /// rest. Only same-role duplicates are touched — one connection per role + /// is the normal dual-role topology — and only connections we own as + /// central: the peer's central subscriptions on our peripheral manager + /// are its connections to cancel, and it runs this same policy. + /// + /// Directness is forgeable (TTL is unsigned), so a replayed announce + /// could nominate the replayer's link as the survivor. Containment + /// mirrors the rotation rebind: only links already BOUND to the peer are + /// retired (announce-evidenced, never pre-announce links), at most one + /// retirement per peer per cooldown window, and the peer keeps a live + /// link either way. + private func retireRedundantPeripheralLinks(_ packet: BitchatPacket, to peerID: PeerID) { + let ingressLink = collectionsQueue.sync { ingressLinks.link(for: packet) } + bleQueue.async { [weak self] in + guard let self else { return } + let now = Date() + self.lastRedundantLinkRetirementAt = self.lastRedundantLinkRetirementAt.filter { + now.timeIntervalSince($0.value) < TransportConfig.bleLinkRebindCooldownSeconds + } + guard self.lastRedundantLinkRetirementAt[peerID] == nil else { return } + + var ingressPeripheralUUID: String? + if case .peripheral(let uuid) = ingressLink { + ingressPeripheralUUID = uuid + } + guard let keptUUID = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: ingressPeripheralUUID, + mostRecentlyBoundUUID: self.linkStateStore.preferredPeripheralBindings[peerID], + links: self.peripheralLinkPolicySnapshot(), + peerID: peerID + ) else { return } + + self.lastRedundantLinkRetirementAt[peerID] = now + // The survivor becomes the peer's reverse-mapped link so directed + // sends follow the consolidation. + self.linkStateStore.bindPeripheral(keptUUID, to: peerID) + self.cancelBoundPeripheralLinks(to: peerID, keeping: keptUUID) + self.refreshLocalTopology() + } + } + + /// Cancels our central-role connections whose link is bound to `peerID`, + /// except `keptUUID`. bleQueue only. Each entry is removed from the link + /// store BEFORE cancelling so didDisconnectPeripheral sees no peer + /// binding and skips its peer-disconnect bookkeeping — the peer is still + /// live (on the kept link, or under its rotated identity). + private func cancelBoundPeripheralLinks(to peerID: PeerID, keeping keptUUID: String?) { + let retiring = BLERedundantLinkPolicy.peripheralUUIDsToRetire( + links: peripheralLinkPolicySnapshot(), + peerID: peerID, + keeping: keptUUID ?? "" + ) + for uuid in retiring { + guard let state = linkStateStore.state(forPeripheralID: uuid) else { continue } + _ = linkStateStore.removePeripheral(uuid) + SecureLogger.info( + "🔗 Retiring redundant link \(uuid.prefix(8))… bound to \(peerID.id.prefix(8))…\(keptUUID.map { " (keeping \($0.prefix(8))…)" } ?? "")", + category: .session + ) + centralManager?.cancelPeripheralConnection(state.peripheral) + } + } + + /// bleQueue only (reads the link store). + private func peripheralLinkPolicySnapshot() -> [BLERedundantLinkPolicy.PeripheralLink] { + linkStateStore.peripheralStates.map { + BLERedundantLinkPolicy.PeripheralLink( + uuid: $0.peripheral.identifier.uuidString, + peerID: $0.peerID, + isConnected: $0.isConnected, + hasCharacteristic: $0.characteristic != nil + ) + } + } + /// After a successful verified rebind the new identity owns a live link, /// but its announce was stored disconnected (the link was still bound to /// the rotated-away ID when the registry upsert ran). Flip it to diff --git a/bitchat/Views/Components/TextMessageView.swift b/bitchat/Views/Components/TextMessageView.swift index 497b4280..2e1009d7 100644 --- a/bitchat/Views/Components/TextMessageView.swift +++ b/bitchat/Views/Components/TextMessageView.swift @@ -131,9 +131,14 @@ struct TextMessageView: View { } // Collapse the revealed caption when the status advances (e.g. // sending → sent → delivered) so a detail opened for one state - // doesn't linger and silently morph into another. + // doesn't linger and silently morph into another. Guarded write: + // under a message storm many rows change status within one frame, + // and an unconditional state write per change trips SwiftUI's + // "tried to update multiple times per frame" re-entrancy warning. .onChange(of: deliveryStatus) { _ in - showDeliveryDetail = false + if showDeliveryDetail { + showDeliveryDetail = false + } } } } diff --git a/bitchat/Views/Media/MediaMessageView.swift b/bitchat/Views/Media/MediaMessageView.swift index dd3b79b0..2b12ee24 100644 --- a/bitchat/Views/Media/MediaMessageView.swift +++ b/bitchat/Views/Media/MediaMessageView.swift @@ -121,9 +121,14 @@ struct MediaMessageView: View { .padding(.vertical, 4) // Collapse the revealed caption when the status advances (e.g. // sending → sent → delivered) so a detail opened for one state - // doesn't linger and silently morph into another. + // doesn't linger and silently morph into another. Guarded write: + // under a message storm many rows change status within one frame, + // and an unconditional state write per change trips SwiftUI's + // "tried to update multiple times per frame" re-entrancy warning. .onChange(of: deliveryStatus) { _ in - showDeliveryDetail = false + if showDeliveryDetail { + showDeliveryDetail = false + } } } diff --git a/bitchatTests/Services/BLEFanoutSelectorTests.swift b/bitchatTests/Services/BLEFanoutSelectorTests.swift index deec44f5..5832b676 100644 --- a/bitchatTests/Services/BLEFanoutSelectorTests.swift +++ b/bitchatTests/Services/BLEFanoutSelectorTests.swift @@ -192,6 +192,75 @@ struct BLEFanoutSelectorTests { #expect(selection.centralIDs == Set(["c-unbound"])) } + @Test + func duplicateBoundLinksToOnePeerCollapseToItsPreferredLink() { + // After a restore the same phone can hold several live links bound to + // one peer; broadcasts must go down exactly one — the most recently + // bound (preferred) one, not dictionary order. + let peer = PeerID(str: "1122334455667788") + let selection = BLEFanoutSelector.selectLinks( + peripheralIDs: ["p-stale", "p-preferred", "p-stale-2"], + centralIDs: ["c-bound"], + ingressLink: nil, + peripheralPeerBindings: [ + "p-stale": peer, + "p-preferred": peer, + "p-stale-2": peer + ], + centralPeerBindings: ["c-bound": peer], + preferredPeripheralPerPeer: [peer: "p-preferred"], + directedPeerHint: nil, + packetType: MessageType.fragment.rawValue, + messageID: "message-1" + ) + + #expect(selection.peripheralIDs == Set(["p-preferred"])) + #expect(selection.centralIDs.isEmpty) + } + + @Test + func directedSendCollapsesDuplicateBoundLinksToPreferred() { + let peer = PeerID(str: "1122334455667788") + let selection = BLEFanoutSelector.selectLinks( + peripheralIDs: ["p-stale", "p-preferred"], + centralIDs: [], + ingressLink: nil, + peripheralPeerBindings: [ + "p-stale": peer, + "p-preferred": peer + ], + preferredPeripheralPerPeer: [peer: "p-preferred"], + directedPeerHint: peer, + packetType: MessageType.noiseEncrypted.rawValue, + messageID: "message-1" + ) + + #expect(selection.peripheralIDs == Set(["p-preferred"])) + #expect(selection.centralIDs.isEmpty) + } + + @Test + func uncollapsedSelectionReachesEveryLinkOfADuplicatelyLinkedPeer() { + // Announce fanout (collapse bypassed by the planner): the announce is + // the packet that binds links, so every live link must receive it. + let peer = PeerID(str: "1122334455667788") + let selection = BLEFanoutSelector.selectLinks( + peripheralIDs: ["p1", "p2"], + centralIDs: ["c1"], + ingressLink: nil, + peripheralPeerBindings: ["p1": peer, "p2": peer], + centralPeerBindings: ["c1": peer], + preferredPeripheralPerPeer: [peer: "p1"], + collapseDuplicatePeerLinks: false, + directedPeerHint: nil, + packetType: MessageType.announce.rawValue, + messageID: "message-1" + ) + + #expect(selection.peripheralIDs == Set(["p1", "p2"])) + #expect(selection.centralIDs == Set(["c1"])) + } + @Test func broadcastWithTwoLinksKeepsBothAfterIngressExclusion() { let selection = BLEFanoutSelector.selectLinks( diff --git a/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift b/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift index 91b54399..c57c8922 100644 --- a/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift +++ b/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift @@ -62,6 +62,114 @@ struct BLEOutboundFragmentTransferSchedulerTests { } } + @Test + func resendWithoutTransferIdOfActiveBroadcastContentIsDropped() { + // Field bug: a gossip-sync replay re-fragmented a 41KB voice file + // that was still being broadcast, sending two complete fragment + // streams. The resend path has no explicit transferId; drop it while + // a covering transfer of the same bytes is in flight. + var scheduler = BLEOutboundFragmentTransferScheduler() + let original = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "app-id", payload: "voice-file") + let resend = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: nil, payload: "voice-file") + + _ = scheduler.submit(original, maxConcurrentTransfers: 2) + let result = scheduler.submit(resend, maxConcurrentTransfers: 2) + + if case let .droppedDuplicate(_, activeTransferId) = result { + #expect(activeTransferId == "app-id") + #expect(scheduler.activeCount == 1) + #expect(scheduler.pendingCount == 0) + } else { + Issue.record("Expected the transferId-less resend of in-flight broadcast content to be dropped") + } + } + + @Test + func directedResendToAnUncoveredAudienceStillRuns() { + // The in-flight copy is directed to one peer; a resend of the same + // bytes to a different peer is not redundant. + var scheduler = BLEOutboundFragmentTransferScheduler() + let toFirstPeer = makeRequest( + type: MessageType.fileTransfer.rawValue, + transferId: "app-id", + payload: "shared-file", + directedPeer: PeerID(str: "1122334455667788") + ) + let toSecondPeer = makeRequest( + type: MessageType.fileTransfer.rawValue, + transferId: nil, + payload: "shared-file", + directedPeer: PeerID(str: "8877665544332211") + ) + + _ = scheduler.submit(toFirstPeer, maxConcurrentTransfers: 2) + let result = scheduler.submit(toSecondPeer, maxConcurrentTransfers: 2) + + if case .start = result { + #expect(scheduler.activeCount == 2) + } else { + Issue.record("Expected a resend directed at an uncovered peer to start") + } + } + + @Test + func explicitTransferIdSendIsNeverDroppedAsDuplicate() { + // App-initiated sends carry a transferId the progress UI tracks; + // only transferId-less resend paths are deduplicated. + var scheduler = BLEOutboundFragmentTransferScheduler() + let first = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "send-1", payload: "same-bytes") + let second = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "send-2", payload: "same-bytes") + + _ = scheduler.submit(first, maxConcurrentTransfers: 2) + let result = scheduler.submit(second, maxConcurrentTransfers: 2) + + if case let .start(_, reservedTransferId?) = result { + #expect(reservedTransferId == "send-2") + } else { + Issue.record("Expected an explicit-transferId send to run despite identical content") + } + } + + @Test + func duplicateOfPendingContentIsDroppedAtSubmit() { + // A duplicate must not queue behind a pending copy of the same + // content and resend the whole file when the slot frees. + var scheduler = BLEOutboundFragmentTransferScheduler() + let active = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "active", payload: "file-a") + let queuedContent = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "waiting", payload: "file-b") + let queuedDuplicate = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: nil, payload: "file-b") + + _ = scheduler.submit(active, maxConcurrentTransfers: 1) + _ = scheduler.submit(queuedContent, maxConcurrentTransfers: 1) + + if case .droppedDuplicate = scheduler.submit(queuedDuplicate, maxConcurrentTransfers: 1) { + // Dropped immediately: the pending "waiting" transfer covers it. + } else { + Issue.record("Expected the duplicate of pending content to be dropped at submit") + } + #expect(scheduler.pendingCount == 1) + } + + @Test + func resendAfterCompletionIsAllowed() { + // Duplicate suppression only covers in-flight transfers: a peer that + // requests the file after the stream completed must get a resend. + var scheduler = BLEOutboundFragmentTransferScheduler() + let original = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "app-id", payload: "voice-file") + let resend = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: nil, payload: "voice-file") + + _ = scheduler.submit(original, maxConcurrentTransfers: 1) + let didActivate = scheduler.activateReservedTransfer(id: "app-id", totalFragments: 1, workItems: []) + #expect(didActivate) + #expect(scheduler.markFragmentSent(transferId: "app-id") == .complete(sentFragments: 1, totalFragments: 1)) + + if case .start = scheduler.submit(resend, maxConcurrentTransfers: 1) { + #expect(scheduler.activeCount == 1) + } else { + Issue.record("Expected a resend after completion to start") + } + } + @Test func cancelActiveTransferReturnsScheduledWorkItems() { var scheduler = BLEOutboundFragmentTransferScheduler() @@ -128,20 +236,25 @@ struct BLEOutboundFragmentTransferSchedulerTests { #expect(scheduler.pendingCount == 0) } - private func makeRequest(type: UInt8, transferId: String?) -> BLEOutboundFragmentTransferRequest { + private func makeRequest( + type: UInt8, + transferId: String?, + payload: String? = nil, + directedPeer: PeerID? = nil + ) -> BLEOutboundFragmentTransferRequest { BLEOutboundFragmentTransferRequest( packet: BitchatPacket( type: type, senderID: Data([0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77]), recipientID: nil, timestamp: 0x0102030405, - payload: Data((transferId ?? "payload").utf8), + payload: Data((payload ?? transferId ?? "payload").utf8), signature: nil, ttl: 3 ), pad: false, maxChunk: nil, - directedPeer: nil, + directedPeer: directedPeer, transferId: transferId ) } diff --git a/bitchatTests/Services/BLEOutboundLinkPlannerTests.swift b/bitchatTests/Services/BLEOutboundLinkPlannerTests.swift index 9d10e218..74bcb9ba 100644 --- a/bitchatTests/Services/BLEOutboundLinkPlannerTests.swift +++ b/bitchatTests/Services/BLEOutboundLinkPlannerTests.swift @@ -113,6 +113,45 @@ struct BLEOutboundLinkPlannerTests { #expect(!plan.shouldSpoolDirectedPacket) } + @Test + func directAnnounceBypassesDuplicateLinkCollapseButRelayedAnnounceDoesNot() { + let peer = PeerID(str: "1122334455667788") + let bindings: [String: PeerID] = ["p1": peer, "p2": peer] + + let direct = BLEOutboundLinkPlanner.plan( + packet: makePacket(type: .announce), + dataCount: 32, + peripheralIDs: ["p1", "p2"], + peripheralWriteLimits: [128, 128], + centralIDs: [], + centralNotifyLimits: [], + ingressRecord: nil, + excludedLinks: [], + peripheralPeerBindings: bindings, + preferredPeripheralPerPeer: [peer: "p1"], + directedOnlyPeer: nil + ) + // A direct announce is the link-binding packet: it must reach every + // live link, including a peer's duplicate connections. + #expect(direct.selectedLinks.peripheralIDs == Set(["p1", "p2"])) + + let relayed = BLEOutboundLinkPlanner.plan( + packet: makePacket(type: .announce, ttl: TransportConfig.messageTTLDefault - 1), + dataCount: 32, + peripheralIDs: ["p1", "p2"], + peripheralWriteLimits: [128, 128], + centralIDs: [], + centralNotifyLimits: [], + ingressRecord: nil, + excludedLinks: [], + peripheralPeerBindings: bindings, + preferredPeripheralPerPeer: [peer: "p1"], + directedOnlyPeer: nil + ) + // Relayed announces keep the per-peer collapse (relay hygiene). + #expect(relayed.selectedLinks.peripheralIDs == Set(["p1"])) + } + @Test func minimumLinkLimitUsesTheSmallestPresentRoleLimit() { #expect(BLEOutboundLinkPlanner.minimumLinkLimit(peripheralWriteLimits: [80, 120], centralNotifyLimits: []) == 80) @@ -123,7 +162,8 @@ struct BLEOutboundLinkPlannerTests { private func makePacket( type: MessageType, sender: PeerID = PeerID(str: "8877665544332211"), - recipient: PeerID? = nil + recipient: PeerID? = nil, + ttl: UInt8 = TransportConfig.messageTTLDefault ) -> BitchatPacket { BitchatPacket( type: type.rawValue, @@ -132,7 +172,7 @@ struct BLEOutboundLinkPlannerTests { timestamp: 1234, payload: Data([0x01, 0x02]), signature: nil, - ttl: TransportConfig.messageTTLDefault + ttl: ttl ) } } diff --git a/bitchatTests/Services/BLERedundantLinkPolicyTests.swift b/bitchatTests/Services/BLERedundantLinkPolicyTests.swift new file mode 100644 index 00000000..db23a5e8 --- /dev/null +++ b/bitchatTests/Services/BLERedundantLinkPolicyTests.swift @@ -0,0 +1,134 @@ +import BitFoundation +import Foundation +import Testing +@testable import bitchat + +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) + } + + @Test + func singleBoundLinkNeedsNoConsolidation() { + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p1", + mostRecentlyBoundUUID: "p1", + links: [link("p1", peer), link("p2", otherPeer)], + peerID: peer + ) + #expect(kept == nil) + } + + @Test + func ingressLinkOfVerifiedAnnounceWinsOverReverseMap() { + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-ingress", + mostRecentlyBoundUUID: "p-reverse", + links: [link("p-ingress", peer), link("p-reverse", peer), link("p-stale", peer)], + peerID: peer + ) + #expect(kept == "p-ingress") + } + + @Test + func centralIngressFallsBackToMostRecentlyBoundLink() { + // The announce arrived on the central link (a write), so no ingress + // peripheral exists; the peer's reverse-mapped link survives. + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: nil, + mostRecentlyBoundUUID: "p-reverse", + links: [link("p-reverse", peer), link("p-stale", peer)], + peerID: peer + ) + #expect(kept == "p-reverse") + } + + @Test + func noLiveCandidateAmongBoundLinksRetiresNothing() { + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: nil, + mostRecentlyBoundUUID: "p-disconnected", + links: [link("p-disconnected", peer, connected: false), link("p1", peer), link("p2", peer)], + peerID: peer + ) + #expect(kept == nil) + } + + @Test + func characteristicLessAnchorLosesToWritableDuplicate() { + // The ingress link is mid-service-rediscovery (no characteristic): + // keeping it and cancelling the writable duplicate would strand + // outbound traffic on the central link, so the writable + // reverse-mapped link wins. + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-charless", + mostRecentlyBoundUUID: "p-writable", + links: [link("p-charless", peer, writable: false), link("p-writable", peer)], + peerID: peer + ) + #expect(kept == "p-writable") + } + + @Test + func writableDuplicateThatIsNoAnchorDefersConsolidation() { + // Both anchors are characteristic-less but a writable third link + // exists: never keep a charless link over it — wait for a later + // announce instead of guessing which link to keep. + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-charless-1", + mostRecentlyBoundUUID: "p-charless-2", + links: [ + link("p-charless-1", peer, writable: false), + link("p-charless-2", peer, writable: false), + link("p-writable", peer) + ], + peerID: peer + ) + #expect(kept == nil) + } + + @Test + func allCharacteristicLessDuplicatesStillConsolidateOnIngress() { + // No writable link exists at all (all mid-rediscovery): the ingress + // anchor still consolidates — no writable duplicate is at risk. + let kept = BLERedundantLinkPolicy.keptPeripheralUUID( + ingressPeripheralUUID: "p-ingress", + mostRecentlyBoundUUID: "p-stale", + links: [link("p-ingress", peer, writable: false), link("p-stale", peer, writable: false)], + peerID: peer + ) + #expect(kept == "p-ingress") + } + + @Test + func retirementSparesKeptLinkUnboundLinksAndOtherPeers() { + let retiring = BLERedundantLinkPolicy.peripheralUUIDsToRetire( + links: [ + link("p-kept", peer), + link("p-dup-1", peer), + link("p-dup-2", peer), + link("p-gone", peer, connected: false), + link("p-unbound", nil), + link("p-other", otherPeer) + ], + peerID: peer, + keeping: "p-kept" + ) + #expect(Set(retiring) == Set(["p-dup-1", "p-dup-2"])) + } + + @Test + func rotationCleanupWithNoSurvivorRetiresEveryBoundLink() { + // Rotated-away identity: the rebound link now belongs to the new ID, + // so every link still bound to the old ID is a stale duplicate. + let retiring = BLERedundantLinkPolicy.peripheralUUIDsToRetire( + links: [link("p-stale-1", peer), link("p-stale-2", peer)], + peerID: peer, + keeping: "" + ) + #expect(Set(retiring) == Set(["p-stale-1", "p-stale-2"])) + } +}