From a0e3eb7549d2ba4dd3af12b9ce65e29158f59078 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Fri, 3 Nov 2023 11:01:52 +0100
Subject: [PATCH 1/7] tx fees, policy: bugfix: move `removeTx` into reason !=
 `BLOCK` condition

If the removal reason of a transaction is BLOCK, then the `removeTx`
boolean argument should be true.

Before this PR, `CBlockPolicyEstimator` have to complete updating the fee stats
before the mempool clears that's why having removeTx call outside reason!= `BLOCK`
in `addUnchecked` was not a bug.

But in a case where the `CBlockPolicyEstimator` update is asynchronous, the mempool might
clear before we update the `CBlockPolicyEstimator` fee stats.
Transactions that are removed for `BLOCK` reasons will also be incorrectly removed from
`CBlockPolicyEstimator` stats as failures.
---
 src/txmempool.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index e057d7ece1c..7fd9c1cc253 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -497,12 +497,16 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
     // even if not directly reported below.
     uint64_t mempool_sequence = GetAndIncrementSequence();
 
+    const auto& hash = it->GetTx().GetHash();
     if (reason != MemPoolRemovalReason::BLOCK) {
         // Notify clients that a transaction has been removed from the mempool
         // for any reason except being included in a block. Clients interested
         // in transactions included in blocks can subscribe to the BlockConnected
         // notification.
         GetMainSignals().TransactionRemovedFromMempool(it->GetSharedTx(), reason, mempool_sequence);
+        if (minerPolicyEstimator) {
+            minerPolicyEstimator->removeTx(hash, false);
+        }
     }
     TRACE5(mempool, removed,
         it->GetTx().GetHash().data(),
@@ -512,7 +516,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
         std::chrono::duration_cast<std::chrono::duration<std::uint64_t>>(it->GetTime()).count()
     );
 
-    const uint256 hash = it->GetTx().GetHash();
     for (const CTxIn& txin : it->GetTx().vin)
         mapNextTx.erase(txin.prevout);
 
@@ -535,7 +538,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
     cachedInnerUsage -= memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst());
     mapTx.erase(it);
     nTransactionsUpdated++;
-    if (minerPolicyEstimator) {minerPolicyEstimator->removeTx(hash, false);}
 }
 
 // Calculates descendants of entry that are not already in setDescendants, and adds to

From 0889e07987294d4ef2814abfca16d8e2a0c5f541 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Fri, 1 Sep 2023 11:03:21 +0100
Subject: [PATCH 2/7] tx fees, policy: cast with static_cast instead of C-Style
 cast

---
 src/policy/fees.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index 654c4cf0ce6..df5885b0595 100644
--- a/src/policy/fees.cpp
+++ b/src/policy/fees.cpp
@@ -610,11 +610,11 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo
     CFeeRate feeRate(entry.GetFee(), entry.GetTxSize());
 
     mapMemPoolTxs[hash].blockHeight = txHeight;
-    unsigned int bucketIndex = feeStats->NewTx(txHeight, (double)feeRate.GetFeePerK());
+    unsigned int bucketIndex = feeStats->NewTx(txHeight, static_cast<double>(feeRate.GetFeePerK()));
     mapMemPoolTxs[hash].bucketIndex = bucketIndex;
-    unsigned int bucketIndex2 = shortStats->NewTx(txHeight, (double)feeRate.GetFeePerK());
+    unsigned int bucketIndex2 = shortStats->NewTx(txHeight, static_cast<double>(feeRate.GetFeePerK()));
     assert(bucketIndex == bucketIndex2);
-    unsigned int bucketIndex3 = longStats->NewTx(txHeight, (double)feeRate.GetFeePerK());
+    unsigned int bucketIndex3 = longStats->NewTx(txHeight, static_cast<double>(feeRate.GetFeePerK()));
     assert(bucketIndex == bucketIndex3);
 }
 
@@ -640,9 +640,9 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM
     // Feerates are stored and reported as BTC-per-kb:
     CFeeRate feeRate(entry->GetFee(), entry->GetTxSize());
 
-    feeStats->Record(blocksToConfirm, (double)feeRate.GetFeePerK());
-    shortStats->Record(blocksToConfirm, (double)feeRate.GetFeePerK());
-    longStats->Record(blocksToConfirm, (double)feeRate.GetFeePerK());
+    feeStats->Record(blocksToConfirm, static_cast<double>(feeRate.GetFeePerK()));
+    shortStats->Record(blocksToConfirm, static_cast<double>(feeRate.GetFeePerK()));
+    longStats->Record(blocksToConfirm, static_cast<double>(feeRate.GetFeePerK()));
     return true;
 }
 

From bfcd401368fc0dc43827a8969a37b7e038d5ca79 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Fri, 3 Nov 2023 12:34:29 +0100
Subject: [PATCH 3/7] CValidationInterface, mempool: add new callback to
 `CValidationInterface`

This commit adds a new callback `MempoolTransactionsRemovedForBlock` which notify
its listeners of the transactions that are removed from the mempool because a new
block is connected, along with the block height the transactions were removed.
The transactions are in `RemovedMempoolTransactionInfo` format.

`CTransactionRef`, base fee, virtual size, and height which the transaction was added
to the mempool are all members of the struct called `RemovedMempoolTransactionInfo`.

A struct `NewMempoolTransactionInfo`, which has fields similar to `RemovedMempoolTransactionInfo`,
will be added in a later commit, create a struct `TransactionInfo` with all similar fields.
They can both have a member with type `TransactionInfo`.
---
 src/kernel/mempool_entry.h  | 30 ++++++++++++++++++++++++++++++
 src/txmempool.cpp           |  4 ++++
 src/validationinterface.cpp | 11 +++++++++++
 src/validationinterface.h   | 17 ++++++++++++++---
 4 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index 7c905ca4f4a..ce32fe20dcc 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -178,4 +178,34 @@ class CTxMemPoolEntry
 
 using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef;
 
+struct TransactionInfo {
+    const CTransactionRef m_tx;
+    /* The fee the transaction paid */
+    const CAmount m_fee;
+    /**
+     * The virtual transaction size.
+     *
+     * This is a policy field which considers the sigop cost of the
+     * transaction as well as its weight, and reinterprets it as bytes.
+     *
+     * It is the primary metric by which the mining algorithm selects
+     * transactions.
+     */
+    const int64_t m_virtual_transaction_size;
+    /* The block height the transaction entered the mempool */
+    const unsigned int txHeight;
+
+    TransactionInfo(const CTransactionRef& tx, const CAmount& fee, const int64_t vsize, const unsigned int height)
+        : m_tx{tx},
+          m_fee{fee},
+          m_virtual_transaction_size{vsize},
+          txHeight{height} {}
+};
+
+struct RemovedMempoolTransactionInfo {
+    TransactionInfo info;
+    explicit RemovedMempoolTransactionInfo(const CTxMemPoolEntry& entry)
+        : info{entry.GetSharedTx(), entry.GetFee(), entry.GetTxSize(), entry.GetHeight()} {}
+};
+
 #endif // BITCOIN_KERNEL_MEMPOOL_ENTRY_H
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 7fd9c1cc253..f75dd7efeaa 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -654,17 +654,21 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
     }
     // Before the txs in the new block have been removed from the mempool, update policy estimates
     if (minerPolicyEstimator) {minerPolicyEstimator->processBlock(nBlockHeight, entries);}
+    std::vector<RemovedMempoolTransactionInfo> txs_removed_for_block;
+    txs_removed_for_block.reserve(vtx.size());
     for (const auto& tx : vtx)
     {
         txiter it = mapTx.find(tx->GetHash());
         if (it != mapTx.end()) {
             setEntries stage;
             stage.insert(it);
+            txs_removed_for_block.emplace_back(*it);
             RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
         }
         removeConflicts(*tx);
         ClearPrioritisation(tx->GetHash());
     }
+    GetMainSignals().MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight);
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;
 }
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index 9241395ad54..893ef695829 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -9,6 +9,7 @@
 #include <chain.h>
 #include <consensus/validation.h>
 #include <kernel/chain.h>
+#include <kernel/mempool_entry.h>
 #include <logging.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
@@ -233,6 +234,16 @@ void CMainSignals::BlockConnected(ChainstateRole role, const std::shared_ptr<con
                           pindex->nHeight);
 }
 
+void CMainSignals::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
+{
+    auto event = [txs_removed_for_block, nBlockHeight, this] {
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight); });
+    };
+    ENQUEUE_AND_LOG_EVENT(event, "%s: block height=%s txs removed=%s", __func__,
+                          nBlockHeight,
+                          txs_removed_for_block.size());
+}
+
 void CMainSignals::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
 {
     auto event = [pblock, pindex, this] {
diff --git a/src/validationinterface.h b/src/validationinterface.h
index eb15aa4d5f7..ea49a45aa87 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -21,6 +21,7 @@ struct CBlockLocator;
 class CValidationInterface;
 class CScheduler;
 enum class MemPoolRemovalReason;
+struct RemovedMempoolTransactionInfo;
 
 /** Register subscriber */
 void RegisterValidationInterface(CValidationInterface* callbacks);
@@ -60,10 +61,10 @@ void CallFunctionInValidationInterfaceQueue(std::function<void ()> func);
 void SyncWithValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main);
 
 /**
- * Implement this to subscribe to events generated in validation
+ * Implement this to subscribe to events generated in validation and mempool
  *
  * Each CValidationInterface() subscriber will receive event callbacks
- * in the order in which the events were generated by validation.
+ * in the order in which the events were generated by validation and mempool.
  * Furthermore, each ValidationInterface() subscriber may assume that
  * callbacks effectively run in a single thread with single-threaded
  * memory consistency. That is, for a given ValidationInterface()
@@ -113,7 +114,7 @@ class CValidationInterface {
      * This does not fire for transactions that are removed from the mempool
      * because they have been included in a block. Any client that is interested
      * in transactions removed from the mempool for inclusion in a block can learn
-     * about those transactions from the BlockConnected notification.
+     * about those transactions from the MempoolTransactionsRemovedForBlock notification.
      *
      * Transactions that are removed from the mempool because they conflict
      * with a transaction in the new block will have
@@ -131,6 +132,14 @@ class CValidationInterface {
      * Called on a background thread.
      */
     virtual void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {}
+    /*
+     * Notifies listeners of transactions removed from the mempool as
+     * as a result of new block being connected.
+     * MempoolTransactionsRemovedForBlock will be fired before BlockConnected.
+     *
+     * Called on a background thread.
+     */
+    virtual void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) {}
     /**
      * Notifies listeners of a block being connected.
      * Provides a vector of transactions evicted from the mempool as a result.
@@ -140,6 +149,7 @@ class CValidationInterface {
     virtual void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex) {}
     /**
      * Notifies listeners of a block being disconnected
+     *  Provides the block that was connected.
      *
      * Called on a background thread. Only called for the active chainstate, since
      * background chainstates should never disconnect blocks.
@@ -202,6 +212,7 @@ class CMainSignals {
     void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload);
     void TransactionAddedToMempool(const CTransactionRef&, uint64_t mempool_sequence);
     void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason, uint64_t mempool_sequence);
+    void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>&, 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 &);

From 91532bd38223d7d04166e05de11d0d0b55e60f13 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Fri, 3 Nov 2023 13:23:30 +0100
Subject: [PATCH 4/7] tx fees, policy: update
 `CBlockPolicyEstimator::processBlock` parameter

Update `processBlock` parameter to reference to a vector of `RemovedMempoolTransactionInfo`.
---
 src/policy/fees.cpp                | 18 +++++++++---------
 src/policy/fees.h                  |  7 ++++---
 src/test/fuzz/policy_estimator.cpp |  8 ++++----
 src/txmempool.cpp                  | 15 ++++-----------
 4 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index df5885b0595..6d550b1d5ac 100644
--- a/src/policy/fees.cpp
+++ b/src/policy/fees.cpp
@@ -618,10 +618,10 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo
     assert(bucketIndex == bucketIndex3);
 }
 
-bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry)
+bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const RemovedMempoolTransactionInfo& tx)
 {
     AssertLockHeld(m_cs_fee_estimator);
-    if (!_removeTx(entry->GetTx().GetHash(), true)) {
+    if (!_removeTx(tx.info.m_tx->GetHash(), true)) {
         // This transaction wasn't being tracked for fee estimation
         return false;
     }
@@ -629,7 +629,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM
     // How many blocks did it take for miners to include this transaction?
     // blocksToConfirm is 1-based, so a transaction included in the earliest
     // possible block has confirmation count of 1
-    int blocksToConfirm = nBlockHeight - entry->GetHeight();
+    int blocksToConfirm = nBlockHeight - tx.info.txHeight;
     if (blocksToConfirm <= 0) {
         // This can't happen because we don't process transactions from a block with a height
         // lower than our greatest seen height
@@ -638,7 +638,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM
     }
 
     // Feerates are stored and reported as BTC-per-kb:
-    CFeeRate feeRate(entry->GetFee(), entry->GetTxSize());
+    CFeeRate feeRate(tx.info.m_fee, tx.info.m_virtual_transaction_size);
 
     feeStats->Record(blocksToConfirm, static_cast<double>(feeRate.GetFeePerK()));
     shortStats->Record(blocksToConfirm, static_cast<double>(feeRate.GetFeePerK()));
@@ -646,8 +646,8 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM
     return true;
 }
 
-void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
-                                         std::vector<const CTxMemPoolEntry*>& entries)
+void CBlockPolicyEstimator::processBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block,
+                                         unsigned int nBlockHeight)
 {
     LOCK(m_cs_fee_estimator);
     if (nBlockHeight <= nBestSeenHeight) {
@@ -676,8 +676,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
 
     unsigned int countedTxs = 0;
     // Update averages with data points from current block
-    for (const auto& entry : entries) {
-        if (processBlockTx(nBlockHeight, entry))
+    for (const auto& tx : txs_removed_for_block) {
+        if (processBlockTx(nBlockHeight, tx))
             countedTxs++;
     }
 
@@ -688,7 +688,7 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
 
 
     LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy estimates updated by %u of %u block txs, since last block %u of %u tracked, mempool map size %u, max target %u from %s\n",
-             countedTxs, entries.size(), trackedTxs, trackedTxs + untrackedTxs, mapMemPoolTxs.size(),
+             countedTxs, txs_removed_for_block.size(), trackedTxs, trackedTxs + untrackedTxs, mapMemPoolTxs.size(),
              MaxUsableEstimate(), HistoricalBlockSpan() > BlockSpan() ? "historical" : "current");
 
     trackedTxs = 0;
diff --git a/src/policy/fees.h b/src/policy/fees.h
index 69bda195be4..cff0c4dbe2f 100644
--- a/src/policy/fees.h
+++ b/src/policy/fees.h
@@ -37,6 +37,7 @@ static constexpr bool DEFAULT_ACCEPT_STALE_FEE_ESTIMATES{false};
 class AutoFile;
 class CTxMemPoolEntry;
 class TxConfirmStats;
+struct RemovedMempoolTransactionInfo;
 
 /* Identifier for each of the 3 different TxConfirmStats which will track
  * history over different time horizons. */
@@ -201,8 +202,8 @@ class CBlockPolicyEstimator
     ~CBlockPolicyEstimator();
 
     /** Process all the transactions that have been included in a block */
-    void processBlock(unsigned int nBlockHeight,
-                      std::vector<const CTxMemPoolEntry*>& entries)
+    void processBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block,
+                      unsigned int nBlockHeight)
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** Process a transaction accepted to the mempool*/
@@ -290,7 +291,7 @@ class CBlockPolicyEstimator
     std::map<double, unsigned int> bucketMap GUARDED_BY(m_cs_fee_estimator); // Map of bucket upper-bound to index into all vectors by bucket
 
     /** Process a transaction confirmed in a block*/
-    bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry) EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator);
+    bool processBlockTx(unsigned int nBlockHeight, const RemovedMempoolTransactionInfo& tx) EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator);
 
     /** Helper for estimateSmartFee */
     double estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon, EstimationResult *result) const EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator);
diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp
index 220799be41f..e5e0b0d8dac 100644
--- a/src/test/fuzz/policy_estimator.cpp
+++ b/src/test/fuzz/policy_estimator.cpp
@@ -61,12 +61,12 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
                     const CTransaction tx{*mtx};
                     mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
                 }
-                std::vector<const CTxMemPoolEntry*> ptrs;
-                ptrs.reserve(mempool_entries.size());
+                std::vector<RemovedMempoolTransactionInfo> txs;
+                txs.reserve(mempool_entries.size());
                 for (const CTxMemPoolEntry& mempool_entry : mempool_entries) {
-                    ptrs.push_back(&mempool_entry);
+                    txs.emplace_back(mempool_entry);
                 }
-                block_policy_estimator.processBlock(fuzzed_data_provider.ConsumeIntegral<unsigned int>(), ptrs);
+                block_policy_estimator.processBlock(txs, fuzzed_data_provider.ConsumeIntegral<unsigned int>());
             },
             [&] {
                 (void)block_policy_estimator.removeTx(ConsumeUInt256(fuzzed_data_provider), /*inBlock=*/fuzzed_data_provider.ConsumeBool());
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index f75dd7efeaa..f5036a93012 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -643,17 +643,6 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
 void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight)
 {
     AssertLockHeld(cs);
-    std::vector<const CTxMemPoolEntry*> entries;
-    for (const auto& tx : vtx)
-    {
-        uint256 hash = tx->GetHash();
-
-        indexed_transaction_set::iterator i = mapTx.find(hash);
-        if (i != mapTx.end())
-            entries.push_back(&*i);
-    }
-    // Before the txs in the new block have been removed from the mempool, update policy estimates
-    if (minerPolicyEstimator) {minerPolicyEstimator->processBlock(nBlockHeight, entries);}
     std::vector<RemovedMempoolTransactionInfo> txs_removed_for_block;
     txs_removed_for_block.reserve(vtx.size());
     for (const auto& tx : vtx)
@@ -668,6 +657,10 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         removeConflicts(*tx);
         ClearPrioritisation(tx->GetHash());
     }
+    // Update policy estimates
+    if (minerPolicyEstimator) {
+        minerPolicyEstimator->processBlock(txs_removed_for_block, nBlockHeight);
+    }
     GetMainSignals().MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight);
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;

From dff5ad3b9944cbb56126ba37a8da180d1327ba39 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Fri, 3 Nov 2023 17:04:30 +0100
Subject: [PATCH 5/7] CValidationInterface: modify the parameter of
 `TransactionAddedToMempool`

Create a new struct `NewMempoolTransactionInfo` that will be used as the new parameter of
`TransactionAddedToMempool` callback.
---
 src/kernel/mempool_entry.h           | 29 ++++++++++++++++++++++++++++
 src/node/interfaces.cpp              |  4 ++--
 src/test/fuzz/package_eval.cpp       | 10 +++++-----
 src/test/fuzz/tx_pool.cpp            |  4 ++--
 src/validation.cpp                   | 16 +++++++++++++--
 src/validationinterface.cpp          |  7 ++++---
 src/validationinterface.h            |  5 +++--
 src/zmq/zmqnotificationinterface.cpp |  5 +++--
 src/zmq/zmqnotificationinterface.h   |  3 ++-
 9 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index ce32fe20dcc..5824a4576bc 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -208,4 +208,33 @@ struct RemovedMempoolTransactionInfo {
         : info{entry.GetSharedTx(), entry.GetFee(), entry.GetTxSize(), entry.GetHeight()} {}
 };
 
+struct NewMempoolTransactionInfo {
+    TransactionInfo info;
+    /*
+     * This boolean indicates whether the transaction was added
+     * without enforcing mempool fee limits.
+     */
+    const bool m_from_disconnected_block;
+    /* This boolean indicates whether the transaction is part of a package. */
+    const bool m_submitted_in_package;
+    /*
+     * This boolean indicates whether the blockchain is up to date when the
+     * transaction is added to the mempool.
+     */
+    const bool m_chainstate_is_current;
+    /* Indicates whether the transaction has unconfirmed parents. */
+    const bool m_has_no_mempool_parents;
+
+    explicit NewMempoolTransactionInfo(const CTransactionRef& tx, const CAmount& fee,
+                                       const int64_t vsize, const unsigned int height,
+                                       const bool from_disconnected_block, const bool submitted_in_package,
+                                       const bool chainstate_is_current,
+                                       const bool has_no_mempool_parents)
+        : info{tx, fee, vsize, height},
+          m_from_disconnected_block{from_disconnected_block},
+          m_submitted_in_package{submitted_in_package},
+          m_chainstate_is_current{chainstate_is_current},
+          m_has_no_mempool_parents{has_no_mempool_parents} {}
+};
+
 #endif // BITCOIN_KERNEL_MEMPOOL_ENTRY_H
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index f4ecfeb9d54..b5691f0e577 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -428,9 +428,9 @@ class NotificationsProxy : public CValidationInterface
     explicit NotificationsProxy(std::shared_ptr<Chain::Notifications> notifications)
         : m_notifications(std::move(notifications)) {}
     virtual ~NotificationsProxy() = default;
-    void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override
+    void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence) override
     {
-        m_notifications->transactionAddedToMempool(tx);
+        m_notifications->transactionAddedToMempool(tx.info.m_tx);
     }
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override
     {
diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp
index 8658c0b45a7..021acc02f22 100644
--- a/src/test/fuzz/package_eval.cpp
+++ b/src/test/fuzz/package_eval.cpp
@@ -55,13 +55,13 @@ struct OutpointsUpdater final : public CValidationInterface {
     explicit OutpointsUpdater(std::set<COutPoint>& r)
         : m_mempool_outpoints{r} {}
 
-    void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t /* mempool_sequence */) override
+    void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /* mempool_sequence */) override
     {
         // for coins spent we always want to be able to rbf so they're not removed
 
         // outputs from this tx can now be spent
-        for (uint32_t index{0}; index < tx->vout.size(); ++index) {
-            m_mempool_outpoints.insert(COutPoint{tx->GetHash(), index});
+        for (uint32_t index{0}; index < tx.info.m_tx->vout.size(); ++index) {
+            m_mempool_outpoints.insert(COutPoint{tx.info.m_tx->GetHash(), index});
         }
     }
 
@@ -85,10 +85,10 @@ struct TransactionsDelta final : public CValidationInterface {
     explicit TransactionsDelta(std::set<CTransactionRef>& a)
         : m_added{a} {}
 
-    void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t /* mempool_sequence */) override
+    void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /* mempool_sequence */) override
     {
         // Transactions may be entered and booted any number of times
-        m_added.insert(tx);
+        m_added.insert(tx.info.m_tx);
     }
 
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t /* mempool_sequence */) override
diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp
index 96095539ece..001b452722f 100644
--- a/src/test/fuzz/tx_pool.cpp
+++ b/src/test/fuzz/tx_pool.cpp
@@ -59,9 +59,9 @@ struct TransactionsDelta final : public CValidationInterface {
     explicit TransactionsDelta(std::set<CTransactionRef>& r, std::set<CTransactionRef>& a)
         : m_removed{r}, m_added{a} {}
 
-    void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t /* mempool_sequence */) override
+    void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /* mempool_sequence */) override
     {
-        Assert(m_added.insert(tx).second);
+        Assert(m_added.insert(tx.info.m_tx).second);
     }
 
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t /* mempool_sequence */) override
diff --git a/src/validation.cpp b/src/validation.cpp
index ed72a7c97a6..1c95ba08c5a 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1222,7 +1222,13 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
         results.emplace(ws.m_ptx->GetWitnessHash(),
                         MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize,
                                          ws.m_base_fees, effective_feerate, effective_feerate_wtxids));
-        GetMainSignals().TransactionAddedToMempool(ws.m_ptx, m_pool.GetAndIncrementSequence());
+        const CTransaction& tx = *ws.m_ptx;
+        const auto tx_info = NewMempoolTransactionInfo(ws.m_ptx, ws.m_base_fees,
+                                                       ws.m_vsize, ws.m_entry->GetHeight(),
+                                                       args.m_bypass_limits, args.m_package_submission,
+                                                       IsCurrentForFeeEstimation(m_active_chainstate),
+                                                       m_pool.HasNoInputsOf(tx));
+        GetMainSignals().TransactionAddedToMempool(tx_info, m_pool.GetAndIncrementSequence());
     }
     return all_submitted;
 }
@@ -1265,7 +1271,13 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
         return MempoolAcceptResult::FeeFailure(ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), {ws.m_ptx->GetWitnessHash()});
     }
 
-    GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence());
+    const CTransaction& tx = *ws.m_ptx;
+    const auto tx_info = NewMempoolTransactionInfo(ws.m_ptx, ws.m_base_fees,
+                                                   ws.m_vsize, ws.m_entry->GetHeight(),
+                                                   args.m_bypass_limits, args.m_package_submission,
+                                                   IsCurrentForFeeEstimation(m_active_chainstate),
+                                                   m_pool.HasNoInputsOf(tx));
+    GetMainSignals().TransactionAddedToMempool(tx_info, m_pool.GetAndIncrementSequence());
 
     return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees,
                                         effective_feerate, single_wtxid);
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index 893ef695829..5e944a7c47d 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -206,13 +206,14 @@ void CMainSignals::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockInd
                           fInitialDownload);
 }
 
-void CMainSignals::TransactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) {
+void CMainSignals::TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence)
+{
     auto event = [tx, mempool_sequence, this] {
         m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(tx, mempool_sequence); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__,
-                          tx->GetHash().ToString(),
-                          tx->GetWitnessHash().ToString());
+                          tx.info.m_tx->GetHash().ToString(),
+                          tx.info.m_tx->GetWitnessHash().ToString());
 }
 
 void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {
diff --git a/src/validationinterface.h b/src/validationinterface.h
index ea49a45aa87..e1d6869fab4 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -22,6 +22,7 @@ class CValidationInterface;
 class CScheduler;
 enum class MemPoolRemovalReason;
 struct RemovedMempoolTransactionInfo;
+struct NewMempoolTransactionInfo;
 
 /** Register subscriber */
 void RegisterValidationInterface(CValidationInterface* callbacks);
@@ -97,7 +98,7 @@ class CValidationInterface {
      *
      * Called on a background thread.
      */
-    virtual void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) {}
+    virtual void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence) {}
 
     /**
      * Notifies listeners of a transaction leaving mempool.
@@ -210,7 +211,7 @@ class CMainSignals {
 
 
     void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload);
-    void TransactionAddedToMempool(const CTransactionRef&, uint64_t mempool_sequence);
+    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 BlockConnected(ChainstateRole, const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp
index 03aae865776..63c27377061 100644
--- a/src/zmq/zmqnotificationinterface.cpp
+++ b/src/zmq/zmqnotificationinterface.cpp
@@ -6,6 +6,7 @@
 
 #include <common/args.h>
 #include <kernel/chain.h>
+#include <kernel/mempool_entry.h>
 #include <logging.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
@@ -152,9 +153,9 @@ void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, co
     });
 }
 
-void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef& ptx, uint64_t mempool_sequence)
+void CZMQNotificationInterface::TransactionAddedToMempool(const NewMempoolTransactionInfo& ptx, uint64_t mempool_sequence)
 {
-    const CTransaction& tx = *ptx;
+    const CTransaction& tx = *(ptx.info.m_tx);
 
     TryForEachAndRemoveFailed(notifiers, [&tx, mempool_sequence](CZMQAbstractNotifier* notifier) {
         return notifier->NotifyTransaction(tx) && notifier->NotifyTransactionAcceptance(tx, mempool_sequence);
diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h
index 4246c53bd37..45d0982bd3e 100644
--- a/src/zmq/zmqnotificationinterface.h
+++ b/src/zmq/zmqnotificationinterface.h
@@ -16,6 +16,7 @@
 class CBlock;
 class CBlockIndex;
 class CZMQAbstractNotifier;
+struct NewMempoolTransactionInfo;
 
 class CZMQNotificationInterface final : public CValidationInterface
 {
@@ -31,7 +32,7 @@ class CZMQNotificationInterface final : public CValidationInterface
     void Shutdown();
 
     // CValidationInterface
-    void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override;
+    void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence) override;
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override;
     void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override;
     void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected) override;

From 714523918ba2b853fc69bee6b04a33ba0c828bf5 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Fri, 3 Nov 2023 18:34:58 +0100
Subject: [PATCH 6/7] tx fees, policy: CBlockPolicyEstimator update from
 `CValidationInterface` notifications

`CBlockPolicyEstimator` will implement `CValidationInterface` and
subscribe to its notification to process transactions added and removed
from the mempool.

Re-delegate calculation of `validForFeeEstimation` from validation to fee estimator.

Also clean up the validForFeeEstimation arg thats no longer needed in `CTxMempool`.

Co-authored-by: Matt Corallo <git@bluematt.me>
---
 src/Makefile.am                    |  1 -
 src/init.cpp                       | 10 ++-
 src/kernel/mempool_options.h       |  4 --
 src/policy/fees.cpp                | 40 ++++++++----
 src/policy/fees.h                  | 22 +++++--
 src/test/fuzz/package_eval.cpp     |  1 -
 src/test/fuzz/policy_estimator.cpp | 13 +++-
 src/test/fuzz/tx_pool.cpp          |  1 -
 src/test/policyestimator_tests.cpp | 97 +++++++++++++++++++++++++++---
 src/test/util/txmempool.cpp        |  1 -
 src/txmempool.cpp                  | 24 ++------
 src/txmempool.h                    |  7 +--
 src/validation.cpp                 | 11 +---
 13 files changed, 158 insertions(+), 74 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 99b2184cf25..b746a931c12 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -954,7 +954,6 @@ libbitcoinkernel_la_SOURCES = \
   node/chainstate.cpp \
   node/utxo_snapshot.cpp \
   policy/feerate.cpp \
-  policy/fees.cpp \
   policy/packages.cpp \
   policy/policy.cpp \
   policy/rbf.cpp \
diff --git a/src/init.cpp b/src/init.cpp
index d6dc62f707f..92aa1876265 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -285,8 +285,12 @@ void Shutdown(NodeContext& node)
         DumpMempool(*node.mempool, MempoolPath(*node.args));
     }
 
-    // Drop transactions we were still watching, and record fee estimations.
-    if (node.fee_estimator) node.fee_estimator->Flush();
+    // Drop transactions we were still watching, record fee estimations and Unregister
+    // fee estimator from validation interface.
+    if (node.fee_estimator) {
+        node.fee_estimator->Flush();
+        UnregisterValidationInterface(node.fee_estimator.get());
+    }
 
     // FlushStateToDisk generates a ChainStateFlushed callback, which we should avoid missing
     if (node.chainman) {
@@ -1258,6 +1262,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         // Flush estimates to disk periodically
         CBlockPolicyEstimator* fee_estimator = node.fee_estimator.get();
         node.scheduler->scheduleEvery([fee_estimator] { fee_estimator->FlushFeeEstimates(); }, FEE_FLUSH_INTERVAL);
+        RegisterValidationInterface(fee_estimator);
     }
 
     // Check port numbers
@@ -1471,7 +1476,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     assert(!node.chainman);
 
     CTxMemPool::Options mempool_opts{
-        .estimator = node.fee_estimator.get(),
         .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0,
     };
     auto result{ApplyArgsManOptions(args, chainparams, mempool_opts)};
diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h
index d09fd2ba352..753aebd4553 100644
--- a/src/kernel/mempool_options.h
+++ b/src/kernel/mempool_options.h
@@ -13,8 +13,6 @@
 #include <cstdint>
 #include <optional>
 
-class CBlockPolicyEstimator;
-
 /** Default for -maxmempool, maximum megabytes of mempool memory usage */
 static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300};
 /** Default for -maxmempool when blocksonly is set */
@@ -37,8 +35,6 @@ namespace kernel {
  * Most of the time, this struct should be referenced as CTxMemPool::Options.
  */
 struct MemPoolOptions {
-    /* Used to estimate appropriate transaction fees. */
-    CBlockPolicyEstimator* estimator{nullptr};
     /* The ratio used to determine how often sanity checks will run.  */
     int check_ratio{0};
     int64_t max_size_bytes{DEFAULT_MAX_MEMPOOL_SIZE_MB * 1'000'000};
diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index 6d550b1d5ac..74c688060d2 100644
--- a/src/policy/fees.cpp
+++ b/src/policy/fees.cpp
@@ -515,15 +515,10 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe
     }
 }
 
-// This function is called from CTxMemPool::removeUnchecked to ensure
-// txs removed from the mempool for any reason are no longer
-// tracked. Txs that were part of a block have already been removed in
-// processBlockTx to ensure they are never double tracked, but it is
-// of no harm to try to remove them again.
-bool CBlockPolicyEstimator::removeTx(uint256 hash, bool inBlock)
+bool CBlockPolicyEstimator::removeTx(uint256 hash)
 {
     LOCK(m_cs_fee_estimator);
-    return _removeTx(hash, inBlock);
+    return _removeTx(hash, /*inBlock=*/false);
 }
 
 bool CBlockPolicyEstimator::_removeTx(const uint256& hash, bool inBlock)
@@ -579,11 +574,26 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath
 
 CBlockPolicyEstimator::~CBlockPolicyEstimator() = default;
 
-void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate)
+void CBlockPolicyEstimator::TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/)
+{
+    processTransaction(tx);
+}
+
+void CBlockPolicyEstimator::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/)
+{
+    removeTx(tx->GetHash());
+}
+
+void CBlockPolicyEstimator::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
+{
+    processBlock(txs_removed_for_block, nBlockHeight);
+}
+
+void CBlockPolicyEstimator::processTransaction(const NewMempoolTransactionInfo& tx)
 {
     LOCK(m_cs_fee_estimator);
-    unsigned int txHeight = entry.GetHeight();
-    uint256 hash = entry.GetTx().GetHash();
+    const unsigned int txHeight = tx.info.txHeight;
+    const auto& hash = tx.info.m_tx->GetHash();
     if (mapMemPoolTxs.count(hash)) {
         LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy error mempool tx %s already being tracked\n",
                  hash.ToString());
@@ -597,17 +607,23 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo
         // It will be synced next time a block is processed.
         return;
     }
+    // This transaction should only count for fee estimation if:
+    // - it's not being re-added during a reorg which bypasses typical mempool fee limits
+    // - the node is not behind
+    // - the transaction is not dependent on any other transactions in the mempool
+    // - it's not part of a package.
+    const bool validForFeeEstimation = !tx.m_from_disconnected_block && !tx.m_submitted_in_package && tx.m_chainstate_is_current && tx.m_has_no_mempool_parents;
 
     // Only want to be updating estimates when our blockchain is synced,
     // otherwise we'll miscalculate how many blocks its taking to get included.
-    if (!validFeeEstimate) {
+    if (!validForFeeEstimation) {
         untrackedTxs++;
         return;
     }
     trackedTxs++;
 
     // Feerates are stored and reported as BTC-per-kb:
-    CFeeRate feeRate(entry.GetFee(), entry.GetTxSize());
+    const CFeeRate feeRate(tx.info.m_fee, tx.info.m_virtual_transaction_size);
 
     mapMemPoolTxs[hash].blockHeight = txHeight;
     unsigned int bucketIndex = feeStats->NewTx(txHeight, static_cast<double>(feeRate.GetFeePerK()));
diff --git a/src/policy/fees.h b/src/policy/fees.h
index cff0c4dbe2f..f34f66d3f0d 100644
--- a/src/policy/fees.h
+++ b/src/policy/fees.h
@@ -12,6 +12,7 @@
 #include <threadsafety.h>
 #include <uint256.h>
 #include <util/fs.h>
+#include <validationinterface.h>
 
 #include <array>
 #include <chrono>
@@ -35,9 +36,9 @@ static constexpr std::chrono::hours MAX_FILE_AGE{60};
 static constexpr bool DEFAULT_ACCEPT_STALE_FEE_ESTIMATES{false};
 
 class AutoFile;
-class CTxMemPoolEntry;
 class TxConfirmStats;
 struct RemovedMempoolTransactionInfo;
+struct NewMempoolTransactionInfo;
 
 /* Identifier for each of the 3 different TxConfirmStats which will track
  * history over different time horizons. */
@@ -144,7 +145,7 @@ struct FeeCalculation
  * a certain number of blocks.  Every time a block is added to the best chain, this class records
  * stats on the transactions included in that block
  */
-class CBlockPolicyEstimator
+class CBlockPolicyEstimator : public CValidationInterface
 {
 private:
     /** Track confirm delays up to 12 blocks for short horizon */
@@ -199,7 +200,7 @@ class CBlockPolicyEstimator
 public:
     /** Create new BlockPolicyEstimator and initialize stats tracking classes with default values */
     CBlockPolicyEstimator(const fs::path& estimation_filepath, const bool read_stale_estimates);
-    ~CBlockPolicyEstimator();
+    virtual ~CBlockPolicyEstimator();
 
     /** Process all the transactions that have been included in a block */
     void processBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block,
@@ -207,11 +208,11 @@ class CBlockPolicyEstimator
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** Process a transaction accepted to the mempool*/
-    void processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate)
+    void processTransaction(const NewMempoolTransactionInfo& tx)
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
-    /** Remove a transaction from the mempool tracking stats*/
-    bool removeTx(uint256 hash, bool inBlock)
+    /** Remove a transaction from the mempool tracking stats for non BLOCK removal reasons*/
+    bool removeTx(uint256 hash)
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** DEPRECATED. Return a feerate estimate */
@@ -261,6 +262,15 @@ class CBlockPolicyEstimator
     /** Calculates the age of the file, since last modified */
     std::chrono::hours GetFeeEstimatorFileAge();
 
+protected:
+    /** Overridden from CValidationInterface. */
+    void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/) override
+        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
+        EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
+
 private:
     mutable Mutex m_cs_fee_estimator;
 
diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp
index 021acc02f22..08b8be26e7a 100644
--- a/src/test/fuzz/package_eval.cpp
+++ b/src/test/fuzz/package_eval.cpp
@@ -121,7 +121,6 @@ CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeConte
     mempool_opts.expiry = std::chrono::hours{fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 999)};
     nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(1, 999);
 
-    mempool_opts.estimator = nullptr;
     mempool_opts.check_ratio = 1;
     mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool();
 
diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp
index e5e0b0d8dac..4a41707edff 100644
--- a/src/test/fuzz/policy_estimator.cpp
+++ b/src/test/fuzz/policy_estimator.cpp
@@ -44,9 +44,16 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
                     return;
                 }
                 const CTransaction tx{*mtx};
-                block_policy_estimator.processTransaction(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx), fuzzed_data_provider.ConsumeBool());
+                const CTxMemPoolEntry& entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx);
+                const auto tx_info = NewMempoolTransactionInfo(entry.GetSharedTx(), entry.GetFee(),
+                                                               entry.GetTxSize(), entry.GetHeight(),
+                                                               /* m_from_disconnected_block */ false,
+                                                               /* m_submitted_in_package */ false,
+                                                               /* m_chainstate_is_current */ true,
+                                                               /* m_has_no_mempool_parents */ fuzzed_data_provider.ConsumeBool());
+                block_policy_estimator.processTransaction(tx_info);
                 if (fuzzed_data_provider.ConsumeBool()) {
-                    (void)block_policy_estimator.removeTx(tx.GetHash(), /*inBlock=*/fuzzed_data_provider.ConsumeBool());
+                    (void)block_policy_estimator.removeTx(tx.GetHash());
                 }
             },
             [&] {
@@ -69,7 +76,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
                 block_policy_estimator.processBlock(txs, fuzzed_data_provider.ConsumeIntegral<unsigned int>());
             },
             [&] {
-                (void)block_policy_estimator.removeTx(ConsumeUInt256(fuzzed_data_provider), /*inBlock=*/fuzzed_data_provider.ConsumeBool());
+                (void)block_policy_estimator.removeTx(ConsumeUInt256(fuzzed_data_provider));
             },
             [&] {
                 block_policy_estimator.FlushUnconfirmed();
diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp
index 001b452722f..cc816f213b3 100644
--- a/src/test/fuzz/tx_pool.cpp
+++ b/src/test/fuzz/tx_pool.cpp
@@ -123,7 +123,6 @@ CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeConte
     CTxMemPool::Options mempool_opts{MemPoolOptionsForTest(node)};
 
     // ...override specific options for this specific fuzz suite
-    mempool_opts.estimator = nullptr;
     mempool_opts.check_ratio = 1;
     mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool();
 
diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
index bc9c672560f..13ec89663ae 100644
--- a/src/test/policyestimator_tests.cpp
+++ b/src/test/policyestimator_tests.cpp
@@ -8,6 +8,7 @@
 #include <txmempool.h>
 #include <uint256.h>
 #include <util/time.h>
+#include <validationinterface.h>
 
 #include <test/util/setup_common.h>
 
@@ -19,7 +20,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
 {
     CBlockPolicyEstimator& feeEst = *Assert(m_node.fee_estimator);
     CTxMemPool& mpool = *Assert(m_node.mempool);
-    LOCK2(cs_main, mpool.cs);
+    RegisterValidationInterface(&feeEst);
     TestMemPoolEntryHelper entry;
     CAmount basefee(2000);
     CAmount deltaFee(100);
@@ -59,8 +60,23 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
         for (int j = 0; j < 10; j++) { // For each fee
             for (int k = 0; k < 4; k++) { // add 4 fee txs
                 tx.vin[0].prevout.n = 10000*blocknum+100*j+k; // make transaction unique
+                {
+                    LOCK2(cs_main, mpool.cs);
+                    mpool.addUnchecked(entry.Fee(feeV[j]).Time(Now<NodeSeconds>()).Height(blocknum).FromTx(tx));
+                    // Since TransactionAddedToMempool callbacks are generated in ATMP,
+                    // not addUnchecked, we cheat and create one manually here
+                    const int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx));
+                    const NewMempoolTransactionInfo tx_info{NewMempoolTransactionInfo(MakeTransactionRef(tx),
+                                                                                      feeV[j],
+                                                                                      virtual_size,
+                                                                                      entry.nHeight,
+                                                                                      /* m_from_disconnected_block */ false,
+                                                                                      /* m_submitted_in_package */ false,
+                                                                                      /* m_chainstate_is_current */ true,
+                                                                                      /* m_has_no_mempool_parents */ true)};
+                    GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
+                }
                 uint256 hash = tx.GetHash();
-                mpool.addUnchecked(entry.Fee(feeV[j]).Time(Now<NodeSeconds>()).Height(blocknum).FromTx(tx));
                 txHashes[j].push_back(hash);
             }
         }
@@ -76,10 +92,17 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                 txHashes[9-h].pop_back();
             }
         }
-        mpool.removeForBlock(block, ++blocknum);
+
+        {
+            LOCK(mpool.cs);
+            mpool.removeForBlock(block, ++blocknum);
+        }
+
         block.clear();
         // Check after just a few txs that combining buckets works as expected
         if (blocknum == 3) {
+            // Wait for fee estimator to catch up
+            SyncWithValidationInterfaceQueue();
             // At this point we should need to combine 3 buckets to get enough data points
             // So estimateFee(1) should fail and estimateFee(2) should return somewhere around
             // 9*baserate.  estimateFee(2) %'s are 100,100,90 = average 97%
@@ -114,8 +137,13 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
 
     // Mine 50 more blocks with no transactions happening, estimates shouldn't change
     // We haven't decayed the moving average enough so we still have enough data points in every bucket
-    while (blocknum < 250)
+    while (blocknum < 250) {
+        LOCK(mpool.cs);
         mpool.removeForBlock(block, ++blocknum);
+    }
+
+    // Wait for fee estimator to catch up
+    SyncWithValidationInterfaceQueue();
 
     BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0));
     for (int i = 2; i < 10;i++) {
@@ -130,14 +158,35 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
         for (int j = 0; j < 10; j++) { // For each fee multiple
             for (int k = 0; k < 4; k++) { // add 4 fee txs
                 tx.vin[0].prevout.n = 10000*blocknum+100*j+k;
+                {
+                    LOCK2(cs_main, mpool.cs);
+                    mpool.addUnchecked(entry.Fee(feeV[j]).Time(Now<NodeSeconds>()).Height(blocknum).FromTx(tx));
+                    // Since TransactionAddedToMempool callbacks are generated in ATMP,
+                    // not addUnchecked, we cheat and create one manually here
+                    const int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx));
+                    const NewMempoolTransactionInfo tx_info{NewMempoolTransactionInfo(MakeTransactionRef(tx),
+                                                                                      feeV[j],
+                                                                                      virtual_size,
+                                                                                      entry.nHeight,
+                                                                                      /* m_from_disconnected_block */ false,
+                                                                                      /* m_submitted_in_package */ false,
+                                                                                      /* m_chainstate_is_current */ true,
+                                                                                      /* m_has_no_mempool_parents */ true)};
+                    GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
+                }
                 uint256 hash = tx.GetHash();
-                mpool.addUnchecked(entry.Fee(feeV[j]).Time(Now<NodeSeconds>()).Height(blocknum).FromTx(tx));
                 txHashes[j].push_back(hash);
             }
         }
-        mpool.removeForBlock(block, ++blocknum);
+        {
+            LOCK(mpool.cs);
+            mpool.removeForBlock(block, ++blocknum);
+        }
     }
 
+    // Wait for fee estimator to catch up
+    SyncWithValidationInterfaceQueue();
+
     for (int i = 1; i < 10;i++) {
         BOOST_CHECK(feeEst.estimateFee(i) == CFeeRate(0) || feeEst.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee);
     }
@@ -152,8 +201,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
             txHashes[j].pop_back();
         }
     }
-    mpool.removeForBlock(block, 266);
+
+    {
+        LOCK(mpool.cs);
+        mpool.removeForBlock(block, 266);
+    }
     block.clear();
+
+    // Wait for fee estimator to catch up
+    SyncWithValidationInterfaceQueue();
+
     BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0));
     for (int i = 2; i < 10;i++) {
         BOOST_CHECK(feeEst.estimateFee(i) == CFeeRate(0) || feeEst.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee);
@@ -165,17 +222,39 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
         for (int j = 0; j < 10; j++) { // For each fee multiple
             for (int k = 0; k < 4; k++) { // add 4 fee txs
                 tx.vin[0].prevout.n = 10000*blocknum+100*j+k;
+                {
+                    LOCK2(cs_main, mpool.cs);
+                    mpool.addUnchecked(entry.Fee(feeV[j]).Time(Now<NodeSeconds>()).Height(blocknum).FromTx(tx));
+                    // Since TransactionAddedToMempool callbacks are generated in ATMP,
+                    // not addUnchecked, we cheat and create one manually here
+                    const int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx));
+                    const NewMempoolTransactionInfo tx_info{NewMempoolTransactionInfo(MakeTransactionRef(tx),
+                                                                                      feeV[j],
+                                                                                      virtual_size,
+                                                                                      entry.nHeight,
+                                                                                      /* m_from_disconnected_block */ false,
+                                                                                      /* m_submitted_in_package */ false,
+                                                                                      /* m_chainstate_is_current */ true,
+                                                                                      /* m_has_no_mempool_parents */ true)};
+                    GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
+                }
                 uint256 hash = tx.GetHash();
-                mpool.addUnchecked(entry.Fee(feeV[j]).Time(Now<NodeSeconds>()).Height(blocknum).FromTx(tx));
                 CTransactionRef ptx = mpool.get(hash);
                 if (ptx)
                     block.push_back(ptx);
 
             }
         }
-        mpool.removeForBlock(block, ++blocknum);
+
+        {
+            LOCK(mpool.cs);
+            mpool.removeForBlock(block, ++blocknum);
+        }
+
         block.clear();
     }
+    // Wait for fee estimator to catch up
+    SyncWithValidationInterfaceQueue();
     BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0));
     for (int i = 2; i < 9; i++) { // At 9, the original estimate was already at the bottom (b/c scale = 2)
         BOOST_CHECK(feeEst.estimateFee(i).GetFeePerK() < origFeeEst[i-1] - deltaFee);
diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp
index 6d3bb01be86..379c3c9329a 100644
--- a/src/test/util/txmempool.cpp
+++ b/src/test/util/txmempool.cpp
@@ -18,7 +18,6 @@ using node::NodeContext;
 CTxMemPool::Options MemPoolOptionsForTest(const NodeContext& node)
 {
     CTxMemPool::Options mempool_opts{
-        .estimator = node.fee_estimator.get(),
         // Default to always checking mempool regardless of
         // chainparams.DefaultConsistencyChecks for tests
         .check_ratio = 1,
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index f5036a93012..8a2af807dfa 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -12,9 +12,9 @@
 #include <consensus/tx_verify.h>
 #include <consensus/validation.h>
 #include <logging.h>
-#include <policy/fees.h>
 #include <policy/policy.h>
 #include <policy/settings.h>
+#include <random.h>
 #include <reverse_iterator.h>
 #include <util/check.h>
 #include <util/moneystr.h>
@@ -402,7 +402,6 @@ void CTxMemPoolEntry::UpdateAncestorState(int32_t modifySize, CAmount modifyFee,
 
 CTxMemPool::CTxMemPool(const Options& opts)
     : m_check_ratio{opts.check_ratio},
-      minerPolicyEstimator{opts.estimator},
       m_max_size_bytes{opts.max_size_bytes},
       m_expiry{opts.expiry},
       m_incremental_relay_feerate{opts.incremental_relay_feerate},
@@ -433,7 +432,7 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n)
     nTransactionsUpdated += n;
 }
 
-void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate)
+void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAncestors)
 {
     // Add to memory pool without checking anything.
     // Used by AcceptToMemoryPool(), which DOES do
@@ -477,9 +476,6 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
     nTransactionsUpdated++;
     totalTxSize += entry.GetTxSize();
     m_total_fee += entry.GetFee();
-    if (minerPolicyEstimator) {
-        minerPolicyEstimator->processTransaction(entry, validFeeEstimate);
-    }
 
     txns_randomized.emplace_back(newit->GetSharedTx());
     newit->idx_randomized = txns_randomized.size() - 1;
@@ -497,16 +493,12 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
     // even if not directly reported below.
     uint64_t mempool_sequence = GetAndIncrementSequence();
 
-    const auto& hash = it->GetTx().GetHash();
     if (reason != MemPoolRemovalReason::BLOCK) {
         // Notify clients that a transaction has been removed from the mempool
         // for any reason except being included in a block. Clients interested
         // in transactions included in blocks can subscribe to the BlockConnected
         // notification.
         GetMainSignals().TransactionRemovedFromMempool(it->GetSharedTx(), reason, mempool_sequence);
-        if (minerPolicyEstimator) {
-            minerPolicyEstimator->removeTx(hash, false);
-        }
     }
     TRACE5(mempool, removed,
         it->GetTx().GetHash().data(),
@@ -519,7 +511,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
     for (const CTxIn& txin : it->GetTx().vin)
         mapNextTx.erase(txin.prevout);
 
-    RemoveUnbroadcastTx(hash, true /* add logging because unchecked */ );
+    RemoveUnbroadcastTx(it->GetTx().GetHash(), true /* add logging because unchecked */);
 
     if (txns_randomized.size() > 1) {
         // Update idx_randomized of the to-be-moved entry.
@@ -638,7 +630,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
 }
 
 /**
- * Called when a block is connected. Removes from mempool and updates the miner fee estimator.
+ * Called when a block is connected. Removes from mempool.
  */
 void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight)
 {
@@ -657,10 +649,6 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         removeConflicts(*tx);
         ClearPrioritisation(tx->GetHash());
     }
-    // Update policy estimates
-    if (minerPolicyEstimator) {
-        minerPolicyEstimator->processBlock(txs_removed_for_block, nBlockHeight);
-    }
     GetMainSignals().MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight);
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;
@@ -1091,10 +1079,10 @@ int CTxMemPool::Expire(std::chrono::seconds time)
     return stage.size();
 }
 
-void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, bool validFeeEstimate)
+void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry)
 {
     auto ancestors{AssumeCalculateMemPoolAncestors(__func__, entry, Limits::NoLimits())};
-    return addUnchecked(entry, ancestors, validFeeEstimate);
+    return addUnchecked(entry, ancestors);
 }
 
 void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add)
diff --git a/src/txmempool.h b/src/txmempool.h
index aed6acd9daa..bac7a445d6f 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -202,8 +202,6 @@ struct entry_time {};
 struct ancestor_score {};
 struct index_by_wtxid {};
 
-class CBlockPolicyEstimator;
-
 /**
  * Information about a mempool transaction.
  */
@@ -303,7 +301,6 @@ class CTxMemPool
 protected:
     const int m_check_ratio; //!< Value n means that 1 times in n we check.
     std::atomic<unsigned int> nTransactionsUpdated{0}; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation
-    CBlockPolicyEstimator* const minerPolicyEstimator;
 
     uint64_t totalTxSize GUARDED_BY(cs){0};      //!< sum of all mempool tx's virtual sizes. Differs from serialized tx size since witness data is discounted. Defined in BIP 141.
     CAmount m_total_fee GUARDED_BY(cs){0};       //!< sum of all mempool tx's fees (NOT modified fee)
@@ -472,8 +469,8 @@ class CTxMemPool
     // Note that addUnchecked is ONLY called from ATMP outside of tests
     // and any other callers may break wallet's in-mempool tracking (due to
     // lack of CValidationInterface::TransactionAddedToMempool callbacks).
-    void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
-    void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
+    void addUnchecked(const CTxMemPoolEntry& entry) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
+    void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
 
     void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
     /** After reorg, filter the entries that would no longer be valid in the next block, and update
diff --git a/src/validation.cpp b/src/validation.cpp
index 1c95ba08c5a..f9e5d1db828 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1126,17 +1126,8 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws)
         ws.m_replaced_transactions.push_back(it->GetSharedTx());
     }
     m_pool.RemoveStaged(ws.m_all_conflicting, false, MemPoolRemovalReason::REPLACED);
-
-    // This transaction should only count for fee estimation if:
-    // - it's not being re-added during a reorg which bypasses typical mempool fee limits
-    // - the node is not behind
-    // - the transaction is not dependent on any other transactions in the mempool
-    // - it's not part of a package. Since package relay is not currently supported, this
-    // transaction has not necessarily been accepted to miners' mempools.
-    bool validForFeeEstimation = !bypass_limits && !args.m_package_submission && IsCurrentForFeeEstimation(m_active_chainstate) && m_pool.HasNoInputsOf(tx);
-
     // Store transaction in memory
-    m_pool.addUnchecked(*entry, ws.m_ancestors, validForFeeEstimation);
+    m_pool.addUnchecked(*entry, ws.m_ancestors);
 
     // trim mempool and check if tx was trimmed
     // If we are validating a package, don't trim here because we could evict a previous transaction

From 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Wed, 8 Nov 2023 11:21:49 +0100
Subject: [PATCH 7/7] rpc: `SyncWithValidationInterfaceQueue` on fee estimation
 RPC's

This ensures that the most recent fee estimation data is used for the
fee estimation with `estimateSmartfee` and `estimaterawfee` RPC's.
---
 src/rpc/fees.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp
index 62396d4c589..57ba486ed90 100644
--- a/src/rpc/fees.cpp
+++ b/src/rpc/fees.cpp
@@ -14,6 +14,7 @@
 #include <txmempool.h>
 #include <univalue.h>
 #include <util/fees.h>
+#include <validationinterface.h>
 
 #include <algorithm>
 #include <array>
@@ -67,6 +68,7 @@ static RPCHelpMan estimatesmartfee()
             const NodeContext& node = EnsureAnyNodeContext(request.context);
             const CTxMemPool& mempool = EnsureMemPool(node);
 
+            SyncWithValidationInterfaceQueue();
             unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
             unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
             bool conservative = true;
@@ -155,6 +157,7 @@ static RPCHelpMan estimaterawfee()
         {
             CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context);
 
+            SyncWithValidationInterfaceQueue();
             unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
             unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
             double threshold = 0.95;