Merge bitcoin/bitcoin#27920: wallet: bugfix, always use apostrophe for spkm descriptor ID

5df988b test: add coverage for descriptor ID (furszy)
6a9510d wallet: bugfix, always use apostrophe for spkm descriptor ID (furszy)
97a965d refactor: extract descriptor ID calculation from spkm GetID() (furszy)
1d207e3 wallet: do not allow loading descriptor with an invalid ID (furszy)

Pull request description:

  Aiming to fix #27915.

  As we re-write the descriptor's db record every time that
  the wallet is loaded (at `TopUp` time), if the spkm ID differs
  from the one in db, the wallet will enter in an unrecoverable
  corruption state (due to the storage of a descriptor with an ID
  that is not linked to any other descriptor record in DB), and
  no soft version will be able to open it anymore.

  Because we cannot change the past, to stay compatible between
  releases, we need to always use the apostrophe version for the
  spkm IDs.

ACKs for top commit:
  achow101:
    ACK 5df988b
  Sjors:
    tACK 5df988b

Tree-SHA512: f63fc4aac7d21a4e515657471758d28857575e751865bfa359298f8b89b2568970029ca487a873c1786a5716325f453f06cd417ed193f3366417f6e8c2987332

Author: achow101

src/script/descriptor.cpp

@@ -191,8 +191,13 @@ struct PubkeyProvider
     /** Get the size of the generated public key(s) in bytes (33 or 65). */
     virtual size_t GetSize() const = 0;
 
+    enum class StringType {
+        PUBLIC,
+        COMPAT // string calculation that mustn't change over time to stay compatible with previous software versions
+    };
+
     /** Get the descriptor string form. */
-    virtual std::string ToString() const = 0;
+    virtual std::string ToString(StringType type=StringType::PUBLIC) const = 0;
 
     /** Get the descriptor string form including private data (if available in arg). */
     virtual bool ToPrivateString(const SigningProvider& arg, std::string& out) const = 0;
@@ -212,9 +217,11 @@ class OriginPubkeyProvider final : public PubkeyProvider
     std::unique_ptr<PubkeyProvider> m_provider;
     bool m_apostrophe;
 
-    std::string OriginString(bool normalized=false) const
+    std::string OriginString(StringType type, bool normalized=false) const
     {
-        return HexStr(m_origin.fingerprint) + FormatHDKeypath(m_origin.path, /*apostrophe=*/!normalized && m_apostrophe);
+        // If StringType==COMPAT, always use the apostrophe to stay compatible with previous versions
+        bool use_apostrophe = (!normalized && m_apostrophe) || type == StringType::COMPAT;
+        return HexStr(m_origin.fingerprint) + FormatHDKeypath(m_origin.path, use_apostrophe);
     }
 
 public:
@@ -228,12 +235,12 @@ class OriginPubkeyProvider final : public PubkeyProvider
     }
     bool IsRange() const override { return m_provider->IsRange(); }
     size_t GetSize() const override { return m_provider->GetSize(); }
-    std::string ToString() const override { return "[" + OriginString() + "]" + m_provider->ToString(); }
+    std::string ToString(StringType type) const override { return "[" + OriginString(type) + "]" + m_provider->ToString(type); }
     bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override
     {
         std::string sub;
         if (!m_provider->ToPrivateString(arg, sub)) return false;
-        ret = "[" + OriginString() + "]" + std::move(sub);
+        ret = "[" + OriginString(StringType::PUBLIC) + "]" + std::move(sub);
         return true;
     }
     bool ToNormalizedString(const SigningProvider& arg, std::string& ret, const DescriptorCache* cache) const override
@@ -245,9 +252,9 @@ class OriginPubkeyProvider final : public PubkeyProvider
         // and append that to our own origin string.
         if (sub[0] == '[') {
             sub = sub.substr(9);
-            ret = "[" + OriginString(/*normalized=*/true) + std::move(sub);
+            ret = "[" + OriginString(StringType::PUBLIC, /*normalized=*/true) + std::move(sub);
         } else {
-            ret = "[" + OriginString(/*normalized=*/true) + "]" + std::move(sub);
+            ret = "[" + OriginString(StringType::PUBLIC, /*normalized=*/true) + "]" + std::move(sub);
         }
         return true;
     }
@@ -275,7 +282,7 @@ class ConstPubkeyProvider final : public PubkeyProvider
     }
     bool IsRange() const override { return false; }
     size_t GetSize() const override { return m_pubkey.size(); }
-    std::string ToString() const override { return m_xonly ? HexStr(m_pubkey).substr(2) : HexStr(m_pubkey); }
+    std::string ToString(StringType type) const override { return m_xonly ? HexStr(m_pubkey).substr(2) : HexStr(m_pubkey); }
     bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override
     {
         CKey key;
@@ -293,7 +300,7 @@ class ConstPubkeyProvider final : public PubkeyProvider
     }
     bool ToNormalizedString(const SigningProvider& arg, std::string& ret, const DescriptorCache* cache) const override
     {
-        ret = ToString();
+        ret = ToString(StringType::PUBLIC);
         return true;
     }
     bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override
@@ -421,19 +428,20 @@ class BIP32PubkeyProvider final : public PubkeyProvider
 
         return true;
     }
-    std::string ToString(bool normalized) const
+    std::string ToString(StringType type, bool normalized) const
     {
-        const bool use_apostrophe = !normalized && m_apostrophe;
+        // If StringType==COMPAT, always use the apostrophe to stay compatible with previous versions
+        const bool use_apostrophe = (!normalized && m_apostrophe) || type == StringType::COMPAT;
         std::string ret = EncodeExtPubKey(m_root_extkey) + FormatHDKeypath(m_path, /*apostrophe=*/use_apostrophe);
         if (IsRange()) {
             ret += "/*";
             if (m_derive == DeriveType::HARDENED) ret += use_apostrophe ? '\'' : 'h';
         }
         return ret;
     }
-    std::string ToString() const override
+    std::string ToString(StringType type=StringType::PUBLIC) const override
     {
-        return ToString(/*normalized=*/false);
+        return ToString(type, /*normalized=*/false);
     }
     bool ToPrivateString(const SigningProvider& arg, std::string& out) const override
     {
@@ -449,7 +457,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider
     bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache) const override
     {
         if (m_derive == DeriveType::HARDENED) {
-            out = ToString(/*normalized=*/true);
+            out = ToString(StringType::PUBLIC, /*normalized=*/true);
 
             return true;
         }
@@ -556,6 +564,7 @@ class DescriptorImpl : public Descriptor
         PUBLIC,
         PRIVATE,
         NORMALIZED,
+        COMPAT, // string calculation that mustn't change over time to stay compatible with previous software versions
     };
 
     bool IsSolvable() const override
@@ -607,6 +616,9 @@ class DescriptorImpl : public Descriptor
                 case StringType::PUBLIC:
                     tmp = pubkey->ToString();
                     break;
+                case StringType::COMPAT:
+                    tmp = pubkey->ToString(PubkeyProvider::StringType::COMPAT);
+                    break;
             }
             ret += tmp;
         }
@@ -617,10 +629,10 @@ class DescriptorImpl : public Descriptor
         return true;
     }
 
-    std::string ToString() const final
+    std::string ToString(bool compat_format) const final
     {
         std::string ret;
-        ToStringHelper(nullptr, ret, StringType::PUBLIC);
+        ToStringHelper(nullptr, ret, compat_format ? StringType::COMPAT : StringType::PUBLIC);
         return AddChecksum(ret);
     }
 
@@ -1778,6 +1790,14 @@ std::unique_ptr<Descriptor> InferDescriptor(const CScript& script, const Signing
     return InferScript(script, ParseScriptContext::TOP, provider);
 }
 
+uint256 DescriptorID(const Descriptor& desc)
+{
+    std::string desc_str = desc.ToString(/*compat_format=*/true);
+    uint256 id;
+    CSHA256().Write((unsigned char*)desc_str.data(), desc_str.size()).Finalize(id.begin());
+    return id;
+}
+
 void DescriptorCache::CacheParentExtPubKey(uint32_t key_exp_pos, const CExtPubKey& xpub)
 {
     m_parent_xpubs[key_exp_pos] = xpub;

src/script/descriptor.h

@@ -106,7 +106,7 @@ struct Descriptor {
     virtual bool IsSolvable() const = 0;
 
     /** Convert the descriptor back to a string, undoing parsing. */
-    virtual std::string ToString() const = 0;
+    virtual std::string ToString(bool compat_format=false) const = 0;
 
     /** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */
     virtual bool IsSingleType() const = 0;
@@ -182,4 +182,9 @@ std::string GetDescriptorChecksum(const std::string& descriptor);
  */
 std::unique_ptr<Descriptor> InferDescriptor(const CScript& script, const SigningProvider& provider);
 
+/** Unique identifier that may not change over time, unless explicitly marked as not backwards compatible.
+*   This is not part of BIP 380, not guaranteed to be interoperable and should not be exposed to the user.
+*/
+uint256 DescriptorID(const Descriptor& desc);
+
 #endif // BITCOIN_SCRIPT_DESCRIPTOR_H