backport: Merge bitcoin#27151, 25849

[vijaydasmp]

bitcoin backports

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

The pull request introduces peer-aware block request removal by modifying the RemoveBlockRequest method to accept an optional from_peer parameter, ensuring blocks are only removed from the in-flight map when requested from a specific peer. Multiple call sites throughout block processing logic are updated to pass appropriate peer context (or std::nullopt where peer-specific removal is not applicable). Additionally, an #include <utility> header is removed from a support file, and RPC help text formatting is simplified in wallet spend methods.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	⚠️ Warning	The title references bitcoin backports (#27151, #25849) but omits #27608 mentioned in the PR description, creating a mismatch between the title and the actual changeset.	Update the title to include all three bitcoin PR numbers: 'backport: Merge bitcoin#27151, #25849, #27608' to accurately reflect all changes in the PR.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description 'bitcoin backports' is related to the changeset as it correctly identifies the PR's purpose of merging upstream Bitcoin changes, though it lacks detail about the specific changes involved.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[vijaydasmp]

Hello @Udjin6 @knst

[vijaydasmp]

Hello @UdjinM6 @knst requesting review

[thepastaclaw]

Code Review

Clean backport of bitcoin#27151, bitcoin#25849, and bitcoin#27608. No issues found — merge resolutions are correct, no Dash subsystem interactions affected, prerequisite chains are complete.

Reviewed commit: 9959cde

[github-actions]

This pull request has conflicts, please rebase.

[thepastaclaw]

✅ Review complete (commit f490c34)

[thepastaclaw]

Code Review

All four agents (Claude backport-reviewer, Claude general, Codex general) converged on no findings after verifying the three Bitcoin Core backports at HEAD 9959cde. The changes are: (1) bitcoin#27151 duplicate-include cleanup in lockedpool.cpp, (2) bitcoin#25849 cosmetic std::string wrapper removal in wallet RPC help text, and (3) bitcoin#27608 scoping RemoveBlockRequest to the originating peer in net_processing.cpp. No CodeRabbit findings to validate.

Reviewed commit: 9959cde

Fresh dispatcher run for this queue item. A same-SHA review already existed, so this records the fresh verification without duplicating inline threads.

Code Review

Reviewed commit: 9959cde

[thepastaclaw]

@vijaydasmp could you please rebase this PR, or close it if it’s no longer needed? Thanks!

[vijaydasmp]

@vijaydasmp could you please rebase this PR, or close it if it’s no longer needed? Thanks!

rebased

[thepastaclaw]

Code Review

Policy gate: an agent-reported missing upstream prerequisite was restored as a blocking finding. This PR is a full Bitcoin backport, and omitted upstream hunks require their prerequisite PRs unless the finding is explicitly allowlisted (intentional_exclusion / policy_override). The agent's original evidence is preserved in the finding(s) below.

Prior verifier summary (overridden by policy gate): Two trivial Bitcoin Core backports: bitcoin#27151 (remove duplicate include) and bitcoin#25849 (strip empty std::string() prefixes in RPC docs). The prior review at 9959cde found no issues, so there are no carried-forward prior findings. Cumulative review of the current head finds that the bitcoin#27151 cherry-pick removed Dash's only direct include while three std::move call sites remain (a prerequisite/conflict mismatch with the earlier bitcoin#26924 backport), and the bitcoin#25849 cherry-pick is incomplete — it updated sendmany/FundTxDoc/send but missed sendtoaddress and sendall, both of which still have the std::string() + prefix.

🔴 5 blocking | 🟡 1 suggestion(s) | 💬 2 nitpick(s)

8 additional finding(s) omitted (not in diff).

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/support/lockedpool.cpp`:
- [SUGGESTION] src/support/lockedpool.cpp:21-23: Add back `#include <utility>` — std::move is used but the include was fully removed
  bitcoin#27151 removed only the *duplicate* `<utility>` include upstream; the upstream pre-PR file still had one direct `<utility>` include in the main std-header block (added by bitcoin#26924). Dash's earlier #26924 backport did not add `<utility>` to the std-header block here, so it kept just the lower include. This PR's #27151 cherry-pick then removed Dash's only direct `<utility>` include, even though `std::move` is still used at lines 284, 383, and 405 (`LockedPool` ctor, `LockedPoolManager` ctor, `LockedPoolManager::Instance`). The TU currently builds through transitive includes, but `std::move` is declared by `<utility>` and the file is no longer self-contained — it can break on a different supported toolchain. Restoring one direct `<utility>` include preserves the upstream intent (remove the duplicate, keep one) and matches the cumulative upstream state.
- [BLOCKING] src/support/lockedpool.cpp:21-23: Missing prerequisite hunk: bitcoin#26924
  bitcoin#27151 removes a duplicate <utility> include from src/support/lockedpool.cpp. In upstream, this was safe because the pre-PR state already had another <utility> include in the main std-header block, added by bitcoin#26924 (merge 392dc68e37, commit fadeb6b103). Dash's earlier bitcoin#26924 backport only added <limits> in this file because Dash still had the lower <utility> include at the time. The current bitcoin#27151 Dash commit then removed that lower include, leaving no direct <utility> include even though lockedpool.cpp still uses std::move at lines 284, 383, and 405. This is a prerequisite/conflict-resolution mismatch exposed by the current head: upstream removed one of two includes, while Dash removed its only include.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.

In `src/wallet/rpc/spend.cpp`:
- [BLOCKING] src/wallet/rpc/spend.cpp:226-227: Incomplete cherry-pick of bitcoin#25849: sendtoaddress estimate_mode still has std::string() prefix
  Upstream bitcoin#25849 (fac09f4f7a) removed the unneeded empty `std::string() +` prefix at six call sites in src/wallet/rpc/spend.cpp (sendtoaddress, sendmany, FundTxDoc, bumpfee_helper, send, sendall). The Dash backport (commit f490c343a2) only updated sendmany, FundTxDoc, and send. The `sendtoaddress` RPC exists in Dash and originally had the same `std::string() + "The fee estimate mode..."` pattern as upstream, but the corresponding hunk was not applied — line 226 still reads `RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n"`. This is cosmetic only (functionally identical), but it leaves the refactor incomplete for no Dash-specific reason.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.
- [BLOCKING] src/wallet/rpc/spend.cpp:1041-1042: Incomplete cherry-pick of bitcoin#25849: sendall estimate_mode still has std::string() prefix
  Same as the sendtoaddress finding: upstream bitcoin#25849 removed the `std::string() +` prefix in the sendall RPCHelpMan. Dash has `sendall` (backported earlier from bitcoin#24118 — release notes file present) and the estimate_mode arg at line 1041 still uses the old `std::string() + "The fee estimate mode..."` form. The 25849 cherry-pick should have removed this prefix here too. Cosmetic only; no functional impact.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.
- [BLOCKING] src/wallet/rpc/spend.cpp:226-227: Incomplete backport: bitcoin#25849 missed sendtoaddress
  Upstream bitcoin#25849 (merge d9cd8b419c) removed the unnecessary std::string() prefix from the estimate_mode help text in sendtoaddress, sendmany, FundTxDoc, send, and sendall, plus a formatting hunk in bumpfee_helper. Dash's f490c343a2 cherry-pick updated sendmany, FundTxDoc, and send, but sendtoaddress is present in Dash and still has the exact std::string() + FeeModes(...) pattern. This is not a missing dependency and has no behavior impact, but it is an omitted upstream hunk in the current-head backport.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.
- [BLOCKING] src/wallet/rpc/spend.cpp:1041-1042: Incomplete backport: bitcoin#25849 missed sendall
  Upstream bitcoin#25849 also removed the unnecessary std::string() prefix from sendall's estimate_mode help text. Dash now has sendall in the current head, but f490c343a2 did not apply that hunk, so the same std::string() + FeeModes(...) pattern remains here. The omitted upstream bumpfee_helper hunk maps to an RPC section Dash does not expose and appears to be a Dash-specific BIP125/RBF omission, but sendall is present and should have received the same cosmetic backport change.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.