Use only checkpoint bypass, skip validate but apply confirmation.

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, ...) \

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;

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_{};

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.

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.

src/chasers/chaser_confirm.cpp

@@ -175,34 +175,50 @@ 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)
         {
             fault(ec);
             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,14 +228,27 @@ 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))
             {
                 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;

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<height_t>(value));
-        ////    break;
-        ////}
-        ////case chase::regressed:
-        ////{
-        ////    POST(do_regressed, possible_narrow_cast<height_t>(value));
-        ////    break;
-        ////}
-        ////case chase::disorganized:
-        ////{
-        ////    POST(do_regressed, possible_narrow_cast<height_t>(value));
-        ////    break;
-        ////}
+        case chase::start:
+        case chase::bump:
+        {
+            POST(do_bump, height_t{});
+            break;
+        }
+        case chase::checked:
+        {
+            POST(do_checked, possible_narrow_cast<height_t>(value));
+            break;
+        }
+        case chase::regressed:
+        {
+            POST(do_regressed, possible_narrow_cast<height_t>(value));
+            break;
+        }
+        case chase::disorganized:
+        {
+            POST(do_regressed, possible_narrow_cast<height_t>(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);
     }
 }
 

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() << "] "