validation: return expected block transactions in `MempoolRemovedForB…

…lock` notification update
ismaelsadeeq · Mar 4, 2025 · a768c64 · a768c64
1 parent a07434f
commit a768c64
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 18 deletions.
diff --git a/src/policy/fees/block_policy_estimator.cpp b/src/policy/fees/block_policy_estimator.cpp
@@ -588,7 +588,7 @@ void CBlockPolicyEstimator::TransactionRemovedFromMempool(const CTransactionRef&
     removeTx(tx->GetHash());
 }
 
-void CBlockPolicyEstimator::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
+void CBlockPolicyEstimator::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, const std::vector<CTransactionRef>& /*unused*/, unsigned int nBlockHeight)
 {
     processBlock(txs_removed_for_block, nBlockHeight);
 }

diff --git a/src/policy/fees/block_policy_estimator.h b/src/policy/fees/block_policy_estimator.h
@@ -274,7 +274,7 @@ class CBlockPolicyEstimator : public Forecaster, public CValidationInterface
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
-    void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) override
+    void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, const std::vector<CTransactionRef>& /*unused*/, unsigned int nBlockHeight) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** Overridden from Forecaster. */

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
@@ -401,7 +401,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
     /* after tx6 is mined, tx7 should move up in the sort */
     std::vector<CTransactionRef> vtx;
     vtx.push_back(MakeTransactionRef(tx6));
-    pool.removeForBlock(vtx, 1);
+    pool.removeForBlock(vtx, {}, 1);
 
     sortedOrder.erase(sortedOrder.begin()+1);
     // Ties are broken by hash
@@ -561,7 +561,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE);
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000);
     // ... we should keep the same min fee until we get a block
-    pool.removeForBlock(vtx, 1);
+    pool.removeForBlock(vtx, {}, 1);
     SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE);
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + 1000)/2.0));
     // ... then feerate should drop 1/2 each halflife

diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
@@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
 
         {
             LOCK(mpool.cs);
-            mpool.removeForBlock(block, ++blocknum);
+            mpool.removeForBlock(block, {}, ++blocknum);
         }
 
         block.clear();
@@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     // We haven't decayed the moving average enough so we still have enough data points in every bucket
     while (blocknum < 250) {
         LOCK(mpool.cs);
-        mpool.removeForBlock(block, ++blocknum);
+        mpool.removeForBlock(block, {}, ++blocknum);
     }
 
     // Wait for fee estimator to catch up
@@ -184,7 +184,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
         }
         {
             LOCK(mpool.cs);
-            mpool.removeForBlock(block, ++blocknum);
+            mpool.removeForBlock(block, {}, ++blocknum);
         }
     }
 
@@ -208,7 +208,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
 
     {
         LOCK(mpool.cs);
-        mpool.removeForBlock(block, 266);
+        mpool.removeForBlock(block, {}, 266);
     }
     block.clear();
 
@@ -252,7 +252,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
 
         {
             LOCK(mpool.cs);
-            mpool.removeForBlock(block, ++blocknum);
+            mpool.removeForBlock(block, {}, ++blocknum);
         }
 
         block.clear();

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
@@ -664,7 +664,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
 /**
  * Called when a block is connected. Removes from mempool.
  */
-void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight)
+void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, const std::vector<CTransactionRef>& expected_block_txs, unsigned int nBlockHeight)
 {
     AssertLockHeld(cs);
     Assume(!m_have_changeset);
@@ -683,7 +683,7 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         ClearPrioritisation(tx->GetHash());
     }
     if (m_opts.signals) {
-        m_opts.signals->MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight);
+        m_opts.signals->MempoolTransactionsRemovedForBlock(txs_removed_for_block, expected_block_txs, nBlockHeight);
     }
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;

diff --git a/src/txmempool.h b/src/txmempool.h
@@ -464,7 +464,7 @@ class CTxMemPool
      * */
     void removeForReorg(CChain& chain, std::function<bool(txiter)> filter_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
     void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
-    void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    void removeForBlock(const std::vector<CTransactionRef>& vtx, const std::vector<CTransactionRef>& expected_block_txs, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid=false);
     bool isSpent(const COutPoint& outpoint) const;

diff --git a/src/validation.cpp b/src/validation.cpp
@@ -31,6 +31,8 @@
 #include <logging.h>
 #include <logging/timer.h>
 #include <node/blockstorage.h>
+#include <node/mini_miner.h>
+#include <node/types.h>
 #include <node/utxo_snapshot.h>
 #include <policy/ephemeral_policy.h>
 #include <policy/policy.h>
@@ -3235,7 +3237,8 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
              Ticks<MillisecondsDouble>(m_chainman.time_chainstate) / m_chainman.num_blocks_total);
     // Remove conflicting transactions from the mempool.;
     if (m_mempool) {
-        m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
+        auto expected_block = GenerateNewBlock(m_chainman.ActiveChainstate(), m_mempool);
+        m_mempool->removeForBlock(blockConnecting.vtx, expected_block->block.vtx, pindexNew->nHeight);
         disconnectpool.removeForBlock(blockConnecting.vtx);
     }
     // Update m_chain & related variables.

diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
@@ -217,10 +217,10 @@ void ValidationSignals::BlockConnected(ChainstateRole role, const std::shared_pt
                           pindex->nHeight);
 }
 
-void ValidationSignals::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
+void ValidationSignals::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, const std::vector<CTransactionRef>& expected_block_txs, unsigned int nBlockHeight)
 {
-    auto event = [txs_removed_for_block, nBlockHeight, this] {
-        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight); });
+    auto event = [txs_removed_for_block, expected_block_txs, nBlockHeight, this] {
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForBlock(txs_removed_for_block, expected_block_txs, nBlockHeight); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: block height=%s txs removed=%s", __func__,
                           nBlockHeight,

diff --git a/src/validationinterface.h b/src/validationinterface.h
@@ -112,7 +112,7 @@ class CValidationInterface {
      *
      * Called on a background thread.
      */
-    virtual void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) {}
+    virtual void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, const std::vector<CTransactionRef>& expected_block_txs, unsigned int nBlockHeight) {}
     /**
      * Notifies listeners of a block being connected.
      * Provides a vector of transactions evicted from the mempool as a result.
@@ -221,7 +221,7 @@ class ValidationSignals {
     void ActiveTipChange(const CBlockIndex&, bool);
     void TransactionAddedToMempool(const NewMempoolTransactionInfo&, uint64_t mempool_sequence);
     void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason, uint64_t mempool_sequence);
-    void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>&, unsigned int nBlockHeight);
+    void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>&, const std::vector<CTransactionRef>&, unsigned int nBlockHeight);
     void BlockConnected(ChainstateRole, const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
     void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);
     void ChainStateFlushed(ChainstateRole, const CBlockLocator &);