tx policy, fees: add sanity check and some fixups

The sanity checks determine whether the mempool is
roughly in sync with miners mempool before making
a fee rate estimate.

src/init.cpp

@@ -321,6 +321,10 @@ void Shutdown(NodeContext& node)
         UnregisterValidationInterface(node.fee_estimator.get());
     }
 
+    if (node.mempool_fee_estimator) {
+        UnregisterValidationInterface(node.mempool_fee_estimator.get());
+    }
+
     // FlushStateToDisk generates a ChainStateFlushed callback, which we should avoid missing
     if (node.chainman) {
         LOCK(cs_main);
@@ -376,9 +380,9 @@ void Shutdown(NodeContext& node)
     node.chain_clients.clear();
     UnregisterAllValidationInterfaces();
     GetMainSignals().UnregisterBackgroundSignalScheduler();
-    node.kernel.reset();
     node.mempool.reset();
     node.fee_estimator.reset();
+    node.mempool_fee_estimator.reset();
     node.chainman.reset();
     node.scheduler.reset();
     node.kernel.reset();
@@ -1598,6 +1602,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     node.mempool_fee_estimator = std::make_unique<MemPoolPolicyEstimator>();
     MemPoolPolicyEstimator* mempool_fee_estimator = node.mempool_fee_estimator.get();
     CBlockPolicyEstimator* fee_estimator = node.fee_estimator.get();
+    RegisterValidationInterface(node.mempool_fee_estimator.get());
     CTxMemPool& mempool = *(node.mempool.get());
     node.scheduler->scheduleEvery([mempool_fee_estimator, fee_estimator, &mempool, &chainman ] { mempool_fee_estimator->EstimateFeeWithMemPool(chainman, mempool, fee_estimator); }, FEE_ESTIMATE_INTERVAL);
     assert(!node.peerman);

src/policy/mempool_fees.cpp

@@ -4,12 +4,15 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <consensus/consensus.h>
+#include <consensus/validation.h>
 #include <logging.h>
 #include <node/miner.h>
 #include <policy/fees.h>
 #include <policy/mempool_fees.h>
 #include <policy/policy.h>
 #include <validation.h>
+#include <validationinterface.h>
+
 
 using node::GetCustomBlockFeeRateHistogram;
 
@@ -30,6 +33,12 @@ CFeeRate MemPoolPolicyEstimator::EstimateFeeWithMemPool(Chainstate& chainstate,
         err_message = "Mempool not finished loading, can't get accurate fee rate estimate.";
         return CFeeRate(0);
     }
+
+    if (!RoughlySynced()) {
+        err_message = "Mempool transactions roughly not in sync with previously mined blocks, fee rate estimate won't be reliable.";
+        return CFeeRate(0);
+    }
+
     if (!force) {
         cached_fee = cache.get(confTarget);
     }
@@ -117,3 +126,80 @@ CFeeRate MemPoolPolicyEstimator::CalculateMedianFeeRate(
     // the block weight is not enough to provide a decent estimate
     return CFeeRate(0);
 }
+
+void MemPoolPolicyEstimator::ExpectedBlockConnected(const CBlock& expected_block_template, const CBlock& block, unsigned int nBlockHeight)
+{
+    std::set<Txid> block_transactions;
+    uint32_t block_weight = 0;
+    for (const auto& tx : block.vtx) {
+        block_transactions.insert(tx->GetHash());
+        block_weight += GetTransactionWeight(*tx);
+    }
+
+    uint32_t removed_txs_weight = 0;
+    for (const auto& tx : expected_block_template.vtx) {
+        if (block_transactions.contains(tx->GetHash())) {
+            removed_txs_weight += GetTransactionWeight(*tx);
+        }
+    }
+
+    bool roughly_synced = removed_txs_weight > (block_weight / 2);
+    const MemPoolPolicyEstimator::block_info new_block_info = {nBlockHeight, roughly_synced};
+    UpdateTopBlocks(new_block_info);
+}
+
+void MemPoolPolicyEstimator::UpdateTopBlocks(const MemPoolPolicyEstimator::block_info& new_blk_info)
+{
+    if (AreTopBlocksInOrder()) {
+        InsertNewBlock(new_blk_info);
+    } else {
+        top_blocks = {new_blk_info, {0, false}, {0, false}};
+    }
+}
+
+void MemPoolPolicyEstimator::InsertNewBlock(const MemPoolPolicyEstimator::block_info& new_blk_info)
+{
+    auto empty_block_it = std::find_if(top_blocks.begin(), top_blocks.end(), [](const auto& blk) {
+        return blk.height == 0;
+    });
+
+    if (empty_block_it != top_blocks.end()) {
+        if (std::prev(empty_block_it)->height + 1 == new_blk_info.height) {
+            *empty_block_it = new_blk_info;
+        } else {
+            top_blocks = {new_blk_info, {0, false}, {0, false}};
+        }
+    }
+
+    if (!top_blocks.empty() && top_blocks.back().height + 1 == new_blk_info.height) {
+        std::rotate(top_blocks.begin(), top_blocks.begin() + 1, top_blocks.end());
+        top_blocks.back() = new_blk_info;
+    } else {
+        top_blocks = {new_blk_info, {0, false}, {0, false}};
+    }
+}
+
+bool MemPoolPolicyEstimator::AreTopBlocksInOrder() const
+{
+    unsigned int curr_height = top_blocks[0].height;
+    if (curr_height == 0) {
+        return true;
+    }
+    for (size_t i = 1; i < top_blocks.size(); ++i) {
+        if (top_blocks[i].height == 0) {
+            return true;
+        }
+        ++curr_height;
+        if (curr_height != top_blocks[i].height) {
+            return false;
+        }
+    }
+    return true;
+}
+
+bool MemPoolPolicyEstimator::RoughlySynced() const
+{
+    return AreTopBlocksInOrder() && std::all_of(top_blocks.begin(), top_blocks.end(), [](const auto& blk) {
+               return blk.roughly_synced;
+           });
+}

src/policy/mempool_fees.h

@@ -11,6 +11,7 @@
 #include <optional>
 #include <shared_mutex>
 #include <string>
+#include <validationinterface.h>
 
 #include <logging.h>
 #include <policy/feerate.h>
@@ -83,12 +84,12 @@ struct CachedMempoolEstimates {
  * fee rate of the txs in the confirmation target block as the approximate fee rate that a tx will pay to
  * likely be included in the block.
  */
-class MemPoolPolicyEstimator
+class MemPoolPolicyEstimator : public CValidationInterface
 {
 public:
     MemPoolPolicyEstimator();
 
-    ~MemPoolPolicyEstimator() = default;
+    virtual ~MemPoolPolicyEstimator() = default;
 
     /**
      * Estimate the fee rate from mempool txs data given a confirmation target.
@@ -121,5 +122,25 @@ class MemPoolPolicyEstimator
      * @return The median fee rate.
      */
     CFeeRate CalculateMedianFeeRate(std::vector<std::tuple<CFeeRate, uint64_t>>::const_iterator start_it, std::vector<std::tuple<CFeeRate, uint64_t>>::const_iterator end_it) const;
+
+    struct block_info {
+        unsigned int height;
+        bool roughly_synced;
+    };
+
+    std::array<block_info, 3> top_blocks;
+
+    // Whenever we receive a new block we record it's status if its in sync or not.
+    void UpdateTopBlocks(const block_info& new_blk_info);
+
+    // Determine whether the last that we tracked are sequential.
+    bool AreTopBlocksInOrder() const;
+
+    bool RoughlySynced() const; // Tells us whether our mempool is roughly in sync with miners mempool.
+
+    void InsertNewBlock(const block_info& new_blk_info);
+
+protected:
+    void ExpectedBlockConnected(const CBlock& expected_block_template, const CBlock& block, unsigned int nBlockHeight) override;
 };
 #endif // BITCOIN_POLICY_MEMPOOL_FEES_H

src/test/fuzz/rpc.cpp

@@ -107,6 +107,7 @@ const std::vector<std::string> RPC_COMMANDS_SAFE_FOR_FUZZING{
     "echojson",
     "estimaterawfee",
     "estimatesmartfee",
+    "estimatefeewithmempool",
     "finalizepsbt",
     "generate",
     "generateblock",

src/test/mempool_fee_tests.cpp

@@ -6,16 +6,14 @@
 #include <util/strencodings.h>
 #include <validation.h>
 
-
 #include <test/util/setup_common.h>
 
 #include <memory>
 #include <string>
 
 #include <boost/test/unit_test.hpp>
 
-BOOST_FIXTURE_TEST_SUITE(mempoolestimator_tests, TestChain100Setup)
-
+BOOST_FIXTURE_TEST_SUITE(mempool_fee_tests, TestChain100Setup)
 
 BOOST_AUTO_TEST_CASE(MempoolEstimator)
 {
@@ -41,8 +39,7 @@ BOOST_AUTO_TEST_CASE(MempoolEstimator)
     m_node.mempool->SetLoadTried(true);
     fee_estimate = m_node.mempool_fee_estimator->EstimateFeeWithMemPool(m_node.chainman->ActiveChainstate(), *m_node.mempool, 1, false, err_message);
     BOOST_CHECK(fee_estimate == CFeeRate(0));
-    BOOST_CHECK(err_message == std::string("No transactions available in the mempool yet."));
-
+    BOOST_CHECK_EQUAL(err_message, std::string("Mempool transactions roughly not in sync with previously mined blocks, fee rate estimate won't be reliable."));
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/validation.cpp

@@ -28,6 +28,7 @@
 #include <logging.h>
 #include <logging/timer.h>
 #include <node/blockstorage.h>
+#include <node/miner.h>
 #include <node/utxo_snapshot.h>
 #include <policy/v3_policy.h>
 #include <policy/policy.h>
@@ -2458,7 +2459,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
             control.Add(std::move(vChecks));
             int64_t txWeight = GetTransactionWeight(tx);
             int64_t txVsize = GetVirtualTransactionSize(txWeight, txSigOpCost, nBytesPerSigOp);
-            totalWeight += txWeight; 
+            totalWeight += txWeight;
             totalVsize += txVsize;
 
             size_t next = i + 1;
@@ -2471,12 +2472,12 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
             if (medianFeeRate == CFeeRate(0)) {
                 const auto mid_size = DEFAULT_BLOCK_MAX_WEIGHT / 2;
                 if (totalWeight == mid_size || (totalWeight + nextTxweight > mid_size)) {
-                    bool hasParent = std::any_of(tx.vin.begin(), tx.vin.end(), [&txIds](const CTxIn& input) { 
-                        return txIds.contains(input.prevout.hash); 
+                    bool hasParent = std::any_of(tx.vin.begin(), tx.vin.end(), [&txIds](const CTxIn& input) {
+                        return txIds.contains(input.prevout.hash);
                     });
                     if (!hasParent) {
                         medianFeeRate = CFeeRate(txfee, txVsize);
-                } 
+                    }
                 }
             }
         }
@@ -3013,6 +3014,17 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
              Ticks<MillisecondsDouble>(time_chainstate) / num_blocks_total);
     // Remove conflicting transactions from the mempool.;
     if (m_mempool) {
+        {
+            CBlockIndex* pindexPrev = this->m_chain.Tip();
+            if (pindexPrev != nullptr) {
+                const CScript scriptPubKey = CScript() << OP_TRUE;
+                node::BlockAssembler::Options options;
+                options.test_block_validity = false;
+                const auto block_template = node::BlockAssembler(*this, m_mempool, options).CreateNewBlock(scriptPubKey);
+                GetMainSignals().ExpectedBlockConnected(block_template->block, blockConnecting, pindexNew->nHeight);
+            }
+        }
+
         m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
         disconnectpool.removeForBlock(blockConnecting.vtx);
     }

src/validationinterface.cpp

@@ -235,6 +235,16 @@ void CMainSignals::BlockConnected(ChainstateRole role, const std::shared_ptr<con
                           pindex->nHeight);
 }
 
+void CMainSignals::ExpectedBlockConnected(const CBlock& expected_block_template, const CBlock& block, unsigned int nBlockHeight)
+{
+    auto event = [expected_block_template, block, nBlockHeight, this] {
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.ExpectedBlockConnected(expected_block_template, block, nBlockHeight); });
+    };
+    ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__,
+                          block.GetHash().ToString(),
+                          nBlockHeight);
+}
+
 void CMainSignals::MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
 {
     auto event = [txs_removed_for_block, nBlockHeight, this] {

src/validationinterface.h

@@ -141,6 +141,11 @@ class CValidationInterface {
      * Called on a background thread.
      */
     virtual void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) {}
+    /**
+     * Notifies listeners of a new block being connected and the expected block template.
+     * returns the new block and the expected block template.
+     */
+    virtual void ExpectedBlockConnected(const CBlock& expected_block_template, const CBlock& block, unsigned int nBlockHeight) {}
     /**
      * Notifies listeners of a block being connected.
      * Provides a vector of transactions evicted from the mempool as a result.
@@ -215,6 +220,7 @@ class CMainSignals {
     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 ExpectedBlockConnected(const CBlock&, const CBlock&, unsigned int);
     void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);
     void ChainStateFlushed(ChainstateRole, const CBlockLocator &);
     void BlockChecked(const CBlock&, const BlockValidationState&);