Skip to content

fix(validator): bound HTTP fallback diagnostics - #1724

Open
oskarszoon wants to merge 1 commit into
bsv-blockchain:mainfrom
oskarszoon:fix/validator-http-error-bound
Open

fix(validator): bound HTTP fallback diagnostics#1724
oskarszoon wants to merge 1 commit into
bsv-blockchain:mainfrom
oskarszoon:fix/validator-http-error-bound

Conversation

@oskarszoon

@oskarszoon oskarszoon commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

The validator HTTP fallback buffers the whole non-200 response before reporting an error. Limit diagnostic reads to a 2 KiB prefix plus one byte to detect truncation, quote control characters, and mark truncated bodies. Preserve validation verdicts from response headers and the existing success behavior.

Responses still close on every successful HTTP exchange. Large error responses close without draining the remainder, which can prevent HTTP/1 connection reuse for those responses.

Validation: focused HTTP fallback tests pass, including under the race detector. Vet, staticcheck, and changed-code golangci-lint pass. The full validator suite reproduces an existing TestValidate_RejectsGenuineUnspendableOutput/ff_varint_full_money_range fee-policy assertion failure on both the original base and this branch. Local security scans also report existing Go toolchain and unchanged-file findings.

CI is currently blocked by the test failures addressed in #1721. This branch will be rebased and checks rerun after that prerequisite merges.

Cap error-response reads before buffering and formatting. Preserve
validator verdicts while quoting diagnostics and marking truncation.
Close long responses without draining their remaining body.
Copilot AI lite review requested due to automatic review settings September 11, 2026 15:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete

Current Review:

No blocking issues found. This is a clean, well-scoped defensive change.

  • The bound is correct: LimitReader(body, max+1) reads one extra byte purely to detect truncation, and the returned prefix is capped at exactly maxHTTPFallbackErrorBodyBytes (2 KiB). Off-by-one boundaries (2047/2048/2049) are explicitly covered by tests.
  • fmt.Sprintf("%q", ...) correctly neutralizes control bytes (CR/LF/ESC/NUL) so a hostile error body cannot forge extra log lines or inject terminal escapes — verified by the escaped-body test cases. Invalid UTF-8 is escaped as \xNN rather than panicking.
  • Verdict-header handling and the success path are preserved; the partial-read (ErrUnexpectedEOF) case is handled gracefully since io.ReadAll returns what it got.
  • Test coverage is thorough (empty, exact-limit, known/unknown length, chunked, malformed header, public verdict, partial reads, success-ignores-verdict).

Notes (non-blocking):

  • As the author already documents, large error responses now close without draining the remainder, which can prevent HTTP/1 keep-alive reuse for those responses. Acceptable here — this is a rarely-hit fallback path (only for txs exceeding the gRPC message limit) with typically small error bodies.
  • Out of scope: the sibling propagation fallback (services/propagation/Client.go:373 and :520) still uses unbounded io.ReadAll(resp.Body) on non-OK responses and would benefit from the same treatment in a follow-up.

@github-actions

Copy link
Copy Markdown
Contributor

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-1724 (e3b329e)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 139
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.719µ 1.721µ ~ 0.400
Block_ValidOrderAndBlessed_DiskVsMemory/leaves=1024/memory-4 12.66m 12.06m ~ 0.100
Block_ValidOrderAndBlessed_DiskVsMemory/leaves=1024/disk_1-4 12.89m 12.90m ~ 1.000
Block_ValidOrderAndBlessed_DiskVsMemory/leaves=1024/disk_2-4 12.91m 12.90m ~ 0.100
Block_ValidOrderAndBlessed_DiskVsMemory/leaves=16384/memo... 28.96m 29.00m ~ 1.000
Block_ValidOrderAndBlessed_DiskVsMemory/leaves=16384/disk... 37.51m 37.43m ~ 1.000
Block_ValidOrderAndBlessed_DiskVsMemory/leaves=16384/disk... 37.43m 36.87m ~ 0.700
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 64.23n 64.16n ~ 0.600
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 64.04n 64.00n ~ 0.500
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 64.00n 64.02n ~ 1.000
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 29.94n 30.25n ~ 0.400
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 51.25n 51.08n ~ 1.000
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 104.9n 105.9n ~ 0.700
MiningCandidate_Stringify_Short-4 243.6n 244.9n ~ 1.000
MiningCandidate_Stringify_Long-4 1.614µ 1.604µ ~ 1.000
MiningSolution_Stringify-4 840.0n 792.8n ~ 0.100
BlockInfo_MarshalJSON-4 1.590µ 1.587µ ~ 0.500
NewFromBytes-4 166.6n 165.3n ~ 0.400
AddTxBatchColumnar_Validation-4 2.027µ 2.021µ ~ 0.400
OffsetValidationLoop-4 634.6n 636.6n ~ 0.400
Mine_EasyDifficulty-4 61.11µ 61.27µ ~ 0.700
Mine_WithAddress-4 7.119µ 7.077µ ~ 1.000
BlockAssembler_AddTx-4 0.03197n 0.02995n ~ 0.400
AddNode-4 10.86 11.06 ~ 0.400
AddNodeWithMap-4 11.51 10.93 ~ 1.000
DirectSubtreeAdd/4_per_subtree-4 59.40n 61.00n ~ 0.700
DirectSubtreeAdd/64_per_subtree-4 30.69n 30.41n ~ 0.400
DirectSubtreeAdd/256_per_subtree-4 29.21n 29.19n ~ 1.000
DirectSubtreeAdd/1024_per_subtree-4 28.08n 28.23n ~ 0.700
DirectSubtreeAdd/2048_per_subtree-4 27.63n 27.75n ~ 0.200
SubtreeProcessorAdd/4_per_subtree-4 240.9n 245.8n ~ 0.100
SubtreeProcessorAdd/64_per_subtree-4 237.1n 238.5n ~ 0.100
SubtreeProcessorAdd/256_per_subtree-4 237.3n 239.8n ~ 0.400
SubtreeProcessorAdd/1024_per_subtree-4 230.8n 231.8n ~ 1.000
SubtreeProcessorAdd/2048_per_subtree-4 229.7n 231.6n ~ 0.700
SubtreeProcessorRotate/4_per_subtree-4 232.1n 236.0n ~ 0.100
SubtreeProcessorRotate/64_per_subtree-4 233.7n 234.0n ~ 0.400
SubtreeProcessorRotate/256_per_subtree-4 234.1n 233.2n ~ 0.700
SubtreeProcessorRotate/1024_per_subtree-4 232.6n 234.8n ~ 0.100
SubtreeNodeAddOnly/4_per_subtree-4 54.84n 56.97n ~ 0.100
SubtreeNodeAddOnly/64_per_subtree-4 34.48n 34.48n ~ 1.000
SubtreeNodeAddOnly/256_per_subtree-4 33.48n 33.49n ~ 1.000
SubtreeNodeAddOnly/1024_per_subtree-4 33.03n 33.00n ~ 1.000
SubtreeCreationOnly/4_per_subtree-4 117.1n 139.7n ~ 0.100
SubtreeCreationOnly/64_per_subtree-4 410.9n 398.4n ~ 0.700
SubtreeCreationOnly/256_per_subtree-4 1.342µ 1.308µ ~ 0.100
SubtreeCreationOnly/1024_per_subtree-4 4.426µ 4.979µ ~ 0.100
SubtreeCreationOnly/2048_per_subtree-4 7.888µ 8.648µ ~ 0.100
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 231.6n 232.4n ~ 1.000
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 233.8n 232.5n ~ 0.400
ParallelGetAndSetIfNotExists/1k_nodes-4 10.46m 12.37m ~ 0.100
ParallelGetAndSetIfNotExists/10k_nodes-4 13.68m 15.67m ~ 0.100
ParallelGetAndSetIfNotExists/50k_nodes-4 16.85m 19.59m ~ 0.100
ParallelGetAndSetIfNotExists/100k_nodes-4 20.03m 18.60m ~ 0.100
SequentialGetAndSetIfNotExists/1k_nodes-4 10.04m 13.23m ~ 0.100
SequentialGetAndSetIfNotExists/10k_nodes-4 14.47m 16.98m ~ 0.100
SequentialGetAndSetIfNotExists/50k_nodes-4 24.47m 23.27m ~ 0.400
SequentialGetAndSetIfNotExists/100k_nodes-4 36.87m 29.24m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 13.01m 13.45m ~ 0.400
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 14.53m 15.11m ~ 0.700
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 21.14m 19.23m ~ 0.700
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 13.80m 15.69m ~ 1.000
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 15.51m 16.42m ~ 0.700
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 61.84m 57.47m ~ 0.400
DiskTxMap_SetIfNotExists-4 4.066µ 3.963µ ~ 0.700
DiskTxMap_SetIfNotExists_Parallel-4 3.625µ 3.747µ ~ 0.200
DiskTxMap_ExistenceOnly-4 424.7n 404.8n ~ 0.200
Queue-4 202.6n 203.9n ~ 0.700
AtomicPointer-4 4.919n 4.761n ~ 0.400
TxMapSetIfNotExists-4 62.81n 62.57n ~ 1.000
TxMapSetIfNotExistsDuplicate-4 46.22n 46.20n ~ 0.700
ChannelSendReceive-4 569.9n 571.2n ~ 1.000
CalcBlockWork-4 469.9n 470.4n ~ 1.000
CalculateWork-4 630.7n 645.6n ~ 0.100
CheckOldBlockIDs/on-chain-prefetch/1000-4 66.56µ 60.95µ ~ 1.000
CheckOldBlockIDs/in-memory-chain-check/1000-4 1.327m 1.326m ~ 1.000
CheckOldBlockIDs/on-chain-prefetch/10000-4 417.4µ 413.1µ ~ 0.100
CheckOldBlockIDs/in-memory-chain-check/10000-4 2.015m 1.992m ~ 0.400
BuildBlockLocatorString_Helpers/Size_10-4 1.342µ 1.349µ ~ 0.200
BuildBlockLocatorString_Helpers/Size_100-4 12.95µ 12.89µ ~ 0.200
BuildBlockLocatorString_Helpers/Size_1000-4 127.8µ 127.2µ ~ 1.000
CatchupWithHeaderCache-4 106.5m 106.4m ~ 0.400
_BufferPoolAllocation/16KB-4 4.079µ 5.634µ ~ 0.200
_BufferPoolAllocation/32KB-4 11.04µ 10.60µ ~ 0.700
_BufferPoolAllocation/64KB-4 17.92µ 17.76µ ~ 0.700
_BufferPoolAllocation/128KB-4 36.19µ 32.79µ ~ 0.200
_BufferPoolAllocation/512KB-4 143.5µ 137.6µ ~ 0.100
_BufferPoolConcurrent/32KB-4 23.39µ 20.17µ ~ 0.100
_BufferPoolConcurrent/64KB-4 34.40µ 32.12µ ~ 0.100
_BufferPoolConcurrent/512KB-4 160.0µ 155.9µ ~ 0.700
_SubtreeDeserializationWithBufferSizes/16KB-4 704.8µ 699.8µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/32KB-4 634.8µ 632.3µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/64KB-4 656.0µ 629.4µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/128KB-4 665.3µ 630.6µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/512KB-4 675.1µ 643.2µ ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/16KB-4 36.57m 36.30m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/32KB-4 36.51m 36.31m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/64KB-4 36.82m 36.20m ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/128KB-4 36.79m 36.12m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/512KB-4 36.70m 35.97m ~ 0.100
_PooledVsNonPooled/Pooled-4 655.1n 738.7n ~ 0.100
_PooledVsNonPooled/NonPooled-4 8.366µ 8.215µ ~ 0.700
_MemoryFootprint/Current_512KB_32concurrent-4 7.202µ 6.978µ ~ 0.100
_MemoryFootprint/Proposed_32KB_32concurrent-4 10.52µ 10.27µ ~ 0.400
_MemoryFootprint/Alternative_64KB_32concurrent-4 10.000µ 9.817µ ~ 0.100
SubtreeSizes/10k_tx_4_per_subtree-4 1.009m 1.004m ~ 1.000
SubtreeSizes/10k_tx_16_per_subtree-4 240.3µ 247.7µ ~ 0.200
SubtreeSizes/10k_tx_64_per_subtree-4 57.71µ 58.72µ ~ 0.400
SubtreeSizes/10k_tx_256_per_subtree-4 14.28µ 14.45µ ~ 0.100
SubtreeSizes/10k_tx_512_per_subtree-4 7.102µ 7.319µ ~ 0.100
SubtreeSizes/10k_tx_1024_per_subtree-4 3.492µ 3.532µ ~ 0.100
SubtreeSizes/10k_tx_2k_per_subtree-4 1.734µ 1.774µ ~ 0.100
BlockSizeScaling/10k_tx_64_per_subtree-4 55.76µ 56.22µ ~ 1.000
BlockSizeScaling/10k_tx_256_per_subtree-4 13.92µ 14.15µ ~ 0.200
BlockSizeScaling/10k_tx_1024_per_subtree-4 3.503µ 3.483µ ~ 0.400
BlockSizeScaling/50k_tx_64_per_subtree-4 293.2µ 295.2µ ~ 1.000
BlockSizeScaling/50k_tx_256_per_subtree-4 70.70µ 69.64µ ~ 0.100
BlockSizeScaling/50k_tx_1024_per_subtree-4 17.28µ 17.30µ ~ 1.000
SubtreeAllocations/small_subtrees_exists_check-4 115.8µ 118.6µ ~ 0.700
SubtreeAllocations/small_subtrees_data_fetch-4 124.5µ 125.0µ ~ 0.700
SubtreeAllocations/small_subtrees_full_validation-4 241.9µ 240.0µ ~ 0.700
SubtreeAllocations/medium_subtrees_exists_check-4 7.101µ 7.192µ ~ 0.700
SubtreeAllocations/medium_subtrees_data_fetch-4 7.642µ 7.434µ ~ 1.000
SubtreeAllocations/medium_subtrees_full_validation-4 13.87µ 14.03µ ~ 0.100
SubtreeAllocations/large_subtrees_exists_check-4 1.722µ 1.690µ ~ 0.700
SubtreeAllocations/large_subtrees_data_fetch-4 1.815µ 1.800µ ~ 0.400
SubtreeAllocations/large_subtrees_full_validation-4 3.513µ 3.488µ ~ 0.400
_prepareTxsPerLevel-4 311.9m 315.6m ~ 0.100
_prepareTxsPerLevelOrdered-4 2.891m 2.943m ~ 0.700
_prepareTxsPerLevel_Comparison/Original-4 321.0m 316.0m ~ 0.400
_prepareTxsPerLevel_Comparison/Optimized-4 2.767m 2.655m ~ 0.100
StoreBlock_Sequential/BelowCSVHeight-4 288.3µ 347.5µ ~ 0.700
StoreBlock_Sequential/AboveCSVHeight-4 347.4µ 345.7µ ~ 1.000
GetUtxoHashes-4 259.7n 266.8n ~ 0.400
GetUtxoHashes_ManyOutputs-4 41.98µ 42.12µ ~ 0.700
MetaBytes-4 87.70n 87.97n ~ 0.700
_NewMetaDataFromBytes-4 276.2n 276.3n ~ 1.000
_Bytes-4 391.5n 391.9n ~ 0.700
_MetaBytes-4 137.3n 136.6n ~ 0.300

Threshold: >10% with p < 0.05 | Generated: 2026-09-11 15:29 UTC

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