Skip to content

fix(validator): require BlockIDs+!Conflicting+!Locked before ErrTxNotFound shortcut#770

Open
ordishs wants to merge 1 commit intobsv-blockchain:mainfrom
ordishs:security/4568-tx-not-found-shortcut
Open

fix(validator): require BlockIDs+!Conflicting+!Locked before ErrTxNotFound shortcut#770
ordishs wants to merge 1 commit intobsv-blockchain:mainfrom
ordishs:security/4568-tx-not-found-shortcut

Conversation

@ordishs
Copy link
Copy Markdown
Collaborator

@ordishs ordishs commented Apr 28, 2026

Closes #4568.

Summary

When spendUtxos returns ErrTxNotFound (parent missing), the validator looked up the current tx in the UTXO store and, if found, returned (meta, nil) with a warning "assuming already blessed" — without checking whether the metadata actually confirmed prior validation. During a reorg or DAH-eviction window, a tx lookup can succeed while its parent is temporarily absent; the shortcut would silently accept the tx on the basis of its own existence rather than a verified prior validation.

Fix

Gate the shortcut on three conditions that together imply prior full validation:

  1. len(BlockIDs) > 0 — tx has been included in at least one block.
  2. !Conflicting — tx is not flagged as conflicting.
  3. !Locked — tx is not locked (e.g., from a conflict aftermath).

If any condition fails, surface the original ErrTxNotFound. Use a local metaErr so the outer err (still ErrTxNotFound) survives the metadata lookup for the fall-through error path.

Also tightened MockUtxostore.GetMeta so Return(error) now correctly surfaces a lookup failure instead of panicking on a *meta.Data cast.

Test plan

  • Shortcut allowed when BlockIDs non-empty + !Conflicting + !Locked.
  • Shortcut denied when BlockIDs empty.
  • Shortcut denied when Conflicting.
  • Shortcut denied when Locked.
  • Shortcut denied when metadata lookup itself fails.
  • go test -race ./services/validator/... passes.
  • go test -race ./stores/utxo/... passes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026 •
edited
Loading

🤖 Claude Code Review

Status: Complete

No issues found.

The fix properly gates the ErrTxNotFound shortcut with three conditions that collectively confirm prior full validation. The logic is sound, test coverage is comprehensive (5 scenarios), and the mock fix correctly handles error returns. The implementation addresses the security concern described in #4568 where a tx could be accepted based on its mere existence rather than verified validation state.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-770 (79ef767)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 142
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.667µ 1.830µ ~ 0.400
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 61.90n 61.58n ~ 0.100
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 61.62n 61.38n ~ 0.200
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 61.64n 61.60n ~ 0.700
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 30.68n 30.09n ~ 0.600
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 51.93n 51.36n ~ 1.000
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 107.4n 107.8n ~ 1.000
MiningCandidate_Stringify_Short-4 263.6n 264.3n ~ 0.400
MiningCandidate_Stringify_Long-4 1.915µ 1.907µ ~ 0.600
MiningSolution_Stringify-4 979.2n 972.5n ~ 0.100
BlockInfo_MarshalJSON-4 1.771µ 1.752µ ~ 0.100
NewFromBytes-4 131.3n 130.6n ~ 0.800
Mine_EasyDifficulty-4 58.67µ 59.13µ ~ 0.400
Mine_WithAddress-4 5.072µ 4.844µ ~ 0.100
BlockAssembler_AddTx-4 0.02603n 0.02613n ~ 1.000
AddNode-4 10.70 10.17 ~ 0.200
AddNodeWithMap-4 10.58 10.28 ~ 1.000
DirectSubtreeAdd/4_per_subtree-4 62.07n 60.35n ~ 0.700
DirectSubtreeAdd/64_per_subtree-4 29.02n 28.26n ~ 0.400
DirectSubtreeAdd/256_per_subtree-4 27.16n 27.01n ~ 0.200
DirectSubtreeAdd/1024_per_subtree-4 26.12n 26.13n ~ 1.000
DirectSubtreeAdd/2048_per_subtree-4 25.80n 25.62n ~ 0.200
SubtreeProcessorAdd/4_per_subtree-4 312.6n 308.2n ~ 0.100
SubtreeProcessorAdd/64_per_subtree-4 313.3n 307.8n ~ 0.400
SubtreeProcessorAdd/256_per_subtree-4 308.6n 307.9n ~ 0.700
SubtreeProcessorAdd/1024_per_subtree-4 314.1n 309.9n ~ 0.200
SubtreeProcessorAdd/2048_per_subtree-4 312.0n 312.6n ~ 1.000
SubtreeProcessorRotate/4_per_subtree-4 318.2n 312.5n ~ 0.200
SubtreeProcessorRotate/64_per_subtree-4 313.3n 316.8n ~ 0.200
SubtreeProcessorRotate/256_per_subtree-4 311.9n 312.3n ~ 0.400
SubtreeProcessorRotate/1024_per_subtree-4 301.3n 304.0n ~ 0.700
SubtreeNodeAddOnly/4_per_subtree-4 65.98n 66.45n ~ 0.700
SubtreeNodeAddOnly/64_per_subtree-4 39.35n 39.23n ~ 1.000
SubtreeNodeAddOnly/256_per_subtree-4 37.43n 38.32n ~ 0.100
SubtreeNodeAddOnly/1024_per_subtree-4 37.04n 37.33n ~ 0.400
SubtreeCreationOnly/4_per_subtree-4 164.8n 164.0n ~ 0.400
SubtreeCreationOnly/64_per_subtree-4 634.4n 648.7n ~ 0.400
SubtreeCreationOnly/256_per_subtree-4 1.990µ 1.986µ ~ 1.000
SubtreeCreationOnly/1024_per_subtree-4 4.791µ 4.736µ ~ 0.700
SubtreeCreationOnly/2048_per_subtree-4 7.954µ 8.841µ ~ 0.700
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 302.6n 306.5n ~ 0.700
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 310.0n 307.0n ~ 0.200
ParallelGetAndSetIfNotExists/1k_nodes-4 957.8µ 966.7µ ~ 1.000
ParallelGetAndSetIfNotExists/10k_nodes-4 1.912m 1.922m ~ 0.100
ParallelGetAndSetIfNotExists/50k_nodes-4 8.718m 8.828m ~ 0.400
ParallelGetAndSetIfNotExists/100k_nodes-4 17.24m 17.30m ~ 0.700
SequentialGetAndSetIfNotExists/1k_nodes-4 756.0µ 752.3µ ~ 0.100
SequentialGetAndSetIfNotExists/10k_nodes-4 2.954m 2.908m ~ 0.400
SequentialGetAndSetIfNotExists/50k_nodes-4 10.71m 10.60m ~ 0.400
SequentialGetAndSetIfNotExists/100k_nodes-4 20.39m 20.18m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 1.051m 1.022m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 4.721m 4.710m ~ 1.000
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 19.22m 19.20m ~ 1.000
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 831.3µ 842.6µ ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 6.025m 6.019m ~ 1.000
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 39.50m 40.89m ~ 0.100
DiskTxMap_SetIfNotExists-4 3.650µ 3.666µ ~ 1.000
DiskTxMap_SetIfNotExists_Parallel-4 3.398µ 3.381µ ~ 0.400
DiskTxMap_ExistenceOnly-4 304.2n 305.4n ~ 1.000
Queue-4 194.5n 192.2n ~ 0.100
AtomicPointer-4 4.472n 4.169n ~ 0.100
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 859.0µ 855.4µ ~ 1.000
ReorgOptimizations/DedupFilterPipeline/New/10K-4 847.2µ 815.9µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 120.2µ 116.8µ ~ 0.400
ReorgOptimizations/AllMarkFalse/New/10K-4 62.31µ 62.33µ ~ 1.000
ReorgOptimizations/HashSlicePool/Old/10K-4 60.15µ 64.65µ ~ 0.200
ReorgOptimizations/HashSlicePool/New/10K-4 11.21µ 11.15µ ~ 0.400
ReorgOptimizations/NodeFlags/Old/10K-4 5.516µ 5.313µ ~ 1.000
ReorgOptimizations/NodeFlags/New/10K-4 1.845µ 1.867µ ~ 1.000
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 10.49m 10.28m ~ 0.700
ReorgOptimizations/DedupFilterPipeline/New/100K-4 10.068m 9.941m ~ 0.400
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.201m 1.143m ~ 0.100
ReorgOptimizations/AllMarkFalse/New/100K-4 679.8µ 679.9µ ~ 1.000
ReorgOptimizations/HashSlicePool/Old/100K-4 654.6µ 739.3µ ~ 0.100
ReorgOptimizations/HashSlicePool/New/100K-4 314.4µ 313.6µ ~ 0.400
ReorgOptimizations/NodeFlags/Old/100K-4 57.11µ 55.47µ ~ 0.100
ReorgOptimizations/NodeFlags/New/100K-4 20.02µ 19.70µ ~ 0.100
TxMapSetIfNotExists-4 51.34n 51.13n ~ 0.700
TxMapSetIfNotExistsDuplicate-4 38.30n 38.35n ~ 1.000
ChannelSendReceive-4 627.4n 631.7n ~ 0.700
CalcBlockWork-4 505.7n 501.8n ~ 0.700
CalculateWork-4 675.1n 719.6n ~ 0.200
BuildBlockLocatorString_Helpers/Size_10-4 1.670µ 1.334µ ~ 0.100
BuildBlockLocatorString_Helpers/Size_100-4 12.70µ 12.66µ ~ 0.400
BuildBlockLocatorString_Helpers/Size_1000-4 125.1µ 135.9µ ~ 0.100
CatchupWithHeaderCache-4 104.0m 104.1m ~ 0.700
_BufferPoolAllocation/16KB-4 2.853µ 2.411µ ~ 0.700
_BufferPoolAllocation/32KB-4 5.195µ 6.361µ ~ 0.200
_BufferPoolAllocation/64KB-4 11.46µ 11.57µ ~ 1.000
_BufferPoolAllocation/128KB-4 21.60µ 23.38µ ~ 0.100
_BufferPoolAllocation/512KB-4 86.01µ 83.07µ ~ 0.700
_BufferPoolConcurrent/32KB-4 12.10µ 14.24µ ~ 0.100
_BufferPoolConcurrent/64KB-4 19.19µ 22.20µ ~ 0.100
_BufferPoolConcurrent/512KB-4 105.0µ 115.5µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/16KB-4 483.7µ 481.3µ ~ 1.000
_SubtreeDeserializationWithBufferSizes/32KB-4 481.3µ 483.1µ ~ 1.000
_SubtreeDeserializationWithBufferSizes/64KB-4 486.1µ 483.6µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/128KB-4 477.4µ 486.3µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/512KB-4 475.9µ 481.1µ ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/16KB-4 27.42m 28.48m ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/32KB-4 27.49m 28.68m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/64KB-4 27.16m 27.87m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/128KB-4 27.39m 27.51m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/512KB-4 27.07m 27.58m ~ 0.100
_PooledVsNonPooled/Pooled-4 641.9n 651.2n ~ 0.100
_PooledVsNonPooled/NonPooled-4 5.181µ 5.401µ ~ 0.100
_MemoryFootprint/Current_512KB_32concurrent-4 5.538µ 5.964µ ~ 0.400
_MemoryFootprint/Proposed_32KB_32concurrent-4 6.978µ 8.945µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 6.673µ 7.740µ ~ 0.100
_prepareTxsPerLevel-4 396.7m 394.6m ~ 0.700
_prepareTxsPerLevelOrdered-4 3.563m 3.520m ~ 1.000
_prepareTxsPerLevel_Comparison/Original-4 401.6m 409.0m ~ 0.400
_prepareTxsPerLevel_Comparison/Optimized-4 3.423m 3.592m ~ 0.100
SubtreeSizes/10k_tx_4_per_subtree-4 1.271m 1.271m ~ 1.000
SubtreeSizes/10k_tx_16_per_subtree-4 302.7µ 304.3µ ~ 0.700
SubtreeSizes/10k_tx_64_per_subtree-4 72.33µ 71.56µ ~ 0.400
SubtreeSizes/10k_tx_256_per_subtree-4 17.75µ 17.99µ ~ 0.100
SubtreeSizes/10k_tx_512_per_subtree-4 8.807µ 8.856µ ~ 0.400
SubtreeSizes/10k_tx_1024_per_subtree-4 4.377µ 4.401µ ~ 0.700
SubtreeSizes/10k_tx_2k_per_subtree-4 2.166µ 2.222µ ~ 0.100
BlockSizeScaling/10k_tx_64_per_subtree-4 68.99µ 69.93µ ~ 0.700
BlockSizeScaling/10k_tx_256_per_subtree-4 17.43µ 17.47µ ~ 1.000
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.340µ 4.329µ ~ 1.000
BlockSizeScaling/50k_tx_64_per_subtree-4 367.2µ 367.3µ ~ 0.700
BlockSizeScaling/50k_tx_256_per_subtree-4 87.02µ 87.58µ ~ 0.700
BlockSizeScaling/50k_tx_1024_per_subtree-4 21.22µ 21.46µ ~ 0.700
SubtreeAllocations/small_subtrees_exists_check-4 147.2µ 146.7µ ~ 1.000
SubtreeAllocations/small_subtrees_data_fetch-4 157.8µ 156.6µ ~ 0.400
SubtreeAllocations/small_subtrees_full_validation-4 307.8µ 308.9µ ~ 0.700
SubtreeAllocations/medium_subtrees_exists_check-4 8.663µ 8.754µ ~ 0.700
SubtreeAllocations/medium_subtrees_data_fetch-4 9.152µ 9.263µ ~ 0.400
SubtreeAllocations/medium_subtrees_full_validation-4 17.24µ 17.67µ ~ 0.100
SubtreeAllocations/large_subtrees_exists_check-4 2.074µ 2.065µ ~ 1.000
SubtreeAllocations/large_subtrees_data_fetch-4 2.230µ 2.240µ ~ 0.700
SubtreeAllocations/large_subtrees_full_validation-4 4.300µ 4.369µ ~ 0.100
StoreBlock_Sequential/BelowCSVHeight-4 331.8µ 336.7µ ~ 0.100
StoreBlock_Sequential/AboveCSVHeight-4 337.3µ 335.3µ ~ 0.100
GetUtxoHashes-4 258.0n 256.2n ~ 0.700
GetUtxoHashes_ManyOutputs-4 42.80µ 42.42µ ~ 0.700
_NewMetaDataFromBytes-4 237.0n 239.4n ~ 1.000
_Bytes-4 610.0n 618.8n ~ 0.300
_MetaBytes-4 566.5n 560.8n ~ 0.400

Threshold: >10% with p < 0.05 | Generated: 2026-04-28 18:12 UTC

@oskarszoon oskarszoon mentioned this pull request May 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ordishs