test(blockassembly/subtreeprocessor): reproduce #852 moveForwardBlock drain loss#856
Merged
liam merged 2 commits intoMay 14, 2026
Merged
Conversation
…veForwardBlock drain loss Adds TestMoveForwardBlockDrainLoss_BatchesLostOnPostDrainError - a deterministic, tests-only reproduction of the rollback gap described in issue bsv-blockchain#852. Setup: * errOnCreateUtxoStore wraps a real sqlitememory utxo store via interface embedding and overrides Create to return a sentinel error. All other methods delegate to the embedded store. This injects a step-6 failure (processCoinbaseUtxos -> utxoStore.Create) AFTER step-5 has already drained the queue. * Clocks pinned via fixedClock so DoubleSpendWindow=0 admits the enqueued batch at drain time (consistent with the bsv-blockchain#846 fix). * SubtreeProcessor built without Start so the event-loop goroutine cannot drain the batch concurrently. moveForwardBlock is invoked directly (lowercase) and the production rollback (lines 711-717) is replayed inline by the test. What the test demonstrates: 1. Pre-call: queue contains one enqueued batch. 2. moveForwardBlock errors with the sentinel from the Create wrapper. 3. After the inline rollback, the snapshotted in-memory fields (chainedSubtrees, currentSubtree, currentTxMap, currentBlockHeader) are restored. 4. The queue is empty: processRemainderTransactionsAndDequeue drained the batch into the now-discarded new subtree state. 5. The drained tx hash is in neither the queue nor any subtree -> the batch is silently lost. This test PINS the current buggy behaviour. When the underlying fix lands (option 1, 2, or 3 from the issue), the assertion at queue length 0 will need to flip to require.Equal(t, preLen, postLen) and require.Contains for the tx hash in the queue. Test plan: - go vet ./services/blockassembly/subtreeprocessor/ - clean - go test -race -count=1 -run TestMoveForwardBlockDrainLoss ./services/blockassembly/subtreeprocessor/ - pass (~7s) - go test -race -count=1 ./services/blockassembly/subtreeprocessor/ - pass (~155s) Refs bsv-blockchain#852.
Contributor
|
🤖 Claude Code Review Status: Complete Current Review: Analysis:
|
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-05-14 08:35 UTC |
…entation gofmt/gci flagged the numbered list in the repro test's doc comment for non-canonical indentation (3-space vs 2-space). Reformat to gofmt's canonical list shape so CI passes.
|
oskarszoon
approved these changes
May 14, 2026
freemans13
approved these changes
May 14, 2026
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
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
Adds
TestMoveForwardBlockDrainLoss_BatchesLostOnPostDrainError- a deterministic, tests-only reproduction of the rollback gap described in #852.Refs #852.
What the test does
errOnCreateUtxoStorewraps a realsqlitememoryutxo store via interface embedding and overridesCreateto return a sentinel error. All other methods delegate. This injects a step-6 failure (processCoinbaseUtxos->utxoStore.Create) after step-5 has already drained the queue.fixedClocksoDoubleSpendWindow=0admits the enqueued batch at drain time (consistent with the fix(blockassembly/subtreeprocessor): zero-guard validFromMillis in dequeueDuringBlockMovement #846 fix).SubtreeProcessorbuilt withoutStartso the event-loop goroutine cannot drain the batch concurrently.moveForwardBlockis invoked directly and the production rollback atSubtreeProcessor.go:711-717is replayed inline.Output
The test pins this current (buggy) behaviour as a regression guard. When the fix lands (any of the three remediation options from #852), the queue-length assertion will flip to
require.Equal(t, preLen, postLen)and an additionalrequire.Containswill assert the batch is still in the queue.Test plan
go vet ./services/blockassembly/subtreeprocessor/- cleango test -race -count=1 -run TestMoveForwardBlockDrainLoss ./services/blockassembly/subtreeprocessor/- pass (~7s)go test -race -count=1 ./services/blockassembly/subtreeprocessor/- pass (~155s)