From f811a24421fe102a96ab8f75427cc6a3c5503dc3 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Sun, 5 Nov 2023 23:01:32 +0100
Subject: [PATCH] wallet: cache descriptor ID to avoid repeated descriptor
 string creation

Right now a wallet descriptor is converted to it's 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 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.
---
 src/wallet/scriptpubkeyman.cpp | 4 ++--
 src/wallet/walletutil.h        | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index bc3327cdb22..d2b2801aa88 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -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()
diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h
index c5975144c13..7ad3ffe9e4a 100644
--- a/src/wallet/walletutil.h
+++ b/src/wallet/walletutil.h
@@ -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