kernel: Move sanity check to context constructor

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
sanity check error.

src/bitcoin-chainstate.cpp

@@ -27,6 +27,7 @@
 #include <script/sigcache.h>
 #include <util/chaintype.h>
 #include <util/fs.h>
+#include <util/signalinterrupt.h>
 #include <util/thread.h>
 #include <validation.h>
 #include <validationinterface.h>
@@ -55,11 +56,11 @@ int main(int argc, char* argv[])
 
 
     // SETUP: Context
-    kernel::Context kernel_context{};
+    auto kernel_context{std::move(kernel::Context::MakeContext().value())};
     // 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));
+    assert(kernel::SanityChecks(*kernel_context));
 
     // Necessary for CheckInputScripts (eventually called by ProcessNewBlock),
     // which will try the script cache first and fall back to actually

src/bitcoind.cpp

@@ -183,9 +183,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;
         }

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>
 
@@ -718,6 +717,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)
 {
@@ -1064,14 +1074,9 @@ static bool LockDataDirectory(bool probeOnly)
     assert(false);
 }
 
-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,

src/init.h

@@ -19,9 +19,6 @@ class ArgsManager;
 namespace interfaces {
 struct BlockAndHeaderTipInfo;
 }
-namespace kernel {
-struct Context;
-}
 namespace node {
 struct NodeContext;
 } // namespace node
@@ -39,6 +36,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.
@@ -55,7 +54,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.

src/kernel/context.cpp

@@ -5,21 +5,31 @@
 #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 <memory>
 #include <string>
-
+#include <utility>
 
 namespace kernel {
-Context::Context()
+
+util::Result<std::unique_ptr<Context>> Context::MakeContext()
 {
+    auto context{std::unique_ptr<Context>(new Context())};
     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()

src/kernel/context.h

@@ -5,7 +5,7 @@
 #ifndef BITCOIN_KERNEL_CONTEXT_H
 #define BITCOIN_KERNEL_CONTEXT_H
 
-#include <util/signalinterrupt.h>
+#include <util/result.h>
 
 #include <memory>
 
@@ -17,9 +17,15 @@ 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.
+//!
+//! Methods should not be inline, so code instantiating the kernel::Context struct
+//! doesn't need to #include class definitions for all the unique_ptr members.
 struct Context {
-    Context();
+    static util::Result<std::unique_ptr<Context>> MakeContext();
     ~Context();
+
+private:
+    Context() = default;
 };
 } // namespace kernel
 

src/node/interfaces.cpp

@@ -98,9 +98,8 @@ 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;

src/test/util/setup_common.cpp

@@ -138,7 +138,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();
 
     ValidationCacheSizes validation_cache_sizes{};