net: check for empty header before calling FillBlock
What the developer wrote
Authored by Eugene Siegel
net: check for empty header before calling FillBlock
Previously in debug builds, this would cause an Assume crash if
FillBlock had been called previously. This could happen when multiple
blocktxn messages were received.
Co-Authored-By: Greg Sanders <gsanders87@gmail.com>
What changed, and why it matters
This change fixes a bug in how Bitcoin Core handles repeated compact-block transaction messages from peers. In debug builds, receiving multiple blocktxn messages for the same block could trigger an internal consistency check (Assume) crash because the code tried to look up information from a header that had already been cleared. The patch detects the empty-header situation, marks the peer as misbehaving, and returns early instead of crashing. It is a robustness fix that prevents a debug-only crash and tightens handling of misbehaving peers.
Apply the patch. It is a low-risk defensive fix. Node operators running debug builds are the most directly affected; release builds do not crash on Assume failure but still benefit from cleaner peer handling. No immediate emergency response is indicated.
Security signals we found
Debug-only assertion crash (Assume) reachable via P2P message handling
Repeated blocktxn messages for same block can trigger null-header dereference/lookup
Peer misbehavior handling added to reject duplicate compact-block reconstruction attempts
No remote code execution or consensus change; denial-of-service/robustness issue
Evidence from the diff
In ProcessCompactBlockTxns, after a previous FillBlock call the PartiallyDownloadedBlock’s header can be nulled, but the PartiallyDownloadedBlock pointer may remain if RemoveBlockRequest was not called. A subsequent blocktxn message would then reach LookupBlockIndex(partialBlock.header.hashPrevBlock) with an empty header, violating an Assume invariant and causing a debug-build assertion failure. The patch adds an explicit IsNull() check, calls RemoveBlockRequest, Misbehaving(), logs, and returns. It also adds a comment explaining why the failed partialBlock is retained after READ_STATUS_FAILED.
Changed components
src/net_processing.cppProcessCompactBlockTxnscompact block reconstruction (BIP 152)PartiallyDownloadedBlockInspect captured patch +13 / −0
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index e1674b6d..336669a8 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3329,6 +3329,16 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl
PartiallyDownloadedBlock& partialBlock = *range_flight.first->second.second->partialBlock;
+ if (partialBlock.header.IsNull()) {
+ // It is possible for the header to be empty if a previous call to FillBlock wiped the header, but left
+ // the PartiallyDownloadedBlock pointer around (i.e. did not call RemoveBlockRequest). In this case, we
+ // should not call LookupBlockIndex below.
+ RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId());
+ Misbehaving(peer, "previous compact block reconstruction attempt failed");
+ LogDebug(BCLog::NET, "Peer %d sent compact block transactions multiple times", pfrom.GetId());
+ return;
+ }
+
// We should not have gotten this far in compact block processing unless it's attached to a known header
const CBlockIndex* prev_block{Assume(m_chainman.m_blockman.LookupBlockIndex(partialBlock.header.hashPrevBlock))};
ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn,
@@ -3340,6 +3350,9 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl
} else if (status == READ_STATUS_FAILED) {
if (first_in_flight) {
// Might have collided, fall back to getdata now :(
+ // We keep the failed partialBlock to disallow processing another compact block announcement from the same
+ // peer for the same block. We let the full block download below continue under the same m_downloading_since
+ // timer.
std::vector<CInv> invs;
invs.emplace_back(MSG_BLOCK | GetFetchFlags(peer), block_transactions.blockhash);
MakeAndPushMessage(pfrom, NetMsgType::GETDATA, invs);
Why this scored 44/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.