Merge bitcoin/bitcoin#24927: Add test util to populate mempool with random transactions, fix #24634 bug

d2f8f1b use testing setup mempool in ComplexMemPool bench (glozow)
aecc332 create and use mempool transactions using real coins in MempoolCheck (glozow)
2118750 [test util] to populate mempool with random transactions/packages (glozow)
5374dfc [test util] use -checkmempool for TestingSetup mempool check ratio (glozow)
d7d9c7b [test util] add chain name to TestChain100Setup ctor (glozow)

Pull request description:

  Fixes #24634 by using the `testing_setup`'s actual mempool rather than a locally-declared mempool for running `check()`.

  Also creates a test utility for populating the mempool with a bunch of random transactions. I imagine this could be useful in other places as well; it was necessary here because we needed the mempool to contain transactions *spending coins available in the current chainstate*. The existing `CreateOrderedCoins()` is insufficient because it creates coins out of thin air.

  Also implements the separate suggestion to use the `TestingSetup` mempool in `ComplexMemPool` bench.

ACKs for top commit:
  laanwj:
    Code review ACK d2f8f1b

Tree-SHA512: 44ab5a9e55b126b5a5bc33f05fbad1380b9c43c84736c7cf487be025e0e3f5d75216ccf5a3088b0935da817e3dacfba99d2885f75bcb6e7eaa24cd20a82c24c8

Author: laanwj

src/bench/mempool_stress.cpp

@@ -88,7 +88,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
     }
     std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /*min_ancestors=*/1);
     const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN);
-    CTxMemPool pool;
+    CTxMemPool& pool = *testing_setup.get()->m_node.mempool;
     LOCK2(cs_main, pool.cs);
     bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
         for (auto& tx : ordered_coins) {
@@ -102,16 +102,15 @@ static void ComplexMemPool(benchmark::Bench& bench)
 static void MempoolCheck(benchmark::Bench& bench)
 {
     FastRandomContext det_rand{true};
-    const int childTxs = bench.complexityN() > 1 ? static_cast<int>(bench.complexityN()) : 2000;
-    const std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /*min_ancestors=*/5);
-    const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN, {"-checkmempool=1"});
-    CTxMemPool pool;
+    auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(CBaseChainParams::REGTEST, {"-checkmempool=1"});
+    CTxMemPool& pool = *testing_setup.get()->m_node.mempool;
     LOCK2(cs_main, pool.cs);
+    testing_setup->PopulateMempool(det_rand, 400, true);
     const CCoinsViewCache& coins_tip = testing_setup.get()->m_node.chainman->ActiveChainstate().CoinsTip();
-    for (auto& tx : ordered_coins) AddTx(tx, pool);
 
     bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
-        pool.check(coins_tip, /*spendheight=*/2);
+        // Bump up the spendheight so we don't hit premature coinbase spend errors.
+        pool.check(coins_tip, /*spendheight=*/300);
     });
 }
 

src/test/txvalidationcache_tests.cpp

@@ -15,7 +15,7 @@
 
 struct Dersig100Setup : public TestChain100Setup {
     Dersig100Setup()
-        : TestChain100Setup{{"-testactivationheight=dersig@102"}} {}
+        : TestChain100Setup{CBaseChainParams::REGTEST, {"-testactivationheight=dersig@102"}} {}
 };
 
 bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,

src/test/util/setup_common.cpp

@@ -43,6 +43,7 @@
 #include <validationinterface.h>
 #include <walletinitinterface.h>
 
+#include <algorithm>
 #include <functional>
 #include <stdexcept>
 
@@ -161,7 +162,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
     GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler);
 
     m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
-    m_node.mempool = std::make_unique<CTxMemPool>(m_node.fee_estimator.get(), 1);
+    m_node.mempool = std::make_unique<CTxMemPool>(m_node.fee_estimator.get(), m_node.args->GetIntArg("-checkmempool", 1));
 
     m_cache_sizes = CalculateCacheSizes(m_args);
 
@@ -242,8 +243,8 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
     }
 }
 
-TestChain100Setup::TestChain100Setup(const std::vector<const char*>& extra_args)
-    : TestingSetup{CBaseChainParams::REGTEST, extra_args}
+TestChain100Setup::TestChain100Setup(const std::string& chain_name, const std::vector<const char*>& extra_args)
+    : TestingSetup{chain_name, extra_args}
 {
     SetMockTime(1598887952);
     constexpr std::array<unsigned char, 32> vchKey = {
@@ -357,6 +358,52 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio
     return mempool_txn;
 }
 
+std::vector<CTransactionRef> TestChain100Setup::PopulateMempool(FastRandomContext& det_rand, size_t num_transactions, bool submit)
+{
+    std::vector<CTransactionRef> mempool_transactions;
+    std::deque<std::pair<COutPoint, CAmount>> unspent_prevouts;
+    std::transform(m_coinbase_txns.begin(), m_coinbase_txns.end(), std::back_inserter(unspent_prevouts),
+        [](const auto& tx){ return std::make_pair(COutPoint(tx->GetHash(), 0), tx->vout[0].nValue); });
+    while (num_transactions > 0 && !unspent_prevouts.empty()) {
+        // The number of inputs and outputs are random, between 1 and 24.
+        CMutableTransaction mtx = CMutableTransaction();
+        const size_t num_inputs = det_rand.randrange(24) + 1;
+        CAmount total_in{0};
+        for (size_t n{0}; n < num_inputs; ++n) {
+            if (unspent_prevouts.empty()) break;
+            const auto& [prevout, amount] = unspent_prevouts.front();
+            mtx.vin.push_back(CTxIn(prevout, CScript()));
+            total_in += amount;
+            unspent_prevouts.pop_front();
+        }
+        const size_t num_outputs = det_rand.randrange(24) + 1;
+        // Approximately 1000sat "fee," equal output amounts.
+        const CAmount amount_per_output = (total_in - 1000) / num_outputs;
+        for (size_t n{0}; n < num_outputs; ++n) {
+            CScript spk = CScript() << CScriptNum(num_transactions + n);
+            mtx.vout.push_back(CTxOut(amount_per_output, spk));
+        }
+        CTransactionRef ptx = MakeTransactionRef(mtx);
+        mempool_transactions.push_back(ptx);
+        if (amount_per_output > 2000) {
+            // If the value is high enough to fund another transaction + fees, keep track of it so
+            // it can be used to build a more complex transaction graph. Insert randomly into
+            // unspent_prevouts for extra randomness in the resulting structures.
+            for (size_t n{0}; n < num_outputs; ++n) {
+                unspent_prevouts.push_back(std::make_pair(COutPoint(ptx->GetHash(), n), amount_per_output));
+                std::swap(unspent_prevouts.back(), unspent_prevouts[det_rand.randrange(unspent_prevouts.size())]);
+            }
+        }
+        if (submit) {
+            LOCK2(m_node.mempool->cs, cs_main);
+            LockPoints lp;
+            m_node.mempool->addUnchecked(CTxMemPoolEntry(ptx, 1000, 0, 1, false, 4, lp));
+        }
+        --num_transactions;
+    }
+    return mempool_transactions;
+}
+
 CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) const
 {
     return FromTx(MakeTransactionRef(tx));

src/test/util/setup_common.h

@@ -122,7 +122,8 @@ class CScript;
  * Testing fixture that pre-creates a 100-block REGTEST-mode block chain
  */
 struct TestChain100Setup : public TestingSetup {
-    TestChain100Setup(const std::vector<const char*>& extra_args = {});
+    TestChain100Setup(const std::string& chain_name = CBaseChainParams::REGTEST,
+                      const std::vector<const char*>& extra_args = {});
 
     /**
      * Create a new block with just given transactions, coinbase paying to
@@ -164,6 +165,19 @@ struct TestChain100Setup : public TestingSetup {
                                                       CAmount output_amount = CAmount(1 * COIN),
                                                       bool submit = true);
 
+    /** Create transactions spending from m_coinbase_txns. These transactions will only spend coins
+     * that exist in the current chain, but may be premature coinbase spends, have missing
+     * signatures, or violate some other consensus rules. They should only be used for testing
+     * mempool consistency. All transactions will have some random number of inputs and outputs
+     * (between 1 and 24). Transactions may or may not be dependent upon each other; if dependencies
+     * exit, every parent will always be somewhere in the list before the child so each transaction
+     * can be submitted in the same order they appear in the list.
+     * @param[in]   submit      When true, submit transactions to the mempool.
+     *                          When false, return them but don't submit them.
+     * @returns A vector of transactions that can be submitted to the mempool.
+     */
+    std::vector<CTransactionRef> PopulateMempool(FastRandomContext& det_rand, size_t num_transactions, bool submit);
+
     std::vector<CTransactionRef> m_coinbase_txns; // For convenience, coinbase transactions
     CKey coinbaseKey; // private/public key needed to spend coinbase transactions
 };