wallet: Delete unnecessary PopulateWalletFromDB() calls
What changed, and why it matters
This commit removes calls to a database-loading helper in three places where they were unnecessary. In two test files and one wallet dump/import tool, the code was calling PopulateWalletFromDB() on a freshly created, empty wallet database before immediately writing new data into it. The change is a code cleanup: it deletes redundant work and, in the dump tool, removes an error check that could never fail meaningfully because the wallet had just been created. There is no direct evidence this fixes a security vulnerability.
Treat as a routine cleanup commit. Reviewers may want to confirm that CreateFromDump() still handles genuine database errors during the subsequent batch operations, since the removed check was the only explicit PopulateWalletFromDB() error check in that path. No urgent security action is indicated.
Security signals we found
Removal of redundant database-load calls in wallet creation/dump path
Removal of error-handling branch that could mask or misreport state in wallet dump tool
No changes to validation, networking, cryptography, or transaction signing
Evidence from the diff
PopulateWalletFromDB() loads existing records from a wallet database into memory. In the two Qt test helpers, the wallet is constructed with CreateMockableWalletDatabase() and then immediately set up with descriptors/keys, so loading an empty mock DB first served no purpose. In wallet/dump.cpp’s CreateFromDump(), a new CWallet is constructed with a fresh database and then PopulateWalletFromDB() was called inside cs_wallet before a batch write; because the DB is new, this load was redundant and its error handling could only report a generic failure. Removing these calls simplifies the code and eliminates a misleading error path. The commit does not change consensus, P2P, or cryptographic logic.
Changed components
src/wallet/dump.cppsrc/qt/test/addressbooktests.cppsrc/qt/test/wallettests.cppInspect captured patch +0 / −9
diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp
index f0f2c594..58b8a6d2 100644
--- a/src/qt/test/addressbooktests.cpp
+++ b/src/qt/test/addressbooktests.cpp
@@ -77,7 +77,6 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
test.m_node.wallet_loader = wallet_loader.get();
node.setContext(&test.m_node);
const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockableWalletDatabase());
- wallet->PopulateWalletFromDB();
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
{
LOCK(wallet->cs_wallet);
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index c5e6e116..22711761 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -192,7 +192,6 @@ void SyncUpWallet(const std::shared_ptr<CWallet>& wallet, interfaces::Node& node
std::shared_ptr<CWallet> SetupDescriptorsWallet(interfaces::Node& node, TestChain100Setup& test, bool watch_only = false)
{
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockableWalletDatabase());
- wallet->PopulateWalletFromDB();
LOCK(wallet->cs_wallet);
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
if (watch_only) {
diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp
index ee800a04..ba71c559 100644
--- a/src/wallet/dump.cpp
+++ b/src/wallet/dump.cpp
@@ -198,13 +198,6 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs::
bool ret = true;
std::shared_ptr<CWallet> wallet(new CWallet(/*chain=*/nullptr, name, std::move(database)), WalletToolReleaseWallet);
{
- LOCK(wallet->cs_wallet);
- DBErrors load_wallet_ret = wallet->PopulateWalletFromDB();
- if (load_wallet_ret != DBErrors::LOAD_OK) {
- error = strprintf(_("Error creating %s"), name);
- return false;
- }
-
// Get the database handle
WalletDatabase& db = wallet->GetDatabase();
std::unique_ptr<DatabaseBatch> batch = db.MakeBatch();
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.