diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index ab1146e1dc9..1d22707075a 100644 --- a/src/bench/mempool_stress.cpp +++ b/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); } diff --git a/src/policy/fees/estimator_man.cpp b/src/policy/fees/estimator_man.cpp index 472367e8fa3..9b5c5093c46 100644 --- a/src/policy/fees/estimator_man.cpp +++ b/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& txs_removed_for_block, unsigned int nBlockHeight) +void FeeRateEstimatorManager::MempoolTransactionsRemovedForBlock(const std::shared_ptr& /*block*/, const std::vector& 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 diff --git a/src/policy/fees/estimator_man.h b/src/policy/fees/estimator_man.h index bbcb8aa1b28..0e8c36b8cc0 100644 --- a/src/policy/fees/estimator_man.h +++ b/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& txs_removed_for_block, unsigned int nBlockHeight) override; + void MempoolTransactionsRemovedForBlock(const std::shared_ptr& block, const std::vector& txs_removed_for_block, unsigned int block_height) override; private: std::unique_ptr m_block_policy_estimator; diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index c14586f446a..ce59be4270a 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/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 hashes_to_update; diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 0d1e6b779bb..80810f387a8 100644 --- a/src/test/mempool_tests.cpp +++ b/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 diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index 15134791eb5..8b41f6090a8 100644 --- a/src/test/rbf_tests.cpp +++ b/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) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index e5ec3b32cab..ed758a3ceb8 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -402,7 +402,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) } } -void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned int nBlockHeight) +std::vector CTxMemPool::removeForBlock(const std::vector& 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& 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 diff --git a/src/txmempool.h b/src/txmempool.h index 35339d3f252..7d7aafde781 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -329,7 +329,7 @@ public: * and updates an entry's LockPoints. * */ void removeForReorg(CChain& chain, std::function filter_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); - void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector removeForBlock(const std::vector& vtx) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Look up wtxids in the mempool and (partially) sort by mining score. * diff --git a/src/validation.cpp b/src/validation.cpp index 09cb93c998d..27e72df6082 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3087,14 +3087,19 @@ bool Chainstate::ConnectTip( Ticks(time_5 - time_4), Ticks(m_chainman.time_chainstate), Ticks(m_chainman.time_chainstate) / m_chainman.num_blocks_total); - // Remove conflicting transactions from the mempool.; + // Remove conflicting transactions from the mempool. + std::vector 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()}; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index ec2f2047da6..128f14a6d54 100644 --- a/src/validationinterface.cpp +++ b/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& txs_removed_for_block, unsigned int nBlockHeight) +void ValidationSignals::MempoolTransactionsRemovedForBlock(std::shared_ptr block, std::vector 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)); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 8cfe41380ec..ec70cc09569 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -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& txs_removed_for_block, unsigned int nBlockHeight) {} + virtual void MempoolTransactionsRemovedForBlock(const std::shared_ptr& block, const std::vector& 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&, unsigned int nBlockHeight); + void MempoolTransactionsRemovedForBlock(std::shared_ptr, std::vector, unsigned int block_height); void BlockConnected(const kernel::ChainstateRole&, std::shared_ptr, const CBlockIndex* pindex); void BlockDisconnected(std::shared_ptr, const CBlockIndex* pindex); void ChainStateFlushed(const kernel::ChainstateRole&, const CBlockLocator&);