validation: emit block mempool removal signal from ConnectTip

Return the removed mempool transaction info from
CTxMemPool::removeForBlock instead of dispatching the
MempoolTransactionsRemovedForBlock notification from the mempool.

Emit it from ConnectTip after mempool removal and before BlockConnected,
passing the connected block, the removed mempool transactions, and the
block height to the callback.

Because the signal now originates from ConnectTip, where the IBD state is
known, gate it on !IsInitialBlockDownload(): the notification is no longer
fired for blocks connected during initial block download or reindex, while
the mempool removal in removeForBlock still runs unconditionally. This keeps
fee rate estimators from recording blocks connected before the node is
synced.

src/bench/mempool_stress.cpp

@@ -156,7 +156,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
     // in the same state at the end of the function, so we benchmark both
     // mining a block and reorging the block's contents back into the mempool.
     bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
-        pool.removeForBlock(tx_remove_for_block, /*nBlockHeight=*/100);
+        pool.removeForBlock(tx_remove_for_block);
         for (auto& tx: tx_remove_for_block) {
             AddTx(tx, pool, det_rand);
         }

src/policy/fees/estimator_man.cpp

@@ -72,9 +72,9 @@ void FeeRateEstimatorManager::TransactionRemovedFromMempool(const CTransactionRe
     m_block_policy_estimator->removeTx(tx->GetHash());
 }
 
-void FeeRateEstimatorManager::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
+void FeeRateEstimatorManager::MempoolTransactionsRemovedForBlock(const std::shared_ptr<const CBlock>& /*block*/, const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int block_height)
 {
-    m_block_policy_estimator->processBlock(txs_removed_for_block, nBlockHeight);
+    m_block_policy_estimator->processBlock(txs_removed_for_block, block_height);
 }
 
 CFeeRate FeeRateEstimatorManager::BlockPolicyEstimateRawFee(unsigned int target, double threshold, FeeEstimateHorizon horizon, EstimationResult* buckets) const

src/policy/fees/estimator_man.h

@@ -81,7 +81,7 @@ protected:
     /** Overridden from CValidationInterface. */
     void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/) override;
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/) override;
-    void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) override;
+    void MempoolTransactionsRemovedForBlock(const std::shared_ptr<const CBlock>& block, const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int block_height) override;
 
 private:
     std::unique_ptr<CBlockPolicyEstimator> m_block_policy_estimator;

src/test/fuzz/tx_pool.cpp

@@ -156,7 +156,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, Cha
 
         // Try updating the mempool for this block, as though it were mined.
         LOCK2(::cs_main, tx_pool.cs);
-        tx_pool.removeForBlock(block_template->block.vtx, chainstate.m_chain.Height() + 1);
+        tx_pool.removeForBlock(block_template->block.vtx);
 
         // Now try to add those transactions back, as though a reorg happened.
         std::vector<Txid> hashes_to_update;

src/test/mempool_tests.cpp

@@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     clock += HALFLIFE;
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + DEFAULT_INCREMENTAL_RELAY_FEE);
     // ... we should keep the same min fee until we get a block
-    pool.removeForBlock(vtx, 1);
+    pool.removeForBlock(vtx);
     clock += HALFLIFE;
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + DEFAULT_INCREMENTAL_RELAY_FEE)/2.0));
     // ... then feerate should drop 1/2 each halflife

src/test/rbf_tests.cpp

@@ -216,7 +216,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_conflicts_calculator, TestChain100Setup)
     dummy.clear();
 
     // If we mine the parent_tx's, then the clusters split (102 clusters).
-    pool.removeForBlock({parent_tx_1, parent_tx_2}, /*nBlockHeight=*/ 1);
+    pool.removeForBlock({parent_tx_1, parent_tx_2});
 
     // Add some descendants now to each of the direct children (we can do this now that the clusters have split).
     for (const auto& child : direct_children) {

src/txmempool.cpp

@@ -402,7 +402,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
     }
 }
 
-void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight)
+std::vector<RemovedMempoolTransactionInfo> CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx)
 {
     // Remove confirmed txs and conflicts when a new block is connected, updating the fee logic
     AssertLockHeld(cs);
@@ -420,14 +420,12 @@ 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);
-    }
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;
     if (!m_txgraph->DoWork(/*max_cost=*/POST_CHANGE_COST)) {
         LogDebug(BCLog::MEMPOOL, "Mempool in non-optimal ordering after block.");
     }
+    return txs_removed_for_block;
 }
 
 void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const

src/txmempool.h

@@ -329,7 +329,7 @@ public:
      *                                        and updates an entry's LockPoints.
      * */
     void removeForReorg(CChain& chain, std::function<bool(txiter)> filter_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
-    void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    std::vector<RemovedMempoolTransactionInfo> removeForBlock(const std::vector<CTransactionRef>& vtx) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** Look up wtxids in the mempool and (partially) sort by mining score.
      *

src/validation.cpp

@@ -3087,14 +3087,19 @@ bool Chainstate::ConnectTip(
              Ticks<MillisecondsDouble>(time_5 - time_4),
              Ticks<SecondsDouble>(m_chainman.time_chainstate),
              Ticks<MillisecondsDouble>(m_chainman.time_chainstate) / m_chainman.num_blocks_total);
-    // Remove conflicting transactions from the mempool.;
+    // Remove conflicting transactions from the mempool.
+    std::vector<RemovedMempoolTransactionInfo> txs_removed_for_block;
     if (m_mempool) {
-        m_mempool->removeForBlock(block_to_connect->vtx, pindexNew->nHeight);
+        txs_removed_for_block = m_mempool->removeForBlock(block_to_connect->vtx);
         disconnectpool.removeForBlock(block_to_connect->vtx);
     }
     // Update m_chain & related variables.
     m_chain.SetTip(*pindexNew);
     m_chainman.UpdateIBDStatus();
+    // Not fired while IBD is active. removeForBlock() above still runs.
+    if (m_mempool && m_chainman.m_options.signals && !m_chainman.IsInitialBlockDownload()) {
+        m_chainman.m_options.signals->MempoolTransactionsRemovedForBlock(block_to_connect, std::move(txs_removed_for_block), pindexNew->nHeight);
+    }
     UpdateTip(pindexNew);
 
     const auto time_6{SteadyClock::now()};

src/validationinterface.cpp

@@ -230,13 +230,16 @@ void ValidationSignals::BlockConnected(const ChainstateRole& role, std::shared_p
     ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg));
 }
 
-void ValidationSignals::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
+void ValidationSignals::MempoolTransactionsRemovedForBlock(std::shared_ptr<const CBlock> block, std::vector<RemovedMempoolTransactionInfo> txs_removed_for_block, unsigned int block_height)
 {
-    auto log_msg = LOG_MSG("%s: block height=%s txs removed=%s", __func__,
-                          nBlockHeight,
-                          txs_removed_for_block.size());
-    auto event = [txs_removed_for_block, nBlockHeight, this] {
-        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight); });
+    Assume(block);
+    auto log_msg = LOG_MSG("%s: block hash=%s block height=%s txs removed=%s block txs=%s", __func__,
+                           block->GetHash().ToString(),
+                           block_height,
+                           txs_removed_for_block.size(),
+                           block->vtx.size());
+    auto event = [block = std::move(block), txs_removed_for_block = std::move(txs_removed_for_block), block_height, this] {
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForBlock(block, txs_removed_for_block, block_height); });
     };
     ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg));
 }

src/validationinterface.h

@@ -9,6 +9,7 @@
 #include <kernel/cs_main.h>
 #include <primitives/transaction.h>
 #include <sync.h>
+#include <uint256.h>
 
 #include <cstddef>
 #include <cstdint>
@@ -112,9 +113,11 @@ protected:
      * as a result of new block being connected.
      * MempoolTransactionsRemovedForBlock will be fired before BlockConnected.
      *
+     * Not fired while initial block download is active.
+     *
      * Called on a background thread.
      */
-    virtual void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) {}
+    virtual void MempoolTransactionsRemovedForBlock(const std::shared_ptr<const CBlock>& block, const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int block_height) {}
     /**
      * Notifies listeners of a block being connected.
      *
@@ -222,7 +225,7 @@ public:
     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(std::shared_ptr<const CBlock>, std::vector<RemovedMempoolTransactionInfo>, unsigned int block_height);
     void BlockConnected(const kernel::ChainstateRole&, std::shared_ptr<const CBlock>, const CBlockIndex* pindex);
     void BlockDisconnected(std::shared_ptr<const CBlock>, const CBlockIndex* pindex);
     void ChainStateFlushed(const kernel::ChainstateRole&, const CBlockLocator&);