Merge #18962: net processing: Only send a getheaders for one block in an INV

746736639e [net processing] Only send a getheaders for one block in an INV (John Newbery)

Pull request description:

  Headers-first is the primary method of announcement on the network. If a node fell back sending blocks by inv, it's probably for a re-org. The final block hash provided should be the highest, so send a getheaders and then fetch the blocks we need to catch up.

  Sending many GETHEADERS messages to the peer would cause them to send a large number of potentially large HEADERS messages with redundant data, which is a waste of bandwidth.

ACKs for top commit:
  sipa:
    utACK 746736639e
  mzumsande:
    utACK 746736639e as per ajtowns' reasoning.
  naumenkogs:
    utACK 7467366
  ajtowns:
    ACK 746736639e
  jonatack:
    ACK 746736639e

Tree-SHA512: 59e243b80d3f0873709dfacb2e4ffba34689aad7de31ec7f69a64e0e3a0756235a0150e4082ff5de823949ba4411ee1aed2344b4749b62e0eb1ea906e41f5ea9
This commit is contained in:
Wladimir J. van der Laan 2020-05-14 20:43:31 +02:00
commit 553bb3fc3d
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D

View file

@ -2420,6 +2420,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
uint32_t nFetchFlags = GetFetchFlags(pfrom);
const auto current_time = GetTime<std::chrono::microseconds>();
uint256* best_block{nullptr};
for (CInv &inv : vInv)
{
@ -2436,17 +2437,14 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
if (inv.type == MSG_BLOCK) {
UpdateBlockAvailability(pfrom->GetId(), inv.hash);
if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) {
// We used to request the full block here, but since headers-announcements are now the
// primary method of announcement on the network, and since, in the case that a node
// fell back to inv we probably have a reorg which we should get the headers for first,
// we now only provide a getheaders response here. When we receive the headers, we will
// then ask for the blocks we need.
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), inv.hash));
LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, inv.hash.ToString(), pfrom->GetId());
// Headers-first is the primary method of announcement on
// the network. If a node fell back to sending blocks by inv,
// it's probably for a re-org. The final block hash
// provided should be the highest, so send a getheaders and
// then fetch the blocks we need to catch up.
best_block = &inv.hash;
}
}
else
{
} else {
pfrom->AddInventoryKnown(inv);
if (fBlocksOnly) {
LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom->GetId());
@ -2457,6 +2455,12 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
}
}
}
if (best_block != nullptr) {
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), *best_block));
LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, best_block->ToString(), pfrom->GetId());
}
return true;
}