Merge bitcoin/bitcoin#23334: fuzz: Descriptor wallet

11115169a14d0d0be5b7b1c3f6fdc9673a9098d9 ci: Build fuzz with libsqlite3-dev (MarcoFalke)
fa7c6efca66627e4c76adecc824f96da220af69c fuzz: Add wallet fuzz test (MarcoFalke)
fa59d2ce5b8d6fe8c610f170a13675c756aef58f refactor: Use local args instead of global gArgs in CWallet::Create (MarcoFalke)
fadb44606f26a80daf4320eee046c9572e85fe3e build: Inline FUZZ_SUITE_LDFLAGS_COMMON (MarcoFalke)

Pull request description:

  Initial sketch to fuzz descriptor wallets. Can be improved in the future.

ACKs for top commit:
  mjdietzx:
    Code review ACK 1111516

Tree-SHA512: b1d2f24504d1ed5f3c6a031210f04c27c13d4e15576c4acbf50ded37ac45f7b7a5c7553e91d81d4a06e9ea73b3d745a552218d3ef3b2932fa5325a8331b0d3fd

6 files changed, 185 insertions, 10 deletions

diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh
index b8ac691346..9c34cfdf50 100755
--- a/ci/test/00_setup_env_native_fuzz.sh
+++ b/ci/test/00_setup_env_native_fuzz.sh
@@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8
 
 export DOCKER_NAME_TAG="ubuntu:20.04"
 export CONTAINER_NAME=ci_native_fuzz
-export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev"
+export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libsqlite3-dev"
 export NO_DEPENDS=1
 export RUN_UNIT_TESTS=false
 export RUN_FUNCTIONAL_TESTS=false
diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
index 2cf672b91e..673326ded7 100755
--- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
+++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
@@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8
 
 export DOCKER_NAME_TAG="ubuntu:20.04"
 export CONTAINER_NAME=ci_native_fuzz_valgrind
-export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev valgrind"
+export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libsqlite3-dev valgrind"
 export NO_DEPENDS=1
 export RUN_UNIT_TESTS=false
 export RUN_FUNCTIONAL_TESTS=false
diff --git a/configure.ac b/configure.ac
index dc3b07e4bb..7e8b124b02 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1244,7 +1244,6 @@ if test "x$enable_fuzz" = "xyes"; then
   bitcoin_enable_qt=no
   bitcoin_enable_qt_test=no
   bitcoin_enable_qt_dbus=no
-  enable_wallet=no
   use_bench=no
   use_external_signer=no
   use_upnp=no
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 27f9382631..5a5c74c044 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -170,6 +170,10 @@ if USE_BDB
 BITCOIN_TESTS += wallet/test/db_tests.cpp
 endif
 
+if USE_SQLITE
+FUZZ_WALLET_SRC = \
+ wallet/test/fuzz/notifications.cpp
+endif # USE_SQLITE
 
 BITCOIN_TEST_SUITE += \
   wallet/test/util.cpp \
@@ -178,7 +182,7 @@ BITCOIN_TEST_SUITE += \
   wallet/test/wallet_test_fixture.h \
   wallet/test/init_test_fixture.cpp \
   wallet/test/init_test_fixture.h
-endif
+endif # ENABLE_WALLET
 
 test_test_bitcoin_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES)
 test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(TESTDEFS) $(EVENT_CFLAGS)
@@ -199,14 +203,13 @@ test_test_bitcoin_LDADD += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS)
 FUZZ_SUITE_LD_COMMON += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS)
 endif
 
-FUZZ_SUITE_LDFLAGS_COMMON = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS)
-
 if ENABLE_FUZZ_BINARY
 test_fuzz_fuzz_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
 test_fuzz_fuzz_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 test_fuzz_fuzz_LDADD = $(FUZZ_SUITE_LD_COMMON)
-test_fuzz_fuzz_LDFLAGS = $(FUZZ_SUITE_LDFLAGS_COMMON) $(RUNTIME_LDFLAGS)
+test_fuzz_fuzz_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) $(RUNTIME_LDFLAGS)
 test_fuzz_fuzz_SOURCES = \
+ $(FUZZ_WALLET_SRC) \
  test/fuzz/addition_overflow.cpp \
  test/fuzz/addrman.cpp \
  test/fuzz/asmap.cpp \
diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp
new file mode 100644
index 0000000000..252832785b
--- /dev/null
+++ b/src/wallet/test/fuzz/notifications.cpp
@@ -0,0 +1,173 @@
+// Copyright (c) 2021 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/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
+#include <util/translation.h>
+#include <wallet/context.h>
+#include <wallet/receive.h>
+#include <wallet/wallet.h>
+#include <wallet/walletdb.h>
+#include <wallet/walletutil.h>
+
+#include <cassert>
+#include <cstdint>
+#include <string>
+#include <vector>
+
+namespace {
+const TestingSetup* g_setup;
+
+void initialize_setup()
+{
+    static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
+    g_setup = testing_setup.get();
+}
+
+/**
+ * Wraps a descriptor wallet for fuzzing. The constructor writes the sqlite db
+ * to disk, the destructor deletes it.
+ */
+struct FuzzedWallet {
+    ArgsManager args;
+    WalletContext context;
+    std::shared_ptr<CWallet> wallet;
+    FuzzedWallet(const std::string& name)
+    {
+        context.args = &args;
+        context.chain = g_setup->m_node.chain.get();
+
+        DatabaseOptions options;
+        options.require_create = true;
+        options.create_flags = WALLET_FLAG_DESCRIPTORS;
+        const std::optional<bool> load_on_start;
+        gArgs.ForceSetArg("-keypool", "0"); // Avoid timeout in TopUp()
+
+        DatabaseStatus status;
+        bilingual_str error;
+        std::vector<bilingual_str> warnings;
+        wallet = CreateWallet(context, name, load_on_start, options, status, error, warnings);
+        assert(wallet);
+        assert(error.empty());
+        assert(warnings.empty());
+        assert(wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
+    }
+    ~FuzzedWallet()
+    {
+        const auto name{wallet->GetName()};
+        std::vector<bilingual_str> warnings;
+        std::optional<bool> load_on_start;
+        assert(RemoveWallet(context, wallet, load_on_start, warnings));
+        assert(warnings.empty());
+        UnloadWallet(std::move(wallet));
+        fs::remove_all(GetWalletDir() / name);
+    }
+    CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider)
+    {
+        auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)};
+        if (type == OutputType::BECH32M) {
+            type = OutputType::BECH32; // TODO: Setup taproot descriptor and remove this line
+        }
+        CTxDestination dest;
+        bilingual_str error;
+        if (fuzzed_data_provider.ConsumeBool()) {
+            assert(wallet->GetNewDestination(type, "", dest, error));
+        } else {
+            assert(wallet->GetNewChangeDestination(type, dest, error));
+        }
+        assert(error.empty());
+        return GetScriptForDestination(dest);
+    }
+};
+
+FUZZ_TARGET_INIT(wallet_notifications, initialize_setup)
+{
+    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
+    // The total amount, to be distributed to the wallets a and b in txs
+    // without fee. Thus, the balance of the wallets should always equal the
+    // total amount.
+    const auto total_amount{ConsumeMoney(fuzzed_data_provider)};
+    FuzzedWallet a{"fuzzed_wallet_a"};
+    FuzzedWallet b{"fuzzed_wallet_b"};
+
+    // Keep track of all coins in this test.
+    // Each tuple in the chain represents the coins and the block created with
+    // those coins. Once the block is mined, the next tuple will have an empty
+    // block and the freshly mined coins.
+    using Coins = std::set<std::tuple<CAmount, COutPoint>>;
+    std::vector<std::tuple<Coins, CBlock>> chain;
+    {
+        // Add the inital entry
+        chain.emplace_back();
+        auto& [coins, block]{chain.back()};
+        coins.emplace(total_amount, COutPoint{uint256::ONE, 1});
+    }
+    LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 200)
+    {
+        CallOneOf(
+            fuzzed_data_provider,
+            [&] {
+                auto& [coins_orig, block]{chain.back()};
+                // Copy the coins for this block and consume all of them
+                Coins coins = coins_orig;
+                while (!coins.empty()) {
+                    // Create a new tx
+                    CMutableTransaction tx{};
+                    // Add some coins as inputs to it
+                    auto num_inputs{fuzzed_data_provider.ConsumeIntegralInRange<int>(1, coins.size())};
+                    CAmount in{0};
+                    while (num_inputs-- > 0) {
+                        const auto& [coin_amt, coin_outpoint]{*coins.begin()};
+                        in += coin_amt;
+                        tx.vin.emplace_back(coin_outpoint);
+                        coins.erase(coins.begin());
+                    }
+                    // Create some outputs spending all inputs, without fee
+                    LIMITED_WHILE(in > 0 && fuzzed_data_provider.ConsumeBool(), 100)
+                    {
+                        const auto out_value{ConsumeMoney(fuzzed_data_provider, in)};
+                        in -= out_value;
+                        auto& wallet{fuzzed_data_provider.ConsumeBool() ? a : b};
+                        tx.vout.emplace_back(out_value, wallet.GetScriptPubKey(fuzzed_data_provider));
+                    }
+                    // Spend the remaining input value, if any
+                    auto& wallet{fuzzed_data_provider.ConsumeBool() ? a : b};
+                    tx.vout.emplace_back(in, wallet.GetScriptPubKey(fuzzed_data_provider));
+                    // Add tx to block
+                    block.vtx.emplace_back(MakeTransactionRef(tx));
+                }
+                // Mine block
+                a.wallet->blockConnected(block, chain.size());
+                b.wallet->blockConnected(block, chain.size());
+                // Store the coins for the next block
+                Coins coins_new;
+                for (const auto& tx : block.vtx) {
+                    uint32_t i{0};
+                    for (const auto& out : tx->vout) {
+                        coins_new.emplace(out.nValue, COutPoint{tx->GetHash(), i++});
+                    }
+                }
+                chain.emplace_back(coins_new, CBlock{});
+            },
+            [&] {
+                if (chain.size() <= 1) return; // The first entry can't be removed
+                auto& [coins, block]{chain.back()};
+                if (block.vtx.empty()) return; // Can only disconnect if the block was submitted first
+                // Disconnect block
+                a.wallet->blockDisconnected(block, chain.size() - 1);
+                b.wallet->blockDisconnected(block, chain.size() - 1);
+                chain.pop_back();
+            });
+        auto& [coins, first_block]{chain.front()};
+        if (!first_block.vtx.empty()) {
+            // Only check balance when at least one block was submitted
+            const auto bal_a{GetBalance(*a.wallet).m_mine_trusted};
+            const auto bal_b{GetBalance(*b.wallet).m_mine_trusted};
+            assert(total_amount == bal_a + bal_b);
+        }
+    }
+}
+} // namespace
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 4eb9d5560d..606a30a999 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2732,11 +2732,11 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
         walletInstance->m_default_max_tx_fee = max_fee.value();
     }
 
-    if (gArgs.IsArgSet("-consolidatefeerate")) {
-        if (std::optional<CAmount> consolidate_feerate = ParseMoney(gArgs.GetArg("-consolidatefeerate", ""))) {
+    if (args.IsArgSet("-consolidatefeerate")) {
+        if (std::optional<CAmount> consolidate_feerate = ParseMoney(args.GetArg("-consolidatefeerate", ""))) {
             walletInstance->m_consolidate_feerate = CFeeRate(*consolidate_feerate);
         } else {
-            error = AmountErrMsg("consolidatefeerate", gArgs.GetArg("-consolidatefeerate", ""));
+            error = AmountErrMsg("consolidatefeerate", args.GetArg("-consolidatefeerate", ""));
             return nullptr;
         }
     }