wallet: Consolidate generation setup callers into one function
What changed, and why it matters
This is a routine internal code cleanup in Bitcoin Core's wallet module. It merges several places that set up a wallet's address-generation seed/descriptors into a single helper function called SetupWalletGeneration(). There is no indication this fixes a security bug; it appears intended to make the code easier to maintain and to avoid duplicating logic when creating or encrypting wallets.
No security action required. Treat as normal refactoring; standard review and testing are sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors wallet creation and encryption paths so that descriptor/seed setup is handled by one new method, CWallet::SetupWalletGeneration(). The helper skips setup for blank wallets and wallets with private keys disabled, unless an external signer is in use. It is called from CreateWallet(), EncryptWallet(), and CreateNew(). The change also moves unsetting of the blank-wallet flag earlier in CreateWallet() and removes a redundant WALLET_FLAG_DISABLE_PRIVATE_KEYS check before encryption. These are structural changes with no observable security-relevant behavior change evident from the diff.
Changed components
src/wallet/wallet.cppsrc/wallet/wallet.hInspect captured patch +27 / −26
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 3724c3f2..ade5b48f 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -429,30 +429,18 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string&
return nullptr;
}
+ // Unset the blank flag if not specified by the user
+ if (!create_blank) {
+ wallet->UnsetWalletFlag(WALLET_FLAG_BLANK_WALLET);
+ }
+
// Encrypt the wallet
- if (!passphrase.empty() && !(wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+ if (!passphrase.empty()) {
if (!wallet->EncryptWallet(passphrase)) {
error = Untranslated("Error: Wallet created but failed to encrypt.");
status = DatabaseStatus::FAILED_ENCRYPT;
return nullptr;
}
- if (!create_blank) {
- // Unlock the wallet
- if (!wallet->Unlock(passphrase)) {
- error = Untranslated("Error: Wallet was encrypted but could not be unlocked");
- status = DatabaseStatus::FAILED_ENCRYPT;
- return nullptr;
- }
-
- // Set a seed for the wallet
- {
- LOCK(wallet->cs_wallet);
- wallet->SetupDescriptorScriptPubKeyMans();
- }
-
- // Relock the wallet
- wallet->Lock();
- }
}
WITH_LOCK(wallet->cs_wallet, wallet->LogStats());
@@ -873,12 +861,12 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)
encrypted_batch = nullptr;
Lock();
- Unlock(strWalletPassphrase);
-
- // Make new descriptors with a new seed
- if (!IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET)) {
- SetupDescriptorScriptPubKeyMans();
+ if (!Unlock(strWalletPassphrase)) {
+ return false;
}
+
+ SetupWalletGeneration();
+
Lock();
// Need to completely rewrite the wallet file; if we don't, the database might keep
@@ -3101,9 +3089,7 @@ std::shared_ptr<CWallet> CWallet::CreateNew(WalletContext& context, const std::s
// Only descriptor wallets can be created
assert(walletInstance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
- if ((wallet_creation_flags & WALLET_FLAG_EXTERNAL_SIGNER) || !(wallet_creation_flags & (WALLET_FLAG_DISABLE_PRIVATE_KEYS | WALLET_FLAG_BLANK_WALLET))) {
- walletInstance->SetupDescriptorScriptPubKeyMans();
- }
+ walletInstance->SetupWalletGeneration();
if (chain) {
std::optional<int> tip_height = chain->getHeight();
@@ -3670,6 +3656,18 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
}
}
+void CWallet::SetupWalletGeneration()
+{
+ AssertLockHeld(cs_wallet);
+ // Skip setup for non-external-signer wallets that are either blank
+ // or have private keys disabled (not having private keys implies blank).
+ if (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER) &&
+ (IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET) || IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS))) {
+ return;
+ }
+ SetupDescriptorScriptPubKeyMans();
+}
+
void CWallet::AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal)
{
WalletBatch batch(GetDatabase());
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 81d40ec8..9a1b4fbc 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -1033,6 +1033,9 @@ public:
//! Create new seed and default DescriptorScriptPubKeyMans for this wallet
void SetupOwnDescriptorScriptPubKeyMans(WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ //! Setup new descriptors or seed for new address generation
+ void SetupWalletGeneration() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+
//! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet
DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const;
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.