Comments on block protocol handling of staleness.

Author: evoskuil

src/protocols/protocol_block_in.cpp

@@ -150,7 +150,8 @@ void protocol_block_in::handle_fetch_block_locator(const code& ec,
 //-----------------------------------------------------------------------------
 
 // TODO: move headers to a derived class protocol_block_in_31800.
-// This originates from send_header->annoucements and get_headers requests.
+// This originates from send_header->annoucements and get_headers requests, or
+// from an unsolicited announcement. There is no way to distinguish.
 bool protocol_block_in::handle_receive_headers(const code& ec,
     headers_const_ptr message)
 {
@@ -176,7 +177,8 @@ bool protocol_block_in::handle_receive_headers(const code& ec,
     return true;
 }
 
-// This originates from default annoucements and get_blocks requests.
+// This originates from default annoucements and get_blocks requests, or from
+// an unsolicited announcement. There is no way to distinguish.
 bool protocol_block_in::handle_receive_inventory(const code& ec,
     inventory_const_ptr message)
 {
@@ -293,7 +295,8 @@ bool protocol_block_in::handle_receive_block(const code& ec,
     mutex.unlock();
     ///////////////////////////////////////////////////////////////////////////
 
-    // It is common for block announcements to cause block requests to be sent
+    // If a peer sends a block unannounced we drop the peer - always. However
+    // it is common for block announcements to cause block requests to be sent
     // out of backlog order due to interleaving of threads. This results in
     // channel drops during initial block download but not after sync. The
     // resolution to this issue is use of headers-first sync, but short of that

src/protocols/protocol_block_out.cpp

@@ -170,6 +170,11 @@ void protocol_block_out::handle_fetch_locator_headers(const code& ec,
     if (message->elements().empty())
         return;
 
+
+    // Allow a peer to sync despite our being stale.
+    ////if (chain_.is_stale())
+    ////    return;
+
     // Respond to get_headers with headers.
     SEND2(*message, handle_send, _1, message->command);
 
@@ -205,6 +210,10 @@ bool protocol_block_out::handle_receive_get_blocks(const code& ec,
         return true;
     }
 
+    // Allow a peer to sync despite our being stale.
+    ////if (chain_.is_stale())
+    ////    return true;
+
     const auto threshold = last_locator_top_.load();
 
     chain_.fetch_locator_block_hashes(message, threshold, max_get_blocks,
@@ -230,6 +239,10 @@ void protocol_block_out::handle_fetch_locator_hashes(const code& ec,
     if (message->inventories().empty())
         return;
 
+    // Allow a peer to sync despite our being stale.
+    ////if (chain_.is_stale())
+    ////    return;
+
     // Respond to get_blocks with inventory.
     SEND2(*message, handle_send, _1, message->command);
 
@@ -248,6 +261,7 @@ bool protocol_block_out::handle_receive_get_data(const code& ec,
     if (stopped(ec))
         return false;
 
+    // TODO: consider rejecting the message for duplicated entries.
     if (message->inventories().size() > max_get_data)
     {
         LOG_WARNING(LOG_NODE)
@@ -257,6 +271,10 @@ bool protocol_block_out::handle_receive_get_data(const code& ec,
         return false;
     }
 
+    // Allow a peer to sync despite our being stale.
+    ////if (chain_.is_stale())
+    ////    return true;
+
     // Create a copy because message is const because it is shared.
     const auto& inventories = message->inventories();
     const auto response = std::make_shared<inventory>();