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`. So move the `ECC_Start` call to the `NodeContext` ctor
instead to completely remove the key module from the kernel library.

The gui tests currently keep multiple `NodeContext` objects in memory,
so relax the `secp256k1_context_sign` check in `ECC_Start` to instead
return early if it has already been initialized.

src/Makefile.am

@@ -944,7 +944,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 \

src/init.cpp

@@ -1073,6 +1073,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.

src/kernel/checks.cpp

@@ -15,10 +15,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.")};
     }

src/kernel/context.cpp

@@ -5,7 +5,6 @@
 #include <kernel/context.h>
 
 #include <crypto/sha256.h>
-#include <key.h>
 #include <logging.h>
 #include <pubkey.h>
 #include <random.h>
@@ -19,12 +18,6 @@ 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

src/kernel/context.h

@@ -19,7 +19,6 @@ namespace kernel {
 //! should be stored to std::unique_ptr members pointing to opaque types.
 struct Context {
     Context();
-    ~Context();
 };
 } // namespace kernel
 

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>
@@ -18,6 +19,14 @@
 #include <validation.h>
 
 namespace node {
-NodeContext::NodeContext() = default;
-NodeContext::~NodeContext() = default;
+NodeContext::NodeContext()
+{
+    ECC_Start();
+}
+
+NodeContext::~NodeContext()
+{
+    ECC_Stop();
+}
+
 } // namespace node

src/qt/test/rpcnestedtests.cpp

@@ -46,6 +46,9 @@ void RPCNestedTests::rpcNestedTests()
         tableRPC.appendCommand(c.name, &c);
     }
 
+    // Call ECC_Stop before instantiating another NodeContext in TestingSetup
+    ECC_Stop();
+
     TestingSetup test;
     m_node.setContext(&test.m_node);
 

src/wallet/test/wallet_tests.cpp

@@ -409,9 +409,8 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart)
     BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 5, 50, 600), 300);
 }
 
-void TestLoadWallet(const std::string& name, DatabaseFormat format, std::function<void(std::shared_ptr<CWallet>)> f)
+void TestLoadWallet(node::NodeContext& node, const std::string& name, DatabaseFormat format, std::function<void(std::shared_ptr<CWallet>)> f)
 {
-    node::NodeContext node;
     auto chain{interfaces::MakeChain(node)};
     DatabaseOptions options;
     options.require_format = format;
@@ -428,7 +427,7 @@ BOOST_FIXTURE_TEST_CASE(LoadReceiveRequests, TestingSetup)
 {
     for (DatabaseFormat format : DATABASE_FORMATS) {
         const std::string name{strprintf("receive-requests-%i", format)};
-        TestLoadWallet(name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
+        TestLoadWallet(m_node, name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
             BOOST_CHECK(!wallet->IsAddressPreviouslySpent(PKHash()));
             WalletBatch batch{wallet->GetDatabase()};
             BOOST_CHECK(batch.WriteAddressPreviouslySpent(PKHash(), true));
@@ -439,7 +438,7 @@ BOOST_FIXTURE_TEST_CASE(LoadReceiveRequests, TestingSetup)
             BOOST_CHECK(wallet->SetAddressReceiveRequest(batch, PKHash(), "1", "val_rr11"));
             BOOST_CHECK(wallet->SetAddressReceiveRequest(batch, ScriptHash(), "2", "val_rr20"));
         });
-        TestLoadWallet(name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
+        TestLoadWallet(m_node, name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
             BOOST_CHECK(wallet->IsAddressPreviouslySpent(PKHash()));
             BOOST_CHECK(wallet->IsAddressPreviouslySpent(ScriptHash()));
             auto requests = wallet->GetAddressReceiveRequests();
@@ -449,7 +448,7 @@ BOOST_FIXTURE_TEST_CASE(LoadReceiveRequests, TestingSetup)
             BOOST_CHECK(batch.WriteAddressPreviouslySpent(PKHash(), false));
             BOOST_CHECK(batch.EraseAddressData(ScriptHash()));
         });
-        TestLoadWallet(name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
+        TestLoadWallet(m_node, name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
             BOOST_CHECK(!wallet->IsAddressPreviouslySpent(PKHash()));
             BOOST_CHECK(!wallet->IsAddressPreviouslySpent(ScriptHash()));
             auto requests = wallet->GetAddressReceiveRequests();