Fix validate/confirm threadpool/strand initialization order.

console/executor_test_reader.cpp

@@ -105,7 +105,7 @@ void executor::read_test(bool dump) const
         if (cancel_)
             return;
 
-        size_t found{};
+        ////size_t found{};
         auto address_it = store_.address.it(key);
         if (address_it.self().is_terminal())
         {
@@ -179,7 +179,7 @@ void executor::read_test(bool dump) const
                 sp_tx_fk = spend.parent_fk;
             }
 
-            ++found;
+            ////++found;
             outs.push_back(out
             {
                 key,

include/bitcoin/node/chasers/chaser_confirm.hpp

@@ -87,15 +87,15 @@ class BCN_API chaser_confirm
     bool get_is_strong(bool& strong, const uint256_t& fork_work,
         size_t fork_point) const NOEXCEPT;
 
-    // These are thread safe.
-    const bool concurrent_;
-    network::asio::strand independent_strand_;
-
     // These are protected by strand.
-    network::threadpool threadpool_;
-    neutrino_header neutrino_{};
-    bool filters_{};
     bool mature_{};
+    bool filters_{};
+    neutrino_header neutrino_{};
+    network::threadpool threadpool_;
+
+    // These are thread safe.
+    network::asio::strand independent_strand_;
+    const bool concurrent_;
 };
 
 } // namespace node

include/bitcoin/node/chasers/chaser_validate.hpp

@@ -66,24 +66,17 @@ class BCN_API chaser_validate
         return backlog_ < maximum_backlog_;
     }
 
-    bool set_neutrino(const database::header_link& link,
-        const system::chain::block& block) NOEXCEPT;
-
-    bool set_prevouts(size_t height,
-        const system::chain::block& block) NOEXCEPT;
+    // These are protected by strand.
+    bool mature_{};
+    size_t backlog_{};
+    network::threadpool threadpool_;
 
     // These are thread safe.
-    const bool concurrent_;
-    const size_t maximum_backlog_;
-    const uint64_t initial_subsidy_;
-    const uint32_t subsidy_interval_;
     network::asio::strand independent_strand_;
-
-    // These are protected by strand.
-    network::threadpool threadpool_;
-    size_t backlog_{};
-    bool filters_{};
-    bool mature_{};
+    const uint32_t subsidy_interval_;
+    const uint64_t initial_subsidy_;
+    const size_t maximum_backlog_;
+    const bool concurrent_;
 };
 
 } // namespace node

src/chasers/chaser_confirm.cpp

@@ -41,9 +41,9 @@ BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
 // Higher priority than validator ensures locality to validator reads.
 chaser_confirm::chaser_confirm(full_node& node) NOEXCEPT
   : chaser(node),
-    concurrent_(node.config().node.concurrent_confirmation),
     threadpool_(one, node.config().node.priority_()),
-    independent_strand_(threadpool_.service().get_executor())
+    independent_strand_(threadpool_.service().get_executor()),
+    concurrent_(node.config().node.concurrent_confirmation)
 {
 }
 
@@ -207,8 +207,10 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
                 fault(error::confirm2);
                 return;
             }
-
+
+            /////////////////////////////////////////
             // Confirmation query.
+            /////////////////////////////////////////
             if ((ec = query.block_confirmable(link)))
             {
                 if (ec == database::error::integrity)
@@ -243,6 +245,12 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
                 fault(error::confirm6);
                 return;
             }
+
+            if (!set_organized(link, height))
+            {
+                fault(error::confirm7);
+                return;
+            }
 
             notify(error::success, chase::confirmable, height);
             fire(events::block_confirmed, height);
@@ -253,7 +261,7 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
         {
             if (!query.set_strong(link))
             {
-                fault(error::confirm7);
+                fault(error::confirm8);
                 return;
             }
 
@@ -271,7 +279,7 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
             // database::error::unknown_state       [shouldn't be here]
             // database::error::unassociated        [shouldn't be here]
             // database::error::unvalidated         [shouldn't be here]
-            fault(error::confirm8);
+            fault(error::confirm9);
             return;
         }
 
@@ -286,7 +294,7 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
 // first) here. Candidate chain reorganizations will result in reported heights
 // moving in any direction. Each is treated as independent and only one
 // representing a stronger chain is considered.
-
+////
 ////// Compute relative work, set fork and fork_point, and invoke reorganize.
 ////void chaser_confirm::do_validated(height_t height) NOEXCEPT
 ////{
@@ -613,7 +621,6 @@ bool chaser_confirm::get_is_strong(bool& strong, const uint256_t& fork_work,
 // neutrino
 // ----------------------------------------------------------------------------
 
-// This can only fail if prevouts are not fully populated.
 bool chaser_confirm::update_neutrino(const header_link& link) NOEXCEPT
 {
     // neutrino_.link is only used for this assertion, should compile away.

src/chasers/chaser_validate.cpp

@@ -43,19 +43,18 @@ BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
 // Higher priority than downloader (net) ensures locality to downloader writes.
 chaser_validate::chaser_validate(full_node& node) NOEXCEPT
   : chaser(node),
-    concurrent_(node.config().node.concurrent_validation),
-    maximum_backlog_(node.config().node.maximum_concurrency_()),
-    initial_subsidy_(node.config().bitcoin.initial_subsidy()),
-    subsidy_interval_(node.config().bitcoin.subsidy_interval_blocks),
     threadpool_(node.config().node.threads_(), node.config().node.priority_()),
-    independent_strand_(threadpool_.service().get_executor())
+    independent_strand_(threadpool_.service().get_executor()),
+    subsidy_interval_(node.config().bitcoin.subsidy_interval_blocks),
+    initial_subsidy_(node.config().bitcoin.initial_subsidy()),
+    maximum_backlog_(node.config().node.maximum_concurrency_()),
+    concurrent_(node.config().node.concurrent_validation)
 {
 }
 
 code chaser_validate::start() NOEXCEPT
 {
     const auto& query = archive();
-    filters_ = query.neutrino_enabled();
     set_position(query.get_fork());
     SUBSCRIBE_EVENTS(handle_event, _1, _2, _3);
     return error::success;
@@ -200,61 +199,61 @@ void chaser_validate::do_bump(height_t) NOEXCEPT
     }
 }
 
-// unstranded (concurrent by block)
+// Unstranded (concurrent by block).
 void chaser_validate::validate_block(const header_link& link) NOEXCEPT
 {
     if (closed())
         return;
 
+    code ec{};
+    chain::context ctx{};
     auto& query = archive();
     const auto block = query.get_block(link);
+
     if (!block)
     {
-        POST(complete_block, error::validate1, link, zero);
-        return;
+        ec = error::validate1;
     }
-
-    chain::context ctx{};
-    if (!query.get_context(ctx, link))
+    else if (!query.get_context(ctx, link))
     {
-        POST(complete_block, error::validate2, link, zero);
-        return;
+        ec = error::validate2;
     }
-
-    if (!block->populate())
+    else if (!block->populate())
     {
-        // input.metadata.locked set invalid for relative locktime (bip68).
-        // Otherwise internal spends do not require confirmability checks.
-        POST(complete_block, error::validate3, link, ctx.height);
-        return;
+        ec = system::error::relative_time_locked;
     }
     else if (!query.populate(*block))
     {
-        // Prepopulated prevouts are bypassed here, including metadata.
-        // .input.metadata.parent (tx link) and .coinbase (bool) are set here.
-        POST(complete_block, error::validate4, link, ctx.height);
-        return;
+        ec = system::error::missing_previous_output;
     }
-
-    code ec{};
-    if (((ec = block->accept(ctx, subsidy_interval_, initial_subsidy_))) ||
-        ((ec = block->connect(ctx))))
+    else if ((ec = block->accept(ctx, subsidy_interval_, initial_subsidy_)))
+    {
+        if (!query.set_block_unconfirmable(link))
+            ec = error::validate3;
+    }
+    else if ((ec = block->connect(ctx)))
     {
         if (!query.set_block_unconfirmable(link))
-            ec = error::validate5;
+            ec = error::validate4;
     }
     else if (!query.set_block_valid(link, block->fees()))
+    {
+        ec = error::validate5;
+    }
+    else if (!query.set_prevouts(ctx.height, *block))
     {
         ec = error::validate6;
     }
+    else if (query.set_filter_body(link, *block))
+    {
+        ec = error::validate7;
+    }
     else
     {
-        set_neutrino(link, *block);
-        set_prevouts(ctx.height, *block);
-
         fire(events::block_validated, ctx.height);
     }
 
+    // Return to strand to handle result.
     POST(complete_block, ec, link, ctx.height);
 }
 
@@ -268,7 +267,14 @@ void chaser_validate::complete_block(const code& ec, const header_link& link,
 
     if (ec)
     {
-        if (ec == error::validate7)
+        // Differentiated fault codes for troubleshooting.
+        if (ec == error::validate1 ||
+            ec == error::validate2 ||
+            ec == error::validate3 ||
+            ec == error::validate4 ||
+            ec == error::validate5 ||
+            ec == error::validate6 ||
+            ec == error::validate7)
         {
             fault(ec);
             return;
@@ -287,36 +293,6 @@ void chaser_validate::complete_block(const code& ec, const header_link& link,
         handle_event(ec, chase::bump, height_t{});
 }
 
-// setters
-// ----------------------------------------------------------------------------
-// unstranded (concurrent by block)
-
-bool chaser_validate::set_neutrino(const header_link& link,
-    const chain::block& block) NOEXCEPT
-{
-    if (!filters_)
-        return true;
-
-    // Avoid computing the filter if already stored.
-    auto& query = archive();
-    if (!query.to_filter(link).is_terminal())
-        return true;
-
-    // Only fails if prevouts are not fully populated.
-    data_chunk filter{};
-    if (!neutrino::compute_filter(filter, block))
-        return false;
-
-    return query.set_filter_body(link, filter);
-}
-
-bool chaser_validate::set_prevouts(size_t, const chain::block&) NOEXCEPT
-{
-    // TODO: need to be able to differentiate internal vs. store.
-    // This tells us what to cache, skip internal and set store populated.
-    return {};
-}
-
 // Strand.
 // ----------------------------------------------------------------------------