tx fees, policy: add second sanity check

Ignore transaction that should be included and were not
repeatedly for some time from our fee estimate calculation.

It's most likely that the transaction were not considered by miners.

src/node/miner.cpp

@@ -94,8 +94,6 @@ BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool
 
 void BlockAssembler::resetBlock()
 {
-    inBlock.clear();
-
     // Reserve space for coinbase tx
     nBlockWeight = 4000;
     nBlockSigOpsCost = 400;
@@ -105,11 +103,17 @@ void BlockAssembler::resetBlock()
     nFees = 0;
 }
 
+void BlockAssembler::resetAndClearInBlock()
+{
+    inBlock.clear();
+    resetBlock();
+}
+
 std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn)
 {
     const auto time_start{SteadyClock::now()};
 
-    resetBlock();
+    if (!excludeTxs) resetAndClearInBlock();
 
     pblocktemplate.reset(new CBlockTemplate());
 
@@ -436,16 +440,26 @@ std::vector<std::tuple<CFeeRate, uint64_t>> BlockAssembler::GetFeeRateStats()
 {
     return std::move(size_per_feerate);
 }
+void BlockAssembler::ExcludeTransactions(const std::set<Txid>& txIds)
+{
+    resetBlock();
+    for (const auto id : txIds) {
+        inBlock.insert(id);
+    }
+    excludeTxs = true;
+}
 
-std::vector<std::tuple<CFeeRate, uint64_t>> GetCustomBlockFeeRateHistogram(Chainstate& chainstate, const CTxMemPool* mempool, size_t block_weight)
+std::vector<std::tuple<CFeeRate, uint64_t>> GetCustomBlockFeeRateHistogram(Chainstate& chainstate, const CTxMemPool* mempool, const std::set<Txid>& txsToExclude, size_t block_weight)
 {
     BlockAssembler::Options options = {
         .nBlockMaxWeight = block_weight,
         .test_block_validity = false,
         .sanity_check_block_weight = false,
     };
     BlockAssembler assembler(chainstate, mempool, options);
+    assembler.ExcludeTransactions(txsToExclude);
     assembler.CreateNewBlock(CScript{});
+    assembler.DoNotExclude();
     return assembler.GetFeeRateStats();
 }
 } // namespace node

src/node/miner.h

@@ -153,6 +153,8 @@ class BlockAssembler
     const CTxMemPool* const m_mempool;
     Chainstate& m_chainstate;
 
+    bool excludeTxs{false};
+
 public:
     struct Options {
         // Configuration parameters for the block size
@@ -177,12 +179,21 @@ class BlockAssembler
      * only be called once. */
     std::vector<std::tuple<CFeeRate, uint64_t>> GetFeeRateStats();
 
+    void ExcludeTransactions(const std::set<Txid>& txIds);
+
+    void DoNotExclude()
+    {
+        excludeTxs = false;
+    }
+
 private:
     const Options m_options;
 
     // utility functions
     /** Clear the block's state and prepare for assembling a new block */
     void resetBlock();
+
+    void resetAndClearInBlock();
     /** Add a tx to the block */
     void AddToBlock(CTxMemPool::txiter iter);
 
@@ -212,7 +223,7 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam
 void RegenerateCommitments(CBlock& block, ChainstateManager& chainman);
 
 /** Get feerate statistics of a block weight from the mempool. */
-std::vector<std::tuple<CFeeRate, uint64_t>> GetCustomBlockFeeRateHistogram(Chainstate& chainstate, const CTxMemPool* mempool, size_t block_weight);
+std::vector<std::tuple<CFeeRate, uint64_t>> GetCustomBlockFeeRateHistogram(Chainstate& chainstate, const CTxMemPool* mempool, const std::set<Txid>& txsToExclude, size_t block_weight);
 
 /** Apply -blockmintxfee and -blockmaxweight options from ArgsManager to BlockAssembler options. */
 void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options);

src/policy/mempool_fees.cpp

@@ -49,7 +49,8 @@ CFeeRate MemPoolPolicyEstimator::EstimateFeeWithMemPool(Chainstate& chainstate,
         // fast enough to run that far while we're locked and in here
         {
             LOCK2(cs_main, mempool.cs);
-            mempool_fee_stats = GetCustomBlockFeeRateHistogram(chainstate, &mempool, MAX_BLOCK_WEIGHT * MAX_CONF_TARGET);
+            const auto txsToExclude = GetTxsToExclude();
+            mempool_fee_stats = GetCustomBlockFeeRateHistogram(chainstate, &mempool, txsToExclude, MAX_BLOCK_WEIGHT * MAX_CONF_TARGET);
         }
         if (mempool_fee_stats.empty()) {
             err_message = "No transactions available in the mempool yet.";
@@ -137,15 +138,20 @@ void MemPoolPolicyEstimator::ExpectedBlockConnected(const CBlock& expected_block
     }
 
     uint32_t removed_txs_weight = 0;
+    std::set<Txid> unremoved_txs;
     for (const auto& tx : expected_block_template.vtx) {
         if (block_transactions.contains(tx->GetHash())) {
             removed_txs_weight += GetTransactionWeight(*tx);
+        } else {
+            unremoved_txs.insert(tx->GetHash());
         }
     }
 
     bool roughly_synced = removed_txs_weight > (block_weight / 2);
     const MemPoolPolicyEstimator::block_info new_block_info = {nBlockHeight, roughly_synced};
     UpdateTopBlocks(new_block_info);
+
+    // if (roughly_synced && !unremoved_txs.empty()) IncrementTxsCount();
 }
 
 void MemPoolPolicyEstimator::UpdateTopBlocks(const MemPoolPolicyEstimator::block_info& new_blk_info)
@@ -203,3 +209,21 @@ bool MemPoolPolicyEstimator::RoughlySynced() const
         return blk.roughly_synced;
     });
 }
+
+void MemPoolPolicyEstimator::IncrementTxsCount(const std::set<Txid>& txs)
+{
+    for (const auto& txId : txs) {
+        expectedMinedTxs[txId] += 1;
+    }
+}
+
+std::set<Txid> MemPoolPolicyEstimator::GetTxsToExclude() const
+{
+    std::set<Txid> txs;
+    for (const auto tx : expectedMinedTxs) {
+        if (tx.second >= MAX_UNCONF_COUNT) {
+            txs.insert(tx.first);
+        }
+    }
+    return txs;
+}

src/policy/mempool_fees.h

@@ -11,6 +11,7 @@
 #include <optional>
 #include <shared_mutex>
 #include <string>
+#include <util/hasher.h>
 #include <validationinterface.h>
 
 #include <logging.h>
@@ -25,6 +26,8 @@ class CTxMemPool;
 // mempool condition might likely change.
 static const unsigned int MAX_CONF_TARGET{3};
 
+static const unsigned int MAX_UNCONF_COUNT{5};
+
 static constexpr std::chrono::minutes FEE_ESTIMATE_INTERVAL{1};
 
 /**
@@ -130,6 +133,8 @@ class MemPoolPolicyEstimator : public CValidationInterface
 
     std::array<block_info, 3> top_blocks;
 
+    std::map<Txid, unsigned int> expectedMinedTxs;
+
     // 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);
 
@@ -140,6 +145,10 @@ class MemPoolPolicyEstimator : public CValidationInterface
 
     void InsertNewBlock(const block_info& new_blk_info);
 
+    void IncrementTxsCount(const std::set<Txid>& txs);
+
+    std::set<Txid> GetTxsToExclude() const;
+
 protected:
     void ExpectedBlockConnected(const CBlock& expected_block_template, const CBlock& block, unsigned int nBlockHeight) override;
 };

src/test/miner_tests.cpp

@@ -286,7 +286,8 @@ void MinerTestingSetup::TestCustomBlockCreation(const CScript& scriptPubKey, con
     BOOST_CHECK(stats_size == 0);
     BOOST_CHECK(pblocktemplate->block.vtx[0]->GetHash() != txRef->GetHash());
 
-    const auto fee_rate_stats = node::GetCustomBlockFeeRateHistogram(m_node.chainman->ActiveChainstate(), &tx_mempool, MAX_BLOCK_WEIGHT * 2);
+    std::set<Txid> emptySet;
+    const auto fee_rate_stats = node::GetCustomBlockFeeRateHistogram(m_node.chainman->ActiveChainstate(), &tx_mempool, emptySet, MAX_BLOCK_WEIGHT * 2);
     BOOST_CHECK_EQUAL(fee_rate_stats.size(), 1);
 }