MOVEONLY: DisconnectedBlockTransactions to its own file

This struct is only used in validation + tests and has very little to do
with txmempool.

src/Makefile.am

@@ -186,6 +186,7 @@ BITCOIN_CORE_H = \
   kernel/coinstats.h \
   kernel/context.h \
   kernel/cs_main.h \
+  kernel/disconnected_transactions.h \
   kernel/mempool_entry.h \
   kernel/mempool_limits.h \
   kernel/mempool_options.h \

src/bench/disconnected_transactions.cpp

@@ -3,9 +3,10 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <bench/bench.h>
+#include <kernel/disconnected_transactions.h>
+#include <primitives/block.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
-#include <validation.h>
 
 constexpr size_t BLOCK_VTX_COUNT{4000};
 constexpr size_t BLOCK_VTX_COUNT_10PERCENT{400};

src/kernel/disconnected_transactions.h

@@ -0,0 +1,128 @@
+// Copyright (c) 2023 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_KERNEL_DISCONNECTED_TRANSACTIONS_H
+#define BITCOIN_KERNEL_DISCONNECTED_TRANSACTIONS_H
+
+#include <core_memusage.h>
+#include <memusage.h>
+#include <primitives/transaction.h>
+#include <util/hasher.h>
+
+#include <list>
+#include <unordered_map>
+
+/**
+ * DisconnectedBlockTransactions
+
+ * During the reorg, it's desirable to re-add previously confirmed transactions
+ * to the mempool, so that anything not re-confirmed in the new chain is
+ * available to be mined. However, it's more efficient to wait until the reorg
+ * is complete and process all still-unconfirmed transactions at that time,
+ * since we expect most confirmed transactions to (typically) still be
+ * confirmed in the new chain, and re-accepting to the memory pool is expensive
+ * (and therefore better to not do in the middle of reorg-processing).
+ * Instead, store the disconnected transactions (in order!) as we go, remove any
+ * that are included in blocks in the new chain, and then process the remaining
+ * still-unconfirmed transactions at the end.
+ *
+ * Order of queuedTx:
+ * The front of the list should be the most recently-confirmed transactions (transactions at the
+ * end of vtx of blocks closer to the tip). If memory usage grows too large, we trim from the front
+ * of the list. After trimming, transactions can be re-added to the mempool from the back of the
+ * list to the front without running into missing inputs.
+ */
+class DisconnectedBlockTransactions {
+private:
+    /** Cached dynamic memory usage for the CTransactions (memory for the shared pointers is
+     * included in the container calculations). */
+    uint64_t cachedInnerUsage = 0;
+    std::list<CTransactionRef> queuedTx;
+    using TxList = decltype(queuedTx);
+    std::unordered_map<uint256, TxList::iterator, SaltedTxidHasher> iters_by_txid;
+
+public:
+    // It's almost certainly a logic bug if we don't clear out queuedTx before
+    // destruction, as we add to it while disconnecting blocks, and then we
+    // need to re-process remaining transactions to ensure mempool consistency.
+    // For now, assert() that we've emptied out this object on destruction.
+    // This assert() can always be removed if the reorg-processing code were
+    // to be refactored such that this assumption is no longer true (for
+    // instance if there was some other way we cleaned up the mempool after a
+    // reorg, besides draining this object).
+    ~DisconnectedBlockTransactions() {
+        assert(queuedTx.empty());
+        assert(iters_by_txid.empty());
+        assert(cachedInnerUsage == 0);
+    }
+
+    size_t DynamicMemoryUsage() const {
+        return cachedInnerUsage + memusage::DynamicUsage(iters_by_txid) + memusage::DynamicUsage(queuedTx);
+    }
+
+    /** Add transactions from the block, iterating through vtx in reverse order. Callers should call
+     * this function for blocks in descending order by block height.
+     * We assume that callers never pass multiple transactions with the same txid, otherwise things
+     * can go very wrong in removeForBlock due to queuedTx containing an item without a
+     * corresponding entry in iters_by_txid.
+     */
+    void AddTransactionsFromBlock(const std::vector<CTransactionRef>& vtx)
+    {
+        iters_by_txid.reserve(iters_by_txid.size() + vtx.size());
+        for (auto block_it = vtx.rbegin(); block_it != vtx.rend(); ++block_it) {
+            auto it = queuedTx.insert(queuedTx.end(), *block_it);
+            iters_by_txid.emplace((*block_it)->GetHash(), it);
+            cachedInnerUsage += RecursiveDynamicUsage(**block_it);
+        }
+    }
+
+    /** Remove any entries that are in this block. */
+    void removeForBlock(const std::vector<CTransactionRef>& vtx)
+    {
+        // Short-circuit in the common case of a block being added to the tip
+        if (queuedTx.empty()) {
+            return;
+        }
+        for (const auto& tx : vtx) {
+            auto iter = iters_by_txid.find(tx->GetHash());
+            if (iter != iters_by_txid.end()) {
+                auto list_iter = iter->second;
+                iters_by_txid.erase(iter);
+                cachedInnerUsage -= RecursiveDynamicUsage(**list_iter);
+                queuedTx.erase(list_iter);
+            }
+        }
+    }
+
+    /** Remove the first entry and update memory usage. */
+    CTransactionRef take_first()
+    {
+        CTransactionRef first_tx;
+        if (!queuedTx.empty()) {
+            first_tx = queuedTx.front();
+            cachedInnerUsage -= RecursiveDynamicUsage(*queuedTx.front());
+            iters_by_txid.erase(queuedTx.front()->GetHash());
+            queuedTx.pop_front();
+        }
+        return first_tx;
+    }
+
+    size_t size() const { return queuedTx.size(); }
+
+    void clear()
+    {
+        cachedInnerUsage = 0;
+        iters_by_txid.clear();
+        queuedTx.clear();
+    }
+
+    /** Clear all data structures and return the list of transactions. */
+    std::list<CTransactionRef> take()
+    {
+        std::list<CTransactionRef> ret = std::move(queuedTx);
+        clear();
+        return ret;
+    }
+};
+#endif // BITCOIN_KERNEL_DISCONNECTED_TRANSACTIONS_H

src/test/validation_chainstatemanager_tests.cpp

@@ -4,6 +4,7 @@
 //
 #include <chainparams.h>
 #include <consensus/validation.h>
+#include <kernel/disconnected_transactions.h>
 #include <node/kernel_notifications.h>
 #include <node/utxo_snapshot.h>
 #include <random.h>

src/txmempool.h

@@ -836,118 +836,4 @@ public:
      * m_temp_added and cannot be flushed to the back end. Only used for package validation. */
     void PackageAddTransaction(const CTransactionRef& tx);
 };
-
-/**
- * DisconnectedBlockTransactions
-
- * During the reorg, it's desirable to re-add previously confirmed transactions
- * to the mempool, so that anything not re-confirmed in the new chain is
- * available to be mined. However, it's more efficient to wait until the reorg
- * is complete and process all still-unconfirmed transactions at that time,
- * since we expect most confirmed transactions to (typically) still be
- * confirmed in the new chain, and re-accepting to the memory pool is expensive
- * (and therefore better to not do in the middle of reorg-processing).
- * Instead, store the disconnected transactions (in order!) as we go, remove any
- * that are included in blocks in the new chain, and then process the remaining
- * still-unconfirmed transactions at the end.
- *
- * Order of queuedTx:
- * The front of the list should be the most recently-confirmed transactions (transactions at the
- * end of vtx of blocks closer to the tip). If memory usage grows too large, we trim from the front
- * of the list. After trimming, transactions can be re-added to the mempool from the back of the
- * list to the front without running into missing inputs.
- */
-class DisconnectedBlockTransactions {
-private:
-    /** Cached dynamic memory usage for the CTransactions (memory for the shared pointers is
-     * included in the container calculations). */
-    uint64_t cachedInnerUsage = 0;
-    std::list<CTransactionRef> queuedTx;
-    using TxList = decltype(queuedTx);
-    std::unordered_map<uint256, TxList::iterator, SaltedTxidHasher> iters_by_txid;
-
-public:
-    // It's almost certainly a logic bug if we don't clear out queuedTx before
-    // destruction, as we add to it while disconnecting blocks, and then we
-    // need to re-process remaining transactions to ensure mempool consistency.
-    // For now, assert() that we've emptied out this object on destruction.
-    // This assert() can always be removed if the reorg-processing code were
-    // to be refactored such that this assumption is no longer true (for
-    // instance if there was some other way we cleaned up the mempool after a
-    // reorg, besides draining this object).
-    ~DisconnectedBlockTransactions() {
-        assert(queuedTx.empty());
-        assert(iters_by_txid.empty());
-        assert(cachedInnerUsage == 0);
-    }
-
-    size_t DynamicMemoryUsage() const {
-        return cachedInnerUsage + memusage::DynamicUsage(iters_by_txid) + memusage::DynamicUsage(queuedTx);
-    }
-
-    /** Add transactions from the block, iterating through vtx in reverse order. Callers should call
-     * this function for blocks in descending order by block height.
-     * We assume that callers never pass multiple transactions with the same txid, otherwise things
-     * can go very wrong in removeForBlock due to queuedTx containing an item without a
-     * corresponding entry in iters_by_txid.
-     */
-    void AddTransactionsFromBlock(const std::vector<CTransactionRef>& vtx)
-    {
-        iters_by_txid.reserve(iters_by_txid.size() + vtx.size());
-        for (auto block_it = vtx.rbegin(); block_it != vtx.rend(); ++block_it) {
-            auto it = queuedTx.insert(queuedTx.end(), *block_it);
-            iters_by_txid.emplace((*block_it)->GetHash(), it);
-            cachedInnerUsage += RecursiveDynamicUsage(**block_it);
-        }
-    }
-
-    /** Remove any entries that are in this block. */
-    void removeForBlock(const std::vector<CTransactionRef>& vtx)
-    {
-        // Short-circuit in the common case of a block being added to the tip
-        if (queuedTx.empty()) {
-            return;
-        }
-        for (const auto& tx : vtx) {
-            auto iter = iters_by_txid.find(tx->GetHash());
-            if (iter != iters_by_txid.end()) {
-                auto list_iter = iter->second;
-                iters_by_txid.erase(iter);
-                cachedInnerUsage -= RecursiveDynamicUsage(**list_iter);
-                queuedTx.erase(list_iter);
-            }
-        }
-    }
-
-    /** Remove the first entry and update memory usage. */
-    CTransactionRef take_first()
-    {
-        CTransactionRef first_tx;
-        if (!queuedTx.empty()) {
-            first_tx = queuedTx.front();
-            cachedInnerUsage -= RecursiveDynamicUsage(*queuedTx.front());
-            iters_by_txid.erase(queuedTx.front()->GetHash());
-            queuedTx.pop_front();
-        }
-        return first_tx;
-    }
-
-    size_t size() const { return queuedTx.size(); }
-
-    void clear()
-    {
-        cachedInnerUsage = 0;
-        iters_by_txid.clear();
-        queuedTx.clear();
-    }
-
-    /** Clear all data structures and return the list of transactions. */
-    std::list<CTransactionRef> take()
-    {
-        std::list<CTransactionRef> ret = std::move(queuedTx);
-        clear();
-        return ret;
-    }
-};
-
 #endif // BITCOIN_TXMEMPOOL_H

src/validation.cpp

@@ -21,6 +21,7 @@
 #include <flatfile.h>
 #include <hash.h>
 #include <kernel/chainparams.h>
+#include <kernel/disconnected_transactions.h>
 #include <kernel/mempool_entry.h>
 #include <kernel/notifications_interface.h>
 #include <logging.h>