[fuzz] v3 transactions and sigop-adjusted vsize

Ensure we are checking sigop-adjusted virtual size by creating setups
and packages where sigop cost is larger than bip141 vsize.

Co-authored-by: Gregory Sanders <gsanders87@gmail.com>

Author: glozow

src/test/fuzz/package_eval.cpp

@@ -6,6 +6,7 @@
 #include <node/context.h>
 #include <node/mempool_args.h>
 #include <node/miner.h>
+#include <policy/v3_policy.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
@@ -119,7 +120,8 @@ CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeConte
     mempool_opts.limits.descendant_size_vbytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 202) * 1'000;
     mempool_opts.max_size_bytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 200) * 1'000'000;
     mempool_opts.expiry = std::chrono::hours{fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 999)};
-    nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(1, 999);
+    // Only interested in 2 cases: sigop cost 0 or when single legacy sigop cost is >> 1KvB
+    nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 1) * 10'000;
 
     mempool_opts.check_ratio = 1;
     mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool();
@@ -171,11 +173,11 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
             // Create transaction to add to the mempool
             const CTransactionRef tx = [&] {
                 CMutableTransaction tx_mut;
-                tx_mut.nVersion = CTransaction::CURRENT_VERSION;
+                tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
                 tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
                 // Last tx will sweep all outpoints in package
                 const auto num_in = last_tx ? package_outpoints.size()  : fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size());
-                const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size() * 2);
+                auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size() * 2);
 
                 auto& outpoints = last_tx ? package_outpoints : mempool_outpoints;
 
@@ -211,17 +213,24 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
                     tx_mut.vin.push_back(tx_mut.vin.back());
                 }
 
-                // Refer to a non-existant input
+                // Refer to a non-existent input
                 if (fuzzed_data_provider.ConsumeBool()) {
                     tx_mut.vin.emplace_back();
                 }
 
+                // Make a p2pk output to make sigops adjusted vsize to violate v3, potentially, which is never spent
+                if (last_tx && amount_in > 1000 && fuzzed_data_provider.ConsumeBool()) {
+                    tx_mut.vout.emplace_back(1000, CScript() << std::vector<unsigned char>(33, 0x02) << OP_CHECKSIG);
+                    // Don't add any other outputs.
+                    num_out = 1;
+                    amount_in -= 1000;
+                }
+
                 const auto amount_fee = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(0, amount_in);
                 const auto amount_out = (amount_in - amount_fee) / num_out;
                 for (int i = 0; i < num_out; ++i) {
                     tx_mut.vout.emplace_back(amount_out, P2WSH_EMPTY);
                 }
-                // TODO vary transaction sizes to catch size-related issues
                 auto tx = MakeTransactionRef(tx_mut);
                 // Restore previously removed outpoints, except in-package outpoints
                 if (!last_tx) {
@@ -261,7 +270,6 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
         std::set<CTransactionRef> added;
         auto txr = std::make_shared<TransactionsDelta>(added);
         RegisterSharedValidationInterface(txr);
-        const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
 
         // When there are multiple transactions in the package, we call ProcessNewPackage(txs, test_accept=false)
         // and AcceptToMemoryPool(txs.back(), test_accept=true). When there is only 1 transaction, we might flip it
@@ -271,17 +279,20 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
         const auto result_package = WITH_LOCK(::cs_main,
                                     return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit));
 
-        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(), bypass_limits, /*test_accept=*/!single_submit));
-        const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
+        // Always set bypass_limits to false because it is not supported in ProcessNewPackage and
+        // can be a source of divergence.
+        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(),
+                                   /*bypass_limits=*/false, /*test_accept=*/!single_submit));
+        const bool passed = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
 
         SyncWithValidationInterfaceQueue();
         UnregisterSharedValidationInterface(txr);
 
         // There is only 1 transaction in the package. We did a test-package-accept and a ATMP
         if (single_submit) {
-            Assert(accepted != added.empty());
-            Assert(accepted == res.m_state.IsValid());
-            if (accepted) {
+            Assert(passed != added.empty());
+            Assert(passed == res.m_state.IsValid());
+            if (passed) {
                 Assert(added.size() == 1);
                 Assert(txs.back() == *added.begin());
             }
@@ -295,6 +306,8 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
             // This is empty if it fails early checks, or "full" if transactions are looked at deeper
             Assert(result_package.m_tx_results.size() == txs.size() || result_package.m_tx_results.empty());
         }
+
+        CheckMempoolV3Invariants(tx_pool);
     }
 
     UnregisterSharedValidationInterface(outpoints_updater);

src/test/fuzz/tx_pool.cpp

@@ -6,6 +6,7 @@
 #include <node/context.h>
 #include <node/mempool_args.h>
 #include <node/miner.h>
+#include <policy/v3_policy.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
@@ -229,7 +230,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
         // Create transaction to add to the mempool
         const CTransactionRef tx = [&] {
             CMutableTransaction tx_mut;
-            tx_mut.nVersion = CTransaction::CURRENT_VERSION;
+            tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
             tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
             const auto num_in = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size());
             const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size() * 2);
@@ -315,6 +316,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
         if (accepted) {
             Assert(added.size() == 1); // For now, no package acceptance
             Assert(tx == *added.begin());
+            CheckMempoolV3Invariants(tx_pool);
         } else {
             // Do not consider rejected transaction removed
             removed.erase(tx);
@@ -407,6 +409,9 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         if (accepted) {
             txids.push_back(tx->GetHash());
+            // Only check fees if accepted and not bypass_limits, otherwise it's not guaranteed that
+            // trimming has happened for this tx and previous iterations.
+            CheckMempoolV3Invariants(tx_pool);
         }
     }
     Finish(fuzzed_data_provider, tx_pool, chainstate);

src/test/util/txmempool.cpp

@@ -7,6 +7,7 @@
 #include <chainparams.h>
 #include <node/context.h>
 #include <node/mempool_args.h>
+#include <policy/v3_policy.h>
 #include <txmempool.h>
 #include <util/check.h>
 #include <util/time.h>
@@ -116,3 +117,28 @@ std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
     }
     return std::nullopt;
 }
+
+void CheckMempoolV3Invariants(const CTxMemPool& tx_pool)
+{
+    LOCK(tx_pool.cs);
+    for (const auto& tx_info : tx_pool.infoAll()) {
+        const auto& entry = *Assert(tx_pool.GetEntry(tx_info.tx->GetHash()));
+        if (tx_info.tx->nVersion == 3) {
+            // Check that special v3 ancestor/descendant limits and rules are always respected
+            Assert(entry.GetCountWithDescendants() <= V3_DESCENDANT_LIMIT);
+            Assert(entry.GetCountWithAncestors() <= V3_ANCESTOR_LIMIT);
+            // If this transaction has at least 1 ancestor, it's a "child" and has restricted weight.
+            if (entry.GetCountWithAncestors() > 1) {
+                Assert(entry.GetTxSize() <= V3_CHILD_MAX_VSIZE);
+                // All v3 transactions must only have v3 unconfirmed parents.
+                const auto& parents = entry.GetMemPoolParentsConst();
+                Assert(parents.begin()->get().GetSharedTx()->nVersion == 3);
+            }
+        } else if (entry.GetCountWithAncestors() > 1) {
+            // All non-v3 transactions must only have non-v3 unconfirmed parents.
+            for (const auto& parent : entry.GetMemPoolParentsConst()) {
+                Assert(parent.get().GetSharedTx()->nVersion != 3);
+            }
+        }
+    }
+}

src/test/util/txmempool.h

@@ -46,4 +46,14 @@ std::optional<std::string>  CheckPackageMempoolAcceptResult(const Package& txns,
                                                             const PackageMempoolAcceptResult& result,
                                                             bool expect_valid,
                                                             const CTxMemPool* mempool);
+
+/** For every transaction in tx_pool, check v3 invariants:
+ * - a v3 tx's ancestor count must be within V3_ANCESTOR_LIMIT
+ * - a v3 tx's descendant count must be within V3_DESCENDANT_LIMIT
+ * - if a v3 tx has ancestors, its sigop-adjusted vsize must be within V3_CHILD_MAX_VSIZE
+ * - any non-v3 tx must only have non-v3 parents
+ * - any v3 tx must only have v3 parents
+ *   */
+void CheckMempoolV3Invariants(const CTxMemPool& tx_pool);
+
 #endif // BITCOIN_TEST_UTIL_TXMEMPOOL_H