Merge pull request #544 from evoskuil/master

Differentiate header/block error codes, comments.

include/bitcoin/node/error.hpp

@@ -50,7 +50,9 @@ enum error_t : uint8_t
 
     // blockchain
     orphan_block,
+    orphan_header,
     duplicate_block,
+    duplicate_header,
     insufficient_work
 };
 

src/chasers/chaser_block.cpp

@@ -88,7 +88,6 @@ void chaser_block::organize(const block::cptr& block,
 // protected
 // ----------------------------------------------------------------------------
 
-// Caller may capture block_ptr in handler closure for detailed logging.
 void chaser_block::do_organize(const block::cptr& block_ptr,
     const result_handler& handler) NOEXCEPT
 {

src/chasers/chaser_header.cpp

@@ -102,7 +102,6 @@ void chaser_header::organize(const header::cptr& header,
 // protected
 // ----------------------------------------------------------------------------
 
-// Caller may capture header_ptr in handler closure for detailed logging.
 void chaser_header::do_organize(const header::cptr& header_ptr,
     const result_handler& handler) NOEXCEPT
 {
@@ -126,14 +125,14 @@ void chaser_header::do_organize(const header::cptr& header_ptr,
     // Header already exists.
     if (tree_.contains(hash) || query.is_header(hash))
     {
-        handler(error::duplicate_block);
+        handler(error::duplicate_header);
         return;
     }
 
     // Peer processing should have precluded orphan submission.
     if (!tree_.contains(previous) && !query.is_header(previous))
     {
-        handler(error::orphan_block);
+        handler(error::orphan_header);
         return;
     }
 

src/error.cpp

@@ -41,7 +41,9 @@ DEFINE_ERROR_T_MESSAGE_MAP(error)
 
     // blockchain
     { orphan_block, "orphan block" },
+    { orphan_header, "orphan header" },
     { duplicate_block, "duplicate block" },
+    { duplicate_header, "duplicate header" },
     { insufficient_work, "insufficient work" }
 };
 

src/protocols/protocol_block_in.cpp

@@ -152,6 +152,9 @@ bool protocol_block_in::handle_receive_block(const code& ec,
     const auto height = add1(top_.height());
 
     // Asynchronous organization serves all channels.
+    // A job backlog will occur when organize is slower than download.
+    // This is not a material issue when checkpoints bypass validation.
+    // The backlog may take minutes to clear upon shutdown.
     organize(block_ptr, BIND3(handle_organize, _1, height, block_ptr));
 
     // Set the new top and continue. Organize error will stop the channel.
@@ -217,6 +220,8 @@ void protocol_block_in::handle_organize(const code& ec, size_t height,
 
 get_blocks protocol_block_in::create_get_inventory() const NOEXCEPT
 {
+    // Block-first sync is from the archived (strong) candidate chain.
+    // All strong block branches are archived, so this will reflect latest.
     // This will bypass all blocks with candidate headers, resulting in block
     // orphans if headers-first is run followed by a restart and blocks-first.
     return create_get_inventory(archive().get_candidate_hashes(
@@ -241,6 +246,8 @@ get_blocks protocol_block_in::create_get_inventory(
     return { std::move(hashes) };
 }
 
+// This will prevent most duplicate block requests despite each channel
+// synchronizing its own inventory branch from startup to complete.
 get_data protocol_block_in::create_get_data(
     const inventory& message) const NOEXCEPT
 {

src/protocols/protocol_block_in_31800.cpp

@@ -220,6 +220,8 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec,
     const auto height = add1(top_.height());
 
     // Asynchronous organization serves all channels.
+    // A job backlog will occur when organize is slower than download.
+    // This should not be a material issue given lack of validation here.
     organize(block_ptr, BIND3(handle_organize, _1, height, block_ptr));
 
     // Set the new top and continue. Organize error will stop the channel.

src/protocols/protocol_header_in_31800.cpp

@@ -61,6 +61,7 @@ void protocol_header_in_31800::start() NOEXCEPT
 // Inbound (headers).
 // ----------------------------------------------------------------------------
 
+// Each channel synchronizes its own header branch from startup to complete.
 // Send get_headers and process responses in order until peer is exhausted.
 bool protocol_header_in_31800::handle_receive_headers(const code& ec,
     const headers::cptr& message) NOEXCEPT
@@ -92,6 +93,8 @@ bool protocol_header_in_31800::handle_receive_headers(const code& ec,
         const auto height = add1(top_.height());
 
         // Asynchronous organization serves all channels.
+        // A job backlog will occur when organize is slower than download.
+        // This is not a material issue for headers, even with validation.
         organize(header_ptr, BIND3(handle_organize, _1, height, header_ptr));
 
         // Set the new top and continue. Organize error will stop the channel.
@@ -127,7 +130,7 @@ void protocol_header_in_31800::handle_organize(const code& ec, size_t height,
     if (ec == network::error::service_stopped)
         return;
 
-    if (!ec || ec == error::duplicate_block)
+    if (!ec || ec == error::duplicate_header)
     {
         LOGP("Header [" << encode_hash(header_ptr->hash())
             << "] at (" << height << ") from [" << authority() << "] "

test/error.cpp

@@ -95,6 +95,15 @@ BOOST_AUTO_TEST_CASE(error_t__code__orphan_block__true_exected_message)
     BOOST_REQUIRE_EQUAL(ec.message(), "orphan block");
 }
 
+BOOST_AUTO_TEST_CASE(error_t__code__orphan_header__true_exected_message)
+{
+    constexpr auto value = error::orphan_header;
+    const auto ec = code(value);
+    BOOST_REQUIRE(ec);
+    BOOST_REQUIRE(ec == value);
+    BOOST_REQUIRE_EQUAL(ec.message(), "orphan header");
+}
+
 BOOST_AUTO_TEST_CASE(error_t__code__duplicate_block__true_exected_message)
 {
     constexpr auto value = error::duplicate_block;
@@ -104,6 +113,15 @@ BOOST_AUTO_TEST_CASE(error_t__code__duplicate_block__true_exected_message)
     BOOST_REQUIRE_EQUAL(ec.message(), "duplicate block");
 }
 
+BOOST_AUTO_TEST_CASE(error_t__code__duplicate_header__true_exected_message)
+{
+    constexpr auto value = error::duplicate_header;
+    const auto ec = code(value);
+    BOOST_REQUIRE(ec);
+    BOOST_REQUIRE(ec == value);
+    BOOST_REQUIRE_EQUAL(ec.message(), "duplicate header");
+}
+
 BOOST_AUTO_TEST_CASE(error_t__code__insufficient_work__true_exected_message)
 {
     constexpr auto value = error::insufficient_work;