From f99d0e85ab545ebdee865e258b126370c71e214d Mon Sep 17 00:00:00 2001 From: ecgang Date: Fri, 31 Jul 2026 08:48:00 -0700 Subject: [PATCH] Drop the cache fallback the comment above it argues against MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit didConnectToPeerSynchronously explained that short BLE IDs get recycled, so a cache entry left by a previous owner of an ID would name the wrong peer — then read exactly that cache when both live sources were absent. Reaching the fallback means unified-peer state and the noise session key both had nothing to say about this link, which is precisely when nothing is left to catch the mistake. Flushing [shortID, wrongStableID] drains a stranger's queue and silently skips the right one. Resolve from evidence about this link only. When neither live source has resolved yet the short-ID flush still runs, and authentication flushes both aliases once the identity is known, so nothing is lost by waiting. Mutation-proved: restoring the fallback turns didConnectToPeer_resolvesTheStableKeyWithoutUnifiedPeerState red. Co-Authored-By: Claude Opus 5 (1M context) --- .../ChatTransportEventCoordinator.swift | 20 +++++++++++-------- ...ransportEventCoordinatorContextTests.swift | 13 ++++++++---- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/bitchat/ViewModels/ChatTransportEventCoordinator.swift b/bitchat/ViewModels/ChatTransportEventCoordinator.swift index 07647128..9cd39bbd 100644 --- a/bitchat/ViewModels/ChatTransportEventCoordinator.swift +++ b/bitchat/ViewModels/ChatTransportEventCoordinator.swift @@ -280,12 +280,18 @@ final class ChatTransportEventCoordinator { context.registerEphemeralSession(peerID: peerID) context.notifyUIChanged() - // Resolve the stable key robustly: unified-peer state may not be - // populated yet at connect time, so fall back to the live noise - // session key and only then to the cache. Order matters — short BLE - // IDs are ephemeral and get recycled, so a cache entry left by a - // previous owner of this ID would name the wrong peer, while the - // session key is the identity of the link we just brought up. + // Resolve the stable key from evidence about *this* link only: + // unified-peer state, or the live noise session key. Both name the peer + // we just brought up. + // + // Deliberately no cache fallback. Short BLE IDs are ephemeral and get + // recycled, so a cache entry left by a previous owner of this ID names + // the wrong peer — and reaching the fallback means both live sources + // were absent, which is exactly when there is nothing to catch the + // mistake. Flushing `[shortID, wrongStableID]` would drain a stranger's + // queue and silently skip the right one. When neither live source has + // resolved yet, the short-ID flush below still runs, and authentication + // flushes both aliases once the identity is known. var stablePeerID: PeerID? if let peer = context.unifiedPeer(for: peerID) { let resolved = PeerID(hexData: peer.noisePublicKey) @@ -295,8 +301,6 @@ final class ChatTransportEventCoordinator { let derived = PeerID(hexData: key) context.cacheStablePeerID(derived, for: peerID) stablePeerID = derived - } else if let cached = context.cachedStablePeerID(for: peerID) { - stablePeerID = cached } // Flush the short ID and the stable 64-hex key together. `flushOutbox` diff --git a/bitchatTests/ChatTransportEventCoordinatorContextTests.swift b/bitchatTests/ChatTransportEventCoordinatorContextTests.swift index f9a8ac41..692e754d 100644 --- a/bitchatTests/ChatTransportEventCoordinatorContextTests.swift +++ b/bitchatTests/ChatTransportEventCoordinatorContextTests.swift @@ -525,20 +525,25 @@ struct ChatTransportEventCoordinatorContextTests { /// The stable key must still resolve when unified-peer state has not been /// populated yet at connect time — otherwise the flush silently no-ops in - /// exactly the case it is for. Falls back to the cache, then to the Noise - /// session key. + /// exactly the case it is for. It resolves from the live Noise session key, + /// never from the cache alone. @Test @MainActor func didConnectToPeer_resolvesTheStableKeyWithoutUnifiedPeerState() { let shortPeerID = PeerID(str: "1122334455667788") let noiseKey = Data((0..<32).map { UInt8(0xC0 &+ $0) }) let stablePeerID = PeerID(hexData: noiseKey) - // Cache only. + // Cache only, with no live evidence for this link. Short BLE IDs are + // recycled, so the entry may belong to a previous owner of this ID — + // flushing it would drain a stranger's queue and skip the right one. let viaCache = MockChatTransportEventContext() viaCache.cacheStablePeerID(stablePeerID, for: shortPeerID) ChatTransportEventCoordinator(context: viaCache) .didConnectToPeerSynchronously(shortPeerID) - #expect(viaCache.flushedOutboxPeerIDs.contains(stablePeerID)) + #expect( + viaCache.flushedOutboxPeerIDs == [shortPeerID], + "a cache entry with no live corroboration named the stable peer" + ) // Noise session key only. let viaSession = MockChatTransportEventContext()