From 75d27fefc7a04ebdda7be5724a014b6a896e7325 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sat, 4 May 2024 15:38:39 -0400 Subject: [PATCH] net: reduce LOCK(cs_main) scope in ProcessGetBlockData This also changes behavior if ReadBlockFromDisk or ReadRawBlockFromDisk fails. Previously, the node would crash due to an assert. This has been replaced with logging the error, disconnecting the peer, and returning early. --- src/net_processing.cpp | 96 ++++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 37 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 84f9a5dc8b..57e2f7409b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2421,38 +2421,48 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } } - LOCK(cs_main); - const CBlockIndex* pindex = m_chainman.m_blockman.LookupBlockIndex(inv.hash); - if (!pindex) { - return; - } - 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; - } - // disconnect node in case we have reached the outbound limit for serving historical blocks - if (m_connman.OutboundTargetReached(true) && - (((m_chainman.m_best_header != nullptr) && (m_chainman.m_best_header->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && - !pfrom.HasPermission(NetPermissionFlags::Download) // nodes with the download permission may exceed target - ) { - LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); - pfrom.fDisconnect = true; - return; - } - // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold - if (!pfrom.HasPermission(NetPermissionFlags::NoBan) && ( - (((peer.m_our_services & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((peer.m_our_services & 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, disconnect peer=%d\n", pfrom.GetId()); - //disconnect node and prevent it from stalling (would otherwise wait for the missing block) - pfrom.fDisconnect = true; - return; - } - // Pruned nodes may have deleted the block, so check whether - // it's available before trying to send. - if (!(pindex->nStatus & BLOCK_HAVE_DATA)) { - return; + const CBlockIndex* pindex{nullptr}; + const CBlockIndex* tip{nullptr}; + bool can_direct_fetch{false}; + FlatFilePos block_pos{}; + { + LOCK(cs_main); + pindex = m_chainman.m_blockman.LookupBlockIndex(inv.hash); + if (!pindex) { + return; + } + 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; + } + // disconnect node in case we have reached the outbound limit for serving historical blocks + if (m_connman.OutboundTargetReached(true) && + (((m_chainman.m_best_header != nullptr) && (m_chainman.m_best_header->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && + !pfrom.HasPermission(NetPermissionFlags::Download) // nodes with the download permission may exceed target + ) { + LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); + pfrom.fDisconnect = true; + return; + } + tip = m_chainman.ActiveChain().Tip(); + // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold + if (!pfrom.HasPermission(NetPermissionFlags::NoBan) && ( + (((peer.m_our_services & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((peer.m_our_services & NODE_NETWORK) != NODE_NETWORK) && (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, disconnect peer=%d\n", pfrom.GetId()); + //disconnect node and prevent it from stalling (would otherwise wait for the missing block) + pfrom.fDisconnect = true; + return; + } + // Pruned nodes may have deleted the block, so check whether + // it's available before trying to send. + if (!(pindex->nStatus & BLOCK_HAVE_DATA)) { + return; + } + can_direct_fetch = CanDirectFetch(); + block_pos = pindex->GetBlockPos(); } + std::shared_ptr pblock; if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { pblock = a_recent_block; @@ -2460,16 +2470,28 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // 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 block_data; - if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos())) { - assert(!"cannot load block from disk"); + if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, block_pos)) { + if (WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.IsBlockPruned(*pindex))) { + LogPrint(BCLog::NET, "Block was pruned before it could be read, disconnect peer=%s\n", pfrom.GetId()); + } else { + LogError("Cannot load block from disk, disconnect peer=%d\n", pfrom.GetId()); + } + pfrom.fDisconnect = true; + return; } MakeAndPushMessage(pfrom, NetMsgType::BLOCK, Span{block_data}); // Don't set pblock as we've sent the block } else { // Send block from disk std::shared_ptr pblockRead = std::make_shared(); - if (!m_chainman.m_blockman.ReadBlockFromDisk(*pblockRead, *pindex)) { - assert(!"cannot load block from disk"); + if (!m_chainman.m_blockman.ReadBlockFromDisk(*pblockRead, block_pos)) { + if (WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.IsBlockPruned(*pindex))) { + LogPrint(BCLog::NET, "Block was pruned before it could be read, disconnect peer=%s\n", pfrom.GetId()); + } else { + LogError("Cannot load block from disk, disconnect peer=%d\n", pfrom.GetId()); + } + pfrom.fDisconnect = true; + return; } pblock = pblockRead; } @@ -2507,7 +2529,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // they won't have a useful mempool to match against a compact block, // and we don't feel like constructing the object for them, so // instead we respond with the full, non-compact block. - if (CanDirectFetch() && pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) { + if (can_direct_fetch && pindex->nHeight >= tip->nHeight - MAX_CMPCTBLOCK_DEPTH) { if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { MakeAndPushMessage(pfrom, NetMsgType::CMPCTBLOCK, *a_recent_compact_block); } else { @@ -2528,7 +2550,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // and we want it right after the last block so they don't // wait for other stuff first. std::vector vInv; - vInv.emplace_back(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash()); + vInv.emplace_back(MSG_BLOCK, tip->GetBlockHash()); MakeAndPushMessage(pfrom, NetMsgType::INV, vInv); peer.m_continuation_block.SetNull(); }