Repository: bitcoin/bitcoin (language: cpp)

Issue:
P2TR Spending Bug - Signing Transaction Failed
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Some Segwit Version 1 Addresses generated with [Achow101's Rust-Vanitygen](https://github.com/achow101/rust-vanitygen), or more generally some random Addresses imported using the `tr(<WIF Private Key>)` Descriptor, cause an issue when spending coins using them. Generally, the "Signing transaction failed" error occurs and the transaction is not created. Though, a workaround still allows to spend the coins.

I don't know if the issue is exclusive to P2TR, but never encountered such problems with Vanity Version 0 Addresses in the past... I also did not encounter such issue with P2TR Addresses directly generated in Bitcoin Core with `getnewaddress` or so.

Maybe it is the same issue as #5828, which was however not been reported with a concrete and reproducible example, and was closed since long... If so, then it would mean that other Address Types may be affected as TapRoot did not exist at this time. I have not found other duplicates, but sorry if I missed one...

### Expected behaviour

Being able to spend using these Addresses without workarounds.

### Steps to reproduce

* Optional: Generate a Vanity P2TR Address with the [Achow101's Rust-Vanitygen](https://github.com/achow101/rust-vanitygen), modding it or converting the Private Key from Mainnet to Testnet if necessary. Or simply some [random WIF Private Key](https://learnmeabitcoin.com/technical/keys/private-key/wif/). Some Addresses will work, others exhibit the present issue... It seems that there is a significant proportion falling in the second case...
* For convenience, you can simply use this one made for this Bug Report:
```
internal_privkey: cNKAo2ZRsaWKcP481cEfj3astPyBrfq56JBtLeRhHUvTSuk2z4MR
internal_pubkey: ac414af4b822d67307fe81bef0a72ddf0be9e6ff129eb1270b1168126f619619
Address: tb1ptestamfzz5zguhrad6xuf7afua0csz5m6k2qclpnpkvsl20t856sxz2d89
```
* Import the Address, for example in Testnet4 via the Bitcoin-Qt Console: 
```
importdescriptors '[{"desc": "tr(cNKAo2ZRsaWKcP481cEfj3astPyBrfq56JBtLeRhHUvTSuk2z4MR)#vhsk8sfh", "timestamp":0}]'
```
* Optional: get some coins from a Testnet4 Faucet (or any other source) for this tb1ptestamfzz5zguhrad6xuf7afua0csz5m6k2qclpnpkvsl20t856sxz2d89 Address. When I submitted the Issue, there were some coins left...
* Optional: Send some amount involving this Address. Immediately after importing the Address, it appears that there is no particular issue. See Transaction [a301d660523de70542d30490052f9e04ba3f9b5e26482745c7dbdc952802f103](https://mempool.space/testnet4/tx/a301d660523de70542d30490052f9e04ba3f9b5e26482745c7dbdc952802f103).
* Restart Bitcoin Core, and try sending again some amount. The interface now shows a "Signing transaction failed" dialog followed by "Transaction creation failed!". Or, in the Console,
```
sendtoaddress tb1qn9rvr53m7qvrpysx48svuxsgahs88xfsskx367 0.0001
Signing transaction failed (code -6)
```
* However, if a new Wallet is created, and the Descriptor is imported again as above, sending coins is possible again (until Bitcoin Core is restarted again). The process can be repeated to further reuse the Address (which is obviously not practical).

### Relevant log output

_No response_

### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitcoin Core are you using?

master@228aba2c4d9ac0b2ca3edd3c2cdf0a92e55f669b

### Operating system and version

Debian 13 Testing

### Machine specifications

_No response_


Discussion / hints:

Fix patch:
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 2e1a30744ec11..499af47ee553f 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -312,15 +312,7 @@ class ConstPubkeyProvider final : public PubkeyProvider
     bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override
     {
         CKey key;
-        if (m_xonly) {
-            for (const auto& keyid : XOnlyPubKey(m_pubkey).GetKeyIDs()) {
-                arg.GetKey(keyid, key);
-                if (key.IsValid()) break;
-            }
-        } else {
-            arg.GetKey(m_pubkey.GetID(), key);
-        }
-        if (!key.IsValid()) return false;
+        if (!GetPrivKey(/*pos=*/0, arg, key)) return false;
         ret = EncodeSecret(key);
         return true;
     }
@@ -331,7 +323,8 @@ class ConstPubkeyProvider final : public PubkeyProvider
     }
     bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override
     {
-        return arg.GetKey(m_pubkey.GetID(), key);
+        return m_xonly ? arg.GetKeyByXOnly(XOnlyPubKey(m_pubkey), key) :
+                         arg.GetKey(m_pubkey.GetID(), key);
     }
     std::optional<CPubKey> GetRootPubKey() const override
     {
@@ -1716,7 +1709,7 @@ struct KeyParser {
         if (miniscript::IsTapscript(m_script_ctx) && end - begin == 32) {
             XOnlyPubKey pubkey;
             std::copy(begin, end, pubkey.begin());
-            if (auto pubkey_provider = InferPubkey(pubkey.GetEvenCorrespondingCPubKey(), ParseContext(), *m_in)) {
+            if (auto pubkey_provider = InferXOnlyPubkey(pubkey, ParseContext(), *m_in)) {
                 m_keys.emplace_back();
                 m_keys.back().push_back(std::move(pubkey_provider));
                 return key;
diff --git a/src/script/signingprovider.h b/src/script/signingprovider.h
index efdfd9ee566b0..5b1da681f837f 100644
--- a/src/script/signingprovider.h
+++ b/src/script/signingprovider.h
@@ -136,6 +136,8 @@ class TaprootBuilder
     std::vector<std::tuple<uint8_t, uint8_t, std::vector<unsigned char>>> GetTreeTuples() const;
     /** Returns true if there are any tapscripts */
     bool HasScripts() const { return !m_branch.empty(); }
+
+    bool operator==(const TaprootBuilder& other) const { return GetTreeTuples() == other.GetTreeTuples(); }
 };
 
 /** Given a TaprootSpendData and the output key, reconstruct its script tree.


Test patch:
diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp
index 288ffc66eb6b2..1b4020e0a061c 100644
--- a/src/test/descriptor_tests.cpp
+++ b/src/test/descriptor_tests.cpp
@@ -62,6 +62,15 @@ bool EqualDescriptor(std::string a, std::string b)
     return a == b;
 }
 
+bool EqualSigningProviders(const FlatSigningProvider& a, const FlatSigningProvider& b)
+{
+    return a.scripts == b.scripts
+        && a.pubkeys == b.pubkeys
+        && a.origins == b.origins
+        && a.keys == b.keys
+        && a.tr_trees == b.tr_trees;
+}
+
 std::string UseHInsteadOfApostrophe(const std::string& desc)
 {
     std::string ret = desc;
@@ -214,6 +223,15 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
             BOOST_CHECK_MESSAGE(EqualDescriptor(prv, prv1), "Private ser: " + prv1 + " Private desc: " + prv);
         }
         BOOST_CHECK(!parse_pub->ToPrivateString(keys_pub, prv1));
+
+        // Check that both can ExpandPrivate and get the same SigningProviders
+        FlatSigningProvider priv_prov;
+        parse_priv->ExpandPrivate(0, keys_priv, priv_prov);
+
+        FlatSigningProvider pub_prov;
+        parse_pub->ExpandPrivate(0, keys_priv, pub_prov);
+
+        BOOST_CHECK_MESSAGE(EqualSigningProviders(priv_prov, pub_prov), "Private desc: " + prv + " Pub desc: " + pub);
     }
 
     // Check that private can produce the normalized descriptors
@@ -808,6 +826,31 @@ BOOST_AUTO_TEST_CASE(descriptor_test)
                 {{0x80000000UL + 48, 0x80000000UL + 1, 0x80000000UL, 0x80000000UL + 2, 1, 0}, {0x80000000UL + 48, 0x80000000UL + 1, 0x80000000UL, 0x80000000UL + 2, 1, 1}, {0x80000000UL + 48, 0x80000000UL + 1, 0x80000000UL, 0x80000000UL + 2, 1, 2}},
             }
     );
+    CheckMultipath("tr(xprv9yYge4PS54XkYT9KiLfCRwc8Jeuz8DucxQGtuEecJZYhKNiqbPxYHTPzXtskmzWBqdqkRAGsghNmZzNsfU2wstaB3XjDQFPv567aQSSuPyo,l:pk(xprvA1ADjaN8H3HGnZSmt4VF7YdWoV9oNq8jhqhurxsrYycBAFK555cECoaY22KWt6BTRNLuvobW5VQTF89PN3iA485LAg7epazevPyjCa4xTzd/<2;3>))",
+        "tr(xpub6CY33ZvKuS63kwDnpNCCo5YrrgkUXgdUKdCVhd4Dru5gCB3z8wGnqFiUP98Za5pYSYF5KmvBHTY3Ra8FAJGggzBjuHS69WzN8gscPupuZwK,l:pk(xpub6E9a95u27Qqa13XEz62FUgaFMWzHnHrb54dWfMHU7K9A33eDccvUkbu1sHYoByHAgJdR326rWqn9pGZgZHz1afDprW5gGwS4gUX8Ri6aGPZ/<2;3>))",
+        {
+            "tr(xprv9yYge4PS54XkYT9KiLfCRwc8Jeuz8DucxQGtuEecJZYhKNiqbPxYHTPzXtskmzWBqdqkRAGsghNmZzNsfU2wstaB3XjDQFPv567aQSSuPyo,l:pk(xprvA1ADjaN8H3HGnZSmt4VF7YdWoV9oNq8jhqhurxsrYycBAFK555cECoaY22KWt6BTRNLuvobW5VQTF89PN3iA485LAg7epazevPyjCa4xTzd/2))",
+            "tr(xprv9yYge4PS54XkYT9KiLfCRwc8Jeuz8DucxQGtuEecJZYhKNiqbPxYHTPzXtskmzWBqdqkRAGsghNmZzNsfU2wstaB3XjDQFPv567aQSSuPyo,l:pk(xprvA1ADjaN8H3HGnZSmt4VF7YdWoV9oNq8jhqhurxsrYycBAFK555cECoaY22KWt6BTRNLuvobW5VQTF89PN3iA485LAg7epazevPyjCa4xTzd/3))",
+        },
+        {
+            "tr(xpub6CY33ZvKuS63kwDnpNCCo5YrrgkUXgdUKdCVhd4Dru5gCB3z8wGnqFiUP98Za5pYSYF5KmvBHTY3Ra8FAJGggzBjuHS69WzN8gscPupuZwK,l:pk(xpub6E9a95u27Qqa13XEz62FUgaFMWzHnHrb54dWfMHU7K9A33eDccvUkbu1sHYoByHAgJdR326rWqn9pGZgZHz1afDprW5gGwS4gUX8Ri6aGPZ/2))",
+            "tr(xpub6CY33ZvKuS63kwDnpNCCo5YrrgkUXgdUKdCVhd4Dru5gCB3z8wGnqFiUP98Za5pYSYF5KmvBHTY3Ra8FAJGggzBjuHS69WzN8gscPupuZwK,l:pk(xpub6E9a95u27Qqa13XEz62FUgaFMWzHnHrb54dWfMHU7K9A33eDccvUkbu1sHYoByHAgJdR326rWqn9pGZgZHz1afDprW5gGwS4gUX8Ri6aGPZ/3))",
+        },
+        {
+            "tr(xpub6CY33ZvKuS63kwDnpNCCo5YrrgkUXgdUKdCVhd4Dru5gCB3z8wGnqFiUP98Za5pYSYF5KmvBHTY3Ra8FAJGggzBjuHS69WzN8gscPupuZwK,l:pk(xpub6E9a95u27Qqa13XEz62FUgaFMWzHnHrb54dWfMHU7K9A33eDccvUkbu1sHYoByHAgJdR326rWqn9pGZgZHz1afDprW5gGwS4gUX8Ri6aGPZ/2))",
+            "tr(xpub6CY33ZvKuS63kwDnpNCCo5YrrgkUXgdUKdCVhd4Dru5gCB3z8wGnqFiUP98Za5pYSYF5KmvBHTY3Ra8FAJGggzBjuHS69WzN8gscPupuZwK,l:pk(xpub6E9a95u27Qqa13XEz62FUgaFMWzHnHrb54dWfMHU7K9A33eDccvUkbu1sHYoByHAgJdR326rWqn9pGZgZHz1afDprW5gGwS4gUX8Ri6aGPZ/3))",
+        },
+        XONLY_KEYS,
+        {
+            {{"512094cb097990da64eebbad7b979b1326f3cbe356357abf4deb4c4ff80c7acbe902"}},
+            {{"5120f091450b88c606f5cbc3f0cebe89e00bc5dd27f92e22f54da06439bc0c401f41"}},
+        },
+        OutputType::BECH32M,
+        {
+            {{2}, {}},
+            {{3}, {}},
+        }
+    );
     CheckUnparsable("pkh(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/<0;1>/<2;3>)", "pkh(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/<0;1>/<2;3>)", "pkh(): Multiple multipath key path specifiers found");
     CheckUnparsable("pkh([deadbeef/<0;1>]xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/0)", "pkh([deadbeef/<0;1>]xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/0)", "pkh(): Key path value \'<0;1>\' specifies multipath in a section where multipath is not allowed");
     CheckUnparsable("tr(xprv9s21ZrQH143K2Zu2kTVKcQi9nKhfgJUkYqG73wXsHuhATm1wkt6kcSZeTYEw2PL7krZtJopEYDvBdYWdAai3n3TWUTCVfHvPHqTYJv7smYe/6/*,{pk(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/<1;2;3>/0/*),pk(xprv9s21ZrQH143K3jUwNHoqQNrtzJnJmx4Yup8NkNLdVQCymYbPbJXnPhwkfTfxZfptcs3rLAPUXS39oDLgrNKQGwbGsEmJJ8BU3RzQuvShEG4/0/0/<3;4>/*)})", "tr(xpub6B4sSbNr8XFYXqqKB7PeUemqgEaVtCLjgd5Lf2VYtezSHozC7ffCvVNCyu9TCgHntRQdimjV3tHbxmNfocxtuh6saNtZEw91gjXLRhQ3Yar/6/*,{pk(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/<1;2;3>/0/*),pk(xpub6AhFhZJJGt9YB8i85RfrJ8jT3T2FF5EejDCXqXfm1DAczFEXkk8HD3CXTg2TmKM8wTbSnSw3wPg5JuyLitUrpRmkjn2BQXyZnqJx16AGy94/0/0/<3;4>/*)})", "tr(): Multipath subscripts have mismatched lengths");