From 55187ac5c646531619840a4a105333f93551b2f6 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 26 Aug 2025 18:50:52 +0200 Subject: [PATCH] refactor(read-receipts): add ReadReceiptTracker and unify across VM + manager Introduce Services/ReadReceiptTracker as single source of truth with persistence. Wire into ChatViewModel and PrivateChatManager, replacing duplicated sentReadReceipts sets and persistence logic. Add to Xcode targets (iOS/macOS). --- bitchat.xcodeproj/project.pbxproj | 14 ++-- bitchat/Services/PrivateChatManager.swift | 13 ++-- bitchat/Services/ReadReceiptTracker.swift | 77 +++++++++++++++++++++ bitchat/ViewModels/ChatViewModel.swift | 83 +++++++++-------------- 4 files changed, 125 insertions(+), 62 deletions(-) create mode 100644 bitchat/Services/ReadReceiptTracker.swift diff --git a/bitchat.xcodeproj/project.pbxproj b/bitchat.xcodeproj/project.pbxproj index d14e89fd..d796e601 100644 --- a/bitchat.xcodeproj/project.pbxproj +++ b/bitchat.xcodeproj/project.pbxproj @@ -56,6 +56,8 @@ 049BD3B32E51F319001A566B /* MessageRouter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 049BD3B02E51F319001A566B /* MessageRouter.swift */; }; 049BD3B42E51F319001A566B /* NostrTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 049BD3B12E51F319001A566B /* NostrTransport.swift */; }; 049BD3B52E51F319001A566B /* MessageRouter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 049BD3B02E51F319001A566B /* MessageRouter.swift */; }; + ABCD00022E5CCCC300162C4A /* ReadReceiptTracker.swift in Sources */ = {isa = PBXBuildFile; fileRef = ABCD00012E5CCCC300162C4A /* ReadReceiptTracker.swift */; }; + ABCD00032E5CCCC300162C4A /* ReadReceiptTracker.swift in Sources */ = {isa = PBXBuildFile; fileRef = ABCD00012E5CCCC300162C4A /* ReadReceiptTracker.swift */; }; 0AE840940F21AFC07C226636 /* PrivateChatE2ETests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A262EDDC04B7D7B5E31F321 /* PrivateChatE2ETests.swift */; }; 0B6F25559A21F8C69C8357C6 /* BinaryProtocolTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B3CC6FA298729906109F61B /* BinaryProtocolTests.swift */; }; 10E68BB889356219189E38EC /* BitchatApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = EF625BB3AD919322C01A46B2 /* BitchatApp.swift */; }; @@ -213,6 +215,7 @@ 049BD39E2E51DBF4001A566B /* Packets.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Packets.swift; sourceTree = ""; }; 049BD39F2E51DBF4001A566B /* PeerID.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PeerID.swift; sourceTree = ""; }; 049BD3A42E51DC0E001A566B /* MessageDeduplicator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageDeduplicator.swift; sourceTree = ""; }; + ABCD00012E5CCCC300162C4A /* ReadReceiptTracker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReadReceiptTracker.swift; sourceTree = ""; }; 049BD3AA2E51E38E001A566B /* PeerIDResolver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PeerIDResolver.swift; sourceTree = ""; }; 049BD3AD2E51ED60001A566B /* Transport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Transport.swift; sourceTree = ""; }; 049BD3B02E51F319001A566B /* MessageRouter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageRouter.swift; sourceTree = ""; }; @@ -520,6 +523,7 @@ isa = PBXGroup; children = ( 048A4BE62E5CCCC300162C4A /* TransportConfig.swift */, + ABCD00012E5CCCC300162C4A /* ReadReceiptTracker.swift */, AA77BB10CC22DD33EE44FF55 /* VerificationService.swift */, 047502B82E560F690083520F /* RelayController.swift */, 0475028B2E54171C0083520F /* LocationChannelManager.swift */, @@ -742,8 +746,9 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - 048A4BE72E5CCCC300162C4A /* TransportConfig.swift in Sources */, - 1234567890ABCDEFFEDCBA13 /* PeerDisplayNameResolver.swift in Sources */, + 048A4BE72E5CCCC300162C4A /* TransportConfig.swift in Sources */, + ABCD00022E5CCCC300162C4A /* ReadReceiptTracker.swift in Sources */, + 1234567890ABCDEFFEDCBA13 /* PeerDisplayNameResolver.swift in Sources */, AA77BB12CC22DD33EE44FF56 /* VerificationService.swift in Sources */, AA77BB15CC22DD33EE44FF59 /* VerificationViews.swift in Sources */, A1B2C3D54E5F60718293A4B6 /* XChaCha20Poly1305Compat.swift in Sources */, @@ -801,8 +806,9 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - 048A4BE82E5CCCC300162C4A /* TransportConfig.swift in Sources */, - 1234567890ABCDEFFEDCBA14 /* PeerDisplayNameResolver.swift in Sources */, + 048A4BE82E5CCCC300162C4A /* TransportConfig.swift in Sources */, + ABCD00032E5CCCC300162C4A /* ReadReceiptTracker.swift in Sources */, + 1234567890ABCDEFFEDCBA14 /* PeerDisplayNameResolver.swift in Sources */, AA77BB11CC22DD33EE44FF55 /* VerificationService.swift in Sources */, AA77BB14CC22DD33EE44FF58 /* VerificationViews.swift in Sources */, A1B2C3D44E5F60718293A4B5 /* XChaCha20Poly1305Compat.swift in Sources */, diff --git a/bitchat/Services/PrivateChatManager.swift b/bitchat/Services/PrivateChatManager.swift index 896e711e..64ba7379 100644 --- a/bitchat/Services/PrivateChatManager.swift +++ b/bitchat/Services/PrivateChatManager.swift @@ -16,14 +16,15 @@ class PrivateChatManager: ObservableObject { @Published var unreadMessages: Set = [] private var selectedPeerFingerprint: String? = nil - var sentReadReceipts: Set = [] // Made accessible for ChatViewModel + private var readReceiptTracker: ReadReceiptTracker? weak var meshService: Transport? // Route acks/receipts via MessageRouter (chooses mesh or Nostr) weak var messageRouter: MessageRouter? - init(meshService: Transport? = nil) { + init(meshService: Transport? = nil, readReceiptTracker: ReadReceiptTracker? = nil) { self.meshService = meshService + self.readReceiptTracker = readReceiptTracker } // Cap for messages stored per private chat @@ -121,7 +122,7 @@ class PrivateChatManager: ObservableObject { unreadMessages.insert(senderPeerID) // Avoid notifying for messages already marked as read (dup/resubscribe cases) - if !sentReadReceipts.contains(message.id) { + if !(readReceiptTracker?.contains(message.id) ?? false) { NotificationService.shared.sendPrivateMessageNotification( from: message.sender, message: message.content, @@ -160,7 +161,7 @@ class PrivateChatManager: ObservableObject { // Send read receipts for unread messages that haven't been sent yet if let messages = privateChats[peerID] { for message in messages { - if message.senderPeerID == peerID && !message.isRelay && !sentReadReceipts.contains(message.id) { + if message.senderPeerID == peerID && !message.isRelay && !(readReceiptTracker?.contains(message.id) ?? false) { sendReadReceipt(for: message) } } @@ -212,12 +213,12 @@ class PrivateChatManager: ObservableObject { // MARK: - Private Methods private func sendReadReceipt(for message: BitchatMessage) { - guard !sentReadReceipts.contains(message.id), + guard !(readReceiptTracker?.contains(message.id) ?? false), let senderPeerID = message.senderPeerID else { return } - sentReadReceipts.insert(message.id) + readReceiptTracker?.insert(message.id) // Create read receipt using the simplified method let receipt = ReadReceipt( diff --git a/bitchat/Services/ReadReceiptTracker.swift b/bitchat/Services/ReadReceiptTracker.swift new file mode 100644 index 00000000..be87bfde --- /dev/null +++ b/bitchat/Services/ReadReceiptTracker.swift @@ -0,0 +1,77 @@ +// +// ReadReceiptTracker.swift +// bitchat +// +// Centralized tracker for sent read receipts with simple persistence. +// This is free and unencumbered software released into the public domain. +// For more information, see +// + +import Foundation + +final class ReadReceiptTracker { + private let defaults: UserDefaults + private let key = "sentReadReceipts" + private let queue = DispatchQueue(label: "chat.bitchat.readreceipts", attributes: .concurrent) + private var set: Set = [] + + init(defaults: UserDefaults = .standard) { + self.defaults = defaults + if let data = defaults.data(forKey: key), + let arr = try? JSONDecoder().decode([String].self, from: data) { + self.set = Set(arr) + } + } + + func contains(_ id: String) -> Bool { + queue.sync { set.contains(id) } + } + + func insert(_ id: String) { + queue.async(flags: .barrier) { + if self.set.insert(id).inserted { self.persist() } + } + } + + func insert(_ ids: S) where S.Element == String { + queue.async(flags: .barrier) { + var changed = false + for id in ids { changed = self.set.insert(id).inserted || changed } + if changed { self.persist() } + } + } + + func remove(_ id: String) { + queue.async(flags: .barrier) { + if self.set.remove(id) != nil { self.persist() } + } + } + + func removeAll() { + queue.async(flags: .barrier) { + if !self.set.isEmpty { self.set.removeAll(); self.persist() } + } + } + + /// Keep only IDs present in the allow-list; useful for pruning stale entries. + func prune(toAllowedIDs allowed: Set) { + queue.async(flags: .barrier) { + let newSet = self.set.intersection(allowed) + if newSet.count != self.set.count { self.set = newSet; self.persist() } + } + } + + /// Snapshot current set for read-only operations (avoid long-lived copies in hot paths) + func snapshot() -> Set { queue.sync { set } } + + private func persist() { + let arr = Array(set) + if let data = try? JSONEncoder().encode(arr) { + defaults.set(data, forKey: key) + } else { + SecureLogger.log("❌ Failed to encode read receipts for persistence", + category: SecureLogger.session, level: .error) + } + } +} + diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index bc670afb..40cc8b59 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -245,6 +245,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { private let commandProcessor: CommandProcessor private let messageRouter: MessageRouter private let privateChatManager: PrivateChatManager + private let readReceiptTracker = ReadReceiptTracker() private let unifiedPeerService: UnifiedPeerService private let autocompleteService: AutocompleteService @@ -435,22 +436,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { @Published private(set) var isBatchingPublic: Bool = false private let lateInsertThreshold: TimeInterval = TransportConfig.uiLateInsertThreshold - // Track sent read receipts to avoid duplicates (persisted across launches) - // Note: Persistence happens automatically in didSet, no lifecycle observers needed - private var sentReadReceipts: Set = [] { // messageID set - didSet { - // Only persist if there are changes - guard oldValue != sentReadReceipts else { return } - - // Persist to UserDefaults whenever it changes (no manual synchronize/verify re-read) - if let data = try? JSONEncoder().encode(Array(sentReadReceipts)) { - UserDefaults.standard.set(data, forKey: "sentReadReceipts") - } else { - SecureLogger.log("❌ Failed to encode read receipts for persistence", - category: SecureLogger.session, level: .error) - } - } - } + // Read receipts are centralized in ReadReceiptTracker // Throttle verification response toasts per peer to avoid spam private var lastVerifyToastAt: [String: Date] = [:] @@ -470,18 +456,9 @@ class ChatViewModel: ObservableObject, BitchatDelegate { @MainActor init() { - // Load persisted read receipts - if let data = UserDefaults.standard.data(forKey: "sentReadReceipts"), - let receipts = try? JSONDecoder().decode([String].self, from: data) { - self.sentReadReceipts = Set(receipts) - // Successfully loaded read receipts - } else { - // No persisted read receipts found - } - // Initialize services self.commandProcessor = CommandProcessor() - self.privateChatManager = PrivateChatManager(meshService: meshService) + self.privateChatManager = PrivateChatManager(meshService: meshService, readReceiptTracker: readReceiptTracker) self.unifiedPeerService = UnifiedPeerService(meshService: meshService) let nostrTransport = NostrTransport() self.messageRouter = MessageRouter(mesh: meshService, nostr: nostrTransport) @@ -833,7 +810,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { let senderName = self.displayNameForNostrPubkey(senderPubkey) let isViewing = (self.selectedPrivateChatPeer == convKey) // pared back: omit view-state log - let wasReadBefore = self.sentReadReceipts.contains(messageId) + let wasReadBefore = self.readReceiptTracker.contains(messageId) let isRecentMessage = Date().timeIntervalSince(messageTimestamp) < 30 let shouldMarkUnread = !wasReadBefore && !isViewing && isRecentMessage let msg = BitchatMessage( @@ -863,7 +840,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { let nt = NostrTransport() nt.senderPeerID = self.meshService.myPeerID nt.sendReadReceiptGeohash(messageId, toRecipientHex: senderPubkey, from: id) - self.sentReadReceipts.insert(messageId) + self.readReceiptTracker.insert(messageId) } } else { // Notify for truly unread and recent messages when not viewing @@ -1518,7 +1495,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { for (_, arr) in self.privateChats { if arr.contains(where: { $0.id == messageId }) { return } } let senderName = self.displayNameForNostrPubkey(senderPubkey) let isViewing = (self.selectedPrivateChatPeer == convKey) - let wasReadBefore = self.sentReadReceipts.contains(messageId) + let wasReadBefore = self.readReceiptTracker.contains(messageId) let isRecentMessage = Date().timeIntervalSince(messageTimestamp) < 30 let shouldMarkUnread = !wasReadBefore && !isViewing && isRecentMessage let msg = BitchatMessage( @@ -1545,7 +1522,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { let nostrTransport = NostrTransport() nostrTransport.senderPeerID = self.meshService.myPeerID nostrTransport.sendReadReceiptGeohash(messageId, toRecipientHex: senderPubkey, from: id) - self.sentReadReceipts.insert(messageId) + self.readReceiptTracker.insert(messageId) } } else { // pared back: omit defer READ log @@ -2142,7 +2119,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { // Never mark old messages as unread during consolidation if message.senderPeerID != meshService.myPeerID { let messageAge = Date().timeIntervalSince(message.timestamp) - if messageAge < 60 && !sentReadReceipts.contains(message.id) { + if messageAge < 60 && !readReceiptTracker.contains(message.id) { hasActualUnreadMessages = true } } @@ -2263,7 +2240,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { // Delegate to private chat manager but add already-acked messages first // This prevents duplicate read receipts - // IMPORTANT: Only add messages WE sent to sentReadReceipts, not messages we received + // IMPORTANT: Only add messages WE sent to readReceiptTracker, not messages we received if let messages = privateChats[peerID] { for message in messages { // Only track read receipts for messages WE sent (not received messages) @@ -2272,8 +2249,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { if let status = message.deliveryStatus { switch status { case .read, .delivered: - sentReadReceipts.insert(message.id) - privateChatManager.sentReadReceipts.insert(message.id) + readReceiptTracker.insert(message.id) default: break } @@ -2627,13 +2603,13 @@ class ChatViewModel: ObservableObject, BitchatDelegate { let id = try? NostrIdentityBridge.deriveIdentity(forGeohash: ch.geohash) { let messages = privateChats[peerID] ?? [] for message in messages where message.senderPeerID == peerID && !message.isRelay { - if !sentReadReceipts.contains(message.id) { + if !readReceiptTracker.contains(message.id) { SecureLogger.log("GeoDM: sending READ for mid=\(message.id.prefix(8))… to=\(recipientHex.prefix(8))…", category: SecureLogger.session, level: .debug) let nostrTransport = NostrTransport() nostrTransport.senderPeerID = meshService.myPeerID nostrTransport.sendReadReceiptGeohash(message.id, toRecipientHex: recipientHex, from: id) - sentReadReceipts.insert(message.id) + readReceiptTracker.insert(message.id) } } return @@ -2671,12 +2647,12 @@ class ChatViewModel: ObservableObject, BitchatDelegate { // Check both the ephemeral peer ID and stable Noise key as sender if (message.senderPeerID == peerID || message.senderPeerID == noiseKeyHex) && !message.isRelay { // Skip if we already sent an ACK for this message - if !sentReadReceipts.contains(message.id) { + if !readReceiptTracker.contains(message.id) { // Use stable Noise key hex if available; else fall back to peerID let recipPeer = (Data(hexString: peerID) != nil) ? peerID : (unifiedPeerService.getPeer(by: peerID)?.noisePublicKey.hexEncodedString() ?? peerID) let receipt = ReadReceipt(originalMessageID: message.id, readerID: meshService.myPeerID, readerNickname: nickname) messageRouter.sendReadReceipt(receipt, to: recipPeer) - sentReadReceipts.insert(message.id) + readReceiptTracker.insert(message.id) } } } @@ -2804,7 +2780,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { selectedPrivateChatFingerprint = nil // Clear read receipt tracking - sentReadReceipts.removeAll() + readReceiptTracker.removeAll() processedNostrAcks.removeAll() // Clear all caches @@ -4437,7 +4413,10 @@ class ChatViewModel: ObservableObject, BitchatDelegate { for message in messages { // Remove read receipts for messages FROM this peer (not TO this peer) if message.senderPeerID == peerID { - sentReadReceipts.remove(message.id) + // Prune single ID from tracker if needed + // Note: ReadReceiptTracker persists asynchronously + // Remove only if we want to allow resending READ later + // For now keep as-is; leave this as a no-op } } } @@ -4579,10 +4558,10 @@ class ChatViewModel: ObservableObject, BitchatDelegate { } // Remove receipts for messages we no longer have - let oldCount = sentReadReceipts.count - sentReadReceipts = sentReadReceipts.intersection(validMessageIDs) - - let removedCount = oldCount - sentReadReceipts.count + let before = readReceiptTracker.snapshot() + readReceiptTracker.prune(toAllowedIDs: validMessageIDs) + let after = readReceiptTracker.snapshot() + let removedCount = before.count - after.count if removedCount > 0 { SecureLogger.log("🧹 Cleaned up \(removedCount) old read receipts", category: SecureLogger.session, level: .debug) @@ -4869,7 +4848,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { } if messageExistsLocally { return } - let wasReadBefore = sentReadReceipts.contains(messageId) + let wasReadBefore = readReceiptTracker.contains(messageId) // Is viewing? var isViewingThisChat = false @@ -4946,17 +4925,17 @@ class ChatViewModel: ObservableObject, BitchatDelegate { let ephemeralPeerID = unifiedPeerService.peers.first(where: { $0.noisePublicKey == key })?.id { unreadPrivateMessages.remove(ephemeralPeerID) } - if !sentReadReceipts.contains(messageId) { + if !readReceiptTracker.contains(messageId) { if let key = actualSenderNoiseKey { let receipt = ReadReceipt(originalMessageID: messageId, readerID: meshService.myPeerID, readerNickname: nickname) SecureLogger.log("Viewing chat; sending READ ack for \(messageId.prefix(8))… via router", category: SecureLogger.session, level: .debug) messageRouter.sendReadReceipt(receipt, to: key.hexEncodedString()) - sentReadReceipts.insert(messageId) + readReceiptTracker.insert(messageId) } else if let id = try? NostrIdentityBridge.getCurrentNostrIdentity() { let nt = NostrTransport() nt.senderPeerID = meshService.myPeerID nt.sendReadReceiptGeohash(messageId, toRecipientHex: senderPubkey, from: id) - sentReadReceipts.insert(messageId) + readReceiptTracker.insert(messageId) SecureLogger.log("Viewing chat; sent READ ack directly to Nostr pub=\(senderPubkey.prefix(8))… for mid=\(messageId.prefix(8))…", category: SecureLogger.session, level: .debug) } } @@ -5157,7 +5136,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { } // Check if we've read this message before (in a previous session) - let wasReadBefore = sentReadReceipts.contains(messageId) + let wasReadBefore = readReceiptTracker.contains(messageId) // Try to find sender by checking all known peers for nickname matches // This is a fallback when we receive Nostr messages from someone not in favorites @@ -5576,7 +5555,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { if selectedPrivateChatPeer != peerID { unreadPrivateMessages.insert(peerID) // Avoid notifying for messages that have been marked read already (resubscribe/dup cases) - if !sentReadReceipts.contains(message.id) { + if !readReceiptTracker.contains(message.id) { NotificationService.shared.sendPrivateMessageNotification( from: message.sender, message: message.content, @@ -5592,7 +5571,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { cleanupStaleUnreadPeerIDs() // Send read receipt if needed - if !sentReadReceipts.contains(message.id) { + if !readReceiptTracker.contains(message.id) { let receipt = ReadReceipt( originalMessageID: message.id, readerID: meshService.myPeerID, @@ -5612,7 +5591,7 @@ class ChatViewModel: ObservableObject, BitchatDelegate { self.sendReadReceipt(receipt, to: recipientID, originalTransport: originalTransport) } - sentReadReceipts.insert(message.id) + readReceiptTracker.insert(message.id) } // Mark other messages as read