Merge bitcoin/bitcoin#28764: Fuzz: Check individual and package transaction invariants

src/test/fuzz/tx_pool.cpp

@@ -122,6 +122,67 @@ void MockTime(FuzzedDataProvider& fuzzed_data_provider, const CChainState& chain
     SetMockTime(time);
 }
 
+CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeContext& node)
+{
+    // Take the default options for tests...
+    CTxMemPool::Options mempool_opts{MemPoolOptionsForTest(node)};
+
+    // ...override specific options for this specific fuzz suite
+    mempool_opts.estimator = nullptr;
+    mempool_opts.check_ratio = 1;
+    mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool();
+
+    // ...and construct a CTxMemPool from it
+    return CTxMemPool{mempool_opts};
+}
+
+void CheckATMPInvariants(const MempoolAcceptResult& res, bool txid_in_mempool, bool wtxid_in_mempool)
+{
+
+    switch (res.m_result_type) {
+    case MempoolAcceptResult::ResultType::VALID:
+    {
+        Assert(txid_in_mempool);
+        Assert(wtxid_in_mempool);
+        Assert(res.m_state.IsValid());
+        Assert(!res.m_state.IsInvalid());
+        Assert(res.m_replaced_transactions);
+        Assert(res.m_vsize);
+        Assert(res.m_base_fees);
+        Assert(res.m_effective_feerate);
+        Assert(res.m_wtxids_fee_calculations);
+        Assert(!res.m_other_wtxid);
+        break;
+    }
+    case MempoolAcceptResult::ResultType::INVALID:
+    {
+        // It may be already in the mempool since in ATMP cases we don't set MEMPOOL_ENTRY or DIFFERENT_WITNESS
+        Assert(!res.m_state.IsValid());
+        Assert(res.m_state.IsInvalid());
+        Assert(!res.m_replaced_transactions);
+        Assert(!res.m_vsize);
+        Assert(!res.m_base_fees);
+        // Unable or unwilling to calculate fees
+        Assert(!res.m_effective_feerate);
+        Assert(!res.m_wtxids_fee_calculations);
+        Assert(!res.m_other_wtxid);
+        break;
+    }
+    case MempoolAcceptResult::ResultType::MEMPOOL_ENTRY:
+    {
+        // ATMP never sets this; only set in package settings
+        Assert(false);
+        break;
+    }
+    case MempoolAcceptResult::ResultType::DIFFERENT_WITNESS:
+    {
+        // ATMP never sets this; only set in package settings
+        Assert(false);
+        break;
+    }
+    }
+}
+
 FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
@@ -247,9 +308,11 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
         SyncWithValidationInterfaceQueue();
         UnregisterSharedValidationInterface(txr);
 
+        bool txid_in_mempool = tx_pool.exists(GenTxid::Txid(tx->GetHash()));
+        bool wtxid_in_mempool = tx_pool.exists(GenTxid::Wtxid(tx->GetWitnessHash()));
+        CheckATMPInvariants(res, txid_in_mempool, wtxid_in_mempool);
+
         Assert(accepted != added.empty());
-        Assert(accepted == res.m_state.IsValid());
-        Assert(accepted != res.m_state.IsInvalid());
         if (accepted) {
             Assert(added.size() == 1); // For now, no package acceptance
             Assert(tx == *added.begin());

src/test/util/txmempool.cpp

@@ -0,0 +1,116 @@
+// Copyright (c) 2022 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/util/txmempool.h>
+
+#include <chainparams.h>
+#include <node/context.h>
+#include <node/mempool_args.h>
+#include <txmempool.h>
+#include <util/check.h>
+#include <util/time.h>
+#include <util/translation.h>
+#include <validation.h>
+
+using node::NodeContext;
+
+CTxMemPool::Options MemPoolOptionsForTest(const NodeContext& node)
+{
+    CTxMemPool::Options mempool_opts{
+        .estimator = node.fee_estimator.get(),
+        // Default to always checking mempool regardless of
+        // chainparams.DefaultConsistencyChecks for tests
+        .check_ratio = 1,
+    };
+    const auto result{ApplyArgsManOptions(*node.args, ::Params(), mempool_opts)};
+    Assert(result);
+    return mempool_opts;
+}
+
+CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) const
+{
+    return FromTx(MakeTransactionRef(tx));
+}
+
+CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const
+{
+    return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch<std::chrono::seconds>(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp};
+}
+
+std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
+                                                           const PackageMempoolAcceptResult& result,
+                                                           bool expect_valid,
+                                                           const CTxMemPool* mempool)
+{
+    if (expect_valid) {
+        if (result.m_state.IsInvalid()) {
+            return strprintf("Package validation unexpectedly failed: %s", result.m_state.ToString());
+        }
+    } else {
+        if (result.m_state.IsValid()) {
+            strprintf("Package validation unexpectedly succeeded. %s", result.m_state.ToString());
+        }
+    }
+    if (result.m_state.GetResult() != PackageValidationResult::PCKG_POLICY && txns.size() != result.m_tx_results.size()) {
+        strprintf("txns size %u does not match tx results size %u", txns.size(), result.m_tx_results.size());
+    }
+    for (const auto& tx : txns) {
+        const auto& wtxid = tx->GetWitnessHash();
+        if (result.m_tx_results.count(wtxid) == 0) {
+            return strprintf("result not found for tx %s", wtxid.ToString());
+        }
+
+        const auto& atmp_result = result.m_tx_results.at(wtxid);
+        const bool valid{atmp_result.m_result_type == MempoolAcceptResult::ResultType::VALID};
+        if (expect_valid && atmp_result.m_state.IsInvalid()) {
+            return strprintf("tx %s unexpectedly failed: %s", wtxid.ToString(), atmp_result.m_state.ToString());
+        }
+
+        //m_replaced_transactions should exist iff the result was VALID
+        if (atmp_result.m_replaced_transactions.has_value() != valid) {
+            return strprintf("tx %s result should %shave m_replaced_transactions",
+                                    wtxid.ToString(), valid ? "" : "not ");
+        }
+
+        // m_vsize and m_base_fees should exist iff the result was VALID or MEMPOOL_ENTRY
+        const bool mempool_entry{atmp_result.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY};
+        if (atmp_result.m_base_fees.has_value() != (valid || mempool_entry)) {
+            return strprintf("tx %s result should %shave m_base_fees", wtxid.ToString(), valid || mempool_entry ? "" : "not ");
+        }
+        if (atmp_result.m_vsize.has_value() != (valid || mempool_entry)) {
+            return strprintf("tx %s result should %shave m_vsize", wtxid.ToString(), valid || mempool_entry ? "" : "not ");
+        }
+
+        // m_other_wtxid should exist iff the result was DIFFERENT_WITNESS
+        const bool diff_witness{atmp_result.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS};
+        if (atmp_result.m_other_wtxid.has_value() != diff_witness) {
+            return strprintf("tx %s result should %shave m_other_wtxid", wtxid.ToString(), diff_witness ? "" : "not ");
+        }
+
+        // m_effective_feerate and m_wtxids_fee_calculations should exist iff the result was valid
+        if (atmp_result.m_effective_feerate.has_value() != valid) {
+            return strprintf("tx %s result should %shave m_effective_feerate",
+                                    wtxid.ToString(), valid ? "" : "not ");
+        }
+        if (atmp_result.m_wtxids_fee_calculations.has_value() != valid) {
+            return strprintf("tx %s result should %shave m_effective_feerate",
+                                    wtxid.ToString(), valid ? "" : "not ");
+        }
+
+        if (mempool) {
+            // The tx by txid should be in the mempool iff the result was not INVALID.
+            const bool txid_in_mempool{atmp_result.m_result_type != MempoolAcceptResult::ResultType::INVALID};
+            if (mempool->exists(GenTxid::Txid(tx->GetHash())) != txid_in_mempool) {
+                strprintf("tx %s should %sbe in mempool", wtxid.ToString(), txid_in_mempool ? "" : "not ");
+            }
+            // Additionally, if the result was DIFFERENT_WITNESS, we shouldn't be able to find the tx in mempool by wtxid.
+            if (tx->HasWitness() && atmp_result.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS) {
+                if (mempool->exists(GenTxid::Wtxid(wtxid))) {
+                    strprintf("wtxid %s should not be in mempool", wtxid.ToString());
+                }
+            }
+        }
+    }
+    return std::nullopt;
+}

src/test/util/txmempool.h

@@ -0,0 +1,49 @@
+// Copyright (c) 2022 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_UTIL_TXMEMPOOL_H
+#define BITCOIN_TEST_UTIL_TXMEMPOOL_H
+
+#include <policy/packages.h>
+#include <txmempool.h>
+#include <util/time.h>
+
+namespace node {
+struct NodeContext;
+}
+struct PackageMempoolAcceptResult;
+
+CTxMemPool::Options MemPoolOptionsForTest(const node::NodeContext& node);
+
+struct TestMemPoolEntryHelper {
+    // Default values
+    CAmount nFee{0};
+    NodeSeconds time{};
+    unsigned int nHeight{1};
+    uint64_t m_sequence{0};
+    bool spendsCoinbase{false};
+    unsigned int sigOpCost{4};
+    LockPoints lp;
+
+    CTxMemPoolEntry FromTx(const CMutableTransaction& tx) const;
+    CTxMemPoolEntry FromTx(const CTransactionRef& tx) const;
+
+    // Change the default value
+    TestMemPoolEntryHelper& Fee(CAmount _fee) { nFee = _fee; return *this; }
+    TestMemPoolEntryHelper& Time(NodeSeconds tp) { time = tp; return *this; }
+    TestMemPoolEntryHelper& Height(unsigned int _height) { nHeight = _height; return *this; }
+    TestMemPoolEntryHelper& Sequence(uint64_t _seq) { m_sequence = _seq; return *this; }
+    TestMemPoolEntryHelper& SpendsCoinbase(bool _flag) { spendsCoinbase = _flag; return *this; }
+    TestMemPoolEntryHelper& SigOpsCost(unsigned int _sigopsCost) { sigOpCost = _sigopsCost; return *this; }
+};
+
+/** Check expected properties for every PackageMempoolAcceptResult, regardless of value. Returns
+ * a string if an error occurs with error populated, nullopt otherwise. If mempool is provided,
+ * checks that the expected transactions are in mempool (this should be set to nullptr for a test_accept).
+*/
+std::optional<std::string>  CheckPackageMempoolAcceptResult(const Package& txns,
+                                                            const PackageMempoolAcceptResult& result,
+                                                            bool expect_valid,
+                                                            const CTxMemPool* mempool);
+#endif // BITCOIN_TEST_UTIL_TXMEMPOOL_H