wallet: remove loading logic from CWallet::Create
What changed, and why it matters
This commit is a code cleanup change in Bitcoin Core's wallet creation logic. It removes the ability to load an existing wallet from within the CWallet::Create function, making Create only handle brand-new wallets. Loading existing wallets is moved elsewhere. The change also updates a test to verify that a version message is logged when loading a wallet, not when creating one. There is no indication this fixes a security vulnerability.
No security action required; treat as normal refactoring. Reviewers may optionally verify that wallet loading still occurs through the appropriate separate code path and that the version metadata write is correct.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors CWallet::Create in src/wallet/wallet.cpp so it no longer calls PopulateWalletFromDB() or handles first-run/loaded-wallet branches. Instead it initializes version metadata and proceeds only with new-wallet setup. The rescan_required flag is hardcoded to false because loading is no longer performed here. The functional test is adjusted to expect the ‘Last client version’ debug log message on loadwallet, not createwallet. This is a separation-of-concerns refactor with no security-relevant signals in the diff or commit message.
Changed components
src/wallet/wallet.cpptest/functional/wallet_createwallet.pyInspect captured patch +15 / −30
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index e5c21dc0..a11f8511 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3038,18 +3038,11 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
return nullptr;
}
- // Load wallet
- auto nLoadWalletRet = walletInstance->PopulateWalletFromDB(error, warnings);
- bool rescan_required = nLoadWalletRet == DBErrors::NEED_RESCAN;
- if (nLoadWalletRet != DBErrors::LOAD_OK && nLoadWalletRet != DBErrors::NONCRITICAL_ERROR && !rescan_required) {
+ // Initialize version key.
+ if(!WalletBatch(walletInstance->GetDatabase()).WriteVersion(CLIENT_VERSION)) {
+ error = strprintf(_("Error creating %s: Could not write version metadata."), walletFile);
return nullptr;
}
-
- // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys
- const bool fFirstRun = walletInstance->m_spk_managers.empty() &&
- !walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) &&
- !walletInstance->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET);
- if (fFirstRun)
{
LOCK(walletInstance->cs_wallet);
@@ -3070,25 +3063,14 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
walletInstance->SetLastBlockProcessed(*tip_height, chain->getBlockHash(*tip_height));
}
}
- } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) {
- // Make it impossible to disable private keys after creation
- error = strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile);
- return nullptr;
- } else if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
- for (auto spk_man : walletInstance->GetActiveScriptPubKeyMans()) {
- if (spk_man->HavePrivateKeys()) {
- warnings.push_back(strprintf(_("Warning: Private keys detected in wallet {%s} with disabled private keys"), walletFile));
- break;
- }
- }
}
- walletInstance->WalletLogPrintf("Wallet completed loading in %15dms\n", Ticks<std::chrono::milliseconds>(SteadyClock::now() - start));
+ walletInstance->WalletLogPrintf("Wallet completed creation in %15dms\n", Ticks<std::chrono::milliseconds>(SteadyClock::now() - start));
// Try to top up keypool. No-op if the wallet is locked.
walletInstance->TopUpKeyPool();
- if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) {
+ if (chain && !AttachChain(walletInstance, *chain, /*rescan_required=*/false, error, warnings)) {
walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded
return nullptr;
}
diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
index cf0d8b4e..44d523a4 100755
--- a/test/functional/wallet_createwallet.py
+++ b/test/functional/wallet_createwallet.py
@@ -167,14 +167,17 @@ class CreateWalletTest(BitcoinTestFramework):
assert_raises_rpc_error(-4, 'descriptors argument must be set to "true"; it is no longer possible to create a legacy wallet.', self.nodes[0].createwallet, wallet_name="legacy", descriptors=False)
self.log.info("Check that the version number is being logged correctly")
- with node.assert_debug_log(expected_msgs=[], unexpected_msgs=["Last client version = "]):
- node.createwallet("version_check")
- wallet = node.get_wallet_rpc("version_check")
+
+ # Craft the expected version message.
client_version = node.getnetworkinfo()["version"]
- wallet.unloadwallet()
- with node.assert_debug_log(
- expected_msgs=[f"Last client version = {client_version}"]
- ):
+ version_message = f"Last client version = {client_version}"
+
+ # Should not be logged when creating.
+ with node.assert_debug_log(expected_msgs=[], unexpected_msgs=[version_message]):
+ node.createwallet("version_check")
+ node.unloadwallet("version_check")
+ # Should be logged when loading.
+ with node.assert_debug_log(expected_msgs=[version_message]):
node.loadwallet("version_check")
Why this scored 12/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.