Merge 4c84f37ad9 into 85f96b01b7

src/CMakeLists.txt

@@ -259,6 +259,7 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL
   policy/ephemeral_policy.cpp
   policy/fees.cpp
   policy/fees_args.cpp
+  policy/fees_util.cpp
   policy/packages.cpp
   policy/rbf.cpp
   policy/settings.cpp

src/node/mini_miner.cpp

@@ -170,7 +170,8 @@ MiniMiner::MiniMiner(const std::vector<MiniMinerMempoolEntry>& manual_entries,
     Assume(m_to_be_replaced.empty());
     Assume(m_requested_outpoints_by_txid.empty());
     Assume(m_bump_fees.empty());
-    Assume(m_inclusion_order.empty());
+    Assume(m_linearize_result.inclusion_order.empty());
+    Assume(m_linearize_result.size_per_feerate.empty());
     SanityCheck();
 }
 
@@ -264,6 +265,7 @@ void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
             break;
         }
 
+        m_linearize_result.size_per_feerate.emplace_back(CFeeRate{ancestor_package_fee, static_cast<uint32_t>(ancestor_package_size)}, static_cast<uint32_t>(ancestor_package_size));
         // Calculate ancestors on the fly. This lookup should be fairly cheap, and ancestor sets
         // change at every iteration, so this is more efficient than maintaining a cache.
         std::set<MockEntryMap::iterator, IteratorComparator> ancestors;
@@ -286,7 +288,7 @@ void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
         }
         // Track the order in which transactions were selected.
         for (const auto& ancestor : ancestors) {
-            m_inclusion_order.emplace(Txid::FromUint256(ancestor->first), sequence_num);
+            m_linearize_result.inclusion_order.emplace(Txid::FromUint256(ancestor->first), sequence_num);
         }
         DeleteAncestorPackage(ancestors);
         SanityCheck();
@@ -298,16 +300,16 @@ void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
         Assume(m_in_block.empty() || m_total_fees >= target_feerate->GetFee(m_total_vsize));
     }
     Assume(m_in_block.empty() || sequence_num > 0);
-    Assume(m_in_block.size() == m_inclusion_order.size());
+    Assume(m_in_block.size() == m_linearize_result.inclusion_order.size());
     // Do not try to continue building the block template with a different feerate.
     m_ready_to_calculate = false;
 }
 
 
-std::map<Txid, uint32_t> MiniMiner::Linearize()
+LinearizationResult MiniMiner::Linearize()
 {
     BuildMockTemplate(std::nullopt);
-    return m_inclusion_order;
+    return m_linearize_result;
 }
 
 std::map<COutPoint, CAmount> MiniMiner::CalculateBumpFees(const CFeeRate& target_feerate)

src/node/mini_miner.h

@@ -21,6 +21,16 @@ class CTxMemPool;
 
 namespace node {
 
+struct LinearizationResult {
+    // Txid to a number representing the order in which this transaction was included (smaller
+    // number = included earlier).  Transactions included in an ancestor set together have the same
+    // sequence number.
+    std::map<Txid, uint32_t> inclusion_order;
+
+    // A vector of all the packages in the block template their feerate and virtual sizes.
+    std::vector<std::tuple<CFeeRate, uint32_t>> size_per_feerate;
+};
+
 // Container for tracking updates to ancestor feerate as we include ancestors in the "block"
 class MiniMinerMempoolEntry
 {
@@ -90,10 +100,7 @@ class MiniMiner
     // the same tx will have the same bumpfee. Excludes non-mempool transactions.
     std::map<uint256, std::vector<COutPoint>> m_requested_outpoints_by_txid;
 
-    // Txid to a number representing the order in which this transaction was included (smaller
-    // number = included earlier).  Transactions included in an ancestor set together have the same
-    // sequence number.
-    std::map<Txid, uint32_t> m_inclusion_order;
+    LinearizationResult m_linearize_result;
     // What we're trying to calculate. Outpoint to the fee needed to bring the transaction to the target feerate.
     std::map<COutPoint, CAmount> m_bump_fees;
 
@@ -162,10 +169,9 @@ public:
     std::optional<CAmount> CalculateTotalBumpFees(const CFeeRate& target_feerate);
 
     /** Construct a new block template with all of the transactions and calculate the order in which
-     * they are selected. Returns the sequence number (lower = selected earlier) with which each
-     * transaction was selected, indexed by txid, or an empty map if it cannot be calculated.
+     * they are selected and the packages fee rate and sizes.
      */
-    std::map<Txid, uint32_t> Linearize();
+    LinearizationResult Linearize();
 };
 } // namespace node
 

src/policy/fees.cpp

@@ -9,7 +9,9 @@
 #include <consensus/amount.h>
 #include <kernel/mempool_entry.h>
 #include <logging.h>
+#include <node/mini_miner.h>
 #include <policy/feerate.h>
+#include <policy/fees_util.h>
 #include <primitives/transaction.h>
 #include <random.h>
 #include <serialize.h>
@@ -665,6 +667,20 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const Remo
     return true;
 }
 
+void CBlockPolicyEstimator::removeCPFPdParentTxs(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block)
+{
+    const auto mini_miner_input = GetMiniMinerInput(txs_removed_for_block);
+    const auto linearizedTransactions = node::MiniMiner(std::get<0>(mini_miner_input), std::get<1>(mini_miner_input)).Linearize();
+    std::set<Txid> seen_transactions;
+    for (const auto& tx : txs_removed_for_block) {
+        const auto tx_inclusion_order = linearizedTransactions.inclusion_order.find(tx.info.m_tx->GetHash())->second;
+        const auto mining_fee_rate = std::get<0>(linearizedTransactions.size_per_feerate[tx_inclusion_order]);
+        if (mining_fee_rate != CFeeRate(tx.info.m_fee, tx.info.m_virtual_transaction_size)) {
+            // Ignore all transactions whose mining score is not the same with it's fee rate
+            _removeTx(tx.info.m_tx->GetHash(), /*inBlock=*/true);
+        }
+    }
+}
 void CBlockPolicyEstimator::processBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block,
                                          unsigned int nBlockHeight)
 {
@@ -693,6 +709,12 @@ void CBlockPolicyEstimator::processBlock(const std::vector<RemovedMempoolTransac
     shortStats->UpdateMovingAverages();
     longStats->UpdateMovingAverages();
 
+    // We only consider parent transactions that are mined
+    // solely by their own fee rate. All transactions that are
+    // CPFP'd by some child should be ignored. Child transactions
+    // are not tracked upon entry into the mempool.
+    removeCPFPdParentTxs(txs_removed_for_block);
+
     unsigned int countedTxs = 0;
     // Update averages with data points from current block
     for (const auto& tx : txs_removed_for_block) {

src/policy/fees.h

@@ -303,6 +303,9 @@ private:
     /** Process a transaction confirmed in a block*/
     bool processBlockTx(unsigned int nBlockHeight, const RemovedMempoolTransactionInfo& tx) EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator);
 
+    /** Remove transactions CPFP'd by some child from fee estimation tracking stats */
+    void removeCPFPdParentTxs(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block) 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);
     /** Helper for estimateSmartFee */

src/policy/fees_util.cpp

@@ -0,0 +1,60 @@
+// Copyright (c) 2024 The Bitcoin Core developers
+// Distributed under the MIT software license. See the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+
+#include <node/mini_miner.h>
+#include <policy/feerate.h>
+#include <policy/fees_util.h>
+#include <primitives/transaction.h>
+
+TxAncestorsAndDescendants GetTxAncestorsAndDescendants(const std::vector<RemovedMempoolTransactionInfo>& transactions)
+{
+    TxAncestorsAndDescendants visited_txs;
+    for (const auto& tx_info : transactions) {
+        const auto& txid = tx_info.info.m_tx->GetHash();
+        visited_txs.emplace(txid, std::make_tuple(std::set<Txid>{txid}, std::set<Txid>{txid}));
+        for (const auto& input : tx_info.info.m_tx->vin) {
+            // If a parent has been visited add all the parent ancestors to the set of transaction ancestor
+            // Also add the transaction into each ancestor descendant set
+            if (visited_txs.find(input.prevout.hash) != visited_txs.end()) {
+                auto& parent_ancestors = std::get<0>(visited_txs[input.prevout.hash]);
+                auto& tx_ancestors = std::get<0>(visited_txs[txid]);
+                for (auto& ancestor : parent_ancestors) {
+                    tx_ancestors.insert(ancestor);
+                    auto& ancestor_descendants = std::get<1>(visited_txs[ancestor]);
+                    ancestor_descendants.insert(txid);
+                }
+            }
+        }
+    }
+    return visited_txs;
+}
+
+MiniMinerInput GetMiniMinerInput(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block)
+{
+    // Cache all the transactions for efficient lookup
+    std::map<Txid, TransactionInfo> tx_caches;
+    for (const auto& tx : txs_removed_for_block) {
+        tx_caches.emplace(tx.info.m_tx->GetHash(), TransactionInfo(tx.info.m_tx, tx.info.m_fee, tx.info.m_virtual_transaction_size, tx.info.txHeight));
+    }
+    const auto& txAncestorsAndDescendants = GetTxAncestorsAndDescendants(txs_removed_for_block);
+    std::vector<node::MiniMinerMempoolEntry> transactions;
+    std::map<Txid, std::set<Txid>> descendant_caches;
+    transactions.reserve(txAncestorsAndDescendants.size());
+    for (const auto& transaction : txAncestorsAndDescendants) {
+        const auto& txid = transaction.first;
+        const auto& [ancestors, descendants] = transaction.second;
+        int64_t vsize_ancestor = 0;
+        CAmount fee_with_ancestors = 0;
+        for (auto& ancestor_id : ancestors) {
+            const auto& ancestor = tx_caches.find(ancestor_id)->second;
+            vsize_ancestor += ancestor.m_virtual_transaction_size;
+            fee_with_ancestors += ancestor.m_fee;
+        }
+        descendant_caches.emplace(txid, descendants);
+        auto tx_info = tx_caches.find(txid)->second;
+        transactions.emplace_back(tx_info.m_tx, tx_info.m_virtual_transaction_size, vsize_ancestor, tx_info.m_fee, fee_with_ancestors);
+    }
+    return std::make_tuple(transactions, descendant_caches);
+}

src/policy/fees_util.h

@@ -0,0 +1,31 @@
+// Copyright (c) 2024 The Bitcoin Core developers
+// Distributed under the MIT software license. See the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_POLICY_FEES_UTIL_H
+#define BITCOIN_POLICY_FEES_UTIL_H
+
+#include <kernel/mempool_entry.h>
+#include <node/mini_miner.h>
+#include <primitives/transaction.h>
+
+#include <map>
+#include <set>
+#include <tuple>
+#include <vector>
+
+using TxAncestorsAndDescendants = std::map<Txid, std::tuple<std::set<Txid>, std::set<Txid>>>;
+using MiniMinerInput = std::tuple<std::vector<node::MiniMinerMempoolEntry>, std::map<Txid, std::set<Txid>>>;
+
+/* GetTxAncestorsAndDescendants takes the vector of transactions removed from the mempool after a block is connected.
+ * The function assumes the order the transactions were included in the block was maintained; that is, all transaction
+ * parents was added into the vector first before descendants.
+ *
+ * GetTxAncestorsAndDescendants computes all the ancestors and descendants of the transactions, the transaction is
+ * also included as a descendant and ancestor of itself.
+ */
+TxAncestorsAndDescendants GetTxAncestorsAndDescendants(const std::vector<RemovedMempoolTransactionInfo>& transactions);
+
+MiniMinerInput GetMiniMinerInput(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block);
+
+#endif // BITCOIN_POLICY_FEES_UTIL_H

src/test/CMakeLists.txt

@@ -62,6 +62,7 @@ add_executable(test_bitcoin
   descriptor_tests.cpp
   disconnected_transactions.cpp
   feefrac_tests.cpp
+  feesutil_tests.cpp
   flatfile_tests.cpp
   fs_tests.cpp
   getarg_tests.cpp

src/test/feesutil_tests.cpp

@@ -0,0 +1,272 @@
+// Copyright (c) 2024 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <kernel/mempool_entry.h>
+#include <policy/fees_util.h>
+#include <test/util/random.h>
+#include <test/util/txmempool.h>
+#include <txmempool.h>
+
+#include <test/util/setup_common.h>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(feesutil_tests, BasicTestingSetup)
+
+static inline CTransactionRef make_tx(const std::vector<COutPoint>& outpoints, int num_outputs = 1)
+{
+    CMutableTransaction tx;
+    tx.vin.reserve(outpoints.size());
+    tx.vout.resize(num_outputs);
+    for (const auto& outpoint : outpoints) {
+        tx.vin.emplace_back(outpoint);
+    }
+    for (int i = 0; i < num_outputs; ++i) {
+        auto scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+        tx.vout.emplace_back(COIN, scriptPubKey);
+    }
+    return MakeTransactionRef(tx);
+}
+
+void verifyTxsInclusion(const std::vector<Txid>& txs_vec, const std::set<Txid>& txs_set)
+{
+    BOOST_CHECK(txs_vec.size() == txs_set.size());
+    for (const auto txid : txs_vec) {
+        BOOST_CHECK(txs_set.find(txid) != txs_set.end());
+    }
+}
+
+void validateTxRelation(const Txid& txid, const std::vector<Txid>& ancestors, const std::vector<Txid>& descendants, const TxAncestorsAndDescendants& txAncestorsAndDescendants)
+{
+    const auto iter = txAncestorsAndDescendants.find(txid);
+    BOOST_CHECK(iter != txAncestorsAndDescendants.end());
+    const auto calc_ancestors = std::get<0>(iter->second);
+    verifyTxsInclusion(ancestors, calc_ancestors);
+    const auto calc_descendants = std::get<1>(iter->second);
+    verifyTxsInclusion(descendants, calc_descendants);
+}
+
+BOOST_AUTO_TEST_CASE(ComputingTxAncestorsAndDescendants)
+{
+    TestMemPoolEntryHelper entry;
+    auto create_and_add_tx = [&](std::vector<RemovedMempoolTransactionInfo>& transactions, const std::vector<COutPoint>& outpoints, int num_outputs = 1) {
+        const CTransactionRef tx = make_tx(outpoints, num_outputs);
+        transactions.emplace_back(entry.FromTx(tx));
+        return tx;
+    };
+
+    // Test 20 unique transactions
+    {
+        std::vector<RemovedMempoolTransactionInfo> transactions;
+        transactions.reserve(20);
+
+        for (auto i = 0; i < 20; ++i) {
+            const std::vector<COutPoint> outpoints{COutPoint(Txid::FromUint256(m_rng.rand256()), 0)};
+            create_and_add_tx(transactions, outpoints);
+        }
+
+        const auto txAncestorsAndDescendants = GetTxAncestorsAndDescendants(transactions);
+        BOOST_CHECK_EQUAL(txAncestorsAndDescendants.size(), transactions.size());
+
+        for (auto& tx : transactions) {
+            const Txid txid = tx.info.m_tx->GetHash();
+            const auto txiter = txAncestorsAndDescendants.find(txid);
+            BOOST_CHECK(txiter != txAncestorsAndDescendants.end());
+            const auto ancestors = std::get<0>(txiter->second);
+            const auto descendants = std::get<1>(txiter->second);
+            BOOST_CHECK(ancestors.size() == 1);
+            BOOST_CHECK(descendants.size() == 1);
+        }
+    }
+    // Test 3 Linear transactions clusters
+    /*
+            Linear Packages
+            A     B     C    D
+            |     |     |    |
+            E     H     J    K
+            |     |
+            F     I
+            |
+            G
+    */
+    {
+        std::vector<RemovedMempoolTransactionInfo> transactions;
+        transactions.reserve(11);
+
+        // Create transaction A B C D
+        for (auto i = 0; i < 4; ++i) {
+            const std::vector<COutPoint> outpoints{COutPoint(Txid::FromUint256(m_rng.rand256()), 0)};
+            create_and_add_tx(transactions, outpoints);
+        }
+
+        // Create cluster A children ---> E->F->G
+        std::vector<COutPoint> outpoints{COutPoint(transactions[0].info.m_tx->GetHash(), 0)};
+        for (auto i = 0; i < 3; ++i) {
+            const CTransactionRef tx = create_and_add_tx(transactions, outpoints);
+            outpoints = {COutPoint(tx->GetHash(), 0)};
+        }
+
+        // Create cluster B children ---> H->I
+        outpoints = {COutPoint(transactions[1].info.m_tx->GetHash(), 0)};
+        for (size_t i = 0; i < 2; ++i) {
+            const CTransactionRef tx = create_and_add_tx(transactions, outpoints);
+            outpoints = {COutPoint(tx->GetHash(), 0)};
+        }
+
+        // Create cluster C child ---> J
+        outpoints = {COutPoint(transactions[2].info.m_tx->GetHash(), 0)};
+        create_and_add_tx(transactions, outpoints);
+
+        // Create cluster D child ---> K
+        outpoints = {COutPoint(transactions[3].info.m_tx->GetHash(), 0)};
+        create_and_add_tx(transactions, outpoints);
+
+        const auto txAncestorsAndDescendants = GetTxAncestorsAndDescendants(transactions);
+        BOOST_CHECK_EQUAL(txAncestorsAndDescendants.size(), transactions.size());
+
+        // Check tx A topology;
+        {
+            const Txid txA_Id = transactions[0].info.m_tx->GetHash();
+            std::vector<Txid> descendants{txA_Id};
+            for (auto i = 4; i <= 6; i++) { // Add E, F, G
+                descendants.emplace_back(transactions[i].info.m_tx->GetHash());
+            }
+            validateTxRelation(txA_Id, std::vector<Txid>{txA_Id}, descendants, txAncestorsAndDescendants);
+        }
+        // Check tx G topology;
+        {
+            const Txid txG_Id = transactions[6].info.m_tx->GetHash();
+            std::vector<Txid> ancestors{transactions[0].info.m_tx->GetHash()};
+            for (auto i = 4; i <= 6; i++) { // Add E, F, G
+                ancestors.emplace_back(transactions[i].info.m_tx->GetHash());
+            }
+            validateTxRelation(txG_Id, ancestors, std::vector<Txid>{txG_Id}, txAncestorsAndDescendants);
+        }
+        // Check tx B topology;
+        {
+            const Txid txB_Id = transactions[1].info.m_tx->GetHash();
+            std::vector<Txid> descendants{txB_Id};
+            for (auto i = 7; i <= 8; i++) { // Add H, I
+                descendants.emplace_back(transactions[i].info.m_tx->GetHash());
+            }
+            validateTxRelation(txB_Id, std::vector<Txid>{txB_Id}, descendants, txAncestorsAndDescendants);
+        }
+        // Check tx H topology;
+        {
+            const Txid txH_Id = transactions[7].info.m_tx->GetHash();
+            const std::vector<Txid> ancestors{txH_Id, transactions[1].info.m_tx->GetHash()};   // H, B
+            const std::vector<Txid> descendants{txH_Id, transactions[8].info.m_tx->GetHash()}; // H, I
+            validateTxRelation(txH_Id, ancestors, descendants, txAncestorsAndDescendants);
+        }
+        // Check tx C topology;
+        {
+            const Txid txC_Id = transactions[2].info.m_tx->GetHash();
+            const std::vector<Txid> descendants{txC_Id, transactions[9].info.m_tx->GetHash()}; // C, J
+            validateTxRelation(txC_Id, std::vector<Txid>{txC_Id}, descendants, txAncestorsAndDescendants);
+        }
+        // Check tx D topology;
+        {
+            const Txid txD_Id = transactions[3].info.m_tx->GetHash();
+            const std::vector<Txid> descendants{txD_Id, transactions[10].info.m_tx->GetHash()}; // D, K
+            validateTxRelation(txD_Id, std::vector<Txid>{txD_Id}, descendants, txAncestorsAndDescendants);
+        }
+    }
+
+    /* Unique transactions with a cluster of transactions
+
+           Cluster A                      Cluster B
+              A                               B
+            /   \                           /   \
+           /     \                         /     \
+          C       D                       I       J
+        /   \     |                               |
+       /     \    |                               |
+      E       F   H                               K
+      \       /
+       \     /
+          G
+    */
+    {
+        std::vector<RemovedMempoolTransactionInfo> transactions;
+        transactions.reserve(11);
+
+        // Create transaction A and B
+        for (auto i = 0; i < 2; ++i) {
+            const std::vector<COutPoint> outpoints{COutPoint(Txid::FromUint256(m_rng.rand256()), 0)};
+            create_and_add_tx(transactions, outpoints, /*num_outputs=*/2);
+        }
+
+        // Cluster 1 Topology
+        // Create a child for A ---> C
+        std::vector<COutPoint> outpoints{COutPoint(transactions[0].info.m_tx->GetHash(), 0)};
+        const CTransactionRef tx_C = create_and_add_tx(transactions, outpoints, /*num_outputs=*/2);
+
+        // Create a child for A ---> D
+        outpoints = {COutPoint(transactions[0].info.m_tx->GetHash(), 1)};
+        create_and_add_tx(transactions, outpoints);
+
+        // Create a child for C ---> E
+        outpoints = {COutPoint(tx_C->GetHash(), 0)};
+        create_and_add_tx(transactions, outpoints);
+
+        // Create a child for C ---> F
+        outpoints = {COutPoint(tx_C->GetHash(), 1)};
+        create_and_add_tx(transactions, outpoints);
+
+        // Create a child for E and F  ---> G
+        outpoints = {COutPoint(transactions[4].info.m_tx->GetHash(), 0), COutPoint(transactions[5].info.m_tx->GetHash(), 0)};
+        create_and_add_tx(transactions, outpoints);
+
+        // Create a child for D ---> H
+        outpoints = {COutPoint(transactions[3].info.m_tx->GetHash(), 0)};
+        create_and_add_tx(transactions, outpoints);
+
+        // Cluster 2
+        // Create a child for B ---> I
+        outpoints = {COutPoint(transactions[1].info.m_tx->GetHash(), 0)};
+        create_and_add_tx(transactions, outpoints);
+
+        // Create a child for B ---> J
+        outpoints = {COutPoint(transactions[1].info.m_tx->GetHash(), 1)};
+        const CTransactionRef tx_J = create_and_add_tx(transactions, outpoints);
+
+        // Create a child for J ---> K
+        outpoints = {COutPoint(tx_J->GetHash(), 0)};
+        create_and_add_tx(transactions, outpoints);
+
+        const auto txAncestorsAndDescendants = GetTxAncestorsAndDescendants(transactions);
+        BOOST_CHECK_EQUAL(txAncestorsAndDescendants.size(), transactions.size());
+
+        // Check tx A topology;
+        {
+            const Txid txA_Id = transactions[0].info.m_tx->GetHash();
+            std::vector<Txid> descendants{txA_Id};
+            for (auto i = 2; i <= 7; i++) { // Add  C, D, E, F, G, H
+                descendants.emplace_back(transactions[i].info.m_tx->GetHash());
+            }
+            validateTxRelation(txA_Id, std::vector<Txid>{txA_Id}, descendants, txAncestorsAndDescendants);
+        }
+        // Check tx C topology;
+        {
+            const Txid txC_Id = transactions[2].info.m_tx->GetHash();
+            std::vector<Txid> ancestors{txC_Id, transactions[0].info.m_tx->GetHash()}; // C, A
+            std::vector<Txid> descendants{txC_Id};
+            for (auto i = 4; i <= 6; i++) { // Add E, F, G
+                descendants.emplace_back(transactions[i].info.m_tx->GetHash());
+            }
+            validateTxRelation(txC_Id, ancestors, descendants, txAncestorsAndDescendants);
+        }
+        // Check tx B topology;
+        {
+            const Txid txB_Id = transactions[1].info.m_tx->GetHash();
+            std::vector<Txid> descendants{txB_Id};
+            for (auto i = 8; i <= 10; i++) { // Add I, J, K
+                descendants.emplace_back(transactions[i].info.m_tx->GetHash());
+            }
+            validateTxRelation(txB_Id, std::vector<Txid>{txB_Id}, descendants, txAncestorsAndDescendants);
+        }
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/test/miniminer_tests.cpp

@@ -349,7 +349,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
     node::MiniMiner miniminer_pool(pool, all_unspent_outpoints);
     BOOST_CHECK(miniminer_manual.IsReadyToCalculate());
     BOOST_CHECK(miniminer_pool.IsReadyToCalculate());
-    for (const auto& sequences : {miniminer_manual.Linearize(), miniminer_pool.Linearize()}) {
+    for (const auto& sequences : {miniminer_manual.Linearize().inclusion_order, miniminer_pool.Linearize().inclusion_order}) {
         // tx6 is selected first: high feerate with no parents to bump
         BOOST_CHECK_EQUAL(Find(sequences, tx6->GetHash()), 0);
 
@@ -567,7 +567,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup)
     node::MiniMiner miniminer_pool(pool, all_unspent_outpoints);
     BOOST_CHECK(miniminer_manual.IsReadyToCalculate());
     BOOST_CHECK(miniminer_pool.IsReadyToCalculate());
-    for (const auto& sequences : {miniminer_manual.Linearize(), miniminer_pool.Linearize()}) {
+    for (const auto& sequences : {miniminer_manual.Linearize().inclusion_order, miniminer_pool.Linearize().inclusion_order}) {
         // tx2 and tx4 selected first: high feerate with nothing to bump
         BOOST_CHECK_EQUAL(Find(sequences, tx4->GetHash()), 0);
         BOOST_CHECK_EQUAL(Find(sequences, tx2->GetHash()), 1);
@@ -690,7 +690,7 @@ BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup)
 
         node::MiniMiner miniminer_manual(miniminer_info, descendant_caches);
         BOOST_CHECK(miniminer_manual.IsReadyToCalculate());
-        const auto sequences{miniminer_manual.Linearize()};
+        const auto sequences{miniminer_manual.Linearize().inclusion_order};
 
         // CPFP zero + high
         BOOST_CHECK_EQUAL(sequences.at(grandparent_zero_fee->GetHash()), 0);

test/functional/feature_fee_estimation.py

@@ -128,6 +128,15 @@ def make_tx(wallet, utxo, feerate):
         fee_rate=Decimal(feerate * 1000) / COIN,
     )
 
+
+def send_tx(wallet, node, utxo, feerate):
+    """Broadcast a 1in-1out transaction with a specific input and feerate (sat/vb)."""
+    return wallet.send_self_transfer(
+        from_node=node,
+        utxo_to_spend=utxo,
+        fee_rate=Decimal(feerate * 1000) / COIN,
+    )
+
 def check_fee_estimates_btw_modes(node, expected_conservative, expected_economical):
     fee_est_conservative = node.estimatesmartfee(1, estimate_mode="conservative")['feerate']
     fee_est_economical = node.estimatesmartfee(1, estimate_mode="economical")['feerate']
@@ -273,6 +282,11 @@ class EstimateFeeTest(BitcoinTestFramework):
             # Broadcast 5 low fee transaction which don't need to
             for _ in range(5):
                 tx = make_tx(self.wallet, utxos.pop(0), low_feerate)
+                self.confutxo.append({
+                    "txid": tx["txid"],
+                    "vout": 0,
+                    "value": Decimal(tx["tx"].vout[0].nValue) / COIN
+                })
                 txs.append(tx)
             batch_send_tx = [node.sendrawtransaction.get_request(tx["hex"]) for tx in txs]
             for n in self.nodes:
@@ -286,12 +300,16 @@ class EstimateFeeTest(BitcoinTestFramework):
             while len(utxos_to_respend) > 0:
                 u = utxos_to_respend.pop(0)
                 tx = make_tx(self.wallet, u, high_feerate)
+                self.confutxo.append({
+                    "txid": tx["txid"],
+                    "vout": 0,
+                    "value": Decimal(tx["tx"].vout[0].nValue) / COIN
+                })
                 node.sendrawtransaction(tx["hex"])
                 txs.append(tx)
             dec_txs = [res["result"] for res in node.batch([node.decoderawtransaction.get_request(tx["hex"]) for tx in txs])]
             self.wallet.scan_txs(dec_txs)
 
-
         # Mine the last replacement txs
         self.sync_mempools(wait=0.1, nodes=[node, miner])
         self.generate(miner, 1)
@@ -390,6 +408,64 @@ class EstimateFeeTest(BitcoinTestFramework):
         self.start_node(0,extra_args=["-acceptstalefeeestimates"])
         assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate)
 
+
+    def send_and_mine_child_tx(self, broadcaster, miner, parent_tx, feerate):
+        u = {"txid": parent_tx["txid"], "vout": 0, "value": Decimal(parent_tx["tx"].vout[0].nValue) / COIN}
+        send_tx(wallet=self.wallet, node=broadcaster, utxo=u, feerate=feerate)
+        self.sync_mempools(wait=0.1, nodes=[broadcaster, miner])
+        self.generate(miner, 1)
+
+    def sanity_check_cpfp_estimates(self, utxos):
+        """The BlockPolicyEstimator currently does not take CPFP into account. This test
+        sanity checks its behaviour when receiving transactions that were confirmed because
+        of their child's feerate.
+        """
+        # The broadcaster and block producer
+        broadcaster = self.nodes[0]
+        miner = self.nodes[1]
+        # In sat/vb
+        [low_feerate, med_feerate, high_feerate] = [Decimal(2), Decimal(15), Decimal(20)]
+
+        self.log.info("Test that fee estimator will ignore all transaction with in block child")
+        # If a transaction got mined and has a child in the same block it was mined
+        # if the child does not CPFP the parent, the parent got accounted in fee estimation.
+        low_fee_parent = send_tx(wallet=self.wallet, node=broadcaster, utxo=None, feerate=low_feerate)
+        self.send_and_mine_child_tx(broadcaster=broadcaster, miner=miner, parent_tx=low_fee_parent, feerate=high_feerate)
+        assert_equal(broadcaster.estimaterawfee(1)["short"]["fail"]["totalconfirmed"], 0)
+
+        # If the child CPFP the parent, the parent is not accounted in fee estimation
+        high_fee_parent = send_tx(wallet=self.wallet, node=broadcaster, utxo=None, feerate=high_feerate)
+        self.send_and_mine_child_tx(broadcaster=broadcaster, miner=miner, parent_tx=high_fee_parent, feerate=low_feerate)
+        assert_equal(broadcaster.estimaterawfee(1)["short"]["fail"]["totalconfirmed"], 1)
+
+        # Generate and mine packages of transactions, 80% of them are a [low fee, high fee] package
+        # which get mined because of the child transaction. 20% are single-transaction packages with
+        # a medium-high feerate.
+        # Test that we don't give the low feerate as estimate, assuming the low fee transactions
+        # got mined on their own.
+        for _ in range(5):
+            txs = []  # Batch the RPCs calls.
+            for _ in range(20):
+                u = utxos.pop(0)
+                parent_tx = make_tx(wallet=self.wallet, utxo=u, feerate=low_feerate)
+                txs.append(parent_tx)
+                u = {
+                    "txid": parent_tx["txid"],
+                    "vout": 0,
+                    "value": Decimal(parent_tx["tx"].vout[0].nValue) / COIN
+                }
+                child_tx = make_tx(wallet=self.wallet, utxo=u, feerate=high_feerate)
+                txs.append(child_tx)
+            for _ in range(5):
+                u = utxos.pop(0)
+                tx = make_tx(wallet=self.wallet, utxo=u, feerate=med_feerate)
+                txs.append(tx)
+            batch_send_tx = (broadcaster.sendrawtransaction.get_request(tx["hex"]) for tx in txs)
+            broadcaster.batch(batch_send_tx)
+            self.sync_mempools(wait=0.1, nodes=[broadcaster, miner])
+            self.generate(miner, 1)
+        assert_equal(broadcaster.estimatesmartfee(2)["feerate"], med_feerate * 1000 / COIN)
+
     def clear_estimates(self):
         self.log.info("Restarting node with fresh estimation")
         self.stop_node(0)
@@ -466,7 +542,11 @@ class EstimateFeeTest(BitcoinTestFramework):
         self.clear_estimates()
 
         self.log.info("Testing estimates with RBF.")
-        self.sanity_check_rbf_estimates(self.confutxo + self.memutxo)
+        self.sanity_check_rbf_estimates(self.confutxo)
+
+        self.clear_estimates()
+        self.log.info("Testing estimates with CPFP.")
+        self.sanity_check_cpfp_estimates(self.confutxo)
 
         self.clear_estimates()
         self.log.info("Test estimatesmartfee modes")