diff --git a/src/wallet/export.cpp b/src/wallet/export.cpp index fb6bdf5a307..91cbc8a9864 100644 --- a/src/wallet/export.cpp +++ b/src/wallet/export.cpp @@ -107,13 +107,13 @@ util::Result 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 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); } } diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index 1020ef6f667..96f1ee6cfcc 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -21,9 +21,9 @@ using common::PSBTError; namespace wallet { -std::unique_ptr ExternalSignerScriptPubKeyMan::LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys) +std::unique_ptr ExternalSignerScriptPubKeyMan::LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys) { - return std::unique_ptr(new ExternalSignerScriptPubKeyMan(storage, descriptor, keypool_size, keys, ckeys)); + return std::unique_ptr(new ExternalSignerScriptPubKeyMan(storage, id, descriptor, keypool_size, keys, ckeys)); } std::unique_ptr ExternalSignerScriptPubKeyMan::CreateNew(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, std::unique_ptr desc) diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index ea17ca1a063..51c7362d983 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -19,7 +19,7 @@ private: using DescriptorScriptPubKeyMan::DescriptorScriptPubKeyMan; public: - static std::unique_ptr LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys); + static std::unique_ptr LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys); static std::unique_ptr CreateNew(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, std::unique_ptr desc); static util::Result GetExternalSigner(); diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 054ab6b7631..2763f1826f6 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -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)); } } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 481d23f7dd3..5a1fe006c9c 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -846,11 +846,12 @@ std::unique_ptr 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::LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys) +std::unique_ptr DescriptorScriptPubKeyMan::LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys) { - return std::unique_ptr(new DescriptorScriptPubKeyMan(storage, descriptor, keypool_size, keys, ckeys)); + return std::unique_ptr(new DescriptorScriptPubKeyMan(storage, id, descriptor, keypool_size, keys, ckeys)); } std::unique_ptr 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 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() diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 0c3dcc6c4d5..0c94862d880 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -301,6 +301,8 @@ private: */ mutable std::map 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 LoadFromStorage(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys); + static std::unique_ptr LoadFromStorage(WalletStorage& storage, const uint256& id, WalletDescriptor& descriptor, int64_t keypool_size, const KeyMap& keys, const CryptedKeyMap& ckeys); static std::unique_ptr CreateFromImport(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size, const FlatSigningProvider& provider); static std::unique_ptr CreateFromMigration(WalletStorage& storage, WalletBatch& batch, WalletDescriptor& descriptor, int64_t keypool_size, const FlatSigningProvider& provider); static std::unique_ptr GenerateNewSingleSig(WalletStorage& storage, WalletBatch& batch, int64_t keypool_size, const CExtKey& master_key, OutputType addr_type, bool internal); diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index abda2bf4cb1..a9710eea90a 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -25,7 +25,7 @@ public: std::string ToCanonicalString() const override { return desc; } std::optional 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(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 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); } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 310e9ea07a1..f81deca52bd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3603,9 +3603,9 @@ void CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc, { std::unique_ptr 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(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(spk_man_pair->second.get()); - if (spk_manager != nullptr && spk_manager->HasWalletDescriptor(desc)) { - return spk_manager; - } + return dynamic_cast(spk_man_pair->second.get()); } return nullptr; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 1a9ebaa3549..93575ef43ff 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -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 diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index 6e50c283dd4..60e136a7e15 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -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; - 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);