aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/net_processing.cpp110
-rwxr-xr-xtest/functional/feature_minchainwork.py2
-rwxr-xr-xtest/functional/p2p_segwit.py4
3 files changed, 72 insertions, 44 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 975ee16615..fe1b06b18c 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -61,6 +61,8 @@ static constexpr auto UNCONDITIONAL_RELAY_DELAY = 2min;
* Timeout = base + per_header * (expected number of headers) */
static constexpr auto HEADERS_DOWNLOAD_TIMEOUT_BASE = 15min;
static constexpr auto HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER = 1ms;
+/** How long to wait for a peer to respond to a getheaders request */
+static constexpr auto HEADERS_RESPONSE_TIME{2min};
/** Protect at least this many outbound peers from disconnection due to slow/
* behind headers chain.
*/
@@ -355,6 +357,9 @@ struct Peer {
/** Work queue of items requested by this peer **/
std::deque<CInv> m_getdata_requests GUARDED_BY(m_getdata_requests_mutex);
+ /** Time of the last getheaders message to this peer */
+ std::atomic<std::chrono::seconds> m_last_getheaders_timestamp{0s};
+
Peer(NodeId id)
: m_id{id}
{}
@@ -501,7 +506,7 @@ public:
private:
/** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
- void ConsiderEviction(CNode& pto, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+ void ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */
void EvictExtraOutboundPeers(std::chrono::seconds now) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
@@ -567,9 +572,12 @@ private:
void HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers);
/** Return true if the headers connect to each other, false otherwise */
bool CheckHeadersAreContinuous(const std::vector<CBlockHeader>& headers) const;
- /** Request further headers from this peer from a given block header */
- void FetchMoreHeaders(CNode& pfrom, const CBlockIndex *pindexLast, const Peer& peer);
- /** Potentially fetch blocks from this peer upon receipt of new headers tip */
+ /** Request further headers from this peer with a given locator.
+ * We don't issue a getheaders message if we have a recent one outstanding.
+ * This returns true if a getheaders is actually sent, and false otherwise.
+ */
+ bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer);
+ /** Potentially fetch blocks from this peer upon receipt of a new headers tip */
void HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* pindexLast);
/** Update peer state based on received headers message */
void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex *pindexLast, bool received_new_header, bool may_have_more_headers);
@@ -2228,15 +2236,14 @@ void PeerManagerImpl::HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer,
CNodeState *nodestate = State(pfrom.GetId());
nodestate->nUnconnectingHeaders++;
-
// Try to fill in the missing headers.
- m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), uint256()));
- LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n",
+ if (MaybeSendGetHeaders(pfrom, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), peer)) {
+ LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n",
headers[0].GetHash().ToString(),
headers[0].hashPrevBlock.ToString(),
m_chainman.m_best_header->nHeight,
pfrom.GetId(), nodestate->nUnconnectingHeaders);
-
+ }
// Set hashLastUnknownBlock for this peer, so that if we
// eventually get the headers - even from a different peer -
// we can use this peer to download.
@@ -2261,23 +2268,19 @@ bool PeerManagerImpl::CheckHeadersAreContinuous(const std::vector<CBlockHeader>&
return true;
}
-/*
- * Continue fetching headers from a given point.
- * pindexLast should be the last header we learned from a peer in their prior
- * headers message.
- *
- * This is used for headers sync with a peer; even if pindexLast is an ancestor
- * of a known chain (such as our tip) we don't yet know where the peer's chain
- * might fork from what we know, so we continue exactly from where the peer
- * left off.
- */
-void PeerManagerImpl::FetchMoreHeaders(CNode& pfrom, const CBlockIndex *pindexLast, const Peer& peer)
+bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer)
{
const CNetMsgMaker msgMaker(pfrom.GetCommonVersion());
- LogPrint(BCLog::NET, "more getheaders (%d) to end to peer=%d (startheight:%d)\n",
- pindexLast->nHeight, pfrom.GetId(), peer.m_starting_height);
- m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, m_chainman.ActiveChain().GetLocator(pindexLast), uint256()));
+ const auto current_time = GetTime<std::chrono::seconds>();
+ // Only allow a new getheaders message to go out if we don't have a recent
+ // one already in-flight
+ if (peer.m_last_getheaders_timestamp.load() < current_time - HEADERS_RESPONSE_TIME) {
+ m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, locator, uint256()));
+ peer.m_last_getheaders_timestamp = current_time;
+ return true;
+ }
+ return false;
}
/*
@@ -2458,7 +2461,10 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer,
// Consider fetching more headers.
if (nCount == MAX_HEADERS_RESULTS) {
// Headers message had its maximum size; the peer may have more headers.
- FetchMoreHeaders(pfrom, pindexLast, peer);
+ if (MaybeSendGetHeaders(pfrom, m_chainman.ActiveChain().GetLocator(pindexLast), peer)) {
+ LogPrint(BCLog::NET, "more getheaders (%d) to end to peer=%d (startheight:%d)\n",
+ pindexLast->nHeight, pfrom.GetId(), peer.m_starting_height);
+ }
}
UpdatePeerStateForReceivedHeaders(pfrom, pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS);
@@ -3228,8 +3234,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
}
if (best_block != nullptr) {
- m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), *best_block));
- LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", m_chainman.m_best_header->nHeight, best_block->ToString(), pfrom.GetId());
+ if (MaybeSendGetHeaders(pfrom, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), *peer)) {
+ LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n",
+ m_chainman.m_best_header->nHeight, best_block->ToString(),
+ pfrom.GetId());
+ }
}
return;
@@ -3402,7 +3411,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// others.
if (m_chainman.ActiveTip() == nullptr ||
(m_chainman.ActiveTip()->nChainWork < nMinimumChainWork && !pfrom.HasPermission(NetPermissionFlags::Download))) {
- LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because active chain has too little work\n", pfrom.GetId());
+ LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because active chain has too little work; sending empty response\n", pfrom.GetId());
+ // Just respond with an empty headers message, to tell the peer to
+ // go away but not treat us as unresponsive.
+ m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, std::vector<CBlock>()));
return;
}
@@ -3683,8 +3695,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
if (!m_chainman.m_blockman.LookupBlockIndex(cmpctblock.header.hashPrevBlock)) {
// Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers
- if (!m_chainman.ActiveChainstate().IsInitialBlockDownload())
- m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), uint256()));
+ if (!m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+ MaybeSendGetHeaders(pfrom, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), *peer);
+ }
return;
}
@@ -3958,6 +3971,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
}
+ // Assume that this is in response to any outstanding getheaders
+ // request we may have sent, and clear out the time of our last request
+ peer->m_last_getheaders_timestamp = 0s;
+
std::vector<CBlockHeader> headers;
// Bypass the normal CBlock deserialization, as we don't want to risk deserializing 2000 full blocks.
@@ -4386,7 +4403,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
return fMoreWork;
}
-void PeerManagerImpl::ConsiderEviction(CNode& pto, std::chrono::seconds time_in_seconds)
+void PeerManagerImpl::ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds)
{
AssertLockHeld(cs_main);
@@ -4424,10 +4441,15 @@ void PeerManagerImpl::ConsiderEviction(CNode& pto, std::chrono::seconds time_in_
pto.fDisconnect = true;
} else {
assert(state.m_chain_sync.m_work_header);
+ // Here, we assume that the getheaders message goes out,
+ // because it'll either go out or be skipped because of a
+ // getheaders in-flight already, in which case the peer should
+ // still respond to us with a sufficiently high work chain tip.
+ MaybeSendGetHeaders(pto,
+ m_chainman.ActiveChain().GetLocator(state.m_chain_sync.m_work_header->pprev),
+ peer);
LogPrint(BCLog::NET, "sending getheaders to outbound peer=%d to verify chain work (current best known block:%s, benchmark blockhash: %s)\n", pto.GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "<none>", state.m_chain_sync.m_work_header->GetBlockHash().ToString());
- m_connman.PushMessage(&pto, msgMaker.Make(NetMsgType::GETHEADERS, m_chainman.ActiveChain().GetLocator(state.m_chain_sync.m_work_header->pprev), uint256()));
state.m_chain_sync.m_sent_getheaders = true;
- constexpr auto HEADERS_RESPONSE_TIME{2min};
// Bump the timeout to allow a response, which could clear the timeout
// (if the response shows the peer has synced), reset the timeout (if
// the peer syncs to the required work but not to our tip), or result
@@ -4835,15 +4857,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
if (!state.fSyncStarted && !pto->fClient && !fImporting && !fReindex) {
// Only actively request headers from a single peer, unless we're close to today.
if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - 24 * 60 * 60) {
- state.fSyncStarted = true;
- state.m_headers_sync_timeout = current_time + HEADERS_DOWNLOAD_TIMEOUT_BASE +
- (
- // Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling
- // to maintain precision
- std::chrono::microseconds{HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER} *
- (GetAdjustedTime() - m_chainman.m_best_header->GetBlockTime()) / consensusParams.nPowTargetSpacing
- );
- nSyncStarted++;
const CBlockIndex* pindexStart = m_chainman.m_best_header;
/* If possible, start at the block preceding the currently
best known header. This ensures that we always get a
@@ -4854,8 +4867,19 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
got back an empty response. */
if (pindexStart->pprev)
pindexStart = pindexStart->pprev;
- LogPrint(BCLog::NET, "initial getheaders (%d) to peer=%d (startheight:%d)\n", pindexStart->nHeight, pto->GetId(), peer->m_starting_height);
- m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETHEADERS, m_chainman.ActiveChain().GetLocator(pindexStart), uint256()));
+ if (MaybeSendGetHeaders(*pto, m_chainman.ActiveChain().GetLocator(pindexStart), *peer)) {
+ LogPrint(BCLog::NET, "initial getheaders (%d) to peer=%d (startheight:%d)\n", pindexStart->nHeight, pto->GetId(), peer->m_starting_height);
+
+ state.fSyncStarted = true;
+ state.m_headers_sync_timeout = current_time + HEADERS_DOWNLOAD_TIMEOUT_BASE +
+ (
+ // Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling
+ // to maintain precision
+ std::chrono::microseconds{HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER} *
+ (GetAdjustedTime() - m_chainman.m_best_header->GetBlockTime()) / consensusParams.nPowTargetSpacing
+ );
+ nSyncStarted++;
+ }
}
}
@@ -5201,7 +5225,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// Check that outbound peers have reasonable chains
// GetTime() is used by this anti-DoS logic so we can test this using mocktime
- ConsiderEviction(*pto, GetTime<std::chrono::seconds>());
+ ConsiderEviction(*pto, *peer, GetTime<std::chrono::seconds>());
//
// Message: getdata (blocks)
diff --git a/test/functional/feature_minchainwork.py b/test/functional/feature_minchainwork.py
index fa10855a98..9d0ad5ef9d 100755
--- a/test/functional/feature_minchainwork.py
+++ b/test/functional/feature_minchainwork.py
@@ -82,7 +82,7 @@ class MinimumChainWorkTest(BitcoinTestFramework):
msg.hashstop = 0
peer.send_and_ping(msg)
time.sleep(5)
- assert "headers" not in peer.last_message
+ assert ("headers" not in peer.last_message or len(peer.last_message["headers"].headers) == 0)
self.log.info("Generating one more block")
self.generate(self.nodes[0], 1)
diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
index 952f1e5cc5..eedb7e6fa1 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -371,6 +371,10 @@ class SegWitTest(BitcoinTestFramework):
block1 = self.build_next_block()
block1.solve()
+ # Send an empty headers message, to clear out any prior getheaders
+ # messages that our peer may be waiting for us on.
+ self.test_node.send_message(msg_headers())
+
self.test_node.announce_block_and_wait_for_getdata(block1, use_header=False)
assert self.test_node.last_message["getdata"].inv[0].type == blocktype
test_witness_block(self.nodes[0], self.test_node, block1, True)