Skip to content

Commit 1811810

Browse files
S3RKS3RK
committed
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.
1 parent 3f56ef7 commit 1811810

File tree

4 files changed

+19
-40
lines changed

4 files changed

+19
-40
lines changed

‎src/wallet/external_signer_scriptpubkeyman.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
1515
ExternalSignerScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor)
1616
: DescriptorScriptPubKeyMan(storage, descriptor)
1717
{}
18-
ExternalSignerScriptPubKeyMan(WalletStorage& storage, bool internal)
19-
: DescriptorScriptPubKeyMan(storage, internal)
18+
ExternalSignerScriptPubKeyMan(WalletStorage& storage)
19+
: DescriptorScriptPubKeyMan(storage)
2020
{}
2121

2222
/** Provide a descriptor at setup time

‎src/wallet/scriptpubkeyman.cpp

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,12 +1613,10 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const
16131613
return set_address;
16141614
}
16151615

1616-
void LegacyScriptPubKeyMan::SetInternal(bool internal) {}
1617-
16181616
bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error)
16191617
{
16201618
// Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later
1621-
if (!CanGetAddresses(m_internal)) {
1619+
if (!CanGetAddresses()) {
16221620
error = "No addresses available";
16231621
return false;
16241622
}
@@ -1894,7 +1892,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const
18941892
}
18951893
}
18961894

1897-
bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type)
1895+
bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal)
18981896
{
18991897
if (addr_type == OutputType::BECH32M) {
19001898
// Don't allow setting up taproot descriptors yet
@@ -1942,7 +1940,7 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_
19421940
desc_prefix += "/0'";
19431941
}
19441942

1945-
std::string internal_path = m_internal ? "/1" : "/0";
1943+
std::string internal_path = internal ? "/1" : "/0";
19461944
std::string desc_str = desc_prefix + "/0'" + internal_path + desc_suffix;
19471945

19481946
// Make the descriptor
@@ -1997,13 +1995,6 @@ int64_t DescriptorScriptPubKeyMan::GetOldestKeyPoolTime() const
19971995
return 0;
19981996
}
19991997

2000-
size_t DescriptorScriptPubKeyMan::KeypoolCountExternalKeys() const
2001-
{
2002-
if (m_internal) {
2003-
return 0;
2004-
}
2005-
return GetKeyPoolSize();
2006-
}
20071998

20081999
unsigned int DescriptorScriptPubKeyMan::GetKeyPoolSize() const
20092000
{
@@ -2205,11 +2196,6 @@ uint256 DescriptorScriptPubKeyMan::GetID() const
22052196
return id;
22062197
}
22072198

2208-
void DescriptorScriptPubKeyMan::SetInternal(bool internal)
2209-
{
2210-
this->m_internal = internal;
2211-
}
2212-
22132199
void DescriptorScriptPubKeyMan::SetCache(const DescriptorCache& cache)
22142200
{
22152201
LOCK(cs_desc_man);

‎src/wallet/scriptpubkeyman.h

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ class ScriptPubKeyMan
216216

217217
virtual int64_t GetOldestKeyPoolTime() const { return GetTime(); }
218218

219-
virtual size_t KeypoolCountExternalKeys() const { return 0; }
220219
virtual unsigned int GetKeyPoolSize() const { return 0; }
221220

222221
virtual int64_t GetTimeFirstKey() const { return 0; }
@@ -239,8 +238,6 @@ class ScriptPubKeyMan
239238

240239
virtual uint256 GetID() const { return uint256(); }
241240

242-
virtual void SetInternal(bool internal) {}
243-
244241
/** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */
245242
template<typename... Params>
246243
void WalletLogPrintf(std::string fmt, Params... parameters) const {
@@ -386,7 +383,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
386383
void RewriteDB() override;
387384

388385
int64_t GetOldestKeyPoolTime() const override;
389-
size_t KeypoolCountExternalKeys() const override;
386+
size_t KeypoolCountExternalKeys() const;
390387
unsigned int GetKeyPoolSize() const override;
391388

392389
int64_t GetTimeFirstKey() const override;
@@ -405,8 +402,6 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
405402

406403
uint256 GetID() const override;
407404

408-
void SetInternal(bool internal) override;
409-
410405
// Map from Key ID to key metadata.
411406
std::map<CKeyID, CKeyMetadata> mapKeyMetadata GUARDED_BY(cs_KeyStore);
412407

@@ -533,8 +528,6 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
533528
PubKeyMap m_map_pubkeys GUARDED_BY(cs_desc_man);
534529
int32_t m_max_cached_index = -1;
535530

536-
bool m_internal = false;
537-
538531
KeyMap m_map_keys GUARDED_BY(cs_desc_man);
539532
CryptedKeyMap m_map_crypted_keys GUARDED_BY(cs_desc_man);
540533

@@ -560,9 +553,8 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
560553
: ScriptPubKeyMan(storage),
561554
m_wallet_descriptor(descriptor)
562555
{}
563-
DescriptorScriptPubKeyMan(WalletStorage& storage, bool internal)
564-
: ScriptPubKeyMan(storage),
565-
m_internal(internal)
556+
DescriptorScriptPubKeyMan(WalletStorage& storage)
557+
: ScriptPubKeyMan(storage)
566558
{}
567559

568560
mutable RecursiveMutex cs_desc_man;
@@ -587,7 +579,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
587579
bool IsHDEnabled() const override;
588580

589581
//! Setup descriptors based on the given CExtkey
590-
bool SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type);
582+
bool SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal);
591583

592584
/** Provide a descriptor at setup time
593585
* Returns false if already setup or setup fails, true if setup is successful
@@ -597,7 +589,6 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
597589
bool HavePrivateKeys() const override;
598590

599591
int64_t GetOldestKeyPoolTime() const override;
600-
size_t KeypoolCountExternalKeys() const override;
601592
unsigned int GetKeyPoolSize() const override;
602593

603594
int64_t GetTimeFirstKey() const override;
@@ -616,8 +607,6 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
616607

617608
uint256 GetID() const override;
618609

619-
void SetInternal(bool internal) override;
620-
621610
void SetCache(const DescriptorCache& cache);
622611

623612
bool AddKey(const CKeyID& key_id, const CKey& key);

‎src/wallet/wallet.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,9 +2079,14 @@ size_t CWallet::KeypoolCountExternalKeys() const
20792079
{
20802080
AssertLockHeld(cs_wallet);
20812081

2082+
auto legacy_spk_man = GetLegacyScriptPubKeyMan();
2083+
if (legacy_spk_man) {
2084+
return legacy_spk_man->KeypoolCountExternalKeys();
2085+
}
2086+
20822087
unsigned int count = 0;
2083-
for (auto spk_man : GetActiveScriptPubKeyMans()) {
2084-
count += spk_man->KeypoolCountExternalKeys();
2088+
for (auto spk_man : m_external_spk_managers) {
2089+
count += spk_man.second->GetKeyPoolSize();
20852090
}
20862091

20872092
return count;
@@ -3097,7 +3102,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
30973102
// TODO: Setup taproot (bech32m) descriptors by default
30983103
continue;
30993104
}
3100-
auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, internal));
3105+
auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this));
31013106
if (IsCrypted()) {
31023107
if (IsLocked()) {
31033108
throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors");
@@ -3106,7 +3111,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
31063111
throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors");
31073112
}
31083113
}
3109-
spk_manager->SetupDescriptorGeneration(master_key, t);
3114+
spk_manager->SetupDescriptorGeneration(master_key, t, internal);
31103115
uint256 id = spk_manager->GetID();
31113116
m_spk_managers[id] = std::move(spk_manager);
31123117
AddActiveScriptPubKeyMan(id, t, internal);
@@ -3132,7 +3137,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
31323137
continue;
31333138
}
31343139
OutputType t = *desc->GetOutputType();
3135-
auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, internal));
3140+
auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this));
31363141
spk_manager->SetupDescriptor(std::move(desc));
31373142
uint256 id = spk_manager->GetID();
31383143
m_spk_managers[id] = std::move(spk_manager);
@@ -3156,7 +3161,6 @@ void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool intern
31563161
WalletLogPrintf("Setting spkMan to active: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(type), static_cast<int>(internal));
31573162
auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers;
31583163
auto spk_man = m_spk_managers.at(id).get();
3159-
spk_man->SetInternal(internal);
31603164
spk_mans[type] = spk_man;
31613165

31623166
NotifyCanGetAddressesChanged();

0 commit comments

Comments
 (0)