wallet: Create separate function for wallet load
What changed, and why it matters
This commit is a simple code cleanup: it takes the part of Bitcoin Core's wallet creation code that handles loading an already-existing wallet and moves it into a new, separate function called LoadExisting. The actual behavior and security checks are unchanged; only the internal organization of the code is different.
No security action needed. Review as ordinary refactoring.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change refactors CWallet::Create() by extracting existing-wallet load logic into a new static method CWallet::LoadExisting(). The new function performs the same steps as before: constructing the CWallet, calling LoadWalletArgs, PopulateWalletFromDB, warning on private keys when disabled, TopUpKeyPool, AttachChain, and LogStats. No logic, ordering, or security semantics appear to change. The header comment for Create() is updated to clarify it creates new wallets, and a new comment is added for LoadExisting().
Changed components
src/wallet/wallet.cppsrc/wallet/wallet.hInspect captured patch +48 / −1
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 8ee2302c..44afd1cd 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3098,6 +3098,50 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
return walletInstance;
}
+std::shared_ptr<CWallet> CWallet::LoadExisting(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, bilingual_str& error, std::vector<bilingual_str>& warnings)
+{
+ interfaces::Chain* chain = context.chain;
+ const std::string& walletFile = database->Filename();
+
+ const auto start{SteadyClock::now()};
+ std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), FlushAndDeleteWallet);
+
+ if (!LoadWalletArgs(walletInstance, context, error, warnings)) {
+ 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) {
+ return nullptr;
+ }
+
+ 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));
+
+ // Try to top up keypool. No-op if the wallet is locked.
+ walletInstance->TopUpKeyPool();
+
+ if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) {
+ walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded
+ return nullptr;
+ }
+
+ WITH_LOCK(walletInstance->cs_wallet, walletInstance->LogStats());
+
+ return walletInstance;
+}
+
+
bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector<bilingual_str>& warnings)
{
LOCK(walletInstance->cs_wallet);
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index e4072c44..d09612a1 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -873,9 +873,12 @@ public:
static bool LoadWalletArgs(std::shared_ptr<CWallet> wallet, const WalletContext& context, bilingual_str& error, std::vector<bilingual_str>& warnings);
- /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */
+ /* Initializes, creates and returns a new CWallet; returns a null pointer in case of an error */
static std::shared_ptr<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);
+ /* Initializes, loads, and returns a CWallet from an existing wallet; returns a null pointer in case of an error */
+ static std::shared_ptr<CWallet> LoadExisting(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, bilingual_str& error, std::vector<bilingual_str>& warnings);
+
/**
* Wallet post-init setup
* Gives the wallet a chance to register repetitive tasks and complete post-init tasks
Why this scored 15/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.