diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 89fdd855a4..e5220b8444 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index d7d15554b3..6a70c8f54b 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -170,7 +170,8 @@ MiniMiner::MiniMiner(const std::vector& 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 target_feerate) break; } + m_linearize_result.size_per_feerate.emplace_back(CFeeRate{ancestor_package_fee, static_cast(ancestor_package_size)}, static_cast(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 ancestors; @@ -286,7 +288,7 @@ void MiniMiner::BuildMockTemplate(std::optional 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 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 MiniMiner::Linearize() +LinearizationResult MiniMiner::Linearize() { BuildMockTemplate(std::nullopt); - return m_inclusion_order; + return m_linearize_result; } std::map MiniMiner::CalculateBumpFees(const CFeeRate& target_feerate) diff --git a/src/node/mini_miner.h b/src/node/mini_miner.h index aec2aaf6b6..40a98f59e9 100644 --- a/src/node/mini_miner.h +++ b/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 inclusion_order; + + // A vector of all the packages in the block template their feerate and virtual sizes. + std::vector> 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> 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 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 m_bump_fees; @@ -162,10 +169,9 @@ public: std::optional 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 Linearize(); + LinearizationResult Linearize(); }; } // namespace node diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index e85b2f2caa..174b1dd8ee 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -9,7 +9,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -665,6 +667,20 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const Remo return true; } +void CBlockPolicyEstimator::removeCPFPdParentTxs(const std::vector& 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 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& txs_removed_for_block, unsigned int nBlockHeight) { @@ -693,6 +709,12 @@ void CBlockPolicyEstimator::processBlock(const std::vectorUpdateMovingAverages(); 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) { diff --git a/src/policy/fees.h b/src/policy/fees.h index a95cc19dd4..f7631ff3b8 100644 --- a/src/policy/fees.h +++ b/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& 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 */ diff --git a/src/policy/fees_util.cpp b/src/policy/fees_util.cpp new file mode 100644 index 0000000000..157ed7989b --- /dev/null +++ b/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 +#include +#include +#include + +TxAncestorsAndDescendants GetTxAncestorsAndDescendants(const std::vector& 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}, std::set{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& txs_removed_for_block) +{ + // Cache all the transactions for efficient lookup + std::map 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 transactions; + std::map> 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); +} diff --git a/src/policy/fees_util.h b/src/policy/fees_util.h new file mode 100644 index 0000000000..f8f2d3f94c --- /dev/null +++ b/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 +#include +#include + +#include +#include +#include +#include + +using TxAncestorsAndDescendants = std::map, std::set>>; +using MiniMinerInput = std::tuple, std::map>>; + +/* 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& transactions); + +MiniMinerInput GetMiniMinerInput(const std::vector& txs_removed_for_block); + +#endif // BITCOIN_POLICY_FEES_UTIL_H diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 859b913206..593b050128 100644 --- a/src/test/CMakeLists.txt +++ b/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 diff --git a/src/test/feesutil_tests.cpp b/src/test/feesutil_tests.cpp new file mode 100644 index 0000000000..df867198ff --- /dev/null +++ b/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 +#include +#include +#include +#include + +#include + +#include + +BOOST_FIXTURE_TEST_SUITE(feesutil_tests, BasicTestingSetup) + +static inline CTransactionRef make_tx(const std::vector& 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& txs_vec, const std::set& 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& ancestors, const std::vector& 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& transactions, const std::vector& 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 transactions; + transactions.reserve(20); + + for (auto i = 0; i < 20; ++i) { + const std::vector 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 transactions; + transactions.reserve(11); + + // Create transaction A B C D + for (auto i = 0; i < 4; ++i) { + const std::vector outpoints{COutPoint(Txid::FromUint256(m_rng.rand256()), 0)}; + create_and_add_tx(transactions, outpoints); + } + + // Create cluster A children ---> E->F->G + std::vector 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 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{txA_Id}, descendants, txAncestorsAndDescendants); + } + // Check tx G topology; + { + const Txid txG_Id = transactions[6].info.m_tx->GetHash(); + std::vector 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{txG_Id}, txAncestorsAndDescendants); + } + // Check tx B topology; + { + const Txid txB_Id = transactions[1].info.m_tx->GetHash(); + std::vector 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{txB_Id}, descendants, txAncestorsAndDescendants); + } + // Check tx H topology; + { + const Txid txH_Id = transactions[7].info.m_tx->GetHash(); + const std::vector ancestors{txH_Id, transactions[1].info.m_tx->GetHash()}; // H, B + const std::vector 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 descendants{txC_Id, transactions[9].info.m_tx->GetHash()}; // C, J + validateTxRelation(txC_Id, std::vector{txC_Id}, descendants, txAncestorsAndDescendants); + } + // Check tx D topology; + { + const Txid txD_Id = transactions[3].info.m_tx->GetHash(); + const std::vector descendants{txD_Id, transactions[10].info.m_tx->GetHash()}; // D, K + validateTxRelation(txD_Id, std::vector{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 transactions; + transactions.reserve(11); + + // Create transaction A and B + for (auto i = 0; i < 2; ++i) { + const std::vector 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 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 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{txA_Id}, descendants, txAncestorsAndDescendants); + } + // Check tx C topology; + { + const Txid txC_Id = transactions[2].info.m_tx->GetHash(); + std::vector ancestors{txC_Id, transactions[0].info.m_tx->GetHash()}; // C, A + std::vector 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 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{txB_Id}, descendants, txAncestorsAndDescendants); + } + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index d1fec479a0..b5f1c68b4c 100644 --- a/src/test/miniminer_tests.cpp +++ b/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); diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 974d8268a2..86d321d2d6 100755 --- a/test/functional/feature_fee_estimation.py +++ b/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")