fix(blockassembly): inverse ProcessConflicting in moveBackBlock — pick counter by first-seen CreatedAt#845
Merged
oskarszoon merged 22 commits intoMay 14, 2026
Conversation
…essConflicting effects
Reorg can leave the UTXO store with a Conflicting flag set on a tx that
SHOULD be the canonical chain spender, and cleared on a tx that is no
longer on the chain. Block assembly then keeps proposing the
no-longer-canonical tx (and any of its unmined descendants) and SVNode
rejects the candidate with bad-txns-inputs-missingorspent.
Sequence observed on teranode-mainnet-eu-1 (2026-05-10):
1. Stale block N_stale arrives first with TX_L, a tx that double-spends
an existing mempool tx TX_W. Validator stores TX_L with
Conflicting=true; subtree validation persists TX_L in the subtree
file's ConflictingNodes list.
2. moveForwardBlock(N_stale) reads ConflictingNodes from the subtree and
calls ProcessConflicting([TX_L]). GetCounterConflicting includes the
input txHash itself in its result set (process_conflicting.go:470),
so MarkConflictingRecursively flips both TX_L and TX_W (and TX_W's
descendants) to Conflicting=true. Phase 2 un-spends them all, phase 3
re-spends as TX_L, phase 4 clears TX_L.Conflicting. Net result:
TX_W=true, TX_L=false, parent.SpendingDatas[v]=TX_L.
3. Reorg fires: moveBackBlock(N_stale) + moveForwardBlock(N_winner) where
N_winner contains TX_W. moveBackBlock does NOT reverse the
ProcessConflicting side effects. N_winner's subtree was produced
upstream before the conflict materialized so its ConflictingNodes is
empty — moveForward's ProcessConflicting branch is skipped. UTXO
state stays inverted from step 2.
Fix: after a successful Reorg, scan the moveBackBlocks' subtree files for
non-empty ConflictingNodes. If any are found, trigger
b.reset(ctx, true) — the same input-validation reset already used on the
Reorg-failure and large-reorg paths. validateUnminedTxInputs Case 2
(BlockAssembler.go:2499) catches the inversion via
parent.ConflictingChildren containing a counter that's confirmed on the
current best chain, then markAsConflicting cascades the flag to the
unmined descendants. Subsequent mining candidates exclude the affected
txs and SVNode accepts the block.
The detection helper (hasConflictingNodesInBlocks) is deliberately
over-eager: missing a single bad reorg leaves block assembly producing
rejected candidates indefinitely, while a spurious validation reset only
reloads unmined transactions.
Recovery for nodes already in the bad state:
teranode-cli resetblockassembly --validate-inputs
The stale comment claiming "validateInputs after reorg is redundant" is
removed — reorgBlocks demonstrably misses conflicts when the winner-side
subtree was validated before the local UTXO conflict was detected.
Comment cited validateUnminedTxInputs at BlockAssembler.go:2459, but the function moved to 2539 (Case 2 at 2579) since the original write-up. File:line refs in same-file comments rot — function name + 'Case 2' is enough for a reader to locate it via grep.
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-05-13 13:22 UTC |
…ge cases Coverage gap on PR 845 was the new branch inside handleReorg — exercising it requires standing up a full BA with blockchain client / subtree processor / utxo store, which the existing setupBlockAssemblyTest helper doesn't reach into for a single-decision case. Extract the conditional reset call into repairConflictStateAfterReorg and unit-test it directly with a mocked blockchain client. Covers: - No-op when moveBackBlocks carry no ConflictingNodes (no blockchain call, no log). - Reset-error path: b.reset() errors at its first GetBestBlockHeader call, the function logs Errorf and returns — does not propagate the error upward (chain move already succeeded). Also extend hasConflictingNodesInBlocks tests for two more skip-paths: - nil subtreeHash entry inside block.Subtrees. - Corrupt subtree blob on disk (truncated file magic only, deserialize fails) — both with and without a healthy fallback subtree in the same block list. 11 → 13 sub-tests, no regression in the existing 383 BA tests.
…onflicting-not-reversed
…or the fix
The post-reorg b.reset(ctx, true) path was the wrong cure: when the new
chain's moveForwardBlocks carry their own ConflictingNodes, reset()
replays moveForward and ProcessConflicting precondition errors out
("tx is not conflicting") because the input txs were already promoted
on the original moveForward. This breaks every reorg whose new tip
carries ConflictingNodes, which includes the
TestComplexForkGrandparentConflict and TestLongestChainFork/with
double spend transaction sequential tests.
The correct fix is in moveBackBlock: inverse the side effects of the
original ProcessConflicting call so reverting a block whose subtree
listed ConflictingNodes also reverts the matching UTXO swap. Follow-up
commits add ReverseProcessConflicting and wire it into moveBackBlock.
Drops:
- repairConflictStateAfterReorg and hasConflictingNodesInBlocks
helpers on BlockAssembler
- reorg_conflicting_nodes_test.go covering those helpers
Restores the deleted "Only validate inputs when the Reorg itself
failed …" comment on the failing-reorg reset branch.
…onflicting-not-reversed
When a block whose subtree carried a non-empty ConflictingNodes list is
moved back during a reorg, the UTXO store has stale state left by the
original moveForward's ProcessConflicting call:
- parent.SpendingDatas[vout] points at the demoted tx (the block's
promoted winner), not the original mempool spender
- the original counter is still flagged Conflicting=true even though
it's the canonical spender once the block is gone
- the demoted tx is flagged Conflicting=false even though its
containing block has been removed from the chain
moveBackBlock previously only collected those conflicting hashes into
processedConflictingHashesMap (to relax the precondition for downstream
moveForward calls). The UTXO state was never actually reverted, so a
later mining candidate would keep including the demoted tx and SVNode
would reject it with bad-txns-inputs-missingorspent — the production
incident on teranode-mainnet-eu-1 on 2026-05-10.
ReverseProcessConflicting walks the demoted hashes; per input it:
1. Picks the counter from parent.ConflictingChildren that
(a) is not itself being demoted,
(b) is currently Conflicting=true (i.e. the original mempool
spender that was flipped to loser), and
(c) spends the same (parent, vout) as the demoted tx — guards
against false-positives where ConflictingChildren contains
sibling-output spenders that don't actually conflict.
2. Marks the demoted tx + descendants Conflicting=true via
MarkConflictingRecursively.
3. Unspends the demoted tx's input spends.
4. Spends the counter's tx (re-pointing parent.SpendingDatas[vout]
at the counter).
5. Clears Conflicting on the counter + descendants via the new
UnmarkConflictingRecursively (BFS mirror of
MarkConflictingRecursively).
Already-reversed demoted txs (Conflicting=true on entry) are skipped
for idempotency. CoinbasePlaceholderHashValue is short-circuited the
same way ProcessConflicting handles it.
reorgBlocks calls ReverseProcessConflicting inside the moveBack loop
right after moveBackBlock returns its conflictingHashes — so reverse
runs *before* the subsequent moveForward passes can re-apply
ProcessConflicting on the new tip's blocks. The
processedConflictingHashesMap population is preserved.
Tests:
- stores/utxo/reverse_process_conflicting_test.go covers happy path,
idempotent re-run, same-output filter, missing-counter, already-non-
conflicting counter, coinbase placeholder, empty input, get-error
propagation, and the un-cascade BFS.
- Existing sequential tests TestComplexForkGrandparentConflict* and
TestLongestChainFork/with_double_spend_transaction* now green again
on aerospike and postgres after the prior reset-trigger approach
was reverted.
…CreatedAt
The v2 reverse promoted every entry in parent.ConflictingChildren that
passed the same-output + Conflicting=true filter, then called
Spend()/UnmarkConflictingRecursively on all of them. Only one tx can be
the canonical spender of (parent, vout), so successive Spend calls
overwrote each other's SpendingDatas while every candidate ended up
Conflicting=false. In triple-fork / multi-counter scenarios the cascade
flipped txs that should remain conflicting, and the subsequent
moveForward's ProcessConflicting failed precondition. CI surfaced this
across 7+ sequential tests on the v2 push.
Per v3 review: the first-seen mempool spender is the canonical
"previous spender" hint the reverse was guessing at. CreatedAt is set
once at insert (Aerospike create.go:706, SQL inserted_at default), so
the lowest-CreatedAt entry in parent.ConflictingChildren is exactly the
tx an earlier ProcessConflicting demoted. Tiebreak on equal CreatedAt
is lexicographic hash compare for cross-node determinism.
Self-converging in chained reorgs: each reverse independently picks the
oldest, so a sequence like reverse([txC]); reverse([txB]) converges to
the canonical pre-fork mempool state. Subsequent reverses on already-
Conflicting=true demoted txs hit the existing idempotency guard and
no-op.
Plumbing:
- stores/utxo/meta/data.go: new CreatedAt int64 field, json
"createdAt,omitempty".
- stores/utxo/aerospike/get.go: populate Data.CreatedAt from the
existing createdAt bin when fields.CreatedAt is requested.
- stores/utxo/sql/sql.go:
- getUnbatched + batchDecorateChunk: SELECT inserted_at; populate
Data.CreatedAt via new parseInsertedAtMillis helper which handles
both Postgres (time.Time) and SQLite (string) return types.
- parseInsertedAtMillis itself: time.Time / []byte / string layouts,
returns 0 on unrecognised/legacy/nil so callers can fall through
to the isOlderCounter missing-timestamp branch.
- stores/utxo/process_conflicting.go:
- selectCountersForDemotedTx picks the single lowest-CreatedAt
candidate per input via the new isOlderCounter helper, deduping
across inputs.
- candidateMeta Get also asks for fields.CreatedAt now.
Cascade-skip in moveForward / reset paths
(services/blockassembly/subtreeprocessor/SubtreeProcessor.go):
- reorgBlocks adds ReverseProcessConflicting's full touched-hashes
return to processedConflictingHashesMap so downstream moveForward
sees them as already-handled.
- Both processConflictingTransactions and the reset() moveForward loop
filter conflictingNodes against the map before calling
ProcessConflicting — re-running ProcessConflicting on hashes already
swapped by the reverse double-spends parent UTXOs and fails. The
filter makes the swap idempotent across both directions of a
multi-fork reorg.
Tests added (stores/utxo/reverse_process_conflicting_test.go):
- TestReverseProcessConflicting_PicksOldestCounterByCreatedAt: two
candidates pass the same-output + Conflicting=true filter, oldest
wins, younger is left untouched.
- TestReverseProcessConflicting_TiebreakOnEqualCreatedAtByHash: equal
millis, lex-lower hash wins; pins determinism.
- TestReverseProcessConflicting_OldestCandidateInDemotedSetSkipped:
demoted-set filter runs before min selection.
- TestIsOlderCounter_OrdersByTimestampThenHash: spot-check on the
comparison helper, including the CreatedAt=0 legacy-record branch.
…ry-over After ReverseProcessConflicting demotes a tx (and its descendants), three things still left the in-memory mining candidate inconsistent with the new UTXO state: 1. moveBackBlockCreateNewSubtrees re-adds the moved-back block's txs to stp.chainedSubtrees BEFORE we run the reverse, so by the time the reverse flips them to Conflicting=true they're already settled in the in-memory subtree. The forward path's dequeueDuringBlockMovement only filters the queue / rebuilds via losingTxHashesMap — neither touches a tx that's already in chainedSubtrees. Mirror what BlockAssembler.markAsConflicting does on the validate-inputs path: after ReverseProcessConflicting returns the cascadedConflicting set, walk it and call stp.Remove on each hash. Counter-side (un-cascade) hashes are not evicted — they're now Conflicting=false and remain valid mempool entries. 2. ReverseProcessConflicting now returns (cascadedToConflicting, allTouched, error). cascadedToConflicting drives the in-memory eviction above; allTouched feeds processedConflictingHashesMap so the downstream moveForward's filter recognises both directions of the swap. 3. If the same tx is in BOTH a moveBackBlock's subtree.ConflictingNodes AND a moveForward block (e.g. testConflictingTxReorg's tx1Conflicting appears in 103a AND 103b), its canonical-spender status carries across the reorg — moveForward will handle it via its own ProcessConflicting and the reverse must not demote it. reorgBlocks now pre-computes moveForwardTxSet via collectMoveForwardTxHashes (walks moveForwardBlocks' subtree files) and filters those hashes out of the input to ReverseProcessConflicting. Symmetric subtree-file persist: - markConflictingTxsInSubtrees is also invoked on the reverse-cascade set so the "subtree.ConflictingNodes is the historical superset of every tx ever flagged conflicting" invariant holds — without it, only the moveForward path would persist, and the filter that short-circuits ProcessConflicting on reverse-cascade hashes would skip that persist too. Caller-side ProcessConflicting filter: - processConflictingTransactions and the reset() moveForward loop filter conflictingNodes against processedConflictingHashesMap before calling ProcessConflicting. Re-running ProcessConflicting on hashes the reverse already swapped fails on the "tx is not conflicting" precondition and double-spends parent UTXOs. The filter short- circuits gracefully; when it skips, it surfaces the cascade as both losingTxHashesMap (so processRemainderTxHashes drops the losers during chainedSubtrees rebuild) and conflictingSet (so the downstream dequeueDuringBlockMovement filters any queue items). SubtreeProcessor state: - New stp.reverseCascadedConflictingSet field scoped to a single reorgBlocks call. processConflictingTransactions reads it via stp.cloneReverseCascadedSet() when its filter short-circuits. Reset to nil on reorg exit. Test plumbing: - Updated all ReverseProcessConflicting call sites in tests for the new (cascadedToConflicting, allTouched, error) signature. Local verification: - go test ./services/blockassembly/... -count=1: 375 pass. - go test ./stores/utxo/... -count=1 -short: 832 pass across 14 pkgs. - go test ./test/sequentialtest/... -tags <full>: 125 pass.
…onflicting-not-reversed # Conflicts: # stores/utxo/sql/sql.go
…s + cloneReverseCascadedSet Sonar new-code coverage was 58.4% — the new reorg-reverse helpers were exercised by the sequential integration suite (postgres + aerospike) but not by unit tests, so sonar's coverage upload didn't see them. Add focused unit tests: - TestCollectMoveForwardTxHashes_BuildsTxSet (8 sub-tests): nil block list, no-subtrees block, nil block entry, nil subtree-hash entry, coinbase-placeholder exclusion, multi-block dedup, FileTypeSubtree → FileTypeSubtreeToCheck fallback, missing-subtree error, corrupt- blob error. Pins both the happy path and the failure-mode contract (errors surface, no silent skip). - TestCloneReverseCascadedSet_RoundTrips (1 test): nil outside an active reorg, populated copy independent of processor state, re-nil after reorg exit. Guards the in-place mutation dequeueDuringBlockMovement performs on the returned set. buildSubtreeWithTxs + mustSerialize helpers are local to the file. plainBlob wrapper isolates the test surface from blob.Store directly so future tests can drop in a different backend without touching this file's signatures. All 203 subtreeprocessor tests pass.
… in reorg_reverse_helpers_test.go depguard rejects stdlib `errors` — must use teranode `errors` package. The import was carrying a placeholder `closingReader` helper that was unused anyway; both removed. Also collapse the import block into a single group to satisfy gci default ordering (the rest of the package files don't separate teranode imports from third-party). CI lint now clean; prealloc warnings in SubtreeProcessor_test.go are pre-existing and unrelated.
… in processConflictingTransactions When the moveBack reverse already promoted a counter and demoted the matching ConflictingNodes hash, processConflictingTransactions filters those hashes out before calling ProcessConflicting. The short-circuit must still surface the reverse cascade as both losingTxHashesMap and conflictingSet so the chainedSubtrees rebuild and the next dequeueDuringBlockMovement evict the now-conflicting txs. Three sub-tests: - all conflicting nodes pre-processed + non-empty reverse cascade -> both returned - all pre-processed + nil reverse cascade -> both nil - empty input short-circuits without touching reverse cascade MockUtxostore with no ProcessConflicting expectation proves the filter skips the call. Could not extend this to a full sqlitememory reorg integration test: SetConflicting holds a sql.Tx and calls GetSpend on the same pool, which deadlocks on sqlite shared-cache (see comment in stores/utxo/tests/tests.go:670-676). The reorgBlocks reverse path is covered end-to-end by the 125 sequential integration tests (postgres + aerospike + sqlite).
…eInsertedAtMillis reverse_process_conflicting_test.go (+9 sub-tests): - DemotedMetaNilSkipsTx — pruned-record short-circuit - SelectCountersErrorPropagates — parent.ConflictingChildren lookup failure - MarkConflictingErrorPropagates — SetConflicting failure on demoted-side cascade - UnspendErrorPropagates — half-reversed-state guard - CounterMetaNilSkipsCounter — pruned counter mid-reverse - CounterSpendErrorPropagates — counter-promotion Spend failure - CounterUnmarkErrorPropagates — Unmark cascade failure - SelectCountersForDemotedTx_ParentMetaNilSkips — nil parent meta - SelectCountersForDemotedTx_CandidateGetErrorPropagates / CandidateMetaNilSkipsCandidate Coverage on stores/utxo/process_conflicting.go: - ReverseProcessConflicting 83.9% -> 96.4% - selectCountersForDemotedTx 85.3% -> 97.1% - UnmarkConflictingRecursively 95.2% -> 100% parse_inserted_at_test.go (new file, 9 sub-tests): pure-function tests for parseInsertedAtMillis: nil, time.Time, SQLite default layout, fractional seconds, RFC3339Nano, RFC3339, []byte, unparseable string, unrecognised type. Pins the per-type branch behaviour so a driver-shape change can't silently bypass the CreatedAt heuristic.
Four sub-tests: - single input -> single Spend record with correct TxID/Vout/SpendingData - multiple inputs -> records in input order with Vin matching input index - nil PreviousTxScript -> UTXOHashFromInput error propagates, nil slice - empty input slice -> empty Spends slice The error path matters: ReverseProcessConflicting uses spendsForTx on the demoted tx before un-spending its inputs. Silently dropping a bad input would leave parent.SpendingDatas[vout] pointing at the demoted tx — the exact failure mode this PR fixes for the canonical-spender swap.
icellan
approved these changes
May 12, 2026
…onflicting-not-reversed # Conflicts: # stores/utxo/process_conflicting.go
…t-reversed' into fix/reorg-process-conflicting-not-reversed
…sharpen Remove-failure comment Two review nits from Simon on reorgBlocks reverse-cascade eviction: 1. The 'h := hash' alias was redundant — chainhash.Hash is a value type, Remove takes a value not a pointer, no goroutine closes over h. Drop it. Now matches the peer site at BlockAssembler.go:2551. 2. Comment now explains why Warnf-and-continue is correct on Remove failure: the UTXO mutations are already committed and can't be rolled back from here; the realistic failure mode (race with concurrent eviction) lands in the desired end state; genuine corruption is recoverable via next reorg/reset or --validate-inputs. Failing loudly would not improve consistency.
…onflicting-not-reversed
Collaborator
|
Pre-approval requests — two items I'd like clarified or addressed before signing off. Both are about end-state correctness, not the happy path. The fix as it stands resolves the production incident; these are about residual edges. 1. Partial-reverse state on Spend/Unmark failure leaves parent permanently orphaned Walk-through inside
On any retry of the same reverse, Two options either of which I'd accept:
Either is fine — I just want to know which it is. 2. Counter-tx lifecycle after un-cascade — Counters were never in One line on the actual lifecycle in the comment would resolve this. Otherwise this is in good shape — the cumulative test pyramid (unit → package → sequentialtest across postgres/aerospike/sqlite) is solid, and the |
Two pre-approval issues from Simon's review of PR bsv-blockchain#845. 1. Partial-reverse state trap (ReverseProcessConflicting). The D.Conflicting=true short-circuit at the top of the loop trapped partial-reverse state. If a previous call succeeded at step 1 (Mark) and step 2 (Unspend) but failed at step 3 (Spend(C)), the next call would short-circuit on D.Conflicting=true and parent.SpendingDatas[vout] would stay empty — counter still Conflicting=true, parent orphaned. Replace the guard with isReverseFullyApplied: checks every input of D for parent.SpendingDatas[vout] pointing to a non-nil, non-D spender. If any input is nil (post-Unspend gap) or still points at D (Unspend never ran), the helper returns false and the loop falls through to re-run the steps to completion. The Mark and Unspend calls are idempotent at the store layer; the missing Spend(C)+Unmark(C) then runs and the partial state is cleared. Cost: one extra Get(parent, fields.Utxos) per demoted hash on the rare retry path. Happy path unchanged. 2. Counter-tx lifecycle comment (reorgBlocks reverse). Old comment claimed counter-side hashes 'remain valid mempool entries' which read like they'd flow back into mining naturally. They don't: counters were Conflicting=true before the reverse so they were never in chainedSubtrees, and the reverse only flips the flag — it doesn't stp.Add them. Mining candidate inclusion relies on the next validator re-feed (gossip) or operator RPC resubmit. The comment now spells this out explicitly, including the rationale for not calling stp.Add here (needs tx body in hand, risks adding txs whose parents are still missing post-reorg). Tests in stores/utxo/reverse_process_conflicting_test.go: - TestReverseProcessConflicting_SkipsAlreadyReversedDemoted updated to cover the new fully-applied confirmation Get. - TestReverseProcessConflicting_PartialStateRetryCompletes — drives the step-3-fails-then-retries flow end-to-end through the helper. - TestReverseProcessConflicting_DConflictingButParentStillPointsToD — pathological case where Mark succeeded but Unspend never ran. - TestIsReverseFullyApplied — unit tests of the predicate covering the non-D spender, nil slot, D-spender, short slice, missing parent, and store-error paths.
Contributor
Author
|
Both addressed in 1. Partial-reverse trap — went with Option 2 (parent-state guard). The Skipped Option 1 because the PR description explicitly says Skipped sub-option (reorder Spend(C) before Mark+Unspend) because Spend(C) would fail against a UTXO still spent by D unless we add an Cost: one extra
2. Counter-tx lifecycle — comment rewritten to spell out the actual state machine:
Documented the trade-off explicitly: alternative is 499 → 508 tests passing on touched packages. Vet + lint clean. |
Contributor
|
🤖 Claude Code Review Status: Complete Analysis SummaryThis PR addresses a critical production bug where reorgs left UTXO state inconsistent. The fix introduces FindingsNo critical issues identified. The implementation demonstrates careful attention to correctness and edge cases. Below are observations and minor suggestions: Logic & Correctness ✅Solid fundamentals:
Edge cases covered:
Test Coverage ✅Comprehensive testing:
Documentation 📝Strong inline documentation:
Minor observation: Recommendation✅ Approve for merge. The fix is well-designed, thoroughly tested, and addresses the root cause of the production issue. The implementation follows the project's careful-engineer philosophy from AGENTS.md. Review completed statically per security constraints for fork PRs |
|
This was referenced May 13, 2026
icellan
approved these changes
May 13, 2026
ordishs
approved these changes
May 14, 2026
oskarszoon
added a commit
that referenced
this pull request
May 14, 2026
…k counter by first-seen CreatedAt (#845)
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
A reorg can leave the UTXO store with
parent.SpendingDatas[vout]pointing at the no-longer-canonical tx, the original mempool spender stuck onConflicting=true, and the now-orphaned spender onConflicting=false. Block assembly keeps proposing the orphaned spender (and any of its unmined descendants), and SVNode rejects the candidate withbad-txns-inputs-missingorspent. Observed in production onteranode-mainnet-eu-1(eu-west-3 cluster) on 2026-05-10 — block assembly emitted invalid candidates for hours after a height-948435 reorg.Root cause: when a block whose subtree carries
ConflictingNodesis moved back during a reorg,moveBackBlockcollects those hashes intoprocessedConflictingHashesMap(to relax the precondition for the new tip'smoveForwards) but never undoes the UTXO-level side effects of the originalProcessConflicting. The chain move succeeds, the UTXO state stays inverted.Fix
Inverse
ProcessConflictinginmoveBackBlock, with three additional safeguards: a first-seenCreatedAtheuristic for picking the right counter in multi-fork scenarios, a carry-over filter that skips re-mined txs, and symmetric in-memory eviction so the mining candidate stays consistent with UTXO state.Core reverse —
stores/utxo/process_conflicting.goA new
utxo.ReverseProcessConflictingwalks the moved-back block'ssubtree.ConflictingNodesand, per demoted tx D and per input(parent, vout):Picks the single counter C from
parent.ConflictingChildrenwith the lowestCreatedAtthatConflicting=true, and(parent, vout).CreatedAtis set once at insert in both backends (Aerospikecreate.go:706, SQLinserted_atdefault), so the lowest-CreatedAtcandidate is the first-seen mempool spender — the canonical spender an earlierProcessConflictingdemoted. Tiebreak on equalCreatedAtis lexicographic hash compare for cross-node determinism. Legacy records with noCreatedAt(value 0) are treated as newer than any timestamped record so the heuristic never picks an unknown-vintage candidate over a known one.Marks D + descendants
Conflicting=trueviaMarkConflictingRecursively.Unspends D's input spends.
Spends C's tx, re-pointing
parent.SpendingDatas[vout]at C.Clears
Conflictingon C + descendants via the new symmetricUnmarkConflictingRecursively(BFS mirror ofMarkConflictingRecursively).Returns
(cascadedToConflicting, allTouched, error)— the cascade drives eviction;allTouchedfeedsprocessedConflictingHashesMapso the downstream moveForward recognises both directions of the swap.Idempotency: a demoted tx already on
Conflicting=trueshort-circuits.CoinbasePlaceholderHashValueshort-circuits the same wayProcessConflictingdoes.Self-converging on chained reorgs. With the lowest-
CreatedAtrule each reverse picks the original first-seen spender independently, so a sequence likereverse([txC]);reverse([txB])converges to the canonical pre-fork mempool state regardless of which intermediate forks the chain passed through.Carry-over filter —
reorgBlocksIf the same tx appears in BOTH a moveBackBlock's
subtree.ConflictingNodesAND a moveForward block (e.g. a conflicting tx that's mined on both branches at the same height), its canonical-spender status carries across the reorg — moveForward handles it via its ownProcessConflicting, and the reverse must not demote it.reorgBlockspre-computesmoveForwardTxSetviacollectMoveForwardTxHashes(walks moveForward subtrees) and filters those hashes out of the input toReverseProcessConflicting.Symmetric in-memory eviction
moveBackBlockCreateNewSubtreesre-adds the moved-back block's txs tostp.chainedSubtreesBEFORE the reverse runs. By the time the reverse flips them toConflicting=true, they're already settled in the in-memory subtree, and neither the forward path'sdequeueDuringBlockMovement(queue-only) norprocessRemainderTxHashes(rebuild vialosingTxHashesMaponly) touches them.After
ReverseProcessConflicting,reorgBlockswalkscascadedToConflictingand callsstp.Remove(ctx, h)on each — mirrors whatBlockAssembler.markAsConflictingalready does on the validate-inputs path (BlockAssembler.go:2535). Counter-side (un-cascade) hashes are intentionally NOT evicted; they're nowConflicting=falseand remain valid mempool entries.Subtree-file persist on the reverse cascade
markConflictingTxsInSubtreesis invoked oncascadedToConflictingso the "subtree.ConflictingNodes is the historical superset of every tx ever flagged conflicting" invariant holds. Without it, only the moveForward path would persist (via its ownProcessConflicting → markConflictingTxsInSubtrees), and the filter that short-circuitsProcessConflictingon reverse-cascade hashes would skip that persist too.Caller-side
ProcessConflictingfilterprocessConflictingTransactionsand thereset()moveForward loop filterconflictingNodesagainstprocessedConflictingHashesMapbefore callingProcessConflicting. Re-runningProcessConflictingon hashes the reverse already swapped fails on the"tx is not conflicting"precondition and double-spends parent UTXOs. When the filter short-circuits, it surfaces the cascade as bothlosingTxHashesMap(soprocessRemainderTxHashesdrops the losers during chainedSubtrees rebuild) andconflictingSet(so the downstreamdequeueDuringBlockMovementfilters any queue items).Plumbing
stores/utxo/meta/data.go— newCreatedAt int64onmeta.Data.stores/utxo/aerospike/get.go— populateCreatedAtfrom the existingcreatedAtbin whenfields.CreatedAtis requested.stores/utxo/sql/sql.go:getUnbatchedandbatchDecorateChunkselectinserted_atand populateCreatedAtviaparseInsertedAtMillis.parseInsertedAtMillishandles both Postgres (time.Time) and SQLite (string) return types; returns 0 on unrecognised values so legacy records flow throughisOlderCounter's missing-timestamp branch.services/blockassembly/subtreeprocessor/SubtreeProcessor.go:stp.reverseCascadedConflictingSetfield scoped to a singlereorgBlockscall.processConflictingTransactionsreads it viastp.cloneReverseCascadedSet()when its filter short-circuits.collectMoveForwardTxHashesbuilds the carry-over filter from moveForward subtree files.Production state mapping (mainnet incident)
Conflictingb0dfd9df(canonical-chain winner, block 948489)89ea7be9(orphaned-block spender, block 948488)89ea7be97b4f64db.SpendingDatas[1]89ea7be9(wrong)b0dfd9dfCreatedAt(b0dfd9df) < CreatedAt(89ea7be9)(mempool spender preceded the stale-block spender) — the heuristic picksb0dfd9dfunambiguously.Test plan
ReverseProcessConflicting(stores/utxo/reverse_process_conflicting_test.go):Geterror propagationUnmarkConflictingRecursivelyBFS cascadeCreatedAt(oldest wins, younger left conflicting)CreatedAtlex hash tiebreakisOlderCounterhelper including legacyCreatedAt=0branchgo test ./stores/utxo/... -count=1 -short— 832 pass across 14 packages.go test ./services/blockassembly/... -count=1— 375 pass.go test ./test/sequentialtest/... -tags <full>— 125 pass (postgres + aerospike + sqlite, all double-spend / longest-chain / large-tx-reorg / triple-fork scenarios green).go vet ./stores/utxo/... ./services/blockassembly/...— clean.Recovery for nodes already in the bad state
Until the fix is deployed:
Marks the unmined side conflicting via
validateUnminedTxInputsCase 2 cascade. Does NOT repairparent.SpendingDatason the winner side; this PR does.Review history
This PR went through several revisions in response to detailed reviews:
b.reset(ctx, true)trigger. Reverted — reset replaysProcessConflictingon the new tip'sConflictingNodes, fails precondition on already-promoted txs.ProcessConflictingthat promoted every candidate matching the same-output +Conflicting=truefilter. Broken on multi-counter scenarios —Spendcalls overwrote each other and every counter ended upConflicting=false.CreatedAtheuristic for counter selection. Lossless, no new schema, nobestBlockIDsMapplumbing. Integrated.processedConflictingHashesMapbefore callingProcessConflicting(vs changingProcessConflicting's precondition). Integrated.(cascadedToConflicting, allTouched); symmetric eviction viastp.Remove; carry-over filter for txs re-mined in moveForward; subtree-file persist on cascade. Integrated.Follow-up issues to file
(parent, vout)same-output check insidevalidateUnminedTxInputsCase 2 (BlockAssembler.go:2579) so--validate-inputsrecovery doesn't cascade-mark unrelated sibling-output spenders.SubtreeProcessor.resetvsGetRemoveMapLengthdata race surfaced in CI on an earlier PR revision.