Merge branch 'rm_batch_updater_stuff' into 'master'

mempool: Remove BatchUpdater classes, consolidate & clean-up removeForBlock Closes Bitcoin-ABC#285 See merge request bitcoin-cash-node/bitcoin-cash-node!1180
bitcoin-cash-node · Apr 19, 2021 · 1b6ddfe · 1b6ddfe
2 parents 68015ed + 14dcc05
commit 1b6ddfe
Show file tree

Hide file tree

Showing 11 changed files with 22 additions and 125 deletions.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -504,7 +504,6 @@ add_library(server
 	net_processing.cpp
 	node/transaction.cpp
 	noui.cpp
-	mempool/defaultbatchupdater.cpp
 	outputtype.cpp
 	policy/fees.cpp
 	policy/policy.cpp

diff --git a/src/Makefile.am b/src/Makefile.am
@@ -166,8 +166,6 @@ BITCOIN_CORE_H = \
   dbwrapper.h \
   limitedmap.h \
   logging.h \
-  mempool/batchupdater.h \
-  mempool/defaultbatchupdater.h \
   memusage.h \
   merkleblock.h \
   miner.h \
@@ -293,7 +291,6 @@ libbitcoin_server_a_SOURCES = \
   interfaces/handler.cpp \
   interfaces/node.cpp \
   dbwrapper.cpp \
-  mempool/defaultbatchupdater.cpp \
   merkleblock.cpp \
   miner.cpp \
   net.cpp \

diff --git a/src/mempool/batchupdater.h b/src/mempool/batchupdater.h
diff --git a/src/mempool/defaultbatchupdater.cpp b/src/mempool/defaultbatchupdater.cpp
diff --git a/src/mempool/defaultbatchupdater.h b/src/mempool/defaultbatchupdater.h
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
@@ -468,7 +468,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) {
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(),
                       maxFeeRateRemoved.GetFeePerK() + feeIncrement);
     // ... we should keep the same min fee until we get a block
-    pool.removeForBlock(vtx, 1);
+    pool.removeForBlock(vtx);
     SetMockTime(42 + 2 * CTxMemPool::ROLLING_FEE_HALFLIFE);
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(),
                       (maxFeeRateRemoved.GetFeePerK() + feeIncrement) / 2);

diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
@@ -51,7 +51,8 @@ BOOST_AUTO_TEST_CASE(MempoolMinimumFeeEstimate) {
             CTransactionRef ptx = mpool.get(txid);
             block.push_back(ptx);
         }
-        mpool.removeForBlock(block, ++blocknum);
+        ++blocknum;
+        mpool.removeForBlock(block);
         block.clear();
     }
 

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
@@ -15,7 +15,6 @@
 #include <consensus/validation.h>
 #include <dsproof/dsproof.h>
 #include <dsproof/storage.h>
-#include <mempool/defaultbatchupdater.h>
 #include <policy/fees.h>
 #include <policy/mempool.h>
 #include <policy/policy.h>
@@ -239,8 +238,6 @@ CTxMemPool::CTxMemPool()
     // transactions becomes O(N^2) where N is the number of transactions in the
     // pool
     nCheckFrequency = 0;
-
-    batchUpdater = std::make_unique<mempool::DefaultBatchUpdater>(*this);
 }
 
 CTxMemPool::~CTxMemPool() {}
@@ -421,14 +418,28 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) {
  * Called when a block is connected. Removes from mempool and updates the miner
  * fee estimator.
  */
-void CTxMemPool::removeForBlock(const std::vector<CTransactionRef> &vtx,
-                                unsigned int nBlockHeight) {
+void CTxMemPool::removeForBlock(const std::vector<CTransactionRef> &vtx) {
+    DisconnectedBlockTransactions disconnectpool;
+    disconnectpool.addForBlock(vtx);
+
     LOCK(cs);
 
-    batchUpdater->removeForBlock(vtx, nBlockHeight);
+    // iterate in topological order (parents before children)
+    for (const CTransactionRef &tx : reverse_iterate(disconnectpool.GetQueuedTx().get<insertion_order>())) {
+        const txiter it = mapTx.find(tx->GetId());
+        if (it != mapTx.end()) {
+            setEntries stage;
+            stage.insert(it);
+            RemoveStaged(stage, MemPoolRemovalReason::BLOCK);
+        }
+        removeConflicts(*tx);
+        ClearPrioritisation(tx->GetId());
+    }
 
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;
+
+    disconnectpool.clear();
 }
 
 void CTxMemPool::_clear(bool clearDspOrphans /*= true*/) {

diff --git a/src/txmempool.h b/src/txmempool.h
@@ -38,8 +38,6 @@ class Config;
 class DoubleSpendProof;
 class DoubleSpendProofStorage;
 
-namespace mempool { class BatchUpdater; }
-
 
 extern RecursiveMutex cs_main;
 
@@ -372,8 +370,6 @@ class CTxMemPool {
 
     bool m_is_loaded GUARDED_BY(cs){false};
 
-    std::unique_ptr<mempool::BatchUpdater> batchUpdater;
-
     //! Used by addUnchecked to generate ever-increasing CTxMemPoolEntry::entryId's
     uint64_t nextEntryId GUARDED_BY(cs) = 1;
 
@@ -584,8 +580,7 @@ class CTxMemPool {
         const CTransaction &tx,
         MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN);
     void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
-    void removeForBlock(const std::vector<CTransactionRef> &vtx,
-                        unsigned int nBlockHeight);
+    void removeForBlock(const std::vector<CTransactionRef> &vtx);
 
     void clear(bool clearDspOrphans = true);
     // lock free

diff --git a/src/validation.cpp b/src/validation.cpp
@@ -2633,7 +2633,7 @@ bool CChainState::ConnectTip(const Config &config, CValidationState &state,
              nTimeChainState * MILLI / nBlocksTotal);
 
     // Remove conflicting transactions from the mempool.;
-    g_mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
+    g_mempool.removeForBlock(blockConnecting.vtx);
     disconnectpool.removeForBlock(blockConnecting.vtx);
 
     // If this block is activating a fork, we move all mempool transactions

diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh
@@ -39,7 +39,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
     "chainparams -> protocol -> config -> chainparams"
     "config -> policy/policy -> validation -> config"
     "config -> policy/policy -> validation -> protocol -> config"
-    "mempool/defaultbatchupdater -> txmempool -> mempool/defaultbatchupdater"
 )
 
 EXIT_CODE=0