Simplify header analysis/reasoning by splitting core_io.h into the expected core_read.h/core_write.h (matching core_read.cpp/core_write.cpp) #16166

Simplify header analysis/reasoning by splitting core_io.h into the expected core_read.h/core_write.h (matching core_read.cpp/core_write.cpp).

As noted by MarcoFalke in the review of #16129:

For some reason core_io is the header file for both core_read and core_write...

Also noted by sipa when writing extra code to handle this specific case in contrib/devtools/circular-dependencies.py:

https://github.com/bitcoin/bitcoin/blob/5d2ccf0ce9ca1571c650a69319fb9c1e0b626ecb/contrib/devtools/circular-dependencies.py#L6-L9

This change makes automated header analysis easier. Also humans will benefit: especially future generations of contributors who joins us with the expectation of normal header naming :-)

Approach ACK

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #15606 ([experimental] UTXO snapshots by jamesob)
• #15294 ([moveonly] wallet: Extract RipeMd160 by Empact)

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.

We don't have a fixed rule that every cpp should be accompanied by a .h with the same name. Maybe this is useful for your tool, but I don't personally think it warrants changing 20 files. (not a hard NACK but pleaasee let's focus on user issues instead of these kind of things that keep breaking other PRs and make rebasing hard)

One alternative would be to merge core_read.cpp and core_write.cpp into core_io.cpp.

Such a change would touch fewer files (five files changed) but has the downside of touching the implementation.

Concept ACK in addition to making the files more focused, it reduces the symbols unnecessarily included in several cases. In the ideal case we'd have a minimal set of definitions per concept/file, included only where needed, this is a modest step in that direction.

I don't personally think it warrants changing 20 files.

Most of the files are include-only changes, which have modest implication, and the reduction in includes across those files is itself a benefit.

Thank you for your contribution. While this stylistic change makes sense on its own, it comes at a cost and risk for the project as a whole. The weak motivation for the change does not justify the burden that it places on the project. A burden could be any of the following:

• Time spent on review
• Accidental introduction of bugs
• (Silent) merge conflicts, either in the branch or a backport branch. Those conflicts demand further developer and reviewer time or introduce bugs.

For more information about refactoring changes and stylistic cleanup, see

• https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring
• https://github.com/bitcoin/bitcoin/blob/master/.github/PULL_REQUEST_TEMPLATE.md
• #15465

Generally, if the style is not mentioned nor enforced by the developer notes, we leave it up to the original author to pick whatever fits them best personally and then leave it that way until the line is touched for other reasons.

Let me know if you have any questions.

I'd rather just merge the two cpp files, but that can be done when someone else touches the file