foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#28799: wallet: cache descriptor ID to avoid repeated descriptor string creation

Browse source 
5e6bc6d830 test: remove custom rpc timeout for `wallet_miniscript.py`, reorder in test_runner (Sebastian Falbesoner)
f811a24421 wallet: cache descriptor ID to avoid repeated descriptor string creation (Sebastian Falbesoner)

Pull request description:

  Right now a wallet descriptor is converted to its string representation (via `Descriptor::ToString`) repeatedly at different instances:
  - on finding a `DescriptorScriptPubKeyMan` for a given descriptor (`CWallet::GetDescriptorScriptPubKeyMan`, e.g. used by the `importdescriptors` RPC); the string representation is created once for each spkm in the wallet and at each iteration again for the searched descriptor (`DescriptorScriptPubKeyMan::HasWalletDescriptor`)
  - whenever `DescriptorScriptPubKeyMan::GetID()` is called, e.g. in `TopUp` or any instances where a descriptor is written to the DB to determine the database key, also at less obvious places like `FastWalletRescanFilter` etc.

  As there is no good reason to calculate a fixed descriptor's string/ID more than once, add the ID as a field to `WalletDescriptor` and calculate it immediately at initialization (or deserialization). `HasWalletDescriptor` is changed to compare the spkm's and searched descriptor's ID instead of the string to take use of that.

  This speeds up the functional test `wallet_miniscript.py` by a factor of 5-6x on my machine (3m30.95s on master vs. 0m38.02s on PR). The recently introduced "max-size TapMiniscript" test-case introduced a descriptor that takes 2-3 seconds to create a string representation, so the repeated calls to that were significantly hurting the performance.

  Fixes https://github.com/bitcoin/bitcoin/issues/28800.

ACKs for top commit:
  Sjors:
    ACK 5e6bc6d830
  S3RK:
    Code Review ACK 5e6bc6d830
  achow101:
    ACK 5e6bc6d830
  BrandonOdiwuor:
    ACK 5e6bc6d830

Tree-SHA512: 98b43963a5dde6055bb26cecd3b878dadd837d6226af4c84142383310495da80b3c4bd552e73b9107f2f2ff1c11f5e18060c6fd3d9e44bbd5224114c4d245c1c
This commit is contained in:
Andrew Chow 2023-11-06 14:57:54 -05:00
commit 0f5e31ce7d
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
4 changed files with 6 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/wallet/scriptpubkeyman.cpp
View file

@ -2601,7 +2601,7 @@ std::unique_ptr<CKeyMetadata> DescriptorScriptPubKeyMan::GetMetadata(const CTxDe
uint256 DescriptorScriptPubKeyMan::GetID() const
{
LOCK(cs_desc_man);
return DescriptorID(*m_wallet_descriptor.descriptor);
return m_wallet_descriptor.id;
}
void DescriptorScriptPubKeyMan::SetCache(const DescriptorCache& cache)
@ -2655,7 +2655,7 @@ bool DescriptorScriptPubKeyMan::AddCryptedKey(const CKeyID& key_id, const CPubKe
bool DescriptorScriptPubKeyMan::HasWalletDescriptor(const WalletDescriptor& desc) const
{
LOCK(cs_desc_man);
return m_wallet_descriptor.descriptor != nullptr && desc.descriptor != nullptr && m_wallet_descriptor.descriptor->ToString() == desc.descriptor->ToString();
return !m_wallet_descriptor.id.IsNull() && !desc.id.IsNull() && m_wallet_descriptor.id == desc.id;
}
void DescriptorScriptPubKeyMan::WriteDescriptor()

4
src/wallet/walletutil.h
View file

@ -85,6 +85,7 @@ class WalletDescriptor
{
public:
std::shared_ptr<Descriptor> descriptor;
uint256 id; // Descriptor ID (calculated once at descriptor initialization/deserialization)
uint64_t creation_time = 0;
int32_t range_start = 0; // First item in range; start of range, inclusive, i.e. [range_start, range_end). This never changes.
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()
@ -99,6 +100,7 @@ public:
if (!descriptor) {
throw std::ios_base::failure("Invalid descriptor: " + error);
}
id = DescriptorID(*descriptor);
}
SERIALIZE_METHODS(WalletDescriptor, obj)
@ -110,7 +112,7 @@ public:
}
WalletDescriptor() {}
WalletDescriptor(std::shared_ptr<Descriptor> descriptor, uint64_t creation_time, int32_t range_start, int32_t range_end, int32_t next_index) : descriptor(descriptor), creation_time(creation_time), range_start(range_start), range_end(range_end), next_index(next_index) {}
WalletDescriptor(std::shared_ptr<Descriptor> descriptor, uint64_t creation_time, int32_t range_start, int32_t range_end, int32_t next_index) : descriptor(descriptor), id(DescriptorID(*descriptor)), creation_time(creation_time), range_start(range_start), range_end(range_end), next_index(next_index) { }
};
} // namespace wallet

2
test/functional/test_runner.py
View file

@ -105,7 +105,6 @@ BASE_SCRIPTS = [
'feature_maxuploadtarget.py',
'mempool_updatefromblock.py',
'mempool_persist.py --descriptors',
'wallet_miniscript.py --descriptors',
# vv Tests less than 60s vv
'rpc_psbt.py --legacy-wallet',
'rpc_psbt.py --descriptors',
@ -149,6 +148,7 @@ BASE_SCRIPTS = [
'p2p_sendheaders.py',
'wallet_listtransactions.py --legacy-wallet',
'wallet_listtransactions.py --descriptors',
'wallet_miniscript.py --descriptors',
# vv Tests less than 30s vv
'p2p_invalid_messages.py',
'rpc_createmultisig.py',

1
test/functional/wallet_miniscript.py
View file

@ -205,7 +205,6 @@ DESCS_PRIV = [
class WalletMiniscriptTest(BitcoinTestFramework):
def add_options(self, parser):
self.add_wallet_options(parser, legacy=False)
self.rpc_timeout = 480
def set_test_params(self):
self.num_nodes = 1