Merge bitcoin/bitcoin#27608: p2p: Avoid prematurely clearing download state for other peers

[DashCoreAutoGuix]

Backports bitcoin#27608

Original commit: dbfc748

This PR avoids letting one peer send us data that clears out the download request (and related timers etc) from another peer.

The exception is if a block is definitely stored to disk, in which case we'll clear the download state (just as we do for compact blocks).

Summary by CodeRabbit

• Bug Fixes
  • Improved block request management to prevent state interference when multiple peers request the same block, enhancing network synchronization reliability and robustness.

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

[coderabbitai]

Walkthrough

This PR updates the RemoveBlockRequest function signature in PeerManagerImpl to add an optional peer filter parameter, enabling block request removal to be scoped to specific peers. All existing call sites throughout the file are updated to supply the new parameter.

Changes

Cohort / File(s)	Change Summary
Block request peer filtering src/net_processing.cpp	Updated RemoveBlockRequest function signature from void RemoveBlockRequest(const uint256& hash) to void RemoveBlockRequest(const uint256& hash, std::optional<NodeId> from_peer). All call sites throughout the file are updated to pass the new from_peer parameter, with std::nullopt indicating no peer-filtering and specific NodeId values enabling peer-scoped removal.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify all RemoveBlockRequest call sites have been updated with the new parameter
• Confirm std::nullopt vs. specific peer ID usage is semantically correct at each call site (e.g., after block processing, cmpctblock/blocktxn completion, and per-peer request handling)
• Review the early-return logic that gates removal when from_peer is specified and does not match the tracked in-flight peer to ensure race condition prevention

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 title accurately reflects the main change: avoiding premature clearing of download state for other peers by adding per-peer filtering to RemoveBlockRequest.

✨ 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-27608

---

📜 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 d08e8e5.

📒 Files selected for processing (1)

• src/net_processing.cpp (10 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

C++20 codebase should be placed under src/

Files:

• 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

• bitcoin → dash in strings/paths
• Bitcoin → Dash 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_processing.cpp

🧠 Learnings (2)

📓 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-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-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-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.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T22:13:54.797Z
Learning: In Dash backports from Bitcoin Core, witness transaction-related code (MSG_WTX, wtxid) should be replaced with regular transaction handling (MSG_TX, txid) for compatibility, as demonstrated in the p2p_filter.py test fix where MSG_WTX was replaced with MSG_TX and irr_wtxid usage was replaced with irr_txid.

📚 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

⏰ 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). (2)

• GitHub Check: Lint / Run linters
• GitHub Check: Build container / Build container

🔇 Additional comments (1)

src/net_processing.cpp (1)

1019-1050: Backport validation summary for bitcoin#27608

VALIDATION: PASS

File Operations: MATCH (This Dash backport only changes src/net_processing.cpp, which is the only runtime file known from upstream metadata to be touched by PR 27608; please cross‑check tests/docs on the Bitcoin side if needed.) (mirror.b10c.me)

Size Ratio: ≈100% (Bitcoin: ≈13 functional lines affecting RemoveBlockRequest and its call sites; Dash: 13 functional lines). Counts are based on upstream coverage/code snippets rather than an authoritative diff, but the shapes and number of modified sites match. (maflcko.github.io)

Scope Creep: NONE. All edits are strictly in-flight block bookkeeping (signature + implementation of RemoveBlockRequest and the expected call sites in BlockRequested, ProcessBlock, CMPCTBLOCK/BLOCKTXN/BLOCK handling). No unrelated refactors, no new logging, and no behavioral changes outside the download-state clearing logic.

Bitcoin-Specific Code: NONE. The backport does not introduce segwit/wtxid/RBF or other Bitcoin-only features; it only threads an optional NodeId through existing Dash block-download plumbing.

Completeness: COMPLETE (for runtime behavior). Every upstream call site that should be peer-scoped now passes either a specific NodeId (when clearing state in response to that peer’s message) or std::nullopt when we intentionally clear download state regardless of which peer we requested from (eg, after a block is stored or before re-requesting). This matches the intent and structure of upstream PR 27608 as reflected in current Bitcoin Core net_processing.cpp. (maflcko.github.io)

Please double-check the upstream PR’s “Files changed” view offline to confirm there were no accompanying test/doc changes you also want in this backport.

Also applies to: 1221-1257, 1259-1291, 3579-3594, 4779-4899, 4987-5052, 5109-5127

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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