From 7023a2ed1b1f9f798fd092e50b4d1033ecf966d5 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 7 Jun 2024 12:57:31 -0400 Subject: [PATCH 1/6] Bypass block.check under checkpoint but not milestone. --- .../protocols/protocol_block_in_31800.hpp | 15 +++++----- src/protocols/protocol_block_in_31800.cpp | 30 ++++++++----------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/include/bitcoin/node/protocols/protocol_block_in_31800.hpp b/include/bitcoin/node/protocols/protocol_block_in_31800.hpp index 94e5c076..886e7823 100644 --- a/include/bitcoin/node/protocols/protocol_block_in_31800.hpp +++ b/include/bitcoin/node/protocols/protocol_block_in_31800.hpp @@ -39,6 +39,8 @@ class BCN_API protocol_block_in_31800 protocol_block_in_31800(const SessionPtr& session, const channel_ptr& channel) NOEXCEPT : protocol_performer(session, channel), + top_checkpoint_height_( + session->config().bitcoin.top_checkpoint().height()), block_type_(session->config().network.witness_node() ? type_id::witness_block : type_id::block), map_(chaser_check::empty_map()) @@ -77,27 +79,24 @@ class BCN_API protocol_block_in_31800 code check(const system::chain::block& block, const system::chain::context& ctx, bool bypass) const NOEXCEPT; - void send_get_data(const map_ptr& map, const job::ptr& job, - size_t bypass) NOEXCEPT; + void send_get_data(const map_ptr& map, const job::ptr& job) NOEXCEPT; network::messages::get_data create_get_data( const map_ptr& map) const NOEXCEPT; void restore(const map_ptr& map) NOEXCEPT; void do_handle_complete(const code& ec) NOEXCEPT; + bool is_under_checkpoint(size_t height) const NOEXCEPT; void handle_put_hashes(const code& ec, size_t count) NOEXCEPT; void handle_get_hashes(const code& ec, const map_ptr& map, - const job::ptr& job, size_t bypass) NOEXCEPT; + const job::ptr& job) NOEXCEPT; - void set_bypass(height_t height) NOEXCEPT; - bool is_bypassed(size_t height) const NOEXCEPT; - - // This is thread safe. + // These are thread safe. + const size_t top_checkpoint_height_; const network::messages::inventory::type_id block_type_; // These are protected by strand. map_ptr map_; job::ptr job_{}; - size_t bypass_{}; }; } // namespace node diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 7b30ed1b..12025208 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/src/protocols/protocol_block_in_31800.cpp @@ -80,7 +80,7 @@ void protocol_block_in_31800::do_handle_complete(const code& ec) NOEXCEPT if (is_current()) { start_performance(); - get_hashes(BIND(handle_get_hashes, _1, _2, _3, _4)); + get_hashes(BIND(handle_get_hashes, _1, _2, _3)); } } @@ -179,7 +179,7 @@ void protocol_block_in_31800::do_get_downloads(count_t) NOEXCEPT { // Assume performance was stopped due to exhaustion. start_performance(); - get_hashes(BIND(handle_get_hashes, _1, _2, _3, _4)); + get_hashes(BIND(handle_get_hashes, _1, _2, _3)); } } @@ -222,7 +222,7 @@ void protocol_block_in_31800::do_report(count_t sequence) NOEXCEPT // ---------------------------------------------------------------------------- void protocol_block_in_31800::send_get_data(const map_ptr& map, - const job::ptr& job, size_t bypass) NOEXCEPT + const job::ptr& job) NOEXCEPT { BC_ASSERT(stranded()); @@ -232,7 +232,6 @@ void protocol_block_in_31800::send_get_data(const map_ptr& map, return; } - set_bypass(bypass); if (map->empty()) return; @@ -311,7 +310,9 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, // Transaction/witness commitments are required under checkpoint. // This ensures that the block/header hash represents expected txs. - const auto bypass = is_bypassed(ctx.height) && !malleable64; + // Do not consider milestone for check because regressions can result in + // stored unchecked blocks, and the cost of mitigation exceeds the check. + const auto bypass = is_under_checkpoint(ctx.height) && !malleable64; // Performs full check if block is mally64 (mally32 caught either way). if (const auto code = check(*block_ptr, ctx, bypass)) @@ -389,7 +390,7 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, if (is_idle()) { job_.reset(); - get_hashes(BIND(handle_get_hashes, _1, _2, _3, _4)); + get_hashes(BIND(handle_get_hashes, _1, _2, _3)); } return true; @@ -432,7 +433,7 @@ void protocol_block_in_31800::handle_put_hashes(const code& ec, } void protocol_block_in_31800::handle_get_hashes(const code& ec, - const map_ptr& map, const job::ptr& job, size_t bypass) NOEXCEPT + const map_ptr& map, const job::ptr& job) NOEXCEPT { LOGV("Got (" << map->size() << ") work for [" << authority() << "]."); @@ -455,22 +456,15 @@ void protocol_block_in_31800::handle_get_hashes(const code& ec, return; } - POST(send_get_data, map, job, bypass); + POST(send_get_data, map, job); } -// bypass +// checkpoint // ---------------------------------------------------------------------------- -void protocol_block_in_31800::set_bypass(height_t height) NOEXCEPT +bool protocol_block_in_31800::is_under_checkpoint(size_t height) const NOEXCEPT { - BC_ASSERT(stranded()); - bypass_ = height; -} - -bool protocol_block_in_31800::is_bypassed(size_t height) const NOEXCEPT -{ - BC_ASSERT(stranded()); - return height <= bypass_; + return height <= top_checkpoint_height_; } BC_POP_WARNING() From 4dc282dae2e9826f0f17d038fc9364a41ff3db3b Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 7 Jun 2024 20:06:41 -0400 Subject: [PATCH 2/6] Set events for block archive only at thresholds. --- include/bitcoin/node/protocols/protocol_block_in_31800.hpp | 2 ++ src/protocols/protocol_block_in_31800.cpp | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/include/bitcoin/node/protocols/protocol_block_in_31800.hpp b/include/bitcoin/node/protocols/protocol_block_in_31800.hpp index 886e7823..6480c23e 100644 --- a/include/bitcoin/node/protocols/protocol_block_in_31800.hpp +++ b/include/bitcoin/node/protocols/protocol_block_in_31800.hpp @@ -39,6 +39,7 @@ class BCN_API protocol_block_in_31800 protocol_block_in_31800(const SessionPtr& session, const channel_ptr& channel) NOEXCEPT : protocol_performer(session, channel), + maximum_concurrency_(session->config().node.maximum_concurrency_()), top_checkpoint_height_( session->config().bitcoin.top_checkpoint().height()), block_type_(session->config().network.witness_node() ? @@ -91,6 +92,7 @@ class BCN_API protocol_block_in_31800 const job::ptr& job) NOEXCEPT; // These are thread safe. + const size_t maximum_concurrency_; const size_t top_checkpoint_height_; const network::messages::inventory::type_id block_type_; diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 12025208..4d64407c 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/src/protocols/protocol_block_in_31800.cpp @@ -383,7 +383,10 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, << "] from [" << authority() << "]."); notify(error::success, chase::checked, ctx.height); - fire(events::block_archived, ctx.height); + + // TODO: remove modulo restriction. + if (is_zero(ctx.height % maximum_concurrency_)) + fire(events::block_archived, ctx.height); count(message->cached_size); map_->erase(it); From e1b105a028b48d700ac6f50eaa1d5710a89792f2 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 7 Jun 2024 20:06:50 -0400 Subject: [PATCH 3/6] Don't invalidate blocks for lack of commitment. --- src/protocols/protocol_block_in_31800.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 4d64407c..5df9843c 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/src/protocols/protocol_block_in_31800.cpp @@ -317,6 +317,17 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, // Performs full check if block is mally64 (mally32 caught either way). if (const auto code = check(*block_ptr, ctx, bypass)) { + // Uncommitted blocks have no creation cost, just bogus data. + if (code == system::error::invalid_transaction_commitment || + code == system::error::invalid_witness_commitment) + { + LOGR("Uncommitted block [" << encode_hash(hash) << ":" + << ctx.height << "] from [" << authority() << "] " + << code.message()); + stop(code); + return false; + } + // Malleated32 is never associated, so drop peer and continue. // Cannot mark unconfirmable as confirmable with same hash may exist. // Do not rely on return code because does not catch non-bypass mally. From 096b4ba5ec4df31b5504d888ab30368a612577e5 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Sat, 8 Jun 2024 14:39:02 -0400 Subject: [PATCH 4/6] Set archive event for block 1. --- src/protocols/protocol_block_in_31800.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 5df9843c..5645b778 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/src/protocols/protocol_block_in_31800.cpp @@ -395,8 +395,8 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, notify(error::success, chase::checked, ctx.height); - // TODO: remove modulo restriction. - if (is_zero(ctx.height % maximum_concurrency_)) + // TODO: remove event restriction. + if (is_one(ctx.height) || (is_zero(ctx.height % maximum_concurrency_))) fire(events::block_archived, ctx.height); count(message->cached_size); From d4e9a096667a038e4b1737c6a47d17c5bd2b330f Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 7 Jun 2024 15:28:05 -0400 Subject: [PATCH 5/6] Use only checkpoint bypass, skip validate but apply confirmation. --- include/bitcoin/node/chasers/chaser.hpp | 11 ++- .../bitcoin/node/chasers/chaser_confirm.hpp | 1 + .../bitcoin/node/chasers/chaser_organize.hpp | 4 - .../node/impl/chasers/chaser_organize.ipp | 19 ++--- src/chasers/chaser.cpp | 13 +++- src/chasers/chaser_confirm.cpp | 61 ++++++++++++--- src/chasers/chaser_validate.cpp | 74 ++++++++++--------- src/protocols/protocol_block_in_31800.cpp | 4 +- 8 files changed, 119 insertions(+), 68 deletions(-) diff --git a/include/bitcoin/node/chasers/chaser.hpp b/include/bitcoin/node/chasers/chaser.hpp index 3b9ce538..bd873c53 100644 --- a/include/bitcoin/node/chasers/chaser.hpp +++ b/include/bitcoin/node/chasers/chaser.hpp @@ -129,8 +129,10 @@ class BCN_API chaser /// ----------------------------------------------------------------------- size_t bypass() const NOEXCEPT; + size_t checkpoint() const NOEXCEPT; void set_bypass(size_t height) NOEXCEPT; bool is_bypassed(size_t height) const NOEXCEPT; + bool is_under_checkpoint(size_t height) const NOEXCEPT; /// Position (requires strand). /// ----------------------------------------------------------------------- @@ -139,13 +141,14 @@ class BCN_API chaser void set_position(size_t height) NOEXCEPT; private: - // These are protected by strand. - size_t bypass_{}; - size_t position_{}; - // These are thread safe (mostly). full_node& node_; network::asio::strand strand_; + const size_t top_checkpoint_height_; + + // These are protected by strand. + size_t bypass_{}; + size_t position_{}; }; #define SUBSCRIBE_EVENTS(method, ...) \ diff --git a/include/bitcoin/node/chasers/chaser_confirm.hpp b/include/bitcoin/node/chasers/chaser_confirm.hpp index 19d511ec..3ddabe57 100644 --- a/include/bitcoin/node/chasers/chaser_confirm.hpp +++ b/include/bitcoin/node/chasers/chaser_confirm.hpp @@ -49,6 +49,7 @@ class BCN_API chaser_confirm private: bool set_organized(header_t link, height_t height) NOEXCEPT; + bool reset_organized(header_t link, height_t height) NOEXCEPT; bool set_reorganized(header_t link, height_t height) NOEXCEPT; bool roll_back(const header_links& popped, size_t fork_point, size_t top) NOEXCEPT; diff --git a/include/bitcoin/node/chasers/chaser_organize.hpp b/include/bitcoin/node/chasers/chaser_organize.hpp index 7987264b..91748ee0 100644 --- a/include/bitcoin/node/chasers/chaser_organize.hpp +++ b/include/bitcoin/node/chasers/chaser_organize.hpp @@ -111,9 +111,6 @@ class chaser_organize /// Height represents a candidate block covered by active milestone. virtual inline bool is_under_milestone(size_t height) const NOEXCEPT; - /// Height represents a candidate block covered by checkpoint. - virtual inline bool is_under_checkpoint(size_t height) const NOEXCEPT; - private: static constexpr auto flag_bits = to_bits(sizeof(system::chain::flags)); static constexpr bool is_block() NOEXCEPT @@ -185,7 +182,6 @@ class chaser_organize const system::settings& settings_; const system::chain::checkpoint& milestone_; const system::chain::checkpoints checkpoints_; - const size_t top_checkpoint_height_; // These are protected by strand. size_t active_milestone_height_{}; diff --git a/include/bitcoin/node/impl/chasers/chaser_organize.ipp b/include/bitcoin/node/impl/chasers/chaser_organize.ipp index 4eaf0f4e..346632d4 100644 --- a/include/bitcoin/node/impl/chasers/chaser_organize.ipp +++ b/include/bitcoin/node/impl/chasers/chaser_organize.ipp @@ -38,8 +38,7 @@ CLASS::chaser_organize(full_node& node) NOEXCEPT : chaser(node), settings_(config().bitcoin), milestone_(config().bitcoin.milestone), - checkpoints_(config().bitcoin.sorted_checkpoints()), - top_checkpoint_height_(config().bitcoin.top_checkpoint().height()) + checkpoints_(config().bitcoin.sorted_checkpoints()) { } @@ -196,7 +195,7 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, return; } - // With a candidate reorg that drop strong below a valid header chain, + // With a candidate reorg that drops strong below a valid header chain, // this will cause a sequence of headers to be bypassed, such that a // parent of a block that doesn't exist will not be a candidate, which // result in a failure of get_chain_state below, because it depends on @@ -304,6 +303,11 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, // BUGBUG: the new branch can become ordered and downloaded under the old // BUGBUG: milestone while the new is pending in the notification queue. // BUGBUG: probably need to provide both fork point and old top. + // BUGBUG: because validation and confirmation are strictly ordered, this + // BUGBUG: only affects check, and so block check is only bypassed under + // BUGBUG: checkpoint. However confirmation writes proceed under milestone. + // BUGBUG: These differ in that their state is cheaply detected, whereas + // BUGBUG: lack of block check would require read/check of each full block. // branch_point reset_milestone(++index); @@ -657,13 +661,6 @@ bool CLASS::push(const system::hash_digest& key) NOEXCEPT // Bypass methods. // ---------------------------------------------------------------------------- -// protected -TEMPLATE -inline bool CLASS::is_under_checkpoint(size_t height) const NOEXCEPT -{ - return height <= top_checkpoint_height_; -} - // protected TEMPLATE inline bool CLASS::is_under_milestone(size_t height) const NOEXCEPT @@ -735,7 +732,7 @@ TEMPLATE void CLASS::notify_bypass() const NOEXCEPT { notify(error::success, chase::bypass, - std::max(active_milestone_height_, top_checkpoint_height_)); + std::max(active_milestone_height_, checkpoint())); } // Logging. diff --git a/src/chasers/chaser.cpp b/src/chasers/chaser.cpp index 1b8674dc..e095596c 100644 --- a/src/chasers/chaser.cpp +++ b/src/chasers/chaser.cpp @@ -34,6 +34,7 @@ BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT) chaser::chaser(full_node& node) NOEXCEPT : node_(node), strand_(node.service().get_executor()), + top_checkpoint_height_(node.config().bitcoin.top_checkpoint().height()), reporter(node.log) { } @@ -147,7 +148,17 @@ void chaser::set_bypass(height_t height) NOEXCEPT bool chaser::is_bypassed(size_t height) const NOEXCEPT { BC_ASSERT(stranded()); - return height <= bypass_; + return height <= bypass(); +} + +bool chaser::is_under_checkpoint(size_t height) const NOEXCEPT +{ + return height <= checkpoint(); +} + +size_t chaser::checkpoint() const NOEXCEPT +{ + return top_checkpoint_height_; } // Position. diff --git a/src/chasers/chaser_confirm.cpp b/src/chasers/chaser_confirm.cpp index a281d66d..dde82430 100644 --- a/src/chasers/chaser_confirm.cpp +++ b/src/chasers/chaser_confirm.cpp @@ -175,7 +175,6 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT // Push candidate headers to confirmed chain. for (const auto& link: views_reverse(fork)) { - // TODO: skip under bypass and not malleable? auto ec = query.get_block_state(link); if (ec == database::error::integrity) { @@ -183,26 +182,43 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT return; } - // TODO: rollback required. if (ec == database::error::block_unconfirmable) { notify(ec, chase::unconfirmable, link); fire(events::block_unconfirmable, index); + + // chase::reorganized & events::block_reorganized + // chase::organized & events::block_organized + if (!roll_back(popped, fork_point, index)) + fault(error::node_roll_back); + return; } - const auto malleable64 = query.is_malleable64(link); - - // TODO: set organized. // error::confirmation_bypass is not used. if (ec == database::error::block_confirmable || - (is_bypassed(index) && !malleable64)) + (is_under_checkpoint(index) && !query.is_malleable64(link))) { notify(ec, chase::confirmable, index); fire(events::confirm_bypassed, index); + + // chase::organized & events::block_organized + if (!query.set_strong(link) || !set_organized(link, index)) + { + fault(error::set_confirmed); + return; + } + continue; } + // Required for block_confirmable. + if (!query.set_strong(link)) + { + fault(error::node_confirm); + return; + } + ec = query.block_confirmable(link); if (ec == database::error::integrity) { @@ -212,7 +228,13 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT if (ec) { - // TODO: rollback required. + // Roll back current block. + if (!query.set_unstrong(link)) + { + fault(error::node_confirm); + return; + } + // Transactions are set strong upon archive when under bypass. Only // malleable blocks are validated under bypass, and not set strong. if (is_bypassed(height)) @@ -220,6 +242,13 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT LOGR("Malleated64 block [" << index << "] " << ec.message()); notify(ec, chase::malleated, link); fire(events::block_malleated, index); + + // chase::reorganized & events::block_reorganized + // chase::organized & events::block_organized + // index has not been confirmed, so start prior. + if (!roll_back(popped, fork_point, sub1(index))) + fault(error::node_roll_back); + return; } @@ -236,10 +265,7 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT // chase::reorganized & events::block_reorganized // chase::organized & events::block_organized if (!roll_back(popped, fork_point, index)) - { fault(error::node_roll_back); - return; - } return; } @@ -281,10 +307,21 @@ bool chaser_confirm::set_organized(header_t link, height_t height) NOEXCEPT return true; } +bool chaser_confirm::reset_organized(header_t link, height_t height) NOEXCEPT +{ + auto& query = archive(); + if (!query.set_strong(link) || !query.push_confirmed(link)) + return false; + + notify(error::success, chase::organized, link); + fire(events::block_organized, height); + return true; +} + bool chaser_confirm::set_reorganized(header_t link, height_t height) NOEXCEPT { auto& query = archive(); - if (!query.set_unstrong(link) || !query.pop_confirmed()) + if (!query.pop_confirmed() || !query.set_unstrong(link)) return false; notify(error::success, chase::reorganized, link); @@ -301,7 +338,7 @@ bool chaser_confirm::roll_back(const header_links& popped, return false; for (const auto& link: views_reverse(popped)) - if (!query.set_strong(link) || !set_organized(link, ++fork_point)) + if (!reset_organized(link, ++fork_point)) return false; return true; diff --git a/src/chasers/chaser_validate.cpp b/src/chasers/chaser_validate.cpp index b481752e..dca49cd4 100644 --- a/src/chasers/chaser_validate.cpp +++ b/src/chasers/chaser_validate.cpp @@ -72,27 +72,27 @@ bool chaser_validate::handle_event(const code&, chase event_, { // Track downloaded. // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ - ////case chase::start: - ////case chase::bump: - ////{ - //// POST(do_bump, height_t{}); - //// break; - ////} - ////case chase::checked: - ////{ - //// POST(do_checked, possible_narrow_cast(value)); - //// break; - ////} - ////case chase::regressed: - ////{ - //// POST(do_regressed, possible_narrow_cast(value)); - //// break; - ////} - ////case chase::disorganized: - ////{ - //// POST(do_regressed, possible_narrow_cast(value)); - //// break; - ////} + case chase::start: + case chase::bump: + { + POST(do_bump, height_t{}); + break; + } + case chase::checked: + { + POST(do_checked, possible_narrow_cast(value)); + break; + } + case chase::regressed: + { + POST(do_regressed, possible_narrow_cast(value)); + break; + } + case chase::disorganized: + { + POST(do_regressed, possible_narrow_cast(value)); + break; + } // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ case chase::bypass: { @@ -151,48 +151,52 @@ void chaser_validate::do_bump(height_t) NOEXCEPT // Validation is always sequential from position, along the candidate // index. It does not care about regressions that may be in process. const auto link = query.to_candidate(height); + const auto ec = query.get_block_state(link); - if (ec == database::error::unassociated) + if (ec == database::error::integrity) { - // Wait until the gap is filled. + fault(error::node_validate); return; } - if (ec == database::error::integrity) + if (ec == database::error::unassociated) { - fault(error::node_validate); + // Wait until the gap is filled. return; } if (ec == database::error::block_unconfirmable) { LOGR("Unconfirmable block [" << height << "] " << ec.message()); - notify(ec, chase::unvalid, link); fire(events::block_unconfirmable, height); + notify(ec, chase::unvalid, link); return; } // error::validation_bypass is not used because fan-out. if (ec == database::error::block_valid || ec == database::error::block_confirmable || - (is_bypassed(height) && !query.is_malleable64(link))) + (is_under_checkpoint(height) && !query.is_malleable64(link))) { update_position(height); - notify(ec, chase::valid, height); fire(events::validate_bypassed, height); + notify(ec, chase::valid, height); continue; } - // TODO: the quantity of work must be throttled. - // This will very rapidly pump all outstanding work into asio queue. - if (!enqueue_block(link)) - { - fault(error::node_validate); - return; - } + // TODO: validation. + ////// TODO: the quantity of work must be throttled. + ////// This will very rapidly pump all outstanding work into asio queue. + ////if (!enqueue_block(link)) + ////{ + //// fault(error::node_validate); + //// return; + ////} // Retain last height in validation sequence, update neutrino. update_position(height); + fire(events::block_validated, height); + notify(ec, chase::valid, height); } } diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 5645b778..7758d208 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/src/protocols/protocol_block_in_31800.cpp @@ -312,6 +312,8 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, // This ensures that the block/header hash represents expected txs. // Do not consider milestone for check because regressions can result in // stored unchecked blocks, and the cost of mitigation exceeds the check. + // Do not consider milestone for set_strong because regressions can result + // in wrong strong or unstrong and cost of mitigation exceeds the benefit. const auto bypass = is_under_checkpoint(ctx.height) && !malleable64; // Performs full check if block is mally64 (mally32 caught either way). @@ -377,7 +379,7 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, const chain::transactions_cptr txs_ptr{ block_ptr->transactions_ptr() }; // Transactions are set_strong here when bypass is true. - if (const auto code = query.set_code(*txs_ptr, link, size, bypass)) + if (const auto code = query.set_code(*txs_ptr, link, size, false)) { LOGF("Failure storing block [" << encode_hash(hash) << ":" << ctx.height << "] from [" << authority() << "] " From 4cae6f46d4d0f98962f9726992203f22444c0b14 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Mon, 10 Jun 2024 00:40:42 -0400 Subject: [PATCH 6/6] Refactor bypass using new store.header.bypass field. --- include/bitcoin/node/chase.hpp | 5 - include/bitcoin/node/chasers/chaser.hpp | 9 +- include/bitcoin/node/chasers/chaser_block.hpp | 24 +- .../bitcoin/node/chasers/chaser_header.hpp | 42 +++- .../bitcoin/node/chasers/chaser_organize.hpp | 72 +++--- .../bitcoin/node/chasers/chaser_validate.hpp | 4 - include/bitcoin/node/define.hpp | 4 +- .../node/impl/chasers/chaser_organize.ipp | 234 ++++-------------- src/chasers/chaser.cpp | 34 +-- src/chasers/chaser_block.cpp | 74 +++--- src/chasers/chaser_check.cpp | 17 +- src/chasers/chaser_confirm.cpp | 65 +++-- src/chasers/chaser_header.cpp | 150 ++++++++++- src/chasers/chaser_validate.cpp | 42 ++-- src/protocols/protocol_block_in_31800.cpp | 40 +-- 15 files changed, 413 insertions(+), 403 deletions(-) diff --git a/include/bitcoin/node/chase.hpp b/include/bitcoin/node/chase.hpp index 7a10f957..784ec228 100644 --- a/include/bitcoin/node/chase.hpp +++ b/include/bitcoin/node/chase.hpp @@ -88,11 +88,6 @@ enum class chase /// Issued by 'organize' and handled by 'check'. regressed, - /// Bypass height has changed for all subsequent notifications (height_t). - /// Issued by 'organize' and handled by 'validate', 'confirm', and - /// 'block_in_31800'. - bypass, - /// Late-stage Invalidity. /// ----------------------------------------------------------------------- diff --git a/include/bitcoin/node/chasers/chaser.hpp b/include/bitcoin/node/chasers/chaser.hpp index bd873c53..1b40133e 100644 --- a/include/bitcoin/node/chasers/chaser.hpp +++ b/include/bitcoin/node/chasers/chaser.hpp @@ -125,13 +125,7 @@ class BCN_API chaser /// Header timestamp is within configured span from current time. bool is_current(uint32_t timestamp) const NOEXCEPT; - /// Bypass (requires strand). - /// ----------------------------------------------------------------------- - - size_t bypass() const NOEXCEPT; - size_t checkpoint() const NOEXCEPT; - void set_bypass(size_t height) NOEXCEPT; - bool is_bypassed(size_t height) const NOEXCEPT; + /// The height is at or below the top checkpoint. bool is_under_checkpoint(size_t height) const NOEXCEPT; /// Position (requires strand). @@ -147,7 +141,6 @@ class BCN_API chaser const size_t top_checkpoint_height_; // These are protected by strand. - size_t bypass_{}; size_t position_{}; }; diff --git a/include/bitcoin/node/chasers/chaser_block.hpp b/include/bitcoin/node/chasers/chaser_block.hpp index a9de47ae..6ab1688d 100644 --- a/include/bitcoin/node/chasers/chaser_block.hpp +++ b/include/bitcoin/node/chasers/chaser_block.hpp @@ -44,21 +44,27 @@ class BCN_API chaser_block virtual const system::chain::header& get_header( const system::chain::block& block) const NOEXCEPT; - /// Query store for const pointer to Block instance. + /// Query store for const pointer to Block instance by candidate height. virtual bool get_block(system::chain::block::cptr& out, - size_t index) const NOEXCEPT; + size_t height) const NOEXCEPT; + + /// True if Block should bypass validation, given its candidate height. + virtual bool get_bypass(const system::chain::block& block, + size_t height) const NOEXCEPT; /// Determine if Block is valid. virtual code validate(const system::chain::block& block, - const system::chain::chain_state& state) const NOEXCEPT; + const chain_state& state) const NOEXCEPT; + + /// Handle malleted message (nop). + virtual void do_malleated(header_t link) NOEXCEPT; - /// Determine if Block is top of a storable branch. - virtual bool is_storable(const system::chain::block& block, - const system::chain::chain_state& state) const NOEXCEPT; + /// Determine if state is top of a storable branch (always true). + virtual bool is_storable(const chain_state& state) const NOEXCEPT; - // Store Block to database and push to top of candidate chain. - virtual database::header_link push(const system::chain::block& block, - const system::chain::context& context) const NOEXCEPT; + /// Milestone tracking. + virtual void update_milestone(const system::chain::header& header, + size_t height, size_t branch_point) NOEXCEPT; private: void set_prevout(const system::chain::input& input) const NOEXCEPT; diff --git a/include/bitcoin/node/chasers/chaser_header.hpp b/include/bitcoin/node/chasers/chaser_header.hpp index b4197bb5..2255fdc4 100644 --- a/include/bitcoin/node/chasers/chaser_header.hpp +++ b/include/bitcoin/node/chasers/chaser_header.hpp @@ -39,22 +39,52 @@ class BCN_API chaser_header chaser_header(full_node& node) NOEXCEPT; + /// Initialize chaser state. + code start() NOEXCEPT override; + protected: /// Get header from Block instance. virtual const system::chain::header& get_header( const system::chain::header& header) const NOEXCEPT; - /// Query store for const pointer to Block instance. + /// Query store for const pointer to Block instance by candidate height. virtual bool get_block(system::chain::header::cptr& out, - size_t index) const NOEXCEPT; + size_t height) const NOEXCEPT; + + /// True if Block should bypass validation, given its candidate height. + virtual bool get_bypass(const system::chain::header& header, + size_t height) const NOEXCEPT; /// Determine if Block is valid. virtual code validate(const system::chain::header& header, - const system::chain::chain_state& state) const NOEXCEPT; + const chain_state& state) const NOEXCEPT; + + /// Disassociate malleated block and notify to redownload header. + virtual void do_malleated(header_t link) NOEXCEPT; + + /// Determine if state is top of a storable branch. + virtual bool is_storable(const chain_state& state) const NOEXCEPT; + + /// Milestone tracking. + virtual void update_milestone(const system::chain::header& header, + size_t height, size_t branch_point) NOEXCEPT; + + /// Milestone methods. + bool initialize_milestone() NOEXCEPT; + bool is_under_milestone(size_t height) const NOEXCEPT; + +private: + // Storable methods. + bool is_checkpoint(const chain_state& state) const NOEXCEPT; + bool is_milestone(const chain_state& state) const NOEXCEPT; + bool is_current(const chain_state& state) const NOEXCEPT; + bool is_hard(const chain_state& state) const NOEXCEPT; + + // This is thread safe. + const system::chain::checkpoint& milestone_; - /// Determine if Block is top of a storable branch. - virtual bool is_storable(const system::chain::header& header, - const system::chain::chain_state& state) const NOEXCEPT; + // This is protected by strand. + size_t active_milestone_height_{}; }; } // namespace node diff --git a/include/bitcoin/node/chasers/chaser_organize.hpp b/include/bitcoin/node/chasers/chaser_organize.hpp index 91748ee0..ea54b48a 100644 --- a/include/bitcoin/node/chasers/chaser_organize.hpp +++ b/include/bitcoin/node/chasers/chaser_organize.hpp @@ -40,7 +40,7 @@ class chaser_organize DELETE_COPY_MOVE_DESTRUCT(chaser_organize); /// Initialize chaser state. - code start() NOEXCEPT override; + virtual code start() NOEXCEPT; /// Validate and organize next block in sequence relative to caller peer. virtual void organize(const typename Block::cptr& block_ptr, @@ -66,26 +66,27 @@ class chaser_organize virtual const system::chain::header& get_header( const Block& block) const NOEXCEPT = 0; - /// Query store for const pointer to Block instance. + /// Query store for const pointer to Block instance by candidate height. virtual bool get_block(typename Block::cptr& out, - size_t index) const NOEXCEPT = 0; + size_t height) const NOEXCEPT = 0; + + /// True if Block should bypass validation, given its candidate height. + virtual bool get_bypass(const Block& block, + size_t height) const NOEXCEPT = 0; /// Determine if Block is valid. virtual code validate(const Block& block, const chain_state& state) const NOEXCEPT = 0; - /// Determine if Block is top of a storable branch. - virtual bool is_storable(const Block& block, - const chain_state& state) const NOEXCEPT = 0; - - /// Properties - /// ----------------------------------------------------------------------- + /// Disassociate malleated block and notify repeat header in current job. + virtual void do_malleated(header_t link) NOEXCEPT = 0; - /// Constant access to Block tree. - virtual const block_tree& tree() const NOEXCEPT; + /// Determine if state is top of a storable branch. + virtual bool is_storable(const chain_state& state) const NOEXCEPT = 0; - /// System configuration settings. - virtual const system::settings& settings() const NOEXCEPT; + /// Milestone tracking. + virtual void update_milestone(const system::chain::header& header, + size_t height, size_t branch_point) NOEXCEPT = 0; /// Methods /// ----------------------------------------------------------------------- @@ -94,25 +95,26 @@ class chaser_organize virtual bool handle_event(const code&, chase event_, event_value value) NOEXCEPT; - /// Reorganize following strong branch discovery. + /// Organize a discovered Block. virtual void do_organize(typename Block::cptr& block_ptr, const organize_handler& handler) NOEXCEPT; /// Reorganize following Block unconfirmability. virtual void do_disorganize(header_t header) NOEXCEPT; - /// Disassociate malleated block and notify repeat header in current job. - virtual void do_malleated(header_t link) NOEXCEPT; + /// Properties + /// ----------------------------------------------------------------------- - /// Store Block to database and push to top of candidate chain. - virtual database::header_link push(const Block& block, - const system::chain::context& context) const NOEXCEPT; + /// Constant access to Block tree. + virtual const block_tree& tree() const NOEXCEPT; - /// Height represents a candidate block covered by active milestone. - virtual inline bool is_under_milestone(size_t height) const NOEXCEPT; + /// System configuration settings. + virtual const system::settings& settings() const NOEXCEPT; private: - static constexpr auto flag_bits = to_bits(sizeof(system::chain::flags)); + // Template differetiators. + // ------------------------------------------------------------------------ + static constexpr bool is_block() NOEXCEPT { return is_same_type; @@ -151,25 +153,11 @@ class chaser_organize size_t branch_point) const NOEXCEPT; // Move tree Block to database and push to top of candidate chain. - bool push(const system::hash_digest& key) NOEXCEPT; + bool push_block(const system::hash_digest& key) NOEXCEPT; - // Bypass methods. - // ------------------------------------------------------------------------ - - // Set milestone cache if exists in candidate chain, send chase::bypass. - bool initialize_bypass() NOEXCEPT; - - // Clear milestone cache if its height is above branch_point. - void reset_milestone(size_t branch_point) NOEXCEPT; - - // Set milestone cache if configured milestone matches given checkpoint. - void update_milestone(const database::header_link& link, - size_t height) NOEXCEPT; - void update_milestone(const system::hash_digest& hash, - size_t height) NOEXCEPT; - - // Notify chase::bypass subscribers of a change in bypass height. - void notify_bypass() const NOEXCEPT; + /// Store Block to database and push to top of candidate chain. + bool push_block(const Block& block, + const system::chain::context& context) const NOEXCEPT; // Logging. // ------------------------------------------------------------------------ @@ -180,11 +168,9 @@ class chaser_organize // These are thread safe. const system::settings& settings_; - const system::chain::checkpoint& milestone_; - const system::chain::checkpoints checkpoints_; + const system::chain::checkpoints& checkpoints_; // These are protected by strand. - size_t active_milestone_height_{}; chain_state::ptr state_{}; block_tree tree_{}; }; diff --git a/include/bitcoin/node/chasers/chaser_validate.hpp b/include/bitcoin/node/chasers/chaser_validate.hpp index 87a1d87a..c7fcf9bb 100644 --- a/include/bitcoin/node/chasers/chaser_validate.hpp +++ b/include/bitcoin/node/chasers/chaser_validate.hpp @@ -62,10 +62,6 @@ class BCN_API chaser_validate const database::context& ctx) NOEXCEPT; private: -#if defined (UNDEFINED) - code validate(const database::header_link& link, size_t height) NOEXCEPT; -#endif // UNDEFINED - // neutrino void update_position(size_t height) NOEXCEPT; system::hash_digest get_neutrino(size_t height) const NOEXCEPT; diff --git a/include/bitcoin/node/define.hpp b/include/bitcoin/node/define.hpp index ba6feca9..4d91f90a 100644 --- a/include/bitcoin/node/define.hpp +++ b/include/bitcoin/node/define.hpp @@ -60,8 +60,8 @@ typedef database::query query; /// Work types. typedef network::race_all job; typedef std::shared_ptr map_ptr; -typedef std::function map_handler; +typedef std::function map_handler; /// Node events. typedef uint64_t object_key; diff --git a/include/bitcoin/node/impl/chasers/chaser_organize.ipp b/include/bitcoin/node/impl/chasers/chaser_organize.ipp index 346632d4..aa984f70 100644 --- a/include/bitcoin/node/impl/chasers/chaser_organize.ipp +++ b/include/bitcoin/node/impl/chasers/chaser_organize.ipp @@ -37,23 +37,15 @@ TEMPLATE CLASS::chaser_organize(full_node& node) NOEXCEPT : chaser(node), settings_(config().bitcoin), - milestone_(config().bitcoin.milestone), - checkpoints_(config().bitcoin.sorted_checkpoints()) + checkpoints_(config().bitcoin.checkpoints) { } TEMPLATE code CLASS::start() NOEXCEPT { - using namespace system; using namespace std::placeholders; - if (!initialize_bypass()) - { - fault(error::store_integrity); - return error::store_integrity; - } - // Initialize cache of top candidate chain state. // Spans full chain to obtain cumulative work. This can be optimized by // storing it with each header, though the scan is fast. The same occurs @@ -69,7 +61,7 @@ code CLASS::start() NOEXCEPT return error::get_candidate_chain_state; } - LOGN("Candidate top [" << encode_hash(state_->hash()) << ":" + LOGN("Candidate top [" << system::encode_hash(state_->hash()) << ":" << state_->height() << "]."); SUBSCRIBE_EVENTS(handle_event, _1, _2, _3); @@ -80,25 +72,10 @@ TEMPLATE void CLASS::organize(const typename Block::cptr& block_ptr, organize_handler&& handler) NOEXCEPT { - if (!closed()) - { - POST(do_organize, block_ptr, std::move(handler)); - } -} - -// Properties -// ---------------------------------------------------------------------------- - -TEMPLATE -const system::settings& CLASS::settings() const NOEXCEPT -{ - return settings_; -} + if (closed()) + return; -TEMPLATE -const typename CLASS::block_tree& CLASS::tree() const NOEXCEPT -{ - return tree_; + POST(do_organize, block_ptr, std::move(handler)); } // Methods @@ -118,11 +95,13 @@ bool CLASS::handle_event(const code&, chase event_, event_value value) NOEXCEPT case chase::unvalid: case chase::unconfirmable: { + // Roll back the candidate chain to confirmed top (via fork point). POST(do_disorganize, possible_narrow_cast(value)); break; } case chase::malleated: { + // Re-obtain the malleated block if it is still a candidate. POST(do_malleated, possible_narrow_cast(value)); break; } @@ -143,16 +122,17 @@ TEMPLATE void CLASS::do_organize(typename Block::cptr& block_ptr, const organize_handler& handler) NOEXCEPT { + using namespace system; BC_ASSERT(stranded()); - using namespace system; - const auto hash = block_ptr->hash(); - const auto header = get_header(*block_ptr); + // block_ptr is + const auto& hash = block_ptr->get_hash(); + const auto& header = get_header(*block_ptr); auto& query = archive(); // Skip existing/orphan, get state. // ........................................................................ - + if (closed()) { handler(network::error::service_stopped, {}); @@ -225,7 +205,7 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, // ........................................................................ const auto height = state->height(); - if (chain::checkpoint::is_conflict(settings_.checkpoints, hash, height)) + if (chain::checkpoint::is_conflict(checkpoints_, hash, height)) { handler(system::error::checkpoint_conflict, height); return; @@ -237,8 +217,8 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, return; } - // Store with checkpoint, milestone, or currency with sufficient work. - if (!is_storable(*block_ptr, *state)) + // Store the chain once it is sufficiently guaranteed. + if (!is_storable(*state)) { log_state_change(*parent, *state); cache(block_ptr, state); @@ -267,7 +247,7 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, return; } - // New top of current weak branch. + // New top of a weak branch. if (!strong) { log_state_change(*parent, *state); @@ -279,6 +259,10 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, // Reorganize candidate chain. // ........................................................................ + // A milestone can only be set within a to-be-archived chain of candidate + // headers/blocks. Once the milestone block is archived it is not useful. + update_milestone(header, height, branch_point); + const auto top_candidate = state_->height(); if (branch_point > top_candidate) { @@ -299,18 +283,9 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, fire(events::header_reorganized, index--); } - // BUGBUG: this is insufficient because downloads race ahead. - // BUGBUG: the new branch can become ordered and downloaded under the old - // BUGBUG: milestone while the new is pending in the notification queue. - // BUGBUG: probably need to provide both fork point and old top. - // BUGBUG: because validation and confirmation are strictly ordered, this - // BUGBUG: only affects check, and so block check is only bypassed under - // BUGBUG: checkpoint. However confirmation writes proceed under milestone. - // BUGBUG: These differ in that their state is cheaply detected, whereas - // BUGBUG: lack of block check would require read/check of each full block. - - // branch_point - reset_milestone(++index); + // Shift chasers to new branch (vs. continuous branch). + if (branch_point < top_candidate) + notify(error::success, chase::regressed, branch_point); // Push stored strong headers to candidate chain. for (const auto& link: views_reverse(store_branch)) @@ -322,13 +297,13 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, } ////fire(events::header_organized, index); - update_milestone(link, index++); + index++; } // Store strong tree headers and push to candidate chain. for (const auto& key: views_reverse(tree_branch)) { - if (!push(key)) + if (!push_block(key)) { handler(fault(error::node_push), height); return; @@ -336,12 +311,12 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, ////fire(events::header_archived, index); ////fire(events::header_organized, index); - update_milestone(key, index++); + index++; } // Push new header as top of candidate chain. { - if (push(*block_ptr, state->context()).is_terminal()) + if (!push_block(*block_ptr, state->context())) { handler(fault(error::node_push), height); return; @@ -349,7 +324,6 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, ////fire(events::header_archived, index); ////fire(events::header_organized, index); - update_milestone(block_ptr->hash(), index); } // Reset top chain state and notify. @@ -371,18 +345,9 @@ void CLASS::do_organize(typename Block::cptr& block_ptr, notify(error::success, chase_object(), branch_point); } - // Check chaser may be working on any of the blocks, and subsequent until - // it receives this message. That will reset to the branch point, but the - // work on the new branch is usable. - if (branch_point < top_candidate) - { - notify(error::success, chase::regressed, branch_point); - } - // Logs from candidate block parent to the candidate (forward sequential). log_state_change(*parent, *state); state_ = state; - handler(error::success, height); } @@ -396,7 +361,8 @@ void CLASS::do_disorganize(header_t link) NOEXCEPT // Skip already reorganized out, get height. // ........................................................................ - // Upon restart candidate chain validation will hit unconfirmable block. + // Upon restart chain validation will hit unconfirmable or gapped block. + // Gapping occurs with a malleated64 from confirm in blocks first handling. if (closed()) return; @@ -466,11 +432,8 @@ void CLASS::do_disorganize(header_t link) NOEXCEPT fire(events::header_reorganized, index); } - // BUGBUG: this is insufficient because downloads race ahead. - // BUGBUG: the new branch can become ordered and downloaded under the old - // BUGBUG: milestone while the new is pending in the notification queue. - // BUGBUG: probably need to provide both fork point and old top. - reset_milestone(fork_point); + // Shift chasers to old branch (will be the currently confirmed). + notify(error::success, chase::disorganized, fork_point); // Push confirmed headers from above fork point onto candidate chain. // ........................................................................ @@ -486,7 +449,6 @@ void CLASS::do_disorganize(header_t link) NOEXCEPT } fire(events::header_organized, index); - update_milestone(confirmed, index); } state = query.get_candidate_chain_state(settings_, top_confirmed); @@ -496,41 +458,12 @@ void CLASS::do_disorganize(header_t link) NOEXCEPT return; } - // Check chaser may be working on any of the blocks, and subsequent until - // it receives this message. That will reset to the branch point, but the - // work on the new branch is usable. - notify(error::success, chase::disorganized, fork_point); - // Logs from previous top candidate to previous fork point (jumps back). log_state_change(*state_, *state); state_ = state; -} -// The archived malleable block was found to be invalid (treat as malleated). -// The block/header hash cannot be marked unconfirmable due to malleability, so -// disassociate the block and then notify check chaser to reisuse the download. -// This must be issued here in order to ensure proper bypass/regress ordering. -TEMPLATE -void CLASS::do_malleated(header_t link) NOEXCEPT -{ - BC_ASSERT(stranded()); - auto& query = archive(); - - // If not disassociated, validation/confirmation will be reattemted. - // This could happen due to shutdown before this step is completed. - if (!query.set_dissasociated(link)) - { - fault(error::set_dissasociated); - return; - } - - // Header is no longer in the candidate chain, so do not announce. - if (!query.is_candidate_header(link)) - return; - - // Announce a singleton header that requires download. - // Since it is in the candidate chain, it must presently be missing. - notify(error::success, chase::header, link); + // Reset all connections to ensure that new connections exist. + notify(error::success, chase::suspend, {}); } // Private @@ -631,108 +564,40 @@ bool CLASS::get_is_strong(bool& strong, const uint256_t& branch_work, } TEMPLATE -database::header_link CLASS::push(const Block& block, +bool CLASS::push_block(const Block& block, const system::chain::context& context) const NOEXCEPT { - using namespace system; auto& query = archive(); - const auto link = query.set_link(block, database::context - { - possible_narrow_cast(context.flags), - possible_narrow_cast(context.height), - context.median_time_past, - }); + const auto bypass = get_bypass(block, context.height); - return query.push_candidate(link) ? link : database::header_link{}; + // TODO: change this to set_code() and return code. + // TODO: add confirm option to set_code() and pass bypass (to both). + // TODO: block bypass/confirm is checkpoints, headers is also milestone. + return query.push_candidate(query.set_link(block, context, bypass)); } TEMPLATE -bool CLASS::push(const system::hash_digest& key) NOEXCEPT +bool CLASS::push_block(const system::hash_digest& key) NOEXCEPT { - const auto value = tree_.extract(key); - BC_ASSERT_MSG(!value.empty(), "missing tree value"); - - auto& query = archive(); - const auto& it = value.mapped(); - const auto link = query.set_link(*it.block, it.state->context()); - return query.push_candidate(link); + const auto handle = tree_.extract(key); + if (!handle) return false; + const auto& value = handle.mapped(); + return push_block(*value.block, value.state->context()); } -// Bypass methods. +// Properties // ---------------------------------------------------------------------------- -// protected -TEMPLATE -inline bool CLASS::is_under_milestone(size_t height) const NOEXCEPT -{ - return height <= active_milestone_height_; -} - -TEMPLATE -bool CLASS::initialize_bypass() NOEXCEPT -{ - active_milestone_height_ = zero; - if (is_zero(milestone_.height()) || - milestone_.hash() == system::null_hash) - return true; - - const auto& query = archive(); - const auto link = query.to_candidate(milestone_.height()); - if (link.is_terminal()) - return true; - - const auto hash = query.get_header_key(link); - if (hash == system::null_hash) - return false; - - if (hash == milestone_.hash()) - active_milestone_height_ = milestone_.height(); - - // Protocols are not started when this is sent. - notify_bypass(); - return true; -} - -TEMPLATE -void CLASS::reset_milestone(size_t branch_point) NOEXCEPT -{ - if (active_milestone_height_ > branch_point) - { - // Allow use of milestone on its partial subbranch. - active_milestone_height_ = branch_point; - notify_bypass(); - } -} - -TEMPLATE -void CLASS::update_milestone(const database::header_link& link, - size_t height) NOEXCEPT -{ - // Defer querying for hash until heights are compared. - if (height != milestone_.height()) - return; - - // Invokes a redundant height comparison, but only once for entire chain. - update_milestone(archive().get_header_key(link), height); -} - TEMPLATE -void CLASS::update_milestone(const system::hash_digest& hash, - size_t height) NOEXCEPT +const system::settings& CLASS::settings() const NOEXCEPT { - if (height == milestone_.height() && hash == milestone_.hash()) - { - BC_ASSERT(is_zero(active_milestone_height_)); - active_milestone_height_ = height; - notify_bypass(); - } + return settings_; } TEMPLATE -void CLASS::notify_bypass() const NOEXCEPT +const typename CLASS::block_tree& CLASS::tree() const NOEXCEPT { - notify(error::success, chase::bypass, - std::max(active_milestone_height_, checkpoint())); + return tree_; } // Logging. @@ -747,6 +612,7 @@ void CLASS::log_state_change(const chain_state& from, { if (from.flags() != to.flags()) { + constexpr auto flag_bits = to_bits(sizeof(chain::flags)); const binary prev{ flag_bits, to_big_endian(from.flags()) }; const binary next{ flag_bits, to_big_endian(to.flags()) }; diff --git a/src/chasers/chaser.cpp b/src/chasers/chaser.cpp index e095596c..92bf2964 100644 --- a/src/chasers/chaser.cpp +++ b/src/chasers/chaser.cpp @@ -130,35 +130,9 @@ bool chaser::is_current(uint32_t timestamp) const NOEXCEPT return node_.is_current(timestamp); } -// Bypass. -// ---------------------------------------------------------------------------- - -size_t chaser::bypass() const NOEXCEPT -{ - BC_ASSERT(stranded()); - return bypass_; -} - -void chaser::set_bypass(height_t height) NOEXCEPT -{ - BC_ASSERT(stranded()); - bypass_ = height; -} - -bool chaser::is_bypassed(size_t height) const NOEXCEPT -{ - BC_ASSERT(stranded()); - return height <= bypass(); -} - bool chaser::is_under_checkpoint(size_t height) const NOEXCEPT { - return height <= checkpoint(); -} - -size_t chaser::checkpoint() const NOEXCEPT -{ - return top_checkpoint_height_; + return height <= top_checkpoint_height_; } // Position. @@ -166,13 +140,15 @@ size_t chaser::checkpoint() const NOEXCEPT size_t chaser::position() const NOEXCEPT { - BC_ASSERT(stranded()); + // Called from start. + ////BC_ASSERT(stranded()); return position_; } void chaser::set_position(size_t height) NOEXCEPT { - BC_ASSERT(stranded()); + // Called from start. + ////BC_ASSERT(stranded()); position_ = height; } diff --git a/src/chasers/chaser_block.cpp b/src/chasers/chaser_block.cpp index e7186d50..46b69d81 100644 --- a/src/chasers/chaser_block.cpp +++ b/src/chasers/chaser_block.cpp @@ -40,12 +40,20 @@ const header& chaser_block::get_header(const block& block) const NOEXCEPT return block.header(); } -bool chaser_block::get_block(block::cptr& out, size_t index) const NOEXCEPT +bool chaser_block::get_block(block::cptr& out, size_t height) const NOEXCEPT { - out = archive().get_block(archive().to_candidate(index)); + const auto& query = archive(); + out = query.get_block(query.to_candidate(height)); return !is_null(out); } +bool chaser_block::get_bypass(const block& block, size_t height) const NOEXCEPT +{ + // Milestones are not relevant to block-first organization. + // TODO: Can a validated block be malleable64 (i.e. can we ignore here). + return is_under_checkpoint(height) && !block.is_malleable64(); +} + code chaser_block::validate(const block& block, const chain_state& state) const NOEXCEPT { @@ -67,9 +75,7 @@ code chaser_block::validate(const block& block, // Transaction/witness commitments are required under checkpoint. // This ensures that the block/header hash represents expected txs. - // Performs full check if block is mally64 (mally32 caught either way). - const auto bypass = is_under_checkpoint(state.height()) && - !block.is_malleable64(); + const auto bypass = get_bypass(block, state.height()); // Transaction commitments and malleated32 are checked under checkpoint. if ((ec = block.check(bypass))) @@ -95,48 +101,60 @@ code chaser_block::validate(const block& block, return block.connect(state.context()); } -// Blocks are accumulated following genesis, not cached until current. -bool chaser_block::is_storable(const block&, const chain_state&) const NOEXCEPT +// The archived malleable block was found to be invalid (treat as malleated). +// The block/header hash cannot be marked unconfirmable due to malleability, so +// disassociate the block and then disorganize the chain to malleation point. +// This will disorganize the candidate chain to match the confirmed. +void chaser_block::do_malleated(header_t link) NOEXCEPT { - return true; + BC_ASSERT(stranded()); + auto& query = archive(); + + // If not disassociated, validation/confirmation will be reattempted. + // This could happen due to shutdown before this step is completed. + if (!query.set_dissasociated(link)) + { + fault(error::set_dissasociated); + return; + } + + // Treat as disorganization, but block is only gapped not invalidated. + do_disorganize(link); } -// Store block to database and push to top of candidate chain. -// Whole blocks pushed here do not require set_txs_connected(), since the block -// is already validated, but do require set_block_confirmable() as the -// confirmation chaser is bypassed (moves straight to confirmation chaser). -database::header_link chaser_block::push(const block& block, - const context& context) const NOEXCEPT +bool chaser_block::is_storable(const chain_state&) const NOEXCEPT { - auto& query = archive(); - const auto link = chaser_organize::push(block, context); - if (!query.set_block_confirmable(link, block.fees())) - return {}; + return true; +} - return link; +void chaser_block::update_milestone(const header&, size_t, size_t) NOEXCEPT +{ } +// Populate methods (private). +// ---------------------------------------------------------------------------- + void chaser_block::set_prevout(const input& input) const NOEXCEPT { const auto& point = input.point(); - // Scan all blocks for matching tx (linear :/ but legacy scenario) + // Scan all tree blocks for matching tx (linear :/ but legacy scenario) std::for_each(tree().begin(), tree().end(), [&](const auto& item) NOEXCEPT { - const auto& txs = *item.second.block->transactions_ptr(); - const auto it = std::find_if(txs.begin(), txs.end(), + const transactions_cptr txs{ item.second.block->transactions_ptr() }; + const auto it = std::find_if(txs->begin(), txs->end(), [&](const auto& tx) NOEXCEPT { return tx->hash(false) == point.hash(); }); - if (it != txs.end()) + if (it != txs->end()) { - const auto& tx = **it; - const auto& outs = *tx.outputs_ptr(); - if (point.index() < outs.size()) + const transaction::cptr tx{ *it }; + const outputs_cptr outs{ tx->outputs_ptr() }; + if (point.index() < outs->size()) { - input.prevout = outs.at(point.index()); + input.prevout = outs->at(point.index()); return; } } @@ -147,7 +165,7 @@ void chaser_block::set_prevout(const input& input) const NOEXCEPT void chaser_block::populate(const block& block) const NOEXCEPT { block.populate(); - const auto ins = block.inputs_ptr(); + const inputs_cptr ins{ block.inputs_ptr() }; std::for_each(ins->begin(), ins->end(), [&](const auto& in) NOEXCEPT { if (!in->prevout && !in->point().is_null()) diff --git a/src/chasers/chaser_check.cpp b/src/chasers/chaser_check.cpp index cec4a555..f330c8eb 100644 --- a/src/chasers/chaser_check.cpp +++ b/src/chasers/chaser_check.cpp @@ -120,11 +120,6 @@ bool chaser_check::handle_event(const code&, chase event_, break; } // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ - case chase::bypass: - { - POST(set_bypass, possible_narrow_cast(value)); - break; - } case chase::header: { POST(do_header, possible_narrow_cast(value)); @@ -176,8 +171,10 @@ void chaser_check::start_tracking() NOEXCEPT void chaser_check::stop_tracking() NOEXCEPT { - BC_ASSERT(stranded()); + // Called by stop (node thread). + ////BC_ASSERT(stranded()); + // shared_ptr.reset() is thread safe so can be called at stop. // Resetting our own pointer allows destruct and call to handle_purged. job_.reset(); } @@ -196,10 +193,6 @@ void chaser_check::do_handle_purged(const code&) NOEXCEPT { BC_ASSERT(stranded()); - // TODO: set_unstrong(link) where link of all associated and not malleable - // TODO: from min(candidate_top, bypass) to > branch_point (do_regressed). - // TODO: cannot rely on height index. Probably need to notify with range. - start_tracking(); do_bump(height_t{}); } @@ -299,11 +292,9 @@ void chaser_check::do_get_hashes(const map_handler& handler) NOEXCEPT return; const auto map = get_map(); - handler(error::success, map, job_, bypass()); + handler(error::success, map, job_); } -// It is possible that this call can be made before a purge has been sent and -// received after. This may result in unnecessary work and incorrect bypass. void chaser_check::do_put_hashes(const map_ptr& map, const result_handler& handler) NOEXCEPT { diff --git a/src/chasers/chaser_confirm.cpp b/src/chasers/chaser_confirm.cpp index dde82430..e38b7894 100644 --- a/src/chasers/chaser_confirm.cpp +++ b/src/chasers/chaser_confirm.cpp @@ -73,11 +73,6 @@ bool chaser_confirm::handle_event(const code&, chase event_, POST(do_validated, possible_narrow_cast(value)); break; } - case chase::bypass: - { - POST(set_bypass, possible_narrow_cast(value)); - break; - } case chase::stop: { return false; @@ -152,13 +147,20 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT header_links popped{}; while (index > fork_point) { - popped.push_back(query.to_confirmed(index)); - if (popped.back().is_terminal()) + const auto link = query.to_confirmed(index); + if (link.is_terminal()) { fault(error::to_confirmed); return; } + popped.push_back(link); + if (!query.set_unstrong(link)) + { + fault(error::node_confirm); + return; + } + if (!query.pop_confirmed()) { fault(error::pop_confirmed); @@ -195,15 +197,34 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT return; } - // error::confirmation_bypass is not used. - if (ec == database::error::block_confirmable || - (is_under_checkpoint(index) && !query.is_malleable64(link))) + // checkpointed blocks are set_strong concurrently by block_in protocol. + const auto malleable64 = query.is_malleable64(link); + const auto stronged = is_under_checkpoint(index) && !malleable64; + + // These are cheap, so do even though checkpoint overlaps bypassed. + auto bypass = stronged || ec == database::error::block_confirmable; + + // malleable64 overrides bypass state. + if (!bypass && !malleable64 && !query.get_bypass(bypass, link)) + { + fault(database::error::integrity); + return; + } + + // Required for block_confirmable and all confirmed blocks. + if (!stronged && !query.set_strong(link)) + { + fault(error::set_confirmed); + return; + } + + if (bypass) { notify(ec, chase::confirmable, index); - fire(events::confirm_bypassed, index); + ////fire(events::confirm_bypassed, index); // chase::organized & events::block_organized - if (!query.set_strong(link) || !set_organized(link, index)) + if (!set_organized(link, index)) { fault(error::set_confirmed); return; @@ -212,13 +233,6 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT continue; } - // Required for block_confirmable. - if (!query.set_strong(link)) - { - fault(error::node_confirm); - return; - } - ec = query.block_confirmable(link); if (ec == database::error::integrity) { @@ -235,20 +249,19 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT return; } - // Transactions are set strong upon archive when under bypass. Only - // malleable blocks are validated under bypass, and not set strong. - if (is_bypassed(height)) + // TODO: Can a validated block be malleable64 (i.e. can we ignore). + if (malleable64) { LOGR("Malleated64 block [" << index << "] " << ec.message()); notify(ec, chase::malleated, link); fire(events::block_malleated, index); - + // chase::reorganized & events::block_reorganized // chase::organized & events::block_organized // index has not been confirmed, so start prior. if (!roll_back(popped, fork_point, sub1(index))) fault(error::node_roll_back); - + return; } @@ -296,14 +309,14 @@ void chaser_confirm::do_validated(height_t height) NOEXCEPT // Private // ---------------------------------------------------------------------------- -bool chaser_confirm::set_organized(header_t link, height_t height) NOEXCEPT +bool chaser_confirm::set_organized(header_t link, height_t) NOEXCEPT { auto& query = archive(); if (!query.push_confirmed(link)) return false; notify(error::success, chase::organized, link); - fire(events::block_organized, height); + ////fire(events::block_organized, height); return true; } diff --git a/src/chasers/chaser_header.cpp b/src/chasers/chaser_header.cpp index e1daeb82..54ffbc29 100644 --- a/src/chasers/chaser_header.cpp +++ b/src/chasers/chaser_header.cpp @@ -29,23 +29,38 @@ namespace node { using namespace system::chain; chaser_header::chaser_header(full_node& node) NOEXCEPT - : chaser_organize
(node) + : chaser_organize
(node), + milestone_(config().bitcoin.milestone) { } +code chaser_header::start() NOEXCEPT +{ + if (!initialize_milestone()) + return fault(error::store_integrity); + + return chaser_organize
::start(); +} + const header& chaser_header::get_header(const header& header) const NOEXCEPT { return header; } -bool chaser_header::get_block(system::chain::header::cptr& out, - size_t index) const NOEXCEPT +bool chaser_header::get_block(header::cptr& out, size_t height) const NOEXCEPT { - out = archive().get_header(archive().to_candidate(index)); + const auto& query = archive(); + out = query.get_header(query.to_candidate(height)); return !is_null(out); } -code chaser_header::validate(const system::chain::header& header, +bool chaser_header::get_bypass(const header&, size_t height) const NOEXCEPT +{ + // Malleability is not known for headers, so must be guarded at validation. + return is_under_milestone(height) || is_under_checkpoint(height); +} + +code chaser_header::validate(const header& header, const chain_state& state) const NOEXCEPT { code ec{ error::success }; @@ -72,15 +87,124 @@ code chaser_header::validate(const system::chain::header& header, return system::error::block_success; } -// Cache valid headers until storable. -bool chaser_header::is_storable(const system::chain::header& header, - const chain_state& state) const NOEXCEPT +// The archived malleable block was found to be invalid (treat as malleated). +// The block/header hash cannot be marked unconfirmable due to malleability, so +// disassociate the block and then notify check chaser to reisuse the download. +// This must be issued here in order to ensure proper bypass/regress ordering. +void chaser_header::do_malleated(header_t link) NOEXCEPT { - return - checkpoint::is_at(settings().checkpoints, state.height()) || - settings().milestone.equals(state.hash(), state.height()) || - (is_current(header.timestamp()) && state.cumulative_work() >= - settings().minimum_work); + BC_ASSERT(stranded()); + auto& query = archive(); + + // If not disassociated, validation/confirmation will be reattempted. + // This could happen due to shutdown before this step is completed. + if (!query.set_dissasociated(link)) + { + fault(error::set_dissasociated); + return; + } + + // Header is no longer in the candidate chain, so do not announce. + if (!query.is_candidate_header(link)) + return; + + // Announce a singleton header that requires download. + // Since it is in the candidate chain, it must presently be missing. + notify(error::success, chase::header, link); +} + +// Storable methods (private). +// ---------------------------------------------------------------------------- + +bool chaser_header::is_storable(const chain_state& state) const NOEXCEPT +{ + return is_checkpoint(state) + || is_milestone(state) + || (is_current(state) && is_hard(state)); +} + +bool chaser_header::is_checkpoint(const chain_state& state) const NOEXCEPT +{ + return checkpoint::is_at(settings().checkpoints, state.height()); +} + +bool chaser_header::is_milestone(const chain_state& state) const NOEXCEPT +{ + return milestone_.equals(state.hash(), state.height()); +} + +bool chaser_header::is_current(const chain_state& state) const NOEXCEPT +{ + return chaser::is_current(state.timestamp()); +} + +bool chaser_header::is_hard(const chain_state& state) const NOEXCEPT +{ + return state.cumulative_work() >= settings().minimum_work; +} + +// Milestone methods (private). +// ---------------------------------------------------------------------------- + +bool chaser_header::is_under_milestone(size_t height) const NOEXCEPT +{ + return height <= active_milestone_height_; +} + +bool chaser_header::initialize_milestone() NOEXCEPT +{ + active_milestone_height_ = zero; + if (is_zero(milestone_.height()) || + milestone_.hash() == system::null_hash) + return true; + + const auto& query = archive(); + const auto link = query.to_candidate(milestone_.height()); + if (link.is_terminal()) + return true; + + const auto hash = query.get_header_key(link); + if (hash == system::null_hash) + return false; + + if (hash == milestone_.hash()) + active_milestone_height_ = milestone_.height(); + + return true; +} + +void chaser_header::update_milestone(const system::chain::header& header, + size_t height, size_t branch_point) NOEXCEPT +{ + if (milestone_.equals(header.get_hash(), height)) + { + active_milestone_height_ = height; + return; + } + + // Use pointer to avoid const/copy. + auto previous = &header.previous_block_hash(); + + // Scan branch for milestone match. + for (auto it = tree().find(*previous); it != tree().end(); + it = tree().find(*previous)) + { + const auto index = it->second.state->height(); + if (milestone_.equals(it->second.state->hash(), index)) + { + active_milestone_height_ = index; + return; + } + + const auto& next = get_header(*it->second.block); + previous = &next.previous_block_hash(); + } + + // The current active milestone is necessarily on the candidate branch. + // New branch doesn't have milestone and reorganizes the branch with it. + // Can retain a milestone at the branch point (below its definition). + if (active_milestone_height_ > branch_point) + active_milestone_height_ = branch_point; } } // namespace node diff --git a/src/chasers/chaser_validate.cpp b/src/chasers/chaser_validate.cpp index dca49cd4..503c60a4 100644 --- a/src/chasers/chaser_validate.cpp +++ b/src/chasers/chaser_validate.cpp @@ -94,11 +94,6 @@ bool chaser_validate::handle_event(const code&, chase event_, break; } // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ - case chase::bypass: - { - POST(set_bypass, possible_narrow_cast(value)); - break; - } case chase::stop: { return false; @@ -173,13 +168,24 @@ void chaser_validate::do_bump(height_t) NOEXCEPT return; } - // error::validation_bypass is not used because fan-out. - if (ec == database::error::block_valid || + // These are cheap, so do even though checkpoint overlaps bypassed. + const auto malleable64 = query.is_malleable64(link); + auto bypass = ( + ec == database::error::block_valid || ec == database::error::block_confirmable || - (is_under_checkpoint(height) && !query.is_malleable64(link))) + (is_under_checkpoint(height) && !malleable64)); + + // malleable64 overrides bypass state because it's set from header only. + if (!bypass && !malleable64 && !query.get_bypass(bypass, link)) + { + fault(database::error::integrity); + return; + } + + if (bypass) { update_position(height); - fire(events::validate_bypassed, height); + ////fire(events::validate_bypassed, height); notify(ec, chase::valid, height); continue; } @@ -350,15 +356,15 @@ void chaser_validate::validate_block(const code& ec, if (ec) { - // Transactions are set strong upon archive when under bypass. Only - // malleable blocks are validated under bypass, and not set strong. - if (is_bypassed(ctx.height)) - { - LOGR("Malleated64 block [" << ctx.height << "] " << ec.message()); - notify(ec, chase::malleated, link); - fire(events::block_malleated, ctx.height); - return; - } + ////// Transactions are set strong upon archive when under bypass. Only + ////// malleable blocks are validated under bypass, and not set strong. + ////if (is_bypassed(ctx.height)) + ////{ + //// LOGR("Malleated64 block [" << ctx.height << "] " << ec.message()); + //// notify(ec, chase::malleated, link); + //// fire(events::block_malleated, ctx.height); + //// return; + ////} if (!query.set_block_unconfirmable(link)) { diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 7758d208..df6a0fef 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/src/protocols/protocol_block_in_31800.cpp @@ -280,7 +280,7 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, auto& query = archive(); const chain::block::cptr block_ptr{ message->block_ptr }; - const auto hash = block_ptr->hash(); + const auto& hash = block_ptr->get_hash(); const auto it = map_->find(hash); if (it == map_->end()) @@ -308,15 +308,22 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, // Check block. // ........................................................................ - // Transaction/witness commitments are required under checkpoint. - // This ensures that the block/header hash represents expected txs. - // Do not consider milestone for check because regressions can result in - // stored unchecked blocks, and the cost of mitigation exceeds the check. - // Do not consider milestone for set_strong because regressions can result - // in wrong strong or unstrong and cost of mitigation exceeds the benefit. - const auto bypass = is_under_checkpoint(ctx.height) && !malleable64; + // set_strong checkpointed blocks as these are not regressable. + // checkpointed and malleable64 blocks must be set_strong post-validation. + const auto strong = is_under_checkpoint(ctx.height) && !malleable64; + + // These are cheap, so do even though checkpoint overlaps bypassed. + auto bypass = strong + || ec == database::error::block_valid + || ec == database::error::block_confirmable; + + // malleable64 overrides bypass state. + if (!bypass && !malleable64 && !query.get_bypass(bypass, link)) + { + stop(fault(database::error::integrity)); + return false; + } - // Performs full check if block is mally64 (mally32 caught either way). if (const auto code = check(*block_ptr, ctx, bypass)) { // Uncommitted blocks have no creation cost, just bogus data. @@ -378,8 +385,14 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, const auto size = block_ptr->serialized_size(true); const chain::transactions_cptr txs_ptr{ block_ptr->transactions_ptr() }; - // Transactions are set_strong here when bypass is true. - if (const auto code = query.set_code(*txs_ptr, link, size, false)) + // TODO: Set strong when bypassed and not malleable64. This requires that + // TODO: candidate reorganization must set_unstrong all bypassed and not + // TODO: malleable64 (associated) blocks and must set_strong on any later + // TODO: reassociation of the same, so that confirm chaser can rely. This + // TODO: has to be performed by the organizer, since it owns candidates. + + // Transactions are set_strong here when checkpointed and not malleable64. + if (const auto code = query.set_code(*txs_ptr, link, size, strong)) { LOGF("Failure storing block [" << encode_hash(hash) << ":" << ctx.height << "] from [" << authority() << "] " @@ -396,10 +409,7 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, << "] from [" << authority() << "]."); notify(error::success, chase::checked, ctx.height); - - // TODO: remove event restriction. - if (is_one(ctx.height) || (is_zero(ctx.height % maximum_concurrency_))) - fire(events::block_archived, ctx.height); + fire(events::block_archived, ctx.height); count(message->cached_size); map_->erase(it);