From 96f2f8c340c80a51c7b021e1a8d56297afa51cd4 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Sat, 24 Feb 2024 15:01:23 -0500 Subject: [PATCH 1/2] Differentiate header/block error codes. --- include/bitcoin/node/error.hpp | 2 ++ src/chasers/chaser_header.cpp | 4 ++-- src/error.cpp | 2 ++ src/protocols/protocol_block_in.cpp | 2 ++ src/protocols/protocol_header_in_31800.cpp | 2 +- test/error.cpp | 18 ++++++++++++++++++ 6 files changed, 27 insertions(+), 3 deletions(-) diff --git a/include/bitcoin/node/error.hpp b/include/bitcoin/node/error.hpp index b3d0ee16..4de70a00 100644 --- a/include/bitcoin/node/error.hpp +++ b/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 }; diff --git a/src/chasers/chaser_header.cpp b/src/chasers/chaser_header.cpp index 00b4aeaa..b194ea75 100644 --- a/src/chasers/chaser_header.cpp +++ b/src/chasers/chaser_header.cpp @@ -126,14 +126,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; } diff --git a/src/error.cpp b/src/error.cpp index 6cc858ed..26b1edad 100644 --- a/src/error.cpp +++ b/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" } }; diff --git a/src/protocols/protocol_block_in.cpp b/src/protocols/protocol_block_in.cpp index f95a375c..b879ceba 100644 --- a/src/protocols/protocol_block_in.cpp +++ b/src/protocols/protocol_block_in.cpp @@ -217,6 +217,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( diff --git a/src/protocols/protocol_header_in_31800.cpp b/src/protocols/protocol_header_in_31800.cpp index 11ad9a41..43260776 100644 --- a/src/protocols/protocol_header_in_31800.cpp +++ b/src/protocols/protocol_header_in_31800.cpp @@ -127,7 +127,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() << "] " diff --git a/test/error.cpp b/test/error.cpp index c152d227..4fd4e65c 100644 --- a/test/error.cpp +++ b/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; From cbb268c2d5e160e1c46bef284177f2614763745e Mon Sep 17 00:00:00 2001 From: evoskuil Date: Sat, 24 Feb 2024 17:21:00 -0500 Subject: [PATCH 2/2] Comments. --- src/chasers/chaser_block.cpp | 1 - src/chasers/chaser_header.cpp | 1 - src/protocols/protocol_block_in.cpp | 5 +++++ src/protocols/protocol_block_in_31800.cpp | 2 ++ src/protocols/protocol_header_in_31800.cpp | 3 +++ 5 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/chasers/chaser_block.cpp b/src/chasers/chaser_block.cpp index 36b1b4d8..a7c216e5 100644 --- a/src/chasers/chaser_block.cpp +++ b/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 { diff --git a/src/chasers/chaser_header.cpp b/src/chasers/chaser_header.cpp index b194ea75..da622a1c 100644 --- a/src/chasers/chaser_header.cpp +++ b/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 { diff --git a/src/protocols/protocol_block_in.cpp b/src/protocols/protocol_block_in.cpp index b879ceba..2b6812c4 100644 --- a/src/protocols/protocol_block_in.cpp +++ b/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. @@ -243,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 { diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 830d305e..a92cf15f 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/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. diff --git a/src/protocols/protocol_header_in_31800.cpp b/src/protocols/protocol_header_in_31800.cpp index 43260776..f525c0a1 100644 --- a/src/protocols/protocol_header_in_31800.cpp +++ b/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.