Merge bitcoin/bitcoin#31242: wallet, desc spkm: Return SigningProvider only if we have the privkey

f6a6d91205 test: add check for getting SigningProvider for a CPubKey (Sebastian Falbesoner)
62a95f5af9 test: refactor: move `CreateDescriptor` helper to wallet test util module (Sebastian Falbesoner)
493656763f desc spkm: Return SigningProvider only if we have the privkey (Ava Chow)

Pull request description:

  If we know about a pubkey that's in our descriptor, but we don't have the private key, don't return a SigningProvider for that pubkey.

  This is specifically an issue for Taproot outputs that use the H point as the resulting PSBTs may end up containing irrelevant information because the H point was detected as a pubkey each unrelated descriptor knew about.

  Split from #29675

ACKs for top commit:
  fjahr:
    ACK f6a6d91205
  theStack:
    re-ACK f6a6d91205
  furszy:
    utACK f6a6d91205. Only reviewed the actual change in detail, not the test commit.

Tree-SHA512: 30a196e611a0c5d9ebe5baf6d896caaa6af66f1615463dbb0c31e52604d53cf342922bb9967b3c697b47083d76b0485c77a5f545bd6381247c8bc44321c70f97

src/wallet/test/ismine_tests.cpp

@@ -20,26 +20,6 @@ using namespace util::hex_literals;
 namespace wallet {
 BOOST_FIXTURE_TEST_SUITE(ismine_tests, BasicTestingSetup)
 
-wallet::ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success)
-{
-    keystore.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
-
-    FlatSigningProvider keys;
-    std::string error;
-    auto parsed_descs = Parse(desc_str, keys, error, false);
-    BOOST_CHECK(success == (!parsed_descs.empty()));
-    if (!success) return nullptr;
-    auto& desc = parsed_descs.at(0);
-
-    const int64_t range_start = 0, range_end = 1, next_index = 0, timestamp = 1;
-
-    WalletDescriptor w_desc(std::move(desc), timestamp, range_start, range_end, next_index);
-
-    LOCK(keystore.cs_wallet);
-
-    return Assert(keystore.AddWalletDescriptor(w_desc, keys,/*label=*/"", /*internal=*/false));
-};
-
 BOOST_AUTO_TEST_CASE(ismine_standard)
 {
     CKey keys[2];

src/wallet/test/scriptpubkeyman_tests.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <key.h>
+#include <key_io.h>
 #include <test/util/setup_common.h>
 #include <script/solver.h>
 #include <wallet/scriptpubkeyman.h>
@@ -40,5 +41,27 @@ BOOST_AUTO_TEST_CASE(CanProvide)
     BOOST_CHECK(keyman.CanProvide(p2sh_script, data));
 }
 
+BOOST_AUTO_TEST_CASE(DescriptorScriptPubKeyManTests)
+{
+    std::unique_ptr<interfaces::Chain>& chain = m_node.chain;
+
+    CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
+    auto key_scriptpath = GenerateRandomKey();
+
+    // Verify that a SigningProvider for a pubkey is only returned if its corresponding private key is available
+    auto key_internal = GenerateRandomKey();
+    std::string desc_str = "tr(" + EncodeSecret(key_internal) + ",pk(" + HexStr(key_scriptpath.GetPubKey()) + "))";
+    auto spk_man1 = dynamic_cast<DescriptorScriptPubKeyMan*>(CreateDescriptor(keystore, desc_str, true));
+    BOOST_CHECK(spk_man1 != nullptr);
+    auto signprov_keypath_spendable = spk_man1->GetSigningProvider(key_internal.GetPubKey());
+    BOOST_CHECK(signprov_keypath_spendable != nullptr);
+
+    desc_str = "tr(" + HexStr(XOnlyPubKey::NUMS_H) + ",pk(" + HexStr(key_scriptpath.GetPubKey()) + "))";
+    auto spk_man2 = dynamic_cast<DescriptorScriptPubKeyMan*>(CreateDescriptor(keystore, desc_str, true));
+    BOOST_CHECK(spk_man2 != nullptr);
+    auto signprov_keypath_nums_h = spk_man2->GetSigningProvider(XOnlyPubKey::NUMS_H.GetEvenCorrespondingCPubKey());
+    BOOST_CHECK(signprov_keypath_nums_h == nullptr);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet

src/wallet/test/util.cpp

@@ -192,4 +192,24 @@ MockableDatabase& GetMockableDatabase(CWallet& wallet)
 {
     return dynamic_cast<MockableDatabase&>(wallet.GetDatabase());
 }
+
+wallet::ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success)
+{
+    keystore.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+
+    FlatSigningProvider keys;
+    std::string error;
+    auto parsed_descs = Parse(desc_str, keys, error, false);
+    Assert(success == (!parsed_descs.empty()));
+    if (!success) return nullptr;
+    auto& desc = parsed_descs.at(0);
+
+    const int64_t range_start = 0, range_end = 1, next_index = 0, timestamp = 1;
+
+    WalletDescriptor w_desc(std::move(desc), timestamp, range_start, range_end, next_index);
+
+    LOCK(keystore.cs_wallet);
+
+    return Assert(keystore.AddWalletDescriptor(w_desc, keys,/*label=*/"", /*internal=*/false));
+};
 } // namespace wallet

src/wallet/test/util.h

@@ -9,6 +9,7 @@
 
 #include <addresstype.h>
 #include <wallet/db.h>
+#include <wallet/scriptpubkeyman.h>
 
 #include <memory>
 
@@ -127,8 +128,9 @@ public:
 };
 
 std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(MockableData records = {});
-
 MockableDatabase& GetMockableDatabase(CWallet& wallet);
+
+ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_TEST_UTIL_H