foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-02 09:46:52 -05:00
Code Issues Activity

Merge #21235: p2p: Clarify disconnect log message in ProcessGetBlockData, remove send bool

Browse source 
fa81773243 style-only: Remove whitespace (MarcoFalke)
fae77b9e6d net: Simplify ProcessGetBlockData execution by removing send flag. (Patrick Strateman)
fae7c0429f log: Clarify that block request below NODE_NETWORK_LIMITED_MIN_BLOCKS disconnects (MarcoFalke)

Pull request description:

  * Clarify that "ignoring" really means "disconnect" in the log
  * Revive a refactor I took from #13670

ACKs for top commit:
  jnewbery:
    utACK fa81773243
  sipa:
    utACK fa81773243

Tree-SHA512: 0a4fcb979cb82c4e26012881eeaf903c38dfbb85d461476c01e35294760744746a79c48ffad827fe31c1b830f40c6e4240529c71e375146e4d0313c3b7d784ca
This commit is contained in:
MarcoFalke 2021-03-19 18:56:12 +01:00
commit 3a12fdba51
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
1 changed files with 87 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

178
src/net_processing.cpp
View file

@ -1530,7 +1530,6 @@ static void RelayAddress(const CNode& originator,
void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& inv) void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& inv)
{ {
bool send = false;
std::shared_ptr<const CBlock> a_recent_block; std::shared_ptr<const CBlock> a_recent_block;
std::shared_ptr<const CBlockHeaderAndShortTxIDs> a_recent_compact_block; std::shared_ptr<const CBlockHeaderAndShortTxIDs> a_recent_compact_block;
bool fWitnessesPresentInARecentCompactBlock; bool fWitnessesPresentInARecentCompactBlock;
@ -1566,120 +1565,117 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
LOCK(cs_main); LOCK(cs_main);
const CBlockIndex* pindex = m_chainman.m_blockman.LookupBlockIndex(inv.hash); const CBlockIndex* pindex = m_chainman.m_blockman.LookupBlockIndex(inv.hash);
if (pindex) { if (!pindex) {
send = BlockRequestAllowed(pindex); return;
if (!send) { }
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom.GetId()); if (!BlockRequestAllowed(pindex)) {
} LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom.GetId());
return;
} }
const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); const CNetMsgMaker msgMaker(pfrom.GetCommonVersion());
// disconnect node in case we have reached the outbound limit for serving historical blocks // disconnect node in case we have reached the outbound limit for serving historical blocks
if (send && if (m_connman.OutboundTargetReached(true) &&
m_connman.OutboundTargetReached(true) &&
(((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) &&
!pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target
) { ) {
LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId());
//disconnect node
pfrom.fDisconnect = true; pfrom.fDisconnect = true;
send = false; return;
} }
// Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold
if (send && !pfrom.HasPermission(PF_NOBAN) && ( if (!pfrom.HasPermission(PF_NOBAN) && (
(((pfrom.GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom.GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) (((pfrom.GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom.GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) )
)) { )) {
LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold from peer=%d\n", pfrom.GetId()); LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold, disconnect peer=%d\n", pfrom.GetId());
//disconnect node and prevent it from stalling (would otherwise wait for the missing block) //disconnect node and prevent it from stalling (would otherwise wait for the missing block)
pfrom.fDisconnect = true; pfrom.fDisconnect = true;
send = false; return;
} }
// Pruned nodes may have deleted the block, so check whether // Pruned nodes may have deleted the block, so check whether
// it's available before trying to send. // it's available before trying to send.
if (send && (pindex->nStatus & BLOCK_HAVE_DATA)) if (!(pindex->nStatus & BLOCK_HAVE_DATA)) {
{ return;
std::shared_ptr<const CBlock> pblock; }
if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { std::shared_ptr<const CBlock> pblock;
pblock = a_recent_block; if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) {
pblock = a_recent_block;
} else if (inv.IsMsgWitnessBlk()) {
// Fast-path: in this case it is possible to serve the block directly from disk,
// as the network format matches the format on disk
std::vector<uint8_t> block_data;
if (!ReadRawBlockFromDisk(block_data, pindex, m_chainparams.MessageStart())) {
assert(!"cannot load block from disk");
}
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, MakeSpan(block_data)));
// Don't set pblock as we've sent the block
} else {
// Send block from disk
std::shared_ptr<CBlock> pblockRead = std::make_shared<CBlock>();
if (!ReadBlockFromDisk(*pblockRead, pindex, m_chainparams.GetConsensus())) {
assert(!"cannot load block from disk");
}
pblock = pblockRead;
}
if (pblock) {
if (inv.IsMsgBlk()) {
m_connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, *pblock));
} else if (inv.IsMsgWitnessBlk()) { } else if (inv.IsMsgWitnessBlk()) {
// Fast-path: in this case it is possible to serve the block directly from disk, m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock));
// as the network format matches the format on disk } else if (inv.IsMsgFilteredBlk()) {
std::vector<uint8_t> block_data; bool sendMerkleBlock = false;
if (!ReadRawBlockFromDisk(block_data, pindex, m_chainparams.MessageStart())) { CMerkleBlock merkleBlock;
assert(!"cannot load block from disk"); if (pfrom.m_tx_relay != nullptr) {
} LOCK(pfrom.m_tx_relay->cs_filter);
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, MakeSpan(block_data))); if (pfrom.m_tx_relay->pfilter) {
// Don't set pblock as we've sent the block sendMerkleBlock = true;
} else { merkleBlock = CMerkleBlock(*pblock, *pfrom.m_tx_relay->pfilter);
// Send block from disk
std::shared_ptr<CBlock> pblockRead = std::make_shared<CBlock>();
if (!ReadBlockFromDisk(*pblockRead, pindex, m_chainparams.GetConsensus())) {
assert(!"cannot load block from disk");
}
pblock = pblockRead;
}
if (pblock) {
if (inv.IsMsgBlk()) {
m_connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, *pblock));
} else if (inv.IsMsgWitnessBlk()) {
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock));
} else if (inv.IsMsgFilteredBlk()) {
bool sendMerkleBlock = false;
CMerkleBlock merkleBlock;
if (pfrom.m_tx_relay != nullptr) {
LOCK(pfrom.m_tx_relay->cs_filter);
if (pfrom.m_tx_relay->pfilter) {
sendMerkleBlock = true;
merkleBlock = CMerkleBlock(*pblock, *pfrom.m_tx_relay->pfilter);
}
} }
if (sendMerkleBlock) { }
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); if (sendMerkleBlock) {
// CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock));
// This avoids hurting performance by pointlessly requiring a round-trip // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see
// Note that there is currently no way for a node to request any single transactions we didn't send here - // This avoids hurting performance by pointlessly requiring a round-trip
// they must either disconnect and retry or request the full block. // Note that there is currently no way for a node to request any single transactions we didn't send here -
// Thus, the protocol spec specified allows for us to provide duplicate txn here, // they must either disconnect and retry or request the full block.
// however we MUST always provide at least what the remote peer needs // Thus, the protocol spec specified allows for us to provide duplicate txn here,
typedef std::pair<unsigned int, uint256> PairType; // however we MUST always provide at least what the remote peer needs
for (PairType& pair : merkleBlock.vMatchedTxn) typedef std::pair<unsigned int, uint256> PairType;
m_connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::TX, *pblock->vtx[pair.first])); for (PairType& pair : merkleBlock.vMatchedTxn)
} m_connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::TX, *pblock->vtx[pair.first]));
// else }
// no response // else
} else if (inv.IsMsgCmpctBlk()) { // no response
// If a peer is asking for old blocks, we're almost guaranteed } else if (inv.IsMsgCmpctBlk()) {
// they won't have a useful mempool to match against a compact block, // If a peer is asking for old blocks, we're almost guaranteed
// and we don't feel like constructing the object for them, so // they won't have a useful mempool to match against a compact block,
// instead we respond with the full, non-compact block. // and we don't feel like constructing the object for them, so
bool fPeerWantsWitness = State(pfrom.GetId())->fWantsCmpctWitness; // instead we respond with the full, non-compact block.
int nSendFlags = fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; bool fPeerWantsWitness = State(pfrom.GetId())->fWantsCmpctWitness;
if (CanDirectFetch() && pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) { int nSendFlags = fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS;
if ((fPeerWantsWitness || !fWitnessesPresentInARecentCompactBlock) && a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { if (CanDirectFetch() && pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) {
m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); if ((fPeerWantsWitness || !fWitnessesPresentInARecentCompactBlock) && a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) {
} else { m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *a_recent_compact_block));
CBlockHeaderAndShortTxIDs cmpctblock(*pblock, fPeerWantsWitness);
m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock));
}
} else { } else {
m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock)); CBlockHeaderAndShortTxIDs cmpctblock(*pblock, fPeerWantsWitness);
m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock));
} }
} else {
m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock));
} }
} }
}
{ {
LOCK(peer.m_block_inv_mutex); LOCK(peer.m_block_inv_mutex);
// Trigger the peer node to send a getblocks request for the next batch of inventory // Trigger the peer node to send a getblocks request for the next batch of inventory
if (inv.hash == peer.m_continuation_block) { if (inv.hash == peer.m_continuation_block) {
// Send immediately. This must send even if redundant, // Send immediately. This must send even if redundant,
// and we want it right after the last block so they don't // and we want it right after the last block so they don't
// wait for other stuff first. // wait for other stuff first.
std::vector<CInv> vInv; std::vector<CInv> vInv;
vInv.push_back(CInv(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash())); vInv.push_back(CInv(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash()));
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv)); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv));
peer.m_continuation_block.SetNull(); peer.m_continuation_block.SetNull();
}
} }
} }
} }