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
ACK	stickies-v
Concept ACK	fanquake
Stale ACK	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.

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.

fanquake commented at 9:10 AM on September 9, 2026: member

@Marisha-Sahay Looks like you've ACK'd a prior commit hash.

fanquake closed this on Sep 9, 2026

fanquake reopened this on Sep 9, 2026

fanquake commented at 11:14 AM on September 9, 2026: member

This needs a rebase.

iwyu, refactor: Fix includes in some source files

This change is required for the subsequent commit to compile.

462ff18d79

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

hebasto force-pushed on Sep 9, 2026

DrahtBot added the label CI failed on Sep 9, 2026

DrahtBot commented at 11:50 AM on September 9, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/34344007409/job/102441186642</sub> <sub>LLM reason (✨ experimental): CI failed because the IWYU (include-what-you-use) lint job generated a failure (Failure generated from IWYU).</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

hebasto commented at 11:50 AM on September 9, 2026: member

This needs a rebase.

Rebased.

stickies-v commented at 12:30 PM on September 9, 2026: contributor

re-ACK 7d54a88b44530e5a37f8881b1ea73c2edd8ef541

DrahtBot requested review from fanquake on Sep 9, 2026

DrahtBot requested review from jeanpablojp on Sep 9, 2026

DrahtBot removed the label CI failed on Sep 9, 2026

fanquake merged this on Sep 9, 2026

fanquake closed this on Sep 9, 2026

hebasto deleted the branch on Sep 9, 2026