From 4aa98b79b266cd526efa577762b0bcfccbdeda11 Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 3 Nov 2023 10:12:35 +0000 Subject: [PATCH 1/8] [lint] update expected boost includes --- test/lint/lint-includes.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py index 8e79ba5121..6386a92c0f 100755 --- a/test/lint/lint-includes.py +++ b/test/lint/lint-includes.py @@ -23,6 +23,7 @@ EXCLUDED_DIRS = ["contrib/devtools/bitcoin-tidy/", ] EXPECTED_BOOST_INCLUDES = ["boost/date_time/posix_time/posix_time.hpp", + "boost/multi_index/detail/hash_index_iterator.hpp", "boost/multi_index/hashed_index.hpp", "boost/multi_index/identity.hpp", "boost/multi_index/indexed_by.hpp", @@ -30,6 +31,7 @@ EXPECTED_BOOST_INCLUDES = ["boost/date_time/posix_time/posix_time.hpp", "boost/multi_index/sequenced_index.hpp", "boost/multi_index/tag.hpp", "boost/multi_index_container.hpp", + "boost/operators.hpp", "boost/process.hpp", "boost/signals2/connection.hpp", "boost/signals2/optional_last_value.hpp", From e3b2e630b219ca15fe0b2640ca422712c86ac33d Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 31 Oct 2023 08:51:20 +0000 Subject: [PATCH 2/8] [refactor] change MiniMinerMempoolEntry ctor to take values, update includes No behavior change. All we are doing is copying out these values before passing them into the ctor instead of within the ctor. This makes it possible to use the MiniMiner algorithms to analyze transactions that haven't been submitted to the mempool yet. It also iwyu's the mini_miner includes. --- src/node/mini_miner.cpp | 12 +++++++++++- src/node/mini_miner.h | 30 +++++++++++++++++++++--------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index 2827242f96..95eac7a243 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -4,9 +4,14 @@ #include +#include +#include #include #include #include +#include +#include +#include #include #include @@ -72,7 +77,12 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector& ou // Add every entry to m_entries_by_txid and m_entries, except the ones that will be replaced. for (const auto& txiter : cluster) { if (!m_to_be_replaced.count(txiter->GetTx().GetHash())) { - auto [mapiter, success] = m_entries_by_txid.emplace(txiter->GetTx().GetHash(), MiniMinerMempoolEntry(txiter)); + auto [mapiter, success] = m_entries_by_txid.emplace(txiter->GetTx().GetHash(), + MiniMinerMempoolEntry{/*fee_self=*/txiter->GetModifiedFee(), + /*fee_ancestor=*/txiter->GetModFeesWithAncestors(), + /*vsize_self=*/txiter->GetTxSize(), + /*vsize_ancestor=*/txiter->GetSizeWithAncestors(), + /*tx_in=*/txiter->GetSharedTx()}); m_entries.push_back(mapiter); } else { auto outpoints_it = m_requested_outpoints_by_txid.find(txiter->GetTx().GetHash()); diff --git a/src/node/mini_miner.h b/src/node/mini_miner.h index 9d9d66bf0b..ae26202965 100644 --- a/src/node/mini_miner.h +++ b/src/node/mini_miner.h @@ -5,33 +5,45 @@ #ifndef BITCOIN_NODE_MINI_MINER_H #define BITCOIN_NODE_MINI_MINER_H -#include +#include +#include +#include +#include #include #include +#include #include +#include + +class CFeeRate; +class CTxMemPool; namespace node { // Container for tracking updates to ancestor feerate as we include ancestors in the "block" class MiniMinerMempoolEntry { - const CAmount fee_individual; const CTransactionRef tx; const int64_t vsize_individual; - CAmount fee_with_ancestors; int64_t vsize_with_ancestors; + const CAmount fee_individual; + CAmount fee_with_ancestors; // This class must be constructed while holding mempool.cs. After construction, the object's // methods can be called without holding that lock. public: - explicit MiniMinerMempoolEntry(CTxMemPool::txiter entry) : - fee_individual{entry->GetModifiedFee()}, - tx{entry->GetSharedTx()}, - vsize_individual(entry->GetTxSize()), - fee_with_ancestors{entry->GetModFeesWithAncestors()}, - vsize_with_ancestors(entry->GetSizeWithAncestors()) + explicit MiniMinerMempoolEntry(CAmount fee_self, + CAmount fee_ancestor, + int64_t vsize_self, + int64_t vsize_ancestor, + const CTransactionRef& tx_in): + tx{tx_in}, + vsize_individual{vsize_self}, + vsize_with_ancestors{vsize_ancestor}, + fee_individual{fee_self}, + fee_with_ancestors{fee_ancestor} { } CAmount GetModifiedFee() const { return fee_individual; } From 5a83f55c96661a886dd6f5231920b2f730cf6773 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 31 Oct 2023 09:29:59 +0000 Subject: [PATCH 3/8] [MiniMiner] allow manual construction with non-mempool txns This is primarily intended for linearizing a package of transactions prior to submitting them to mempool. Note that, if this ctor is used, bump fees will not be calculated because we haven't instructed MiniMiner which outpoints for which we want bump fees to be calculated. --- src/node/mini_miner.cpp | 42 +++++++++++++++++++++++++++++++++++++++++ src/node/mini_miner.h | 17 +++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index 95eac7a243..c7bb4a6a42 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -132,6 +132,48 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector& ou SanityCheck(); } +MiniMiner::MiniMiner(const std::vector& manual_entries, + const std::map>& descendant_caches) +{ + for (const auto& entry : manual_entries) { + const auto& txid = entry.GetTx().GetHash(); + // We need to know the descendant set of every transaction. + if (!Assume(descendant_caches.count(txid) > 0)) { + m_ready_to_calculate = false; + return; + } + // Just forward these args onto MiniMinerMempoolEntry + auto [mapiter, success] = m_entries_by_txid.emplace(txid, entry); + // Txids must be unique; this txid shouldn't already be an entry in m_entries_by_txid + if (Assume(success)) m_entries.push_back(mapiter); + } + // Descendant cache is already built, but we need to translate them to m_entries_by_txid iters. + for (const auto& [txid, desc_txids] : descendant_caches) { + // Descendant cache should include at least the tx itself. + if (!Assume(!desc_txids.empty())) { + m_ready_to_calculate = false; + return; + } + std::vector cached_descendants; + for (const auto& desc_txid : desc_txids) { + auto desc_it{m_entries_by_txid.find(desc_txid)}; + // Descendants should only include transactions with corresponding entries. + if (!Assume(desc_it != m_entries_by_txid.end())) { + m_ready_to_calculate = false; + return; + } else { + cached_descendants.emplace_back(desc_it); + } + } + m_descendant_set_by_txid.emplace(txid, cached_descendants); + } + Assume(m_to_be_replaced.empty()); + Assume(m_requested_outpoints_by_txid.empty()); + Assume(m_bump_fees.empty()); + SanityCheck(); +} + + // Compare by min(ancestor feerate, individual feerate), then iterator // // Under the ancestor-based mining approach, high-feerate children can pay for parents, but high-feerate diff --git a/src/node/mini_miner.h b/src/node/mini_miner.h index ae26202965..f1713bc868 100644 --- a/src/node/mini_miner.h +++ b/src/node/mini_miner.h @@ -120,8 +120,25 @@ public: /** Returns set of txids in the block template if one has been constructed. */ std::set GetMockTemplateTxids() const { return m_in_block; } + /** Constructor that takes a list of outpoints that may or may not belong to transactions in the + * mempool. Copies out information about the relevant transactions in the mempool into + * MiniMinerMempoolEntrys. + */ MiniMiner(const CTxMemPool& mempool, const std::vector& outpoints); + /** Constructor in which the MiniMinerMempoolEntry entries have been constructed manually, + * presumably because these transactions are not in the mempool (yet). It is assumed that all + * entries are unique and their values are correct, otherwise results computed by MiniMiner may + * be incorrect. Callers should check IsReadyToCalculate() after construction. + * @param[in] descendant_caches A map from each transaction to the set of txids of this + * transaction's descendant set, including itself. Each tx in + * manual_entries must have a corresponding entry in this map, and + * all of the txids in a descendant set must correspond to a tx in + * manual_entries. + */ + MiniMiner(const std::vector& manual_entries, + const std::map>& descendant_caches); + /** Construct a new block template and, for each outpoint corresponding to a transaction that * did not make it into the block, calculate the cost of bumping those transactions (and their * ancestors) to the minimum feerate. Returns a map from outpoint to bump fee, or an empty map From fe6332c0badf07e99044b00084fc6b07f735a051 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 31 Oct 2023 11:43:14 +0000 Subject: [PATCH 4/8] [MiniMiner] make target_feerate optional Add an option to keep building the template regardless of feerate. We can't just use target_feerate=0 because it's possible for transactions to have negative modified feerates. No behavior change for users that pass in a target_feerate. --- src/node/mini_miner.cpp | 13 +++++++++---- src/node/mini_miner.h | 8 +++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index c7bb4a6a42..ea702ab734 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -173,7 +173,6 @@ MiniMiner::MiniMiner(const std::vector& manual_entries, SanityCheck(); } - // Compare by min(ancestor feerate, individual feerate), then iterator // // Under the ancestor-based mining approach, high-feerate children can pay for parents, but high-feerate @@ -253,8 +252,9 @@ void MiniMiner::SanityCheck() const [&](const auto& txid){return m_entries_by_txid.find(txid) == m_entries_by_txid.end();})); } -void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate) +void MiniMiner::BuildMockTemplate(std::optional target_feerate) { + const auto num_txns{m_entries_by_txid.size()}; while (!m_entries_by_txid.empty()) { // Sort again, since transaction removal may change some m_entries' ancestor feerates. std::sort(m_entries.begin(), m_entries.end(), AncestorFeerateComparator()); @@ -265,7 +265,8 @@ void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate) const auto ancestor_package_size = (*best_iter)->second.GetSizeWithAncestors(); const auto ancestor_package_fee = (*best_iter)->second.GetModFeesWithAncestors(); // Stop here. Everything that didn't "make it into the block" has bumpfee. - if (ancestor_package_fee < target_feerate.GetFee(ancestor_package_size)) { + if (target_feerate.has_value() && + ancestor_package_fee < target_feerate->GetFee(ancestor_package_size)) { break; } @@ -292,7 +293,11 @@ void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate) DeleteAncestorPackage(ancestors); SanityCheck(); } - Assume(m_in_block.empty() || m_total_fees >= target_feerate.GetFee(m_total_vsize)); + if (!target_feerate.has_value()) { + Assume(m_in_block.size() == num_txns); + } else { + Assume(m_in_block.empty() || m_total_fees >= target_feerate->GetFee(m_total_vsize)); + } // Do not try to continue building the block template with a different feerate. m_ready_to_calculate = false; } diff --git a/src/node/mini_miner.h b/src/node/mini_miner.h index f1713bc868..e0051f3364 100644 --- a/src/node/mini_miner.h +++ b/src/node/mini_miner.h @@ -84,7 +84,7 @@ class MiniMiner // the same tx will have the same bumpfee. Excludes non-mempool transactions. std::map> m_requested_outpoints_by_txid; - // What we're trying to calculate. + // What we're trying to calculate. Outpoint to the fee needed to bring the transaction to the target feerate. std::map m_bump_fees; // The constructed block template @@ -114,8 +114,9 @@ public: /** Returns true if CalculateBumpFees may be called, false if not. */ bool IsReadyToCalculate() const { return m_ready_to_calculate; } - /** Build a block template until the target feerate is hit. */ - void BuildMockTemplate(const CFeeRate& target_feerate); + /** Build a block template until the target feerate is hit. If target_feerate is not given, + * builds a block template until all transactions have been selected. */ + void BuildMockTemplate(std::optional target_feerate); /** Returns set of txids in the block template if one has been constructed. */ std::set GetMockTemplateTxids() const { return m_in_block; } @@ -149,6 +150,7 @@ public: * not make it into the block to the target feerate. Returns the total bump fee, or std::nullopt * if it cannot be calculated. */ std::optional CalculateTotalBumpFees(const CFeeRate& target_feerate); + }; } // namespace node From 004075963f12f17c3c399d81e3b48d6a8151aebd Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 31 Oct 2023 12:00:48 +0000 Subject: [PATCH 5/8] [test] add case for MiniMiner working with negative fee txns --- src/test/miniminer_tests.cpp | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index f65356936b..c599040883 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -67,6 +67,40 @@ Value Find(const std::map& map, const Key& key) return it->second; } +BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup) +{ + CTxMemPool& pool = *Assert(m_node.mempool); + LOCK2(::cs_main, pool.cs); + TestMemPoolEntryHelper entry; + + // Create a transaction that will be prioritised to have a negative modified fee. + const CAmount positive_base_fee{1000}; + const CAmount negative_fee_delta{-50000}; + const CAmount negative_modified_fees{positive_base_fee + negative_fee_delta}; + BOOST_CHECK(negative_modified_fees < 0); + const auto tx_mod_negative = make_tx({COutPoint{m_coinbase_txns[4]->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(positive_base_fee).FromTx(tx_mod_negative)); + pool.PrioritiseTransaction(tx_mod_negative->GetHash(), negative_fee_delta); + const COutPoint only_outpoint{tx_mod_negative->GetHash(), 0}; + + // When target feerate is 0, transactions with negative fees are not selected. + node::MiniMiner mini_miner_target0(pool, {only_outpoint}); + BOOST_CHECK(mini_miner_target0.IsReadyToCalculate()); + const CFeeRate feerate_zero(0); + mini_miner_target0.BuildMockTemplate(feerate_zero); + // Check the quit condition: + BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(pool.GetIter(tx_mod_negative->GetHash()).value()->GetTxSize())); + BOOST_CHECK(mini_miner_target0.GetMockTemplateTxids().empty()); + + // With no target feerate, the template includes all transactions, even negative feerate ones. + node::MiniMiner mini_miner_no_target(pool, {only_outpoint}); + BOOST_CHECK(mini_miner_no_target.IsReadyToCalculate()); + mini_miner_no_target.BuildMockTemplate(std::nullopt); + const auto template_txids{mini_miner_no_target.GetMockTemplateTxids()}; + BOOST_CHECK_EQUAL(template_txids.size(), 1); + BOOST_CHECK(template_txids.count(tx_mod_negative->GetHash().ToUint256()) > 0); +} + BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) { CTxMemPool& pool = *Assert(m_node.mempool); From f4b1b24a3bcd0199a6d2e828ad46033e1368336e Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 14 Jul 2023 11:28:00 +0100 Subject: [PATCH 6/8] [MiniMiner] track inclusion order and add Linearize() function Sometimes we are just interested in the order in which transactions would be included in a block (we want to "linearize" the transactions). Track and store this information. This doesn't change any of the bump fee calculations. --- src/node/mini_miner.cpp | 16 ++++++++++++++++ src/node/mini_miner.h | 19 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index ea702ab734..3d24a3f58e 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -170,6 +170,7 @@ 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()); SanityCheck(); } @@ -255,6 +256,7 @@ void MiniMiner::SanityCheck() const void MiniMiner::BuildMockTemplate(std::optional target_feerate) { const auto num_txns{m_entries_by_txid.size()}; + uint32_t sequence_num{0}; while (!m_entries_by_txid.empty()) { // Sort again, since transaction removal may change some m_entries' ancestor feerates. std::sort(m_entries.begin(), m_entries.end(), AncestorFeerateComparator()); @@ -290,18 +292,32 @@ void MiniMiner::BuildMockTemplate(std::optional target_feerate) to_process.erase(iter); } } + // Track the order in which transactions were selected. + for (const auto& ancestor : ancestors) { + m_inclusion_order.emplace(Txid::FromUint256(ancestor->first), sequence_num); + } DeleteAncestorPackage(ancestors); SanityCheck(); + ++sequence_num; } if (!target_feerate.has_value()) { Assume(m_in_block.size() == num_txns); } else { 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()); // Do not try to continue building the block template with a different feerate. m_ready_to_calculate = false; } + +std::map MiniMiner::Linearize() +{ + BuildMockTemplate(std::nullopt); + return m_inclusion_order; +} + std::map MiniMiner::CalculateBumpFees(const CFeeRate& target_feerate) { if (!m_ready_to_calculate) return {}; diff --git a/src/node/mini_miner.h b/src/node/mini_miner.h index e0051f3364..8f86709ae4 100644 --- a/src/node/mini_miner.h +++ b/src/node/mini_miner.h @@ -67,8 +67,14 @@ struct IteratorComparator } }; -/** A minimal version of BlockAssembler. Allows us to run the mining algorithm on a subset of - * mempool transactions, ignoring consensus rules, to calculate mining scores. */ +/** A minimal version of BlockAssembler, using the same ancestor set scoring algorithm. Allows us to + * run this algorithm on a limited set of transactions (e.g. subset of mempool or transactions that + * are not yet in mempool) instead of the entire mempool, ignoring consensus rules. + * Callers may use this to: + * - Calculate the "bump fee" needed to spend an unconfirmed UTXO at a given feerate + * - "Linearize" a list of transactions to see the order in which they would be selected for + * inclusion in a block + */ class MiniMiner { // When true, a caller may use CalculateBumpFees(). Becomes false if we failed to retrieve @@ -84,6 +90,10 @@ 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; // What we're trying to calculate. Outpoint to the fee needed to bring the transaction to the target feerate. std::map m_bump_fees; @@ -151,6 +161,11 @@ public: * if it cannot be calculated. */ 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. + */ + std::map Linearize(); }; } // namespace node From dfd6a3788c35be121eba1ad84f20effadcb7e7dc Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 3 Nov 2023 10:08:56 +0000 Subject: [PATCH 7/8] [refactor] unify fee amounts in miniminer_tests Name {low,med,high}_fee constants for reuse across file. --- src/test/miniminer_tests.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index c599040883..840093f524 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -15,6 +15,11 @@ BOOST_FIXTURE_TEST_SUITE(miniminer_tests, TestingSetup) +const CAmount low_fee{CENT/2000}; // 500 ṩ +const CAmount med_fee{CENT/200}; // 5000 ṩ +const CAmount high_fee{CENT/10}; // 100_000 ṩ + + static inline CTransactionRef make_tx(const std::vector& inputs, size_t num_outputs) { CMutableTransaction tx = CMutableTransaction(); @@ -107,15 +112,11 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) LOCK2(::cs_main, pool.cs); TestMemPoolEntryHelper entry; - const CAmount low_fee{CENT/2000}; - const CAmount normal_fee{CENT/200}; - const CAmount high_fee{CENT/10}; - // Create a parent tx0 and child tx1 with normal fees: const auto tx0 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx0)); + pool.addUnchecked(entry.Fee(med_fee).FromTx(tx0)); const auto tx1 = make_tx({COutPoint{tx0->GetHash(), 0}}, /*num_outputs=*/1); - pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx1)); + pool.addUnchecked(entry.Fee(med_fee).FromTx(tx1)); // Create a low-feerate parent tx2 and high-feerate child tx3 (cpfp) const auto tx2 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); @@ -128,9 +129,10 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) pool.addUnchecked(entry.Fee(low_fee).FromTx(tx4)); const auto tx5 = make_tx({COutPoint{tx4->GetHash(), 0}}, /*num_outputs=*/1); pool.addUnchecked(entry.Fee(low_fee).FromTx(tx5)); + const CAmount tx5_delta{CENT/100}; // Make tx5's modified fee much higher than its base fee. This should cause it to pass // the fee-related checks despite being low-feerate. - pool.PrioritiseTransaction(tx5->GetHash(), CENT/100); + pool.PrioritiseTransaction(tx5->GetHash(), tx5_delta); // Create a high-feerate parent tx6, low-feerate child tx7 const auto tx6 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); @@ -342,10 +344,6 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) LOCK2(::cs_main, pool.cs); TestMemPoolEntryHelper entry; - const CAmount low_fee{CENT/2000}; // 500 ṩ - const CAmount med_fee{CENT/200}; // 5000 ṩ - const CAmount high_fee{CENT/10}; // 100_000 ṩ - // Create 3 parents of different feerates, and 1 child spending outputs from all 3 parents. const auto tx0 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); pool.addUnchecked(entry.Fee(low_fee).FromTx(tx0)); From d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 31 Oct 2023 09:24:23 +0000 Subject: [PATCH 8/8] [test] MiniMiner::Linearize and manual construction --- src/test/miniminer_tests.cpp | 163 +++++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index 840093f524..76c079a6e7 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -133,6 +133,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) // Make tx5's modified fee much higher than its base fee. This should cause it to pass // the fee-related checks despite being low-feerate. pool.PrioritiseTransaction(tx5->GetHash(), tx5_delta); + const CAmount tx5_mod_fee{low_fee + tx5_delta}; // Create a high-feerate parent tx6, low-feerate child tx7 const auto tx6 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); @@ -309,6 +310,64 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) } } } + + // Check m_inclusion_order for equivalent mempool- and manually-constructed MiniMiners. + // (We cannot check bump fees in manually-constructed MiniMiners because it doesn't know what + // outpoints are requested). + std::vector miniminer_info; + { + const int32_t tx0_vsize{tx_dims.at(tx0->GetHash()).vsize}; + const int32_t tx1_vsize{tx_dims.at(tx1->GetHash()).vsize}; + const int32_t tx2_vsize{tx_dims.at(tx2->GetHash()).vsize}; + const int32_t tx3_vsize{tx_dims.at(tx3->GetHash()).vsize}; + const int32_t tx4_vsize{tx_dims.at(tx4->GetHash()).vsize}; + const int32_t tx5_vsize{tx_dims.at(tx5->GetHash()).vsize}; + const int32_t tx6_vsize{tx_dims.at(tx6->GetHash()).vsize}; + const int32_t tx7_vsize{tx_dims.at(tx7->GetHash()).vsize}; + + miniminer_info.emplace_back(/*fee_self=*/med_fee,/*fee_ancestor=*/med_fee,/*vsize_self=*/tx0_vsize,/*vsize_ancestor=*/tx0_vsize, tx0); + miniminer_info.emplace_back( med_fee, 2*med_fee, tx1_vsize, tx0_vsize + tx1_vsize, tx1); + miniminer_info.emplace_back( low_fee, low_fee, tx2_vsize, tx2_vsize, tx2); + miniminer_info.emplace_back( high_fee, low_fee + high_fee, tx3_vsize, tx2_vsize + tx3_vsize, tx3); + miniminer_info.emplace_back( low_fee, low_fee, tx4_vsize, tx4_vsize, tx4); + miniminer_info.emplace_back( tx5_mod_fee, low_fee + tx5_mod_fee, tx5_vsize, tx4_vsize + tx5_vsize, tx5); + miniminer_info.emplace_back( high_fee, high_fee, tx6_vsize, tx6_vsize, tx6); + miniminer_info.emplace_back( low_fee, high_fee + low_fee, tx7_vsize, tx6_vsize + tx7_vsize, tx7); + } + std::map> descendant_caches; + descendant_caches.emplace(tx0->GetHash(), std::set{tx0->GetHash(), tx1->GetHash()}); + descendant_caches.emplace(tx1->GetHash(), std::set{tx1->GetHash()}); + descendant_caches.emplace(tx2->GetHash(), std::set{tx2->GetHash(), tx3->GetHash()}); + descendant_caches.emplace(tx3->GetHash(), std::set{tx3->GetHash()}); + descendant_caches.emplace(tx4->GetHash(), std::set{tx4->GetHash(), tx5->GetHash()}); + descendant_caches.emplace(tx5->GetHash(), std::set{tx5->GetHash()}); + descendant_caches.emplace(tx6->GetHash(), std::set{tx6->GetHash(), tx7->GetHash()}); + descendant_caches.emplace(tx7->GetHash(), std::set{tx7->GetHash()}); + + node::MiniMiner miniminer_manual(miniminer_info, descendant_caches); + // Use unspent outpoints to avoid entries being omitted. + 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()}) { + // tx6 is selected first: high feerate with no parents to bump + BOOST_CHECK_EQUAL(Find(sequences, tx6->GetHash()), 0); + + // tx2 + tx3 CPFP are selected next + BOOST_CHECK_EQUAL(Find(sequences, tx2->GetHash()), 1); + BOOST_CHECK_EQUAL(Find(sequences, tx3->GetHash()), 1); + + // tx4 + prioritised tx5 CPFP + BOOST_CHECK_EQUAL(Find(sequences, tx4->GetHash()), 2); + BOOST_CHECK_EQUAL(Find(sequences, tx5->GetHash()), 2); + + BOOST_CHECK_EQUAL(Find(sequences, tx0->GetHash()), 3); + BOOST_CHECK_EQUAL(Find(sequences, tx1->GetHash()), 3); + + + // tx7 is selected last: low feerate with no children + BOOST_CHECK_EQUAL(Find(sequences, tx7->GetHash()), 4); + } } BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) @@ -482,6 +541,50 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) BOOST_CHECK(tx7_bumpfee != bump_fees.end()); BOOST_CHECK_EQUAL(tx7_bumpfee->second, 0); } + // Check linearization order + std::vector miniminer_info; + miniminer_info.emplace_back(/*fee_self=*/low_fee, /*fee_ancestor=*/low_fee,/*vsize_self=*/tx_vsizes[0], /*vsize_ancestor=*/tx_vsizes[0], tx0); + miniminer_info.emplace_back( med_fee, med_fee, tx_vsizes[1], tx_vsizes[1], tx1); + miniminer_info.emplace_back( high_fee, high_fee, tx_vsizes[2], tx_vsizes[2], tx2); + miniminer_info.emplace_back( high_fee, low_fee+med_fee+2*high_fee, tx_vsizes[3], tx_vsizes[0]+tx_vsizes[1]+tx_vsizes[2]+tx_vsizes[3], tx3); + + miniminer_info.emplace_back( high_fee, high_fee, tx_vsizes[4], tx_vsizes[4], tx4); + miniminer_info.emplace_back( low_fee, low_fee + high_fee, tx_vsizes[5], tx_vsizes[4]+tx_vsizes[5], tx5); + miniminer_info.emplace_back( med_fee, high_fee+low_fee+med_fee, tx_vsizes[6], tx_vsizes[4]+tx_vsizes[5]+tx_vsizes[6], tx6); + miniminer_info.emplace_back( high_fee, high_fee+low_fee+high_fee, tx_vsizes[7], tx_vsizes[4]+tx_vsizes[5]+tx_vsizes[7], tx7); + + std::map> descendant_caches; + descendant_caches.emplace(tx0->GetHash(), std::set{tx0->GetHash(), tx3->GetHash()}); + descendant_caches.emplace(tx1->GetHash(), std::set{tx1->GetHash(), tx3->GetHash()}); + descendant_caches.emplace(tx2->GetHash(), std::set{tx2->GetHash(), tx3->GetHash()}); + descendant_caches.emplace(tx3->GetHash(), std::set{tx3->GetHash()}); + descendant_caches.emplace(tx4->GetHash(), std::set{tx4->GetHash(), tx5->GetHash(), tx6->GetHash(), tx7->GetHash()}); + descendant_caches.emplace(tx5->GetHash(), std::set{tx5->GetHash(), tx6->GetHash(), tx7->GetHash()}); + descendant_caches.emplace(tx6->GetHash(), std::set{tx6->GetHash()}); + descendant_caches.emplace(tx7->GetHash(), std::set{tx7->GetHash()}); + + node::MiniMiner miniminer_manual(miniminer_info, descendant_caches); + // Use unspent outpoints to avoid entries being omitted. + 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()}) { + // 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); + + // tx5 + tx7 CPFP + BOOST_CHECK_EQUAL(Find(sequences, tx5->GetHash()), 2); + BOOST_CHECK_EQUAL(Find(sequences, tx7->GetHash()), 2); + + // tx0 and tx1 CPFP'd by tx3 + BOOST_CHECK_EQUAL(Find(sequences, tx0->GetHash()), 3); + BOOST_CHECK_EQUAL(Find(sequences, tx1->GetHash()), 3); + BOOST_CHECK_EQUAL(Find(sequences, tx3->GetHash()), 3); + + // tx6 at medium feerate + BOOST_CHECK_EQUAL(Find(sequences, tx6->GetHash()), 4); + } } BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) { @@ -539,4 +642,64 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) } } +BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup) +{ + CTxMemPool& pool = *Assert(m_node.mempool); + LOCK2(cs_main, pool.cs); + { + // 3 pairs of fee-bumping grandparent + parent, plus 1 low-feerate child. + // 0 fee + high fee + auto grandparent_zero_fee = make_tx({{m_coinbase_txns.at(0)->GetHash(), 0}}, 1); + auto parent_high_feerate = make_tx({{grandparent_zero_fee->GetHash(), 0}}, 1); + // double low fee + med fee + auto grandparent_double_low_feerate = make_tx({{m_coinbase_txns.at(2)->GetHash(), 0}}, 1); + auto parent_med_feerate = make_tx({{grandparent_double_low_feerate->GetHash(), 0}}, 1); + // low fee + double low fee + auto grandparent_low_feerate = make_tx({{m_coinbase_txns.at(1)->GetHash(), 0}}, 1); + auto parent_double_low_feerate = make_tx({{grandparent_low_feerate->GetHash(), 0}}, 1); + // child is below the cpfp package feerates because it is larger than everything else + auto child = make_tx({{parent_high_feerate->GetHash(), 0}, {parent_double_low_feerate->GetHash(), 0}, {parent_med_feerate->GetHash(), 0}}, 1); + + // We artificially record each transaction (except the child) with a uniform vsize of 100vB. + const int64_t tx_vsize{100}; + const int64_t child_vsize{1000}; + + std::vector miniminer_info; + miniminer_info.emplace_back(/*fee_self=*/0, /*fee_ancestor=*/0,/*vsize_self=*/tx_vsize,/*vsize_ancestor=*/tx_vsize, grandparent_zero_fee); + miniminer_info.emplace_back( high_fee, high_fee, tx_vsize, 2*tx_vsize, parent_high_feerate); + miniminer_info.emplace_back( 2*low_fee, 2*low_fee, tx_vsize, tx_vsize, grandparent_double_low_feerate); + miniminer_info.emplace_back( med_fee, 2*low_fee+med_fee, tx_vsize, 2*tx_vsize, parent_med_feerate); + miniminer_info.emplace_back( low_fee, low_fee, tx_vsize, tx_vsize, grandparent_low_feerate); + miniminer_info.emplace_back( 2*low_fee, 3*low_fee, tx_vsize, 2*tx_vsize, parent_double_low_feerate); + miniminer_info.emplace_back( low_fee, high_fee+med_fee+6*low_fee, child_vsize, 6*tx_vsize+child_vsize, child); + std::map> descendant_caches; + descendant_caches.emplace(grandparent_zero_fee->GetHash(), std::set{grandparent_zero_fee->GetHash(), parent_high_feerate->GetHash(), child->GetHash()}); + descendant_caches.emplace(grandparent_low_feerate->GetHash(), std::set{grandparent_low_feerate->GetHash(), parent_double_low_feerate->GetHash(), child->GetHash()}); + descendant_caches.emplace(grandparent_double_low_feerate->GetHash(), std::set{grandparent_double_low_feerate->GetHash(), parent_med_feerate->GetHash(), child->GetHash()}); + descendant_caches.emplace(parent_high_feerate->GetHash(), std::set{parent_high_feerate->GetHash(), child->GetHash()}); + descendant_caches.emplace(parent_med_feerate->GetHash(), std::set{parent_med_feerate->GetHash(), child->GetHash()}); + descendant_caches.emplace(parent_double_low_feerate->GetHash(), std::set{parent_double_low_feerate->GetHash(), child->GetHash()}); + descendant_caches.emplace(child->GetHash(), std::set{child->GetHash()}); + + node::MiniMiner miniminer_manual(miniminer_info, descendant_caches); + BOOST_CHECK(miniminer_manual.IsReadyToCalculate()); + const auto sequences{miniminer_manual.Linearize()}; + + // CPFP zero + high + BOOST_CHECK_EQUAL(sequences.at(grandparent_zero_fee->GetHash()), 0); + BOOST_CHECK_EQUAL(sequences.at(parent_high_feerate->GetHash()), 0); + + // CPFP double low + med + BOOST_CHECK_EQUAL(sequences.at(grandparent_double_low_feerate->GetHash()), 1); + BOOST_CHECK_EQUAL(sequences.at(parent_med_feerate->GetHash()), 1); + + // CPFP low + med + BOOST_CHECK_EQUAL(sequences.at(grandparent_low_feerate->GetHash()), 2); + BOOST_CHECK_EQUAL(sequences.at(parent_double_low_feerate->GetHash()), 2); + + // Child at the end + BOOST_CHECK_EQUAL(sequences.at(child->GetHash()), 3); + } +} + BOOST_AUTO_TEST_SUITE_END()