test(blockassembly/subtreeprocessor): add clock seam for deterministic tests#841
Merged
liam merged 3 commits intoMay 11, 2026
Merged
Conversation
Adds an unexported queueClock seam on LockFreeQueue so tests can stamp
batches with a controlled time. Default is realQueueClock{} which calls
time.Now(); production behaviour is unchanged.
Verified zero-cost on the per-batch ingestion hot path (queue.go) via
BenchmarkQueue with -benchtime=10s -count=30: +0.5 ns/op, p=0.625 -
indistinguishable from noise.
…cessor Promotes the clock interface to a package-level type (clock / realClock, replacing queue-local queueClock / realQueueClock) and threads it through SubtreeProcessor so the two validFromMillis calculations - in the Start dequeue loop and in dequeueDuringBlockMovement - read through stp.clock. Tests can now install one fake clock and have both batch timestamps (LockFreeQueue) and DoubleSpendWindow boundary calculations honour it. No behaviour change in production. Verified zero-cost on BenchmarkQueue same-day same-machine: 170.4n -> 170.3n, p=0.566 (n=10 x 10s).
Use the new clock seam to characterize the queue's validFromMillis filter
deterministically, without time.Sleep. Five tests / nine subtests pin:
* Start-loop vs dequeueDuringBlockMovement asymmetry at
DoubleSpendWindow=0 (default). The Start loop's zero-guard
short-circuits the filter; the drain formula does not, so
same-millisecond batches get held back during block-movement drains.
Pinned at the formula level (queue_test.go) and at the real call
site (conflicting_queue_race_test.go) with a +1ms control case
that mirrors the existing time.Sleep(5ms) workaround at
conflicting_queue_race_test.go:75 deterministically.
* Inclusive-reject semantics at batch.time == validFromMillis, and
admission one millisecond below the boundary.
* Negative validFromMillis short-circuits the > 0 guard and disables
filtering. Dormant in production (Now().UnixMilli() is in the
trillions) but worth pinning so a future caller or test built on
time.Time{} does not silently lose double-spend protection.
* Clock-backward jump under NTP correction holds batches until the
drain clock recovers past batch.time + window.
Contributor
|
🤖 Claude Code Review Status: Complete Current Review: Summary: Strengths:
|
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-05-11 11:51 UTC |
|
icellan
approved these changes
May 11, 2026
freemans13
approved these changes
May 11, 2026
This was referenced May 12, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces an unexported
clockinterface inservices/blockassembly/subtreeprocessor, injected intoLockFreeQueueandSubtreeProcessor. The production default isrealClock{}(callstime.Now()); tests install a fake to drive deterministic timestamps withouttime.Sleep.Three
time.Now()call sites are now mediated by the seam:queue.go:63-batch.timeon enqueueSubtreeProcessor.go:812-validFromMillisin the Start-loop dequeueSubtreeProcessor.go:3789-validFromMillisindequeueDuringBlockMovementFive new tests / nine subtests in
queue_test.goandconflicting_queue_race_test.gouse the seam to characterize the queue'svalidFromMillisfilter at queue.go:96 (boundary semantics, negative-cutoff bypass, clock-backstep behaviour, and the asymmetry between the twovalidFromMillisformulas atDoubleSpendWindow=0).Motivation
blockassemblyhas time-dependent correctness logic gated byDoubleSpendWindow. The existing test workaround for this istime.Sleep(conflicting_queue_race_test.go:75sleeps 5ms before callingdequeueDuringBlockMovement). A clock seam replaces wall-time waits with deterministic time, which is the prerequisite for any property/invariant testing of reorgs and drain ordering.Perf gate
The hot path is
LockFreeQueue.enqueueBatch(one timestamp per batch, called from every producer). Benchmarked on this machine with-count=30 -benchtime=10sbefore merging, and again same-day same-machine after the SubtreeProcessor seam was added:main)No measurable regression. Interface dispatch at the timestamp call is free here.
Latent finding (not addressed in this PR)
While writing the characterization tests, the two
validFromMillisformulas diverge atDoubleSpendWindow == 0(the documented default)::807-813) - zero-guarded:validFromMillis = 0→ queue filter off.dequeueDuringBlockMovement(:3789) - unconditional:validFromMillis = Now().UnixMilli()→ filter active, rejects same-ms batches.conflicting_queue_race_test.go:71-75papers over this with a 5ms sleep. The tests in this PR pin the current (asymmetric) behaviour. I'll open a follow-up to align the drain formula with the Start-loop zero-guard once this seam lands.Test plan
go vet ./services/blockassembly/subtreeprocessor/- cleango test -race -count=1 ./services/blockassembly/subtreeprocessor/- pass (153s)go test -race -count=1 ./services/blockassembly/- pass (102s)staticcheckandgolangci-lintare pinned to Go 1.25.5 locally vs the repo's Go 1.26.0 toolchain - pre-existing tooling mismatch, expecting CI to run clean.