Skip to content

fix(p2p): use per-connection context in HandleWebSocket#774

Merged
ordishs merged 2 commits into
bsv-blockchain:mainfrom
ordishs:security/4573-ws-per-conn-context
May 13, 2026
Merged

fix(p2p): use per-connection context in HandleWebSocket#774
ordishs merged 2 commits into
bsv-blockchain:mainfrom
ordishs:security/4573-ws-per-conn-context

Conversation

@ordishs

@ordishs ordishs commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Closes #4573.

Summary

HandleWebSocket created a single ctx, cancel pair at server setup and shared it across every WebSocket connection. A single failed upgrade (malformed headers, etc.) called cancel(), killing the notification processor for ALL connected clients. Severity HIGH because any peer can trigger it remotely.

Fix

  • Each connection gets its own connCtx, connCancel := context.WithCancel(serverCtx); defer connCancel() cleans up on handler return.
  • Upgrade failures no longer touch the shared notification processor's context.
  • The server-level context is now derived from s.gCtx (the lifecycle context passed into NewServer), so the notification processor stops on server shutdown without leaking the goroutine.

Test plan

  • Regression test TestHandleWebSocket_PerConnectionContext: drives an upgrade failure on one connection, then connects a real WebSocket client and asserts a notification is still delivered. Fails pre-fix (processor goroutine has exited; client gets EOF), passes post-fix.
  • go test -race ./services/p2p/... passes.

@github-actions

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

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete

Current Review:

This is a well-crafted security fix that correctly addresses a critical DoS vulnerability. The PR demonstrates careful engineering with minimal changes, comprehensive test coverage, and clear documentation.

Summary: No issues found. The fix correctly isolates per-connection contexts, preventing upgrade failures from terminating the shared notification processor. The regression test validates the fix effectively.

Strengths:

  • Minimal, surgical change that follows the Small Diff Rule
  • Comprehensive regression test that demonstrates the bug and validates the fix
  • Proper context hierarchy: server context derived from s.gCtx, connection contexts derived from serverCtx
  • Clean resource management with defer connCancel()
  • Test correctly verifies processor remains alive after upgrade failure

@github-actions

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

Copy link
Copy Markdown
Contributor

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-774 (e909bdd)

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.686µ 1.983µ ~ 0.600
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 61.69n 61.66n ~ 0.700
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 61.70n 61.55n ~ 0.100
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 61.74n 61.58n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 30.31n 31.70n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 52.33n 52.22n ~ 1.000
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 110.8n 107.8n ~ 0.200
MiningCandidate_Stringify_Short-4 265.4n 267.1n ~ 0.100
MiningCandidate_Stringify_Long-4 1.913µ 1.913µ ~ 0.800
MiningSolution_Stringify-4 981.8n 986.5n ~ 0.100
BlockInfo_MarshalJSON-4 1.772µ 1.777µ ~ 0.800
NewFromBytes-4 129.1n 130.1n ~ 0.200
Mine_EasyDifficulty-4 63.70µ 63.65µ ~ 1.000
Mine_WithAddress-4 7.285µ 7.248µ ~ 0.700
BlockAssembler_AddTx-4 0.02743n 0.02720n ~ 1.000
AddNode-4 10.33 10.08 ~ 0.400
AddNodeWithMap-4 10.76 10.71 ~ 1.000
DirectSubtreeAdd/4_per_subtree-4 62.99n 61.35n ~ 0.200
DirectSubtreeAdd/64_per_subtree-4 31.77n 31.73n ~ 1.000
DirectSubtreeAdd/256_per_subtree-4 30.29n 30.33n ~ 0.700
DirectSubtreeAdd/1024_per_subtree-4 29.09n 29.27n ~ 0.300
DirectSubtreeAdd/2048_per_subtree-4 28.66n 28.72n ~ 0.400
SubtreeProcessorAdd/4_per_subtree-4 277.3n 280.0n ~ 0.400
SubtreeProcessorAdd/64_per_subtree-4 272.1n 275.9n ~ 0.400
SubtreeProcessorAdd/256_per_subtree-4 275.5n 277.4n ~ 0.700
SubtreeProcessorAdd/1024_per_subtree-4 268.8n 268.6n ~ 0.800
SubtreeProcessorAdd/2048_per_subtree-4 268.2n 268.8n ~ 0.600
SubtreeProcessorRotate/4_per_subtree-4 271.3n 273.3n ~ 0.700
SubtreeProcessorRotate/64_per_subtree-4 272.7n 271.7n ~ 0.400
SubtreeProcessorRotate/256_per_subtree-4 273.6n 269.8n ~ 0.200
SubtreeProcessorRotate/1024_per_subtree-4 272.7n 270.8n ~ 0.200
SubtreeNodeAddOnly/4_per_subtree-4 54.59n 54.28n ~ 0.400
SubtreeNodeAddOnly/64_per_subtree-4 34.39n 34.24n ~ 0.700
SubtreeNodeAddOnly/256_per_subtree-4 33.47n 33.66n ~ 0.400
SubtreeNodeAddOnly/1024_per_subtree-4 32.86n 32.88n ~ 1.000
SubtreeCreationOnly/4_per_subtree-4 113.1n 111.8n ~ 0.100
SubtreeCreationOnly/64_per_subtree-4 399.0n 394.5n ~ 0.400
SubtreeCreationOnly/256_per_subtree-4 1.338µ 1.394µ ~ 0.100
SubtreeCreationOnly/1024_per_subtree-4 4.391µ 4.540µ ~ 0.700
SubtreeCreationOnly/2048_per_subtree-4 7.939µ 8.198µ ~ 0.100
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 272.6n 270.3n ~ 0.700
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 271.4n 272.6n ~ 0.600
ParallelGetAndSetIfNotExists/1k_nodes-4 793.0µ 581.7µ ~ 0.100
ParallelGetAndSetIfNotExists/10k_nodes-4 1.558m 1.348m ~ 0.100
ParallelGetAndSetIfNotExists/50k_nodes-4 6.702m 6.790m ~ 0.100
ParallelGetAndSetIfNotExists/100k_nodes-4 13.62m 13.58m ~ 1.000
SequentialGetAndSetIfNotExists/1k_nodes-4 654.1µ 655.6µ ~ 0.700
SequentialGetAndSetIfNotExists/10k_nodes-4 2.768m 2.916m ~ 0.700
SequentialGetAndSetIfNotExists/50k_nodes-4 10.59m 10.47m ~ 0.200
SequentialGetAndSetIfNotExists/100k_nodes-4 20.57m 20.27m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 838.1µ 635.6µ ~ 0.700
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 4.134m 4.158m ~ 0.400
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 16.63m 16.74m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 697.5µ 696.2µ ~ 1.000
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 5.645m 5.644m ~ 1.000
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 37.97m 37.66m ~ 0.200
DiskTxMap_SetIfNotExists-4 3.432µ 3.502µ ~ 0.200
DiskTxMap_SetIfNotExists_Parallel-4 3.269µ 3.283µ ~ 0.700
DiskTxMap_ExistenceOnly-4 286.0n 286.8n ~ 0.700
Queue-4 195.5n 196.3n ~ 0.100
AtomicPointer-4 4.690n 5.020n ~ 0.200
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 860.4µ 863.9µ ~ 1.000
ReorgOptimizations/DedupFilterPipeline/New/10K-4 821.1µ 840.4µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 123.7µ 129.0µ ~ 0.200
ReorgOptimizations/AllMarkFalse/New/10K-4 62.56µ 62.23µ ~ 0.100
ReorgOptimizations/HashSlicePool/Old/10K-4 62.71µ 75.29µ ~ 0.100
ReorgOptimizations/HashSlicePool/New/10K-4 11.63µ 11.26µ ~ 0.200
ReorgOptimizations/NodeFlags/Old/10K-4 5.365µ 5.615µ ~ 0.700
ReorgOptimizations/NodeFlags/New/10K-4 1.825µ 1.803µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 9.572m 9.253m ~ 0.100
ReorgOptimizations/DedupFilterPipeline/New/100K-4 9.162m 9.161m ~ 1.000
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.068m 1.114m ~ 0.700
ReorgOptimizations/AllMarkFalse/New/100K-4 681.2µ 681.3µ ~ 1.000
ReorgOptimizations/HashSlicePool/Old/100K-4 631.5µ 681.2µ ~ 0.100
ReorgOptimizations/HashSlicePool/New/100K-4 285.1µ 285.8µ ~ 1.000
ReorgOptimizations/NodeFlags/Old/100K-4 55.06µ 56.92µ ~ 0.100
ReorgOptimizations/NodeFlags/New/100K-4 19.34µ 18.91µ ~ 1.000
TxMapSetIfNotExists-4 51.09n 51.38n ~ 0.700
TxMapSetIfNotExistsDuplicate-4 38.12n 37.91n ~ 1.000
ChannelSendReceive-4 622.5n 595.9n ~ 0.100
CalcBlockWork-4 492.6n 496.2n ~ 0.700
CalculateWork-4 672.4n 671.9n ~ 1.000
BuildBlockLocatorString_Helpers/Size_10-4 1.361µ 1.532µ ~ 0.700
BuildBlockLocatorString_Helpers/Size_100-4 15.69µ 12.87µ ~ 0.400
BuildBlockLocatorString_Helpers/Size_1000-4 127.4µ 127.3µ ~ 1.000
CatchupWithHeaderCache-4 104.5m 104.3m ~ 0.200
_BufferPoolAllocation/16KB-4 4.164µ 3.357µ ~ 0.100
_BufferPoolAllocation/32KB-4 8.570µ 8.230µ ~ 0.700
_BufferPoolAllocation/64KB-4 15.35µ 15.52µ ~ 0.400
_BufferPoolAllocation/128KB-4 28.65µ 32.62µ ~ 0.100
_BufferPoolAllocation/512KB-4 120.1µ 112.2µ ~ 0.100
_BufferPoolConcurrent/32KB-4 19.49µ 20.49µ ~ 0.100
_BufferPoolConcurrent/64KB-4 31.17µ 32.28µ ~ 0.100
_BufferPoolConcurrent/512KB-4 146.6µ 155.2µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/16KB-4 607.1µ 631.1µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/32KB-4 617.4µ 618.1µ ~ 1.000
_SubtreeDeserializationWithBufferSizes/64KB-4 602.9µ 622.5µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/128KB-4 610.7µ 621.4µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/512KB-4 639.4µ 632.0µ ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/16KB-4 36.11m 36.22m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/32KB-4 35.85m 35.97m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/64KB-4 36.04m 36.11m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/128KB-4 35.92m 35.68m ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/512KB-4 35.42m 35.58m ~ 0.100
_PooledVsNonPooled/Pooled-4 650.1n 743.6n ~ 0.100
_PooledVsNonPooled/NonPooled-4 6.979µ 7.583µ ~ 0.100
_MemoryFootprint/Current_512KB_32concurrent-4 6.979µ 7.581µ ~ 0.100
_MemoryFootprint/Proposed_32KB_32concurrent-4 9.718µ 11.734µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 9.577µ 10.445µ ~ 0.100
_prepareTxsPerLevel-4 424.5m 434.1m ~ 0.700
_prepareTxsPerLevelOrdered-4 3.845m 3.977m ~ 0.100
_prepareTxsPerLevel_Comparison/Original-4 451.3m 437.9m ~ 0.700
_prepareTxsPerLevel_Comparison/Optimized-4 3.778m 4.341m ~ 0.100
SubtreeSizes/10k_tx_4_per_subtree-4 1.373m 1.373m ~ 0.700
SubtreeSizes/10k_tx_16_per_subtree-4 325.9µ 326.4µ ~ 0.700
SubtreeSizes/10k_tx_64_per_subtree-4 77.63µ 77.00µ ~ 0.100
SubtreeSizes/10k_tx_256_per_subtree-4 19.47µ 19.44µ ~ 1.000
SubtreeSizes/10k_tx_512_per_subtree-4 9.697µ 9.738µ ~ 0.700
SubtreeSizes/10k_tx_1024_per_subtree-4 4.861µ 4.825µ ~ 0.800
SubtreeSizes/10k_tx_2k_per_subtree-4 2.392µ 2.409µ ~ 0.400
BlockSizeScaling/10k_tx_64_per_subtree-4 76.98µ 76.12µ ~ 0.400
BlockSizeScaling/10k_tx_256_per_subtree-4 19.34µ 19.40µ ~ 0.700
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.835µ 4.853µ ~ 0.400
BlockSizeScaling/50k_tx_64_per_subtree-4 407.2µ 406.9µ ~ 0.700
BlockSizeScaling/50k_tx_256_per_subtree-4 96.71µ 96.52µ ~ 0.700
BlockSizeScaling/50k_tx_1024_per_subtree-4 23.89µ 23.84µ ~ 0.700
SubtreeAllocations/small_subtrees_exists_check-4 162.6µ 162.9µ ~ 1.000
SubtreeAllocations/small_subtrees_data_fetch-4 171.2µ 168.1µ ~ 0.400
SubtreeAllocations/small_subtrees_full_validation-4 338.0µ 337.8µ ~ 1.000
SubtreeAllocations/medium_subtrees_exists_check-4 9.721µ 9.779µ ~ 1.000
SubtreeAllocations/medium_subtrees_data_fetch-4 10.20µ 10.14µ ~ 0.700
SubtreeAllocations/medium_subtrees_full_validation-4 19.74µ 19.56µ ~ 1.000
SubtreeAllocations/large_subtrees_exists_check-4 2.323µ 2.341µ ~ 0.700
SubtreeAllocations/large_subtrees_data_fetch-4 2.466µ 2.478µ ~ 0.100
SubtreeAllocations/large_subtrees_full_validation-4 4.918µ 4.917µ ~ 1.000
StoreBlock_Sequential/BelowCSVHeight-4 336.4µ 316.8µ ~ 0.100
StoreBlock_Sequential/AboveCSVHeight-4 333.9µ 335.7µ ~ 0.700
GetUtxoHashes-4 260.4n 258.2n ~ 1.000
GetUtxoHashes_ManyOutputs-4 44.91µ 42.90µ ~ 0.100
_NewMetaDataFromBytes-4 238.4n 240.0n ~ 0.300
_Bytes-4 625.8n 629.4n ~ 0.700
_MetaBytes-4 569.3n 569.5n ~ 1.000

Threshold: >10% with p < 0.05 | Generated: 2026-05-12 15:13 UTC

@sonarqubecloud

sonarqubecloud Bot commented May 12, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

@ordishs ordishs requested review from gokutheengineer and liam May 13, 2026 08:59
@ordishs ordishs enabled auto-merge (squash) May 13, 2026 09:01
@ordishs ordishs merged commit 38b1a35 into bsv-blockchain:main May 13, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liam liam liam approved these changes

@gokutheengineer gokutheengineer gokutheengineer 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

@ordishs @liam @gokutheengineer