Merge bitcoin/bitcoin#31734: miniscript: account for all StringType variants in Miniscriptdescriptor::ToString()

28a4fcb test: check listdescriptors do not return a mix of hardened derivation marker (pythcoiner)
975783c descriptor: account for all StringType in MiniscriptDescriptor::ToStringHelper() (pythcoiner)

Pull request description:

  In `MiniscriptDescriptor::ToStringHelper()` only the `StringType::Private` variant of the `type` argument was handled. This PR implements serializing w/ all variants of `StringType` & add a functional test for the descriptor triggering the related issue.

  Closes #31694: previously when calling `listdescriptors` RPC on a wallet containing a taproot descriptor w/ a (miniscript) taptree, origins of internal key & taptree were serialized w/ differents hardened derivation markers:
   - origin of the internal key were serialized w/ `StringType::Normalized` type (using `h` as marker)
   - origins of taptree keys were serialized w/ `StringType::Private` type (using `'` as marker)

  Note: Origins in segwit (`wsh()`) miniscript descriptors were also serialized w/ `StringType::Private` type (`'` marker) and are now serialized w/ `StringType::Normalized` type (`h` marker).

ACKs for top commit:
  sipa:
    Code review ACK 28a4fcb
  achow101:
    ACK 28a4fcb
  rkrux:
    Concept ACK 28a4fcb

Tree-SHA512: 15d14000b5951ca69a64a05b9a0b138c48a07b81eaf2fa86b91ac20cc8735533355a787363c64ba88403dd8a56ef5232cba57d34bea80835a0f40774d62fbc2b

Author: achow101

src/script/descriptor.cpp

@@ -1483,20 +1483,33 @@ class StringMaker {
     const SigningProvider* m_arg;
     //! Keys contained in the Miniscript (a reference to DescriptorImpl::m_pubkey_args).
     const std::vector<std::unique_ptr<PubkeyProvider>>& m_pubkeys;
-    //! Whether to serialize keys as private or public.
-    bool m_private;
+    //! StringType to serialize keys
+    const DescriptorImpl::StringType m_type;
+    const DescriptorCache* m_cache;
 
 public:
-    StringMaker(const SigningProvider* arg LIFETIMEBOUND, const std::vector<std::unique_ptr<PubkeyProvider>>& pubkeys LIFETIMEBOUND, bool priv)
-        : m_arg(arg), m_pubkeys(pubkeys), m_private(priv) {}
+    StringMaker(const SigningProvider* arg LIFETIMEBOUND,
+                const std::vector<std::unique_ptr<PubkeyProvider>>& pubkeys LIFETIMEBOUND,
+                DescriptorImpl::StringType type,
+                const DescriptorCache* cache LIFETIMEBOUND)
+        : m_arg(arg), m_pubkeys(pubkeys), m_type(type), m_cache(cache) {}
 
     std::optional<std::string> ToString(uint32_t key) const
     {
         std::string ret;
-        if (m_private) {
-            if (!m_pubkeys[key]->ToPrivateString(*m_arg, ret)) return {};
-        } else {
+        switch (m_type) {
+        case DescriptorImpl::StringType::PUBLIC:
             ret = m_pubkeys[key]->ToString();
+            break;
+        case DescriptorImpl::StringType::PRIVATE:
+            if (!m_pubkeys[key]->ToPrivateString(*m_arg, ret)) return {};
+            break;
+        case DescriptorImpl::StringType::NORMALIZED:
+            if (!m_pubkeys[key]->ToNormalizedString(*m_arg, ret, m_cache)) return {};
+            break;
+        case DescriptorImpl::StringType::COMPAT:
+            ret = m_pubkeys[key]->ToString(PubkeyProvider::StringType::COMPAT);
+            break;
         }
         return ret;
     }
@@ -1529,7 +1542,7 @@ class MiniscriptDescriptor final : public DescriptorImpl
     bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type,
                         const DescriptorCache* cache = nullptr) const override
     {
-        if (const auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type == StringType::PRIVATE))) {
+        if (const auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache))) {
             out = *res;
             return true;
         }

test/functional/wallet_listdescriptors.py

@@ -126,6 +126,26 @@ def run_test(self):
         }
         assert_equal(expected, wallet.listdescriptors())
 
+        self.log.info('Test taproot descriptor do not have mixed hardened derivation marker')
+        node.createwallet(wallet_name='w5', descriptors=True, disable_private_keys=True)
+        wallet = node.get_wallet_rpc('w5')
+        wallet.importdescriptors([{
+            'desc': "tr([1dce71b2/48'/1'/0'/2']tpubDEeP3GefjqbaDTTaVAF5JkXWhoFxFDXQ9KuhVrMBViFXXNR2B3Lvme2d2AoyiKfzRFZChq2AGMNbU1qTbkBMfNv7WGVXLt2pnYXY87gXqcs/0/*,and_v(v:pk([c658b283/48'/1'/0'/2']tpubDFL5wzgPBYK5pZ2Kh1T8qrxnp43kjE5CXfguZHHBrZSWpkfASy5rVfj7prh11XdqkC1P3kRwUPBeX7AHN8XBNx8UwiprnFnEm5jyswiRD4p/0/*),older(65535)))#xl20m6md",
+            'timestamp': TIME_GENESIS_BLOCK,
+        }])
+        expected = {
+            'wallet_name': 'w5',
+            'descriptors': [
+                {'active': False,
+                 'desc': 'tr([1dce71b2/48h/1h/0h/2h]tpubDEeP3GefjqbaDTTaVAF5JkXWhoFxFDXQ9KuhVrMBViFXXNR2B3Lvme2d2AoyiKfzRFZChq2AGMNbU1qTbkBMfNv7WGVXLt2pnYXY87gXqcs/0/*,and_v(v:pk([c658b283/48h/1h/0h/2h]tpubDFL5wzgPBYK5pZ2Kh1T8qrxnp43kjE5CXfguZHHBrZSWpkfASy5rVfj7prh11XdqkC1P3kRwUPBeX7AHN8XBNx8UwiprnFnEm5jyswiRD4p/0/*),older(65535)))#m4uznndk',
+                 'timestamp': TIME_GENESIS_BLOCK,
+                 'range': [0, 0],
+                 'next': 0,
+                 'next_index': 0},
+            ]
+        }
+        assert_equal(expected, wallet.listdescriptors())
+
 
 if __name__ == '__main__':
     ListDescriptorsTest(__file__).main()