log,blocks: avoid `ComputeTotalSize` and `GetHash` work when logging is disabled
What the developer wrote
Authored by Lőrinc
log,blocks: avoid `ComputeTotalSize` and `GetHash` work when logging is disabled
`PartiallyDownloadedBlock::FillBlock()` computed the block header hash and summed missing transaction sizes for debug logging unconditionally, including when cmpctblock debug logging is disabled.
Guard the debug-only hash and size computations with `LogAcceptCategory`.
Since `txn_available` is invalidated after the first loop (needed for efficient moving), we compute `tx_missing_size` by iterating `vtx_missing` directly. This is safe because the later `tx_missing_offset` check guarantees `vtx_missing` was fully consumed during reconstruction.
Use `block.GetHash()` instead of `header.GetHash()`, since header is cleared before logging.
No behavior change when debug logging is enabled: the reported counts, hashes, and byte totals remain the same.
What changed, and why it matters
This is a small performance cleanup, not a security fix. Bitcoin Core was doing unnecessary work—calculating a block hash and adding up transaction byte sizes—every time it reconstructed a compact block, even when the relevant debug logging was turned off. The change simply skips that work when debug logging is disabled. The log output is unchanged when logging is on, and no network behavior or consensus rules change.
No security action needed. Treat as a normal performance/refactoring commit. Reviewers may verify that the new vtx_missing iteration is safe and that log output remains identical when BCLog::CMPCTBLOCK debug logging is enabled.
Security signals we found
No security-relevant signal present
Performance optimization only
No input validation, memory safety, or cryptographic changes
Evidence from the diff
PartiallyDownloadedBlock::FillBlock() previously computed header.GetHash() and summed missing transaction sizes (ComputeTotalSize) unconditionally to prepare a debug log line. The patch wraps those computations in LogAcceptCategory(BCLog::CMPCTBLOCK, BCLog::Level::Debug). Because txn_available is cleared/moved during reconstruction, tx_missing_size is now computed by iterating vtx_missing directly, which is safe because the earlier tx_missing_offset check confirms vtx_missing was fully consumed. The log message is unchanged when debug logging is enabled. There is no consensus, P2P protocol, or validation behavior change.
Changed components
src/blockencodings.cppCompact block reconstruction (PartiallyDownloadedBlock::FillBlock)Inspect captured patch +16 / −12
diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index eebf7bf4..fd528309 100644
--- a/src/blockencodings.cpp
+++ b/src/blockencodings.cpp
@@ -192,40 +192,44 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<
{
if (header.IsNull()) return READ_STATUS_INVALID;
- uint256 hash = header.GetHash();
block = header;
block.vtx.resize(txn_available.size());
- unsigned int tx_missing_size = 0;
size_t tx_missing_offset = 0;
for (size_t i = 0; i < txn_available.size(); i++) {
if (!txn_available[i]) {
- if (vtx_missing.size() <= tx_missing_offset)
+ if (tx_missing_offset >= vtx_missing.size()) {
return READ_STATUS_INVALID;
+ }
block.vtx[i] = vtx_missing[tx_missing_offset++];
- tx_missing_size += block.vtx[i]->ComputeTotalSize();
- } else
+ } else {
block.vtx[i] = std::move(txn_available[i]);
+ }
}
// Make sure we can't call FillBlock again.
header.SetNull();
txn_available.clear();
- if (vtx_missing.size() != tx_missing_offset)
+ if (vtx_missing.size() != tx_missing_offset) {
return READ_STATUS_INVALID;
+ }
// Check for possible mutations early now that we have a seemingly good block
IsBlockMutatedFn check_mutated{m_check_block_mutated_mock ? m_check_block_mutated_mock : IsBlockMutated};
- if (check_mutated(/*block=*/block,
- /*check_witness_root=*/segwit_active)) {
+ if (check_mutated(/*block=*/block, /*check_witness_root=*/segwit_active)) {
return READ_STATUS_FAILED; // Possible Short ID collision
}
- LogDebug(BCLog::CMPCTBLOCK, "Successfully reconstructed block %s with %u txn prefilled, %u txn from mempool (incl at least %u from extra pool) and %u txn (%u bytes) requested\n", hash.ToString(), prefilled_count, mempool_count, extra_count, vtx_missing.size(), tx_missing_size);
- if (vtx_missing.size() < 5) {
- for (const auto& tx : vtx_missing) {
- LogDebug(BCLog::CMPCTBLOCK, "Reconstructed block %s required tx %s\n", hash.ToString(), tx->GetHash().ToString());
+ if (LogAcceptCategory(BCLog::CMPCTBLOCK, BCLog::Level::Debug)) {
+ const uint256 hash{block.GetHash()};
+ uint32_t tx_missing_size{0};
+ for (const auto& tx : vtx_missing) tx_missing_size += tx->ComputeTotalSize();
+ LogDebug(BCLog::CMPCTBLOCK, "Successfully reconstructed block %s with %u txn prefilled, %u txn from mempool (incl at least %u from extra pool) and %u txn (%u bytes) requested\n", hash.ToString(), prefilled_count, mempool_count, extra_count, vtx_missing.size(), tx_missing_size);
+ if (vtx_missing.size() < 5) {
+ for (const auto& tx : vtx_missing) {
+ LogDebug(BCLog::CMPCTBLOCK, "Reconstructed block %s required tx %s\n", hash.ToString(), tx->GetHash().ToString());
+ }
}
}
Why this scored 21/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.