Merge bitcoin/bitcoin#36230: wallet: Improve `HasWalletDescriptor` performance and other canonical descriptor string followups

### src/script/descriptor.cpp
@@ -266,6 +266,26 @@ struct PubkeyProvider
 
     /** Whether this PubkeyProvider can always provide a public key without cache or private key arguments */
     virtual bool CanSelfExpand() const = 0;
+
+protected:
+    static bool DetermineApostropheUse(StringType type, bool normalized, bool public_apostrophe)
+    {
+        bool use_apostrophe{false};
+        switch (type) {
+        case StringType::COMPAT:
+            // COMPAT always uses apostrophe to stay compatible with previous versions
+            use_apostrophe = true;
+            break;
+        case StringType::CANONICAL:
+            // CANONICAL always uses h
+            use_apostrophe = false;
+            break;
+        case StringType::PUBLIC:
+            use_apostrophe = !normalized && public_apostrophe;
+            break;
+        } // no default case, so the compiler can warn about missing cases
+        return use_apostrophe;
+    }
 };
 
 class OriginPubkeyProvider final : public PubkeyProvider
@@ -276,8 +296,7 @@ class OriginPubkeyProvider final : public PubkeyProvider
 
     std::string OriginString(StringType type, bool normalized=false) const
     {
-        // If StringType==COMPAT, always use the apostrophe to stay compatible with previous versions
-        bool use_apostrophe = (type != StringType::CANONICAL && !normalized && m_apostrophe) || type == StringType::COMPAT;
+        bool use_apostrophe{DetermineApostropheUse(type, normalized, m_apostrophe)};
         return HexStr(m_origin.fingerprint) + FormatHDKeypath(m_origin.path, use_apostrophe);
     }
 
@@ -526,8 +545,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider
     }
     std::string ToString(StringType type, bool normalized) const
     {
-        // If StringType==COMPAT, always use the apostrophe to stay compatible with previous versions
-        const bool use_apostrophe = (type != StringType::CANONICAL && !normalized && m_apostrophe) || type == StringType::COMPAT;
+        bool use_apostrophe{DetermineApostropheUse(type, normalized, m_apostrophe)};
         std::string ret = EncodeExtPubKey(m_root_extkey) + FormatHDKeypath(m_path, /*apostrophe=*/use_apostrophe);
         if (IsRange()) {
             ret += "/*";

### src/script/descriptor.h
@@ -256,7 +256,7 @@ std::unique_ptr<Descriptor> InferDescriptor(const CScript& script, const Signing
  * Due to the hash's usage in previous versions, the COMPAT string is computed with some quirks.
  *
  * The hash is the sha256 of the public descriptor using apostrophes as the hardened indicator, except inside of
- * Miniscript expressions, where "h" is the hardened indicator.
+ * Miniscript expressions, where the public serialization is used as provided.
 */
 uint256 CompatDescriptorHash(const Descriptor& desc);
 

### src/wallet/scriptpubkeyman.cpp
@@ -1529,8 +1529,7 @@ void DescriptorScriptPubKeyMan::Load()
 bool DescriptorScriptPubKeyMan::HasWalletDescriptor(const WalletDescriptor& desc) const
 {
     LOCK(cs_desc_man);
-    // Compare by using the canonical string to make the hardened indicators consistent for comparison
-    return m_wallet_descriptor.descriptor->ToCanonicalString() == desc.descriptor->ToCanonicalString();
+    return m_wallet_descriptor.IsCanonicallyEquivalent(desc);
 }
 
 void DescriptorScriptPubKeyMan::WriteDescriptor()

### src/wallet/test/wallet_tests.cpp
@@ -1046,8 +1046,8 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup)
     vw << int32_t{1};
 
     SpanReader vr{malformed_record};
-    WalletDescriptor w_desc;
-    BOOST_CHECK_EXCEPTION(vr >> w_desc, std::ios_base::failure, malformed_descriptor);
+    std::optional<WalletDescriptor> w_desc;
+    BOOST_CHECK_EXCEPTION(w_desc.emplace(WalletDescriptor::FromStream(deserialize, vr)), std::ios_base::failure, malformed_descriptor);
 }
 
 //! Test CWallet::CreateNew() and its behavior handling potential race

### src/wallet/walletdb.cpp
@@ -769,9 +769,9 @@ static DBErrors LoadDescriptorWalletRecords(CWallet* pwallet, DatabaseBatch& bat
 
         uint256 id;
         key >> id;
-        WalletDescriptor desc;
+        std::optional<WalletDescriptor> desc;
         try {
-            value >> desc;
+            desc.emplace(WalletDescriptor::FromStream(deserialize, value));
         } catch (const std::ios_base::failure& e) {
             strErr = strprintf("Error: Unrecognized descriptor found in wallet %s. ", pwallet->GetName());
             strErr += (last_client > CLIENT_VERSION) ? "The wallet might have been created on a newer version. " :
@@ -837,7 +837,7 @@ static DBErrors LoadDescriptorWalletRecords(CWallet* pwallet, DatabaseBatch& bat
         result = std::max(result, lh_cache_res.m_result);
 
         // Set the cache to the WalletDescriptor
-        desc.cache = cache;
+        desc->cache = cache;
 
         // Get unencrypted keys
         KeyMap keys;
@@ -906,7 +906,7 @@ static DBErrors LoadDescriptorWalletRecords(CWallet* pwallet, DatabaseBatch& bat
         num_ckeys = ckey_res.m_records;
 
         try {
-            pwallet->LoadDescriptorScriptPubKeyMan(id, desc, keys, ckeys);
+            pwallet->LoadDescriptorScriptPubKeyMan(id, *desc, keys, ckeys);
         } catch (std::runtime_error& e) {
             strErr = e.what();
             return DBErrors::CORRUPT;

### src/wallet/walletutil.cpp
@@ -87,7 +87,7 @@ WalletDescriptor GenerateWalletDescriptor(const CExtPubKey& master_key, const Ou
 
 void WalletDescriptor::UpdateFrom(const WalletDescriptor& other)
 {
-    if (descriptor->ToCanonicalString() != other.descriptor->ToCanonicalString()) {
+    if (!IsCanonicallyEquivalent(other)) {
         return;
     }
     range_start = other.range_start;
@@ -97,4 +97,19 @@ void WalletDescriptor::UpdateFrom(const WalletDescriptor& other)
     cache = other.cache;
 }
 
+uint256 WalletDescriptor::GetCanonicalHash() const
+{
+    if (!m_canonical_hash) {
+        m_canonical_hash.emplace();
+        std::string canonical = descriptor->ToCanonicalString();
+        CSHA256().Write((unsigned char*)canonical.data(), canonical.size()).Finalize(m_canonical_hash->begin());
+    }
+    return *m_canonical_hash;
+}
+
+bool WalletDescriptor::IsCanonicallyEquivalent(const WalletDescriptor& other) const
+{
+    return GetCanonicalHash() == other.GetCanonicalHash();
+}
+
 } // namespace wallet

### src/wallet/walletutil.h
@@ -66,8 +66,13 @@ class WalletDescriptor
     int32_t range_start = 0; // First item in range; start of range, inclusive, i.e. [range_start, range_end). This never changes.
     int32_t next_index = 0; // Position of the next item to generate
     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()
+
+    mutable std::optional<uint256> m_canonical_hash; // Hash of the canonical string, used as a shortcut for comparing canonical strings
+
+    uint256 GetCanonicalHash() const;
+
 public:
-    std::shared_ptr<Descriptor> descriptor;
+    const std::shared_ptr<const Descriptor> descriptor;
     uint64_t creation_time = 0;
     DescriptorCache cache;
 
@@ -96,37 +101,50 @@ class WalletDescriptor
         range_end = end;
     }
 
-    void DeserializeDescriptor(const std::string& str)
+    template <typename Stream>
+    void Serialize(Stream& s) const
     {
+        std::string descriptor_str = descriptor->ToString();
+        s << descriptor_str << creation_time << next_index << range_start << range_end;
+    }
+
+    template <typename Stream>
+    static WalletDescriptor FromStream(deserialize_type, Stream& s)
+    {
+        std::string descriptor_str;
+        uint64_t creation_time;
+        int32_t next_index, range_start, range_end;
+        s >> descriptor_str >> creation_time >> next_index >> range_start >> range_end;
+
         std::string error;
         FlatSigningProvider keys;
-        auto descs = Parse(str, keys, error, true);
+        auto descs = Parse(descriptor_str, keys, error, true);
         if (descs.empty()) {
             throw std::ios_base::failure("Invalid descriptor: " + error);
         }
         if (descs.size() > 1) {
             throw std::ios_base::failure("Can't load a multipath descriptor from databases");
         }
-        descriptor = std::move(descs.at(0));
-    }
-
-    SERIALIZE_METHODS(WalletDescriptor, obj)
-    {
-        std::string descriptor_str;
-        SER_WRITE(obj, descriptor_str = obj.descriptor->ToString());
-        READWRITE(descriptor_str, obj.creation_time, obj.next_index, obj.range_start, obj.range_end);
-        SER_READ(obj, obj.DeserializeDescriptor(descriptor_str));
+        return WalletDescriptor(std::move(descs.at(0)), creation_time, range_start, range_end, next_index);
     }
 
-    WalletDescriptor() = default;
+    WalletDescriptor() = delete;
     WalletDescriptor(std::shared_ptr<Descriptor> descriptor, uint64_t creation_time, int32_t range_start, int32_t range_end, int32_t next_index)
     : range_start(descriptor->IsRange() ? range_start : 0),
       next_index(next_index),
       range_end(descriptor->IsRange() ? range_end : 1),
       descriptor(descriptor),
-      creation_time(creation_time) {}
+      creation_time(creation_time)
+    {}
 
+    /** Replaces all metadata (range, start, end, creation time), and cache from another WalletDescriptor if it has the same canonical descriptor string.
+     *  The descriptor itself is not replaced to preserve existing serialization to maintain compatibility with previous software versions that expect
+     *  specific serialization.
+     */
     void UpdateFrom(const WalletDescriptor& other);
+
+    // Compare by using the canonical string to make the hardened indicators consistent for comparison
+    bool IsCanonicallyEquivalent(const WalletDescriptor& other) const;
 };
 
 WalletDescriptor GenerateWalletDescriptor(const CExtPubKey& master_key, const OutputType& output_type, bool internal);

### test/functional/wallet_backwards_compatibility.py
@@ -36,11 +36,12 @@
 class BackwardsCompatibilityTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
-        self.num_nodes = 10
+        self.num_nodes = 11
         # Add new version after each release:
         self.extra_args = [
             ["-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # Pre-release: use to mine blocks. noban for immediate tx relay
             ["-nowallet", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # Pre-release: use to receive coins, swap wallets, etc
+            ["-nowallet", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v31.1
             ["-nowallet", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v31.0
             ["-nowallet", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v30.2
             ["-nowallet", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v25.0
@@ -60,6 +61,7 @@ def setup_nodes(self):
         self.add_nodes(self.num_nodes, extra_args=self.extra_args, versions=[
             None,
             None,
+            310100,
             310000,
             300200,
             250000,
@@ -365,11 +367,11 @@ def run_test(self):
                     continue
                 # Also try to reopen on master after opening on old
                 for n in [node, node_master]:
-                    # 31.0 has a descriptor id calculation incompatibility.
+                    # 31.0 and 31.1 have a descriptor id calculation incompatibility.
                     # Miniscript descriptors imported into node versions other than 31.0 will
-                    # result in wallets that cannot be loaded into 31.0.
+                    # result in wallets that cannot be loaded into 31.0 and 31.1.
                     # These wallets will emit a "Wallet corrupted" error.
-                    if wallet_name == "miniscript" and n.version == 310000:
+                    if wallet_name == "miniscript" and n.version in (310000, 310100):
                         assert_raises_rpc_error(-4, "Wallet corrupted", n.loadwallet, wallet_name)
                         continue
 
@@ -398,11 +400,8 @@ def run_test(self):
                         assert_equal(info['private_keys_enabled'], True)
                         assert_equal(info['keypoolsize'], 0)
                     elif wallet_name == "miniscript":
-                        for desc in wallet.listdescriptors()["descriptors"]:
-                            if desc["desc"].startswith("wsh(or_b(pk"):
-                                break
-                        else:
-                            assert False, "Did not find miniscript descriptor"
+                        descs = wallet.listdescriptors()["descriptors"]
+                        assert any(desc["desc"].startswith("wsh(or_b(pk") for desc in descs), "Miniscript descriptor missing"
 
                     # Copy back to master
                     wallet.unloadwallet()
@@ -466,11 +465,8 @@ def get_flags(conn):
             assert_equal(info["desc"], descsum_create(descriptor))
 
             if self.major_version_at_least(node, 24):
-                for desc in wallet.listdescriptors()["descriptors"]:
-                    if desc["desc"].startswith("wsh(or_b(pk"):
-                        break
-                else:
-                    assert False, "Did not find miniscript descriptor"
+                descs = wallet.listdescriptors()["descriptors"]
+                assert any(desc["desc"].startswith("wsh(or_b(pk") for desc in descs), "Miniscript descriptor missing"
 
             # Make backup so the wallet can be copied back to old node
             down_wallet_name = f"re_down_{node.version}"

### test/get_previous_releases.py
@@ -109,6 +109,15 @@
     "56824dd705bc2a3b22d42e8aa02ed53498d491ff7c2c8aa96831333871887ead": {"tag": "v31.0", "archive": "bitcoin-31.0-x86_64-apple-darwin.tar.gz"},
     "d3e4c58a35b1d0a97a457462c94f55501ad167c660c245cb1ffa565641c65074": {"tag": "v31.0", "archive": "bitcoin-31.0-x86_64-linux-gnu.tar.gz"},
     "82fd2c504a0f20a31d4d13bd407783d6fc7bf17622d0ce85228a9b92694e03f0": {"tag": "v31.0", "archive": "bitcoin-31.0-win64.zip"},
+
+    "dcf1873f2208ba4f962f3398d47e154c39c0084be8f4553e05c940d0ace3d004": {"tag": "v31.1", "archive": "bitcoin-31.1-aarch64-linux-gnu.tar.gz"},
+    "66b2b45359efa161031a49898f96aa7cf1455db46ca6102acd16a7197dc3b96f": {"tag": "v31.1", "archive": "bitcoin-31.1-arm-linux-gnueabihf.tar.gz"},
+    "16a097c09fbd7eb78b240ce1dae123663ea2e5e377cfd6a951e71e227e23cf2f": {"tag": "v31.1", "archive": "bitcoin-31.1-arm64-apple-darwin.tar.gz"},
+    "f81dd017a551c5af7fa2d6fa67b885077a8353322a8019e8fd538366bae1eff7": {"tag": "v31.1", "archive": "bitcoin-31.1-powerpc64-linux-gnu.tar.gz"},
+    "8a9213348a111438472653b8bd46c12184c60cc35ce0c2af02b853de4297cf94": {"tag": "v31.1", "archive": "bitcoin-31.1-riscv64-linux-gnu.tar.gz"},
+    "bc506958d0f387c1ea770bdc7c7192a505fa645ff62cabcc7761fa7eb89e867e": {"tag": "v31.1", "archive": "bitcoin-31.1-x86_64-apple-darwin.tar.gz"},
+    "b80d9c3e04da78fb6f0569685673418cf686fadba9042d926d13fb87ff503f9e": {"tag": "v31.1", "archive": "bitcoin-31.1-x86_64-linux-gnu.tar.gz"},
+    "c99ef173471c58e6766d9eebd12e6c35349082eeed3939bc99eed58ef57db587": {"tag": "v31.1", "archive": "bitcoin-31.1-win64.zip"},
 }