MOVEONLY: DisconnectedBlockTransactions from txmempool to validation

This is only used by validation and has very little to do with mempool. It should live in validation.
glozow · Aug 31, 2023 · 6c8152f · 6c8152f
1 parent 4774b43
commit 6c8152f
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 78 deletions.
diff --git a/src/txmempool.h b/src/txmempool.h
@@ -837,81 +837,4 @@ class CCoinsViewMemPool : public CCoinsViewBacked
     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.
- */
-struct DisconnectedBlockTransactions {
-    uint64_t cachedInnerUsage = 0;
-    std::list<CTransactionRef> queuedTx;
-
-    // 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()); }
-
-    // Estimate the overhead of std::list to be 3 pointers + an allocation per entry.
-    size_t DynamicMemoryUsage() const {
-        return memusage::MallocUsage(sizeof(CTransactionRef) + 3 * sizeof(void*)) * queuedTx.size() + cachedInnerUsage;
-    }
-
-    void addTransaction(const CTransactionRef& tx)
-    {
-        queuedTx.push_back(tx);
-        cachedInnerUsage += RecursiveDynamicUsage(tx);
-    }
-
-    // Remove entries based on txid_index, and update memory usage.
-    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;
-        }
-        // Create a set of all block txids.
-        std::unordered_set<uint256, SaltedTxidHasher> txids;
-        std::transform(vtx.cbegin(), vtx.cend(), std::inserter(txids, txids.end()), [](const auto& tx) { return tx->GetHash(); });
-        // Iterate through entire list once, removing any transactions in the block.
-        // We expect to remove the majority of transactions, i.e. the new chain
-        // confirmed mostly the same transactions.
-        auto it = queuedTx.begin();
-        while (it != queuedTx.end()) {
-            auto it_next = std::next(it);
-            if (txids.count((*it)->GetHash()) > 0) {
-                cachedInnerUsage -= RecursiveDynamicUsage(*it);
-                queuedTx.erase(it);
-            }
-            it = it_next;
-        }
-    }
-
-    // Remove an entry by insertion_order index, and update memory usage.
-    void remove_first()
-    {
-        cachedInnerUsage -= RecursiveDynamicUsage(queuedTx.front());
-        queuedTx.pop_front();
-    }
-
-    void clear()
-    {
-        cachedInnerUsage = 0;
-        queuedTx.clear();
-    }
-};
-
 #endif // BITCOIN_TXMEMPOOL_H
diff --git a/src/validation.h b/src/validation.h
@@ -51,7 +51,6 @@ class CBlockTreeDB;
 class CTxMemPool;
 class ChainstateManager;
 struct ChainTxData;
-struct DisconnectedBlockTransactions;
 struct PrecomputedTransactionData;
 struct LockPoints;
 struct AssumeutxoData;
@@ -233,6 +232,83 @@ struct PackageMempoolAcceptResult
         : m_tx_results{ {wtxid, result} } {}
 };
 
+/**
+ * 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.
+ */
+struct DisconnectedBlockTransactions {
+    uint64_t cachedInnerUsage = 0;
+    std::list<CTransactionRef> queuedTx;
+
+    // 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()); }
+
+    // Estimate the overhead of std::list to be 3 pointers + an allocation per entry.
+    size_t DynamicMemoryUsage() const {
+        return memusage::MallocUsage(sizeof(CTransactionRef) + 3 * sizeof(void*)) * queuedTx.size() + cachedInnerUsage;
+    }
+
+    void addTransaction(const CTransactionRef& tx)
+    {
+        queuedTx.push_back(tx);
+        cachedInnerUsage += RecursiveDynamicUsage(tx);
+    }
+
+    // Remove entries based on txid_index, and update memory usage.
+    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;
+        }
+        // Create a set of all block txids.
+        std::unordered_set<uint256, SaltedTxidHasher> txids;
+        std::transform(vtx.cbegin(), vtx.cend(), std::inserter(txids, txids.end()), [](const auto& tx) { return tx->GetHash(); });
+        // Iterate through entire list once, removing any transactions in the block.
+        // We expect to remove the majority of transactions, i.e. the new chain
+        // confirmed mostly the same transactions.
+        auto it = queuedTx.begin();
+        while (it != queuedTx.end()) {
+            auto it_next = std::next(it);
+            if (txids.count((*it)->GetHash()) > 0) {
+                cachedInnerUsage -= RecursiveDynamicUsage(*it);
+                queuedTx.erase(it);
+            }
+            it = it_next;
+        }
+    }
+
+    // Remove an entry by insertion_order index, and update memory usage.
+    void remove_first()
+    {
+        cachedInnerUsage -= RecursiveDynamicUsage(queuedTx.front());
+        queuedTx.pop_front();
+    }
+
+    void clear()
+    {
+        cachedInnerUsage = 0;
+        queuedTx.clear();
+    }
+};
+
 /**
  * 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()