backport: Merge bitcoin/bitcoin#27357, 25977, (partial) 27594, 28583, 27608

[vijaydasmp]

What was done?

Backports from Bitcoin Core v26,

How Has This Been Tested?

Run unit & functional tests

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

This pull request enables the modernize-use-emplace clang-tidy check and applies it throughout the codebase, replacing push_back calls with emplace_back across 40+ files for in-place container construction. It refactors the LoadAddrman function to return util::Result<std::unique_ptr<AddrMan>> instead of using output parameters and optional error types. The Result class template parameter is renamed and enhanced with a default constructor. The RemoveBlockRequest function signature is updated to accept an optional peer identifier for conditional block removal. The global warningcache array is relocated from global scope to per-chainstate storage via ChainstateManager::m_warningcache. Configuration files are updated to exclude nanobench sources and enable the modernize check.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~40 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.59% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description 'bitcoin backports' is vague and generic, lacking specific details about the actual changes made in the pull request.	Provide a more descriptive PR title and description that specifically explains what is being backported and why, rather than using a generic term like 'bitcoin backports'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the PR as a backport of multiple Bitcoin Core commits with specific PR numbers listed.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/util/result.h (1)

46-46: Constrain the default constructor to Result<void> using a requires clause.

Line 46 is intended only for void, but it is currently available for all specializations. This default constructor initializes the variant with std::monostate, which is semantically incorrect for Result<non-void> types. Constraining it improves API clarity and prevents accidental misuse.

♻️ Proposed fix

-    Result() : m_variant{std::in_place_index_t<1>{}, std::monostate{}} {}  // constructor for void
+    Result() requires std::is_same_v<M, void>
+        : m_variant{std::in_place_index_t<1>{}, std::monostate{}} {} // constructor for void

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/util/result.h` at line 46, The default constructor currently available
for all Result<T> specializations should be constrained to only Result<void>;
update the constructor declaration for Result() to add a requires clause (e.g.,
requires std::is_void_v<T>) so it only participates when T is void, leaving
other specializations without this monostate constructor; modify the Result()
definition that initializes m_variant{std::in_place_index_t<1>{},
std::monostate{}} to use that requires constraint (or equivalent SFINAE) so
non-void Result<T> cannot be default-constructed into monostate.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/util/result.h`:
- Line 46: The default constructor currently available for all Result<T>
specializations should be constrained to only Result<void>; update the
constructor declaration for Result() to add a requires clause (e.g., requires
std::is_void_v<T>) so it only participates when T is void, leaving other
specializations without this monostate constructor; modify the Result()
definition that initializes m_variant{std::in_place_index_t<1>{},
std::monostate{}} to use that requires constraint (or equivalent SFINAE) so
non-void Result<T> cannot be default-constructed into monostate.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6bf73ca4-aba1-482f-9bb9-90fadee4257f

📥 Commits

Reviewing files that changed from the base of the PR and between af926ae and 0c2eedd.

📒 Files selected for processing (38)

• src/.bear-tidy-config
• src/.clang-tidy
• src/addrdb.cpp
• src/addrdb.h
• src/bench/block_assemble.cpp
• src/bench/wallet_loading.cpp
• src/external_signer.cpp
• src/httpserver.cpp
• src/init.cpp
• src/net_permissions.cpp
• src/net_processing.cpp
• src/node/blockstorage.cpp
• src/qt/rpcconsole.cpp
• src/rest.cpp
• src/rpc/blockchain.cpp
• src/rpc/net.cpp
• src/rpc/rawtransaction.cpp
• src/rpc/server.cpp
• src/script/sign.cpp
• src/test/addrman_tests.cpp
• src/test/bip32_tests.cpp
• src/test/net_tests.cpp
• src/test/rpc_tests.cpp
• src/test/settings_tests.cpp
• src/test/sighash_tests.cpp
• src/test/txpackage_tests.cpp
• src/test/util/setup_common.cpp
• src/test/util/txmempool.cpp
• src/test/util_tests.cpp
• src/test/util_threadnames_tests.cpp
• src/test/validation_block_tests.cpp
• src/util/result.h
• src/validation.cpp
• src/validation.h
• src/wallet/rpc/backup.cpp
• src/wallet/salvage.cpp
• src/wallet/spend.cpp
• src/wallet/test/wallet_tests.cpp