init: Initialize globals with kernel::Context's life

...instead of explicitly calling init::{Set,Unset}Globals.

Cool thing about this is that in both the testing and bitcoin-chainstate
codepaths, we no longer need to explicitly unset globals. The
kernel::Context goes out of scope and the globals are unset
"automatically".

Also construct kernel::Context outside of AppInitSanityChecks()

Author: dongcarl

src/Makefile.am

@@ -357,6 +357,7 @@ libbitcoin_node_a_SOURCES = \
   index/txindex.cpp \
   init.cpp \
   kernel/coinstats.cpp \
+  kernel/context.cpp \
   mapport.cpp \
   net.cpp \
   netgroup.cpp \
@@ -865,8 +866,8 @@ libbitcoinkernel_la_SOURCES = \
   flatfile.cpp \
   fs.cpp \
   hash.cpp \
-  init/common.cpp \
   kernel/coinstats.cpp \
+  kernel/context.cpp \
   key.cpp \
   logging.cpp \
   node/blockstorage.cpp \

src/bitcoin-chainstate.cpp

@@ -11,6 +11,8 @@
 //
 // It is part of the libbitcoinkernel project.
 
+#include <kernel/context.h>
+
 #include <chainparams.h>
 #include <consensus/validation.h>
 #include <core_io.h>
@@ -49,7 +51,7 @@ int main(int argc, char* argv[])
     SelectParams(CBaseChainParams::MAIN);
     const CChainParams& chainparams = Params();
 
-    init::SetGlobals(); // ECC_Start, etc.
+    kernel::Context kernel_context{};
 
     // Necessary for CheckInputScripts (eventually called by ProcessNewBlock),
     // which will try the script cache first and fall back to actually
@@ -254,6 +256,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;
         }
+
+        node.kernel = std::make_unique<kernel::Context>();
         if (!AppInitSanityChecks())
         {
             // 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

@@ -304,7 +304,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();
@@ -1092,9 +1092,6 @@ static bool LockDataDirectory(bool probeOnly)
 bool AppInitSanityChecks()
 {
     // ********************************************************* Step 4: sanity checks
-
-    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

src/init/common.cpp

@@ -7,41 +7,21 @@
 #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()) {

src/init/common.h

@@ -11,8 +11,6 @@
 class ArgsManager;
 
 namespace init {
-void SetGlobals();
-void UnsetGlobals();
 /**
  *  Ensure a usable environment with all
  *  necessary library support.

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

src/kernel/context.h

@@ -5,6 +5,10 @@
 #ifndef BITCOIN_KERNEL_CONTEXT_H
 #define BITCOIN_KERNEL_CONTEXT_H
 
+#include <memory>
+
+class ECCVerifyHandle;
+
 namespace kernel {
 //! Context struct holding the kernel library's logically global state, and
 //! passed to external libbitcoin_kernel functions which need access to this
@@ -14,6 +18,13 @@ namespace kernel {
 //! State stored directly in this struct should be simple. More complex state
 //! should be stored to std::unique_ptr members pointing to opaque types.
 struct Context {
+    std::unique_ptr<ECCVerifyHandle> ecc_verify_handle;
+
+    //! Declare default constructor and destructor that are not inline, so code
+    //! instantiating the kernel::Context struct doesn't need to #include class
+    //! definitions for all the unique_ptr members.
+    Context();
+    ~Context();
 };
 } // namespace kernel
 

src/node/context.cpp

@@ -7,6 +7,7 @@
 #include <addrman.h>
 #include <banman.h>
 #include <interfaces/chain.h>
+#include <kernel/context.h>
 #include <net.h>
 #include <net_processing.h>
 #include <netgroup.h>

src/node/interfaces.cpp

@@ -90,8 +90,16 @@ class NodeImpl : public Node
     uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
     bool baseInitialize() override
     {
-        return AppInitBasicSetup(gArgs) && AppInitParameterInteraction(gArgs, /*use_syscall_sandbox=*/false) && AppInitSanityChecks() &&
-               AppInitLockDataDirectory() && AppInitInterfaces(*m_context);
+        if (!AppInitBasicSetup(gArgs)) return false;
+        if (!AppInitParameterInteraction(gArgs, /*use_syscall_sandbox=*/false)) return false;
+
+        m_context->kernel = std::make_unique<kernel::Context>();
+        if (!AppInitSanityChecks()) return false;
+
+        if (!AppInitLockDataDirectory()) return false;
+        if (!AppInitInterfaces(*m_context)) return false;
+
+        return true;
     }
     bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override
     {

src/test/util/setup_common.cpp

@@ -126,7 +126,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
     InitLogging(*m_node.args);
     AppInitParameterInteraction(*m_node.args);
     LogInstance().StartLogging();
-    init::SetGlobals();
+    m_node.kernel = std::make_unique<kernel::Context>();
     SetupEnvironment();
     SetupNetworking();
     InitSignatureCache();
@@ -146,7 +146,6 @@ BasicTestingSetup::~BasicTestingSetup()
     LogInstance().DisconnectTestLogger();
     fs::remove_all(m_path_root);
     gArgs.ClearArgs();
-    init::UnsetGlobals();
 }
 
 ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::vector<const char*>& extra_args)

src/test/util/setup_common.h

@@ -81,7 +81,7 @@ static constexpr CAmount CENT{1000000};
  * This just configures logging, data dir and chain parameters.
  */
 struct BasicTestingSetup {
-    node::NodeContext m_node;
+    node::NodeContext m_node; // keep as first member to be destructed last
 
     explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN, const std::vector<const char*>& extra_args = {});
     ~BasicTestingSetup();