Final re-grade fixes: parked EOSE fallback, panic atomicity, perf-gate calibration, serial benchmarks (#1336)

Final re-grade fixes: parked EOSE fallback, panic-reset atomicity, overflow visibility
This commit is contained in:
jack
2026-06-11 22:25:26 +02:00
committed by GitHub
7 changed files with 240 additions and 34 deletions
+20 -8
View File
@@ -10,6 +10,9 @@ jobs:
test:
name: Run Swift Tests (${{ matrix.name }})
runs-on: macos-latest
# A hung test must fail fast, not hold a runner for GitHub's 360-minute
# default (observed: intermittent app-suite hangs starving the queue).
timeout-minutes: 15
strategy:
fail-fast: false # Don't cancel other matrix jobs when one fails
@@ -39,17 +42,25 @@ jobs:
${{ runner.os }}-${{ matrix.name }}-
- name: Run Tests
# BITCHAT_PERF_LOG captures the PERF[...] lines that
# PerformanceBaselineTests reports (swift test --parallel swallows
# stdout of passing tests, so the floor gate reads this file instead).
# Perf benchmarks are excluded here and run in their own serial step
# below: measuring while parallel test processes contend for cores
# produces noisy numbers, and the XCTest measure machinery has hung
# intermittently under parallel workers on loaded runners.
env:
BITCHAT_PERF_LOG: ${{ github.workspace }}/perf-output.log
BITCHAT_SKIP_PERF_BASELINES: "1"
run: swift test --parallel --quiet --enable-code-coverage --package-path ${{ matrix.path }}
# Order-of-magnitude performance regression gate (app tests only — the
# package matrix entries write no PERF lines and the gate would skip
# anyway). Floors are deliberately generous (~25% of healthy local
# throughput, see bitchatTests/Performance/perf-floors.json) so this
# Benchmarks run serially on an otherwise idle runner for stable
# numbers; BITCHAT_PERF_LOG captures the PERF[...] lines for the gate.
- name: Run performance benchmarks (serial)
if: matrix.name == 'app'
timeout-minutes: 6
env:
BITCHAT_PERF_LOG: ${{ github.workspace }}/perf-output.log
run: swift test --quiet --filter PerformanceBaselineTests
# Order-of-magnitude performance regression gate. Floors are deliberately
# generous (see bitchatTests/Performance/perf-floors.json) so this
# catches algorithmic regressions, never runner variance.
- name: Performance floor gate
if: matrix.name == 'app'
@@ -76,6 +87,7 @@ jobs:
ios-build:
name: Build iOS app (simulator)
runs-on: macos-latest
timeout-minutes: 15
steps:
- name: Checkout code
+41 -1
View File
@@ -198,6 +198,9 @@ final class NostrRelayManager: ObservableObject {
}
private var messageQueue: [PendingSend] = []
private let messageQueueLock = NSLock()
// Total pending sends dropped at the queue cap; drives the sampled
// overflow warning (first + every Nth drop).
private var pendingSendDropCount = 0
private let encoder = JSONEncoder()
private var shouldUseTor: Bool { dependencies.userTorEnabled() }
@@ -352,6 +355,18 @@ final class NostrRelayManager: ObservableObject {
messageQueue.removeFirst(overflow)
}
messageQueueLock.unlock()
guard overflow > 0 else { return }
// Dropped events are ephemeral (presence/geo), so no status surfacing
// is needed but the drops should be visible. Sampled so a sustained
// relay stall can't flood the log.
pendingSendDropCount += overflow
if pendingSendDropCount == 1 ||
pendingSendDropCount.isMultiple(of: TransportConfig.nostrPendingSendDropLogInterval) {
SecureLogger.warning(
"📤 Relay send queue full — dropped \(pendingSendDropCount) oldest event(s)",
category: .session
)
}
}
/// Try to flush any queued messages for relays that are now connected.
@@ -452,7 +467,7 @@ final class NostrRelayManager: ObservableObject {
if urls.isEmpty {
onEOSE()
} else if shouldWaitForTorBeforeConnecting {
pendingEOSECallbacks[id] = onEOSE
parkEOSECallbackUntilTorReady(id: id, callback: onEOSE)
} else {
startEOSETracking(id: id, relayURLs: Set(urls), callback: onEOSE)
}
@@ -623,6 +638,31 @@ final class NostrRelayManager: ObservableObject {
}
}
/// Park an EOSE callback while Tor is not yet ready, and schedule the same
/// fallback timeout `startEOSETracking` uses. Without it, a parked callback
/// would only be unblocked by Tor-readiness retry exhaustion (several
/// awaitReady timeouts, i.e. minutes), leaving callers hanging far past the
/// normal EOSE fallback. If Tor recovers first the callback is promoted to
/// a real EOSE tracker (`startPendingEOSETrackingIfNeeded`), and if retry
/// exhaustion fires first it is drained by `unblockPendingEOSECallbacks`;
/// either way it leaves `pendingEOSECallbacks` and this timer is a no-op.
private func parkEOSECallbackUntilTorReady(id: String, callback: @escaping () -> Void) {
pendingEOSECallbacks[id] = callback
let generation = connectionGeneration
dependencies.scheduleAfter(TransportConfig.nostrSubscriptionEOSEFallbackSeconds) { [weak self] in
Task { @MainActor [weak self] in
guard let self else { return }
// Stale timers from a previous connection generation are void.
guard generation == self.connectionGeneration else { return }
// Already fired (unsubscribe, retry-exhaustion unblock) or
// promoted to a real EOSE tracker: nothing to do.
guard let callback = self.pendingEOSECallbacks.removeValue(forKey: id) else { return }
SecureLogger.warning("Unblocking Tor-parked EOSE callback for \(id) after fallback timeout", category: .session)
callback()
}
}
}
/// Fire and clear all EOSE callbacks that are parked waiting for Tor.
/// Callers treat EOSE as "initial fetch finished"; firing with no data is
/// safe and prevents indefinite hangs when Tor cannot bootstrap.
+37 -12
View File
@@ -74,6 +74,8 @@ final class BLEService: NSObject {
private let identityManager: SecureIdentityStateManagerProtocol
private let keychain: KeychainManagerProtocol
private let idBridge: NostrIdentityBridge
/// Binary form of `myPeerID`; same contract mutated only inside a
/// `messageQueue` barrier via `refreshPeerIdentity()`.
private var myPeerIDData: Data = Data()
// MARK: - Advertising Privacy
@@ -315,13 +317,20 @@ final class BLEService: NSObject {
}
disconnectNotifyDebouncer.removeAll()
noiseService.clearEphemeralStateForPanic()
noiseService.clearPersistentIdentity()
// The crypto-service replacement and the derived identity swap must be
// one atomic unit with respect to messageQueue senders: a queued send
// must never observe the new Noise service alongside the old peer ID
// (it would sign with the new identity while carrying the old sender).
// refreshPeerIdentity() executes inline here via its re-entrancy check.
messageQueue.sync(flags: .barrier) {
noiseService.clearEphemeralStateForPanic()
noiseService.clearPersistentIdentity()
let newNoise = NoiseEncryptionService(keychain: keychain)
noiseService = newNoise
configureNoiseServiceCallbacks(for: newNoise)
refreshPeerIdentity()
let newNoise = NoiseEncryptionService(keychain: keychain)
noiseService = newNoise
configureNoiseServiceCallbacks(for: newNoise)
refreshPeerIdentity()
}
restartGossipManager()
setNickname(currentNickname)
@@ -407,8 +416,10 @@ final class BLEService: NSObject {
// MARK: Identity
/// Derived from the Noise identity fingerprint; rotated only via
/// `refreshPeerIdentity()` (e.g. panic reset). Externally read-only
/// no out-of-band mutation may bypass that derivation.
/// `refreshPeerIdentity()` (e.g. panic reset), which performs the swap
/// inside a `messageQueue` barrier so concurrent queue work never sees a
/// half-updated identity. Externally read-only no out-of-band mutation
/// may bypass that derivation.
private(set) var myPeerID = PeerID(str: "")
/// Externally read-only; mutate via `setNickname(_:)`, which also
/// broadcasts the change to peers.
@@ -2370,11 +2381,25 @@ extension BLEService {
}
}
/// Swaps `myPeerID`/`myPeerIDData` to match the current Noise identity.
/// The swap runs as a `messageQueue` barrier so in-flight work items that
/// read the identity (e.g. `sendMessage` building packets) complete
/// against the old value and everything after sees the new one atomically.
/// Callers (init, panic reset on the main thread) are never on
/// `messageQueue`; the re-entrancy check keeps any future on-queue caller
/// from deadlocking.
private func refreshPeerIdentity() {
let fingerprint = noiseService.getIdentityFingerprint()
myPeerID = PeerID(str: fingerprint.prefix(16))
myPeerIDData = Data(hexString: myPeerID.id) ?? Data()
meshTopology.reset()
let swap = {
let fingerprint = self.noiseService.getIdentityFingerprint()
self.myPeerID = PeerID(str: fingerprint.prefix(16))
self.myPeerIDData = Data(hexString: self.myPeerID.id) ?? Data()
self.meshTopology.reset()
}
if DispatchQueue.getSpecific(key: messageQueueKey) != nil {
swap()
} else {
messageQueue.sync(flags: .barrier, execute: swap)
}
}
+3
View File
@@ -167,6 +167,9 @@ enum TransportConfig {
// bootstrap deadline) to attempt before unblocking pending EOSE callers.
static let nostrTorReadyMaxWaitAttempts: Int = 3
static let nostrPendingSendQueueCap: Int = 200
// Sample interval for the send-queue overflow warning (first + every Nth
// dropped event). Drops are ephemeral presence/geo traffic log-only.
static let nostrPendingSendDropLogInterval: Int = 10
// Pending (not-yet-flushed) REQs are bounded per relay: oldest-by-insertion
// eviction at the cap, plus an age sweep on connect attempts. Durable
// subscription intent survives in subscriptionRequestState either way.
+17
View File
@@ -164,6 +164,23 @@ struct BLEServiceCoreTests {
#expect(!ble._test_recordIngressIfNew(packet: packet, linkID: "central-b"))
}
@Test
func panicReset_rotatesPeerIDDerivedFromNewNoiseFingerprint() async throws {
let ble = makeService()
let originalPeerID = ble.myPeerID
let originalFingerprint = ble.noiseIdentityFingerprint()
#expect(originalPeerID == PeerID(str: originalFingerprint.prefix(16)))
ble.resetIdentityForPanic(currentNickname: "anon")
// The Noise identity is regenerated and the peer ID swaps with it
// (atomically, behind a messageQueue barrier).
let newFingerprint = ble.noiseIdentityFingerprint()
#expect(newFingerprint != originalFingerprint)
#expect(ble.myPeerID != originalPeerID)
#expect(ble.myPeerID == PeerID(str: newFingerprint.prefix(16)))
}
@Test
func modifiedServices_rediscoverWhenBitChatServiceIsInvalidated() async throws {
let otherService = CBUUID(string: "0000180F-0000-1000-8000-00805F9B34FB")
+29 -13
View File
@@ -2,11 +2,14 @@
"_philosophy": [
"Floor throughputs for the PERF[...] lines printed by PerformanceBaselineTests.",
"Floors catch algorithmic regressions (O(n) -> O(n^2), accidental sync I/O,",
"quadratic re-scans), NOT tuning noise: each floor is deliberately set at",
"~25% of the throughput measured on a local dev machine (2026-06, Apple",
"Silicon), leaving ~4x headroom for CI runner variance so the gate never",
"flakes on a slow runner while still failing loudly on order-of-magnitude",
"regressions.",
"quadratic re-scans), NOT tuning noise. Basis: ~50% of the SLOWEST observed",
"CI run (GitHub macos-latest), not local numbers - CI slowdown is",
"benchmark-dependent (sub-millisecond passes amplify runner overhead, e.g.",
"nostrInbound.duplicate runs at ~20% of local speed on CI while most",
"benchmarks run at ~40-60%). Every floor remains 10-200x above known",
"regression values (the pre-optimization duplicate path measured 2.2k/sec",
"against a 250k floor), so the gate still fails loudly on order-of-",
"magnitude regressions while never flaking on a slow runner.",
"Raise floors deliberately after intentional performance improvements;",
"lower them only with a written justification in the PR. If a benchmark is",
"renamed or removed, update this file in the same change - the gate fails",
@@ -28,16 +31,29 @@
"store.audit": 362
},
"floors": {
"nostrInbound.fresh": 530,
"nostrInbound.duplicate": 600000,
"nostrInbound.fresh": 450,
"nostrInbound.duplicate": 250000,
"bleInbound.roundTripAndDedup": 9500,
"gcs.buildAndDecode": 190,
"delivery.incrementalUpdate": 43000,
"delivery.storeUpdate": 39000,
"formatting.formatMessage": 3000,
"pipeline.privateIngest": 6000,
"pipeline.publicIngest": 3200,
"store.append": 53000,
"store.audit": 90
"formatting.formatMessage": 2200,
"pipeline.privateIngest": 3000,
"pipeline.publicIngest": 2400,
"store.append": 48000,
"store.audit": 70
},
"_slowest_observed_ci_numbers_2026_06": {
"nostrInbound.fresh": 918,
"nostrInbound.duplicate": 524924,
"bleInbound.roundTripAndDedup": 20489,
"gcs.buildAndDecode": 504,
"delivery.incrementalUpdate": 110892,
"delivery.storeUpdate": 96531,
"formatting.formatMessage": 4575,
"pipeline.privateIngest": 6388,
"pipeline.publicIngest": 5006,
"store.append": 97423,
"store.audit": 140
}
}
}
@@ -133,6 +133,99 @@ final class NostrRelayManagerTests: XCTestCase {
XCTAssertTrue(context.sessionFactory.requestedURLs.isEmpty)
}
func test_subscribe_parkedEOSEFiresAfterFallbackTimeoutWhenTorNeverBecomesReady() async {
let relayURL = "wss://tor-eose-parked-fallback.example"
let context = makeContext(permission: .denied, userTorEnabled: true, torEnforced: true, torIsReady: false)
var eoseCount = 0
context.manager.subscribe(
filter: makeFilter(),
id: "tor-parked-fallback",
relayUrls: [relayURL],
handler: { _ in },
onEOSE: { eoseCount += 1 }
)
// Parking the callback schedules the normal EOSE fallback, not just
// the Tor retry-exhaustion unblock (~minutes later).
XCTAssertEqual(context.scheduler.scheduled.first?.delay, TransportConfig.nostrSubscriptionEOSEFallbackSeconds)
XCTAssertEqual(eoseCount, 0)
context.scheduler.runNext()
let unblocked = await waitUntil { eoseCount == 1 }
XCTAssertTrue(unblocked)
XCTAssertTrue(context.sessionFactory.requestedURLs.isEmpty)
// A later retry-exhaustion unblock must not fire the callback again.
for _ in 0..<TransportConfig.nostrTorReadyMaxWaitAttempts {
context.torWaiter.resolve(false)
}
try? await Task.sleep(nanoseconds: 20_000_000)
XCTAssertEqual(eoseCount, 1)
}
func test_subscribe_parkedEOSEFallbackIsNoOpWhenTorRecoversFirst() async throws {
let relayURL = "wss://tor-eose-parked-recover.example"
let context = makeContext(permission: .denied, userTorEnabled: true, torEnforced: true, torIsReady: false)
var eoseCount = 0
context.manager.subscribe(
filter: makeFilter(),
id: "tor-parked-recover",
relayUrls: [relayURL],
handler: { _ in },
onEOSE: { eoseCount += 1 }
)
// Tor recovers before the fallback fires; the callback is promoted to
// a real EOSE tracker when the subscription flushes.
context.torWaiter.resolve(true)
let subscribed = await waitUntil {
context.sessionFactory.latestConnection(for: relayURL)?.sentStrings.count == 1
}
XCTAssertTrue(subscribed)
// The parked fallback (scheduled first) is now a no-op.
context.scheduler.runNext()
try? await Task.sleep(nanoseconds: 20_000_000)
XCTAssertEqual(eoseCount, 0)
// The real EOSE still completes the initial load exactly once...
try context.sessionFactory.latestConnection(for: relayURL)?.emitEOSE(subscriptionID: "tor-parked-recover")
let eoseCompleted = await waitUntil { eoseCount == 1 }
XCTAssertTrue(eoseCompleted)
// ...and the promoted tracker's own fallback does not double-fire.
context.scheduler.runNext()
try? await Task.sleep(nanoseconds: 20_000_000)
XCTAssertEqual(eoseCount, 1)
}
func test_subscribe_parkedEOSEFallbackIsNoOpAfterRetryExhaustionUnblock() async {
let relayURL = "wss://tor-eose-parked-exhausted.example"
let context = makeContext(permission: .denied, userTorEnabled: true, torEnforced: true, torIsReady: false)
var eoseCount = 0
context.manager.subscribe(
filter: makeFilter(),
id: "tor-parked-exhausted",
relayUrls: [relayURL],
handler: { _ in },
onEOSE: { eoseCount += 1 }
)
// Retry exhaustion unblocks the parked callback first.
for _ in 0..<TransportConfig.nostrTorReadyMaxWaitAttempts {
context.torWaiter.resolve(false)
}
XCTAssertEqual(eoseCount, 1)
// The parked fallback finds nothing to do.
context.scheduler.runNext()
try? await Task.sleep(nanoseconds: 20_000_000)
XCTAssertEqual(eoseCount, 1)
}
func test_sendEvent_survivesFailedTorWaitAndSendsWhenTorRecovers() async throws {
let relayURL = "wss://tor-send-retry.example"
let context = makeContext(permission: .denied, userTorEnabled: true, torEnforced: true, torIsReady: false)