kernel: Remove StartShutdown calls from validation code

This change drops the last kernel dependency on shutdown.cpp. It also adds new
hooks for libbitcoinkernel applications to be able to interrupt kernel
operations when the chain tip changes.

This is a refactoring that does not affect behavior. (Looking at the code it
can appear like the new break statement in the ActivateBestChain function is a
change in behavior, but actually the previous StartShutdown call was indirectly
triggering a break before, because it was causing m_chainman.m_interrupt to be
true. The new code just makes the break more obvious.)

src/Makefile.am

@@ -956,7 +956,6 @@ libbitcoinkernel_la_SOURCES = \
   script/script_error.cpp \
   script/sigcache.cpp \
   script/standard.cpp \
-  shutdown.cpp \
   signet.cpp \
   support/cleanse.cpp \
   support/lockedpool.cpp \

src/bitcoin-chainstate.cpp

@@ -81,9 +81,10 @@ int main(int argc, char* argv[])
     class KernelNotifications : public kernel::Notifications
     {
     public:
-        void blockTip(SynchronizationState, CBlockIndex&) override
+        kernel::InterruptResult blockTip(SynchronizationState, CBlockIndex&) override
         {
             std::cout << "Block tip changed" << std::endl;
+            return {};
         }
         void headerTip(SynchronizationState, int64_t height, int64_t timestamp, bool presync) override
         {

src/init.cpp

@@ -123,6 +123,7 @@ using node::CacheSizes;
 using node::CalculateCacheSizes;
 using node::DEFAULT_PERSIST_MEMPOOL;
 using node::DEFAULT_PRINTPRIORITY;
+using node::DEFAULT_STOPATHEIGHT;
 using node::fReindex;
 using node::KernelNotifications;
 using node::LoadChainstate;
@@ -1408,6 +1409,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // ********************************************************* Step 7: load block chain
 
     node.notifications = std::make_unique<KernelNotifications>(node.exit_status);
+    ReadNotificationArgs(args, *node.notifications);
     fReindex = args.GetBoolArg("-reindex", false);
     bool fReindexChainState = args.GetBoolArg("-reindex-chainstate", false);
     ChainstateManager::Options chainman_opts{

src/kernel/chainstatemanager_opts.h

@@ -21,7 +21,6 @@ class CChainParams;
 
 static constexpr bool DEFAULT_CHECKPOINTS_ENABLED{true};
 static constexpr auto DEFAULT_MAX_TIP_AGE{24h};
-static constexpr int DEFAULT_STOPATHEIGHT{0};
 
 namespace kernel {
 
@@ -46,7 +45,6 @@ struct ChainstateManagerOpts {
     DBOptions coins_db{};
     CoinsViewOptions coins_view{};
     Notifications& notifications;
-    int stop_at_height{DEFAULT_STOPATHEIGHT};
 };
 
 } // namespace kernel

src/kernel/notifications_interface.h

@@ -9,12 +9,25 @@
 
 #include <cstdint>
 #include <string>
+#include <variant>
 
 class CBlockIndex;
 enum class SynchronizationState;
 
 namespace kernel {
 
+//! Result type for use with std::variant to indicate that an operation should be interrupted.
+struct Interrupted{};
+
+//! Simple result type for functions that need to propagate an interrupt status and don't have other return values.
+using InterruptResult = std::variant<std::monostate, Interrupted>;
+
+template <typename T>
+bool IsInterrupted(const T& result)
+{
+    return std::holds_alternative<kernel::Interrupted>(result);
+}
+
 /**
  * A base class defining functions for notifying about certain kernel
  * events.
@@ -24,7 +37,7 @@ class Notifications
 public:
     virtual ~Notifications(){};
 
-    virtual void blockTip(SynchronizationState state, CBlockIndex& index) {}
+    [[nodiscard]] virtual InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) { return {}; }
     virtual void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) {}
     virtual void progress(const bilingual_str& title, int progress_percent, bool resume_possible) {}
     virtual void warning(const bilingual_str& warning) {}

src/node/chainstatemanager_args.cpp

@@ -37,8 +37,6 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
 
     if (auto value{args.GetIntArg("-maxtipage")}) opts.max_tip_age = std::chrono::seconds{*value};
 
-    if (auto value{args.GetIntArg("-stopatheight")}) opts.stop_at_height = *value;
-
     ReadDatabaseArgs(args, opts.block_tree_db);
     ReadDatabaseArgs(args, opts.coins_db);
     ReadCoinsViewArgs(args, opts.coins_view);

src/node/kernel_notifications.cpp

@@ -8,6 +8,7 @@
 #include <config/bitcoin-config.h>
 #endif
 
+#include <chain.h>
 #include <common/args.h>
 #include <common/system.h>
 #include <kernel/context.h>
@@ -57,9 +58,14 @@ static void DoWarning(const bilingual_str& warning)
 
 namespace node {
 
-void KernelNotifications::blockTip(SynchronizationState state, CBlockIndex& index)
+kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state, CBlockIndex& index)
 {
     uiInterface.NotifyBlockTip(state, &index);
+    if (m_stop_at_height && index.nHeight >= m_stop_at_height) {
+        StartShutdown();
+        return kernel::Interrupted{};
+    }
+    return {};
 }
 
 void KernelNotifications::headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync)
@@ -87,4 +93,9 @@ void KernelNotifications::fatalError(const std::string& debug_message, const bil
     node::AbortNode(m_exit_status, debug_message, user_message, m_shutdown_on_fatal_error);
 }
 
+void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications)
+{
+    if (auto value{args.GetIntArg("-stopatheight")}) notifications.m_stop_at_height = *value;
+}
+
 } // namespace node

src/node/kernel_notifications.h

@@ -11,17 +11,21 @@
 #include <cstdint>
 #include <string>
 
+class ArgsManager;
 class CBlockIndex;
 enum class SynchronizationState;
 struct bilingual_str;
 
 namespace node {
+
+static constexpr int DEFAULT_STOPATHEIGHT{0};
+
 class KernelNotifications : public kernel::Notifications
 {
 public:
     KernelNotifications(std::atomic<int>& exit_status) : m_exit_status{exit_status} {}
 
-    void blockTip(SynchronizationState state, CBlockIndex& index) override;
+    [[nodiscard]] kernel::InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) override;
 
     void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) override;
 
@@ -33,11 +37,16 @@ public:
 
     void fatalError(const std::string& debug_message, const bilingual_str& user_message = {}) override;
 
+    //! Block height after which blockTip notification will return Interrupted{}, if >0.
+    int m_stop_at_height{DEFAULT_STOPATHEIGHT};
     //! Useful for tests, can be set to false to avoid shutdown on fatal error.
     bool m_shutdown_on_fatal_error{true};
 private:
     std::atomic<int>& m_exit_status;
 };
+
+void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications);
+
 } // namespace node
 
 #endif // BITCOIN_NODE_KERNEL_NOTIFICATIONS_H

src/validation.cpp

@@ -37,7 +37,6 @@
 #include <reverse_iterator.h>
 #include <script/script.h>
 #include <script/sigcache.h>
-#include <shutdown.h>
 #include <signet.h>
 #include <tinyformat.h>
 #include <txdb.h>
@@ -3172,13 +3171,17 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
                 GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload);
 
                 // Always notify the UI if a new block tip was connected
-                m_chainman.GetNotifications().blockTip(GetSynchronizationState(fInitialDownload), *pindexNewTip);
+                if (kernel::IsInterrupted(m_chainman.GetNotifications().blockTip(GetSynchronizationState(fInitialDownload), *pindexNewTip))) {
+                    // Just breaking and returning success for now. This could
+                    // be changed to bubble up the kernel::Interrupted value to
+                    // the caller so the caller could distinguish between
+                    // completed and interrupted operations.
+                    break;
+                }
             }
         }
         // When we reach this point, we switched to a new tip (stored in pindexNewTip).
 
-        if (m_chainman.StopAtHeight() && pindexNewTip && pindexNewTip->nHeight >= m_chainman.StopAtHeight()) StartShutdown();
-
         if (WITH_LOCK(::cs_main, return m_disabled)) {
             // Background chainstate has reached the snapshot base block, so exit.
             break;
@@ -3369,7 +3372,14 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
 
     // Only notify about a new block tip if the active chain was modified.
     if (pindex_was_in_chain) {
-        m_chainman.GetNotifications().blockTip(GetSynchronizationState(IsInitialBlockDownload()), *to_mark_failed->pprev);
+        // Ignoring return value for now, this could be changed to bubble up
+        // kernel::Interrupted value to the caller so the caller could
+        // distinguish between completed and interrupted operations. It might
+        // also make sense for the blockTip notification to have an enum
+        // parameter indicating the source of the tip change so hooks can
+        // distinguish user-initiated invalidateblock changes from other
+        // changes.
+        (void)m_chainman.GetNotifications().blockTip(GetSynchronizationState(IsInitialBlockDownload()), *to_mark_failed->pprev);
     }
     return true;
 }

src/validation.h

@@ -23,7 +23,6 @@
 #include <policy/packages.h>
 #include <policy/policy.h>
 #include <script/script_error.h>
-#include <shutdown.h>
 #include <sync.h>
 #include <txdb.h>
 #include <txmempool.h> // For CTxMemPool::cs
@@ -970,7 +969,6 @@ public:
     const arith_uint256& MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); }
     const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); }
     kernel::Notifications& GetNotifications() const { return m_options.notifications; };
-    int StopAtHeight() const { return m_options.stop_at_height; };
 
     /**
      * Alias for ::cs_main.