net: serve `MSG_BLOCK` by stripping witnesses in place #36548

Problem: Serving blocks to legacy nodes currently decodes and reserializes them just to remove witnesses. MSG_WITNESS_BLOCK already avoids this work by serving the stored bytes directly. Faster witnessless IBD, as proposed in #36476, also needs peers to serve these blocks efficiently.

Fix: Read disk-backed, non-Signet MSG_BLOCK replies into an owned byte buffer, check the header's proof of work and expected hash, and strip witness ranges from that buffer in place.

Extract a shared helper so decoding transactions and skipping their bytes follow the same encoding rules. Signet keeps ReadBlock for its solution check. Header checks now precede transaction parsing, so records corrupted in both places can log a different error.

Performance:

Time to read a block from disk and build its non-witness message:

The synthetic fixture adds a 512-byte witness item and an empty item to every input.

Related work: This extends the raw witness-block serving added in #13151. The #13011 discussion and the closed #13098 identify unnecessary transaction hashing when serving historical blocks. The implementation also builds on the bulk block reads introduced in #31551.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

External sites

• Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36548
• Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36548.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36548#author-ralph

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #36521 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36521.svg">} (Keep witness stacks in serialized form in CScriptWitness by sipa)
• #36074 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36074.svg">} (scripted-diff: [test] Add util/check.h includes for assertions by maflcko)
• #36000 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36000.svg">} (validation: prefetch blocks while connecting by l0rinc)
• #35731 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35731.svg">} (Indexes: Harden the flush-error notification invariant by arejula27)
• #32554 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/32554.svg">} (bench: replace embedded raw block with configurable block generator by l0rinc)

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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

LLM Linter (✨ experimental)

Possible places where comparison-specific test macros should replace generic comparisons:

• test/functional/p2p_getdata.py: assert len(from_binary(CBlock, full).serialize(with_witness=False)) < len(full) → Prefer assert_greater_than(len(full), len(from_binary(CBlock, full).serialize(with_witness=False))).

^{2026-10-16 13:37:48}

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/38833479928/job/116757258223} _{LLM reason (✨ experimental): CI failed because IWYU modified includes in src/bench/readwriteblock.cpp, and the formatting check detected uncommitted changes.}

<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>

tACK 0b01ce05e162ee61fb0e3c0d8413e0e1af4c1976

I checked that the new MSG_BLOCK reply is byte for byte the old one. On 214 blocks, the head's ReadBlockWithoutWitness matches its ReadBlock plus TX_NO_WITNESS, that old path matches the base's, and btclib, a library I maintain, agrees. The blocks are mainnet 481824, 709632, 774628, 920000 and the bench's 413567, plus 209 regtest blocks with segwit v0 and v1 spends, 1,000 witness items, empty stacks and items, a 3,990,000-byte annex, 300 inputs, 316 outputs, 260 transactions, or no witness. Over P2P, a head node and a base node give the same MSG_BLOCK and MSG_WITNESS_BLOCK payloads for all 209: 208 from disk, and the two successive tips from the recent-block cache.

Each of seven mutants of StripBlockWitness and ReadBlockWithoutWitness fails blockmanager_tests. They range from keeping the marker or the witnesses to skipping the proof-of-work or hash check. The new p2p_getdata case also passes against a base node, so it guards the payload format rather than the new path.

Building the reply got 7.2x to 9.0x faster than on the base for those five mainnet blocks and the two largest regtest blocks (median of three runs, Release, Apple M5). Peak heap fell from 2.9 to 2.0 MB for 481824 and from 7.9 to 4.0 MB for 774628. I measured with a Boost test that writes each block with WriteBlock and reads it back both ways, and a functional test running a head and a base node; I can share both as a gist.

<!--cf906140f33d8803c4a75a2196329ecb-->

🐙 This pull request conflicts with the target branch and needs rebase.