Skip to content

Conversation

@DashCoreAutoGuix
Copy link
Owner

@DashCoreAutoGuix DashCoreAutoGuix commented Dec 3, 2025

Summary

Backport of bitcoin#27626 to Dash Core.

This PR attempts to mitigate a single case, where high bandwidth peers can bail us out of a flakey peer not completing blocks for us. We allow up to 2 additional getblocktxns requests per unique block. This would hopefully allow the chance for an honest high bandwidth peer to hand us the transactions even if the first in flight peer stalls out.

Key Changes:

  • Convert mapBlocksInFlight from std::map to std::multimap to allow multiple peers to be downloading the same block
  • Add IsBlockRequestedFromOutbound() to check if block is being fetched from an outbound peer
  • Update RemoveBlockRequest() to take optional from_peer parameter to selectively remove only that peer's request
  • Add MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK = 3 constant
  • Remove nBlocksInFlight counter, use vBlocksInFlight.size() instead
  • Add parallel download test in p2p_compactblocks.py

Validation:

  • ✅ Build succeeds
  • ✅ Size ratio: 106% (337 vs 318 lines)
  • ✅ Same files modified as Bitcoin (5 files)
  • ✅ Unit tests pass

Bitcoin PR Reference

bitcoin#27626

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Optimized block download performance with improved multi-peer coordination
    • Enhanced compact block reconstruction across multiple peer connections
  • Documentation

    • Updated getblockfrompeer RPC method documentation

✏️ Tip: You can customize this high-level summary in your review settings.

This is a backport of bitcoin#27626

This PR attempts to mitigate a single case, where high bandwidth peers
can bail us out of a flakey peer not completing blocks for us. We allow
up to 2 additional getblocktxns requests per unique block. This would
hopefully allow the chance for an honest high bandwidth peer to hand us
the transactions even if the first in flight peer stalls out.

Changes:
- Convert mapBlocksInFlight from std::map to std::multimap to allow
  multiple peers to be downloading the same block
- Add IsBlockRequestedFromOutbound() to check if block is being
  fetched from an outbound peer
- Update RemoveBlockRequest() to take optional from_peer parameter
  to selectively remove only that peers request
- Add MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK = 3 constant
- Remove nBlocksInFlight counter, use vBlocksInFlight.size() instead
- Add parallel download test in p2p_compactblocks.py

Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

This pull request refactors in-flight block request tracking to support per-peer, per-block state instead of single-peer tracking. It introduces a multimap-based data structure replacing a map, adds BIP152 high-bandwidth tracking fields, introduces new methods for outbound peer queries, and updates compact block workflows accordingly. A new constant limits concurrent CMPCTBLOCK requests per block, and test coverage for parallel peer reconstruction is added.

Changes

Cohort / File(s) Summary
Data structure enhancements
src/net.h, src/net_processing.h
Added m_bip152_highbandwidth_to and m_bip152_highbandwidth_from boolean fields to CNodeStats; introduced MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK constant set to 3.
In-flight block tracking refactoring
src/net_processing.cpp
Replaced mapBlocksInFlight map with multimap BlockDownloadMap to track per-peer, per-block requests; introduced IsBlockRequestedFromOutbound() method; extended RemoveBlockRequest() to accept optional from_peer parameter; replaced deprecated nBlocksInFlight counter with vBlocksInFlight.size() checks; updated block fetch, stall detection, eviction, and compact block workflows to use range-based iteration and per-peer removal semantics; adjusted multiple in-flight state transitions for multi-peer scenarios.
Documentation
src/rpc/blockchain.cpp
Updated getblockfrompeer RPC help text from "will cause" to "may cause" regarding response handling for subsequent calls.
Test coverage
test/functional/p2p_compactblocks.py
Added clear_getblocktxn() helper method to TestP2PConn; renamed test_compactblock_reconstruction_multiple_peers() to test_compactblock_reconstruction_stalling_peer(); introduced new test_compactblock_reconstruction_parallel_reconstruction() test with four-peer setup to validate parallel block reconstruction; expanded p2p peer instances with onemore_inbound_node and outbound_node.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • src/net_processing.cpp: Extensive refactoring of in-flight tracking logic affecting block synchronization state machine; multimap transition requires verification of all call sites and state transition correctness across block fetch, stall detection, eviction, and compact block workflows.
  • Per-peer in-flight semantics: Verify that the new multimap-based model correctly handles multiple in-flight requests for the same block hash across different peers, particularly in outbound vs. inbound distinctions and CMPCTBLOCK request limiting.
  • Block fetch and timeout handling: Complex interdependencies between the new multimap, vBlocksInFlight size checks, and per-block/per-peer limits require careful validation across diverse code paths.
  • Test expansion: New parallel reconstruction test validates multi-peer scenarios; verify that test assertions correctly cover edge cases in concurrent peer responses.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: it's a backport of Bitcoin PR bitcoin#27626 focused on enabling parallel compact block downloads.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport-0.26-batch-477-pr-27626

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e63410e and 7b8553f.

📒 Files selected for processing (5)
  • src/net.h (1 hunks)
  • src/net_processing.cpp (20 hunks)
  • src/net_processing.h (1 hunks)
  • src/rpc/blockchain.cpp (1 hunks)
  • test/functional/p2p_compactblocks.py (4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

C++20 codebase should be placed under src/

Files:

  • src/net.h
  • src/rpc/blockchain.cpp
  • src/net_processing.h
  • src/net_processing.cpp
**

⚙️ CodeRabbit configuration file

**: # CodeRabbit AI Review Instructions for Dash Backports

Your Role

You are reviewing Bitcoin Core backports to Dash Core. Your ONLY job is to validate that the Dash commit faithfully represents the original Bitcoin commit with minimal, necessary adaptations.

Critical Validation Rules

1. File Operations Must Match (AUTO-REJECT if violated)

  • If Bitcoin modifies an existing file → Dash MUST modify (not create new)
  • If Bitcoin creates a new file → Dash creates
  • If Bitcoin deletes a file → Dash deletes
  • Common failure: Bitcoin modifies keys.txt, Dash creates new file with 58 keys

2. Size Ratio Check (80-150% of Bitcoin)

  • Count functional lines changed (exclude comments/whitespace)
  • Dash changes should be 80-150% of Bitcoin's size
  • Red flag: 2-line Bitcoin fix becoming 150+ lines in Dash

3. No Scope Creep

  • Reject if you see: "TODO:", "FIXME:", "while we're here", "also fix"
  • No unrelated refactoring or style changes
  • Only Bitcoin's intended changes + minimal Dash adaptations

4. Bitcoin-Specific Code Detection

  • Auto-reject witness/segwit code: msg_wtxidrelay, MSG_WTX, witness imports
  • Auto-reject RBF (replace-by-fee) functionality
  • Note: PSBT is supported in Dash (don't flag)

5. Mandatory Adaptations Only

  • bitcoindash in strings/paths
  • BitcoinDash in user-facing text
  • Port numbers: 8332→9998 (RPC), 8333→9999 (P2P)
  • Hardcoded test values specific to Dash
  • No other changes unless absolutely required

6. Completeness Check

  • All files changed in Bitcoin must be present
  • Extra files need clear justification (Dash-specific compatibility)
  • Missing files = incomplete backport

Review Process

  1. First: Check file operations match exactly
  2. Second: Calculate size ratio
  3. Third: Scan for scope creep patterns
  4. Fourth: Detect Bitcoin-specific code
  5. Fifth: Verify all changes are minimal adaptations

Output Format

VALIDATION: [PASS/FAIL]

File Operatio...

Files:

  • src/net.h
  • src/rpc/blockchain.cpp
  • src/net_processing.h
  • src/net_processing.cpp
  • test/functional/p2p_compactblocks.py
test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests should be placed in test/functional/ and written in Python

Files:

  • test/functional/p2p_compactblocks.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-31T01:14:55.631Z
Learning: DashCoreAutoGuix successfully completed a complex Bitcoin Core backport (PR #29412) for block mutation detection by implementing the IsBlockMutated function, adding net processing integration, creating comprehensive unit tests, and properly adapting all Bitcoin-specific witness code for Dash compatibility. The backport maintains full security functionality while respecting Dash's non-witness transaction architecture.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T19:54:21.426Z
Learning: In Dash backports from Bitcoin Core, including necessary compilation fixes (such as API compatibility changes like UniValue get_int() → getInt<int>()) alongside the core backport is standard and expected practice. These compatibility fixes ensure the backported code compiles in Dash's evolved codebase while preserving Bitcoin's original functionality and intent.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T21:29:32.827Z
Learning: DashCoreAutoGuix successfully fixed scope creep in Bitcoin Core commit fcdb39d3ee backport by removing the parse test case from src/test/uint256_tests.cpp that was not part of the original Bitcoin commit. The fix was implemented in commit 16748115ce and verified through range-diff analysis.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: When backporting Bitcoin Core commits that use Python's textwrap.dedent() function in test files, the textwrap import statement needs to be explicitly added if it's missing in the Dash test file.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-27T22:35:10.176Z
Learning: In Dash backports, src/dashbls files are vendored dependencies that should not be modified during Bitcoin Core backports unless there is specific justification. Unauthorized modifications to vendored dependencies should be removed to maintain code integrity.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: In Dash backports from Bitcoin Core, when the DIFFICULTY_ADJUSTMENT_INTERVAL constant is missing, it should be defined as 24 for Dash (different from Bitcoin's value), as seen in the getnetworkhashps RPC backport fix.
📚 Learning: 2025-07-31T01:14:55.631Z
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-31T01:14:55.631Z
Learning: DashCoreAutoGuix successfully completed a complex Bitcoin Core backport (PR #29412) for block mutation detection by implementing the IsBlockMutated function, adding net processing integration, creating comprehensive unit tests, and properly adapting all Bitcoin-specific witness code for Dash compatibility. The backport maintains full security functionality while respecting Dash's non-witness transaction architecture.

Applied to files:

  • src/net_processing.cpp
📚 Learning: 2025-07-28T20:34:29.061Z
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.

Applied to files:

  • test/functional/p2p_compactblocks.py
📚 Learning: 2025-07-28T22:03:12.364Z
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.

Applied to files:

  • test/functional/p2p_compactblocks.py
🧬 Code graph analysis (2)
src/net_processing.cpp (1)
src/util/time.h (1)
  • count_seconds (90-90)
test/functional/p2p_compactblocks.py (3)
test/functional/test_framework/p2p.py (1)
  • send_and_ping (750-752)
test/functional/test_framework/util.py (1)
  • assert_equal (69-74)
test/functional/test_framework/test_node.py (2)
  • add_p2p_connection (651-708)
  • add_outbound_p2p_connection (710-772)
🪛 Ruff (0.14.7)
test/functional/p2p_compactblocks.py

798-798: Missing return type annotation for private function announce_cmpct_block

(ANN202)


811-811: Logging statement uses f-string

(G004)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: mac-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: Lint / Run linters
  • GitHub Check: x86_64-pc-linux-gnu / Build depends
🔇 Additional comments (22)
src/rpc/blockchain.cpp (2)

487-487: Wording change aligns with parallel download semantics.

The help text clarification appropriately reflects the new per-peer, per-block in-flight tracking introduced in this backport. Changing "will cause" to "may cause" is semantically correct—with parallel downloads enabled, responses from prior peers may or may not be ignored depending on parallel request state.


481-530: Backport validation incomplete: Only 1 of 5 modified files provided.

Per the PR objectives, this backport modifies 5 files across the networking and block-fetch code paths. However, only src/rpc/blockchain.cpp is provided for review. To fully validate the backport against the coding guidelines (file operations match, size ratio 80–150%, no scope creep, no Bitcoin-specific code), I need to review the other 4 modified files.

Please provide:

  1. The other 4 modified files
  2. Confirmation of the original Bitcoin commit hash for cross-reference

This will allow verification of:

  • mapBlocksInFlight multimap conversion
  • IsBlockRequestedFromOutbound() implementation
  • RemoveBlockRequest() signature changes
  • MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK constant
  • Any Dash-specific adaptations (e.g., witness code exclusions, API compatibility fixes)
src/net_processing.h (1)

43-44: LGTM - Faithful backport of MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK constant.

The constant value of 3 and the comment describing it as the "Maximum number of outstanding CMPCTBLOCK requests for the same block" aligns with Bitcoin Core PR bitcoin#27626's implementation for parallel compact block downloads.

src/net.h (1)

224-227: LGTM - BIP152 high-bandwidth state tracking fields added correctly.

The new m_bip152_highbandwidth_to and m_bip152_highbandwidth_from fields in CNodeStats correspond to the existing atomic fields in CNode (lines 926-928) and enable RPC exposure of BIP152 high-bandwidth mode states. This is a faithful backport from Bitcoin Core PR bitcoin#27626.

test/functional/p2p_compactblocks.py (5)

105-107: LGTM - Helper method follows established pattern.

The clear_getblocktxn() method follows the same pattern as clear_block_announcement() (lines 98-103), providing a clean way to reset state between test iterations.


791-859: LGTM - Comprehensive test for parallel compact block reconstruction.

The test correctly validates the key behavior from Bitcoin Core PR bitcoin#27626:

  1. Lines 828-832: Second peer to announce gets getblocktxn (2nd parallel slot)
  2. Lines 834-838: Third inbound peer does NOT get getblocktxn (slot reserved for outbound)
  3. Lines 840-844: Third outbound peer DOES get getblocktxn (takes the reserved outbound slot)

This validates that the 3rd slot (of MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK = 3) is reserved for outbound peers to prevent potential attack vectors where an attacker could fill all slots with inbound connections.

The iteration over num_missing in [1, 5, 20] (line 821) provides good coverage across different transaction counts.


868-869: LGTM - New peer connections properly configured.

The outbound_node is correctly configured as "outbound-full-relay" connection type, which is necessary for testing the outbound peer preference in the parallel reconstruction logic. The p2p_idx=3 is appropriate given the existing peer indices.


895-899: LGTM - Test integration is correct.

The renamed test test_compactblock_reconstruction_stalling_peer and new test test_compactblock_reconstruction_parallel_reconstruction are properly integrated into run_test(). The parameter mapping correctly assigns peers to their roles based on connection type (inbound vs outbound).


821-859: Consider clearing stalling_peer state for test robustness.

Unlike delivery_peer, inbound_peer, and outbound_peer which have clear_getblocktxn() called at lines 857-859, stalling_peer doesn't have its state cleared between iterations. While the test should still pass (since stalling_peer announces first and will receive a new getblocktxn), explicitly clearing its state would make the test more robust and consistent with the pattern used for other peers.

             # Nothing bad should happen if we get a late fill from the first peer...
             stalling_peer.send_and_ping(msg)
             self.utxos.append([block.vtx[-1].sha256, 0, block.vtx[-1].vout[0].nValue])

+            stalling_peer.clear_getblocktxn()
             delivery_peer.clear_getblocktxn()
             inbound_peer.clear_getblocktxn()
             outbound_peer.clear_getblocktxn()
⛔ Skipped due to learnings
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.
src/net_processing.cpp (13)

1018-1048: New block-request helpers and multimap alias look faithful to upstream design

  • Adding IsBlockRequestedFromOutbound and extending RemoveBlockRequest with an optional from_peer parameter matches the per-peer, per-block bookkeeping introduced by Bitcoin PR Parallel compact block downloads, take 3 bitcoin/bitcoin#27626.
  • Converting mapBlocksInFlight to the BlockDownloadMap multimap (block hash → (peer, queued-block iterator)) is exactly what’s needed to support multiple in-flight peers per block without changing Dash-specific behavior elsewhere.

1225-1239: IsBlockRequested / IsBlockRequestedFromOutbound correctly use the multimap

  • IsBlockRequested() now delegates to mapBlocksInFlight.count(hash), which is the right abstraction once multiple entries per hash are allowed.
  • IsBlockRequestedFromOutbound() iterates equal_range(hash) and checks !nodestate.m_is_inbound, which is the minimal, correct way to detect at least one outbound downloader for that block.
  • No scope creep or Dash-specific behavior added here.

1241-1276: RemoveBlockRequest: per-peer removal and counters updated correctly

  • Using equal_range(hash) over the multimap and conditionally filtering by from_peer mirrors Bitcoin’s logic and ensures we can:
    • Drop all in-flight entries for a block, or
    • Drop just a single peer’s entry, without disturbing other peers’ state.
  • Updating state.vBlocksInFlight, state.m_downloading_since, state.m_stalling_since, and m_peers_downloading_from in one place keeps the invariants that other code relies on (stall detection, download timeouts).
  • The Assume(mapBlocksInFlight.count(hash) <= MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK) sanity check matches the upstream safety invariant.

1278-1308: BlockRequested: multimap integration and vBlocksInFlight usage are consistent

  • The early-return path scanning equal_range(hash) for an existing entry from nodeid is the right way to avoid double-booking the same (block, peer) pair in a multimap.
  • The subsequent RemoveBlockRequest(hash, nodeid) call is consistent with upstream (even if usually a no-op after the early scan) and keeps behavior aligned.
  • Inserting into state->vBlocksInFlight and incrementing m_peers_downloading_from only when size() == 1 cleanly replaces the old nBlocksInFlight counter without adding any Dash-specific quirks.

1484-1510: FindNextBlocksToDownload: staller identification correctly updated for multimap

  • When a candidate block is already in-flight, waitingfor is now sourced from mapBlocksInFlight.lower_bound(pindex->GetBlockHash())->second.first, which picks the first (earliest) entry for that hash in the multimap, matching Bitcoin’s “first requester” notion for potential stallers.
  • This is consistent with the new multi-peer per-block model and doesn’t introduce extra behavior beyond what upstream PR Parallel compact block downloads, take 3 bitcoin/bitcoin#27626 does.

1788-1802: FinalizeNode: in-flight map cleanup for this peer is correct

  • Iterating state->vBlocksInFlight and erasing only entries from mapBlocksInFlight whose node_id equals this peer ensures we:
    • Don’t disturb parallel in-flight entries for other peers on the same block.
    • Avoid leaving dangling iterators in the multimap when the peer disconnects.
  • Adjusting m_peers_downloading_from via !state->vBlocksInFlight.empty() is the expected replacement for the old nBlocksInFlight-based accounting.

2039-2045: FetchBlock: clearing prior in-flight state before RPC re-request is intentional

  • Calling RemoveBlockRequest(block_index.GetBlockHash(), std::nullopt) before BlockRequested(peer_id, block_index) correctly forgets any prior in-flight state for that block across all peers, matching Bitcoin’s behavior for RPC-driven retries.
  • This avoids mixed state (old peers still “owning” the block) and ensures the RPC-selected peer is now the single source tracked for that block.

3177-3199: HeadersDirectFetchBlocks: per-peer budget and global inflight count updated safely

  • The per-peer cap check nodestate->vBlocksInFlight.size() >= MAX_BLOCKS_IN_TRANSIT_PER_PEER and the global check mapBlocksInFlight.size() == 1 are the direct replacements for the removed nBlocksInFlight counter and align with upstream parallel-download semantics.
  • This preserves the behavior that only when a single block is in-flight globally, and exactly one is requested here, we consider switching to compact blocks for that tip.

4873-5039: CMPCTBLOCK handling: multi-peer in-flight logic matches upstream mitigation

  • The new equal_range / already_in_flight / requested_block_from_this_peer / first_in_flight logic is a faithful port of Bitcoin’s mitigation for CVE-2024-52922:
    • Limits concurrent CMPCTBLOCK fetches per block via MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK.
    • Ensures at least one high-bandwidth outbound peer participates via IsBlockRequestedFromOutbound() in the secondary GETBLOCKTXN path.
    • Falls back to full GETDATA only from the peer we treat as first-in-flight, and otherwise drops per-peer state when reconstruction fails.
  • Using RemoveBlockRequest to reset per-peer or per-block inflight state on invalid/failed compact blocks keeps Dash’s internal counters and queues aligned with Bitcoin’s structure, with no added Dash-only behavior here.

5055-5117: BLOCKTXN reconstruction: per-peer inflight validation and fallback logic are correct

  • Reusing the same equal_range/already_in_flight/first_in_flight pattern for BLOCKTXN ensures we only process BLOCKTXN from peers we actually requested partial data from, and only where a partialBlock exists.
  • On READ_STATUS_INVALID, calling RemoveBlockRequest(resp.blockhash, pfrom.GetId()) before misbehavior handling exactly mirrors upstream and avoids leaving stale in-flight entries if the peer survives.
  • On READ_STATUS_FAILED, only the first-in-flight peer triggers a full GETDATA retry, while other peers simply have their inflight entry cleared and log a message, matching the multi-peer parallel download design.
  • Successful reconstruction clears all inflight entries for that block with RemoveBlockRequest(resp.blockhash, std::nullopt) and sets force_processing=true, as in the upstream PR.

5193-5203: BLOCK handling: centralized inflight cleanup via RemoveBlockRequest

  • Using RemoveBlockRequest(hash, std::nullopt) after computing forceProcessing = IsBlockRequested(hash) is the right way to retire all inflight records for this block once a full block is received, regardless of which peer it arrived from.
  • This keeps the multimap and per-peer vBlocksInFlight lists consistent without adding any new behavior vs. Bitcoin’s implementation.

5768-5832: Outbound eviction logic: nBlocksInFlight removal is cleanly handled

  • Both eviction sites now gate disconnection on vBlocksInFlight.empty() instead of an explicit nBlocksInFlight counter, and log vBlocksInFlight.size() in diagnostics.
  • This preserves the policy of never evicting peers we are actively downloading blocks from, while aligning the implementation with the new per-peer list structure.

6538-6554: SendMessages block getdata scheduling aligns with new inflight tracking

  • The block GETDATA condition now uses state.vBlocksInFlight.size() < MAX_BLOCKS_IN_TRANSIT_PER_PEER and FindNextBlocksToDownload(*peer, …); this is consistent with the multimap-based inflight representation and upstream’s parallel download window handling.
  • Stall detection uses state.vBlocksInFlight.empty() and updates State(staller)->m_stalling_since when no blocks can be queued, which matches the logic introduced in Bitcoin PR Parallel compact block downloads, take 3 bitcoin/bitcoin#27626 while respecting Dash’s existing CanServeBlocks / CanRelay gating.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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