Merge bitcoin/bitcoin#26076: Switch hardened derivation marker to h

fe49f06 doc: clarify PR 26076 release note (Sjors Provoost)
bd13dc2 Switch hardened derivation marker to h in descriptors (Sjors Provoost)

Pull request description:

  This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet.

  For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually.

  Both markers can still be parsed.

  The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet.

  See discussion in #15740

ACKs for top commit:
  achow101:
    ACK fe49f06
  darosior:
    re-ACK fe49f06

Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89

Author: achow101

doc/release-notes-26076.md

@@ -0,0 +1,13 @@
+RPC
+---
+
+- The `listdescriptors`, `decodepsbt` and similar RPC methods now show `h` rather than apostrophe (`'`) to indicate
+  hardened derivation. This does not apply when using the `private` parameter, which
+  matches the marker used when descriptor was generated or imported. Newly created
+  wallets use `h`. This change makes it easier to handle descriptor strings manually.
+  E.g. the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`.
+  With this change `listdescriptors` will use `h`, so you can copy-paste the result,
+  without having to add escape characters or switch `'` to 'h' manually.
+  Note that this changes the descriptor checksum.
+  For legacy wallets the `hdkeypath` field in `getaddressinfo` is unchanged,
+  nor is the serialization format of wallet dumps. (#26076)

src/script/descriptor.cpp

@@ -197,7 +197,9 @@ struct PubkeyProvider
     /** Get the descriptor string form including private data (if available in arg). */
     virtual bool ToPrivateString(const SigningProvider& arg, std::string& out) const = 0;
 
-    /** Get the descriptor string form with the xpub at the last hardened derivation */
+    /** Get the descriptor string form with the xpub at the last hardened derivation,
+     *  and always use h for hardened derivation.
+     */
     virtual bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache = nullptr) const = 0;
 
     /** Derive a private key, if private data is available in arg. */
@@ -208,14 +210,15 @@ class OriginPubkeyProvider final : public PubkeyProvider
 {
     KeyOriginInfo m_origin;
     std::unique_ptr<PubkeyProvider> m_provider;
+    bool m_apostrophe;
 
-    std::string OriginString() const
+    std::string OriginString(bool normalized=false) const
     {
-        return HexStr(m_origin.fingerprint) + FormatHDKeypath(m_origin.path);
+        return HexStr(m_origin.fingerprint) + FormatHDKeypath(m_origin.path, /*apostrophe=*/!normalized && m_apostrophe);
     }
 
 public:
-    OriginPubkeyProvider(uint32_t exp_index, KeyOriginInfo info, std::unique_ptr<PubkeyProvider> provider) : PubkeyProvider(exp_index), m_origin(std::move(info)), m_provider(std::move(provider)) {}
+    OriginPubkeyProvider(uint32_t exp_index, KeyOriginInfo info, std::unique_ptr<PubkeyProvider> provider, bool apostrophe) : PubkeyProvider(exp_index), m_origin(std::move(info)), m_provider(std::move(provider)), m_apostrophe(apostrophe) {}
     bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key, KeyOriginInfo& info, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override
     {
         if (!m_provider->GetPubKey(pos, arg, key, info, read_cache, write_cache)) return false;
@@ -242,9 +245,9 @@ class OriginPubkeyProvider final : public PubkeyProvider
         // and append that to our own origin string.
         if (sub[0] == '[') {
             sub = sub.substr(9);
-            ret = "[" + OriginString() + std::move(sub);
+            ret = "[" + OriginString(/*normalized=*/true) + std::move(sub);
         } else {
-            ret = "[" + OriginString() + "]" + std::move(sub);
+            ret = "[" + OriginString(/*normalized=*/true) + "]" + std::move(sub);
         }
         return true;
     }
@@ -312,6 +315,8 @@ class BIP32PubkeyProvider final : public PubkeyProvider
     CExtPubKey m_root_extkey;
     KeyPath m_path;
     DeriveType m_derive;
+    // Whether ' or h is used in harded derivation
+    bool m_apostrophe;
 
     bool GetExtKey(const SigningProvider& arg, CExtKey& ret) const
     {
@@ -348,7 +353,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider
     }
 
 public:
-    BIP32PubkeyProvider(uint32_t exp_index, const CExtPubKey& extkey, KeyPath path, DeriveType derive) : PubkeyProvider(exp_index), m_root_extkey(extkey), m_path(std::move(path)), m_derive(derive) {}
+    BIP32PubkeyProvider(uint32_t exp_index, const CExtPubKey& extkey, KeyPath path, DeriveType derive, bool apostrophe) : PubkeyProvider(exp_index), m_root_extkey(extkey), m_path(std::move(path)), m_derive(derive), m_apostrophe(apostrophe) {}
     bool IsRange() const override { return m_derive != DeriveType::NO; }
     size_t GetSize() const override { return 33; }
     bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key_out, KeyOriginInfo& final_info_out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override
@@ -416,31 +421,36 @@ class BIP32PubkeyProvider final : public PubkeyProvider
 
         return true;
     }
-    std::string ToString() const override
+    std::string ToString(bool normalized) const
     {
-        std::string ret = EncodeExtPubKey(m_root_extkey) + FormatHDKeypath(m_path);
+        const bool use_apostrophe = !normalized && m_apostrophe;
+        std::string ret = EncodeExtPubKey(m_root_extkey) + FormatHDKeypath(m_path, /*apostrophe=*/use_apostrophe);
         if (IsRange()) {
             ret += "/*";
-            if (m_derive == DeriveType::HARDENED) ret += '\'';
+            if (m_derive == DeriveType::HARDENED) ret += use_apostrophe ? '\'' : 'h';
         }
         return ret;
     }
+    std::string ToString() const override
+    {
+        return ToString(/*normalized=*/false);
+    }
     bool ToPrivateString(const SigningProvider& arg, std::string& out) const override
     {
         CExtKey key;
         if (!GetExtKey(arg, key)) return false;
-        out = EncodeExtKey(key) + FormatHDKeypath(m_path);
+        out = EncodeExtKey(key) + FormatHDKeypath(m_path, /*apostrophe=*/m_apostrophe);
         if (IsRange()) {
             out += "/*";
-            if (m_derive == DeriveType::HARDENED) out += '\'';
+            if (m_derive == DeriveType::HARDENED) out += m_apostrophe ? '\'' : 'h';
         }
         return true;
     }
     bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache) const override
     {
-        // For hardened derivation type, just return the typical string, nothing to normalize
         if (m_derive == DeriveType::HARDENED) {
-            out = ToString();
+            out = ToString(/*normalized=*/true);
+
             return true;
         }
         // Step backwards to find the last hardened step in the path
@@ -1049,15 +1059,27 @@ enum class ParseScriptContext {
     P2TR,    //!< Inside tr() (either internal key, or BIP342 script leaf)
 };
 
-/** Parse a key path, being passed a split list of elements (the first element is ignored). */
-[[nodiscard]] bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out, std::string& error)
+/**
+ * Parse a key path, being passed a split list of elements (the first element is ignored).
+ *
+ * @param[in] split BIP32 path string, using either ' or h for hardened derivation
+ * @param[out] out the key path
+ * @param[out] apostrophe only updated if hardened derivation is found
+ * @param[out] error parsing error message
+ * @returns false if parsing failed
+ **/
+[[nodiscard]] bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out, bool& apostrophe, std::string& error)
 {
     for (size_t i = 1; i < split.size(); ++i) {
         Span<const char> elem = split[i];
         bool hardened = false;
-        if (elem.size() > 0 && (elem[elem.size() - 1] == '\'' || elem[elem.size() - 1] == 'h')) {
-            elem = elem.first(elem.size() - 1);
-            hardened = true;
+        if (elem.size() > 0) {
+            const char last = elem[elem.size() - 1];
+            if (last == '\'' || last == 'h') {
+                elem = elem.first(elem.size() - 1);
+                hardened = true;
+                apostrophe = last == '\'';
+            }
         }
         uint32_t p;
         if (!ParseUInt32(std::string(elem.begin(), elem.end()), &p)) {
@@ -1073,7 +1095,7 @@ enum class ParseScriptContext {
 }
 
 /** Parse a public key that excludes origin information. */
-std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error)
+std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, bool& apostrophe, std::string& error)
 {
     using namespace spanparsing;
 
@@ -1130,15 +1152,16 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const S
         split.pop_back();
         type = DeriveType::UNHARDENED;
     } else if (split.back() == Span{"*'"}.first(2) || split.back() == Span{"*h"}.first(2)) {
+        apostrophe = split.back() == Span{"*'"}.first(2);
         split.pop_back();
         type = DeriveType::HARDENED;
     }
-    if (!ParseKeyPath(split, path, error)) return nullptr;
+    if (!ParseKeyPath(split, path, apostrophe, error)) return nullptr;
     if (extkey.key.IsValid()) {
         extpubkey = extkey.Neuter();
         out.keys.emplace(extpubkey.pubkey.GetID(), extkey.key);
     }
-    return std::make_unique<BIP32PubkeyProvider>(key_exp_index, extpubkey, std::move(path), type);
+    return std::make_unique<BIP32PubkeyProvider>(key_exp_index, extpubkey, std::move(path), type, apostrophe);
 }
 
 /** Parse a public key including origin information (if enabled). */
@@ -1151,7 +1174,11 @@ std::unique_ptr<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c
         error = "Multiple ']' characters found for a single pubkey";
         return nullptr;
     }
-    if (origin_split.size() == 1) return ParsePubkeyInner(key_exp_index, origin_split[0], ctx, out, error);
+    // This is set if either the origin or path suffix contains a hardened derivation.
+    bool apostrophe = false;
+    if (origin_split.size() == 1) {
+        return ParsePubkeyInner(key_exp_index, origin_split[0], ctx, out, apostrophe, error);
+    }
     if (origin_split[0].empty() || origin_split[0][0] != '[') {
         error = strprintf("Key origin start '[ character expected but not found, got '%c' instead",
                           origin_split[0].empty() ? /** empty, implies split char */ ']' : origin_split[0][0]);
@@ -1172,18 +1199,18 @@ std::unique_ptr<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c
     static_assert(sizeof(info.fingerprint) == 4, "Fingerprint must be 4 bytes");
     assert(fpr_bytes.size() == 4);
     std::copy(fpr_bytes.begin(), fpr_bytes.end(), info.fingerprint);
-    if (!ParseKeyPath(slash_split, info.path, error)) return nullptr;
-    auto provider = ParsePubkeyInner(key_exp_index, origin_split[1], ctx, out, error);
+    if (!ParseKeyPath(slash_split, info.path, apostrophe, error)) return nullptr;
+    auto provider = ParsePubkeyInner(key_exp_index, origin_split[1], ctx, out, apostrophe, error);
     if (!provider) return nullptr;
-    return std::make_unique<OriginPubkeyProvider>(key_exp_index, std::move(info), std::move(provider));
+    return std::make_unique<OriginPubkeyProvider>(key_exp_index, std::move(info), std::move(provider), apostrophe);
 }
 
 std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider)
 {
     std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, false);
     KeyOriginInfo info;
     if (provider.GetKeyOrigin(pubkey.GetID(), info)) {
-        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
+        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider), /*apostrophe=*/false);
     }
     return key_provider;
 }
@@ -1196,7 +1223,7 @@ std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseS
     std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, true);
     KeyOriginInfo info;
     if (provider.GetKeyOriginByXOnly(xkey, info)) {
-        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
+        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider), /*apostrophe=*/false);
     }
     return key_provider;
 }