wallet: Move argument parsing to before DB load
What the developer wrote
Authored by David Gumberg
wallet: Move argument parsing to before DB load
`m_keypool_size` must be set before `CWallet::PopulateWalletFromDB()`,
in order to move parsing of `-keypool` into `CWallet::LoadWalletArgs`,
`LoadWalletArgs()` invocation in `CWallet::Create()` must be moved
before `PopulateWalletFromDB()` is called.
What changed, and why it matters
This commit fixes the order in which a Bitcoin Core wallet reads startup settings. Previously, the `-keypool` setting (which controls how many spare Bitcoin addresses the wallet keeps ready) was read after the wallet had already loaded data from its database. The change moves all argument parsing, including `-keypool`, to happen before the database is loaded so the correct keypool size is available during wallet initialization. If the wrong size is used, the wallet might not generate or top up the expected number of addresses, which could affect backups and address availability.
Treat as a low-severity reliability fix. Review whether the misordered initialization could have caused wallets to be created or loaded with an unexpectedly small keypool, and consider whether any user-facing guidance is needed for backups. No urgent security response appears required based on the commit alone.
Security signals we found
Wallet initialization order corrected to ensure keypool size is configured before database population
Potential for wallet to operate with default rather than user-configured keypool size prior to fix
No explicit security impact described by vendor in commit message
Evidence from the diff
The patch moves the LoadWalletArgs() call in CWallet::Create() to occur before PopulateWalletFromDB(). It also relocates parsing of -keypool and -walletnotify from the inline code in CWallet::Create() into LoadWalletArgs(). The stated reason is that m_keypool_size must be set before PopulateWalletFromDB() runs. This is a correctness/reliability fix in wallet initialization ordering.
Changed components
src/wallet/wallet.cppCWallet::Create()CWallet::LoadWalletArgs()CWallet::PopulateWalletFromDB()Inspect captured patch +7 / −7
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d95701fd..d30bdb7c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3016,6 +3016,9 @@ bool CWallet::LoadWalletArgs(std::shared_ptr<CWallet> wallet, const WalletContex
wallet->m_confirm_target = args.GetIntArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET);
wallet->m_spend_zero_conf_change = args.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE);
wallet->m_signal_rbf = args.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF);
+
+ wallet->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
+ wallet->m_notify_tx_changed_script = args.GetArg("-walletnotify", "");
wallet->SetBroadcastTransactions(args.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST));
return true;
@@ -3024,15 +3027,16 @@ bool CWallet::LoadWalletArgs(std::shared_ptr<CWallet> wallet, const WalletContex
std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings)
{
interfaces::Chain* chain = context.chain;
- ArgsManager& args = *Assert(context.args);
const std::string& walletFile = database->Filename();
const auto start{SteadyClock::now()};
// TODO: Can't use std::make_shared because we need a custom deleter but
// should be possible to use std::allocate_shared.
std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), FlushAndDeleteWallet);
- walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
- walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", "");
+
+ if (!LoadWalletArgs(walletInstance, context, error, warnings)) {
+ return nullptr;
+ }
// Load wallet
auto nLoadWalletRet = walletInstance->PopulateWalletFromDB(error, warnings);
@@ -3079,10 +3083,6 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
}
}
- if (!LoadWalletArgs(walletInstance, context, error, warnings)) {
- return nullptr;
- }
-
walletInstance->WalletLogPrintf("Wallet completed loading in %15dms\n", Ticks<std::chrono::milliseconds>(SteadyClock::now() - start));
// Try to top up keypool. No-op if the wallet is locked.
Why this scored 31/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.