mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-27 00:45:23 +00:00
Reconnect hygiene: paced acks, persistent gift-wrap dedup, self-fragment sync fix (#1398)
* Reconnect hygiene: paced acks, persistent gift-wrap dedup, self-fragment sync fix
Remaining findings from the July 7 locked-phone test sessions, all rooted
in reconnect/relaunch behavior:
- All Nostr acks (READ and DELIVERED, direct and geohash) now flow through
the paced queue that previously only throttled direct READ receipts.
Reconnect redelivery produced 8 DELIVERED acks in under a second, which
damus rejects ("noting too much").
- Processed gift-wrap event IDs persist across launches
(NostrProcessedEventStore, wired through MessageDeduplicationService,
debounced writes, wiped on the existing clear/panic paths). NIP-59
randomizes gift-wrap timestamps, so the 24h-lookback DM subscriptions
redeliver the same events every launch; without a cross-launch record
each relaunch reprocessed old PMs and acks — the re-ack bursts and the
"delivered ack for unknown mid" warnings (now debug: a stale ack is
expected occasionally and not actionable).
- Own fragments handed back by sync replay (the deliberate RSR ttl=0
restore path) now re-enter the gossip sync store before the self-drop.
The fragment store is not archived, so after a relaunch our sync filter
did not cover our own fragments and peers re-offered them every 30s
round indefinitely; recording them stops the redelivery after one round
while keeping assembly skipped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Address Codex review on #1398: shared ack pacer, transient clears keep disk record
- Geohash acks are sent through short-lived NostrTransport instances
(makeGeohashNostrTransport creates one per ack), so the per-instance ack
queue never paced a burst. Acks now flow through a pacer shared across
instances: Dependencies.live wires the process-wide sharedAckPacer, and
the default Dependencies init builds an isolated pacer from the same
injected scheduleAfter so tests keep stepping the throttle manually.
- clearNostrCaches() runs on every geohash channel switch, so it no longer
wipes the persisted gift-wrap record (that stays on the clearAll/panic
path). Persistence is now append-merge instead of snapshot-overwrite —
serialized on the store's IO queue — so a transient in-memory clear
between debounced flushes can't shrink the on-disk record either.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
jack
Claude Fable 5
parent
963d3a089f
commit
81c45e9649
@@ -19,6 +19,36 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
/// doesn't count: DM sends target the default relay set and would
|
||||
/// still queue.
|
||||
let relayConnectivity: @MainActor () -> AnyPublisher<Bool, Never>
|
||||
/// Paces outbound acks. Defaults to an isolated pacer so tests don't
|
||||
/// serialize behind each other; `live` passes the process-wide one.
|
||||
let ackPacer: AckPacer
|
||||
|
||||
init(
|
||||
notificationCenter: NotificationCenter,
|
||||
loadFavorites: @escaping @MainActor () -> [Data: FavoritesPersistenceService.FavoriteRelationship],
|
||||
favoriteStatusForNoiseKey: @escaping @MainActor (Data) -> FavoritesPersistenceService.FavoriteRelationship?,
|
||||
favoriteStatusForPeerID: @escaping @MainActor (PeerID) -> FavoritesPersistenceService.FavoriteRelationship?,
|
||||
currentIdentity: @escaping @MainActor () throws -> NostrIdentity?,
|
||||
registerPendingGiftWrap: @escaping @MainActor (String) -> Void,
|
||||
sendEvent: @escaping @MainActor (NostrEvent) -> Void,
|
||||
scheduleAfter: @escaping @Sendable (TimeInterval, @escaping @Sendable () -> Void) -> Void,
|
||||
relayConnectivity: @escaping @MainActor () -> AnyPublisher<Bool, Never>,
|
||||
ackPacer: AckPacer? = nil
|
||||
) {
|
||||
self.notificationCenter = notificationCenter
|
||||
self.loadFavorites = loadFavorites
|
||||
self.favoriteStatusForNoiseKey = favoriteStatusForNoiseKey
|
||||
self.favoriteStatusForPeerID = favoriteStatusForPeerID
|
||||
self.currentIdentity = currentIdentity
|
||||
self.registerPendingGiftWrap = registerPendingGiftWrap
|
||||
self.sendEvent = sendEvent
|
||||
self.scheduleAfter = scheduleAfter
|
||||
self.relayConnectivity = relayConnectivity
|
||||
// Default pacer drives its throttle through the same injected
|
||||
// scheduler, so tests that step scheduleAfter manually keep
|
||||
// control of the ack cadence.
|
||||
self.ackPacer = ackPacer ?? AckPacer(scheduleAfter: scheduleAfter)
|
||||
}
|
||||
|
||||
@MainActor
|
||||
static func live(idBridge: NostrIdentityBridge) -> Dependencies {
|
||||
@@ -33,7 +63,8 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
scheduleAfter: { delay, action in
|
||||
DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: action)
|
||||
},
|
||||
relayConnectivity: { NostrRelayManager.shared.$isDMRelayConnected.eraseToAnyPublisher() }
|
||||
relayConnectivity: { NostrRelayManager.shared.$isDMRelayConnected.eraseToAnyPublisher() },
|
||||
ackPacer: NostrTransport.sharedAckPacer
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -41,14 +72,61 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
// Provide BLE short peer ID for BitChat embedding
|
||||
var senderPeerID = PeerID(str: "")
|
||||
|
||||
// Throttle READ receipts to avoid relay rate limits
|
||||
private struct QueuedRead {
|
||||
let receipt: ReadReceipt
|
||||
let peerID: PeerID
|
||||
// Throttle outbound acks — READ receipts and DELIVERED acks, direct and
|
||||
// geohash — to avoid relay rate limits. Reconnect redelivery produces a
|
||||
// burst of acks at once: 8 DELIVERED in under a second tripped damus's
|
||||
// "noting too much" during July 2026 device testing.
|
||||
private enum QueuedAck {
|
||||
case readDirect(ReadReceipt, PeerID)
|
||||
case deliveredDirect(messageID: String, peerID: PeerID)
|
||||
case deliveredGeohash(messageID: String, recipientHex: String, identity: NostrIdentity)
|
||||
case readGeohash(messageID: String, recipientHex: String, identity: NostrIdentity)
|
||||
}
|
||||
private var readQueue: [QueuedRead] = []
|
||||
private var isSendingReadAcks = false
|
||||
private let readAckInterval: TimeInterval = TransportConfig.nostrReadAckInterval
|
||||
|
||||
/// Ack pacing shared across transport instances. Geohash acks are sent
|
||||
/// through short-lived transports created per ack
|
||||
/// (`makeGeohashNostrTransport()`), so a per-instance queue would only
|
||||
/// ever hold one item and never pace a burst (flagged by Codex on
|
||||
/// #1398). Production wires `sharedAckPacer` via `Dependencies.live`;
|
||||
/// tests get an isolated instance per `Dependencies` by default.
|
||||
final class AckPacer {
|
||||
typealias Scheduler = @Sendable (TimeInterval, @escaping @Sendable () -> Void) -> Void
|
||||
|
||||
private let queue = DispatchQueue(label: "chat.bitchat.nostr-ack-pacer")
|
||||
private var pending: [() -> Void] = []
|
||||
private var isSending = false
|
||||
private let interval: TimeInterval = TransportConfig.nostrReadAckInterval
|
||||
private let scheduleAfter: Scheduler
|
||||
|
||||
init(scheduleAfter: @escaping Scheduler = { delay, action in
|
||||
DispatchQueue.global(qos: .utility).asyncAfter(deadline: .now() + delay, execute: action)
|
||||
}) {
|
||||
self.scheduleAfter = scheduleAfter
|
||||
}
|
||||
|
||||
func enqueue(_ send: @escaping () -> Void) {
|
||||
queue.async {
|
||||
self.pending.append(send)
|
||||
self.processNext()
|
||||
}
|
||||
}
|
||||
|
||||
/// Must be called on `queue`.
|
||||
private func processNext() {
|
||||
guard !isSending, !pending.isEmpty else { return }
|
||||
isSending = true
|
||||
let send = pending.removeFirst()
|
||||
send()
|
||||
scheduleAfter(interval) { [weak self] in
|
||||
guard let self else { return }
|
||||
self.queue.async {
|
||||
self.isSending = false
|
||||
self.processNext()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
static let sharedAckPacer = AckPacer()
|
||||
private let keychain: KeychainManagerProtocol
|
||||
private let idBridge: NostrIdentityBridge
|
||||
private let dependencies: Dependencies
|
||||
@@ -187,12 +265,14 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
}
|
||||
|
||||
func sendReadReceipt(_ receipt: ReadReceipt, to peerID: PeerID) {
|
||||
// Enqueue and process with throttling to avoid relay rate limits
|
||||
// Use barrier to synchronize access to readQueue
|
||||
queue.async(flags: .barrier) {
|
||||
self.readQueue.append(QueuedRead(receipt: receipt, peerID: peerID))
|
||||
self.processReadQueueIfNeeded()
|
||||
}
|
||||
enqueueAck(.readDirect(receipt, peerID))
|
||||
}
|
||||
|
||||
/// Enqueue an ack for paced sending. Captures self strongly on purpose:
|
||||
/// geohash acks ride throwaway transport instances that must stay alive
|
||||
/// until their ack leaves the queue.
|
||||
private func enqueueAck(_ ack: QueuedAck) {
|
||||
dependencies.ackPacer.enqueue { self.sendAckItem(ack) }
|
||||
}
|
||||
|
||||
func sendFavoriteNotification(to peerID: PeerID, isFavorite: Bool) {
|
||||
@@ -212,17 +292,7 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
|
||||
func sendBroadcastAnnounce() { /* no-op for Nostr */ }
|
||||
func sendDeliveryAck(for messageID: String, to peerID: PeerID) {
|
||||
Task { @MainActor in
|
||||
guard let recipientNpub = resolveRecipientNpub(for: peerID),
|
||||
let recipientHex = npubToHex(recipientNpub),
|
||||
let senderIdentity = try? dependencies.currentIdentity() else { return }
|
||||
SecureLogger.debug("NostrTransport: preparing DELIVERED ack id=\(messageID.prefix(8))…", category: .session)
|
||||
guard let ack = NostrEmbeddedBitChat.encodeAckForNostr(type: .delivered, messageID: messageID, recipientPeerID: peerID, senderPeerID: senderPeerID) else {
|
||||
SecureLogger.error("NostrTransport: failed to embed DELIVERED ack", category: .session)
|
||||
return
|
||||
}
|
||||
sendWrappedMessage(content: ack, recipientHex: recipientHex, senderIdentity: senderIdentity)
|
||||
}
|
||||
enqueueAck(.deliveredDirect(messageID: messageID, peerID: peerID))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -232,19 +302,11 @@ extension NostrTransport {
|
||||
|
||||
// MARK: Geohash ACK helpers
|
||||
func sendDeliveryAckGeohash(for messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity) {
|
||||
Task { @MainActor in
|
||||
SecureLogger.debug("GeoDM: send DELIVERED mid=\(messageID.prefix(8))…", category: .session)
|
||||
guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .delivered, messageID: messageID, senderPeerID: senderPeerID) else { return }
|
||||
sendWrappedMessage(content: embedded, recipientHex: recipientHex, senderIdentity: identity, registerPending: true)
|
||||
}
|
||||
enqueueAck(.deliveredGeohash(messageID: messageID, recipientHex: recipientHex, identity: identity))
|
||||
}
|
||||
|
||||
func sendReadReceiptGeohash(_ messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity) {
|
||||
Task { @MainActor in
|
||||
SecureLogger.debug("GeoDM: send READ mid=\(messageID.prefix(8))…", category: .session)
|
||||
guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .readReceipt, messageID: messageID, senderPeerID: senderPeerID) else { return }
|
||||
sendWrappedMessage(content: embedded, recipientHex: recipientHex, senderIdentity: identity, registerPending: true)
|
||||
}
|
||||
enqueueAck(.readGeohash(messageID: messageID, recipientHex: recipientHex, identity: identity))
|
||||
}
|
||||
|
||||
// MARK: Geohash DMs (per-geohash identity)
|
||||
@@ -290,36 +352,42 @@ extension NostrTransport {
|
||||
dependencies.sendEvent(event)
|
||||
}
|
||||
|
||||
/// Must be called within a barrier on `queue`
|
||||
private func processReadQueueIfNeeded() {
|
||||
guard !isSendingReadAcks else { return }
|
||||
guard !readQueue.isEmpty else { return }
|
||||
isSendingReadAcks = true
|
||||
let item = readQueue.removeFirst()
|
||||
sendReadAckItem(item)
|
||||
}
|
||||
|
||||
/// Sends a single read ack item (called after extraction from queue within barrier)
|
||||
private func sendReadAckItem(_ item: QueuedRead) {
|
||||
/// Sends a single ack item (invoked by the pacer, one per interval)
|
||||
private func sendAckItem(_ item: QueuedAck) {
|
||||
Task { @MainActor in
|
||||
defer { scheduleNextReadAck() }
|
||||
guard let recipientNpub = resolveRecipientNpub(for: item.peerID),
|
||||
let recipientHex = npubToHex(recipientNpub),
|
||||
let senderIdentity = try? dependencies.currentIdentity() else { return }
|
||||
SecureLogger.debug("NostrTransport: preparing READ ack id=\(item.receipt.originalMessageID.prefix(8))…", category: .session)
|
||||
guard let ack = NostrEmbeddedBitChat.encodeAckForNostr(type: .readReceipt, messageID: item.receipt.originalMessageID, recipientPeerID: item.peerID, senderPeerID: senderPeerID) else {
|
||||
SecureLogger.error("NostrTransport: failed to embed READ ack", category: .session)
|
||||
return
|
||||
}
|
||||
sendWrappedMessage(content: ack, recipientHex: recipientHex, senderIdentity: senderIdentity)
|
||||
}
|
||||
}
|
||||
switch item {
|
||||
case .readDirect(let receipt, let peerID):
|
||||
guard let recipientNpub = resolveRecipientNpub(for: peerID),
|
||||
let recipientHex = npubToHex(recipientNpub),
|
||||
let senderIdentity = try? dependencies.currentIdentity() else { return }
|
||||
SecureLogger.debug("NostrTransport: preparing READ ack id=\(receipt.originalMessageID.prefix(8))…", category: .session)
|
||||
guard let ack = NostrEmbeddedBitChat.encodeAckForNostr(type: .readReceipt, messageID: receipt.originalMessageID, recipientPeerID: peerID, senderPeerID: senderPeerID) else {
|
||||
SecureLogger.error("NostrTransport: failed to embed READ ack", category: .session)
|
||||
return
|
||||
}
|
||||
sendWrappedMessage(content: ack, recipientHex: recipientHex, senderIdentity: senderIdentity)
|
||||
|
||||
private func scheduleNextReadAck() {
|
||||
dependencies.scheduleAfter(readAckInterval) { [weak self] in
|
||||
self?.queue.async(flags: .barrier) { [weak self] in
|
||||
self?.isSendingReadAcks = false
|
||||
self?.processReadQueueIfNeeded()
|
||||
case .deliveredDirect(let messageID, let peerID):
|
||||
guard let recipientNpub = resolveRecipientNpub(for: peerID),
|
||||
let recipientHex = npubToHex(recipientNpub),
|
||||
let senderIdentity = try? dependencies.currentIdentity() else { return }
|
||||
SecureLogger.debug("NostrTransport: preparing DELIVERED ack id=\(messageID.prefix(8))…", category: .session)
|
||||
guard let ack = NostrEmbeddedBitChat.encodeAckForNostr(type: .delivered, messageID: messageID, recipientPeerID: peerID, senderPeerID: senderPeerID) else {
|
||||
SecureLogger.error("NostrTransport: failed to embed DELIVERED ack", category: .session)
|
||||
return
|
||||
}
|
||||
sendWrappedMessage(content: ack, recipientHex: recipientHex, senderIdentity: senderIdentity)
|
||||
|
||||
case .deliveredGeohash(let messageID, let recipientHex, let identity):
|
||||
SecureLogger.debug("GeoDM: send DELIVERED mid=\(messageID.prefix(8))…", category: .session)
|
||||
guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .delivered, messageID: messageID, senderPeerID: senderPeerID) else { return }
|
||||
sendWrappedMessage(content: embedded, recipientHex: recipientHex, senderIdentity: identity, registerPending: true)
|
||||
|
||||
case .readGeohash(let messageID, let recipientHex, let identity):
|
||||
SecureLogger.debug("GeoDM: send READ mid=\(messageID.prefix(8))…", category: .session)
|
||||
guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .readReceipt, messageID: messageID, senderPeerID: senderPeerID) else { return }
|
||||
sendWrappedMessage(content: embedded, recipientHex: recipientHex, senderIdentity: identity, registerPending: true)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user