Skip to content

fix: Mark losing conflicting transactions as NOT on longest chain during reorg#1

Merged
sugh01 merged 1 commit intosugh01:test/longest-chain-double-spendfrom
icellan:test/longest-chain-double-spend
Jan 9, 2026
Merged

fix: Mark losing conflicting transactions as NOT on longest chain during reorg#1
sugh01 merged 1 commit intosugh01:test/longest-chain-double-spendfrom
icellan:test/longest-chain-double-spend

Conversation

@icellan
Copy link

@icellan icellan commented Jan 9, 2026

Summary

Fixes a critical bug in blockchain reorganization where losing conflicting transactions were not being marked as NOT on longest chain in the UTXO store.

The Problem

During blockchain reorganizations in SubtreeProcessor.reorgBlocks():

  1. ✅ Conflicting transactions that lost were correctly removed from block assembly
  2. But they were never marked as NOT on longest chain in the UTXO store
  3. This left them with UnminedSince = 0, incorrectly appearing as mined

The Solution

  • Modified moveForwardBlock() to return losingTxHashesMap alongside transactionMap
  • Implemented two-pass processing in reorgBlocks():
    • Pass 1: Process all moveForward blocks, build winningTxSet and collect losing transactions
    • Pass 2: Filter losing transactions (exclude winners from same block due to GetCounterConflicting behavior), then remove losers from markOnLongestChain to handle transactions that win in an earlier block but lose in a later block
  • Updated all 3 call sites of moveForwardBlock() to handle the new return value

Test Changes

Fixed testLongestChainWithDoubleSpendTransaction:

  • Added parentTx2 to both Fork A (block5a) and Fork B (block5b) so tx3 dependencies exist on both chains
  • Removed impossible REORG 2 scenario where tx3 would be mined on Fork A after tx2 already consumed the shared UTXO (parentTx output 1)
  • Updated test assertions and comments to reflect correct Bitcoin UTXO semantics

Test Results

Aerospike (Original Failing Backend): ✅ All tests passing

  • simple ✅
  • invalid_block ✅
  • invalid_block_with_old_tx ✅
  • fork_with_different_tx_inclusion ✅
  • transaction_chain_dependency ✅

SQLite/Postgres: Mixed results with some timeouts (separate infrastructure issue)

  • Key "partial_output_consumption" test passes on all backends ✅

Files Modified

  • services/blockassembly/subtreeprocessor/SubtreeProcessor.go
  • services/blockassembly/subtreeprocessor/SubtreeProcessor_test.go
  • test/sequentialtest/longest_chain/longest_chain_test.go
  • test/sequentialtest/longest_chain/03_longest_chain_invalidate_fork_test.go (gci formatting fix)

🤖 Generated with Claude Code

…ing reorg

During blockchain reorganizations in SubtreeProcessor.reorgBlocks(), conflicting
transactions that lost to newly-mined transactions were being removed from block
assembly but never marked as NOT on longest chain in the UTXO store, leaving them
with UnminedSince = 0.

Changes:
- Modified moveForwardBlock() to return losingTxHashesMap alongside transactionMap
- Updated reorgBlocks() with two-pass processing:
  * Pass 1: Collect winning and losing transactions from all moveForward blocks
  * Pass 2: Filter and mark transactions appropriately, removing losers from winners
- Fixed test scenario in testLongestChainWithDoubleSpendTransaction to ensure
  parentTx2 is mined in both forks, making tx3 valid on both chains
- Removed impossible test scenario where tx3 would be mined after tx2 already
  consumed the same UTXO

This ensures proper UTXO state tracking during chain reorganizations and fixes
failures in the longest_chain test suite, particularly on Aerospike backend.

Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
@sugh01 sugh01 merged commit 318bf21 into sugh01:test/longest-chain-double-spend Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants