Merge bitcoin/bitcoin#23465: Remove CTxMemPool params from ATMP

f1f10c0 Remove CTxMemPool params from ATMP (lsilva01)

Pull request description:

  Remove `CTxMemPool` parameter from `AcceptToMemoryPool` function, as suggested in bitcoin/bitcoin#23437 (comment) .

  This requires that `CChainState` has access to `MockedTxPool` in  `tx_pool.cpp` as mentioned bitcoin/bitcoin#23173 (comment). So the `MockedTxPool` is attributed to `CChainState::m_mempool` before calling `AcceptToMemoryPool`.

  Requires #23437.

ACKs for top commit:
  jnewbery:
    utACK f1f10c0
  MarcoFalke:
    review ACK f1f10c0 🔙

Tree-SHA512: 2a4885f4645014fc1fa98bb1090f13721c1a0796bc0021b9cb43bc8cc13920b6eaf057d1f5ed796e0a110e7813e41fe0196334ce7c80d1231fc057a9a3bdf349

Author: MarcoFalke

src/test/fuzz/tx_pool.cpp

@@ -29,6 +29,15 @@ struct MockedTxPool : public CTxMemPool {
     }
 };
 
+class DummyChainState final : public CChainState
+{
+public:
+    void SetMempool(CTxMemPool* mempool)
+    {
+        m_mempool = mempool;
+    }
+};
+
 void initialize_tx_pool()
 {
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
@@ -114,7 +123,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const auto& node = g_setup->m_node;
-    auto& chainstate = node.chainman->ActiveChainstate();
+    auto& chainstate{static_cast<DummyChainState&>(node.chainman->ActiveChainstate())};
 
     MockTime(fuzzed_data_provider, chainstate);
     SetMempoolConstraints(*node.args, fuzzed_data_provider);
@@ -134,6 +143,8 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
     CTxMemPool tx_pool_{/*estimator=*/nullptr, /*check_ratio=*/1};
     MockedTxPool& tx_pool = *static_cast<MockedTxPool*>(&tx_pool_);
 
+    chainstate.SetMempool(&tx_pool);
+
     // Helper to query an amount
     const CCoinsViewMemPool amount_view{WITH_LOCK(::cs_main, return &chainstate.CoinsTip()), tx_pool};
     const auto GetAmount = [&](const COutPoint& outpoint) {
@@ -230,7 +241,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
         Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
                it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
 
-        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(tx_pool, chainstate, tx, GetTime(), bypass_limits, /* test_accept= */ false));
+        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         SyncWithValidationInterfaceQueue();
         UnregisterSharedValidationInterface(txr);
@@ -330,7 +341,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool)
         const auto tx = MakeTransactionRef(mut_tx);
         const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
         ::fRequireStandard = fuzzed_data_provider.ConsumeBool();
-        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(tx_pool, chainstate, tx, GetTime(), bypass_limits, /* test_accept= */ false));
+        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         if (accepted) {
             txids.push_back(tx->GetHash());

src/validation.cpp

@@ -331,8 +331,8 @@ void CChainState::MaybeUpdateMempoolForReorg(
     while (it != disconnectpool.queuedTx.get<insertion_order>().rend()) {
         // ignore validation errors in resurrected transactions
         if (!fAddToMempool || (*it)->IsCoinBase() ||
-            AcceptToMemoryPool(*m_mempool, *this, *it, GetTime(),
-                /* bypass_limits= */true, /* test_accept= */ false).m_result_type !=
+            AcceptToMemoryPool(*this, *it, GetTime(),
+                /*bypass_limits=*/true, /*test_accept=*/false).m_result_type !=
                     MempoolAcceptResult::ResultType::VALID) {
             // If the transaction doesn't make it in to the mempool, remove any
             // transactions that depend on it (which would now be orphans).
@@ -1094,11 +1094,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
 
 } // anon namespace
 
-MempoolAcceptResult AcceptToMemoryPool(CTxMemPool& pool, CChainState& active_chainstate, const CTransactionRef& tx,
+MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, const CTransactionRef& tx,
                                        int64_t accept_time, bool bypass_limits, bool test_accept)
     EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 {
     const CChainParams& chainparams{active_chainstate.m_params};
+    assert(active_chainstate.GetMempool() != nullptr);
+    CTxMemPool& pool{*active_chainstate.GetMempool()};
+
     std::vector<COutPoint> coins_to_uncache;
     auto args = MemPoolAccept::ATMPArgs::SingleAccept(chainparams, accept_time, bypass_limits, coins_to_uncache, test_accept);
     const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args);
@@ -3496,13 +3499,13 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s
 MempoolAcceptResult ChainstateManager::ProcessTransaction(const CTransactionRef& tx, bool test_accept)
 {
     CChainState& active_chainstate = ActiveChainstate();
-    if (!active_chainstate.m_mempool) {
+    if (!active_chainstate.GetMempool()) {
         TxValidationState state;
         state.Invalid(TxValidationResult::TX_NO_MEMPOOL, "no-mempool");
         return MempoolAcceptResult::Failure(state);
     }
-    auto result = AcceptToMemoryPool(*active_chainstate.m_mempool, active_chainstate, tx, GetTime(), /* bypass_limits= */ false, test_accept);
-    active_chainstate.m_mempool->check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1);
+    auto result = AcceptToMemoryPool(active_chainstate, tx, GetTime(), /*bypass_limits=*/ false, test_accept);
+    active_chainstate.GetMempool()->check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1);
     return result;
 }
 
@@ -4568,8 +4571,8 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka
             }
             if (nTime > nNow - nExpiryTimeout) {
                 LOCK(cs_main);
-                if (AcceptToMemoryPool(pool, active_chainstate, tx, nTime, /* bypass_limits= */ false,
-                                               /* test_accept= */ false).m_result_type == MempoolAcceptResult::ResultType::VALID) {
+                const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false);
+                if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) {
                     ++count;
                 } else {
                     // mempool may contain the transaction already, e.g. from

src/validation.h

@@ -211,18 +211,16 @@ struct PackageMempoolAcceptResult
  * Try to add a transaction to the mempool. This is an internal function and is exposed only for testing.
  * Client code should use ChainstateManager::ProcessTransaction()
  *
- * @param[in]  pool               Reference to the node's mempool.
  * @param[in]  active_chainstate  Reference to the active chainstate.
  * @param[in]  tx                 The transaction to submit for mempool acceptance.
- * @param[in]  accept_time        The timestamp for adding the transaction to the mempool. Usually
- *                                the current system time, but may be different.
+ * @param[in]  accept_time        The timestamp for adding the transaction to the mempool.
  *                                It is also used to determine when the entry expires.
  * @param[in]  bypass_limits      When true, don't enforce mempool fee and capacity limits.
  * @param[in]  test_accept        When true, run validation checks but don't submit to mempool.
  *
  * @returns a MempoolAcceptResult indicating whether the transaction was accepted/rejected with reason.
  */
-MempoolAcceptResult AcceptToMemoryPool(CTxMemPool& pool, CChainState& active_chainstate, const CTransactionRef& tx,
+MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, const CTransactionRef& tx,
                                        int64_t accept_time, bool bypass_limits, bool test_accept)
     EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
@@ -649,6 +647,12 @@ class CChainState
         return m_coins_views->m_dbview;
     }
 
+    //! @returns A pointer to the mempool.
+    CTxMemPool* GetMempool()
+    {
+        return m_mempool;
+    }
+
     //! @returns A reference to a wrapped view of the in-memory UTXO set that
     //!     handles disk read errors gracefully.
     CCoinsViewErrorCatcher& CoinsErrorCatcher() EXCLUSIVE_LOCKS_REQUIRED(cs_main)