From 2e3104b9753abfa39fc02929cd7c3795097343a8 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Mon, 18 May 2026 16:21:05 -0400 Subject: [PATCH 1/3] Rename and add error codes. --- include/bitcoin/node/error.hpp | 12 +++++--- src/chasers/chaser_estimate.cpp | 8 ++--- src/error.cpp | 12 +++++--- test/error.cpp | 54 ++++++++++++++++++++++----------- 4 files changed, 56 insertions(+), 30 deletions(-) diff --git a/include/bitcoin/node/error.hpp b/include/bitcoin/node/error.hpp index a4562997..b0315def 100644 --- a/include/bitcoin/node/error.hpp +++ b/include/bitcoin/node/error.hpp @@ -59,6 +59,11 @@ enum error_t : uint8_t duplicate_block, duplicate_header, + /// fee estimation + estimate_disabled, + estimate_premature, + estimate_false, + /// faults (terminal, code error and store corruption assumed) protocol1, protocol2, @@ -98,10 +103,9 @@ enum error_t : uint8_t confirm10, confirm11, confirm12, - estimate_failed, - estimates_failed, - estimates_disabled, - estimates_premature + estimates_initialize, + estimates_push, + estimates_pop }; // No current need for error_code equivalence mapping. diff --git a/src/chasers/chaser_estimate.cpp b/src/chasers/chaser_estimate.cpp index 5573985d..23c89b36 100644 --- a/src/chasers/chaser_estimate.cpp +++ b/src/chasers/chaser_estimate.cpp @@ -66,7 +66,7 @@ void chaser_estimate::estimate(size_t target, estimator::mode mode, { if (!node_settings().fee_estimate_enabled()) { - handler(error::estimates_disabled, {}); + handler(error::estimate_disabled, {}); return; } @@ -82,13 +82,13 @@ void chaser_estimate::do_estimate(size_t target, estimator::mode mode, // Check this under strand so that chase can initialize first. if (!initialized()) { - handler(error::estimates_premature, {}); + handler(error::estimate_premature, {}); return; } const auto value = estimator_->estimate(target, mode); const auto ec = (value < to_unsigned(max_int64) ? error::success : - error::estimate_failed); + error::estimate_false); // Successful value is always castable to int64_t. handler(ec, value); @@ -181,7 +181,7 @@ bool chaser_estimate::initialize() NOEXCEPT estimator_ = std::make_unique(); if (!estimator_->initialize(stopping_, archive(), horizon)) { - fault(error::estimates_failed); + fault(error::estimates_initialize); estimator_.release(); return false; } diff --git a/src/error.cpp b/src/error.cpp index 173935ea..95a9b334 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -49,6 +49,11 @@ DEFINE_ERROR_T_MESSAGE_MAP(error) { duplicate_block, "duplicate block" }, { duplicate_header, "duplicate header" }, + // fee estimation + { estimate_disabled, "estimate_disabled" }, + { estimate_premature, "estimate_premature" }, + { estimate_false, "estimate_false" }, + // faults { protocol1, "protocol1" }, { protocol2, "protocol2" }, @@ -88,10 +93,9 @@ DEFINE_ERROR_T_MESSAGE_MAP(error) { confirm10, "confirm10" }, { confirm11, "confirm11" }, { confirm12, "confirm12" }, - { estimate_failed, "estimate_failed" }, - { estimates_failed, "estimates_failed" }, - { estimates_disabled, "estimates_disabled" }, - { estimates_premature, "estimates_premature" } + { estimates_initialize, "estimates_initialize" }, + { estimates_push, "estimates_push" }, + { estimates_pop, "estimates_pop" } }; DEFINE_ERROR_T_CATEGORY(error, "node", "node code") diff --git a/test/error.cpp b/test/error.cpp index 9b385bbc..6ae3fd4e 100644 --- a/test/error.cpp +++ b/test/error.cpp @@ -166,6 +166,33 @@ BOOST_AUTO_TEST_CASE(error_t__code__duplicate_header__true_expected_message) BOOST_REQUIRE_EQUAL(ec.message(), "duplicate header"); } +BOOST_AUTO_TEST_CASE(error_t__code__estimate_disabled__true_expected_message) +{ + constexpr auto value = error::estimate_disabled; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "estimate_disabled"); +} + +BOOST_AUTO_TEST_CASE(error_t__code__estimate_premature__true_expected_message) +{ + constexpr auto value = error::estimate_premature; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "estimate_premature"); +} + +BOOST_AUTO_TEST_CASE(error_t__code__estimate_false__true_expected_message) +{ + constexpr auto value = error::estimate_false; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "estimate_false"); +} + // faults BOOST_AUTO_TEST_CASE(error_t__code__protocol1__true_expected_message) @@ -225,40 +252,31 @@ BOOST_AUTO_TEST_CASE(error_t__code__confirm1__true_expected_message) BOOST_REQUIRE_EQUAL(ec.message(), "confirm1"); } -BOOST_AUTO_TEST_CASE(error_t__code__estimate_failed__true_expected_message) -{ - constexpr auto value = error::estimate_failed; - const auto ec = code(value); - BOOST_REQUIRE(ec); - BOOST_REQUIRE(ec == value); - BOOST_REQUIRE_EQUAL(ec.message(), "estimate_failed"); -} - -BOOST_AUTO_TEST_CASE(error_t__code__estimates_failed__true_expected_message) +BOOST_AUTO_TEST_CASE(error_t__code__estimates_initialize__true_expected_message) { - constexpr auto value = error::estimates_failed; + constexpr auto value = error::estimates_initialize; const auto ec = code(value); BOOST_REQUIRE(ec); BOOST_REQUIRE(ec == value); - BOOST_REQUIRE_EQUAL(ec.message(), "estimates_failed"); + BOOST_REQUIRE_EQUAL(ec.message(), "estimates_initialize"); } -BOOST_AUTO_TEST_CASE(error_t__code__estimates_disabled__true_expected_message) +BOOST_AUTO_TEST_CASE(error_t__code__estimates_push__true_expected_message) { - constexpr auto value = error::estimates_disabled; + constexpr auto value = error::estimates_push; const auto ec = code(value); BOOST_REQUIRE(ec); BOOST_REQUIRE(ec == value); - BOOST_REQUIRE_EQUAL(ec.message(), "estimates_disabled"); + BOOST_REQUIRE_EQUAL(ec.message(), "estimates_push"); } -BOOST_AUTO_TEST_CASE(error_t__code__estimates_premature__true_expected_message) +BOOST_AUTO_TEST_CASE(error_t__code__estimates_pop__true_expected_message) { - constexpr auto value = error::estimates_premature; + constexpr auto value = error::estimates_pop; const auto ec = code(value); BOOST_REQUIRE(ec); BOOST_REQUIRE(ec == value); - BOOST_REQUIRE_EQUAL(ec.message(), "estimates_premature"); + BOOST_REQUIRE_EQUAL(ec.message(), "estimates_pop"); } BOOST_AUTO_TEST_SUITE_END() From 02e73aa26da1063b3b03ef3697e0ddc2ad3241ca Mon Sep 17 00:00:00 2001 From: evoskuil Date: Mon, 18 May 2026 16:21:17 -0400 Subject: [PATCH 2/3] Make fee chaser safe against currency gaps. --- .../bitcoin/node/chasers/chaser_estimate.hpp | 2 +- src/chasers/chaser_estimate.cpp | 76 +++++++++++-------- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/include/bitcoin/node/chasers/chaser_estimate.hpp b/include/bitcoin/node/chasers/chaser_estimate.hpp index 6fa6af33..a196040f 100644 --- a/include/bitcoin/node/chasers/chaser_estimate.hpp +++ b/include/bitcoin/node/chasers/chaser_estimate.hpp @@ -55,11 +55,11 @@ class BCN_API chaser_estimate virtual bool handle_chase(const code& ec, chase event_, event_value value) NOEXCEPT; + virtual void do_initialize(header_t value) NOEXCEPT; virtual void do_organized(header_t value) NOEXCEPT; virtual void do_reorganized(header_t value) NOEXCEPT; private: - bool initialize() NOEXCEPT; void do_estimate(size_t target, estimator::mode mode, const estimate_handler& handler) NOEXCEPT; diff --git a/src/chasers/chaser_estimate.cpp b/src/chasers/chaser_estimate.cpp index 23c89b36..0617b038 100644 --- a/src/chasers/chaser_estimate.cpp +++ b/src/chasers/chaser_estimate.cpp @@ -120,19 +120,39 @@ bool chaser_estimate::handle_chase(const code&, chase event_, switch (event_) { - // chase::block is only sent when current (may need to file gaps). + // chase::block is only sent when current. This is captured as a cheap + // way to test currency for initialization. Once initialized it is not + // used again. chase::organized is used instead, to ensure that there + // are no push() gaps due to falling out of currency. case chase::block: { - BC_ASSERT(std::holds_alternative(value)); - POST(do_organized, std::get(value)); + if (!initialized()) + { + BC_ASSERT(std::holds_alternative(value)); + POST(do_initialize, std::get(value)); + } + break; } - case chase::reorganized: + case chase::organized: { - BC_ASSERT(std::holds_alternative(value)); - POST(do_reorganized, std::get(value)); + if (initialized()) + { + BC_ASSERT(std::holds_alternative(value)); + POST(do_organized, std::get(value)); + } + break; } + case chase::reorganized: + { + if (initialized()) + { + BC_ASSERT(std::holds_alternative(value)); + POST(do_reorganized, std::get(value)); + break; + } + } case chase::stop: { return false; @@ -146,36 +166,17 @@ bool chaser_estimate::handle_chase(const code&, chase event_, return true; } -void chaser_estimate::do_organized(header_t) NOEXCEPT +void chaser_estimate::do_initialize(header_t) NOEXCEPT { BC_ASSERT(stranded()); - // TODO: make gap safe (get estimator top and adjust). - if (initialized() || initialize()) - estimator_->push(archive()); -} - -void chaser_estimate::do_reorganized(header_t) NOEXCEPT -{ - BC_ASSERT(stranded()); - - // TODO: make gap safe (get estimator top and adjust). if (initialized()) - estimator_->pop(archive()); -} - -// utility -// ---------------------------------------------------------------------------- -// private - -bool chaser_estimate::initialize() NOEXCEPT -{ - BC_ASSERT(stranded()); + return; // Preempt initialize fault when horizon exceeds chain length. const auto horizon = node_settings().fee_estimate_horizon_(); if (horizon > add1(archive().get_top_confirmed())) - return false; + return; // Heap-allocate the estimator due to size. estimator_ = std::make_unique(); @@ -183,11 +184,26 @@ bool chaser_estimate::initialize() NOEXCEPT { fault(error::estimates_initialize); estimator_.release(); - return false; + return; } initialized_.store(true, std::memory_order_relaxed); - return true; +} + +void chaser_estimate::do_organized(header_t) NOEXCEPT +{ + BC_ASSERT(stranded()); + + if (initialized() && !estimator_->push(archive())) + fault(error::estimates_push); +} + +void chaser_estimate::do_reorganized(header_t) NOEXCEPT +{ + BC_ASSERT(stranded()); + + if (initialized() && !estimator_->pop(archive())) + fault(error::estimates_pop); } BC_POP_WARNING() From 5ff5549e57bcd40bbea53e9caaec17220b08f427 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Mon, 18 May 2026 17:08:08 -0400 Subject: [PATCH 3/3] Guard against initialization race in fee estiamtor. --- .../bitcoin/node/chasers/chaser_estimate.hpp | 6 +-- include/bitcoin/node/error.hpp | 6 ++- src/chasers/chaser_estimate.cpp | 44 ++++++++++++++++--- src/error.cpp | 6 ++- test/error.cpp | 30 ++++++++++--- 5 files changed, 73 insertions(+), 19 deletions(-) diff --git a/include/bitcoin/node/chasers/chaser_estimate.hpp b/include/bitcoin/node/chasers/chaser_estimate.hpp index a196040f..34c9d6ec 100644 --- a/include/bitcoin/node/chasers/chaser_estimate.hpp +++ b/include/bitcoin/node/chasers/chaser_estimate.hpp @@ -55,9 +55,9 @@ class BCN_API chaser_estimate virtual bool handle_chase(const code& ec, chase event_, event_value value) NOEXCEPT; - virtual void do_initialize(header_t value) NOEXCEPT; - virtual void do_organized(header_t value) NOEXCEPT; - virtual void do_reorganized(header_t value) NOEXCEPT; + virtual void do_initialize(header_t link) NOEXCEPT; + virtual void do_organized(header_t link) NOEXCEPT; + virtual void do_reorganized(header_t link) NOEXCEPT; private: void do_estimate(size_t target, estimator::mode mode, diff --git a/include/bitcoin/node/error.hpp b/include/bitcoin/node/error.hpp index b0315def..065e6c94 100644 --- a/include/bitcoin/node/error.hpp +++ b/include/bitcoin/node/error.hpp @@ -104,8 +104,10 @@ enum error_t : uint8_t confirm11, confirm12, estimates_initialize, - estimates_push, - estimates_pop + estimates_push1, + estimates_push2, + estimates_pop1, + estimates_pop2 }; // No current need for error_code equivalence mapping. diff --git a/src/chasers/chaser_estimate.cpp b/src/chasers/chaser_estimate.cpp index 0617b038..7cf975f4 100644 --- a/src/chasers/chaser_estimate.cpp +++ b/src/chasers/chaser_estimate.cpp @@ -190,20 +190,52 @@ void chaser_estimate::do_initialize(header_t) NOEXCEPT initialized_.store(true, std::memory_order_relaxed); } -void chaser_estimate::do_organized(header_t) NOEXCEPT +void chaser_estimate::do_organized(header_t link) NOEXCEPT { BC_ASSERT(stranded()); - if (initialized() && !estimator_->push(archive())) - fault(error::estimates_push); + if (initialized()) + { + const auto& query = archive(); + const auto height = query.get_height(link); + + if (height.is_terminal()) + { + fault(error::estimates_push1); + return; + } + + // Organization events backlog during initialization. + if (height.value <= estimator_->top_height()) + return; + + if (!estimator_->push(query)) + fault(error::estimates_push2); + } } -void chaser_estimate::do_reorganized(header_t) NOEXCEPT +void chaser_estimate::do_reorganized(header_t link) NOEXCEPT { BC_ASSERT(stranded()); - if (initialized() && !estimator_->pop(archive())) - fault(error::estimates_pop); + if (initialized()) + { + const auto& query = archive(); + const auto height = query.get_height(link); + + if (height.is_terminal()) + { + fault(error::estimates_pop1); + return; + } + + // Organization events backlog during initialization. + if (height.value > estimator_->top_height()) + return; + + if (!estimator_->push(query)) + fault(error::estimates_pop2); + } } BC_POP_WARNING() diff --git a/src/error.cpp b/src/error.cpp index 95a9b334..894f823d 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -94,8 +94,10 @@ DEFINE_ERROR_T_MESSAGE_MAP(error) { confirm11, "confirm11" }, { confirm12, "confirm12" }, { estimates_initialize, "estimates_initialize" }, - { estimates_push, "estimates_push" }, - { estimates_pop, "estimates_pop" } + { estimates_push1, "estimates_push1" }, + { estimates_push2, "estimates_push2" }, + { estimates_pop1, "estimates_pop1" }, + { estimates_pop2, "estimates_pop2" } }; DEFINE_ERROR_T_CATEGORY(error, "node", "node code") diff --git a/test/error.cpp b/test/error.cpp index 6ae3fd4e..52539775 100644 --- a/test/error.cpp +++ b/test/error.cpp @@ -261,22 +261,40 @@ BOOST_AUTO_TEST_CASE(error_t__code__estimates_initialize__true_expected_message) BOOST_REQUIRE_EQUAL(ec.message(), "estimates_initialize"); } -BOOST_AUTO_TEST_CASE(error_t__code__estimates_push__true_expected_message) +BOOST_AUTO_TEST_CASE(error_t__code__estimates_push1__true_expected_message) { - constexpr auto value = error::estimates_push; + constexpr auto value = error::estimates_push1; const auto ec = code(value); BOOST_REQUIRE(ec); BOOST_REQUIRE(ec == value); - BOOST_REQUIRE_EQUAL(ec.message(), "estimates_push"); + BOOST_REQUIRE_EQUAL(ec.message(), "estimates_push1"); } -BOOST_AUTO_TEST_CASE(error_t__code__estimates_pop__true_expected_message) +BOOST_AUTO_TEST_CASE(error_t__code__estimates_push2__true_expected_message) { - constexpr auto value = error::estimates_pop; + constexpr auto value = error::estimates_push2; const auto ec = code(value); BOOST_REQUIRE(ec); BOOST_REQUIRE(ec == value); - BOOST_REQUIRE_EQUAL(ec.message(), "estimates_pop"); + BOOST_REQUIRE_EQUAL(ec.message(), "estimates_push2"); +} + +BOOST_AUTO_TEST_CASE(error_t__code__estimates_pop1__true_expected_message) +{ + constexpr auto value = error::estimates_pop1; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "estimates_pop1"); +} + +BOOST_AUTO_TEST_CASE(error_t__code__estimates_pop2__true_expected_message) +{ + constexpr auto value = error::estimates_pop2; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "estimates_pop2"); } BOOST_AUTO_TEST_SUITE_END()