iwyu: Fix warnings in `src/rpc` and treat them as errors #36116

hebasto commented at 3:08 PM on August 28, 2026: member

This PR continues the ongoing effort to enforce IWYU warnings.

See Developer Notes.

hebasto added the label Refactoring on Aug 28, 2026

DrahtBot commented at 3:08 PM on August 28, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36116.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Stale ACK	stickies-v, jeanpablojp, Marisha-Sahay

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36005 (rpc, descriptor: warn when multipath expansion clones single-path key expressions by pythcoiner)
#36002 (txindex: allow running in pruned mode by andrewtoth)
#35733 (sign: Remove FillableSigningProvider by achow101)
#35675 (mining: add block template manager by ismaelsadeeq)
#35281 (univalue: respect token end pointer by ferminquant)
#33741 (rpc: Optionally print feerates in sat/vb by polespinasa)
#33117 (Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications by D33r-Gee)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

hebasto force-pushed on Sep 1, 2026

hebasto marked this as ready for review on Sep 1, 2026

hebasto commented at 10:44 AM on September 1, 2026: member

Rebased on top of the merged bitcoin/bitcoin#36112 and undrafted.

hebasto force-pushed on Sep 1, 2026

DrahtBot added the label CI failed on Sep 1, 2026

hebasto force-pushed on Sep 1, 2026

DrahtBot removed the label CI failed on Sep 1, 2026

in ci/test/03_test_script.sh:237 in baaca93bff

 233 | @@ -234,7 +234,7 @@ fi
 234 |  
 235 |  if [[ "${RUN_IWYU}" == true ]]; then
 236 |    # TODO: Consider enforcing IWYU across the entire codebase.
 237 | -  FILES_WITH_ENFORCED_IWYU='/src/((bench|common|consensus|crypto|index|init|kernel|primitives|script|univalue/(lib|test)|util|zmq)/.*|node/(blockstorage|interfaces|miner|mining_args|utxo_snapshot)|test/fuzz/minisketch|rpc/mining|clientversion|core_io|signet|init)\.cpp'
 238 | +  FILES_WITH_ENFORCED_IWYU='/src/((bench|common|consensus|crypto|index|init|kernel|primitives|rpc|script|univalue/(lib|test)|util|zmq)/.*|node/(blockstorage|interfaces|miner|mining_args|utxo_snapshot)|test/fuzz/(kitchen_sink|minisketch|parse_univalue)|clientversion|core_io|signet|init)\.cpp'

stickies-v commented at 11:13 AM on September 2, 2026:

Should rest.cpp also be covered here, since you've now fixed it up?

hebasto commented at 12:09 PM on September 2, 2026:

Good catch! That was lost during rebasing. I've restored it now.

hebasto force-pushed on Sep 2, 2026

in ci/test/03_test_script.sh:237 in 3817b33dbb

 233 | @@ -234,7 +234,7 @@ fi
 234 |  
 235 |  if [[ "${RUN_IWYU}" == true ]]; then
 236 |    # TODO: Consider enforcing IWYU across the entire codebase.
 237 | -  FILES_WITH_ENFORCED_IWYU='/src/((bench|common|consensus|crypto|index|init|kernel|primitives|script|univalue/(lib|test)|util|zmq)/.*|node/(blockstorage|interfaces|miner|mining_args|utxo_snapshot)|test/fuzz/minisketch|rpc/mining|clientversion|core_io|signet|init)\.cpp'
 238 | +  FILES_WITH_ENFORCED_IWYU='/src/((bench|common|consensus|crypto|index|init|kernel|primitives|rpc|script|univalue/(lib|test)|util|zmq)/.*|node/(blockstorage|interfaces|miner|mining_args|utxo_snapshot)|test/fuzz/(kitchen_sink|minisketch|parse_univalue)|rpc/mining|clientversion|core_io|rest|signet|init)\.cpp'

stickies-v commented at 1:03 PM on September 2, 2026:

I think rpc/mining is covered under rpc now and can be removed? (this was added in latest rebase)

hebasto commented at 1:08 PM on September 2, 2026:

Thanks! Fixed.

hebasto force-pushed on Sep 2, 2026

DrahtBot added the label CI failed on Sep 2, 2026

stickies-v commented at 1:29 PM on September 2, 2026: contributor

ACK 1bc42956730d95ba7ab7441586ec5186e362e12a

DrahtBot removed the label CI failed on Sep 2, 2026

jeanpablojp commented at 12:50 PM on September 3, 2026: contributor

tACK 1bc42956730d95ba7ab7441586ec5186e362e12a

Built and ran the unit suite and a slice of the RPC functional tests. I also ran the old and the new FILES_WITH_ENFORCED_IWYU against compile_commands.json. Nineteen files come into the enforced set, none drops out, and rpc/mining.cpp stays covered.

iwyu, refactor: Fix includes in some source files

This change is required for the subsequent commit to compile.

d8135c93b2

iwyu: Fix warnings in `src/rpc` and treat them as errors fe646b6749

in src/rpc/signmessage.cpp:7 in 1bc4295673 outdated

   3 | @@ -4,6 +4,7 @@
   4 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   5 |  
   6 |  #include <common/signmessage.h>
   7 | +

jeanpablojp commented at 12:50 PM on September 3, 2026:

nit: ten of the eleven files in src/rpc that define a Register*RPCCommands function include <rpc/register.h> as an associated header. That includes blockchain.cpp and mempool.cpp, where IWYU already infers one. Was leaving signmessage.cpp out deliberate?

hebasto commented at 1:31 PM on September 3, 2026:

Thanks! Fixed.

hebasto force-pushed on Sep 3, 2026

Marisha-Sahay commented at 2:32 PM on September 3, 2026: none

tACK 1bc4295 on macOS. Replicated @jeanpablojp : Built the node from scratch with CMake, ran the unit tests (ctest), and ran the RPC functional tests (test/functional/test_runner.py rpc*). Everything compiled cleanly and tests passed.