Public commit record
67/100 · Adequate
What the developer wrote
Authored by David Gumberg
test: wallet: Split create and load
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Mentions testing or verification! No meaningful explanatory body
What changed, and why it matters
This commit only changes Bitcoin Core's internal test helpers and benchmarks. It splits a single test function called TestLoadWallet into two separate helpers: TestCreateWallet (for making a new wallet) and TestLoadWallet (for opening an existing wallet). No production wallet code, network code, or consensus code is changed. There is no security issue here.
Recommended action
No action required. This is a benign test-only refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff refactors wallet test utilities in src/wallet/test/util.cpp/.h. Previously TestLoadWallet accepted a create_flags argument and called CWallet::Create, conflating wallet creation and loading. The patch introduces TestCreateWallet that wraps CWallet::Create and NotifyWalletLoaded, and changes TestLoadWallet to wrap CWallet::LoadExisting. Call sites in benchmarks and unit tests are updated accordingly. This is a pure test-code refactor with no functional change to runtime behavior.
Changed components
src/wallet/test/util.cppsrc/wallet/test/util.hsrc/wallet/test/wallet_tests.cppsrc/bench/wallet_ismine.cppsrc/bench/wallet_loading.cppInspect captured patch +38 / −11
diff --git a/src/bench/wallet_ismine.cpp b/src/bench/wallet_ismine.cpp
index 6eef1efd..b0b9c2c5 100644
--- a/src/bench/wallet_ismine.cpp
+++ b/src/bench/wallet_ismine.cpp
@@ -36,7 +36,7 @@ static void WalletIsMine(benchmark::Bench& bench, int num_combo = 0)
// Loading the wallet will also create it
uint64_t create_flags = WALLET_FLAG_DESCRIPTORS;
auto database = CreateMockableWalletDatabase();
- auto wallet = TestLoadWallet(std::move(database), context, create_flags);
+ auto wallet = TestCreateWallet(std::move(database), context, create_flags);
// For a descriptor wallet, fill with num_combo combo descriptors with random keys
// This benchmarks a non-HD wallet migrated to descriptors
diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index 3397fb00..8131f192 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -43,7 +43,7 @@ static void WalletLoadingDescriptors(benchmark::Bench& bench)
// Loading the wallet will also create it
uint64_t create_flags = WALLET_FLAG_DESCRIPTORS;
auto database = CreateMockableWalletDatabase();
- auto wallet = TestLoadWallet(std::move(database), context, create_flags);
+ auto wallet = TestCreateWallet(std::move(database), context, create_flags);
// Generate a bunch of transactions and addresses to put into the wallet
for (int i = 0; i < 1000; ++i) {
@@ -56,7 +56,7 @@ static void WalletLoadingDescriptors(benchmark::Bench& bench)
TestUnloadWallet(std::move(wallet));
bench.epochs(5).run([&] {
- wallet = TestLoadWallet(std::move(database), context, create_flags);
+ wallet = TestLoadWallet(std::move(database), context);
// Cleanup
database = DuplicateMockDatabase(wallet->GetDatabase());
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index c06c6af5..db85b5ef 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -47,11 +47,36 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc
return wallet;
}
-std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags)
+std::shared_ptr<CWallet> TestCreateWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags)
{
+ bilingual_str _error;
+ std::vector<bilingual_str> _warnings;
+ auto wallet = CWallet::Create(context, "", std::move(database), create_flags, _error, _warnings);
+ NotifyWalletLoaded(context, wallet);
+ if (context.chain) {
+ wallet->postInitProcess();
+ }
+ return wallet;
+}
+
+std::shared_ptr<CWallet> TestCreateWallet(WalletContext& context)
+{
+ DatabaseOptions options;
+ options.require_create = true;
+ options.create_flags = WALLET_FLAG_DESCRIPTORS;
+ DatabaseStatus status;
bilingual_str error;
std::vector<bilingual_str> warnings;
- auto wallet = CWallet::Create(context, "", std::move(database), create_flags, error, warnings);
+ auto database = MakeWalletDatabase("", options, status, error);
+ return TestCreateWallet(std::move(database), context, options.create_flags);
+}
+
+
+std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context)
+{
+ bilingual_str error;
+ std::vector<bilingual_str> warnings;
+ auto wallet = CWallet::LoadExisting(context, "", std::move(database), error, warnings);
NotifyWalletLoaded(context, wallet);
if (context.chain) {
wallet->postInitProcess();
@@ -62,12 +87,12 @@ std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database
std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context)
{
DatabaseOptions options;
- options.create_flags = WALLET_FLAG_DESCRIPTORS;
+ options.require_existing = true;
DatabaseStatus status;
bilingual_str error;
std::vector<bilingual_str> warnings;
auto database = MakeWalletDatabase("", options, status, error);
- return TestLoadWallet(std::move(database), context, options.create_flags);
+ return TestLoadWallet(std::move(database), context);
}
void TestUnloadWallet(std::shared_ptr<CWallet>&& wallet)
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index ecedbd90..18305fb5 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -32,8 +32,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq
std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key);
+std::shared_ptr<CWallet> TestCreateWallet(WalletContext& context);
+std::shared_ptr<CWallet> TestCreateWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags);
std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context);
-std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags);
+std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context);
void TestUnloadWallet(std::shared_ptr<CWallet>&& wallet);
// Creates a copy of the provided database
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 50d2f78b..4c422904 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -584,7 +584,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
WalletContext context;
context.args = &m_args;
context.chain = m_node.chain.get();
- auto wallet = TestLoadWallet(context);
+ auto wallet = TestCreateWallet(context);
CKey key = GenerateRandomKey();
AddKey(*wallet, key);
TestUnloadWallet(std::move(wallet));
@@ -681,7 +681,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup)
{
WalletContext context;
context.args = &m_args;
- auto wallet = TestLoadWallet(context);
+ auto wallet = TestCreateWallet(context);
BOOST_CHECK(wallet);
WaitForDeleteWallet(std::move(wallet));
}
@@ -692,7 +692,7 @@ BOOST_FIXTURE_TEST_CASE(RemoveTxs, TestChain100Setup)
WalletContext context;
context.args = &m_args;
context.chain = m_node.chain.get();
- auto wallet = TestLoadWallet(context);
+ auto wallet = TestCreateWallet(context);
CKey key = GenerateRandomKey();
AddKey(*wallet, key);
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.