Merge bitcoin/bitcoin#28578: fuzz: add target for DescriptorScriptPubKeyMan

47e5c9994c087d1826ccc0d539e916845b5648fb fuzz: add target for `DescriptorScriptPubKeyMan` (brunoerg)
641dddf01812407d163520def83f5975413691e4 fuzz: create ConsumeCoins (brunoerg)
2e1833ca1341ab4dc92508a59181aa6c7c38db88 fuzz: move `MockedDescriptorConverter` to `fuzz/util` (brunoerg)

Pull request description:

  This PR adds fuzz target for `DescriptorScriptPubKeyMan`. Also, moves `MockedDescriptorConverter` to `fuzz/util/descriptor` to be used here and in `descriptor` target.

ACKs for top commit:
  maflcko:
    lgtm ACK 47e5c9994c087d1826ccc0d539e916845b5648fb 🏓
  dergoegge:
    ACK 47e5c9994c087d1826ccc0d539e916845b5648fb

Tree-SHA512: 519acca6d7b7a3a0bfc031441b02d5980b12bfb97198bd1958a83cd815ceb9eb1499a48a3f0a7fe20e5d06d83b89335d987376fc0a014e2106b0bc0e9838dd02

src/Makefile.test.include

@@ -205,7 +205,8 @@ FUZZ_WALLET_SRC = \
 
 if USE_SQLITE
 FUZZ_WALLET_SRC += \
- wallet/test/fuzz/notifications.cpp
+ wallet/test/fuzz/notifications.cpp \
+ wallet/test/fuzz/scriptpubkeyman.cpp
 endif # USE_SQLITE
 
 BITCOIN_TEST_SUITE += \

src/Makefile.test_fuzz.include

@@ -11,6 +11,7 @@ TEST_FUZZ_H = \
     test/fuzz/fuzz.h \
     test/fuzz/FuzzedDataProvider.h \
     test/fuzz/util.h \
+    test/fuzz/util/descriptor.h \
     test/fuzz/util/mempool.h \
     test/fuzz/util/net.h
 
@@ -19,6 +20,7 @@ libtest_fuzz_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 libtest_fuzz_a_SOURCES = \
   test/fuzz/fuzz.cpp \
   test/fuzz/util.cpp \
+  test/fuzz/util/descriptor.cpp \
   test/fuzz/util/mempool.cpp \
   test/fuzz/util/net.cpp \
   $(TEST_FUZZ_H)

src/test/fuzz/descriptor_parse.cpp

@@ -7,104 +7,10 @@
 #include <pubkey.h>
 #include <script/descriptor.h>
 #include <test/fuzz/fuzz.h>
+#include <test/fuzz/util/descriptor.h>
 #include <util/chaintype.h>
 #include <util/strencodings.h>
 
-//! Types are raw (un)compressed pubkeys, raw xonly pubkeys, raw privkeys (WIF), xpubs, xprvs.
-static constexpr uint8_t KEY_TYPES_COUNT{6};
-//! How many keys we'll generate in total.
-static constexpr size_t TOTAL_KEYS_GENERATED{std::numeric_limits<uint8_t>::max() + 1};
-
-/**
- * Converts a mocked descriptor string to a valid one. Every key in a mocked descriptor key is
- * represented by 2 hex characters preceded by the '%' character. We parse the two hex characters
- * as an index in a list of pre-generated keys. This list contains keys of the various types
- * accepted in descriptor keys expressions.
- */
-class MockedDescriptorConverter {
-    //! 256 keys of various types.
-    std::array<std::string, TOTAL_KEYS_GENERATED> keys_str;
-
-public:
-    // We derive the type of key to generate from the 1-byte id parsed from hex.
-    bool IdIsCompPubKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 0; }
-    bool IdIsUnCompPubKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 1; }
-    bool IdIsXOnlyPubKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 2; }
-    bool IdIsConstPrivKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 3; }
-    bool IdIsXpub(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 4; }
-    bool IdIsXprv(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 5; }
-
-    //! When initializing the target, populate the list of keys.
-    void Init() {
-        // The data to use as a private key or a seed for an xprv.
-        std::array<std::byte, 32> key_data{std::byte{1}};
-        // Generate keys of all kinds and store them in the keys array.
-        for (size_t i{0}; i < TOTAL_KEYS_GENERATED; i++) {
-            key_data[31] = std::byte(i);
-
-            // If this is a "raw" key, generate a normal privkey. Otherwise generate
-            // an extended one.
-            if (IdIsCompPubKey(i) || IdIsUnCompPubKey(i) || IdIsXOnlyPubKey(i) || IdIsConstPrivKey(i)) {
-                CKey privkey;
-                privkey.Set(UCharCast(key_data.begin()), UCharCast(key_data.end()), !IdIsUnCompPubKey(i));
-                if (IdIsCompPubKey(i) || IdIsUnCompPubKey(i)) {
-                    CPubKey pubkey{privkey.GetPubKey()};
-                    keys_str[i] = HexStr(pubkey);
-                } else if (IdIsXOnlyPubKey(i)) {
-                    const XOnlyPubKey pubkey{privkey.GetPubKey()};
-                    keys_str[i] = HexStr(pubkey);
-                } else {
-                    keys_str[i] = EncodeSecret(privkey);
-                }
-            } else {
-                CExtKey ext_privkey;
-                ext_privkey.SetSeed(key_data);
-                if (IdIsXprv(i)) {
-                    keys_str[i] = EncodeExtKey(ext_privkey);
-                } else {
-                    const CExtPubKey ext_pubkey{ext_privkey.Neuter()};
-                    keys_str[i] = EncodeExtPubKey(ext_pubkey);
-                }
-            }
-        }
-    }
-
-    //! Parse an id in the keys vectors from a 2-characters hex string.
-    std::optional<uint8_t> IdxFromHex(std::string_view hex_characters) const {
-        if (hex_characters.size() != 2) return {};
-        auto idx = ParseHex(hex_characters);
-        if (idx.size() != 1) return {};
-        return idx[0];
-    }
-
-    //! Get an actual descriptor string from a descriptor string whose keys were mocked.
-    std::optional<std::string> GetDescriptor(std::string_view mocked_desc) const {
-        // The smallest fragment would be "pk(%00)"
-        if (mocked_desc.size() < 7) return {};
-
-        // The actual descriptor string to be returned.
-        std::string desc;
-        desc.reserve(mocked_desc.size());
-
-        // Replace all occurrences of '%' followed by two hex characters with the corresponding key.
-        for (size_t i = 0; i < mocked_desc.size();) {
-            if (mocked_desc[i] == '%') {
-                if (i + 3 >= mocked_desc.size()) return {};
-                if (const auto idx = IdxFromHex(mocked_desc.substr(i + 1, 2))) {
-                    desc += keys_str[*idx];
-                    i += 3;
-                } else {
-                    return {};
-                }
-            } else {
-                desc += mocked_desc[i++];
-            }
-        }
-
-        return desc;
-    }
-};
-
 //! The converter of mocked descriptors, needs to be initialized when the target is.
 MockedDescriptorConverter MOCKED_DESC_CONVERTER;
 

src/test/fuzz/script_sign.cpp

@@ -125,18 +125,7 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign)
                 }
                 (void)signature_creator.CreateSig(provider, vch_sig, address, ConsumeScript(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}));
             }
-            std::map<COutPoint, Coin> coins;
-            LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
-                const std::optional<COutPoint> outpoint = ConsumeDeserializable<COutPoint>(fuzzed_data_provider);
-                if (!outpoint) {
-                    break;
-                }
-                const std::optional<Coin> coin = ConsumeDeserializable<Coin>(fuzzed_data_provider);
-                if (!coin) {
-                    break;
-                }
-                coins[*outpoint] = *coin;
-            }
+            std::map<COutPoint, Coin> coins{ConsumeCoins(fuzzed_data_provider)};
             std::map<int, bilingual_str> input_errors;
             (void)SignTransaction(sign_transaction_tx_to, &provider, coins, fuzzed_data_provider.ConsumeIntegral<int>(), input_errors);
         }

src/test/fuzz/util.cpp

@@ -164,6 +164,24 @@ uint32_t ConsumeSequence(FuzzedDataProvider& fuzzed_data_provider) noexcept
                fuzzed_data_provider.ConsumeIntegral<uint32_t>();
 }
 
+std::map<COutPoint, Coin> ConsumeCoins(FuzzedDataProvider& fuzzed_data_provider) noexcept
+{
+    std::map<COutPoint, Coin> coins;
+    LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
+        const std::optional<COutPoint> outpoint{ConsumeDeserializable<COutPoint>(fuzzed_data_provider)};
+        if (!outpoint) {
+            break;
+        }
+        const std::optional<Coin> coin{ConsumeDeserializable<Coin>(fuzzed_data_provider)};
+        if (!coin) {
+            break;
+        }
+        coins[*outpoint] = *coin;
+    }
+
+    return coins;
+}
+
 CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept
 {
     CTxDestination tx_destination;

src/test/fuzz/util.h

@@ -181,6 +181,8 @@ template <typename WeakEnumType, size_t size>
     return UintToArith256(ConsumeUInt256(fuzzed_data_provider));
 }
 
+[[nodiscard]] std::map<COutPoint, Coin> ConsumeCoins(FuzzedDataProvider& fuzzed_data_provider) noexcept;
+
 [[nodiscard]] CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept;
 
 [[nodiscard]] CKey ConsumePrivateKey(FuzzedDataProvider& fuzzed_data_provider, std::optional<bool> compressed = std::nullopt) noexcept;

src/test/fuzz/util/descriptor.cpp

@@ -0,0 +1,72 @@
+// Copyright (c) 2023-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <test/fuzz/util/descriptor.h>
+
+void MockedDescriptorConverter::Init() {
+    // The data to use as a private key or a seed for an xprv.
+    std::array<std::byte, 32> key_data{std::byte{1}};
+    // Generate keys of all kinds and store them in the keys array.
+    for (size_t i{0}; i < TOTAL_KEYS_GENERATED; i++) {
+        key_data[31] = std::byte(i);
+
+        // If this is a "raw" key, generate a normal privkey. Otherwise generate
+        // an extended one.
+        if (IdIsCompPubKey(i) || IdIsUnCompPubKey(i) || IdIsXOnlyPubKey(i) || IdIsConstPrivKey(i)) {
+            CKey privkey;
+            privkey.Set(UCharCast(key_data.begin()), UCharCast(key_data.end()), !IdIsUnCompPubKey(i));
+            if (IdIsCompPubKey(i) || IdIsUnCompPubKey(i)) {
+                CPubKey pubkey{privkey.GetPubKey()};
+                keys_str[i] = HexStr(pubkey);
+            } else if (IdIsXOnlyPubKey(i)) {
+                const XOnlyPubKey pubkey{privkey.GetPubKey()};
+                keys_str[i] = HexStr(pubkey);
+            } else {
+                keys_str[i] = EncodeSecret(privkey);
+            }
+        } else {
+            CExtKey ext_privkey;
+            ext_privkey.SetSeed(key_data);
+            if (IdIsXprv(i)) {
+                keys_str[i] = EncodeExtKey(ext_privkey);
+            } else {
+                const CExtPubKey ext_pubkey{ext_privkey.Neuter()};
+                keys_str[i] = EncodeExtPubKey(ext_pubkey);
+            }
+        }
+    }
+}
+
+std::optional<uint8_t> MockedDescriptorConverter::IdxFromHex(std::string_view hex_characters) const {
+    if (hex_characters.size() != 2) return {};
+    auto idx = ParseHex(hex_characters);
+    if (idx.size() != 1) return {};
+    return idx[0];
+}
+
+std::optional<std::string> MockedDescriptorConverter::GetDescriptor(std::string_view mocked_desc) const {
+    // The smallest fragment would be "pk(%00)"
+    if (mocked_desc.size() < 7) return {};
+
+    // The actual descriptor string to be returned.
+    std::string desc;
+    desc.reserve(mocked_desc.size());
+
+    // Replace all occurrences of '%' followed by two hex characters with the corresponding key.
+    for (size_t i = 0; i < mocked_desc.size();) {
+        if (mocked_desc[i] == '%') {
+            if (i + 3 >= mocked_desc.size()) return {};
+            if (const auto idx = IdxFromHex(mocked_desc.substr(i + 1, 2))) {
+                desc += keys_str[*idx];
+                i += 3;
+            } else {
+                return {};
+            }
+        } else {
+            desc += mocked_desc[i++];
+        }
+    }
+
+    return desc;
+}

src/test/fuzz/util/descriptor.h

@@ -0,0 +1,48 @@
+// Copyright (c) 2023-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H
+#define BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H
+
+#include <key_io.h>
+#include <util/strencodings.h>
+#include <script/descriptor.h>
+
+#include <functional>
+
+/**
+ * Converts a mocked descriptor string to a valid one. Every key in a mocked descriptor key is
+ * represented by 2 hex characters preceded by the '%' character. We parse the two hex characters
+ * as an index in a list of pre-generated keys. This list contains keys of the various types
+ * accepted in descriptor keys expressions.
+ */
+class MockedDescriptorConverter {
+private:
+    //! Types are raw (un)compressed pubkeys, raw xonly pubkeys, raw privkeys (WIF), xpubs, xprvs.
+    static constexpr uint8_t KEY_TYPES_COUNT{6};
+    //! How many keys we'll generate in total.
+    static constexpr size_t TOTAL_KEYS_GENERATED{std::numeric_limits<uint8_t>::max() + 1};
+    //! 256 keys of various types.
+    std::array<std::string, TOTAL_KEYS_GENERATED> keys_str;
+
+public:
+    // We derive the type of key to generate from the 1-byte id parsed from hex.
+    bool IdIsCompPubKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 0; }
+    bool IdIsUnCompPubKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 1; }
+    bool IdIsXOnlyPubKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 2; }
+    bool IdIsConstPrivKey(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 3; }
+    bool IdIsXpub(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 4; }
+    bool IdIsXprv(uint8_t idx) const { return idx % KEY_TYPES_COUNT == 5; }
+
+    //! When initializing the target, populate the list of keys.
+    void Init();
+
+    //! Parse an id in the keys vectors from a 2-characters hex string.
+    std::optional<uint8_t> IdxFromHex(std::string_view hex_characters) const;
+
+    //! Get an actual descriptor string from a descriptor string whose keys were mocked.
+    std::optional<std::string> GetDescriptor(std::string_view mocked_desc) const;
+};
+
+#endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

src/wallet/test/fuzz/scriptpubkeyman.cpp

@@ -0,0 +1,165 @@
+// Copyright (c) 2023-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <chainparams.h>
+#include <validation.h>
+#include <test/fuzz/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
+#include <test/fuzz/util/descriptor.h>
+#include <test/util/setup_common.h>
+#include <wallet/scriptpubkeyman.h>
+#include <wallet/wallet.h>
+#include <wallet/test/util.h>
+
+namespace wallet {
+namespace {
+const TestingSetup* g_setup;
+
+//! The converter of mocked descriptors, needs to be initialized when the target is.
+MockedDescriptorConverter MOCKED_DESC_CONVERTER;
+
+void initialize_spkm()
+{
+    static const auto testing_setup{MakeNoLogFileContext<const TestingSetup>()};
+    g_setup = testing_setup.get();
+    SelectParams(ChainType::MAIN);
+    MOCKED_DESC_CONVERTER.Init();
+}
+
+static std::optional<std::pair<WalletDescriptor, FlatSigningProvider>> CreateWalletDescriptor(FuzzedDataProvider& fuzzed_data_provider)
+{
+    const std::string mocked_descriptor{fuzzed_data_provider.ConsumeRandomLengthString()};
+    const auto desc_str{MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)};
+    if (!desc_str.has_value()) return std::nullopt;
+
+    FlatSigningProvider keys;
+    std::string error;
+    std::unique_ptr<Descriptor> parsed_desc{Parse(desc_str.value(), keys, error, false)};
+    if (!parsed_desc) return std::nullopt;
+
+    WalletDescriptor w_desc{std::move(parsed_desc), /*creation_time=*/0, /*range_start=*/0, /*range_end=*/1, /*next_index=*/1};
+    return std::make_pair(w_desc, keys);
+}
+
+static DescriptorScriptPubKeyMan* CreateDescriptor(WalletDescriptor& wallet_desc, FlatSigningProvider& keys, CWallet& keystore)
+{
+    LOCK(keystore.cs_wallet);
+    keystore.AddWalletDescriptor(wallet_desc, keys, /*label=*/"", /*internal=*/false);
+    return keystore.GetDescriptorScriptPubKeyMan(wallet_desc);
+};
+
+FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)
+{
+    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
+    const auto& node{g_setup->m_node};
+    Chainstate& chainstate{node.chainman->ActiveChainstate()};
+    std::unique_ptr<CWallet> wallet_ptr{std::make_unique<CWallet>(node.chain.get(), "", CreateMockableWalletDatabase())};
+    CWallet& wallet{*wallet_ptr};
+    {
+        LOCK(wallet.cs_wallet);
+        wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+        wallet.SetLastBlockProcessed(chainstate.m_chain.Height(), chainstate.m_chain.Tip()->GetBlockHash());
+    }
+
+    auto wallet_desc{CreateWalletDescriptor(fuzzed_data_provider)};
+    if (!wallet_desc.has_value()) return;
+    auto spk_manager{CreateDescriptor(wallet_desc->first, wallet_desc->second, wallet)};
+    if (spk_manager == nullptr) return;
+
+    bool good_data{true};
+    LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 300) {
+        CallOneOf(
+            fuzzed_data_provider,
+            [&] {
+                auto wallet_desc{CreateWalletDescriptor(fuzzed_data_provider)};
+                if (!wallet_desc.has_value()) {
+                    good_data = false;
+                    return;
+                }
+                std::string error;
+                if (spk_manager->CanUpdateToWalletDescriptor(wallet_desc->first, error)) {
+                    auto new_spk_manager{CreateDescriptor(wallet_desc->first, wallet_desc->second, wallet)};
+                    if (new_spk_manager != nullptr) spk_manager = new_spk_manager;
+                }
+            },
+            [&] {
+                const CScript script{ConsumeScript(fuzzed_data_provider)};
+                auto is_mine{spk_manager->IsMine(script)};
+                if (is_mine == isminetype::ISMINE_SPENDABLE) {
+                    assert(spk_manager->GetScriptPubKeys().count(script));
+                }
+            },
+            [&] {
+                auto spks{spk_manager->GetScriptPubKeys()};
+                for (const CScript& spk : spks) {
+                    assert(spk_manager->IsMine(spk) == ISMINE_SPENDABLE);
+                    CTxDestination dest;
+                    bool extract_dest{ExtractDestination(spk, dest)};
+                    if (extract_dest) {
+                        const std::string msg{fuzzed_data_provider.ConsumeRandomLengthString()};
+                        PKHash pk_hash{fuzzed_data_provider.ConsumeBool() ? PKHash{ConsumeUInt160(fuzzed_data_provider)} : *std::get_if<PKHash>(&dest)};
+                        std::string str_sig;
+                        (void)spk_manager->SignMessage(msg, pk_hash, str_sig);
+                    }
+                }
+            },
+            [&] {
+                CKey key{ConsumePrivateKey(fuzzed_data_provider, /*compressed=*/fuzzed_data_provider.ConsumeBool())};
+                if (!key.IsValid()) {
+                    good_data = false;
+                    return;
+                }
+                spk_manager->AddDescriptorKey(key, key.GetPubKey());
+                spk_manager->TopUp();
+            },
+            [&] {
+                std::string descriptor;
+                (void)spk_manager->GetDescriptorString(descriptor, /*priv=*/fuzzed_data_provider.ConsumeBool());
+            },
+            [&] {
+                LOCK(spk_manager->cs_desc_man);
+                auto wallet_desc{spk_manager->GetWalletDescriptor()};
+                if (wallet_desc.descriptor->IsSingleType()) {
+                    auto output_type{wallet_desc.descriptor->GetOutputType()};
+                    if (output_type.has_value()) {
+                        auto dest{spk_manager->GetNewDestination(*output_type)};
+                        if (dest) {
+                            assert(IsValidDestination(*dest));
+                            assert(spk_manager->IsHDEnabled());
+                        }
+                    }
+                }
+            },
+            [&] {
+                CMutableTransaction tx_to;
+                const std::optional<CMutableTransaction> opt_tx_to{ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS)};
+                if (!opt_tx_to) {
+                    good_data = false;
+                    return;
+                }
+                tx_to = *opt_tx_to;
+
+                std::map<COutPoint, Coin> coins{ConsumeCoins(fuzzed_data_provider)};
+                const int sighash{fuzzed_data_provider.ConsumeIntegral<int>()};
+                std::map<int, bilingual_str> input_errors;
+                (void)spk_manager->SignTransaction(tx_to, coins, sighash, input_errors);
+            },
+            [&] {
+                std::optional<PartiallySignedTransaction> opt_psbt{ConsumeDeserializable<PartiallySignedTransaction>(fuzzed_data_provider)};
+                if (!opt_psbt) {
+                    good_data = false;
+                    return;
+                }
+                auto psbt{*opt_psbt};
+                const PrecomputedTransactionData txdata{PrecomputePSBTData(psbt)};
+                const int sighash_type{fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 150)};
+                (void)spk_manager->FillPSBT(psbt, txdata, sighash_type, fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool(), nullptr, fuzzed_data_provider.ConsumeBool());
+            }
+        );
+    }
+}
+
+} // namespace
+} // namespace wallet