From 181181019c5baa3e2d5b675d1843a45aa028781c Mon Sep 17 00:00:00 2001 From: S3RK <1466284+S3RK@users.noreply.github.com> Date: Tue, 29 Jun 2021 08:29:25 +0200 Subject: refactor: remove m_internal from DescriptorSPKman Descriptor in itself is neither internal or external. It's responsibility of a wallet to assign and manage descriptors for a specific purpose. Duplicating such information could lead to inconsistencies and unexpected behaviour. --- src/wallet/external_signer_scriptpubkeyman.h | 4 ++-- src/wallet/scriptpubkeyman.cpp | 20 +++----------------- src/wallet/scriptpubkeyman.h | 19 ++++--------------- src/wallet/wallet.cpp | 16 ++++++++++------ 4 files changed, 19 insertions(+), 40 deletions(-) (limited to 'src') diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 8eed947b7b..61df3d0015 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -15,8 +15,8 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan ExternalSignerScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor) : DescriptorScriptPubKeyMan(storage, descriptor) {} - ExternalSignerScriptPubKeyMan(WalletStorage& storage, bool internal) - : DescriptorScriptPubKeyMan(storage, internal) + ExternalSignerScriptPubKeyMan(WalletStorage& storage) + : DescriptorScriptPubKeyMan(storage) {} /** Provide a descriptor at setup time diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 44c3912544..5467495fff 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1613,12 +1613,10 @@ std::set LegacyScriptPubKeyMan::GetKeys() const return set_address; } -void LegacyScriptPubKeyMan::SetInternal(bool internal) {} - bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) { // Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later - if (!CanGetAddresses(m_internal)) { + if (!CanGetAddresses()) { error = "No addresses available"; return false; } @@ -1894,7 +1892,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const } } -bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type) +bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal) { if (addr_type == OutputType::BECH32M) { // Don't allow setting up taproot descriptors yet @@ -1942,7 +1940,7 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ desc_prefix += "/0'"; } - std::string internal_path = m_internal ? "/1" : "/0"; + std::string internal_path = internal ? "/1" : "/0"; std::string desc_str = desc_prefix + "/0'" + internal_path + desc_suffix; // Make the descriptor @@ -1997,13 +1995,6 @@ int64_t DescriptorScriptPubKeyMan::GetOldestKeyPoolTime() const return 0; } -size_t DescriptorScriptPubKeyMan::KeypoolCountExternalKeys() const -{ - if (m_internal) { - return 0; - } - return GetKeyPoolSize(); -} unsigned int DescriptorScriptPubKeyMan::GetKeyPoolSize() const { @@ -2205,11 +2196,6 @@ uint256 DescriptorScriptPubKeyMan::GetID() const return id; } -void DescriptorScriptPubKeyMan::SetInternal(bool internal) -{ - this->m_internal = internal; -} - void DescriptorScriptPubKeyMan::SetCache(const DescriptorCache& cache) { LOCK(cs_desc_man); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index b2ca354b0a..128197e567 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -216,7 +216,6 @@ public: virtual int64_t GetOldestKeyPoolTime() const { return GetTime(); } - virtual size_t KeypoolCountExternalKeys() const { return 0; } virtual unsigned int GetKeyPoolSize() const { return 0; } virtual int64_t GetTimeFirstKey() const { return 0; } @@ -239,8 +238,6 @@ public: virtual uint256 GetID() const { return uint256(); } - virtual void SetInternal(bool internal) {} - /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ template void WalletLogPrintf(std::string fmt, Params... parameters) const { @@ -386,7 +383,7 @@ public: void RewriteDB() override; int64_t GetOldestKeyPoolTime() const override; - size_t KeypoolCountExternalKeys() const override; + size_t KeypoolCountExternalKeys() const; unsigned int GetKeyPoolSize() const override; int64_t GetTimeFirstKey() const override; @@ -405,8 +402,6 @@ public: uint256 GetID() const override; - void SetInternal(bool internal) override; - // Map from Key ID to key metadata. std::map mapKeyMetadata GUARDED_BY(cs_KeyStore); @@ -533,8 +528,6 @@ private: PubKeyMap m_map_pubkeys GUARDED_BY(cs_desc_man); int32_t m_max_cached_index = -1; - bool m_internal = false; - KeyMap m_map_keys GUARDED_BY(cs_desc_man); CryptedKeyMap m_map_crypted_keys GUARDED_BY(cs_desc_man); @@ -560,9 +553,8 @@ public: : ScriptPubKeyMan(storage), m_wallet_descriptor(descriptor) {} - DescriptorScriptPubKeyMan(WalletStorage& storage, bool internal) - : ScriptPubKeyMan(storage), - m_internal(internal) + DescriptorScriptPubKeyMan(WalletStorage& storage) + : ScriptPubKeyMan(storage) {} mutable RecursiveMutex cs_desc_man; @@ -587,7 +579,7 @@ public: bool IsHDEnabled() const override; //! Setup descriptors based on the given CExtkey - bool SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type); + bool SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal); /** Provide a descriptor at setup time * Returns false if already setup or setup fails, true if setup is successful @@ -597,7 +589,6 @@ public: bool HavePrivateKeys() const override; int64_t GetOldestKeyPoolTime() const override; - size_t KeypoolCountExternalKeys() const override; unsigned int GetKeyPoolSize() const override; int64_t GetTimeFirstKey() const override; @@ -616,8 +607,6 @@ public: uint256 GetID() const override; - void SetInternal(bool internal) override; - void SetCache(const DescriptorCache& cache); bool AddKey(const CKeyID& key_id, const CKey& key); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c2586b60b8..5b108a5069 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2079,9 +2079,14 @@ size_t CWallet::KeypoolCountExternalKeys() const { AssertLockHeld(cs_wallet); + auto legacy_spk_man = GetLegacyScriptPubKeyMan(); + if (legacy_spk_man) { + return legacy_spk_man->KeypoolCountExternalKeys(); + } + unsigned int count = 0; - for (auto spk_man : GetActiveScriptPubKeyMans()) { - count += spk_man->KeypoolCountExternalKeys(); + for (auto spk_man : m_external_spk_managers) { + count += spk_man.second->GetKeyPoolSize(); } return count; @@ -3097,7 +3102,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() // TODO: Setup taproot (bech32m) descriptors by default continue; } - auto spk_manager = std::unique_ptr(new DescriptorScriptPubKeyMan(*this, internal)); + auto spk_manager = std::unique_ptr(new DescriptorScriptPubKeyMan(*this)); if (IsCrypted()) { if (IsLocked()) { throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); @@ -3106,7 +3111,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors"); } } - spk_manager->SetupDescriptorGeneration(master_key, t); + spk_manager->SetupDescriptorGeneration(master_key, t, internal); uint256 id = spk_manager->GetID(); m_spk_managers[id] = std::move(spk_manager); AddActiveScriptPubKeyMan(id, t, internal); @@ -3132,7 +3137,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() continue; } OutputType t = *desc->GetOutputType(); - auto spk_manager = std::unique_ptr(new ExternalSignerScriptPubKeyMan(*this, internal)); + auto spk_manager = std::unique_ptr(new ExternalSignerScriptPubKeyMan(*this)); spk_manager->SetupDescriptor(std::move(desc)); uint256 id = spk_manager->GetID(); m_spk_managers[id] = std::move(spk_manager); @@ -3156,7 +3161,6 @@ void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool intern WalletLogPrintf("Setting spkMan to active: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast(type), static_cast(internal)); auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers; auto spk_man = m_spk_managers.at(id).get(); - spk_man->SetInternal(internal); spk_mans[type] = spk_man; NotifyCanGetAddressesChanged(); -- cgit v1.2.3