From ab9f2c6e2b31339eee9f8e811c71512263417f05 Mon Sep 17 00:00:00 2001 From: TheCharlatan 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`. 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 | 1 - src/init.cpp | 4 ++++ src/kernel/checks.cpp | 4 ---- src/kernel/context.cpp | 7 ------- src/kernel/context.h | 1 - src/node/context.cpp | 13 +++++++++++-- src/qt/test/rpcnestedtests.cpp | 3 +++ src/wallet/test/wallet_tests.cpp | 9 ++++----- 8 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index b6f0daaabadbdd..f25eddbd891846 100644 --- a/src/Makefile.am +++ b/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 \ diff --git a/src/init.cpp b/src/init.cpp index b825c8ce217985..69a6d5156f8d8f 100644 --- a/src/init.cpp +++ b/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. diff --git a/src/kernel/checks.cpp b/src/kernel/checks.cpp index bf8a2ec74c0475..9551a58f0ec687 100644 --- a/src/kernel/checks.cpp +++ b/src/kernel/checks.cpp @@ -15,10 +15,6 @@ namespace kernel { util::Result 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 c60f1638d119bb..5138f4d81b9b24 100644 --- a/src/kernel/context.cpp +++ b/src/kernel/context.cpp @@ -5,7 +5,6 @@ #include #include -#include #include #include #include @@ -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 diff --git a/src/kernel/context.h b/src/kernel/context.h index ff4df20473e7c9..3406b47f9560af 100644 --- a/src/kernel/context.h +++ b/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 diff --git a/src/node/context.cpp b/src/node/context.cpp index ca56fa0b86624c..7c3be74e123924 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -18,6 +19,14 @@ #include namespace node { -NodeContext::NodeContext() = default; -NodeContext::~NodeContext() = default; +NodeContext::NodeContext() +{ + ECC_Start(); +} + +NodeContext::~NodeContext() +{ + ECC_Stop(); +} + } // namespace node diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 72e80554255a17..e3ae80b7bcb596 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/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); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 65297054df5b19..a8b8128195fb96 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/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)> f) +void TestLoadWallet(node::NodeContext& node, const std::string& name, DatabaseFormat format, std::function)> 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 wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) { + TestLoadWallet(m_node, name, format, [](std::shared_ptr 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 wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) { + TestLoadWallet(m_node, name, format, [](std::shared_ptr 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 wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) { + TestLoadWallet(m_node, name, format, [](std::shared_ptr wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) { BOOST_CHECK(!wallet->IsAddressPreviouslySpent(PKHash())); BOOST_CHECK(!wallet->IsAddressPreviouslySpent(ScriptHash())); auto requests = wallet->GetAddressReceiveRequests();