#35724 cmpctblock: Improve logging of `cmpctblock` message reconstruction statistics [part of prefill series]
https://github.com/bitcoin/bitcoin/pull/35724 · · +109/-14 in 4 files, 3 commits · labels: none
Goal
- Give node operators and developers detailed logging on compact block reconstruction and prefill redundancy
- Helps evaluate block relay efficiency and measure improvements to compact block prefilling
This PR enhances logging for compact block (BIP 152) reconstruction. It logs counts and byte sizes of transactions supplied via prefill, mempool, extrapool, and GETBLOCKTXN requests, tracks redundant prefilled transactions, and logs missing transaction IDs at trace level.
Problem: Observing and diagnosing compact block reconstruction efficiency is difficult because existing logs do not separate sources or track redundant prefills, hindering analysis of proposed prefill improvements such as #35558.
Category: P2P (#41 of 65)
P3 · unblocks #35558
- P3 because diagnostic tooling provides developer visibility without altering wire protocol behavior
- Unblocks evaluating compact block relay performance and prefill experiments in #35558
P3 because this provides useful diagnostic logging to monitor compact block relay performance and unblocks observability for the prefill series in #35558, without altering P2P protocol rules or network behavior.
Membership: Modifies compact block reconstruction logging and missing transaction accounting in src/blockencodings.cpp and src/net_processing.cpp
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 1
Reviewability: Ready
- Wait for author update addressing mempool lock feedback before reviewing
The code compiles, passes tests, and has been tested on signet. An upcoming author update to address lock efficiency in the prefill check does not invalidate review of the logging interface.
Author status: Active; replied on 2026-09-16 acknowledging andrewtoth's feedback and indicating an update will follow soon
Open concerns:
- andrewtoth noted that checking prefill existence via pool->exists acquires pool->cs repeatedly in a loop, suggesting a map-based approach under a single lock instead
Resolved concerns:
- w0xlt identified an incorrect extrapool counter decrement on collisions, resolved after #35727
- w0xlt noted mismatched log levels between ShouldTraceLog and LogDebug, resolved by author in a follow-up push
Agreement: Mild
- General support for compact block relay diagnostics with no concept objections
- Approved after earlier logging level fixes were addressed (w0xlt)
- Verified by testing on signet and suggested minor log format tweaks (0xB10C)
- Concerned repeated mempool lock acquisition in loop should be batched (andrewtoth)
Mild: andrewtoth raised a nonblocking concern regarding repeated lock acquisitions in debug logging; author plans to address it
Reviewers support the diagnostic improvement and have tested it on signet, but andrewtoth's efficiency feedback regarding mempool locking remains open while the author prepares an update.
- 0xB10C (2026-09-16): 'Doing another review pass over the code and also running it on signet a bit. Saw this log, which looks good to me... Before leaving an ACK, it would be good to have andrew's comment addressed/answered.'
- w0xlt (2026-09-08): 'ACK a55e3bb55ad473556e3e5011bb9782a941f26f84'
- davidgumberg (2026-09-16): 'I just have to think about how to integrate this with the other PR and haven't had a chance yet, will update soon.'
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| w0xlt | correctness | decrementing extrapool counter when candidate may have come from mempool | resolved | no | yes | 2026-07-15: 'removing this decrement assumes that the existing candidate came from the extrapool' Settled: 2026-08-04: author reworked after #35727 merged; w0xlt ACKed on 2026-09-08 |
| w0xlt | correctness | contradictory logging level (calling LogDebug inside ShouldTraceLog block) | resolved | no | yes | 2026-08-19: 'The code has contradictory logging levels' Settled: 2026-08-28: author updated to use LogTrace in one line |
| andrewtoth | approach | takes pool lock repeatedly via pool->exists in a loop instead of checking under a single lock | open | no | yes | 2026-09-11: 'The main thing is that we\'re taking the lock inside each loop via pool->exists with the current approach.' |
| andrewtoth | maintenance | unnecessary code complexity by separating count increments from reconstruction logic | open | no | yes | 2026-08-23: "I'm not sure I agree with the approach of 78715aab9475b25bb10c82d2e8c1c827560b76ed. Incrementing the counts inline is very cheap. I can't see it be worth the code complexity to move these out." Settled: 2026-08-25: "I moved the count inc/dec out because I thought it made the code easier to read, not for performance... it's nice to scope all the logging code" |
Support:
- 0xB10C: Ran on signet and verified log output is helpful for monitoring compact block reconstruction
- w0xlt: Tested and confirmed counter fixes [not substantive]
Participants: w0xlt (objection), l0rinc (neutral), 0xB10C (support), andrewtoth (objection)
State derived from the lists: nonblocking objection open (andrewtoth, andrewtoth)
Review verdicts (DrahtBot): 1
- ACK: w0xlt
Dependencies
Enables:
- #35558 p2p: Prefill compact blocks
Files
2 lines under test/bench/ci.
- src/blockencodings.cpp +80/-13
- src/net_processing.cpp +17/-0
- src/blockencodings.h +11/-0
- src/test/blockencodings_tests.cpp +1/-1
Card
PR #35724 improves debug and trace logging for BIP 152 compact block reconstruction by recording transaction counts, byte sizes, and redundancy across the mempool, extrapool, and prefill sections. It assists developers and node operators in analyzing compact block relay efficiency and unblocks performance evaluation for the prefill series in #35558. The PR is tested on signet with support from 0xB10C and an ACK from w0xlt, with a nonblocking comment from andrewtoth on locking efficiency pending an author update.