#36548 net: serve `MSG_BLOCK` by stripping witnesses in place
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36548 net: serve `MSG_BLOCK` by stripping witnesses in place | Ready | 1 | Strong | 152 + 330 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Optimizes serving disk-backed `MSG_BLOCK` P2P requests by stripping witness data directly from serialized block buffers in place instead of decoding into `CBlock` and reserializing. It refactors transaction deserialization to share `UnserializeTransactionBody` between standard decoding and witness skipping, verifying proof-of-work and block hash before stripping. Signet requests continue to use `ReadBlock` to preserve its block solution check.
Problem: Serving witness-stripped blocks to legacy nodes or future witnessless sync clients decodes full blocks and transactions into heap allocations and recomputes transaction hashes, only to discard witness fields during reserialization. This introduces significant CPU and memory overhead compared to `MSG_WITNESS_BLOCK`, which is streamed directly from disk.
Evidence of importance:
- PR description benchmarks show reading and serializing a non-witness message drops from 1.31 ms to 0.147 ms (8.9x speedup) for block 413567 and from 3.47 ms to 0.374 ms (9.3x speedup) for the same block with synthetic witness data
- Enables fast non-witness block serving needed by proposed witnessless assumevalid IBD in issue #36476
Scope: Touches transaction deserialization primitives in `src/primitives/transaction.h` by extracting common parsing logic into `UnserializeTransactionBody`, and introduces in-place buffer manipulation using `std::memmove` in `StripBlockWitness`.
Risk of a mistake: medium (p2p, validation). A parsing or memory offset bug in in-place witness stripping could corrupt block messages sent to peers, causing peer disconnections or potential memory safety violations.
Changed since the previous assessment: No structural changes; the implementation continues to use in-place buffer stripping and shared transaction parsing rules with minor rebasing or test adjustments.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- nothing: nothing pending; waiting for further review
Agreement
Strong
substantive support, no open objection (fametrano, fametrano)
Thread notes: Added fametrano's updated tACK review on rebase.
Objections, suggestions, and questions: none recorded.
Support:
- fametrano (none) (Tested ACK, 2026-10-25) [both: blockmanager_tests]: verified byte-for-byte equivalence of MSG_BLOCK output on 214 blocks, tested mutants against blockmanager_tests, and confirmed 7.2x-9.0x speedups and heap reduction
- fametrano (none) (Tested ACK, 2026-11-01) [both: ReadBlockWithoutWitness, blockmanager_tests, p2p_getdata, p2p_segwit, p2p_compactblocks]: re-ran byte-for-byte check on rebased head against 263 regtest blocks with extreme witness stacks and annexes, verified P2P MSG_BLOCK and MSG_WITNESS_BLOCK serialization, and confirmed test suite passes
Participants:
- fametrano (none, support, 2 statements 2026-10-25 to 2026-11-01): tested ACK after re-testing byte-for-byte equivalence and functional tests on rebase
Dependencies
Enables:
Files
333 lines under test/bench/ci.
- src/test/blockmanager_tests.cpp +185/-2
- src/node/blockstorage.cpp +87/-7
- src/primitives/transaction.h +45/-29
- src/bench/readwriteblock.cpp +52/-0
- test/functional/p2p_getdata.py +47/-1
- src/test/fuzz/block.cpp +46/-0
- src/net_processing.cpp +11/-9
- src/node/blockstorage.h +9/-0
Card
The short text the ranking pass compares across PRs.
Serves disk-backed MSG_BLOCK peer requests by reading raw block bytes and stripping witness data in place rather than fully deserializing and reserializing transactions. This eliminates unnecessary object allocations and transaction rehashing when serving legacy peers or clients syncing without witnesses. Benchmarks report an 8.9x to 9.3x speedup when preparing witness-free block payloads from disk. The optimization also unblocks efficient block serving for proposed witnessless assumevalid IBD (#36476).
Data
thread read through 2026-11-01 (2 statements), head d36943b49c
labels: P2P
Categories
P2P
P3 · speedup
Optimizes serving non-witness `MSG_BLOCK` requests by stripping witnesses in-place from serialized buffers, achieving an 8.9x–9.3x speedup and reducing heap allocations in benchmarks. While legacy `MSG_BLOCK` traffic is currently minor, this mitigates resource overhead when serving blocks and unblocks proposed witnessless IBD (#36476).
Membership: Carries the P2P label and modifies block serving logic in `src/net_processing.cpp` for `MSG_BLOCK` requests.
Factors: security/stability 1, bug 0, performance 2, user value 1, leverage 1