Surface router drops as failed status; bound relay pending subscriptions; jitter backoff

MessageRouter outbox drops (attempt cap, TTL expiry in flush and
cleanup, per-peer overflow eviction) now invoke onMessageDropped, wired
to mark the message .failed in the ConversationStore - guarded so a
late failure never downgrades an already delivered/read status.

NostrRelayManager pending subscriptions gain a per-relay cap (64,
oldest-by-sequence eviction; durable intent still replays from
subscriptionRequestState) and a 10-minute age sweep on the existing
connect path. Reconnect backoff gets injectable +/-20% jitter so
recovering relays don't thundering-herd.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

bitchat/Nostr/NostrRelayManager.swift

@@ -66,6 +66,9 @@ struct NostrRelayManagerDependencies {
     var makeSession: () -> NostrRelaySessionProtocol
     var scheduleAfter: @Sendable (TimeInterval, @escaping @Sendable () -> Void) -> Void
     var now: () -> Date
+    /// Uniform random value in [0, 1) used to jitter reconnect backoff.
+    /// Injectable so tests can pin or sweep the jitter deterministically.
+    var jitterUnit: () -> Double
 }
 
 private extension NostrRelayManagerDependencies {
@@ -93,7 +96,8 @@ private extension NostrRelayManagerDependencies {
             scheduleAfter: { delay, action in
                 DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: action)
             },
-            now: Date.init
+            now: Date.init,
+            jitterUnit: { Double.random(in: 0..<1) }
         )
     }
 }
@@ -140,7 +144,16 @@ final class NostrRelayManager: ObservableObject {
     private var hasLocationPermission: Bool = false
     private var connections: [String: NostrRelayConnectionProtocol] = [:]
     private var subscriptions: [String: Set<String>] = [:] // relay URL -> active subscription IDs
-    private var pendingSubscriptions: [String: [String: String]] = [:] // relay URL -> (subscription id -> encoded REQ JSON)
+    // Not-yet-flushed REQs per relay, bounded by a per-relay cap (oldest by
+    // insertion order evicted) and an age sweep on connect attempts. Dicts are
+    // unordered, so each entry carries an insertion sequence and queue time.
+    private struct PendingSubscription {
+        let messageString: String // encoded REQ JSON
+        let queuedAt: Date
+        let sequence: UInt64
+    }
+    private var pendingSubscriptions: [String: [String: PendingSubscription]] = [:] // relay URL -> (subscription id -> pending REQ)
+    private var pendingSubscriptionSequence: UInt64 = 0
     private var messageHandlers: [String: (NostrEvent) -> Void] = [:]
     private struct InboundEventKey: Hashable {
         let subscriptionID: String
@@ -432,9 +445,7 @@ final class NostrRelayManager: ObservableObject {
                 existingSet.insert(url)
             }
             for url in urls {
-                var map = self.pendingSubscriptions[url] ?? [:]
-                map[id] = messageString
-                self.pendingSubscriptions[url] = map
+                queuePendingSubscription(id: id, messageString: messageString, for: url)
             }
             // Initialize EOSE tracking if requested
             if let onEOSE = onEOSE {
@@ -550,6 +561,7 @@ final class NostrRelayManager: ObservableObject {
 
     private func connectToRelays(_ relayUrls: [String], shouldLog: Bool = false) {
         guard dependencies.activationAllowed() else { return }
+        sweepStalePendingSubscriptions()
         let targets = allowedRelayList(from: relayUrls).filter {
             connections[$0] == nil && !isPermanentlyFailed($0)
         }
@@ -627,11 +639,53 @@ final class NostrRelayManager: ObservableObject {
     private func subscriptionStateExists(id: String, requestState: SubscriptionRequestState) -> Bool {
         guard !requestState.relayURLs.isEmpty else { return true }
         return requestState.relayURLs.allSatisfy { url in
-            pendingSubscriptions[url]?[id] == requestState.messageString ||
+            pendingSubscriptions[url]?[id]?.messageString == requestState.messageString ||
                 subscriptions[url]?.contains(id) == true
         }
     }
 
+    private func queuePendingSubscription(id: String, messageString: String, for url: String) {
+        var map = pendingSubscriptions[url] ?? [:]
+        pendingSubscriptionSequence &+= 1
+        map[id] = PendingSubscription(
+            messageString: messageString,
+            queuedAt: dependencies.now(),
+            sequence: pendingSubscriptionSequence
+        )
+        // Bound per-relay pending REQs; evict oldest by insertion order. The
+        // durable intent stays in subscriptionRequestState, so an evicted REQ
+        // is still replayed if its subscription is active when the relay
+        // (re)connects.
+        while map.count > TransportConfig.nostrPendingSubscriptionsPerRelayCap,
+              let oldest = map.min(by: { $0.value.sequence < $1.value.sequence }) {
+            map.removeValue(forKey: oldest.key)
+            SecureLogger.warning(
+                "📋 Pending subscription overflow for \(url) - evicted oldest id=\(oldest.key)",
+                category: .session
+            )
+        }
+        pendingSubscriptions[url] = map
+    }
+
+    /// Drop pending REQs older than the TTL. Runs on connect attempts (the
+    /// natural maintenance path: connect/ensureConnections/reconnects all
+    /// funnel through connectToRelays) so stale entries for relays that never
+    /// come up cannot accumulate without bound.
+    private func sweepStalePendingSubscriptions() {
+        let now = dependencies.now()
+        for (url, map) in pendingSubscriptions {
+            let fresh = map.filter {
+                now.timeIntervalSince($0.value.queuedAt) <= TransportConfig.nostrPendingSubscriptionTTLSeconds
+            }
+            guard fresh.count != map.count else { continue }
+            SecureLogger.debug(
+                "📋 Swept \(map.count - fresh.count) stale pending subscription(s) for \(url)",
+                category: .session
+            )
+            pendingSubscriptions[url] = fresh.isEmpty ? nil : fresh
+        }
+    }
+
     private func startEOSETracking(id: String, relayURLs: Set<String>, callback: @escaping () -> Void) {
         eoseTrackerEpoch += 1
         let epoch = eoseTrackerEpoch
@@ -770,7 +824,7 @@ final class NostrRelayManager: ObservableObject {
     /// active subscription targeting this relay must be re-sent.
     private func flushPendingSubscriptions(for relayUrl: String) {
         guard let connection = connections[relayUrl] else { return }
-        var toSend = pendingSubscriptions[relayUrl] ?? [:]
+        var toSend = (pendingSubscriptions[relayUrl] ?? [:]).mapValues(\.messageString)
         for (id, state) in subscriptionRequestState where state.relayURLs.contains(relayUrl) && toSend[id] == nil {
             toSend[id] = state.messageString
         }
@@ -987,12 +1041,16 @@ final class NostrRelayManager: ObservableObject {
             return
         }
         
-        // Calculate backoff interval
-        let backoffInterval = min(
+        // Calculate backoff interval with ±jitterRatio random jitter so relays
+        // that dropped together don't all reconnect at the same instant.
+        let baseBackoffInterval = min(
             initialBackoffInterval * pow(backoffMultiplier, Double(relays[index].reconnectAttempts - 1)),
             maxBackoffInterval
         )
-        
+        let jitterRatio = TransportConfig.nostrRelayBackoffJitterRatio
+        let jitterFactor = 1.0 + (dependencies.jitterUnit() * 2.0 - 1.0) * jitterRatio
+        let backoffInterval = baseBackoffInterval * jitterFactor
+
         let nextReconnectTime = dependencies.now().addingTimeInterval(backoffInterval)
         relays[index].nextReconnectTime = nextReconnectTime
         
@@ -1054,6 +1112,11 @@ final class NostrRelayManager: ObservableObject {
         pendingSubscriptions[relayUrl]?.count ?? 0
     }
 
+    func debugPendingSubscriptionIDs(for relayUrl: String) -> Set<String> {
+        guard let map = pendingSubscriptions[relayUrl] else { return [] }
+        return Set(map.keys)
+    }
+
     var debugDuplicateInboundEventDropCount: Int {
         duplicateInboundEventDropCount
     }

bitchat/Services/MessageRouter.swift

@@ -6,6 +6,13 @@ import Foundation
 @MainActor
 final class MessageRouter {
     private let transports: [Transport]
+    private let now: () -> Date
+
+    /// Invoked whenever a retained private message is dropped without a
+    /// delivery ack (attempt cap, TTL expiry, or per-peer overflow eviction)
+    /// so the UI can surface the failure instead of leaving the message in a
+    /// stale "sending/sent" state forever.
+    var onMessageDropped: ((_ messageID: String, _ peerID: PeerID) -> Void)?
 
     // Outbox entry with timestamp for TTL-based eviction
     private struct QueuedMessage {
@@ -25,8 +32,9 @@ final class MessageRouter {
     // get a delivery ack (e.g. peer on an old client that doesn't ack).
     private static let maxSendAttempts = 8
 
-    init(transports: [Transport]) {
+    init(transports: [Transport], now: @escaping () -> Date = Date.init) {
         self.transports = transports
+        self.now = now
 
         // Observe favorites changes to learn Nostr mapping and flush queued messages
         NotificationCenter.default.addObserver(
@@ -72,7 +80,7 @@ final class MessageRouter {
             return
         }
 
-        let message = QueuedMessage(content: content, nickname: recipientNickname, messageID: messageID, timestamp: Date(), sendAttempts: 1)
+        let message = QueuedMessage(content: content, nickname: recipientNickname, messageID: messageID, timestamp: now(), sendAttempts: 1)
         if let transport = reachableTransport(for: peerID) {
             // Reachability without a connection is a freshness heuristic (e.g.
             // the mesh retention window), so the send can silently go nowhere.
@@ -108,6 +116,7 @@ final class MessageRouter {
         if queue.count > Self.maxMessagesPerPeer {
             let evicted = queue.removeFirst()
             SecureLogger.warning("📤 Outbox overflow for \(peerID.id.prefix(8))… - evicted oldest message: \(evicted.messageID.prefix(8))…", category: .session)
+            onMessageDropped?(evicted.messageID, peerID)
         }
         outbox[peerID] = queue
     }
@@ -142,13 +151,14 @@ final class MessageRouter {
         guard let queued = outbox[peerID], !queued.isEmpty else { return }
         SecureLogger.debug("Flushing outbox for \(peerID.id.prefix(8))… count=\(queued.count)", category: .session)
 
-        let now = Date()
+        let now = now()
         var remaining: [QueuedMessage] = []
 
         for message in queued {
             // Skip expired messages (TTL exceeded)
             if now.timeIntervalSince(message.timestamp) > Self.messageTTLSeconds {
                 SecureLogger.debug("⏰ Expired queued message for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))… (age: \(Int(now.timeIntervalSince(message.timestamp)))s)", category: .session)
+                onMessageDropped?(message.messageID, peerID)
                 continue
             }
 
@@ -161,6 +171,7 @@ final class MessageRouter {
                 // bounded by attempt count for peers that never ack.
                 guard message.sendAttempts < Self.maxSendAttempts else {
                     SecureLogger.warning("📤 Dropping unacked PM for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))… after \(message.sendAttempts) attempts", category: .session)
+                    onMessageDropped?(message.messageID, peerID)
                     continue
                 }
                 SecureLogger.debug("Outbox -> \(type(of: transport)) (reachable) for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))…", category: .session)
@@ -186,12 +197,21 @@ final class MessageRouter {
 
     /// Periodically clean up expired messages from all outboxes
     func cleanupExpiredMessages() {
-        let now = Date()
+        let now = now()
         for peerID in Array(outbox.keys) {
-            outbox[peerID]?.removeAll { now.timeIntervalSince($0.timestamp) > Self.messageTTLSeconds }
+            var expiredMessageIDs: [String] = []
+            outbox[peerID]?.removeAll { message in
+                guard now.timeIntervalSince(message.timestamp) > Self.messageTTLSeconds else { return false }
+                expiredMessageIDs.append(message.messageID)
+                return true
+            }
             if outbox[peerID]?.isEmpty == true {
                 outbox.removeValue(forKey: peerID)
             }
+            for messageID in expiredMessageIDs {
+                SecureLogger.debug("⏰ Expired queued message for \(peerID.id.prefix(8))… id=\(messageID.prefix(8))…", category: .session)
+                onMessageDropped?(messageID, peerID)
+            }
         }
     }
 }

bitchat/Services/TransportConfig.swift

@@ -148,11 +148,19 @@ enum TransportConfig {
     static let nostrRelayMaxBackoffSeconds: TimeInterval = 300.0
     static let nostrRelayBackoffMultiplier: Double = 2.0
     static let nostrRelayMaxReconnectAttempts: Int = 10
+    // Reconnect delays get ±20% random jitter so relays that dropped together
+    // (e.g. a network blip) don't thundering-herd the same reconnect instant.
+    static let nostrRelayBackoffJitterRatio: Double = 0.2
     static let nostrRelayDefaultFetchLimit: Int = 100
     // How many consecutive Tor-readiness waits (each bounded by TorManager's
     // bootstrap deadline) to attempt before unblocking pending EOSE callers.
     static let nostrTorReadyMaxWaitAttempts: Int = 3
     static let nostrPendingSendQueueCap: Int = 200
+    // Pending (not-yet-flushed) REQs are bounded per relay: oldest-by-insertion
+    // eviction at the cap, plus an age sweep on connect attempts. Durable
+    // subscription intent survives in subscriptionRequestState either way.
+    static let nostrPendingSubscriptionsPerRelayCap: Int = 64
+    static let nostrPendingSubscriptionTTLSeconds: TimeInterval = 600.0
     // Fallback deadline for treating a subscription's initial fetch as complete
     // when a relay never sends EOSE (generous to cover Tor circuit setup).
     static let nostrSubscriptionEOSEFallbackSeconds: TimeInterval = 10.0

bitchat/ViewModels/ChatViewModelBootstrapper.swift

@@ -78,6 +78,23 @@ private extension ChatViewModelBootstrapper {
         viewModel.privateChatManager.messageRouter = viewModel.messageRouter
         viewModel.privateChatManager.unifiedPeerService = viewModel.unifiedPeerService
         viewModel.unifiedPeerService.messageRouter = viewModel.messageRouter
+        // Surface silent outbox drops (attempt cap, TTL expiry, overflow
+        // eviction) as a visible failure. The store's no-downgrade rule does
+        // not cover `.failed` over confirmed receipts, so guard here: a drop
+        // of an already-delivered/read message (e.g. a stale retained copy)
+        // must not downgrade its status.
+        viewModel.messageRouter.onMessageDropped = { [weak viewModel] messageID, _ in
+            guard let viewModel else { return }
+            switch viewModel.conversations.deliveryStatus(forMessageID: messageID) {
+            case .delivered, .read:
+                return
+            default:
+                viewModel.conversations.setDeliveryStatus(
+                    .failed(reason: "Not delivered"),
+                    forMessageID: messageID
+                )
+            }
+        }
         viewModel.commandProcessor.contextProvider = viewModel
         viewModel.commandProcessor.meshService = viewModel.meshService
         viewModel.participantTracker.configure(context: viewModel)