net: Avoid duplicate getheaders requests.

The current logic for syncing headers may lead to lots of duplicate
getheaders requests being sent:  If a new block arrives while the node
is in headers sync, it will send getheaders in response to the block
announcement.  When the headers arrive, the message will be of maximum
size and so a follow-up request will be sent---all of that in addition
to the existing headers syncing.  This will create a second "chain" of
getheaders requests.  If more blocks arrive, this may even lead to
arbitrarily many parallel chains of redundant requests.

This patch changes the behaviour to only request more headers after a
maximum-sized message when it contained at least one unknown header.
This avoids sustaining parallel chains of redundant requests.

Note that this patch avoids the issues raised in the discussion of
https://github.com/bitcoin/bitcoin/pull/6821:  There is no risk of the
node being permanently blocked.  At the latest when a new block arrives
this will trigger a new getheaders request and restart syncing.

1 files changed, 8 insertions, 1 deletions

diff --git a/src/main.cpp b/src/main.cpp
index ee30954b48..5816f799a0 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -5091,6 +5091,13 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
             return true;
         }
 
+        // If we already know the last header in the message, then it contains
+        // no new information for us.  In this case, we do not request
+        // more headers later.  This prevents multiple chains of redundant
+        // getheader requests from running in parallel if triggered by incoming
+        // blocks while the node is still in initial headers sync.
+        const bool hasNewHeaders = (mapBlockIndex.count(headers.back().GetHash()) == 0);
+
         CBlockIndex *pindexLast = NULL;
         BOOST_FOREACH(const CBlockHeader& header, headers) {
             CValidationState state;
@@ -5111,7 +5118,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
         if (pindexLast)
             UpdateBlockAvailability(pfrom->GetId(), pindexLast->GetBlockHash());
 
-        if (nCount == MAX_HEADERS_RESULTS && pindexLast) {
+        if (nCount == MAX_HEADERS_RESULTS && pindexLast && hasNewHeaders) {
             // Headers message had its maximum size; the peer may have more headers.
             // TODO: optimize: if pindexLast is an ancestor of chainActive.Tip or pindexBestHeader, continue
             // from there instead.