phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-25 18:53:23 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#20191: wallet, refactor: make DescriptorScriptPubKeyMan agnostic of internal flag

Browse source 
181181019c refactor: remove m_internal from DescriptorSPKman (S3RK)

Pull request description:

  Rationale: improve consistency between `CWallet` and `DescriptorScriptPubKeyMan`; simplify `ScriptPubKeyMan` interface.

  Descriptor in itself is neither internal or external. It's responsibility of a wallet to assign and manage descriptors for a specific purpose. Duplicating information about internalness of a descriptor could lead to inconsistencies and unexpected behaviour (for example misreporting keypool size).

ACKs for top commit:
  instagibbs:
    reACK 181181019c
  achow101:
    reACK 181181019c

Tree-SHA512: d5613b7f6795b290bfa0fd8cb0536de1714d0cf72cba402266bd06d550758ebad690b54fc0a336a1c7414b5814aa4a37c90a6ae89926474a97d30956d7e034ff
This commit is contained in:
fanquake 2021-07-01 10:08:33 +08:00
commit 5a95c5179c
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
4 changed files with 19 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/wallet/external_signer_scriptpubkeyman.h
View file

@ -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

20
src/wallet/scriptpubkeyman.cpp
View file

@ -1613,12 +1613,10 @@ std::set<CKeyID> 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;
}
@ -1876,7 +1874,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
@ -1924,7 +1922,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
@ -1979,13 +1977,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
{
@ -2187,11 +2178,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);

19
src/wallet/scriptpubkeyman.h
View file

@ -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<typename... Params>
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<CKeyID, CKeyMetadata> 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);

16
src/wallet/wallet.cpp
View file

@ -2094,9 +2094,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;
@ -3112,7 +3117,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
// TODO: Setup taproot (bech32m) descriptors by default
continue;
}
auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, internal));
auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this));
if (IsCrypted()) {
if (IsLocked()) {
throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors");
@ -3121,7 +3126,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);
@ -3147,7 +3152,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
continue;
}
OutputType t = *desc->GetOutputType();
auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, internal));
auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this));
spk_manager->SetupDescriptor(std::move(desc));
uint256 id = spk_manager->GetID();
m_spk_managers[id] = std::move(spk_manager);
@ -3176,7 +3181,6 @@ void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool intern
auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers;
auto& spk_mans_other = internal ? m_external_spk_managers : m_internal_spk_managers;
auto spk_man = m_spk_managers.at(id).get();
spk_man->SetInternal(internal);
spk_mans[type] = spk_man;
if (spk_mans_other[type] == spk_man) {