Skip to content

fix: don't filter block messages from low-reputation peers#661

Merged
freemans13 merged 12 commits into
bsv-blockchain:mainfrom
freemans13:fix/dont-filter-blocks-from-unhealthy-peers
Jun 5, 2026
Merged

fix: don't filter block messages from low-reputation peers#661
freemans13 merged 12 commits into
bsv-blockchain:mainfrom
freemans13:fix/dont-filter-blocks-from-unhealthy-peers

Conversation

@freemans13

@freemans13 freemans13 commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Removes the shouldSkipUnhealthyPeer filter from handleBlockTopic in the P2P service
  • Block validation already handles bad blocks safely, so this filter is unnecessary
  • Filtering blocks from low-reputation peers prevents catchup when the node is behind, since the only available peers may have low reputation

Test plan

  • Verified on teratestnet that catchup proceeds normally with this change
  • Confirm no regression in block validation for invalid blocks from peers

🤖 Generated with Claude Code

@freemans13 @claude
fix: don't filter block messages from low-reputation peers
d1d072b 
Block validation already handles bad blocks safely. Filtering blocks
from unhealthy/low-reputation peers prevents catchup when the node
is behind, since the only available peers may have low reputation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions

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

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete

Current Review:
No issues found. The change is well-reasoned and properly tested.

Summary:
This PR removes the shouldSkipUnhealthyPeer filter from block announcement handling to fix a catchup deadlock scenario. The fix is correct:

  1. Root cause identified: When a node falls behind, it may only have low-reputation peers available. The old code filtered their block announcements, preventing catchup from starting.

  2. Safe to remove: Block announcements trigger catchup, but catchup fetches blocks via HTTP (not the gossip subtree handler). Block validation is the actual gatekeeper for malicious blocks.

  3. Asymmetry preserved: The filter correctly remains in handleSubtreeTopic (line 228), since subtree gossip is different from HTTP-based catchup.

  4. Test coverage: The new regression test validates that block announcements from low-reputation (but not banned) peers reach Kafka, which would fail if the filter were reintroduced.

  5. Documentation: The inline comment clearly explains the rationale and prevents accidental reintroduction of the bug.

freemans13 added a commit to freemans13/teranode that referenced this pull request Apr 2, 2026
@freemans13 @claude
fix: clean up PR — remove unrelated files and restore accidental dele…
8c6cc00 
…tion

- Revert server_helpers.go change (split to separate PR bsv-blockchain#661)
- Restore accidentally deleted docs/p2p-silent-mode.md
- Remove AGENTS.md from tracking (personal config)
- Remove batcher design spec and gitignore docs/superpowers/

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions

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

Copy link
Copy Markdown
Contributor

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-661 (1b3f705)

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.824µ 1.800µ ~ 0.700
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 61.73n 61.71n ~ 1.000
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 62.05n 61.77n ~ 0.400
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 61.77n 61.87n ~ 0.400
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 36.73n 34.73n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 59.47n 56.33n ~ 0.700
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 162.2n 157.8n ~ 0.400
MiningCandidate_Stringify_Short-4 268.2n 267.4n ~ 0.400
MiningCandidate_Stringify_Long-4 1.974µ 1.972µ ~ 0.500
MiningSolution_Stringify-4 1005.0n 996.0n ~ 0.400
BlockInfo_MarshalJSON-4 1.897µ 1.869µ ~ 0.100
NewFromBytes-4 124.5n 124.4n ~ 0.400
AddTxBatchColumnar_Validation-4 2.434µ 2.644µ ~ 0.700
OffsetValidationLoop-4 547.8n 545.5n ~ 0.300
Mine_EasyDifficulty-4 67.00µ 68.14µ ~ 1.000
Mine_WithAddress-4 7.046µ 6.977µ ~ 0.200
BlockAssembler_AddTx-4 0.02776n 0.02543n ~ 1.000
AddNode-4 11.19 11.02 ~ 0.400
AddNodeWithMap-4 11.75 11.60 ~ 0.100
DirectSubtreeAdd/4_per_subtree-4 59.05n 57.82n ~ 0.700
DirectSubtreeAdd/64_per_subtree-4 29.23n 30.42n ~ 0.100
DirectSubtreeAdd/256_per_subtree-4 28.48n 27.99n ~ 0.100
DirectSubtreeAdd/1024_per_subtree-4 26.78n 26.65n ~ 0.700
DirectSubtreeAdd/2048_per_subtree-4 26.38n 26.23n ~ 0.100
SubtreeProcessorAdd/4_per_subtree-4 314.5n 301.3n ~ 0.100
SubtreeProcessorAdd/64_per_subtree-4 317.5n 290.9n ~ 0.100
SubtreeProcessorAdd/256_per_subtree-4 315.0n 291.0n ~ 0.100
SubtreeProcessorAdd/1024_per_subtree-4 299.3n 282.3n ~ 0.100
SubtreeProcessorAdd/2048_per_subtree-4 297.7n 283.3n ~ 0.100
SubtreeProcessorRotate/4_per_subtree-4 299.4n 285.9n ~ 0.200
SubtreeProcessorRotate/64_per_subtree-4 296.8n 283.8n ~ 0.200
SubtreeProcessorRotate/256_per_subtree-4 300.0n 299.6n ~ 1.000
SubtreeProcessorRotate/1024_per_subtree-4 390.7n 313.4n ~ 0.100
SubtreeNodeAddOnly/4_per_subtree-4 56.13n 55.88n ~ 0.100
SubtreeNodeAddOnly/64_per_subtree-4 36.39n 36.36n ~ 0.700
SubtreeNodeAddOnly/256_per_subtree-4 35.37n 35.31n ~ 0.600
SubtreeNodeAddOnly/1024_per_subtree-4 34.73n 34.83n ~ 0.700
SubtreeCreationOnly/4_per_subtree-4 113.2n 114.5n ~ 0.200
SubtreeCreationOnly/64_per_subtree-4 371.5n 371.0n ~ 1.000
SubtreeCreationOnly/256_per_subtree-4 1.295µ 1.286µ ~ 0.100
SubtreeCreationOnly/1024_per_subtree-4 4.027µ 4.025µ ~ 0.700
SubtreeCreationOnly/2048_per_subtree-4 7.430µ 7.456µ ~ 1.000
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 380.9n 297.6n ~ 0.100
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 333.1n 295.3n ~ 0.100
ParallelGetAndSetIfNotExists/1k_nodes-4 2.117m 2.039m ~ 0.100
ParallelGetAndSetIfNotExists/10k_nodes-4 5.459m 5.211m ~ 0.100
ParallelGetAndSetIfNotExists/50k_nodes-4 8.445m 7.271m ~ 0.100
ParallelGetAndSetIfNotExists/100k_nodes-4 12.38m 10.32m ~ 0.100
SequentialGetAndSetIfNotExists/1k_nodes-4 1.925m 1.789m ~ 0.100
SequentialGetAndSetIfNotExists/10k_nodes-4 7.064m 4.548m ~ 0.100
SequentialGetAndSetIfNotExists/50k_nodes-4 23.86m 15.08m ~ 0.100
SequentialGetAndSetIfNotExists/100k_nodes-4 40.40m 26.95m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 2.186m 2.093m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 8.936m 8.482m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 15.34m 13.84m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 1.952m 1.826m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 11.154m 9.165m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 83.96m 45.46m ~ 0.100
DiskTxMap_SetIfNotExists-4 3.876µ 3.861µ ~ 0.700
DiskTxMap_SetIfNotExists_Parallel-4 3.689µ 3.707µ ~ 1.000
DiskTxMap_ExistenceOnly-4 326.1n 315.2n ~ 0.100
Queue-4 204.7n 197.3n ~ 0.100
AtomicPointer-4 8.133n 8.133n ~ 1.000
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 833.6µ 799.7µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/New/10K-4 731.4µ 712.6µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 118.4µ 108.3µ ~ 0.100
ReorgOptimizations/AllMarkFalse/New/10K-4 58.49µ 57.96µ ~ 0.100
ReorgOptimizations/HashSlicePool/Old/10K-4 67.94µ 69.86µ ~ 0.400
ReorgOptimizations/HashSlicePool/New/10K-4 11.77µ 11.79µ ~ 0.200
ReorgOptimizations/NodeFlags/Old/10K-4 5.036µ 4.682µ ~ 0.400
ReorgOptimizations/NodeFlags/New/10K-4 1.579µ 1.583µ ~ 0.400
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 9.357m 9.041m ~ 0.700
ReorgOptimizations/DedupFilterPipeline/New/100K-4 10.20m 10.14m ~ 0.700
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.160m 1.120m ~ 0.100
ReorgOptimizations/AllMarkFalse/New/100K-4 724.2µ 724.1µ ~ 1.000
ReorgOptimizations/HashSlicePool/Old/100K-4 576.2µ 566.7µ ~ 0.100
ReorgOptimizations/HashSlicePool/New/100K-4 305.3µ 319.4µ ~ 0.200
ReorgOptimizations/NodeFlags/Old/100K-4 49.31µ 49.72µ ~ 0.700
ReorgOptimizations/NodeFlags/New/100K-4 16.97µ 17.45µ ~ 0.100
TxMapSetIfNotExists-4 51.52n 52.49n ~ 0.200
TxMapSetIfNotExistsDuplicate-4 48.01n 48.29n ~ 0.700
ChannelSendReceive-4 679.1n 662.7n ~ 0.100
CalcBlockWork-4 475.6n 485.3n ~ 1.000
CalculateWork-4 640.0n 643.2n ~ 1.000
BuildBlockLocatorString_Helpers/Size_10-4 1.480µ 1.353µ ~ 1.000
BuildBlockLocatorString_Helpers/Size_100-4 12.81µ 12.88µ ~ 0.400
BuildBlockLocatorString_Helpers/Size_1000-4 127.5µ 127.2µ ~ 0.700
CatchupWithHeaderCache-4 104.1m 104.5m ~ 0.100
_prepareTxsPerLevel-4 412.2m 413.9m ~ 1.000
_prepareTxsPerLevelOrdered-4 3.815m 4.008m ~ 0.100
_prepareTxsPerLevel_Comparison/Original-4 418.8m 414.4m ~ 0.400
_prepareTxsPerLevel_Comparison/Optimized-4 3.905m 3.801m ~ 0.100
_BufferPoolAllocation/16KB-4 5.208µ 3.808µ ~ 0.100
_BufferPoolAllocation/32KB-4 9.501µ 8.453µ ~ 0.100
_BufferPoolAllocation/64KB-4 16.21µ 20.96µ ~ 0.200
_BufferPoolAllocation/128KB-4 29.14µ 35.05µ ~ 0.100
_BufferPoolAllocation/512KB-4 113.4µ 141.7µ ~ 0.100
_BufferPoolConcurrent/32KB-4 19.10µ 19.46µ ~ 1.000
_BufferPoolConcurrent/64KB-4 30.86µ 30.90µ ~ 1.000
_BufferPoolConcurrent/512KB-4 150.1µ 153.5µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/16KB-4 626.5µ 705.8µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/32KB-4 634.8µ 700.0µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/64KB-4 652.3µ 696.9µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/128KB-4 641.0µ 702.2µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/512KB-4 621.3µ 637.2µ ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/16KB-4 37.43m 37.72m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/32KB-4 37.28m 37.93m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/64KB-4 37.12m 38.00m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/128KB-4 36.85m 38.21m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/512KB-4 36.28m 37.50m ~ 0.200
_PooledVsNonPooled/Pooled-4 842.5n 833.7n ~ 0.700
_PooledVsNonPooled/NonPooled-4 7.668µ 7.975µ ~ 0.100
_MemoryFootprint/Current_512KB_32concurrent-4 7.219µ 7.708µ ~ 0.100
_MemoryFootprint/Proposed_32KB_32concurrent-4 9.710µ 10.139µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 9.475µ 10.010µ ~ 0.100
SubtreeSizes/10k_tx_4_per_subtree-4 1.267m 1.278m ~ 0.400
SubtreeSizes/10k_tx_16_per_subtree-4 295.6µ 298.7µ ~ 0.700
SubtreeSizes/10k_tx_64_per_subtree-4 72.10µ 71.90µ ~ 0.700
SubtreeSizes/10k_tx_256_per_subtree-4 17.55µ 17.89µ ~ 0.100
SubtreeSizes/10k_tx_512_per_subtree-4 8.686µ 8.823µ ~ 0.200
SubtreeSizes/10k_tx_1024_per_subtree-4 4.362µ 4.362µ ~ 0.800
SubtreeSizes/10k_tx_2k_per_subtree-4 2.169µ 2.176µ ~ 1.000
BlockSizeScaling/10k_tx_64_per_subtree-4 68.66µ 69.34µ ~ 0.100
BlockSizeScaling/10k_tx_256_per_subtree-4 17.40µ 17.34µ ~ 1.000
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.305µ 4.327µ ~ 0.700
BlockSizeScaling/50k_tx_64_per_subtree-4 362.7µ 371.1µ ~ 0.700
BlockSizeScaling/50k_tx_256_per_subtree-4 87.05µ 86.54µ ~ 1.000
BlockSizeScaling/50k_tx_1024_per_subtree-4 21.42µ 21.38µ ~ 1.000
SubtreeAllocations/small_subtrees_exists_check-4 150.9µ 147.5µ ~ 0.700
SubtreeAllocations/small_subtrees_data_fetch-4 157.2µ 160.5µ ~ 0.200
SubtreeAllocations/small_subtrees_full_validation-4 308.2µ 308.1µ ~ 1.000
SubtreeAllocations/medium_subtrees_exists_check-4 8.763µ 8.831µ ~ 1.000
SubtreeAllocations/medium_subtrees_data_fetch-4 9.193µ 9.295µ ~ 0.700
SubtreeAllocations/medium_subtrees_full_validation-4 17.43µ 17.35µ ~ 1.000
SubtreeAllocations/large_subtrees_exists_check-4 2.065µ 2.086µ ~ 0.300
SubtreeAllocations/large_subtrees_data_fetch-4 2.237µ 2.203µ ~ 0.400
SubtreeAllocations/large_subtrees_full_validation-4 4.341µ 4.272µ ~ 0.200
StoreBlock_Sequential/BelowCSVHeight-4 311.7µ 320.6µ ~ 0.100
StoreBlock_Sequential/AboveCSVHeight-4 322.8µ 315.7µ ~ 0.400
GetUtxoHashes-4 259.5n 262.2n ~ 1.000
GetUtxoHashes_ManyOutputs-4 52.81µ 55.81µ ~ 0.100
_NewMetaDataFromBytes-4 253.4n 247.3n ~ 0.200
_Bytes-4 477.2n 448.6n ~ 0.100
_MetaBytes-4 146.0n 147.0n ~ 1.000

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

@freemans13 freemans13 self-assigned this Apr 9, 2026
@sonarqubecloud

sonarqubecloud Bot commented Apr 23, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

freemans13 and others added 8 commits May 14, 2026 09:32
@freemans13
Merge remote-tracking branch 'upstream/main' into fix/dont-filter-blo…
201e988 
…cks-from-unhealthy-peers
@freemans13
Merge remote-tracking branch 'upstream/main' into fix/dont-filter-blo…
c8c067d 
…cks-from-unhealthy-peers
@freemans13
Merge remote-tracking branch 'upstream/main' into fix/dont-filter-blo…
d2cbb40 
…cks-from-unhealthy-peers
@freemans13
Merge remote-tracking branch 'upstream/main' into fix/dont-filter-blo…
315ca7b 
…cks-from-unhealthy-peers
@freemans13
Merge remote-tracking branch 'upstream/main' into HEAD
ee45d94
@freemans13
Merge upstream/main into fix/dont-filter-blocks-from-unhealthy-peers
9bffde9
@freemans13 @claude
docs(p2p): explain why handleBlockTopic omits the reputation filter
79a5335 
Expand the comment to document the block-vs-subtree asymmetry: block
announcements trigger catchup, so filtering them from low-reputation
peers would stop a node that is behind from ever catching up. Catchup
fetches blocks and their subtrees directly over HTTP rather than via
the gossip subtree handler, so the reputation filter retained in
handleSubtreeTopic does not affect catchup.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@freemans13 @claude
test(p2p): guard that low-reputation peers' blocks still reach Kafka
2ac59ae 
handleBlockTopic must forward a block announcement to the blocks Kafka
topic even when the originating peer's reputation is below the unhealthy
threshold, otherwise a node that is behind cannot start catchup when its
only available peers have low reputation. Pins a registered peer to a
reputation of 5.0 (not banned) and asserts the block is published.

Verified the test fails when a shouldSkipUnhealthyPeer filter is
reintroduced into handleBlockTopic and passes without it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@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
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@freemans13 freemans13 requested a review from ordishs June 4, 2026 16:06
@freemans13 freemans13 merged commit 5f7ee97 into bsv-blockchain:main Jun 5, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ordishs ordishs ordishs approved these changes

@oskarszoon oskarszoon oskarszoon approved these changes

Assignees

@freemans13 freemans13

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@freemans13 @ordishs @oskarszoon