Skip to content

blockassembly: make GetTransactionHashes context-aware and prevent hanging GetBlockAssemblyTxs#939

Merged
gokutheengineer merged 4 commits into
bsv-blockchain:mainfrom
gokutheengineer:gokhan/subtreeprocessor-txcontexttaware
Jun 8, 2026
Merged

blockassembly: make GetTransactionHashes context-aware and prevent hanging GetBlockAssemblyTxs#939
gokutheengineer merged 4 commits into
bsv-blockchain:mainfrom
gokutheengineer:gokhan/subtreeprocessor-txcontexttaware

Conversation

@gokutheengineer

@gokutheengineer gokutheengineer commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

This change fixes a hanging-path in block assembly diagnostics by making subtree transaction-hash retrieval context-aware.

  • Updated subtreeprocessor.GetTransactionHashes to accept context.Context, use a buffered response channel, and return early on cancellation/timeouts.
  • Updated GetBlockAssemblyTxs to pass the RPC context into subtree processor calls and propagate cancellation as a gRPC error.
  • Added periodic context checks while converting large hash slices to strings, so very large responses can be interrupted promptly.
  • Updated subtree processor interfaces/mocks and call sites to use the new context-aware signature.
  • Added regression tests for cancellation behavior:
    • request returns when processor is not serving the request channel
    • processor reply after caller cancellation does not block
    • GetBlockAssemblyTxs returns on context timeout instead of hanging

Why

GetBlockAssemblyTxs could block indefinitely when the subtree processor loop was busy/unavailable, even if the RPC context was canceled. This makes the diagnostic endpoint safer under incident conditions and avoids tying up gRPC workers.

Test plan

  • go test ./services/blockassembly/subtreeprocessor -run 'TestGetTransactionHashes'
  • go test ./services/blockassembly -run 'TestGetBlockAssemblyTxs'
  • Confirmed no linter errors on touched files.

@github-actions

github-actions Bot commented May 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete

Current Review:

This PR correctly implements context-aware cancellation for GetTransactionHashes to prevent hanging diagnostics under incident conditions. The implementation is well-designed with proper buffering and comprehensive tests.

Findings:

  1. [Minor] Potential race condition in nil-check logic - In Server.go:1925, the check if txHashes == nil && ctx.Err() != nil has a subtle race window. Between checking ctx.Err() and the subsequent len(txHashes) call, the context could be canceled. While unlikely to cause issues (Go handles len(nil) correctly), consider simplifying to if txHashes == nil since GetTransactionHashes only returns nil on cancellation.

  2. Test coverage is thorough - The regression tests properly verify:

    • Early return when processor is not serving the request
    • No goroutine leak when caller cancels before processor responds
    • End-to-end cancellation in GetBlockAssemblyTxs

Verified:

  • Context propagation follows Go conventions
  • Buffered channel (size 1) prevents processor deadlock
  • All call sites updated to pass context
  • Mock interfaces updated correctly
  • Periodic context checks (every 1024 iterations) during hash-to-string conversion

@github-actions

github-actions Bot commented May 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-939 (afc87ac)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 144
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.225µ 1.323µ ~ 0.200
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 55.09n 55.01n ~ 0.300
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 54.91n 55.06n ~ 0.400
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 54.93n 54.94n ~ 0.500
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 25.50n 25.27n ~ 0.400
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 43.16n 42.44n ~ 0.200
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 96.21n 96.41n ~ 0.700
MiningCandidate_Stringify_Short-4 168.9n 177.8n ~ 0.100
MiningCandidate_Stringify_Long-4 1.265µ 1.270µ ~ 0.300
MiningSolution_Stringify-4 660.7n 649.6n ~ 0.700
BlockInfo_MarshalJSON-4 1.335µ 1.324µ ~ 0.100
NewFromBytes-4 170.0n 139.4n ~ 0.700
AddTxBatchColumnar_Validation-4 2.605µ 2.614µ ~ 1.000
OffsetValidationLoop-4 595.6n 597.2n ~ 0.700
Mine_EasyDifficulty-4 66.96µ 66.99µ ~ 1.000
Mine_WithAddress-4 7.009µ 7.721µ ~ 0.700
BlockAssembler_AddTx-4 0.02812n 0.02659n ~ 1.000
AddNode-4 11.29 11.70 ~ 0.400
AddNodeWithMap-4 11.97 12.32 ~ 0.700
DirectSubtreeAdd/4_per_subtree-4 58.02n 57.41n ~ 1.000
DirectSubtreeAdd/64_per_subtree-4 29.97n 32.04n ~ 0.100
DirectSubtreeAdd/256_per_subtree-4 29.44n 30.56n ~ 0.100
DirectSubtreeAdd/1024_per_subtree-4 27.83n 29.28n ~ 0.100
DirectSubtreeAdd/2048_per_subtree-4 27.47n 28.93n ~ 0.100
SubtreeProcessorAdd/4_per_subtree-4 283.5n 284.6n ~ 0.800
SubtreeProcessorAdd/64_per_subtree-4 281.5n 280.1n ~ 0.700
SubtreeProcessorAdd/256_per_subtree-4 278.8n 285.6n ~ 0.200
SubtreeProcessorAdd/1024_per_subtree-4 269.6n 271.6n ~ 0.100
SubtreeProcessorAdd/2048_per_subtree-4 273.9n 277.9n ~ 0.700
SubtreeProcessorRotate/4_per_subtree-4 277.1n 275.5n ~ 0.600
SubtreeProcessorRotate/64_per_subtree-4 275.2n 274.0n ~ 0.100
SubtreeProcessorRotate/256_per_subtree-4 275.5n 274.8n ~ 0.400
SubtreeProcessorRotate/1024_per_subtree-4 273.9n 275.4n ~ 0.100
SubtreeNodeAddOnly/4_per_subtree-4 54.00n 53.78n ~ 0.700
SubtreeNodeAddOnly/64_per_subtree-4 34.15n 34.01n ~ 0.700
SubtreeNodeAddOnly/256_per_subtree-4 33.17n 33.18n ~ 0.500
SubtreeNodeAddOnly/1024_per_subtree-4 32.59n 32.44n ~ 0.100
SubtreeCreationOnly/4_per_subtree-4 115.6n 112.8n ~ 0.100
SubtreeCreationOnly/64_per_subtree-4 403.5n 393.6n ~ 0.100
SubtreeCreationOnly/256_per_subtree-4 1.351µ 1.324µ ~ 0.100
SubtreeCreationOnly/1024_per_subtree-4 4.427µ 4.426µ ~ 0.700
SubtreeCreationOnly/2048_per_subtree-4 8.116µ 8.013µ ~ 0.100
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 276.2n 274.1n ~ 0.400
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 276.5n 272.3n ~ 0.100
ParallelGetAndSetIfNotExists/1k_nodes-4 2.196m 2.186m ~ 0.700
ParallelGetAndSetIfNotExists/10k_nodes-4 5.337m 5.324m ~ 0.700
ParallelGetAndSetIfNotExists/50k_nodes-4 7.167m 7.229m ~ 0.400
ParallelGetAndSetIfNotExists/100k_nodes-4 10.30m 10.03m ~ 0.100
SequentialGetAndSetIfNotExists/1k_nodes-4 1.966m 1.951m ~ 0.100
SequentialGetAndSetIfNotExists/10k_nodes-4 4.443m 4.365m ~ 0.200
SequentialGetAndSetIfNotExists/50k_nodes-4 12.44m 12.63m ~ 0.400
SequentialGetAndSetIfNotExists/100k_nodes-4 22.75m 23.99m ~ 0.200
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 2.263m 2.287m ~ 0.400
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 8.195m 8.333m ~ 0.200
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 12.93m 13.65m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 1.987m 2.003m ~ 1.000
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 7.634m 7.948m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 41.41m 42.73m ~ 0.400
DiskTxMap_SetIfNotExists-4 3.408µ 3.319µ ~ 0.100
DiskTxMap_SetIfNotExists_Parallel-4 3.160µ 3.156µ ~ 0.700
DiskTxMap_ExistenceOnly-4 293.5n 295.8n ~ 0.400
Queue-4 188.7n 189.8n ~ 1.000
AtomicPointer-4 4.842n 4.467n ~ 0.200
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 876.8µ 849.1µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/New/10K-4 847.4µ 784.5µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 110.0µ 121.8µ ~ 0.100
ReorgOptimizations/AllMarkFalse/New/10K-4 62.53µ 61.91µ ~ 0.200
ReorgOptimizations/HashSlicePool/Old/10K-4 55.36µ 55.54µ ~ 1.000
ReorgOptimizations/HashSlicePool/New/10K-4 12.04µ 12.14µ ~ 1.000
ReorgOptimizations/NodeFlags/Old/10K-4 5.266µ 4.567µ ~ 0.100
ReorgOptimizations/NodeFlags/New/10K-4 2.084µ 1.580µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 9.754m 9.417m ~ 0.400
ReorgOptimizations/DedupFilterPipeline/New/100K-4 9.305m 9.321m ~ 1.000
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.102m 1.150m ~ 0.100
ReorgOptimizations/AllMarkFalse/New/100K-4 682.7µ 685.9µ ~ 0.700
ReorgOptimizations/HashSlicePool/Old/100K-4 570.5µ 658.2µ ~ 0.700
ReorgOptimizations/HashSlicePool/New/100K-4 315.8µ 317.0µ ~ 0.700
ReorgOptimizations/NodeFlags/Old/100K-4 51.67µ 50.73µ ~ 0.200
ReorgOptimizations/NodeFlags/New/100K-4 17.87µ 17.85µ ~ 0.700
TxMapSetIfNotExists-4 52.62n 52.64n ~ 1.000
TxMapSetIfNotExistsDuplicate-4 40.49n 40.07n ~ 0.100
ChannelSendReceive-4 609.6n 608.6n ~ 0.700
CalcBlockWork-4 472.1n 472.4n ~ 1.000
CalculateWork-4 644.8n 644.5n ~ 1.000
BuildBlockLocatorString_Helpers/Size_10-4 1.371µ 1.359µ ~ 0.100
BuildBlockLocatorString_Helpers/Size_100-4 14.73µ 15.15µ ~ 1.000
BuildBlockLocatorString_Helpers/Size_1000-4 128.9µ 129.1µ ~ 0.100
CatchupWithHeaderCache-4 104.6m 104.6m ~ 0.700
_prepareTxsPerLevel-4 413.8m 416.8m ~ 1.000
_prepareTxsPerLevelOrdered-4 3.679m 3.571m ~ 0.700
_prepareTxsPerLevel_Comparison/Original-4 414.9m 413.2m ~ 1.000
_prepareTxsPerLevel_Comparison/Optimized-4 4.158m 3.682m ~ 0.200
_BufferPoolAllocation/16KB-4 3.993µ 3.866µ ~ 0.100
_BufferPoolAllocation/32KB-4 8.637µ 10.688µ ~ 0.100
_BufferPoolAllocation/64KB-4 19.47µ 18.19µ ~ 0.700
_BufferPoolAllocation/128KB-4 32.42µ 31.40µ ~ 0.700
_BufferPoolAllocation/512KB-4 108.8µ 106.7µ ~ 1.000
_BufferPoolConcurrent/32KB-4 18.86µ 19.10µ ~ 1.000
_BufferPoolConcurrent/64KB-4 31.60µ 29.33µ ~ 0.700
_BufferPoolConcurrent/512KB-4 154.7µ 142.9µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/16KB-4 643.5µ 637.6µ ~ 0.700
_SubtreeDeserializationWithBufferSizes/32KB-4 620.1µ 636.7µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/64KB-4 643.2µ 626.7µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/128KB-4 650.3µ 639.7µ ~ 0.700
_SubtreeDeserializationWithBufferSizes/512KB-4 627.2µ 597.0µ ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/16KB-4 37.00m 36.98m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/32KB-4 37.01m 37.19m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/64KB-4 36.54m 36.73m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/128KB-4 36.53m 36.56m ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/512KB-4 36.67m 35.98m ~ 0.200
_PooledVsNonPooled/Pooled-4 832.6n 831.1n ~ 0.300
_PooledVsNonPooled/NonPooled-4 8.073µ 7.780µ ~ 0.100
_MemoryFootprint/Current_512KB_32concurrent-4 7.085µ 7.266µ ~ 1.000
_MemoryFootprint/Proposed_32KB_32concurrent-4 9.633µ 9.497µ ~ 0.700
_MemoryFootprint/Alternative_64KB_32concurrent-4 9.287µ 9.248µ ~ 1.000
SubtreeSizes/10k_tx_4_per_subtree-4 1.275m 1.275m ~ 0.700
SubtreeSizes/10k_tx_16_per_subtree-4 302.6µ 298.5µ ~ 0.700
SubtreeSizes/10k_tx_64_per_subtree-4 72.19µ 73.95µ ~ 0.100
SubtreeSizes/10k_tx_256_per_subtree-4 18.35µ 18.14µ ~ 0.400
SubtreeSizes/10k_tx_512_per_subtree-4 9.029µ 9.010µ ~ 0.400
SubtreeSizes/10k_tx_1024_per_subtree-4 4.407µ 4.408µ ~ 1.000
SubtreeSizes/10k_tx_2k_per_subtree-4 2.202µ 2.234µ ~ 0.700
BlockSizeScaling/10k_tx_64_per_subtree-4 71.51µ 70.09µ ~ 0.100
BlockSizeScaling/10k_tx_256_per_subtree-4 17.94µ 17.65µ ~ 0.100
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.402µ 4.428µ ~ 0.100
BlockSizeScaling/50k_tx_64_per_subtree-4 366.4µ 370.5µ ~ 1.000
BlockSizeScaling/50k_tx_256_per_subtree-4 88.15µ 88.82µ ~ 0.400
BlockSizeScaling/50k_tx_1024_per_subtree-4 21.76µ 21.83µ ~ 0.400
SubtreeAllocations/small_subtrees_exists_check-4 151.3µ 150.5µ ~ 0.700
SubtreeAllocations/small_subtrees_data_fetch-4 159.5µ 160.6µ ~ 0.200
SubtreeAllocations/small_subtrees_full_validation-4 309.0µ 313.7µ ~ 0.400
SubtreeAllocations/medium_subtrees_exists_check-4 8.841µ 8.935µ ~ 0.100
SubtreeAllocations/medium_subtrees_data_fetch-4 9.448µ 9.472µ ~ 1.000
SubtreeAllocations/medium_subtrees_full_validation-4 17.54µ 17.61µ ~ 1.000
SubtreeAllocations/large_subtrees_exists_check-4 2.084µ 2.082µ ~ 1.000
SubtreeAllocations/large_subtrees_data_fetch-4 2.273µ 2.226µ ~ 0.700
SubtreeAllocations/large_subtrees_full_validation-4 4.397µ 4.381µ ~ 1.000
StoreBlock_Sequential/BelowCSVHeight-4 339.9µ 338.3µ ~ 0.200
StoreBlock_Sequential/AboveCSVHeight-4 344.3µ 340.7µ ~ 0.100
GetUtxoHashes-4 264.3n 266.4n ~ 0.300
GetUtxoHashes_ManyOutputs-4 46.40µ 43.21µ ~ 0.100
_NewMetaDataFromBytes-4 227.7n 226.9n ~ 0.600
_Bytes-4 402.6n 400.9n ~ 0.400
_MetaBytes-4 138.0n 136.6n ~ 0.100

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

freemans13
freemans13 approved these changes Jun 4, 2026
View reviewed changes

@freemans13 freemans13 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approve. Clean, minimal fix that closes a real availability hazard on the GetBlockAssemblyTxs diagnostic path.

  • Correctness solid: size-1 buffered response channel absorbs the main loop's unconditional send (SubtreeProcessor.go:766) even after the caller returns on ctx.Done() — no goroutine leak. Both select arms (request send + response receive) are ctx-guarded.
  • nil vs empty is safe: the loop always builds the slice with make(..., 0, ...), so the txHashes == nil && ctx.Err() != nil guard can't misfire on a legitimately empty result.
  • Consistent with the established GetSubtreeHashes pattern in the same file.
  • Good, deterministic regression tests, including the post-cancellation processor-send case.

Minor (non-blocking) suggestion: add a one-line comment to TestGetBlockAssemblyTxsReturnsWhenContextCancelled noting it relies on the processor not being started (so the timeout always wins) — protects it from a future edit adding Start() and racing.

Recommend confirming CI is green before merge (I didn't run the local Go suite).

@sonarqubecloud

sonarqubecloud Bot commented Jun 4, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@gokutheengineer gokutheengineer merged commit ba71b5d into bsv-blockchain:main Jun 8, 2026
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@icellan icellan icellan approved these changes

@freemans13 freemans13 freemans13 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gokutheengineer @icellan @freemans13