From 9ae440be6afbad1d8dc57b83c95d3f57ce93b235 Mon Sep 17 00:00:00 2001 From: jack Date: Mon, 6 Jul 2026 21:12:52 +0200 Subject: [PATCH] Geohash: serialize PoW sends so order matches send order Two location-channel sends back-to-back only cancelled the previous mining task and started a new one. Cancellation merely *expedites* NIP-13 mining (the target is polled and steps down; it never aborts the send), so the cancelled task still appended + relayed once mining returned. Both tasks ran concurrently and the second (shorter to mine) could finish first, reordering messages in the timeline and on relays. Chain the mining tasks: each geohash send captures the previous send's task, cancels it (to expedite, so delays never stack), and awaits its completion before it echoes and relays. Order is now always send order. The >2s mining cap is preserved: cancellation expedites the awaited task, so a send is never blocked beyond NostrPoW.miningTimeCap. Test: two rapid sends where the first mines longer (larger content) still land in send order for both the local echo and the relayed events. Co-Authored-By: Claude Fable 5 --- .../ViewModels/ChatOutgoingCoordinator.swift | 14 +++++++-- .../ChatOutgoingCoordinatorContextTests.swift | 31 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/bitchat/ViewModels/ChatOutgoingCoordinator.swift b/bitchat/ViewModels/ChatOutgoingCoordinator.swift index b53de6c9..8d8b6a9e 100644 --- a/bitchat/ViewModels/ChatOutgoingCoordinator.swift +++ b/bitchat/ViewModels/ChatOutgoingCoordinator.swift @@ -157,10 +157,18 @@ private extension ChatOutgoingCoordinator { let teleported = context.isTeleported let nickname = context.nickname - // A newer send expedites the previous one's mining so sends keep - // their order and mining delays never stack. - geohashMiningTask?.cancel() + // Serialize geohash sends: each send awaits the previous send's task + // before it appends + relays, so user-visible order always matches + // send order even when an earlier message mines longer than a later + // one. Cancelling the previous task only *expedites* its mining (the + // NIP-13 target is polled, not aborted), so it still finishes and + // sends — and it finishes fast, so awaiting it never stacks mining + // delays or blocks a send beyond `NostrPoW.miningTimeCap`. + let previousSend = geohashMiningTask + previousSend?.cancel() geohashMiningTask = Task { @MainActor [weak context = self.context] in + await previousSend?.value + let event: NostrEvent do { event = try await NostrProtocol.createMinedEphemeralGeohashEvent( diff --git a/bitchatTests/ChatOutgoingCoordinatorContextTests.swift b/bitchatTests/ChatOutgoingCoordinatorContextTests.swift index e8d11baa..25aa8300 100644 --- a/bitchatTests/ChatOutgoingCoordinatorContextTests.swift +++ b/bitchatTests/ChatOutgoingCoordinatorContextTests.swift @@ -218,4 +218,35 @@ struct ChatOutgoingCoordinatorContextTests { #expect(context.appendedPublicMessages.count == 1) #expect(context.sentGeohashContexts.count == 1) } + + @Test @MainActor + func sendMessage_onLocationChannel_serializesRapidSendsInSendOrder() async { + let context = MockChatOutgoingContext() + let coordinator = ChatOutgoingCoordinator(context: context) + let channel = GeohashChannel(level: .city, geohash: "u4pruydq") + context.activeChannel = .location(channel) + + // Two back-to-back sends. The first carries much larger content, so + // its NIP-13 mining hashes a bigger event per attempt and runs longer + // than the second's. Without serialization the second (faster) task + // could finish first and reorder both the local timeline and the + // relayed events. The coordinator chains the mining tasks — each send + // awaits the previous send's task before it echoes and relays — so the + // visible order must always match the send order. + let first = "first " + String(repeating: "x", count: 4000) + let second = "second" + coordinator.sendMessage(first) + coordinator.sendMessage(second) + + // The stored task is the second send, which awaits the first. + await coordinator.geohashMiningTask?.value + await drainMainActorTasks() + + // Local echoes land in send order… + #expect(context.appendedPublicMessages.map(\.message.content) == [first, second]) + // …and so do the relayed events (IDs match the echoes 1:1, in order). + #expect(context.sentGeohashContexts.count == 2) + #expect(context.sentGeohashContexts.map(\.event.id) + == context.appendedPublicMessages.map(\.message.id)) + } }