Skip to content

Commit

Permalink
kernel: Remove key module from kernel library
Browse files Browse the repository at this point in the history
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 call `ECC_Stop` before instantiating a new `NodeContext`.
  • Loading branch information
TheCharlatan committed Jan 16, 2024
1 parent 05c4c5a commit 2cb38d9
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 20 deletions.
1 change: 0 additions & 1 deletion src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
4 changes: 4 additions & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 0 additions & 4 deletions src/kernel/checks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.")};
}
Expand Down
7 changes: 0 additions & 7 deletions src/kernel/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <kernel/context.h>

#include <crypto/sha256.h>
#include <key.h>
#include <logging.h>
#include <pubkey.h>
#include <random.h>
Expand All @@ -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
1 change: 0 additions & 1 deletion src/kernel/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ namespace kernel {
//! should be stored to std::unique_ptr members pointing to opaque types.
struct Context {
Context();
~Context();
};
} // namespace kernel

Expand Down
13 changes: 11 additions & 2 deletions src/node/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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
3 changes: 3 additions & 0 deletions src/qt/test/rpcnestedtests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
9 changes: 4 additions & 5 deletions src/wallet/test/wallet_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
Expand All @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit 2cb38d9

Please sign in to comment.