kernel: Move sanity check to context constructor

sedited · sedited · commit d0daf3edf400 · 2023-09-06T08:30:02.000+02:00
This ensures that it is impossible to construct a context that does not
pass the sanity checks.

Also de-duplicate the init error messages in case of a kernel context
initialization error.
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
@@ -184,9 +184,12 @@ static bool AppInit(NodeContext& node)
             return false;
         }
 
-        node.kernel = std::make_unique<kernel::Context>();
-        if (!AppInitSanityChecks(*node.kernel))
-        {
+        if (!InitKernel(node)) {
+            // InitError will have been called with detailed error, which ends up on console
+            return false;
+        }
+
+        if (!AppInitSanityChecks()) {
             // InitError will have been called with detailed error, which ends up on console
             return false;
         }
diff --git a/src/init.cpp b/src/init.cpp
@@ -9,7 +9,6 @@
 
 #include <init.h>
 
-#include <kernel/checks.h>
 #include <kernel/mempool_persist.h>
 #include <kernel/validation_cache_sizes.h>
 
@@ -680,6 +679,17 @@ static bool AppInitServers(NodeContext& node)
     return true;
 }
 
+bool InitKernel(NodeContext& node)
+{
+    auto result{kernel::Context::MakeContext()};
+    if (!result) {
+        return InitError(strprintf(_("Initialization kernel sanity check failed: %s. %s is shutting down."),
+                                   util::ErrorString(result), PACKAGE_NAME));
+    }
+    node.kernel = std::move(result.value());
+    return true;
+}
+
 // Parameter interaction based on rules
 void InitParameterInteraction(ArgsManager& args)
 {
@@ -1026,14 +1036,9 @@ static bool LockDataDirectory(bool probeOnly)
     return true;
 }
 
-bool AppInitSanityChecks(const kernel::Context& kernel)
+bool AppInitSanityChecks()
 {
     // ********************************************************* Step 4: sanity checks
-    auto result{kernel::SanityChecks(kernel)};
-    if (!result) {
-        InitError(util::ErrorString(result));
-        return InitError(strprintf(_("Initialization sanity check failed. %s is shutting down."), PACKAGE_NAME));
-    }
 
     // Probe the data directory lock to give an early error message, if possible
     // We cannot hold the data directory lock here, as the forking for daemon() hasn't yet happened,
diff --git a/src/init.h b/src/init.h
@@ -19,9 +19,6 @@ class ArgsManager;
 namespace interfaces {
 struct BlockAndHeaderTipInfo;
 }
-namespace kernel {
-struct Context;
-}
 namespace node {
 struct NodeContext;
 } // namespace node
@@ -34,6 +31,8 @@ void InitLogging(const ArgsManager& args);
 //!Parameter interaction: change current parameters depending on various rules
 void InitParameterInteraction(ArgsManager& args);
 
+bool InitKernel(node::NodeContext& node);
+
 /** Initialize bitcoin core: Basic context setup.
  *  @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.
@@ -50,7 +49,7 @@ bool AppInitParameterInteraction(const ArgsManager& args);
  * @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(const kernel::Context& kernel);
+bool AppInitSanityChecks();
 /**
  * Lock bitcoin core data directory.
  * @note This should only be done after daemonization. Do not call Shutdown() if this function fails.
diff --git a/src/kernel/context.cpp b/src/kernel/context.cpp
@@ -5,25 +5,35 @@
 #include <kernel/context.h>
 
 #include <crypto/sha256.h>
+#include <kernel/checks.h>
 #include <key.h>
 #include <logging.h>
-#include <pubkey.h>
 #include <random.h>
+#include <util/result.h>
 
+#include <cassert>
+#include <memory>
 #include <string>
-
+#include <utility>
 
 namespace kernel {
 Context* g_context;
 
-Context::Context()
+util::Result<std::unique_ptr<Context>> Context::MakeContext()
 {
     assert(!g_context);
-    g_context = this;
+    auto context{std::unique_ptr<Context>(new Context())};
+    g_context = context.get();
     std::string sha256_algo = SHA256AutoDetect();
     LogPrintf("Using the '%s' SHA256 implementation\n", sha256_algo);
     RandomInit();
     ECC_Start();
+
+    if (auto res{SanityChecks(*context)}; !res) {
+        return util::Error{ErrorString(res)};
+    }
+
+    return {std::move(context)};
 }
 
 Context::~Context()
diff --git a/src/kernel/context.h b/src/kernel/context.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_KERNEL_CONTEXT_H
 #define BITCOIN_KERNEL_CONTEXT_H
 
+#include <util/result.h>
 #include <util/signalinterrupt.h>
 
 #include <memory>
@@ -24,8 +25,11 @@ struct Context {
     //! 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();
+    static util::Result<std::unique_ptr<Context>> MakeContext();
     ~Context();
+
+private:
+    Context() = default;
 };
 
 //! Global pointer to kernel::Context for legacy code. New code should avoid
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
@@ -97,8 +97,9 @@ class NodeImpl : public Node
         if (!AppInitBasicSetup(args(), Assert(context())->exit_status)) return false;
         if (!AppInitParameterInteraction(args())) return false;
 
-        m_context->kernel = std::make_unique<kernel::Context>();
-        if (!AppInitSanityChecks(*m_context->kernel)) return false;
+        if (!InitKernel(*m_context)) return false;
+
+        if (!AppInitSanityChecks()) return false;
 
         if (!AppInitLockDataDirectory()) return false;
         if (!AppInitInterfaces(*m_context)) return false;
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -127,7 +127,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
     InitLogging(*m_node.args);
     AppInitParameterInteraction(*m_node.args);
     LogInstance().StartLogging();
-    m_node.kernel = std::make_unique<kernel::Context>();
+    m_node.kernel = std::move(Assert(kernel::Context::MakeContext()).value());
     SetupEnvironment();
     SetupNetworking();
 
