nik/bitchat
1
0
Fork 0
mirror of https://github.com/permissionlesstech/bitchat.git synced 2026-08-29 07:27:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Courier spray: enforce the offer span instead of describing it

Browse Source 
The scan-to-commit span in offerSprayCopies was held by a comment asking
callers not to suspend inside it. Addresses the #1438 review ask.

Two things now hold it. The rebase onto main supplied the first for free:
notifyUI takes a non-async @MainActor closure, so `await` inside the sole
caller's block is a compile error rather than a review catch. That is a
property of that call site, not of the method, so beginSprayOfferSpan adds a
debug-build detector for a future caller reached from some other context.

An isolation assertion would have been the wrong check. MainActor.assertIsolated
needs iOS 17 / macOS 14 and both packages pin 16 / 13; it would fail every
existing test, since CourierStoreTests is not MainActor-isolated; and the
violation that would actually ship is a suspension inside an already-MainActor
block, which it passes. Overlap is the real predicate.

The flag covers transferSprayCopies too — the two paths spend from one budget,
so a transfer overlapping an offer overcommits exactly as two offers would.

Restructuring the API was considered and rejected: no moment exists where the
caller holds every courier (announces arrive one peer at a time), and the store
still loops `accepting` per copy, so the invariant would only move down a level.
Moving the commit inside the store queue closes the race but inverts a rule this
file states twice, and would hold the store queue across radio I/O. Neither is
worth it for a gap the type system now blocks at the only call site.

Both guards are proven by mutation: dropping either span call fails its test,
and the first attempt at this proof removed the wrong one and stayed green,
which is how the transfer-path case got written.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ecgang 2026-07-26 11:32:28 -07:00
parent bd0d1a3188
commit 77b31f2188
2 changed files with 138 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
bitchat/Services/Courier/CourierStore.swift
View File

@ -150,6 +150,52 @@ final class CourierStore {
/// from a deleted generation can never inflate a new deposit's budget.
private var pendingSprayOffers: [PendingSprayOfferKey: PendingSprayOffer] = [:]
private let queue = DispatchQueue(label: "chat.bitchat.courier.store")
#if DEBUG
/// True while a spray offer is between its scan and its last commit.
///
/// Both spray paths spend from one budget and both release the store queue
/// across `accepting` (they must: it enters BLE/collections queues). The
/// scan is therefore only sound if offers do not overlap — see the caller
/// invariant on `offerSprayCopies`. Guarded rather than asserted on actor
/// isolation, because the violation that would actually ship is a
/// suspension *inside* an already-MainActor block, which every isolation
/// check passes.
private var sprayOfferInFlight = false
/// Test seam for the overlap detector, mirroring `_test_onOutboundPacket`
/// in `BLEService`. Unset in normal debug runs, where an overlap trips
/// `assertionFailure` instead.
static var _test_onSprayOfferOverlap: (() -> Void)?
#endif
/// Marks the start of a spray offer's scan-to-commit span, reporting an
/// overlap with one already in flight. No-op in release builds.
private func beginSprayOfferSpan(_ function: StaticString = #function) {
#if DEBUG
queue.sync {
guard sprayOfferInFlight else {
sprayOfferInFlight = true
return
}
if let hook = Self._test_onSprayOfferOverlap {
hook()
} else {
assertionFailure("""
\(function) overlapped another spray offer. Copies are on the \
wire before either commit runs, so the second scan reads a \
budget the first has already spent and the excess ships \
uncharged. Offers must run to completion one at a time.
""")
}
}
#endif
}
private func endSprayOfferSpan() {
#if DEBUG
queue.sync { sprayOfferInFlight = false }
#endif
}
private let fileURL: URL?
private let now: () -> Date
private let readData: (URL) throws -> Data
@ -417,6 +463,11 @@ final class CourierStore {
}
var acceptedCount = 0
// Same span rule as `offerSprayCopies` — both spend from one budget and
// both release the queue across `accepting`, so an overlap between them
// inflates copies exactly as an overlap within either one would.
beginSprayOfferSpan()
defer { endSprayOfferSpan() }
for copy in offered where accepting(copy) {
// As with direct handover, BLE acceptance runs outside the store
// queue. Revalidate and commit the exact budget that left this
@ -495,11 +546,27 @@ final class CourierStore {
// commit below. `accepting` puts copies on the wire irreversibly, so a
// commit that then loses its revalidation leaves those copies uncharged
// — two couriers each offered `copies / 2` before either commits would
// put 6 copies out from a budget of 4. What prevents it is that the sole
// caller (`BLEService.sprayCourierMail`) runs scan/send/commit inside a
// single `Task { @MainActor }` containing no `await`, so the actor's
// executor runs it to completion. Adding an `await` anywhere in that
// block reopens this. See `concurrentOffersToDifferentCouriersConserveCopies`.
// put 6 copies out from a budget of 4.
//
// Two things hold it, in order of strength:
//
// 1. The sole caller (`BLEService.sprayCourierMail`) runs scan/send/
// commit inside `notifyUI`, whose closure is a NON-ASYNC
// `@MainActor () -> Void`. `await` inside it is a compile error, so
// at that call site the invariant is enforced by the type system,
// not by this comment.
// 2. `beginSprayOfferSpan` traps an overlap in debug builds, which
// covers a future caller reached from some other context — the type
// guarantee above is a property of that one call site, not of this
// method.
//
// No lock here can substitute: the copies are already on the wire
// before either commit runs, so the revalidation below protects the
// ledger and nothing else.
// See `concurrentOffersToDifferentCouriersConserveCopies` and
// `overlappingSprayOffersAreDetected`.
beginSprayOfferSpan()
defer { endSprayOfferSpan() }
for copy in offered where accepting(copy) {
// As with `transferSprayCopies`, BLE acceptance runs outside the
// store queue. Revalidate and commit the exact budget that left this

66
bitchatTests/CourierStoreTests.swift
View File

@ -733,6 +733,72 @@ struct CourierStoreTests {
#expect(offerAll(store, to: courierB) == 2)
}
#if DEBUG
/// The scan-to-commit span guard.
///
/// Note this test's sibling below runs its two offers *in turn*, which is
/// safe and is what production does. This one nests the second offer inside
/// the first's accept closure — the one interleaving that breaks the
/// accounting, because `accepting` runs outside the store queue, so B's scan
/// reads a budget A has not yet spent while A's copies are already on the
/// wire.
///
/// Production cannot reach this today: the sole caller runs inside
/// `notifyUI`, whose closure is a non-async `@MainActor () -> Void`, so
/// `await` there is a compile error. That is precisely why the guard needs a
/// test — nothing else would notice a future caller reintroducing the gap.
@Test func overlappingSprayOffersAreDetected() {
let store = makeStore()
let recipientKey = Data(repeating: 0xB0, count: 32)
let envelope = makeEnvelope(recipientKey: recipientKey).withCopies(8)
#expect(store.deposit(envelope, from: depositorA))
let courierA = Data(repeating: 0xC1, count: 32)
let courierB = Data(repeating: 0xC2, count: 32)
let overlaps = OverlapCounter()
CourierStore._test_onSprayOfferOverlap = { overlaps.count += 1 }
defer { CourierStore._test_onSprayOfferOverlap = nil }
_ = store.offerSprayCopies(to: courierA) { _ in
// Copies for A are on the wire; B now scans before A has committed.
_ = store.offerSprayCopies(to: courierB) { _ in true }
return true
}
#expect(overlaps.count == 1)
}
/// The same guard across the two spray paths, which is the reason one flag
/// covers both: `transferSprayCopies` and `offerSprayCopies` spend from a
/// single budget, so a transfer started inside an offer's accept closure
/// overcommits exactly as two offers would.
@Test func sprayTransferOverlappingAnOfferIsDetected() {
let store = makeStore()
let recipientKey = Data(repeating: 0xB0, count: 32)
let envelope = makeEnvelope(recipientKey: recipientKey).withCopies(8)
#expect(store.deposit(envelope, from: depositorA))
let courierA = Data(repeating: 0xC1, count: 32)
let courierB = Data(repeating: 0xC2, count: 32)
let overlaps = OverlapCounter()
CourierStore._test_onSprayOfferOverlap = { overlaps.count += 1 }
defer { CourierStore._test_onSprayOfferOverlap = nil }
_ = store.offerSprayCopies(to: courierA) { _ in
_ = store.takeSprayCopies(for: courierB)
return true
}
#expect(overlaps.count == 1)
}
/// Box so the detector's escaping closure can tally without capturing a
/// local `var`.
private final class OverlapCounter: @unchecked Sendable {
var count = 0
}
#endif
@Test func concurrentOffersToDifferentCouriersConserveCopies() {
let store = makeStore()
let recipientKey = Data(repeating: 0xB0, count: 32)