mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-04-25 16:10:33 +02:00
Merge bitcoin/bitcoin#26326: net: don't lock cs_main while reading blocks in net processing
75d27fefc7a04ebdda7be5724a014b6a896e7325 net: reduce LOCK(cs_main) scope in ProcessGetBlockData (Andrew Toth)
613a45cd4b5482aedbdc7c61c839ea05996935c6 net: reduce LOCK(cs_main) scope in GETBLOCKTXN (Andrew Toth)
Pull request description:
Inspired by https://github.com/bitcoin/bitcoin/pull/11913 and https://github.com/bitcoin/bitcoin/pull/26308.
`cs_main` doesn't need to be locked while reading blocks. This removes the locks in `net_processing`.
ACKs for top commit:
sr-gi:
ACK [75d27fe](75d27fefc7
)
achow101:
ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325
furszy:
ACK 75d27fefc with a non-blocking nit.
mzumsande:
Code Review ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325
TheCharlatan:
ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325
Tree-SHA512: 79b85f748f68ecfb2f2afd3267857dd41b8e76dd482c9c922037399dcbce7b1e5d4c708a4f5fd17c3fb6699b0d88f26a17cc1d92db115dd43c8d4392ae27cec4
This commit is contained in:
commit
573f631165
@ -118,6 +118,7 @@ static const unsigned int MAX_HEADERS_RESULTS = 2000;
|
|||||||
static const int MAX_CMPCTBLOCK_DEPTH = 5;
|
static const int MAX_CMPCTBLOCK_DEPTH = 5;
|
||||||
/** Maximum depth of blocks we're willing to respond to GETBLOCKTXN requests for. */
|
/** Maximum depth of blocks we're willing to respond to GETBLOCKTXN requests for. */
|
||||||
static const int MAX_BLOCKTXN_DEPTH = 10;
|
static const int MAX_BLOCKTXN_DEPTH = 10;
|
||||||
|
static_assert(MAX_BLOCKTXN_DEPTH <= MIN_BLOCKS_TO_KEEP, "MAX_BLOCKTXN_DEPTH too high");
|
||||||
/** Size of the "block download window": how far ahead of our current height do we fetch?
|
/** Size of the "block download window": how far ahead of our current height do we fetch?
|
||||||
* Larger windows tolerate larger download speed differences between peer, but increase the potential
|
* Larger windows tolerate larger download speed differences between peer, but increase the potential
|
||||||
* degree of disordering of blocks on disk (which make reindexing and pruning harder). We'll probably
|
* degree of disordering of blocks on disk (which make reindexing and pruning harder). We'll probably
|
||||||
@ -2420,38 +2421,48 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
LOCK(cs_main);
|
const CBlockIndex* pindex{nullptr};
|
||||||
const CBlockIndex* pindex = m_chainman.m_blockman.LookupBlockIndex(inv.hash);
|
const CBlockIndex* tip{nullptr};
|
||||||
if (!pindex) {
|
bool can_direct_fetch{false};
|
||||||
return;
|
FlatFilePos block_pos{};
|
||||||
}
|
{
|
||||||
if (!BlockRequestAllowed(pindex)) {
|
LOCK(cs_main);
|
||||||
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom.GetId());
|
pindex = m_chainman.m_blockman.LookupBlockIndex(inv.hash);
|
||||||
return;
|
if (!pindex) {
|
||||||
}
|
return;
|
||||||
// disconnect node in case we have reached the outbound limit for serving historical blocks
|
}
|
||||||
if (m_connman.OutboundTargetReached(true) &&
|
if (!BlockRequestAllowed(pindex)) {
|
||||||
(((m_chainman.m_best_header != nullptr) && (m_chainman.m_best_header->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) &&
|
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom.GetId());
|
||||||
!pfrom.HasPermission(NetPermissionFlags::Download) // nodes with the download permission may exceed target
|
return;
|
||||||
) {
|
}
|
||||||
LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId());
|
// disconnect node in case we have reached the outbound limit for serving historical blocks
|
||||||
pfrom.fDisconnect = true;
|
if (m_connman.OutboundTargetReached(true) &&
|
||||||
return;
|
(((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
|
||||||
// Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold
|
) {
|
||||||
if (!pfrom.HasPermission(NetPermissionFlags::NoBan) && (
|
LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId());
|
||||||
(((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 */) )
|
pfrom.fDisconnect = true;
|
||||||
)) {
|
return;
|
||||||
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)
|
tip = m_chainman.ActiveChain().Tip();
|
||||||
pfrom.fDisconnect = true;
|
// Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold
|
||||||
return;
|
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 */) )
|
||||||
// Pruned nodes may have deleted the block, so check whether
|
)) {
|
||||||
// it's available before trying to send.
|
LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold, disconnect peer=%d\n", pfrom.GetId());
|
||||||
if (!(pindex->nStatus & BLOCK_HAVE_DATA)) {
|
//disconnect node and prevent it from stalling (would otherwise wait for the missing block)
|
||||||
return;
|
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<const CBlock> pblock;
|
std::shared_ptr<const CBlock> pblock;
|
||||||
if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) {
|
if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) {
|
||||||
pblock = a_recent_block;
|
pblock = a_recent_block;
|
||||||
@ -2459,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,
|
// Fast-path: in this case it is possible to serve the block directly from disk,
|
||||||
// as the network format matches the format on disk
|
// as the network format matches the format on disk
|
||||||
std::vector<uint8_t> block_data;
|
std::vector<uint8_t> block_data;
|
||||||
if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos())) {
|
if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, block_pos)) {
|
||||||
assert(!"cannot load block from disk");
|
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});
|
MakeAndPushMessage(pfrom, NetMsgType::BLOCK, Span{block_data});
|
||||||
// Don't set pblock as we've sent the block
|
// Don't set pblock as we've sent the block
|
||||||
} else {
|
} else {
|
||||||
// Send block from disk
|
// Send block from disk
|
||||||
std::shared_ptr<CBlock> pblockRead = std::make_shared<CBlock>();
|
std::shared_ptr<CBlock> pblockRead = std::make_shared<CBlock>();
|
||||||
if (!m_chainman.m_blockman.ReadBlockFromDisk(*pblockRead, *pindex)) {
|
if (!m_chainman.m_blockman.ReadBlockFromDisk(*pblockRead, block_pos)) {
|
||||||
assert(!"cannot load block from disk");
|
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;
|
pblock = pblockRead;
|
||||||
}
|
}
|
||||||
@ -2506,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,
|
// 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
|
// and we don't feel like constructing the object for them, so
|
||||||
// instead we respond with the full, non-compact block.
|
// 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()) {
|
if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) {
|
||||||
MakeAndPushMessage(pfrom, NetMsgType::CMPCTBLOCK, *a_recent_compact_block);
|
MakeAndPushMessage(pfrom, NetMsgType::CMPCTBLOCK, *a_recent_compact_block);
|
||||||
} else {
|
} else {
|
||||||
@ -2527,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
|
// 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.emplace_back(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash());
|
vInv.emplace_back(MSG_BLOCK, tip->GetBlockHash());
|
||||||
MakeAndPushMessage(pfrom, NetMsgType::INV, vInv);
|
MakeAndPushMessage(pfrom, NetMsgType::INV, vInv);
|
||||||
peer.m_continuation_block.SetNull();
|
peer.m_continuation_block.SetNull();
|
||||||
}
|
}
|
||||||
@ -4366,6 +4389,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
FlatFilePos block_pos{};
|
||||||
{
|
{
|
||||||
LOCK(cs_main);
|
LOCK(cs_main);
|
||||||
|
|
||||||
@ -4376,15 +4400,21 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_BLOCKTXN_DEPTH) {
|
if (pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_BLOCKTXN_DEPTH) {
|
||||||
CBlock block;
|
block_pos = pindex->GetBlockPos();
|
||||||
const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, *pindex)};
|
|
||||||
assert(ret);
|
|
||||||
|
|
||||||
SendBlockTransactions(pfrom, *peer, block, req);
|
|
||||||
return;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!block_pos.IsNull()) {
|
||||||
|
CBlock block;
|
||||||
|
const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, block_pos)};
|
||||||
|
// If height is above MAX_BLOCKTXN_DEPTH then this block cannot get
|
||||||
|
// pruned after we release cs_main above, so this read should never fail.
|
||||||
|
assert(ret);
|
||||||
|
|
||||||
|
SendBlockTransactions(pfrom, *peer, block, req);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// If an older block is requested (should never happen in practice,
|
// If an older block is requested (should never happen in practice,
|
||||||
// but can happen in tests) send a block response instead of a
|
// but can happen in tests) send a block response instead of a
|
||||||
// blocktxn response. Sending a full block response instead of a
|
// blocktxn response. Sending a full block response instead of a
|
||||||
|
Loading…
x
Reference in New Issue
Block a user