mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-24 22:45:19 +00:00
Courier: deposit in parallel when the only route is a send queue (#1368)
* Deposit with couriers in parallel when the only route is a send queue The courier path was nearly unreachable: NostrTransport claims any favorite with a known npub as "reachable" regardless of connectivity, and the mesh favorite exchange shares npubs, so for essentially every courier-eligible recipient the router picked Nostr's reachable branch. With no internet the message just sat in the relay send queue — in the flagship scenario (internet shutdown, mutual friend standing right there) the courier walked away carrying nothing. Add Transport.canDeliverPromptly(to:), defaulting to reachability for radio-backed transports; NostrTransport answers honestly by mirroring the relay manager's connection state (fail-closed behind Tor). When the chosen transport can't hand the message off promptly, the router now also deposits a sealed copy with connected couriers. Double delivery is harmless: receivers dedup by message ID, and delivered/read acks never downgrade the carried status. When relays are up, sends are trusted and no courier quota is spent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Track DM-relay connectivity, not any-relay, for prompt delivery Codex review: NostrRelayManager.isConnected is true when any relay is up, including geohash/custom relays — but private messages target the default (gift-wrap-capable) relay set and queue when none of those are connected. A lone geohash relay would have suppressed the parallel courier deposit while the DM sat in the queue. Publish a DM-scoped connectivity flag and drive canDeliverPromptly from it. 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
75da63c9d7
commit
a66c591f8e
@@ -137,6 +137,10 @@ final class NostrRelayManager: ObservableObject {
|
||||
|
||||
@Published private(set) var relays: [Relay] = []
|
||||
@Published private(set) var isConnected = false
|
||||
/// Whether a relay that carries private messages is connected. DMs
|
||||
/// target the default (gift-wrap-capable) relay set, so a connected
|
||||
/// geohash/custom relay alone must not count — sends would still queue.
|
||||
@Published private(set) var isDMRelayConnected = false
|
||||
|
||||
private let dependencies: NostrRelayManagerDependencies
|
||||
private var allowDefaultRelays: Bool = false
|
||||
@@ -1087,6 +1091,9 @@ final class NostrRelayManager: ObservableObject {
|
||||
|
||||
private func updateConnectionStatus() {
|
||||
isConnected = relays.contains { $0.isConnected }
|
||||
// Relay URLs are normalized before entries are created, so direct
|
||||
// set membership is sound.
|
||||
isDMRelayConnected = relays.contains { $0.isConnected && Self.defaultRelaySet.contains($0.url) }
|
||||
}
|
||||
|
||||
/// A relay that drops before sending EOSE must not stall initial-load
|
||||
|
||||
@@ -128,6 +128,14 @@ final class MessageRouter {
|
||||
SecureLogger.debug("Routing PM via \(type(of: transport)) (reachable) to \(peerID.id.prefix(8))… id=\(messageID.prefix(8))…", category: .session)
|
||||
transport.sendPrivateMessage(content, to: peerID, recipientNickname: recipientNickname, messageID: messageID)
|
||||
enqueue(message, for: peerID)
|
||||
// "Reachable" without prompt delivery means the send only joined
|
||||
// a queue (Nostr with relays down): also hand a sealed copy to
|
||||
// any connected couriers rather than waiting for internet that
|
||||
// may never come. Double delivery is harmless — receivers dedup
|
||||
// by message ID, and delivered/read acks never downgrade.
|
||||
if !transport.canDeliverPromptly(to: peerID) {
|
||||
attemptCourierDeposit(content: content, messageID: messageID, for: peerID)
|
||||
}
|
||||
} else {
|
||||
var unsent = message
|
||||
unsent.sendAttempts = 0
|
||||
@@ -137,11 +145,12 @@ final class MessageRouter {
|
||||
}
|
||||
}
|
||||
|
||||
/// Last resort when no transport can reach the peer: seal the message to
|
||||
/// their known static key and hand it to connected mutual favorites who
|
||||
/// may physically encounter them. The queued copy above stays retained,
|
||||
/// so direct delivery still wins if the peer reappears first (receivers
|
||||
/// dedup by message ID).
|
||||
/// Last resort when no transport can deliver promptly — the peer is
|
||||
/// unreachable, or only reachable through a send queue waiting on
|
||||
/// internet: seal the message to their known static key and hand it to
|
||||
/// connected mutual favorites who may physically encounter them. The
|
||||
/// queued copy above stays retained, so direct delivery still wins if
|
||||
/// the peer reappears first (receivers dedup by message ID).
|
||||
private func attemptCourierDeposit(content: String, messageID: String, for peerID: PeerID) {
|
||||
guard let recipientKey = courierDirectory.noiseKey(peerID) else { return }
|
||||
for transport in transports {
|
||||
|
||||
@@ -14,7 +14,13 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
let registerPendingGiftWrap: @MainActor (String) -> Void
|
||||
let sendEvent: @MainActor (NostrEvent) -> Void
|
||||
let scheduleAfter: @Sendable (TimeInterval, @escaping @Sendable () -> Void) -> Void
|
||||
/// Emits whether a relay that carries private messages is up
|
||||
/// (fail-closed behind Tor). A connected geohash/custom relay alone
|
||||
/// doesn't count: DM sends target the default relay set and would
|
||||
/// still queue.
|
||||
let relayConnectivity: @MainActor () -> AnyPublisher<Bool, Never>
|
||||
|
||||
@MainActor
|
||||
static func live(idBridge: NostrIdentityBridge) -> Dependencies {
|
||||
Dependencies(
|
||||
notificationCenter: .default,
|
||||
@@ -26,7 +32,8 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
sendEvent: { NostrRelayManager.shared.sendEvent($0) },
|
||||
scheduleAfter: { delay, action in
|
||||
DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: action)
|
||||
}
|
||||
},
|
||||
relayConnectivity: { NostrRelayManager.shared.$isDMRelayConnected.eraseToAnyPublisher() }
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -49,6 +56,10 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
|
||||
// Reachability Cache (thread-safe)
|
||||
private var reachablePeers: Set<PeerID> = []
|
||||
// Mirror of the relay manager's connection state, cached here because
|
||||
// canDeliverPromptly is called synchronously off the main actor.
|
||||
private var relaysConnected = false
|
||||
private var relayConnectivityCancellable: AnyCancellable?
|
||||
private let queue = DispatchQueue(label: "nostr.transport.state", attributes: .concurrent)
|
||||
|
||||
@MainActor
|
||||
@@ -72,6 +83,12 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
queue.sync(flags: .barrier) {
|
||||
self.reachablePeers = Set(reachable)
|
||||
}
|
||||
|
||||
relayConnectivityCancellable = self.dependencies.relayConnectivity()
|
||||
.sink { [weak self] connected in
|
||||
guard let self else { return }
|
||||
self.queue.async(flags: .barrier) { self.relaysConnected = connected }
|
||||
}
|
||||
}
|
||||
|
||||
deinit {
|
||||
@@ -134,6 +151,14 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
}
|
||||
}
|
||||
|
||||
func canDeliverPromptly(to peerID: PeerID) -> Bool {
|
||||
// A known npub makes a peer "reachable", but with no relay
|
||||
// connection a send only joins the local queue. Answering honestly
|
||||
// here lets the router hand a sealed copy to a courier in parallel
|
||||
// instead of waiting for internet that may never come.
|
||||
isPeerReachable(peerID) && queue.sync { relaysConnected }
|
||||
}
|
||||
|
||||
func peerNickname(peerID: PeerID) -> String? { nil }
|
||||
func getPeerNicknames() -> [PeerID: String] { [:] }
|
||||
|
||||
|
||||
@@ -54,6 +54,11 @@ protocol Transport: AnyObject {
|
||||
// Connectivity and peers
|
||||
func isPeerConnected(_ peerID: PeerID) -> Bool
|
||||
func isPeerReachable(_ peerID: PeerID) -> Bool
|
||||
/// Whether a send to this peer is likely to leave the device promptly.
|
||||
/// Distinct from reachability: Nostr claims any favorite with a known
|
||||
/// npub as reachable even with no relay connection, where a send only
|
||||
/// joins a queue waiting for internet that may never come.
|
||||
func canDeliverPromptly(to peerID: PeerID) -> Bool
|
||||
func peerNickname(peerID: PeerID) -> String?
|
||||
func getPeerNicknames() -> [PeerID: String]
|
||||
|
||||
@@ -111,6 +116,10 @@ protocol Transport: AnyObject {
|
||||
}
|
||||
|
||||
extension Transport {
|
||||
// Reachability implies prompt delivery for transports that hand packets
|
||||
// straight to the radio; queue-backed transports override this.
|
||||
func canDeliverPromptly(to peerID: PeerID) -> Bool { isPeerReachable(peerID) }
|
||||
|
||||
// Noise identity hooks default to inert for transports that do not carry
|
||||
// Noise sessions (e.g. NostrTransport).
|
||||
func noiseSessionPublicKeyData(for peerID: PeerID) -> Data? { nil }
|
||||
|
||||
@@ -544,7 +544,16 @@ private final class CourierCaptureTransport: Transport {
|
||||
func isPeerConnected(_ peerID: PeerID) -> Bool {
|
||||
snapshots.contains { $0.peerID == peerID && $0.isConnected }
|
||||
}
|
||||
func isPeerReachable(_ peerID: PeerID) -> Bool { isPeerConnected(peerID) }
|
||||
// Nostr-style reachability: claimed for peers with no live link (known
|
||||
// npub), where prompt delivery additionally needs a relay connection.
|
||||
var reachablePeers: Set<PeerID> = []
|
||||
var promptDelivery = true
|
||||
func isPeerReachable(_ peerID: PeerID) -> Bool {
|
||||
isPeerConnected(peerID) || reachablePeers.contains(peerID)
|
||||
}
|
||||
func canDeliverPromptly(to peerID: PeerID) -> Bool {
|
||||
isPeerReachable(peerID) && promptDelivery
|
||||
}
|
||||
func peerNickname(peerID: PeerID) -> String? { nil }
|
||||
func getPeerNicknames() -> [PeerID: String] { [:] }
|
||||
|
||||
@@ -652,4 +661,70 @@ struct MessageRouterCourierTests {
|
||||
#expect(transport.directSends == ["m3"])
|
||||
#expect(transport.courierSends.isEmpty)
|
||||
}
|
||||
|
||||
/// A peer can be "reachable" through a transport that cannot deliver
|
||||
/// promptly (Nostr claims any favorite with a known npub, even with no
|
||||
/// relay connection). The queued send must not shadow the courier: a
|
||||
/// sealed copy goes to connected couriers in parallel, and receivers
|
||||
/// dedup by message ID if both arrive.
|
||||
@Test @MainActor
|
||||
func queuedReachableSendAlsoDepositsWithCourier() {
|
||||
let bobKey = Data(repeating: 0xB0, count: 32)
|
||||
let bobID = PeerID(publicKey: bobKey)
|
||||
let carolKey = Data(repeating: 0xC0, count: 32)
|
||||
let carolID = PeerID(publicKey: carolKey)
|
||||
|
||||
let transport = CourierCaptureTransport()
|
||||
transport.snapshots = [
|
||||
TransportPeerSnapshot(peerID: carolID, nickname: "carol", isConnected: true, noisePublicKey: carolKey, lastSeen: Date())
|
||||
]
|
||||
transport.reachablePeers = [bobID]
|
||||
transport.promptDelivery = false
|
||||
|
||||
let directory = CourierDirectory(
|
||||
noiseKey: { peerID in peerID == bobID ? bobKey : nil },
|
||||
isTrustedCourier: { $0 == carolKey }
|
||||
)
|
||||
let router = MessageRouter(transports: [transport], courierDirectory: directory)
|
||||
var carried: [String] = []
|
||||
router.onMessageCarried = { messageID, _ in carried.append(messageID) }
|
||||
|
||||
router.sendPrivate("hi bob", to: bobID, recipientNickname: "bob", messageID: "m4")
|
||||
|
||||
#expect(transport.directSends == ["m4"])
|
||||
#expect(transport.courierSends.count == 1)
|
||||
#expect(transport.courierSends.first?.messageID == "m4")
|
||||
#expect(transport.courierSends.first?.couriers == [carolID])
|
||||
#expect(carried == ["m4"])
|
||||
}
|
||||
|
||||
/// When the reachable transport can deliver promptly (relays up), the
|
||||
/// send is trusted and no courier quota is spent.
|
||||
@Test @MainActor
|
||||
func promptlyDeliverableReachablePeerSkipsCourier() {
|
||||
let bobKey = Data(repeating: 0xB0, count: 32)
|
||||
let bobID = PeerID(publicKey: bobKey)
|
||||
let carolKey = Data(repeating: 0xC0, count: 32)
|
||||
let carolID = PeerID(publicKey: carolKey)
|
||||
|
||||
let transport = CourierCaptureTransport()
|
||||
transport.snapshots = [
|
||||
TransportPeerSnapshot(peerID: carolID, nickname: "carol", isConnected: true, noisePublicKey: carolKey, lastSeen: Date())
|
||||
]
|
||||
transport.reachablePeers = [bobID]
|
||||
|
||||
let directory = CourierDirectory(
|
||||
noiseKey: { peerID in peerID == bobID ? bobKey : nil },
|
||||
isTrustedCourier: { $0 == carolKey }
|
||||
)
|
||||
let router = MessageRouter(transports: [transport], courierDirectory: directory)
|
||||
var carried: [String] = []
|
||||
router.onMessageCarried = { messageID, _ in carried.append(messageID) }
|
||||
|
||||
router.sendPrivate("hi bob", to: bobID, recipientNickname: "bob", messageID: "m5")
|
||||
|
||||
#expect(transport.directSends == ["m5"])
|
||||
#expect(transport.courierSends.isEmpty)
|
||||
#expect(carried.isEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
// For more information, see <https://unlicense.org>
|
||||
//
|
||||
|
||||
import Combine
|
||||
import Foundation
|
||||
import Testing
|
||||
import BitFoundation
|
||||
@@ -85,6 +86,51 @@ struct NostrTransportTests {
|
||||
#expect(didRefresh)
|
||||
}
|
||||
|
||||
@Test("Prompt delivery requires both a known npub and a relay connection")
|
||||
@MainActor
|
||||
func canDeliverPromptlyTracksRelayConnectivity() async throws {
|
||||
let keychain = MockKeychain()
|
||||
let idBridge = NostrIdentityBridge(keychain: keychain)
|
||||
let recipient = try NostrIdentity.generate()
|
||||
let noiseKey = Data((0..<32).map(UInt8.init))
|
||||
let peerID = PeerID(hexData: noiseKey)
|
||||
let relationship = makeRelationship(
|
||||
peerNoisePublicKey: noiseKey,
|
||||
peerNostrPublicKey: recipient.npub,
|
||||
peerNickname: "Alice"
|
||||
)
|
||||
let connectivity = CurrentValueSubject<Bool, Never>(false)
|
||||
|
||||
let transport = NostrTransport(
|
||||
keychain: keychain,
|
||||
idBridge: idBridge,
|
||||
dependencies: makeDependencies(
|
||||
loadFavorites: { [noiseKey: relationship] },
|
||||
relayConnectivity: { connectivity.eraseToAnyPublisher() }
|
||||
)
|
||||
)
|
||||
|
||||
// Reachable (npub known) but relays down: the peer must not be
|
||||
// treated as promptly deliverable, or the router would skip the
|
||||
// courier and let the message rot in the Nostr send queue.
|
||||
#expect(transport.isPeerReachable(peerID))
|
||||
#expect(!transport.canDeliverPromptly(to: peerID))
|
||||
|
||||
connectivity.send(true)
|
||||
let deliverable = await TestHelpers.waitUntil(
|
||||
{ transport.canDeliverPromptly(to: peerID) },
|
||||
timeout: 5.0
|
||||
)
|
||||
#expect(deliverable)
|
||||
|
||||
connectivity.send(false)
|
||||
let undeliverable = await TestHelpers.waitUntil(
|
||||
{ !transport.canDeliverPromptly(to: peerID) },
|
||||
timeout: 5.0
|
||||
)
|
||||
#expect(undeliverable)
|
||||
}
|
||||
|
||||
@Test("Private message resolves short peer ID and emits decryptable packet")
|
||||
@MainActor
|
||||
func sendPrivateMessageResolvesShortPeerID() async throws {
|
||||
@@ -375,7 +421,8 @@ struct NostrTransportTests {
|
||||
currentIdentity: @escaping @MainActor () throws -> NostrIdentity? = { nil },
|
||||
registerPendingGiftWrap: @escaping @MainActor (String) -> Void = { _ in },
|
||||
sendEvent: @escaping @MainActor (NostrEvent) -> Void = { _ in },
|
||||
scheduleAfter: @escaping @Sendable (TimeInterval, @escaping @Sendable () -> Void) -> Void = { _, _ in }
|
||||
scheduleAfter: @escaping @Sendable (TimeInterval, @escaping @Sendable () -> Void) -> Void = { _, _ in },
|
||||
relayConnectivity: @escaping @MainActor () -> AnyPublisher<Bool, Never> = { Just(false).eraseToAnyPublisher() }
|
||||
) -> NostrTransport.Dependencies {
|
||||
NostrTransport.Dependencies(
|
||||
notificationCenter: notificationCenter,
|
||||
@@ -385,7 +432,8 @@ struct NostrTransportTests {
|
||||
currentIdentity: currentIdentity,
|
||||
registerPendingGiftWrap: registerPendingGiftWrap,
|
||||
sendEvent: sendEvent,
|
||||
scheduleAfter: scheduleAfter
|
||||
scheduleAfter: scheduleAfter,
|
||||
relayConnectivity: relayConnectivity
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user