mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-15 07:06:11 +00:00
Drop the cache fallback the comment above it argues against
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) <noreply@anthropic.com>
This commit is contained in:
parent
1adf22070b
commit
f99d0e85ab
@ -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`
|
||||
|
||||
@ -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()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user