highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-11 21:20:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

wallet, spkm: Treat Descriptor ID as an opaque SPKM ID

Browse Source 
Instead of treating the descriptor ID as something which has a meaning
which can be verified, treat the ID read from disk as some opaque blob
used solely to identify and tie together specific records from disk.

This removes the usage of the ID for duplication checks or comparison,
and removes the check that the read ID matches a computed ID.

When writing new descriptors to disk, the ID is still calculated from
the old Descriptor ID method for backwards compatibility. But this fact
is opaque to all further usages of the ID.
This commit is contained in:
Ava Chow
2026-06-02 14:16:25 -07:00
parent 62e826fa76
commit 2a6c53371b
10 changed files with 31 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/wallet/export.cpp
View File

@@ -107,13 +107,13 @@ util::Result<std::string> ExportWatchOnlyWallet(const CWallet& wallet, const fs:
WalletDescriptor w_desc(std::move(descs.at(0)), desc_info.creation_time, range_start, range_end, desc_info.next_index);
// For descriptors that cannot self expand (i.e. needs private keys or cache), set the cache
uint256 desc_id = w_desc.id;
if (!w_desc.descriptor->CanSelfExpand()) {
w_desc.cache = desc_info.cache;
}
// Add to the watchonly wallet
if (auto spkm_res = watchonly_wallet->AddWalletDescriptor(w_desc, dummy_keys, /*label=*/"", /*internal=*/false); !spkm_res) {
auto spkm_res = watchonly_wallet->AddWalletDescriptor(w_desc, dummy_keys, /*label=*/"", /*internal=*/false);
if (!spkm_res) {
return util::Error{util::ErrorString(spkm_res)};
}
@@ -125,7 +125,7 @@ util::Result<std::string> ExportWatchOnlyWallet(const CWallet& wallet, const fs:
if (desc_info.internal) {
internal = *desc_info.internal;
}
watchonly_wallet->AddActiveScriptPubKeyMan(desc_id, *Assert(w_desc.descriptor->GetOutputType()), internal);
watchonly_wallet->AddActiveScriptPubKeyMan(spkm_res->get().GetID(), *Assert(w_desc.descriptor->GetOutputType()), internal);
}
}

4
src/wallet/external_signer_scriptpubkeyman.cpp
View File

@@ -21,9 +21,9 @@
using common::PSBTError;
namespace wallet {
std::unique_ptr<ExternalSignerScriptPubKeyMan> ExternalSignerScriptPubKeyMan::LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys)
std::unique_ptr<ExternalSignerScriptPubKeyMan> ExternalSignerScriptPubKeyMan::LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys)
{
return std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(storage, descriptor, keypool_size, keys, ckeys));
return std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(storage, id, descriptor, keypool_size, keys, ckeys));
}
std::unique_ptr<ExternalSignerScriptPubKeyMan> ExternalSignerScriptPubKeyMan::CreateNew(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, std::unique_ptr<Descriptor> desc)

2
src/wallet/external_signer_scriptpubkeyman.h
View File

@@ -19,7 +19,7 @@ private:
using DescriptorScriptPubKeyMan::DescriptorScriptPubKeyMan;
public:
static std::unique_ptr<ExternalSignerScriptPubKeyMan> LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys);
static std::unique_ptr<ExternalSignerScriptPubKeyMan> LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys);
static std::unique_ptr<ExternalSignerScriptPubKeyMan> CreateNew(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, std::unique_ptr<Descriptor> desc);
static util::Result<ExternalSigner> GetExternalSigner();

3
src/wallet/rpc/wallet.cpp
View File

@@ -812,8 +812,7 @@ static RPCMethod createwalletdescriptor()
WalletBatch batch{pwallet->GetDatabase()};
for (bool internal : internals) {
WalletDescriptor w_desc = GenerateWalletDescriptor(xpub, *output_type, internal);
uint256 w_id = DescriptorID(*w_desc.descriptor);
if (!pwallet->GetScriptPubKeyMan(w_id)) {
if (!pwallet->GetDescriptorScriptPubKeyMan(w_desc)) {
spkms.emplace_back(pwallet->SetupDescriptorScriptPubKeyMan(batch, active_hdkey, *output_type, internal));
}
}

13
src/wallet/scriptpubkeyman.cpp
View File

@@ -846,11 +846,12 @@ std::unique_ptr<DescriptorScriptPubKeyMan> DescriptorScriptPubKeyMan::CreateFrom
return spkm;
}
DescriptorScriptPubKeyMan::DescriptorScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys)
DescriptorScriptPubKeyMan::DescriptorScriptPubKeyMan(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys)
: ScriptPubKeyMan(storage),
m_map_keys(keys),
m_map_crypted_keys(ckeys),
m_keypool_size(keypool_size),
m_id(id),
m_wallet_descriptor(descriptor)
{
if (!keys.empty() && !ckeys.empty()) {
@@ -859,9 +860,9 @@ DescriptorScriptPubKeyMan::DescriptorScriptPubKeyMan(WalletStorage& storage, Wal
Load();
}
std::unique_ptr<DescriptorScriptPubKeyMan> DescriptorScriptPubKeyMan::LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys)
std::unique_ptr<DescriptorScriptPubKeyMan> DescriptorScriptPubKeyMan::LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys)
{
return std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(storage, descriptor, keypool_size, keys, ckeys));
return std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(storage, id, descriptor, keypool_size, keys, ckeys));
}
std::unique_ptr<DescriptorScriptPubKeyMan> DescriptorScriptPubKeyMan::GenerateNewSingleSig(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, const CExtKey& master_key, OutputType addr_type, bool internal)
@@ -1498,8 +1499,7 @@ std::unique_ptr<CKeyMetadata> DescriptorScriptPubKeyMan::GetMetadata(const CTxDe
uint256 DescriptorScriptPubKeyMan::GetID() const
{
LOCK(cs_desc_man);
return m_wallet_descriptor.id;
return m_id;
}
void DescriptorScriptPubKeyMan::Load()
@@ -1538,7 +1538,8 @@ void DescriptorScriptPubKeyMan::Load()
bool DescriptorScriptPubKeyMan::HasWalletDescriptor(const WalletDescriptor& desc) const
{
LOCK(cs_desc_man);
return !m_wallet_descriptor.id.IsNull() && !desc.id.IsNull() && m_wallet_descriptor.id == desc.id;
// Compare by using the canonical string to make the hardened indicators consistent for comparison
return m_wallet_descriptor.descriptor->ToCanonicalString() == desc.descriptor->ToCanonicalString();
}
void DescriptorScriptPubKeyMan::WriteDescriptor()

7
src/wallet/scriptpubkeyman.h
View File

@@ -301,6 +301,8 @@ private:
*/
mutable std::map<uint256, MuSig2SecNonce> m_musig2_secnonces;
const uint256 m_id;
bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
@@ -319,12 +321,13 @@ private:
protected:
//! Create a DescriptorScriptPubKeyMan from existing data (i.e. during loading)
DescriptorScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys);
DescriptorScriptPubKeyMan(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys);
//! Create a new DescriptorScriptPubKeyMan from a descriptor (e.g. from an import, newly generated)
DescriptorScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size)
: ScriptPubKeyMan(storage),
m_keypool_size(keypool_size),
m_id(DescriptorID(*descriptor.descriptor)),
m_wallet_descriptor(descriptor)
{}
@@ -337,7 +340,7 @@ protected:
bool TopUpWithDB(WalletBatch& batch, unsigned int size = 0);
public:
static std::unique_ptr<DescriptorScriptPubKeyMan> LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys);
static std::unique_ptr<DescriptorScriptPubKeyMan> LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys);
static std::unique_ptr<DescriptorScriptPubKeyMan> CreateFromImport(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const FlatSigningProvider& provider);
static std::unique_ptr<DescriptorScriptPubKeyMan> CreateFromMigration(WalletStorage& storage, WalletBatch& batch, WalletDescriptor& descriptor, int64_t keypool_size, const FlatSigningProvider& provider);
static std::unique_ptr<DescriptorScriptPubKeyMan> GenerateNewSingleSig(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, const CExtKey& master_key, OutputType addr_type, bool internal);

19
src/wallet/test/walletload_tests.cpp
View File

@@ -25,7 +25,7 @@ public:
std::string ToCanonicalString() const override { return desc; }
std::optional<OutputType> GetOutputType() const override { return OutputType::UNKNOWN; }
bool IsRange() const override { return false; }
bool IsRange() const override { return true; }
bool IsSolvable() const override { return false; }
bool IsSingleType() const override { return true; }
bool HavePrivateKeys(const SigningProvider&) const override { return false; }
@@ -66,19 +66,11 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup)
}
// Test 2
// Now write a valid descriptor with an invalid ID.
// As the software produces another ID for the descriptor, the loading process must be aborted.
// Now write a valid descriptor with a different ID which must be accepted
database = CreateMockableWalletDatabase();
// Verify the error
bool found = false;
DebugLogHelper logHelper("The descriptor ID calculated by the wallet differs from the one in DB", [&](const std::string* s) {
found = true;
return false;
});
{
// Write valid descriptor with invalid ID
// Write valid descriptor with arbitrary ID
WalletBatch batch(*database);
std::string desc = "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu";
WalletDescriptor wallet_descriptor(std::make_shared<DummyDescriptor>(desc), 0, 0, 0, 0);
@@ -86,10 +78,9 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup)
}
{
// Now try to load the wallet and verify the error.
// Now try to load the wallet and verify the result.
const std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(database)));
BOOST_CHECK_EQUAL(wallet->PopulateWalletFromDB(_error, _warnings), DBErrors::CORRUPT);
BOOST_CHECK(found); // The error must be logged
BOOST_CHECK_EQUAL(wallet->PopulateWalletFromDB(_error, _warnings), DBErrors::LOAD_OK);
}
}

15
src/wallet/wallet.cpp
View File

@@ -3603,9 +3603,9 @@ void CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc,
{
std::unique_ptr<DescriptorScriptPubKeyMan> spk_manager;
if (IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) {
spk_manager = ExternalSignerScriptPubKeyMan::LoadFromStorage(*this, desc, m_keypool_size, keys, ckeys);
spk_manager = ExternalSignerScriptPubKeyMan::LoadFromStorage(*this, id, desc, m_keypool_size, keys, ckeys);
} else {
spk_manager = DescriptorScriptPubKeyMan::LoadFromStorage(*this, desc, m_keypool_size, keys, ckeys);
spk_manager = DescriptorScriptPubKeyMan::LoadFromStorage(*this, id, desc, m_keypool_size, keys, ckeys);
}
AddScriptPubKeyMan(id, std::move(spk_manager));
}
@@ -3765,14 +3765,13 @@ void CWallet::DeactivateScriptPubKeyMan(uint256 id, OutputType type, bool intern
DescriptorScriptPubKeyMan* CWallet::GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const
{
auto spk_man_pair = m_spk_managers.find(desc.id);
auto spk_man_pair = std::find_if(m_spk_managers.begin(), m_spk_managers.end(), [&desc](const auto& item) {
DescriptorScriptPubKeyMan* spk_manager = dynamic_cast<DescriptorScriptPubKeyMan*>(item.second.get());
return spk_manager != nullptr && spk_manager->HasWalletDescriptor(desc);
});
if (spk_man_pair != m_spk_managers.end()) {
// Try to downcast to DescriptorScriptPubKeyMan then check if the descriptors match
DescriptorScriptPubKeyMan* spk_manager = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man_pair->second.get());
if (spk_manager != nullptr && spk_manager->HasWalletDescriptor(desc)) {
return spk_manager;
}
return dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man_pair->second.get());
}
return nullptr;

5
src/wallet/walletdb.cpp
View File

@@ -777,11 +777,6 @@ static DBErrors LoadDescriptorWalletRecords(CWallet* pwallet, DatabaseBatch& bat
return DBErrors::UNKNOWN_DESCRIPTOR;
}
if (id != desc.id) {
strErr = "The descriptor ID calculated by the wallet differs from the one in DB";
return DBErrors::CORRUPT;
}
DescriptorCache cache;
// Get key cache for this descriptor

3
src/wallet/walletutil.h
View File

@@ -68,7 +68,6 @@ private:
int32_t range_end = 0; // Item after the last; end of range, exclusive, i.e. [range_start, range_end). This will increment with each TopUp()
public:
std::shared_ptr<Descriptor> descriptor;
uint256 id; // Descriptor ID (calculated once at descriptor initialization/deserialization)
uint64_t creation_time = 0;
DescriptorCache cache;
@@ -109,7 +108,6 @@ public:
throw std::ios_base::failure("Can't load a multipath descriptor from databases");
}
descriptor = std::move(descs.at(0));
id = DescriptorID(*descriptor);
}
SERIALIZE_METHODS(WalletDescriptor, obj)
@@ -126,7 +124,6 @@ public:
next_index(next_index),
range_end(descriptor->IsRange() ? range_end : 1),
descriptor(descriptor),
id(DescriptorID(*descriptor)),
creation_time(creation_time) {}
void UpdateFrom(const WalletDescriptor& other);