From 41eba5bd716bea47c8731d156d053afee92a7f12 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Mon, 15 Jan 2024 12:55:31 +0100
Subject: [PATCH] kernel: Remove key module from kernel library

The key module's functionality is not used by the kernel library, but
currently kernel users are still required to initialize the key module's
`secp256k1_context_sign` global as part of the `kernel::Context` through
`ECC_Start`.
---
 src/Makefile.am                   |  1 -
 src/bitcoin-chainstate.cpp        |  1 +
 src/bitcoind.cpp                  |  3 +++
 src/init.cpp                      |  8 ++++++++
 src/kernel/checks.cpp             |  6 +-----
 src/kernel/context.cpp            |  7 -------
 src/kernel/context.h              |  5 -----
 src/node/context.cpp              |  2 ++
 src/node/context.h                | 11 ++++++++---
 src/node/interfaces.cpp           |  2 ++
 src/node/kernel_notifications.cpp |  1 +
 src/rpc/mining.cpp                |  1 +
 src/test/util/setup_common.cpp    |  2 ++
 src/test/util/setup_common.h      |  2 ++
 14 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 639aecf3b35..669bf403629 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -945,7 +945,6 @@ libbitcoinkernel_la_SOURCES = \
   kernel/disconnected_transactions.cpp \
   kernel/mempool_persist.cpp \
   kernel/mempool_removal_reason.cpp \
-  key.cpp \
   logging.cpp \
   node/blockstorage.cpp \
   node/chainstate.cpp \
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 642af06e82d..49276342333 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -26,6 +26,7 @@
 #include <script/sigcache.h>
 #include <util/chaintype.h>
 #include <util/fs.h>
+#include <util/signalinterrupt.h>
 #include <util/task_runner.h>
 #include <validation.h>
 #include <validationinterface.h>
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index 7685bdea79c..0b89aa42afe 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -14,11 +14,13 @@
 #include <init.h>
 #include <interfaces/chain.h>
 #include <interfaces/init.h>
+#include <kernel/context.h>
 #include <node/context.h>
 #include <node/interface_ui.h>
 #include <noui.h>
 #include <util/check.h>
 #include <util/exception.h>
+#include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 #include <util/syserror.h>
 #include <util/threadnames.h>
@@ -180,6 +182,7 @@ static bool AppInit(NodeContext& node)
         }
 
         node.kernel = std::make_unique<kernel::Context>();
+        node.ecc_context = std::make_unique<ECC_Context>();
         if (!AppInitSanityChecks(*node.kernel))
         {
             // InitError will have been called with detailed error, which ends up on console
diff --git a/src/init.cpp b/src/init.cpp
index fbf25a0341f..5895883ad08 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -32,6 +32,8 @@
 #include <interfaces/chain.h>
 #include <interfaces/init.h>
 #include <interfaces/node.h>
+#include <kernel/context.h>
+#include <key.h>
 #include <logging.h>
 #include <mapport.h>
 #include <net.h>
@@ -75,6 +77,7 @@
 #include <util/fs_helpers.h>
 #include <util/moneystr.h>
 #include <util/result.h>
+#include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/syserror.h>
@@ -371,6 +374,7 @@ void Shutdown(NodeContext& node)
     node.chainman.reset();
     node.validation_signals.reset();
     node.scheduler.reset();
+    node.ecc_context.reset();
     node.kernel.reset();
 
     RemovePidFile(*node.args);
@@ -1084,6 +1088,10 @@ bool AppInitSanityChecks(const kernel::Context& kernel)
         return InitError(strprintf(_("Initialization sanity check failed. %s is shutting down."), PACKAGE_NAME));
     }
 
+    if (!ECC_InitSanityCheck()) {
+        return InitError(strprintf(_("Elliptic curve cryptography sanity check failure. %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,
     // and a fork will cause weird behavior to it.
diff --git a/src/kernel/checks.cpp b/src/kernel/checks.cpp
index 45a5e25093f..e4a13ee4cc2 100644
--- a/src/kernel/checks.cpp
+++ b/src/kernel/checks.cpp
@@ -4,8 +4,8 @@
 
 #include <kernel/checks.h>
 
-#include <key.h>
 #include <random.h>
+#include <util/result.h>
 #include <util/translation.h>
 
 #include <memory>
@@ -14,10 +14,6 @@ namespace kernel {
 
 util::Result<void> SanityChecks(const Context&)
 {
-    if (!ECC_InitSanityCheck()) {
-        return util::Error{Untranslated("Elliptic curve cryptography sanity check failure. Aborting.")};
-    }
-
     if (!Random_SanityCheck()) {
         return util::Error{Untranslated("OS cryptographic RNG sanity check failure. Aborting.")};
     }
diff --git a/src/kernel/context.cpp b/src/kernel/context.cpp
index c60f1638d11..bfb17915fd6 100644
--- a/src/kernel/context.cpp
+++ b/src/kernel/context.cpp
@@ -5,9 +5,7 @@
 #include <kernel/context.h>
 
 #include <crypto/sha256.h>
-#include <key.h>
 #include <logging.h>
-#include <pubkey.h>
 #include <random.h>
 
 #include <string>
@@ -19,12 +17,7 @@ Context::Context()
     std::string sha256_algo = SHA256AutoDetect();
     LogPrintf("Using the '%s' SHA256 implementation\n", sha256_algo);
     RandomInit();
-    ECC_Start();
 }
 
-Context::~Context()
-{
-    ECC_Stop();
-}
 
 } // namespace kernel
diff --git a/src/kernel/context.h b/src/kernel/context.h
index ff4df20473e..159af126896 100644
--- a/src/kernel/context.h
+++ b/src/kernel/context.h
@@ -5,10 +5,6 @@
 #ifndef BITCOIN_KERNEL_CONTEXT_H
 #define BITCOIN_KERNEL_CONTEXT_H
 
-#include <util/signalinterrupt.h>
-
-#include <memory>
-
 namespace kernel {
 //! Context struct holding the kernel library's logically global state, and
 //! passed to external libbitcoin_kernel functions which need access to this
@@ -19,7 +15,6 @@ namespace kernel {
 //! should be stored to std::unique_ptr members pointing to opaque types.
 struct Context {
     Context();
-    ~Context();
 };
 } // namespace kernel
 
diff --git a/src/node/context.cpp b/src/node/context.cpp
index ca56fa0b866..e32d21b3833 100644
--- a/src/node/context.cpp
+++ b/src/node/context.cpp
@@ -8,6 +8,7 @@
 #include <banman.h>
 #include <interfaces/chain.h>
 #include <kernel/context.h>
+#include <key.h>
 #include <net.h>
 #include <net_processing.h>
 #include <netgroup.h>
@@ -16,6 +17,7 @@
 #include <scheduler.h>
 #include <txmempool.h>
 #include <validation.h>
+#include <validationinterface.h>
 
 namespace node {
 NodeContext::NodeContext() = default;
diff --git a/src/node/context.h b/src/node/context.h
index 245f230aece..a7d92989ddf 100644
--- a/src/node/context.h
+++ b/src/node/context.h
@@ -5,10 +5,7 @@
 #ifndef BITCOIN_NODE_CONTEXT_H
 #define BITCOIN_NODE_CONTEXT_H
 
-#include <kernel/context.h>
-
 #include <atomic>
-#include <cassert>
 #include <cstdlib>
 #include <functional>
 #include <memory>
@@ -24,6 +21,7 @@ class ValidationSignals;
 class CScheduler;
 class CTxMemPool;
 class ChainstateManager;
+class ECC_Context;
 class NetGroupManager;
 class PeerManager;
 namespace interfaces {
@@ -32,6 +30,12 @@ class ChainClient;
 class Init;
 class WalletLoader;
 } // namespace interfaces
+namespace kernel {
+struct Context;
+}
+namespace util {
+class SignalInterrupt;
+}
 
 namespace node {
 class KernelNotifications;
@@ -49,6 +53,7 @@ class KernelNotifications;
 struct NodeContext {
     //! libbitcoin_kernel context
     std::unique_ptr<kernel::Context> kernel;
+    std::unique_ptr<ECC_Context> ecc_context;
     //! Init interface for initializing current process and connecting to other processes.
     interfaces::Init* init{nullptr};
     //! Interrupt object used to track whether node shutdown was requested.
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 84fc92e69d6..c886357a349 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -17,6 +17,7 @@
 #include <interfaces/node.h>
 #include <interfaces/wallet.h>
 #include <kernel/chain.h>
+#include <kernel/context.h>
 #include <kernel/mempool_entry.h>
 #include <logging.h>
 #include <mapport.h>
@@ -99,6 +100,7 @@ public:
         if (!AppInitParameterInteraction(args())) return false;
 
         m_context->kernel = std::make_unique<kernel::Context>();
+        m_context->ecc_context = std::make_unique<ECC_Context>();
         if (!AppInitSanityChecks(*m_context->kernel)) return false;
 
         if (!AppInitLockDataDirectory()) return false;
diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp
index 6578b383f7a..e326d4a1f20 100644
--- a/src/node/kernel_notifications.cpp
+++ b/src/node/kernel_notifications.cpp
@@ -14,6 +14,7 @@
 #include <node/abort.h>
 #include <node/interface_ui.h>
 #include <util/check.h>
+#include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/translation.h>
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 73987669a51..2391392bd75 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -31,6 +31,7 @@
 #include <script/signingprovider.h>
 #include <txmempool.h>
 #include <univalue.h>
+#include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/time.h>
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 0fb5e4eb45e..3b890c09db3 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -183,6 +183,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
     AppInitParameterInteraction(*m_node.args);
     LogInstance().StartLogging();
     m_node.kernel = std::make_unique<kernel::Context>();
+    m_node.ecc_context = std::make_unique<ECC_Context>();
     SetupEnvironment();
 
     ValidationCacheSizes validation_cache_sizes{};
@@ -200,6 +201,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
 
 BasicTestingSetup::~BasicTestingSetup()
 {
+    m_node.ecc_context.reset();
     m_node.kernel.reset();
     SetMockTime(0s); // Reset mocktime for following tests
     LogInstance().DisconnectTestLogger();
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index 8ccf9b571c8..dbd66e3585a 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -6,6 +6,7 @@
 #define BITCOIN_TEST_UTIL_SETUP_COMMON_H
 
 #include <common/args.h> // IWYU pragma: export
+#include <kernel/context.h>
 #include <key.h>
 #include <node/caches.h>
 #include <node/context.h> // IWYU pragma: export
@@ -15,6 +16,7 @@
 #include <util/chaintype.h> // IWYU pragma: export
 #include <util/check.h>
 #include <util/fs.h>
+#include <util/signalinterrupt.h>
 #include <util/string.h>
 #include <util/vector.h>