kernel: Remove non-kernel module includes

sedited commented at 9:30 pm on December 15, 2025: contributor

Found these while attempting to isolate the kernel library sources into their own repository. There still is no mechanism for preventing including headers into the kernel library that don’t belong to kernel modules, but it is also fairly straight forward to correct manually for now. However, the changes here might be incomplete.

DrahtBot added the label Validation on Dec 15, 2025

DrahtBot commented at 9:31 pm on December 15, 2025: 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/34079.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto, maflcko, janb84
Stale ACK	musaHaruna

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33779 (ci, iwyu: Fix warnings in src/kernel and treat them as errors by hebasto)

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.

sedited force-pushed on Dec 15, 2025

DrahtBot added the label CI failed on Dec 15, 2025

DrahtBot commented at 10:09 pm on December 15, 2025: contributor

🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/20248118058/job/58133590667 LLM reason (✨ experimental): IWYU check failed (include/dependency issues detected) causing the CI to fail.

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.

hebasto commented at 10:42 pm on December 15, 2025: member

Concept ACK.

sedited force-pushed on Dec 16, 2025

?

project_v2_item_status_changed sedited

?

added_to_project_v2 sedited

DrahtBot removed the label CI failed on Dec 16, 2025

hebasto commented at 11:36 am on December 16, 2025: member

@sedited

Could you please elaborate on the motivation for the last commit? Is it intended to avoid #include <interfaces/...> in kernel sources?

sedited force-pushed on Dec 16, 2025

sedited commented at 11:48 am on December 16, 2025: contributor

Could you please elaborate on the motivation for the last commit? Is it intended to avoid #include <interfaces/…> in kernel sources?

Yes, interfaces in turn include a bunch of stuff that should not belong in the library. I elaborated a bit in the commit message.

hebasto approved

hebasto commented at 11:51 am on December 16, 2025: member

ACK 0f757542416b75690c68145964b9094a558f4d38, I have reviewed the code and it looks OK. IWYU in the CI agreed on these changes as well.

#33779 is somewhat related to this PR.

musaHaruna commented at 2:49 pm on December 16, 2025: contributor

Code Review ACK 0f75754. Code looks good to me even though my knowledge around the kernal is lacking.

DrahtBot added the label Needs rebase on Dec 16, 2025

sedited force-pushed on Dec 16, 2025

sedited commented at 4:36 pm on December 16, 2025: contributor

Rebased 0f757542416b75690c68145964b9094a558f4d38 -> 065639200505035428df01584ff43172c4b2dd90 (kernelPruneHeaders_0 -> kernelPruneHeaders_1, compare)

Fixed conflict with #30214

DrahtBot removed the label Needs rebase on Dec 16, 2025

DrahtBot added the label CI failed on Dec 16, 2025

DrahtBot removed the label CI failed on Dec 16, 2025

hebasto approved

hebasto commented at 10:18 pm on December 16, 2025: member

ACK 065639200505035428df01584ff43172c4b2dd90, only rebased since my recent review.

DrahtBot requested review from musaHaruna on Dec 16, 2025

in src/node/blockstorage.cpp:1 in 0656392005 outdated

janb84 commented at 10:10 am on December 17, 2025:

NIT: I think #include <consensus/validation.h> can also be removed, IWYU does not complain after removal and compiles + tests fine.

janb84 commented at 7:17 pm on December 17, 2025:

NIT: Also #include <util/strencodings.h> should be fine to remove compiles, tests etc. fine.

janb84 commented at 7:24 pm on December 17, 2025: contributor

Concept ACK 065639200505035428df01584ff43172c4b2dd90

Think the changes are correct. Have added some NITS** to remove some more unused includes, this to optimally use the file churn.

**Small disclaimer on those removals, have tried my best to research if they are really unused (code review, compile, test, IWYU, Guix build) but not sure if I have hit all the edges.

sedited force-pushed on Dec 19, 2025

sedited commented at 8:25 pm on December 19, 2025: contributor

Updated 065639200505035428df01584ff43172c4b2dd90 -> 26c475c08e350ba383cea3e8da9cc1191b24fc11 (kernelPruneHeaders_1 -> kernelPruneHeaders_2, compare)

Addressed @janb84’s comment_1, comment_2, expanding the include changes with complete iwyu suggestions.

in src/kernel/chain.cpp:5 in 26c475c08e outdated

0@@ -1,11 +1,11 @@
1-// Copyright (c) 2022-present The Bitcoin Core developers
2+// Copyright (c) 2022 The Bitcoin Core developers
3 // Distributed under the MIT software license, see the accompanying
4 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
5 
6 #include <chain.h>
7-#include <interfaces/chain.h>
8 #include <kernel/chain.h>

hebasto commented at 10:35 am on December 20, 2025:

 0--- a/src/kernel/chain.cpp
 1+++ b/src/kernel/chain.cpp
 2@@ -2,8 +2,9 @@
 3 // Distributed under the MIT software license, see the accompanying
 4 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 5 
 6-#include <chain.h>
 7 #include <kernel/chain.h>
 8+
 9+#include <chain.h>
10 #include <kernel/types.h>
11 #include <kernel/cs_main.h>
12 #include <sync.h>

hebasto commented at 10:35 am on December 20, 2025: member

Addressed @janb84’s comment_1, comment_2, expanding the include changes with complete iwyu suggestions.

In that case, we could enforce IWYU checks on the touched files:

 0--- a/ci/test/03_test_script.sh
 1+++ b/ci/test/03_test_script.sh
 2@@ -206,7 +206,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
 3   fi
 4 
 5   # TODO: Consider enforcing IWYU across the entire codebase.
 6-  FILES_WITH_ENFORCED_IWYU="/src/(crypto|index)/.*\\.cpp"
 7+  FILES_WITH_ENFORCED_IWYU="/src/((crypto|index)/.*\\.cpp|kernel/chain.cpp|node/blockstorage.cpp|core_read.cpp|signet.cpp)"
 8   jq --arg patterns "$FILES_WITH_ENFORCED_IWYU" 'map(select(.file | test($patterns)))' "${BASE_BUILD_DIR}/compile_commands.json" > "${BASE_BUILD_DIR}/compile_commands_iwyu_errors.json"
 9   jq --arg patterns "$FILES_WITH_ENFORCED_IWYU" 'map(select(.file | test($patterns) | not))' "${BASE_BUILD_DIR}/compile_commands.json" > "${BASE_BUILD_DIR}/compile_commands_iwyu_warnings.json"
10

sedited commented at 11:35 am on December 20, 2025: contributor

Re #34079#pullrequestreview-3601031588

In that case, we could enforce IWYU checks on the touched files:

Some of them contain broken suggestions. I.e. applying all of core_read’s suggestions:

02025-12-19T20:41:14.3277108Z /home/admin/actions-runner/_work/_temp/src/core_read.cpp should add these lines:
12025-12-19T20:41:14.3277343Z class CBlock;
22025-12-19T20:41:14.3277458Z class CBlockHeader;
32025-12-19T20:41:14.3277529Z 
42025-12-19T20:41:14.3277667Z /home/admin/actions-runner/_work/_temp/src/core_read.cpp should remove these lines:
52025-12-19T20:41:14.3277902Z - #include <primitives/block.h>  // lines 7-7

Will lead to a compilation error.

hebasto commented at 4:36 pm on December 20, 2025: member

Re #34079 (review)

In that case, we could enforce IWYU checks on the touched files:

Some of them contain broken suggestions. I.e. applying all of core_read’s suggestions:
02025-12-19T20:41:14.3277108Z /home/admin/actions-runner/_work/_temp/src/core_read.cpp should add these lines:
12025-12-19T20:41:14.3277343Z class CBlock;
22025-12-19T20:41:14.3277458Z class CBlockHeader;
32025-12-19T20:41:14.3277529Z 
42025-12-19T20:41:14.3277667Z /home/admin/actions-runner/_work/_temp/src/core_read.cpp should remove these lines:
52025-12-19T20:41:14.3277902Z - #include <primitives/block.h>  // lines 7-7
Will lead to a compilation error.

Sounds like an IWYU bug. This branch contains IWYU pragma: keep to workaround it.

sedited force-pushed on Dec 20, 2025

DrahtBot added the label CI failed on Dec 20, 2025

sedited force-pushed on Dec 20, 2025

sedited commented at 9:08 pm on December 20, 2025: contributor

Updated 26c475c08e350ba383cea3e8da9cc1191b24fc11 -> fb4e9cc3f20ebf80249ac879fa9d71e526e7c4cc (kernelPruneHeaders_2 -> kernelPruneHeaders_3, compare)

Addressed @hebasto’s comment, added touched files to the iwyu enforcement list and clean up both headers and implementation for each module.
Addressed @hebasto’s comment, fixed formatting in the include list.

DrahtBot removed the label CI failed on Dec 20, 2025

hebasto approved

hebasto commented at 0:07 am on December 21, 2025: member

ACK fb4e9cc3f20ebf80249ac879fa9d71e526e7c4cc.

DrahtBot requested review from janb84 on Dec 21, 2025

in src/kernel/chain.h:33 in fb4e9cc3f2

31+    // A timestamp that can only move forward.
32+    unsigned int chain_time_max{0};
33+
34+    BlockInfo(const uint256& hash LIFETIMEBOUND) : hash(hash) {}
35+};
36+}

hebasto commented at 0:15 am on December 21, 2025:

nit:

0} // namespace interfaces

hebasto commented at 0:17 am on December 21, 2025: member

f9bc2337a4b3d39a6136d51f4fc1e95c47226baf:

nit: Headers could be sorted properly using clang-format-diff.py.

kernel: Remove some unnecessary non-kernel includes

Specifically gets rid of batchpriority, chainparams, script/sign.h and
system includes.

Also take the opportunity of cleaning up the headers for the effected
files and adding them to the iwyu-enforced set.

d69a582e72

kernel: Move BlockInfo to a kernel file

This should avoid having to include interfaces/chain.h from a kernel
module. interfaces/chain.h in turn includes a bunch of non-kernel
headers, that break the desired library topology and might introduce
entanglement regressions.

d3a479cb07

sedited force-pushed on Dec 21, 2025

sedited commented at 9:26 am on December 21, 2025: contributor

Updated fb4e9cc3f20ebf80249ac879fa9d71e526e7c4cc -> d3a479cb077d9a9a4451dc4ecb43fe0daf94f172 (kernelPruneHeaders_3 -> kernelPruneHeaders_4, compare)

Ran clang-format-diff as requested by @hebasto.

hebasto approved

hebasto commented at 12:25 pm on December 21, 2025: member

re-ACK d3a479cb077d9a9a4451dc4ecb43fe0daf94f172.

in src/signet.cpp:17 in d69a582e72

17-#include <span.h>
18+#include <script/script.h>
19 #include <streams.h>
20 #include <uint256.h>
21-#include <util/strencodings.h>
22+#include <util/check.h>

maflcko commented at 9:22 am on December 22, 2025:

this looks a bit confusing, because it is not used directly. I guess it comes via the newly added Assume in the logging macros.

maflcko approved

maflcko commented at 9:26 am on December 22, 2025: member

review ACK d3a479cb077d9a9a4451dc4ecb43fe0daf94f172 🦏

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK d3a479cb077d9a9a4451dc4ecb43fe0daf94f172 🦏
3GD5j+um4ZXDdq2C/poLbIxvd9kJUa+Ii7eEllMa5cRpHK3v0/5/ujn98DM78mp42oZSZO9KOhINm95Jde/aeAw==

janb84 commented at 1:05 pm on December 22, 2025: contributor

ACK d3a479cb077d9a9a4451dc4ecb43fe0daf94f172

PR cleans includes and adds IWYU enforcement for kernel project. See graphs to visually see the improvement with this PR.

 0flowchart
 1    subgraph C_0007162612065402976890[..]
 2    subgraph C_0009881953776078837911[src]
 3    subgraph C_0005721719369906011104[consensus]
 4    end
 5    subgraph C_0013409324594247833141[crypto]
 6    end
 7    subgraph C_0008715219809326032762[util]
 8    end
 9    subgraph C_0012188910115774263636[kernel]
10    end
11    subgraph C_0004679740005580579026[script]
12    end
13    subgraph C_0015301371342078761975[primitives]
14    end
15    subgraph C_0016251290524000539074[node]
16    end
17    subgraph C_0001329797251136332775[policy]
18    end
19    subgraph C_0007881301851887086192[common]
20    end
21    subgraph C_0004661196650956799315[interfaces]
22    end
23    end
24    end
25    C_0005721719369906011104 -.-> C_0004679740005580579026
26    C_0005721719369906011104 -.-> C_0015301371342078761975
27    C_0008715219809326032762 -.-> C_0013409324594247833141
28    C_0008715219809326032762 -.-> C_0015301371342078761975
29    C_0012188910115774263636 -.-> C_0004679740005580579026
30    C_0012188910115774263636 -.-> C_0013409324594247833141
31    C_0012188910115774263636 -.-> C_0015301371342078761975
32    C_0012188910115774263636 -.-> C_0016251290524000539074
33    C_0012188910115774263636 -.-> C_0008715219809326032762
34    C_0012188910115774263636 -.-> C_0005721719369906011104
35    C_0012188910115774263636 -.-> C_0001329797251136332775
36    C_0012188910115774263636 -.-> C_0004661196650956799315
37    C_0004679740005580579026 -.-> C_0008715219809326032762
38    C_0004679740005580579026 -.-> C_0015301371342078761975
39    C_0004679740005580579026 -.-> C_0013409324594247833141
40    C_0015301371342078761975 -.-> C_0004679740005580579026
41    C_0015301371342078761975 -.-> C_0008715219809326032762
42    C_0016251290524000539074 -.-> C_0005721719369906011104
43    C_0016251290524000539074 -.-> C_0008715219809326032762
44    C_0016251290524000539074 -.-> C_0012188910115774263636
45    C_0016251290524000539074 -.-> C_0015301371342078761975
46    C_0016251290524000539074 -.-> C_0004679740005580579026
47    C_0001329797251136332775 -.-> C_0008715219809326032762
48    C_0001329797251136332775 -.-> C_0015301371342078761975
49    C_0001329797251136332775 -.-> C_0004679740005580579026
50    C_0001329797251136332775 -.-> C_0005721719369906011104
51    C_0007881301851887086192 -.-> C_0008715219809326032762
52    C_0004661196650956799315 -.-> C_0015301371342078761975
53    C_0004661196650956799315 -.-> C_0008715219809326032762
54    C_0004661196650956799315 -.-> C_0016251290524000539074
55    C_0004661196650956799315 -.-> C_0001329797251136332775
56    C_0004661196650956799315 -.-> C_0007881301851887086192
57    C_0004661196650956799315 -.-> C_0012188910115774263636

 0flowchart
 1    subgraph C_0007162612065402976890[..]
 2    subgraph C_0009881953776078837911[src]
 3    subgraph C_0005721719369906011104[consensus]
 4    end
 5    subgraph C_0013409324594247833141[crypto]
 6    end
 7    subgraph C_0008715219809326032762[util]
 8    end
 9    subgraph C_0012188910115774263636[kernel]
10    end
11    subgraph C_0004679740005580579026[script]
12    end
13    subgraph C_0015301371342078761975[primitives]
14    end
15    subgraph C_0016251290524000539074[node]
16    end
17    subgraph C_0001329797251136332775[policy]
18    end
19    end
20    end
21    C_0005721719369906011104 -.-> C_0004679740005580579026
22    C_0005721719369906011104 -.-> C_0015301371342078761975
23    C_0008715219809326032762 -.-> C_0013409324594247833141
24    C_0008715219809326032762 -.-> C_0015301371342078761975
25    C_0012188910115774263636 -.-> C_0004679740005580579026
26    C_0012188910115774263636 -.-> C_0013409324594247833141
27    C_0012188910115774263636 -.-> C_0015301371342078761975
28    C_0012188910115774263636 -.-> C_0016251290524000539074
29    C_0012188910115774263636 -.-> C_0008715219809326032762
30    C_0012188910115774263636 -.-> C_0005721719369906011104
31    C_0012188910115774263636 -.-> C_0001329797251136332775
32    C_0004679740005580579026 -.-> C_0008715219809326032762
33    C_0004679740005580579026 -.-> C_0015301371342078761975
34    C_0004679740005580579026 -.-> C_0013409324594247833141
35    C_0015301371342078761975 -.-> C_0004679740005580579026
36    C_0015301371342078761975 -.-> C_0008715219809326032762
37    C_0016251290524000539074 -.-> C_0005721719369906011104
38    C_0016251290524000539074 -.-> C_0008715219809326032762
39    C_0016251290524000539074 -.-> C_0012188910115774263636
40    C_0016251290524000539074 -.-> C_0015301371342078761975
41    C_0001329797251136332775 -.-> C_0008715219809326032762
42    C_0001329797251136332775 -.-> C_0015301371342078761975
43    C_0001329797251136332775 -.-> C_0004679740005580579026
44    C_0001329797251136332775 -.-> C_0005721719369906011104

hebasto merged this on Dec 22, 2025

hebasto closed this on Dec 22, 2025

?

project_v2_item_status_changed github-project-automation[bot]

in src/core_read.cpp:7 in d3a479cb07

3@@ -4,17 +4,27 @@
4 
5 #include <core_io.h>
6 
7-#include <primitives/block.h>
8+#include <primitives/block.h> // IWYU pragma: keep

maflcko commented at 2:11 pm on December 22, 2025:

it would be good to reduce and report this bug upstream. Otherwise, wide-spread use of iwyu in this code-base seems risky.

fanquake commented at 2:21 pm on December 22, 2025:

Would have been good if it was documented, rather than adding undocumented workarounds for buggy tools.

hebasto commented at 11:21 pm on January 2, 2026:

it would be good to reduce and report this bug upstream. Otherwise, wide-spread use of iwyu in this code-base seems risky.

It appears that the IWYU warning is in fact correct, and that applying the suggested fix merely exposes a circular dependency in our heavily templated serialization code.

Specifically, https://github.com/bitcoin/bitcoin/blob/c631553732f9c8e4f99d02edb04b5fa43a909bf3/src/core_read.cpp#L223 instantiates https://github.com/bitcoin/bitcoin/blob/c631553732f9c8e4f99d02edb04b5fa43a909bf3/src/streams.h#L247-L252 for T = CBlockHeader.

This, in turn, instantiates https://github.com/bitcoin/bitcoin/blob/c631553732f9c8e4f99d02edb04b5fa43a909bf3/src/serialize.h#L750-L755 for Stream = DataStream, T = CBlockHeader, which requires CBlockHeader to be a complete type. Consequently, serialize.h must include <primitives/block.h>.

However, primitives/block.h itself already (and correctly) includes <serialize.h>, resulting in a circular dependency.

sedited referenced this in commit f8db928648 on Dec 27, 2025

joshdoman referenced this in commit 57661ceac9 on Dec 27, 2025

kernel: Remove non-kernel module includes #34079

Code Coverage & Benchmarks

Reviews

Conflicts