Merge bitcoin/bitcoin#25065: [kernel 2c/n] Introduce kernel::Context, encapsulate global init/teardown

d87784a kernel: SanityChecks: Return an error struct (Carl Dong)
265d639 Move init::SanityCheck to kernel::SanityCheck (Carl Dong)
fed085a init: Initialize globals with kernel::Context's life (Carl Dong)
7d03fee kernel: Introduce empty and unused kernel::Context (Carl Dong)
eeb4fc2 test: Use Set/UnsetGlobals in BasicTestingSetup (Carl Dong)

Pull request description:

  The full `init/common.cpp` is dependent on things like ArgsManager (which we wish to remove from libbitcoinkernel in the future) and sanity checks. These aren't necessary for libbitcoinkernel so we only extract the portion that is necessary (namely `init::{Set,Unset}Globals()`.

ACKs for top commit:
  theuni:
    ACK d87784a
  vasild:
    ACK d87784a

Tree-SHA512: cd6b4923ea1865001b5f0caed9a4ff99c198d22bf74154d935dc09a47fda22ebe585ec912398cea69f722454ed1dbb4898faab5a2d02fb4c5e719c5c8d71a3f9

Author: fanquake

src/Makefile.am

@@ -171,7 +171,9 @@ BITCOIN_CORE_H = \
   interfaces/node.h \
   interfaces/wallet.h \
   kernel/chainstatemanager_opts.h \
+  kernel/checks.h \
   kernel/coinstats.h \
+  kernel/context.h \
   key.h \
   key_io.h \
   logging.h \
@@ -356,7 +358,9 @@ libbitcoin_node_a_SOURCES = \
   index/coinstatsindex.cpp \
   index/txindex.cpp \
   init.cpp \
+  kernel/checks.cpp \
   kernel/coinstats.cpp \
+  kernel/context.cpp \
   mapport.cpp \
   net.cpp \
   netgroup.cpp \
@@ -866,8 +870,9 @@ libbitcoinkernel_la_SOURCES = \
   flatfile.cpp \
   fs.cpp \
   hash.cpp \
-  init/common.cpp \
+  kernel/checks.cpp \
   kernel/coinstats.cpp \
+  kernel/context.cpp \
   key.cpp \
   logging.cpp \
   node/blockstorage.cpp \

src/bitcoin-chainstate.cpp

@@ -11,10 +11,12 @@
 //
 // It is part of the libbitcoinkernel project.
 
+#include <kernel/checks.h>
+#include <kernel/context.h>
+
 #include <chainparams.h>
 #include <consensus/validation.h>
 #include <core_io.h>
-#include <init/common.h>
 #include <node/blockstorage.h>
 #include <node/chainstate.h>
 #include <scheduler.h>
@@ -24,6 +26,7 @@
 #include <validation.h>
 #include <validationinterface.h>
 
+#include <cassert>
 #include <filesystem>
 #include <functional>
 #include <iosfwd>
@@ -49,7 +52,11 @@ int main(int argc, char* argv[])
     SelectParams(CBaseChainParams::MAIN);
     const CChainParams& chainparams = Params();
 
-    init::SetGlobals(); // ECC_Start, etc.
+    kernel::Context kernel_context{};
+    // We can't use a goto here, but we can use an assert since none of the
+    // things instantiated so far requires running the epilogue to be torn down
+    // properly
+    assert(!kernel::SanityChecks(kernel_context).has_value());
 
     // Necessary for CheckInputScripts (eventually called by ProcessNewBlock),
     // which will try the script cache first and fall back to actually
@@ -254,6 +261,4 @@ int main(int argc, char* argv[])
         }
     }
     GetMainSignals().UnregisterBackgroundSignalScheduler();
-
-    init::UnsetGlobals();
 }

src/bitcoind.cpp

@@ -188,11 +188,14 @@ static bool AppInit(NodeContext& node, int argc, char* argv[])
             // InitError will have been called with detailed error, which ends up on console
             return false;
         }
-        if (!AppInitSanityChecks())
+
+        node.kernel = std::make_unique<kernel::Context>();
+        if (!AppInitSanityChecks(*node.kernel))
         {
             // InitError will have been called with detailed error, which ends up on console
             return false;
         }
+
         if (args.GetBoolArg("-daemon", DEFAULT_DAEMON) || args.GetBoolArg("-daemonwait", DEFAULT_DAEMONWAIT)) {
 #if HAVE_DECL_FORK
             tfm::format(std::cout, PACKAGE_NAME " starting\n");

src/init.cpp

@@ -9,6 +9,8 @@
 
 #include <init.h>
 
+#include <kernel/checks.h>
+
 #include <addrman.h>
 #include <banman.h>
 #include <blockfilter.h>
@@ -304,7 +306,7 @@ void Shutdown(NodeContext& node)
     node.chain_clients.clear();
     UnregisterAllValidationInterfaces();
     GetMainSignals().UnregisterBackgroundSignalScheduler();
-    init::UnsetGlobals();
+    node.kernel.reset();
     node.mempool.reset();
     node.fee_estimator.reset();
     node.chainman.reset();
@@ -1089,13 +1091,24 @@ static bool LockDataDirectory(bool probeOnly)
     return true;
 }
 
-bool AppInitSanityChecks()
+bool AppInitSanityChecks(const kernel::Context& kernel)
 {
     // ********************************************************* Step 4: sanity checks
+    auto maybe_error = kernel::SanityChecks(kernel);
+
+    if (maybe_error.has_value()) {
+        switch (maybe_error.value()) {
+        case kernel::SanityCheckError::ERROR_ECC:
+            InitError(Untranslated("Elliptic curve cryptography sanity check failure. Aborting."));
+            break;
+        case kernel::SanityCheckError::ERROR_RANDOM:
+            InitError(Untranslated("OS cryptographic RNG sanity check failure. Aborting."));
+            break;
+        case kernel::SanityCheckError::ERROR_CHRONO:
+            InitError(Untranslated("Clock epoch mismatch. Aborting."));
+            break;
+        } // no default case, so the compiler can warn about missing cases
 
-    init::SetGlobals();
-
-    if (!init::SanityChecks()) {
         return InitError(strprintf(_("Initialization sanity check failed. %s is shutting down."), PACKAGE_NAME));
     }
 

src/init.h

@@ -19,6 +19,9 @@ class ArgsManager;
 namespace interfaces {
 struct BlockAndHeaderTipInfo;
 }
+namespace kernel {
+struct Context;
+}
 namespace node {
 struct NodeContext;
 } // namespace node
@@ -47,7 +50,7 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
  * @note This can be done before daemonization. Do not call Shutdown() if this function fails.
  * @pre Parameters should be parsed and config file should be read, AppInitParameterInteraction should have been called.
  */
-bool AppInitSanityChecks();
+bool AppInitSanityChecks(const kernel::Context& kernel);
 /**
  * Lock bitcoin core data directory.
  * @note This should only be done after daemonization. Do not call Shutdown() if this function fails.

src/init/common.cpp

@@ -7,58 +7,19 @@
 #endif
 
 #include <clientversion.h>
-#include <crypto/sha256.h>
 #include <fs.h>
-#include <key.h>
 #include <logging.h>
 #include <node/ui_interface.h>
-#include <pubkey.h>
-#include <random.h>
 #include <tinyformat.h>
 #include <util/system.h>
 #include <util/time.h>
 #include <util/translation.h>
 
 #include <algorithm>
-#include <memory>
 #include <string>
 #include <vector>
 
-static std::unique_ptr<ECCVerifyHandle> globalVerifyHandle;
-
 namespace init {
-void SetGlobals()
-{
-    std::string sha256_algo = SHA256AutoDetect();
-    LogPrintf("Using the '%s' SHA256 implementation\n", sha256_algo);
-    RandomInit();
-    ECC_Start();
-    globalVerifyHandle.reset(new ECCVerifyHandle());
-}
-
-void UnsetGlobals()
-{
-    globalVerifyHandle.reset();
-    ECC_Stop();
-}
-
-bool SanityChecks()
-{
-    if (!ECC_InitSanityCheck()) {
-        return InitError(Untranslated("Elliptic curve cryptography sanity check failure. Aborting."));
-    }
-
-    if (!Random_SanityCheck()) {
-        return InitError(Untranslated("OS cryptographic RNG sanity check failure. Aborting."));
-    }
-
-    if (!ChronoSanityCheck()) {
-        return InitError(Untranslated("Clock epoch mismatch. Aborting."));
-    }
-
-    return true;
-}
-
 void AddLoggingArgs(ArgsManager& argsman)
 {
     argsman.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (-nodebuglogfile to disable; default: %s)", DEFAULT_DEBUGLOGFILE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);

src/init/common.h

@@ -11,13 +11,6 @@
 class ArgsManager;
 
 namespace init {
-void SetGlobals();
-void UnsetGlobals();
-/**
- *  Ensure a usable environment with all
- *  necessary library support.
- */
-bool SanityChecks();
 void AddLoggingArgs(ArgsManager& args);
 void SetLoggingOptions(const ArgsManager& args);
 void SetLoggingCategories(const ArgsManager& args);

src/kernel/checks.cpp

@@ -0,0 +1,30 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <kernel/checks.h>
+
+#include <key.h>
+#include <random.h>
+#include <util/time.h>
+
+namespace kernel {
+
+std::optional<SanityCheckError> SanityChecks(const Context&)
+{
+    if (!ECC_InitSanityCheck()) {
+        return SanityCheckError::ERROR_ECC;
+    }
+
+    if (!Random_SanityCheck()) {
+        return SanityCheckError::ERROR_RANDOM;
+    }
+
+    if (!ChronoSanityCheck()) {
+        return SanityCheckError::ERROR_CHRONO;
+    }
+
+    return std::nullopt;
+}
+
+}

src/kernel/checks.h

@@ -0,0 +1,27 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_KERNEL_CHECKS_H
+#define BITCOIN_KERNEL_CHECKS_H
+
+#include <optional>
+
+namespace kernel {
+
+struct Context;
+
+enum class SanityCheckError {
+    ERROR_ECC,
+    ERROR_RANDOM,
+    ERROR_CHRONO,
+};
+
+/**
+ *  Ensure a usable environment with all necessary library support.
+ */
+std::optional<SanityCheckError> SanityChecks(const Context&);
+
+}
+
+#endif // BITCOIN_KERNEL_CHECKS_H

src/kernel/context.cpp

@@ -0,0 +1,33 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <kernel/context.h>
+
+#include <crypto/sha256.h>
+#include <key.h>
+#include <logging.h>
+#include <pubkey.h>
+#include <random.h>
+
+#include <string>
+
+
+namespace kernel {
+
+Context::Context()
+{
+    std::string sha256_algo = SHA256AutoDetect();
+    LogPrintf("Using the '%s' SHA256 implementation\n", sha256_algo);
+    RandomInit();
+    ECC_Start();
+    ecc_verify_handle.reset(new ECCVerifyHandle());
+}
+
+Context::~Context()
+{
+    ecc_verify_handle.reset();
+    ECC_Stop();
+}
+
+} // namespace kernel