-
Notifications
You must be signed in to change notification settings - Fork 35.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
p2p: make block download logic aware of limited peers threshold #28120
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1451,6 +1451,7 @@ void PeerManagerImpl::FindNextBlocks(std::vector<const CBlockIndex*>& vBlocks, c | |
{ | ||
std::vector<const CBlockIndex*> vToFetch; | ||
int nMaxHeight = std::min<int>(state->pindexBestKnownBlock->nHeight, nWindowEnd + 1); | ||
bool is_limited_peer = IsLimitedPeer(peer); | ||
NodeId waitingfor = -1; | ||
while (pindexWalk->nHeight < nMaxHeight) { | ||
// Read up to 128 (or more, if more blocks than that are needed) successors of pindexWalk (towards | ||
|
@@ -1473,30 +1474,46 @@ void PeerManagerImpl::FindNextBlocks(std::vector<const CBlockIndex*>& vBlocks, c | |
// We consider the chain that this peer is on invalid. | ||
return; | ||
} | ||
|
||
if (!CanServeWitnesses(peer) && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { | ||
// We wouldn't download this block or its descendants from this peer. | ||
return; | ||
} | ||
|
||
furszy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (pindex->nStatus & BLOCK_HAVE_DATA || (activeChain && activeChain->Contains(pindex))) { | ||
if (activeChain && pindex->HaveNumChainTxs()) | ||
if (activeChain && pindex->HaveNumChainTxs()) { | ||
state->pindexLastCommonBlock = pindex; | ||
} else if (!IsBlockRequested(pindex->GetBlockHash())) { | ||
// The block is not already downloaded, and not yet in flight. | ||
if (pindex->nHeight > nWindowEnd) { | ||
// We reached the end of the window. | ||
if (vBlocks.size() == 0 && waitingfor != peer.m_id) { | ||
// We aren't able to fetch anything, but we would be if the download window was one larger. | ||
if (nodeStaller) *nodeStaller = waitingfor; | ||
} | ||
return; | ||
} | ||
vBlocks.push_back(pindex); | ||
if (vBlocks.size() == count) { | ||
return; | ||
continue; | ||
} | ||
|
||
// Is block in-flight? | ||
if (IsBlockRequested(pindex->GetBlockHash())) { | ||
if (waitingfor == -1) { | ||
// This is the first already-in-flight block. | ||
waitingfor = mapBlocksInFlight.lower_bound(pindex->GetBlockHash())->second.first; | ||
} | ||
} else if (waitingfor == -1) { | ||
// This is the first already-in-flight block. | ||
waitingfor = mapBlocksInFlight.lower_bound(pindex->GetBlockHash())->second.first; | ||
continue; | ||
} | ||
|
||
// The block is not already downloaded, and not yet in flight. | ||
if (pindex->nHeight > nWindowEnd) { | ||
// We reached the end of the window. | ||
if (vBlocks.size() == 0 && waitingfor != peer.m_id) { | ||
// We aren't able to fetch anything, but we would be if the download window was one larger. | ||
if (nodeStaller) *nodeStaller = waitingfor; | ||
} | ||
return; | ||
} | ||
|
||
// Don't request blocks that go further than what limited peers can provide | ||
if (is_limited_peer && (state->pindexBestKnownBlock->nHeight - pindex->nHeight >= static_cast<int>(NODE_NETWORK_LIMITED_MIN_BLOCKS) - 2 /* two blocks buffer for possible races */)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might as well jump There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what you are referring to. Could you rephrase it please. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think what @naumenkogs is referring to here is that if this condition is true then So, we can either change the loop from range based to regular for-loop and advance the index, or move this check out of this loop and into the loop on line 1464 and break early if the condition is met. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ok, we are on the same page now. Thanks. Let's leave it for a follow-up. It would be nice to re-think this function and benchmark it properly. In a first glance, I think we would be optimizing the function for the less likely case and may degrade performance for the likely ones. |
||
continue; | ||
} | ||
|
||
vBlocks.push_back(pindex); | ||
if (vBlocks.size() == count) { | ||
return; | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
const bool is_limited_peer{IsLimitedPeer};