test: wallet: Constructing a DSPKM that can't TopUp() throws.
What changed, and why it matters
This commit is a test-only change for Bitcoin Core's wallet code. It moves two internal type definitions to a more private scope and adds a unit test verifying that creating a certain kind of watch-only descriptor wallet (one using hardened derivation without private keys) throws an error because it cannot generate new addresses. There is no runtime bug fix or exploit here; it improves test coverage and code encapsulation.
No action required. This is a routine test/encapsulation improvement. Reviewers may verify the new test passes and that the visibility changes do not break downstream code.
Security signals we found
Added defensive unit test for invalid wallet construction
Reduced class member visibility (protected -> private, file-scope -> class-scope)
No change to consensus, networking, or cryptographic code
Evidence from the diff
The patch narrows visibility of ScriptPubKeyMap/PubKeyMap from file-level to inside DescriptorScriptPubKeyMan, and changes LegacyDataSPKM members from protected to private. It adds a BOOST_AUTO_TEST_CASE that constructs a DescriptorScriptPubKeyMan from a wpkh(
Changed components
src/wallet/scriptpubkeyman.hsrc/wallet/test/scriptpubkeyman_tests.cppInspect captured patch +17 / −3
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index 621630c9..8b80a3da 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -168,14 +168,12 @@ static const std::unordered_set<OutputType> LEGACY_OUTPUT_TYPES {
using KeyMap = std::map<CKeyID, CKey>;
using CryptedKeyMap = std::map<CKeyID, std::pair<CPubKey, std::vector<unsigned char>>>;
-using ScriptPubKeyMap = std::map<CScript, int32_t>; // Map of scripts to descriptor range index
-using PubKeyMap = std::map<CPubKey, int32_t>; // Map of pubkeys involved in scripts to descriptor range index
// Manages the data for a LegacyScriptPubKeyMan.
// This is the minimum necessary to load a legacy wallet so that it can be migrated.
class LegacyDataSPKM : public ScriptPubKeyMan, public FillableSigningProvider
{
-protected:
+private:
using WatchOnlySet = std::set<CScript>;
using WatchKeyMap = std::map<CKeyID, CPubKey>;
@@ -277,6 +275,9 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
{
friend class LegacyDataSPKM;
private:
+ using ScriptPubKeyMap = std::map<CScript, int32_t>; // Map of scripts to descriptor range index
+ using PubKeyMap = std::map<CPubKey, int32_t>; // Map of pubkeys involved in scripts to descriptor range index
+
ScriptPubKeyMap m_map_script_pub_keys GUARDED_BY(cs_desc_man);
PubKeyMap m_map_pubkeys GUARDED_BY(cs_desc_man);
int32_t m_max_cached_index = -1;
diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp
index 9c62436d..74389a54 100644
--- a/src/wallet/test/scriptpubkeyman_tests.cpp
+++ b/src/wallet/test/scriptpubkeyman_tests.cpp
@@ -4,6 +4,7 @@
#include <key.h>
#include <key_io.h>
+#include <test/util/common.h>
#include <test/util/setup_common.h>
#include <script/solver.h>
#include <wallet/scriptpubkeyman.h>
@@ -37,5 +38,17 @@ BOOST_AUTO_TEST_CASE(DescriptorScriptPubKeyManTests)
BOOST_CHECK(signprov_keypath_nums_h == nullptr);
}
+BOOST_AUTO_TEST_CASE(desc_spkm_topup_fail)
+{
+ // Attempting to construct a DescriptorSPKM that cannot be topped up (hardened derivation without private keys)
+ // should throw even though it is valid and can be parsed
+ CExtKey extkey;
+ extkey.SetSeed(std::array<std::byte, 32>{});
+ CWallet keystore(m_node.chain.get(), "", CreateMockableWalletDatabase());
+ BOOST_CHECK_EXCEPTION(
+ CreateDescriptor(keystore, "wpkh(" + EncodeExtPubKey(extkey.Neuter()) + "/*h)", /*success=*/true),
+ std::runtime_error, HasReason("Could not top up scriptPubKeys"));
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace wallet
Why this scored 17/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.