mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-29 07:27:16 +00:00
Flush the stable-key outbox on reconnect and authentication
Re-scoped to the half of #1408 that is still a real bug, as the review asked. A DM composed while the recipient is an offline favorite is queued in the router outbox under their stable 64-hex Noise key. `flushOutbox` is keyed by exactly the id it is handed, and the reconnect path only ever handed it the short 16-hex id, so that queue was never reached. #1462's retry does not cover it either — that filters on `sendAttempts > 0`, and this mail was never transmitted at all. Absent a courier it waited for an app relaunch, a favorite-status change, or the 24h TTL. Connect now resolves the stable key (unified peer, else the cache, else the Noise session key, mirroring the disconnect path) and flushes it alongside the short id. Authentication does the same, once the link is authenticated and the identity is known. Dropped from the original PR: the ChatPrivateConversationCoordinator send-path changes and the mutual-favorite guard on offline Nostr delivery. #1415 rewrote that block — "always hand the message to the router; it owns delivery" — and removed the very gate those commits preserved, so both are superseded rather than merged. Rebuilt on current main instead of resolving four conflicts in code that no longer needed to change. Three tests, and one existing expectation updated because the behaviour it pinned is the bug: a connect flushes both queues, the stable key still resolves when unified-peer state has not populated yet (the case the fix exists for), and an unresolvable peer issues no bogus second flush. Mutation-verified: removing the stable-key flush fails all three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c1ce9029d8
commit
623f3855c5
@ -274,13 +274,33 @@ 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 cache and then to
|
||||
// the noise session key (mirroring the disconnect path).
|
||||
var stablePeerID: PeerID?
|
||||
if let peer = context.unifiedPeer(for: peerID) {
|
||||
let stablePeerID = PeerID(hexData: peer.noisePublicKey)
|
||||
context.cacheStablePeerID(stablePeerID, for: peerID)
|
||||
let resolved = PeerID(hexData: peer.noisePublicKey)
|
||||
context.cacheStablePeerID(resolved, for: peerID)
|
||||
stablePeerID = resolved
|
||||
} else if let cached = context.cachedStablePeerID(for: peerID) {
|
||||
stablePeerID = cached
|
||||
} else if let key = context.noiseSessionPublicKeyData(for: peerID) {
|
||||
let derived = PeerID(hexData: key)
|
||||
context.cacheStablePeerID(derived, for: peerID)
|
||||
stablePeerID = derived
|
||||
}
|
||||
|
||||
context.flushRouterOutbox(for: peerID)
|
||||
context.retryCourierDeposits(via: peerID)
|
||||
|
||||
// Also flush under the stable 64-hex key. `flushOutbox` is keyed by
|
||||
// exactly the id it is handed, and a DM composed while the recipient
|
||||
// was an offline favorite is queued under their stable key — so the
|
||||
// short-id flush above never reaches it, and absent a courier it waits
|
||||
// for a relaunch, a favorite-status change, or the 24h TTL.
|
||||
if let stablePeerID, stablePeerID != peerID {
|
||||
context.flushRouterOutbox(for: stablePeerID)
|
||||
}
|
||||
}
|
||||
|
||||
func didDisconnectFromPeer(_ peerID: PeerID) {
|
||||
|
||||
@ -45,6 +45,9 @@ protocol ChatVerificationContext: AnyObject {
|
||||
func resolveNickname(for peerID: PeerID) -> String
|
||||
func cachedStablePeerID(for shortPeerID: PeerID) -> PeerID?
|
||||
func cacheStablePeerID(_ stablePeerID: PeerID, for shortPeerID: PeerID)
|
||||
/// Flushes the message router's disk outbox for the given (stable) key so
|
||||
/// mail queued while the peer was offline delivers once it authenticates.
|
||||
func flushRouterOutbox(for peerID: PeerID)
|
||||
|
||||
// MARK: Noise sessions & verification transport
|
||||
/// Installs the Noise service's session callbacks (single registration point).
|
||||
@ -78,7 +81,8 @@ extension ChatViewModel: ChatVerificationContext {
|
||||
// `isVerifiedFingerprint(_:)`, `setEncryptionStatus(_:for:)`,
|
||||
// `resolveNickname(for:)`, `cachedStablePeerID(for:)`,
|
||||
// `cacheStablePeerID(_:for:)`, `noiseSessionPublicKeyData(for:)`,
|
||||
// `hasEstablishedNoiseSession(with:)`, and `triggerHandshake(with:)` are
|
||||
// `hasEstablishedNoiseSession(with:)`, `triggerHandshake(with:)`, and
|
||||
// `flushRouterOutbox(for:)` (shared with `ChatTransportEventContext`) are
|
||||
// shared requirements with the other contexts or satisfied by existing
|
||||
// `ChatViewModel` members. The members below flatten nested service
|
||||
// accesses into intent-named calls.
|
||||
@ -245,9 +249,9 @@ final class ChatVerificationCoordinator {
|
||||
// retry it now that this newly authenticated/replacement
|
||||
// session can actually decrypt it.
|
||||
var peerIDAliases = [peerID]
|
||||
if let stablePeerID = authenticatedStablePeerID
|
||||
?? self.context.cachedStablePeerID(for: peerID),
|
||||
stablePeerID != peerID {
|
||||
let stablePeerID = authenticatedStablePeerID
|
||||
?? self.context.cachedStablePeerID(for: peerID)
|
||||
if let stablePeerID, stablePeerID != peerID {
|
||||
// Conversations can migrate from the ephemeral BLE ID
|
||||
// to the authenticated Noise-key ID. Retry both aliases
|
||||
// because either may own the retained outbox entry.
|
||||
@ -255,6 +259,18 @@ final class ChatVerificationCoordinator {
|
||||
}
|
||||
self.context.retrySecurePrivateMessagesAfterAuthentication(for: peerIDAliases)
|
||||
|
||||
// The retry above only reaches messages already transmitted
|
||||
// through a secure session (it filters on `sendAttempts > 0`).
|
||||
// A DM composed while this peer was offline was never
|
||||
// transmitted at all — it sits in the outbox under the
|
||||
// stable 64-hex key — so it is in neither that set nor the
|
||||
// short-id flush on connect. Flush it here, now that the
|
||||
// link is authenticated and the stable identity is known,
|
||||
// rather than leaving it for the TTL.
|
||||
if let stablePeerID, stablePeerID != peerID {
|
||||
self.context.flushRouterOutbox(for: stablePeerID)
|
||||
}
|
||||
|
||||
if var pending = self.pendingQRVerifications[peerID], pending.sent == false {
|
||||
self.context.sendVerifyChallenge(
|
||||
to: peerID,
|
||||
|
||||
@ -343,7 +343,10 @@ struct ChatTransportEventCoordinatorContextTests {
|
||||
#expect(context.isConnected)
|
||||
#expect(context.registeredEphemeralSessions == [peerID])
|
||||
#expect(context.stablePeerIDCache[peerID] == PeerID(hexData: noiseKey))
|
||||
#expect(context.flushedOutboxPeerIDs == [peerID])
|
||||
// Both queues: the short id, then the stable 64-hex key that offline-
|
||||
// composed mail is queued under (#1408). Order matters only in that the
|
||||
// short-id flush is not delayed behind the stable-key resolution.
|
||||
#expect(context.flushedOutboxPeerIDs == [peerID, PeerID(hexData: noiseKey)])
|
||||
#expect(context.notifyUIChangedCount == 1)
|
||||
|
||||
// Their messages' read receipts are un-marked on disconnect so READ
|
||||
@ -480,4 +483,71 @@ struct ChatTransportEventCoordinatorContextTests {
|
||||
#expect(context.handledPrivateMessages.count == 1)
|
||||
#expect(context.meshDeliveryAcks.count == 1)
|
||||
}
|
||||
|
||||
/// #1408: a DM composed while the recipient was an offline favorite is
|
||||
/// queued in the router outbox under the peer's STABLE 64-hex Noise key.
|
||||
/// `flushOutbox` is keyed by exactly the id it is handed, so flushing only
|
||||
/// the short 16-hex id on connect never reaches that queue — the mail waits
|
||||
/// for an app relaunch, a favorite-status change, or the 24h TTL.
|
||||
///
|
||||
/// Reconnect must flush both.
|
||||
@Test @MainActor
|
||||
func didConnectToPeer_flushesTheStableKeyOutboxAsWellAsTheShortID() {
|
||||
let context = MockChatTransportEventContext()
|
||||
let coordinator = ChatTransportEventCoordinator(context: context)
|
||||
|
||||
let shortPeerID = PeerID(str: "1122334455667788")
|
||||
let noiseKey = Data((0..<32).map { UInt8(0xB0 &+ $0) })
|
||||
let stablePeerID = PeerID(hexData: noiseKey)
|
||||
#expect(stablePeerID != shortPeerID)
|
||||
|
||||
// The peer is known by its Noise key, as it is after a handshake.
|
||||
context.peersByID[shortPeerID] = BitchatPeer(
|
||||
peerID: shortPeerID,
|
||||
noisePublicKey: noiseKey,
|
||||
nickname: "alice"
|
||||
)
|
||||
|
||||
coordinator.didConnectToPeerSynchronously(shortPeerID)
|
||||
|
||||
// Both queues are drained, and the stable key is the one that carries
|
||||
// the offline-composed mail.
|
||||
#expect(context.flushedOutboxPeerIDs.contains(shortPeerID))
|
||||
#expect(
|
||||
context.flushedOutboxPeerIDs.contains(stablePeerID),
|
||||
"offline-queued mail under the stable key was never flushed"
|
||||
)
|
||||
}
|
||||
|
||||
/// 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.
|
||||
@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.
|
||||
let viaCache = MockChatTransportEventContext()
|
||||
viaCache.cacheStablePeerID(stablePeerID, for: shortPeerID)
|
||||
ChatTransportEventCoordinator(context: viaCache)
|
||||
.didConnectToPeerSynchronously(shortPeerID)
|
||||
#expect(viaCache.flushedOutboxPeerIDs.contains(stablePeerID))
|
||||
|
||||
// Noise session key only.
|
||||
let viaSession = MockChatTransportEventContext()
|
||||
viaSession.noiseSessionKeysByPeerID[shortPeerID] = noiseKey
|
||||
ChatTransportEventCoordinator(context: viaSession)
|
||||
.didConnectToPeerSynchronously(shortPeerID)
|
||||
#expect(viaSession.flushedOutboxPeerIDs.contains(stablePeerID))
|
||||
|
||||
// Nothing resolvable: the short-id flush still happens, and no bogus
|
||||
// second flush is issued.
|
||||
let unresolvable = MockChatTransportEventContext()
|
||||
ChatTransportEventCoordinator(context: unresolvable)
|
||||
.didConnectToPeerSynchronously(shortPeerID)
|
||||
#expect(unresolvable.flushedOutboxPeerIDs == [shortPeerID])
|
||||
}
|
||||
}
|
||||
|
||||
@ -91,6 +91,9 @@ private final class MockChatVerificationContext: ChatVerificationContext {
|
||||
stablePeerIDCache[shortPeerID] = stablePeerID
|
||||
}
|
||||
|
||||
private(set) var flushedOutboxPeerIDs: [PeerID] = []
|
||||
func flushRouterOutbox(for peerID: PeerID) { flushedOutboxPeerIDs.append(peerID) }
|
||||
|
||||
// Noise sessions & verification transport
|
||||
var myNoiseStaticKey = Data(repeating: 0x42, count: 32)
|
||||
var establishedNoiseSessions: Set<PeerID> = []
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user