Merge pull request #770 from evoskuil/master

evoskuil · web-flow · commit 6b496d5a2b53 · 2025-05-03T12:43:08.000-04:00
Implement candidate reorganization interlock.
diff --git a/console/executor_commands.cpp b/console/executor_commands.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_dumps.cpp b/console/executor_dumps.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_events.cpp b/console/executor_events.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_logging.cpp b/console/executor_logging.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_options.cpp b/console/executor_options.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_runner.cpp b/console/executor_runner.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_scans.cpp b/console/executor_scans.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_store.cpp b/console/executor_store.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_test_reader.cpp b/console/executor_test_reader.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 
diff --git a/console/executor_test_writer.cpp b/console/executor_test_writer.cpp
@@ -25,7 +25,6 @@
 #include <future>
 #include <iostream>
 #include <map>
-#include <mutex>
 #include <boost/format.hpp>
 #include <bitcoin/node.hpp>
 namespace libbitcoin {
diff --git a/include/bitcoin/node/block_arena.hpp b/include/bitcoin/node/block_arena.hpp
@@ -19,7 +19,6 @@
 #ifndef LIBBITCOIN_NODE_BLOCK_ARENA_HPP
 #define LIBBITCOIN_NODE_BLOCK_ARENA_HPP
 
-#include <shared_mutex>
 #include <bitcoin/system.hpp>
 #include <bitcoin/node/define.hpp>
 
diff --git a/include/bitcoin/node/chasers/chaser.hpp b/include/bitcoin/node/chasers/chaser.hpp
@@ -93,6 +93,9 @@ class BCN_API chaser
     /// Reset store disk full condition.
     virtual code reload(const store::event_handler& handler) NOEXCEPT;
 
+    /// Get reorganization lock.
+    virtual lock get_reorganization_lock() NOEXCEPT;
+
     /// Events.
     /// -----------------------------------------------------------------------
 
diff --git a/include/bitcoin/node/define.hpp b/include/bitcoin/node/define.hpp
@@ -22,6 +22,7 @@
 /// Standard includes (do not include directly).
 #include <functional>
 #include <memory>
+#include <mutex>
 #include <utility>
 #include <variant>
 
@@ -57,6 +58,7 @@ typedef std::error_code code;
 typedef std::function<void(const code&, size_t)> organize_handler;
 typedef database::store<database::map> store;
 typedef database::query<store> query;
+typedef std::unique_lock<std::mutex> lock;
 
 /// Work types.
 typedef network::race_all<const code&> job;
diff --git a/include/bitcoin/node/full_node.hpp b/include/bitcoin/node/full_node.hpp
@@ -139,6 +139,10 @@ class BCN_API full_node
     /// Get the memory resource.
     virtual network::memory& get_memory() NOEXCEPT;
 
+    /// Get reorganization lock.
+    /// Used to prevent candidate chain reorganization during confirmation.
+    virtual lock get_reorganization_lock() NOEXCEPT;
+
 protected:
     /// Session attachments.
     /// -----------------------------------------------------------------------
@@ -161,6 +165,7 @@ class BCN_API full_node
 
     // These are thread safe.
     const configuration& config_;
+    std::mutex reorganization_mutex_{};
     memory_controller memory_;
     query& query_;
 
diff --git a/include/bitcoin/node/impl/chasers/chaser_organize.ipp b/include/bitcoin/node/impl/chasers/chaser_organize.ipp
@@ -118,7 +118,6 @@ void CLASS::do_organize(typename Block::cptr block,
 {
     BC_ASSERT(stranded());
 
-    // shared_ptr copy keeps block ref in scope until completion of set_code.
     const auto& hash = block->get_hash();
     const auto& header = get_header(*block);
 
@@ -235,12 +234,16 @@ void CLASS::do_organize(typename Block::cptr block,
 
     // Pop down to the branch point.
     auto index = top_candidate;
-    while (index > branch_point)
+    if (top_candidate > branch_point)
     {
-        if (!set_reorganized(index--))
+        get_reorganization_lock();
+        while (index > branch_point)
         {
-            handler(fault(error::organize5), height);
-            return;
+            if (!set_reorganized(index--))
+            {
+                handler(fault(error::organize5), height);
+                return;
+            }
         }
     }
 
@@ -356,6 +359,7 @@ void CLASS::do_disorganize(header_t link) NOEXCEPT
     // Height and above excluded from tree since they are unconfirmable blocks.
     // ........................................................................
     // Forward order is required to advance chain state for tree.
+    // Can't pop in loop because that requires reverse order.
 
     typename Block::cptr block{};
     for (auto index = add1(fork_point); index < height; ++index)
@@ -374,15 +378,18 @@ void CLASS::do_disorganize(header_t link) NOEXCEPT
 
     // Pop candidates from top candidate down to above fork point.
     // ........................................................................
-    // Can't pop in loop above because state chaining requires forward order.
 
     const auto top_candidate = state_->height();
-    for (auto index = top_candidate; index > fork_point; --index)
+    if (top_candidate > fork_point)
     {
-        if (!set_reorganized(index))
+        get_reorganization_lock();
+        for (auto index = top_candidate; index > fork_point; --index)
         {
-            fault(error::organize11);
-            return;
+            if (!set_reorganized(index))
+            {
+                fault(error::organize11);
+                return;
+            }
         }
     }
 
diff --git a/src/block_arena.cpp b/src/block_arena.cpp
@@ -19,7 +19,6 @@
 #include <bitcoin/node/block_arena.hpp>
 
 #include <algorithm>
-#include <shared_mutex>
 #include <bitcoin/system.hpp>
 
 namespace libbitcoin {
diff --git a/src/chasers/chaser.cpp b/src/chasers/chaser.cpp
@@ -18,6 +18,7 @@
  */
 #include <bitcoin/node/chasers/chaser.hpp>
 
+#include <mutex>
 #include <bitcoin/database.hpp>
 #include <bitcoin/network.hpp>
 #include <bitcoin/node/configuration.hpp>
@@ -82,6 +83,11 @@ code chaser::reload(const store::event_handler& handler) NOEXCEPT
     return node_.reload(handler);
 }
 
+lock chaser::get_reorganization_lock() NOEXCEPT
+{
+    return node_.get_reorganization_lock();
+}
+
 // Events.
 // ----------------------------------------------------------------------------
 
diff --git a/src/chasers/chaser_confirm.cpp b/src/chasers/chaser_confirm.cpp
@@ -159,6 +159,9 @@ void chaser_confirm::do_bumped(height_t height) NOEXCEPT
     if (closed())
         return;
 
+    // Prevents organizer from popping candidates (does not block push).
+    get_reorganization_lock();
+
     // Scan from candidate height to first confirmed, return links and work.
     uint256_t work{};
     header_links fork{};
@@ -291,6 +294,7 @@ void chaser_confirm::organize(header_links& fork, const header_links& popped,
     }
 
     // Prevent stall by posting internal event, avoiding external handlers.
+    // Posts new work, preventing recursion and releasing reorganization lock.
     handle_event(error::success, chase::bump, height_t{});
 }
 
diff --git a/src/full_node.cpp b/src/full_node.cpp
@@ -370,6 +370,11 @@ network::memory& full_node::get_memory() NOEXCEPT
     return memory_;
 }
 
+lock full_node::get_reorganization_lock() NOEXCEPT
+{
+    return lock{ reorganization_mutex_ };
+}
+
 // Session attachments.
 // ----------------------------------------------------------------------------
 
diff --git a/src/parser.cpp b/src/parser.cpp
@@ -125,7 +125,7 @@ parser::parser(system::chain::selection context) NOEXCEPT
     configured.database.prevout_rate = 5;
 
     configured.database.validated_bk_buckets = 850'001;
-    configured.database.validated_bk_size = 3'700'000;
+    configured.database.validated_bk_size = 1'700'000;
     configured.database.validated_bk_rate = 5;
 
     configured.database.validated_tx_buckets = 1;
@@ -863,7 +863,7 @@ options_metadata parser::load_settings() THROWS
     (
         "database.validated_bk_size",
         value<uint64_t>(&configured.database.validated_bk_size),
-        "The minimum allocation of the validated_bk table body, defaults to '3400000'."
+        "The minimum allocation of the validated_bk table body, defaults to '1700000'."
     )
     (
         "database.validated_bk_rate",
