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/34296.

Reviews

See the guideline for information on the review process.

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:

• #34319 (Drop some IWYU pragma: export and document IWYU usage by hebasto)
• #34308 (doc: Document IWYU workaround by hebasto)
• #33779 (ci, iwyu: Fix warnings in src/kernel and treat them as errors by hebasto)
• #32729 (test,refactor: extract script template helpers & widen sigop count coverage by l0rinc)
• #31650 (refactor: Avoid copies by using const references or by move-construction by maflcko)
• #28690 (build: Introduce internal kernel library by sedited)

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.

Concept ACK.

Concept ACK

This became possible after introducing a bespoke small hex parser in bitcoin-chainstate.cpp in #30595. We should eventually re-use our own internal utilities for things like hex parsing in our small tool binaries. The strencodings module can be used for that, so removing core_read from the kernel doesn't make this more complicated.

I agree, and we may even go further: Make core_io use the kernel, instead of re-implementing the logic? I think in combination with #30595 (comment) this would even lead to a speedup for free? Happy to work on this in a follow-up, if there is no obvious issue I am missing.

The strencodings module can be used for that, so removing core_read from the kernel doesn't make this more complicated.

Although looks like the strencodings and string modules could be removed from the kernel lib now too?