From e999d57c19abd0953762249532778926c3959ba1 Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 2 Aug 2026 10:22:45 -0700 Subject: [PATCH 1/3] Count the simulated-mesh storm bound per packet type `publicMessageRelaysAcrossLineTopologyWithinTTLBudget` bounded every frame the mesh delivered, announces included. Announce *admission* is the one decision the manual scheduler does not own: `sendAnnounceNow` asks `announceThrottle.shouldSend(force:now: Date())` (BLEService.swift:2760), so announce volume tracks real elapsed seconds rather than `advanceTime`, and the forced-announce window is only 0.15s. #1651 has since made that harmless here, by draining the mesh to quiet before the baseline is taken: the announce traffic now lands in the baseline instead of in the measured delta. Re-measured on top of it, burning 0.2s of wall clock and changing nothing else: burn baseline total/announce delta total/announce/message none 66 / 22 4 / 0 / 4 0.2s early 80 / 36 4 / 0 / 4 0.2s late 66 / 22 4 / 0 / 4 The burn moves the baseline and leaves the delta alone. So the flake is fixed, and this is not the fix. What is left is legibility. The assertion now names the traffic it bounds, so a later change on the announce path cannot spend the message budget's headroom unnoticed. The message-only cost is 4 and is invariant under the perturbation above, so the budget is 8. Full SimulatedMeshTests suite green (macOS scheme, 8/8). Co-Authored-By: Claude Opus 5 (1M context) --- bitchatTests/Simulation/SimulatedMesh.swift | 24 +++++++++++++++++++ .../Simulation/SimulatedMeshTests.swift | 20 ++++++++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/bitchatTests/Simulation/SimulatedMesh.swift b/bitchatTests/Simulation/SimulatedMesh.swift index 6caf379d..c4bf6312 100644 --- a/bitchatTests/Simulation/SimulatedMesh.swift +++ b/bitchatTests/Simulation/SimulatedMesh.swift @@ -29,6 +29,29 @@ final class SimulatedMesh { private var pendingDeliveries: [(from: Int, packet: BitchatPacket)] = [] /// Total (packet, receiving-node) deliveries pumped — the storm bound. private(set) var deliveredFrameCount = 0 + /// The same deliveries split by packet type. A whole-mesh frame budget + /// silently includes announce traffic, and announce *admission* is the one + /// decision the manual scheduler does not own: `sendAnnounceNow` asks + /// `announceThrottle.shouldSend(force:now: Date())` (BLEService), so + /// whether a scheduled re-announce reaches the wire depends on real + /// elapsed seconds, not on `advanceTime`. Burn 0.2s of wall clock in the + /// middle of a test — the forced-announce window is 0.15s — and announce + /// deliveries rise (measured: 22 → 36) while every other type is + /// unchanged. A test that drains the mesh to quiet before it measures + /// absorbs that into its baseline; one that cannot has a budget moving + /// with runner load. Counting per type lets a test bound the traffic it + /// is actually about either way. + private var deliveredFrameCountsByType: [UInt8: Int] = [:] + + /// Deliveries of one packet type — the storm bound for a specific kind of + /// traffic, immune to unrelated announce frames. + /// Unlocked deliberately, matching `deliveredFrameCount` above: both + /// counters are written only by `pump` on the test thread (the outbound + /// tap touches `pendingDeliveries`/`emitted`, never these). Taking the + /// lock here would advertise a protection the writer does not hold. + func deliveredFrameCount(ofType type: MessageType) -> Int { + deliveredFrameCountsByType[type.rawValue] ?? 0 + } private(set) var nodes: [Node] = [] private var neighbors: [Set] = [] @@ -168,6 +191,7 @@ final class SimulatedMesh { for receiver in neighbors[from] { for link in links(from: from, at: receiver) { deliveredFrameCount += 1 + deliveredFrameCountsByType[packet.type, default: 0] += 1 nodes[receiver].service._test_ingestFrame(packet, link: link) } } diff --git a/bitchatTests/Simulation/SimulatedMeshTests.swift b/bitchatTests/Simulation/SimulatedMeshTests.swift index b4630076..512165d8 100644 --- a/bitchatTests/Simulation/SimulatedMeshTests.swift +++ b/bitchatTests/Simulation/SimulatedMeshTests.swift @@ -63,7 +63,8 @@ struct SimulatedMeshTests { // baseline snapshot depends on the draw — the budget assertion below // flaked on CI at 14 and 18 frames for exactly that reason. Advance // until the mesh goes a full window with no new frames, so the - // baseline only ever measures the message under test. + // baseline only ever measures the message under test. Quiescence is + // judged on every frame; the baseline itself is taken per type. var settled = mesh.deliveredFrameCount for _ in 0..<20 { mesh.advanceTime(by: 2) @@ -71,7 +72,7 @@ struct SimulatedMeshTests { if now == settled { break } settled = now } - let baseline = mesh.deliveredFrameCount + let baseline = mesh.deliveredFrameCount(ofType: .message) let capture = TransportEventCapture() c.service.eventDelegate = capture @@ -84,9 +85,18 @@ struct SimulatedMeshTests { let arrived = await capture.drainedPublicMessageCount(content: "hello line") == 1 #expect(arrived) - // Storm bound: a single public message across one relay hop must - // not multiply into more than a handful of frames. - #expect(mesh.deliveredFrameCount - baseline <= 12) + // Storm bound: a single public message across one relay hop must not + // multiply into more than a handful of frames. Counted per type so + // the budget measures relay fan-out and nothing else. The whole-mesh + // total also carries announces, and announce admission is the one + // decision the manual scheduler does not own — `sendAnnounceNow` asks + // `announceThrottle.shouldSend(force:now: Date())` — so announce + // volume tracks real elapsed seconds. The settle loop above keeps + // that traffic in the baseline rather than in the delta: burning 0.2s + // of wall clock before it moves the baseline 66 → 80 frames and + // leaves this delta at 4. Per-type counting is what makes the + // assertion say which traffic it bounds. + #expect(mesh.deliveredFrameCount(ofType: .message) - baseline <= 8) } @Test From 208dfd759272aae5fe192d8c36c1bdc6cf2fc515 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sun, 9 Aug 2026 15:06:41 +0530 Subject: [PATCH 2/3] fix(tests): drive announce admission from an injectable clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SimulatedMeshTests.publicMessageRelaysAcrossLineTopologyWithinTTLBudget` flakes in CI on PRs that never touch BLE or mesh code: (mesh.deliveredFrameCount - baseline → 16) <= 12 The harness advances only simulated time, but the announce-admission decision in `sendAnnounceNow` read wall-clock `Date()`. On a loaded runner the 0.15 s forced-minimum window elapses *between* announces the simulation places at a single instant, so announce-backs and afterglows that should have been throttled are admitted instead and fan out to every neighbour — 12 frames becomes 16. The bound was measuring machine speed, not relay behaviour. Give `BLEService` an injectable `dateProvider`, defaulted to `Date.init` the way `BLEIncomingFileStore` and the other services here already do, and read it at the admission gate. `BLEEngineManualScheduler` exposes the counter `advance(by:)` already moves as a `Date`, and `SimulatedMesh` hands that in, so the throttle window now moves with `advanceTime` and only with `advanceTime`. Production behaviour is unchanged: the default provider is `Date.init`, so every non-test caller keeps the wall clock it had. Adds `announceAdmissionFollowsSimulatedTimeNotWallClock`, which pins both halves on a lone node (nothing answers it, so every emitted announce is one its own throttle admitted): a second forced announce at the same simulated instant is refused, and one issued after the window has elapsed in simulated time is admitted. Also corrects the comments in the harness and in `_test_resetAnnounceThrottle` that described the window as wall-clock and unreachable from scheduler time. The reset stays: it is still how a test gets a second announce without advancing time and releasing other scheduled work. Fixes #1630 Fixes #1621 --- bitchat/Services/BLE/BLEService.swift | 23 +++++++--- .../Mocks/BLEEngineManualScheduler.swift | 11 +++++ bitchatTests/Simulation/SimulatedMesh.swift | 22 ++++++---- .../Simulation/SimulatedMeshTests.swift | 44 ++++++++++++++++++- 4 files changed, 83 insertions(+), 17 deletions(-) diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index 5f9b97af..e4fc6c1d 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -327,7 +327,14 @@ final class BLEService: NSObject { // Simple announce throttling private let announceThrottle = BLEAnnounceThrottle() - + /// Clock backing announce admission. Injectable so a test harness that + /// advances only simulated time can move the throttle window too; + /// production passes `Date.init`. Without this the admission decision + /// reads the runner's wall clock while the harness's clock stands still, + /// which makes the simulated-mesh storm bound a measure of machine speed + /// rather than relay behaviour. + private let dateProvider: () -> Date + // Application state tracking (thread-safe) #if os(iOS) var isAppActive: Bool = true // Assume active initially @@ -509,9 +516,11 @@ final class BLEService: NSObject { startSuspendedForPanicRecovery: Bool = false, noiseResponderHandshakeTimeout: TimeInterval = NoiseSecurityConstants.ordinaryResponderHandshakeTimeout, - engineScheduler: BLEEngineScheduling = BLEEngineDispatchScheduler() + engineScheduler: BLEEngineScheduling = BLEEngineDispatchScheduler(), + dateProvider: @escaping () -> Date = Date.init ) { self.engineScheduler = engineScheduler + self.dateProvider = dateProvider self.keychain = keychain self.idBridge = idBridge self.incomingFileStore = incomingFileStore @@ -2757,7 +2766,7 @@ final class BLEService: NSObject { // after this announce was scheduled but before it runs. guard !isPanicSuspended else { return } // Throttle announces to prevent flooding - if !announceThrottle.shouldSend(force: forceSend, now: Date()) { + if !announceThrottle.shouldSend(force: forceSend, now: dateProvider()) { return } @@ -3097,9 +3106,11 @@ extension BLEService { onEngine { sendAnnounceNow(forceSend: true) } } - /// Clears the announce throttle's wall-clock debt — the simulator's - /// stand-in for "enough real time has passed", since scheduler time - /// cannot move the throttle's Date-based window. Deliberately NOT + /// Clears the announce throttle's debt so an announce issued at the + /// same instant as the previous one is admitted. With an injected + /// clock the simulator can also just advance time past the window; + /// this stays for the cases that want a second announce *without* + /// moving time (and so releasing scheduled work). Deliberately NOT /// part of `_test_forceAnnounce`: the panic-rotation mesh test relies /// on the production panic path performing its own reset, and a /// blanket reset here would mask that regression. diff --git a/bitchatTests/Mocks/BLEEngineManualScheduler.swift b/bitchatTests/Mocks/BLEEngineManualScheduler.swift index cfe4d712..86844241 100644 --- a/bitchatTests/Mocks/BLEEngineManualScheduler.swift +++ b/bitchatTests/Mocks/BLEEngineManualScheduler.swift @@ -24,6 +24,17 @@ final class BLEEngineManualScheduler: BLEEngineScheduling, @unchecked Sendable { lock.withLock { pending.count } } + /// The same counter `advance(by:)` moves, exposed as a `Date` so + /// production code whose decisions are windowed on elapsed time (the + /// announce throttle) can be driven by test time instead of the + /// runner's speed. Anchored at a plausible wall-clock instant rather + /// than the epoch so any timestamp derived from it stays sane. + var currentDate: Date { + lock.withLock { Self.epoch.addingTimeInterval(now) } + } + + private static let epoch = Date(timeIntervalSince1970: 1_780_000_000) + /// Advances the clock, releasing due work in deadline order. /// Cancellation keeps its production semantics: dispatch skips a /// cancelled `DispatchWorkItem` at execution. diff --git a/bitchatTests/Simulation/SimulatedMesh.swift b/bitchatTests/Simulation/SimulatedMesh.swift index c4bf6312..2dc5186e 100644 --- a/bitchatTests/Simulation/SimulatedMesh.swift +++ b/bitchatTests/Simulation/SimulatedMesh.swift @@ -77,7 +77,11 @@ final class SimulatedMesh { idBridge: idBridge, identityManager: identityManager, initializeBluetoothManagers: false, - engineScheduler: scheduler + engineScheduler: scheduler, + // Announce admission reads this, so the throttle window moves + // with `advanceTime` instead of with however long the runner + // took to get here. + dateProvider: { scheduler.currentDate } ) let index = nodes.count let node = Node(service: service, scheduler: scheduler) @@ -91,9 +95,8 @@ final class SimulatedMesh { // The tap must be live before `setNickname` below: setNickname // force-announces asynchronously on the engine, and if that slot // ran in the gap before a later tap install, the announce was - // emitted invisibly while still stamping the wall-clock announce - // throttle — swallowing `announceAll`'s forced announce on a - // starved runner (the CI flake this ordering fixes). + // emitted invisibly while still stamping the announce throttle — + // swallowing `announceAll`'s forced announce. service._test_onOutboundPacket = { [weak self] packet in // Runs on the sender's engine; only buffer here — delivering // inline would nest one engine inside another. @@ -210,11 +213,12 @@ final class SimulatedMesh { /// Full discovery round: every node announces, traffic settles. /// - /// Resets each node's announce throttle first: the throttle window is - /// wall-clock, so any announce that already ran (setNickname's, in - /// `addNode`) would otherwise swallow this forced one whenever the two - /// land within the forced minimum interval — which is always, on any - /// runner. `forceAnnounce(from:)` deliberately does NOT reset — the + /// Resets each node's announce throttle first: `setNickname`'s announce + /// in `addNode` lands at the same simulated instant as this one, well + /// inside the forced minimum interval, and would otherwise swallow it. + /// (Advancing time would clear the window too, but it would also + /// release scheduled work that callers of `announceAll` have not asked + /// for yet.) `forceAnnounce(from:)` deliberately does NOT reset — the /// panic-rotation tests pin the production reset behavior through it. func announceAll() { for node in nodes { diff --git a/bitchatTests/Simulation/SimulatedMeshTests.swift b/bitchatTests/Simulation/SimulatedMeshTests.swift index 512165d8..3b6a6f7e 100644 --- a/bitchatTests/Simulation/SimulatedMeshTests.swift +++ b/bitchatTests/Simulation/SimulatedMeshTests.swift @@ -99,6 +99,45 @@ struct SimulatedMeshTests { #expect(mesh.deliveredFrameCount(ofType: .message) - baseline <= 8) } + /// Guards the storm bound above. Announce admission used to consult the + /// wall clock while the harness advanced only simulated time, so on a + /// loaded runner the 0.15 s forced-minimum window elapsed *between* + /// announces that the simulation places at one instant. The extra + /// announces that let through fanned out to every neighbour and pushed + /// `deliveredFrameCount` past the bound — the test measured machine + /// speed, not relay behaviour. Admission must move with simulated time + /// and only with simulated time. + /// + /// A lone node keeps this exact: nothing answers it, so every emitted + /// announce is one its own throttle admitted. + @Test + func announceAdmissionFollowsSimulatedTimeNotWallClock() { + let mesh = SimulatedMesh() + _ = mesh.addNode(nickname: "alice") + mesh.announceAll() + + func announceCount() -> Int { + mesh.emittedPackets(from: 0) + .filter { $0.type == MessageType.announce.rawValue } + .count + } + + // Same simulated instant: refused, however many real milliseconds + // the runner burned between `announceAll` and here. + let admitted = announceCount() + mesh.forceAnnounce(from: 0) + #expect(announceCount() == admitted) + + // Simulated time is the only thing that opens the window. The first + // advance drains any work already scheduled, so the second crosses + // the forced minimum interval with nothing else due. + mesh.advanceTime(by: 5) + mesh.advanceTime(by: TransportConfig.bleForceAnnounceMinIntervalSeconds + 0.01) + let beforeAdmitted = announceCount() + mesh.forceAnnounce(from: 0) + #expect(announceCount() == beforeAdmitted + 1) + } + @Test func duplicateFloodIsDeliveredOnce() async { let mesh = SimulatedMesh() @@ -154,7 +193,8 @@ struct SimulatedMeshTests { // A fresh announce over the (re-established) link binds and // reconnects — the same heal path a real reconnection drives. - // (The announce throttle runs on wall clock; model elapsed time.) + // (Clear the throttle rather than advance time: this announce must + // land without releasing any other scheduled work.) b.service._test_resetAnnounceThrottle() mesh.forceAnnounce(from: 1) mesh.settleUntil { @@ -312,7 +352,7 @@ struct SimulatedMeshTests { // Containment: further announces (and the rebind cooldown) leave // the healed binding alone — no flip-flop back to the dead ID. - // (Reset the wall-clock announce throttles so these actually send.) + // (Reset the announce throttles so these actually send.) b.service._test_resetAnnounceThrottle() mesh.forceAnnounce(from: 1) a.service._test_resetAnnounceThrottle()