Merge bitcoin/bitcoin#33296: net: check for empty header before calling FillBlock

8b6264768030db1840041abeeaeefd6c227a2644 test: send duplicate blocktxn message in p2p_compactblocks.py (Eugene Siegel)
5e585a0fc4fd68dd7b4982054b34deae2e7aeb89 net: check for empty header before calling FillBlock (Eugene Siegel)

Pull request description:

  This avoids an Assume crash if multiple blocktxn messages are received. The first call to `FillBlock` would make the header empty via `SetNull` and the call right before the second `FillBlock` would crash [here](689a321976/src/net_processing.cpp (L3333)) since `LookupBlockIndex` won't find anything. Fix that by checking for an empty header before the Assume.

ACKs for top commit:
  instagibbs:
    reACK https://github.com/bitcoin/bitcoin/pull/33296/commits/8b6264768030db1840041abeeaeefd6c227a2644
  fjahr:
    tACK 8b6264768030db1840041abeeaeefd6c227a2644
  achow101:
    ACK 8b6264768030db1840041abeeaeefd6c227a2644
  mzumsande:
    Code Review ACK 8b6264768030db1840041abeeaeefd6c227a2644

Tree-SHA512: d43a6f652161d4f7e6137f207a3e95259fc51509279d20347b1698c91179c39c8fcb75d2668b13a6b220f478a03578573208a415804be1d8843acb057fa1a73a
This commit is contained in:
Ava Chow 2025-09-08 17:16:28 -07:00 committed by Tom Trevethan
parent 98bd7180c2
commit 53a5bd0790
2 changed files with 56 additions and 2 deletions

View file

@ -3810,6 +3810,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
}
PartiallyDownloadedBlock& partialBlock = *it->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(resp.blockhash, pfrom.GetId());
Misbehaving(pfrom.GetId(), 100, "previous compact block reconstruction attempt failed");
LogPrint(BCLog::NET, "Peer %d sent compact block transactions multiple times", pfrom.GetId());
return;
}
ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn);
if (status == READ_STATUS_INVALID) {
RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect
@ -3817,6 +3828,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
} else if (status == READ_STATUS_FAILED) {
// 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.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom), resp.blockhash));
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs));