wallet: Construct ExternalSignerSPKM with the new descriptor
What the developer wrote
Authored by Ava Chow
wallet: Construct ExternalSignerSPKM with the new descriptor
Instead of constructing then setting the descriptor with
SetupDescriptor, just pass in that descriptor to the constructor.
What changed, and why it matters
This is a small internal code cleanup in Bitcoin Core's wallet module. It changes how an external-signer wallet manager object is created so that its descriptor is passed directly through the constructor rather than being set afterward via a separate SetupDescriptor method. There is no user-facing behavior change and no security fix.
No security action required. Treat as a normal code-quality refactor during review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors ExternalSignerScriptPubKeyMan construction. SetupDescriptor is removed from the public interface and replaced with a static factory method CreateNew that constructs the object, assigns the WalletDescriptor, writes it to the wallet database, tops up the keypool, and unsets the blank-wallet flag. The former public constructor taking only storage and keypool size is made private. CWallet::SetupDescriptorScriptPubKeyMans is updated to call CreateNew instead of new + SetupDescriptor. Functionality is preserved; this is purely a structural improvement.
Changed components
src/wallet/external_signer_scriptpubkeyman.cppsrc/wallet/external_signer_scriptpubkeyman.hsrc/wallet/wallet.cppInspect captured patch +18 / −20
diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp
index 189f7698..e07de305 100644
--- a/src/wallet/external_signer_scriptpubkeyman.cpp
+++ b/src/wallet/external_signer_scriptpubkeyman.cpp
@@ -26,28 +26,30 @@ std::unique_ptr<ExternalSignerScriptPubKeyMan> ExternalSignerScriptPubKeyMan::Lo
return std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(storage, descriptor, keypool_size, keys, ckeys));
}
-bool ExternalSignerScriptPubKeyMan::SetupDescriptor(WalletBatch& batch, std::unique_ptr<Descriptor> desc)
+std::unique_ptr<ExternalSignerScriptPubKeyMan> ExternalSignerScriptPubKeyMan::CreateNew(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, std::unique_ptr<Descriptor> desc)
{
- LOCK(cs_desc_man);
- assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
- assert(m_storage.IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER));
+ auto spkm = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(storage, keypool_size));
+
+ LOCK(spkm->cs_desc_man);
+ assert(storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
+ assert(storage.IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER));
int64_t creation_time = GetTime();
// Make the descriptor
WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0);
- m_wallet_descriptor = w_desc;
+ spkm->m_wallet_descriptor = w_desc;
// Store the descriptor
- if (!batch.WriteDescriptor(GetID(), m_wallet_descriptor)) {
+ if (!batch.WriteDescriptor(spkm->GetID(), spkm->m_wallet_descriptor)) {
throw std::runtime_error(std::string(__func__) + ": writing descriptor failed");
}
// TopUp
- TopUpWithDB(batch);
+ spkm->TopUpWithDB(batch);
- m_storage.UnsetBlankWalletFlag(batch);
- return true;
+ storage.UnsetBlankWalletFlag(batch);
+ return spkm;
}
util::Result<ExternalSigner> ExternalSignerScriptPubKeyMan::GetExternalSigner() {
diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h
index 1c3324f1..3ee6b5cd 100644
--- a/src/wallet/external_signer_scriptpubkeyman.h
+++ b/src/wallet/external_signer_scriptpubkeyman.h
@@ -16,21 +16,18 @@ namespace wallet {
class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
{
private:
+ //! Create an ExternalSPKM from existing wallet data
ExternalSignerScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys)
: DescriptorScriptPubKeyMan(storage, descriptor, keypool_size, keys, ckeys)
{}
-public:
- ExternalSignerScriptPubKeyMan(WalletStorage& storage, int64_t keypool_size)
- : DescriptorScriptPubKeyMan(storage, keypool_size)
- {}
+ ExternalSignerScriptPubKeyMan(WalletStorage& storage, int64_t keypool_size)
+ : DescriptorScriptPubKeyMan(storage, keypool_size)
+ {}
+public:
static std::unique_ptr<ExternalSignerScriptPubKeyMan> LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys);
-
- /** Provide a descriptor at setup time
- * Returns false if already setup or setup fails, true if setup is successful
- */
- bool SetupDescriptor(WalletBatch& batch, std::unique_ptr<Descriptor>desc);
+ static std::unique_ptr<ExternalSignerScriptPubKeyMan> CreateNew(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, std::unique_ptr<Descriptor> desc);
static util::Result<ExternalSigner> GetExternalSigner();
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 7f40fa70..5a5af893 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3634,8 +3634,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
continue;
}
OutputType t = *desc->GetOutputType();
- auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, m_keypool_size));
- spk_manager->SetupDescriptor(batch, std::move(desc));
+ auto spk_manager = ExternalSignerScriptPubKeyMan::CreateNew(*this, batch, m_keypool_size, std::move(desc));
uint256 id = spk_manager->GetID();
AddScriptPubKeyMan(id, std::move(spk_manager));
AddActiveScriptPubKeyManWithDb(batch, id, t, internal);
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.