From d1fd7f794e40977598faf4f5f1756c43b5417831 Mon Sep 17 00:00:00 2001
From: Peter Tschipper <peter.tschipper@gmail.com>
Date: Wed, 26 Apr 2017 06:58:26 -0700
Subject: [PATCH] Clear ThinBlockTimer after the request is sent rather than
 when  received

Fix for issue #477

Clearing the timer after the request is sent rather than when the block
is received prevents the possiblity of downloading a block twice before
the RequestManager re-request timeout is exceeded.  Rely instead
only on the RequestManager to determine whether a block download timeout
has been exceeded.

Add documentation explaining the functioning of the thinblock timer.
---
 src/main.cpp      | 6 +++---
 src/parallel.cpp  | 3 ---
 src/thinblock.cpp | 8 +++++++-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/main.cpp b/src/main.cpp
index 14dae534c93..4ec7ffc8f64 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -628,6 +628,9 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<CBl
     CNodeState *state = State(nodeid);
     DbgAssert(state != NULL, return);
 
+    // If started then clear the thinblock timer used for preferential downloading
+    thindata.ClearThinBlockTimer(hash);
+
     // BU why mark as received? because this erases it from the inflight list.  Instead we'll check for it
     // BU removed: MarkBlockAsReceived(hash);
     std::map<uint256, std::pair<NodeId, std::list<QueuedBlock>::iterator> >::iterator itInFlight = mapBlocksInFlight.find(hash);
@@ -6181,9 +6184,6 @@ bool ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vRecv, in
                 pfrom->thinBlock.SetNull();
             }
 
-            // Clear the thinblock timer used for preferential download
-            thindata.ClearThinBlockTimer(inv.hash);
-
             LOCK(cs_main);
             Misbehaving(pfrom->GetId(), 100);
             return error("incorrectly constructed xblocktx or inconsistent thinblock data received.  Banning peer=%d", pfrom->id);
diff --git a/src/parallel.cpp b/src/parallel.cpp
index 71e4e6c7560..18a8a917e4a 100644
--- a/src/parallel.cpp
+++ b/src/parallel.cpp
@@ -576,9 +576,6 @@ void HandleBlockMessageThread(CNode *pfrom, const string &strCommand, const CBlo
         }
     }
 
-    // Clear the thinblock timer used for preferential download
-    thindata.ClearThinBlockTimer(inv.hash);
-
     // Erase any txns from the orphan cache that are no longer needed
     PV.ClearOrphanCache(block);
 
diff --git a/src/thinblock.cpp b/src/thinblock.cpp
index b89ea4597de..e433cb1fb87 100644
--- a/src/thinblock.cpp
+++ b/src/thinblock.cpp
@@ -891,6 +891,12 @@ string CThinBlockData::MempoolLimiterBytesSavedToString()
     return ss.str();
 }
 
+// Preferential Thinblock Timer:
+// The purpose of the timer is to ensure that we more often download an XTHINBLOCK rather than a full block.
+// The timer is started when we receive the first announcement indicating there is a new block to download.  If the
+// block inventory is from a non XTHIN node then we will continue to wait for block announcements until either we
+// get one from an XTHIN capable node or the timer is exceeded.  If the timer is exceeded before receiving an
+// announcement from an XTHIN node then we just download a full block instead of an xthin.
 bool CThinBlockData::CheckThinblockTimer(uint256 hash)
 {
     LOCK(cs_mapThinBlockTimer);
@@ -913,7 +919,7 @@ bool CThinBlockData::CheckThinblockTimer(uint256 hash)
     }
     return true;
 }
-
+// The timer is cleared as soon as we request a block or thinblock.
 void CThinBlockData::ClearThinBlockTimer(uint256 hash)
 {
     LOCK(cs_mapThinBlockTimer);