From 56484f0fdc44261e723563f59df886d5acdd851f Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 21 Feb 2022 11:48:10 +0000 Subject: [PATCH 1/4] =?UTF-8?q?[mempool]=20find=20connected=20mempool=20en?= =?UTF-8?q?tries=20with=20GatherClusters(=E2=80=A6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We limit GatherClusters’s result to a maximum of 500 transactions as clusters can be made arbitrarily large by third parties. Co-authored-by: Murch --- src/txmempool.cpp | 42 +++++++++++++++++++++++++++++++++++++++++- src/txmempool.h | 15 ++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 378123ce0fe..2bac419f84c 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -898,6 +899,19 @@ CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set& hashes) c return ret; } +std::vector CTxMemPool::GetIterVec(const std::vector& txids) const +{ + AssertLockHeld(cs); + std::vector ret; + ret.reserve(txids.size()); + for (const auto& txid : txids) { + const auto it{GetIter(txid)}; + if (!it) return {}; + ret.push_back(*it); + } + return ret; +} + bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const { for (unsigned int i = 0; i < tx.vin.size(); i++) @@ -1127,7 +1141,6 @@ void CTxMemPool::SetLoadTried(bool load_tried) m_load_tried = load_tried; } - std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept { switch (r) { @@ -1140,3 +1153,30 @@ std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept } assert(false); } + +std::vector CTxMemPool::GatherClusters(const std::vector& txids) const +{ + AssertLockHeld(cs); + std::vector clustered_txs{GetIterVec(txids)}; + // Use epoch: visiting an entry means we have added it to the clustered_txs vector. It does not + // necessarily mean the entry has been processed. + WITH_FRESH_EPOCH(m_epoch); + for (const auto& it : clustered_txs) { + visited(it); + } + // i = index of where the list of entries to process starts + for (size_t i{0}; i < clustered_txs.size(); ++i) { + // DoS protection: if there are 500 or more entries to process, just quit. + if (clustered_txs.size() > 500) return {}; + const txiter& tx_iter = clustered_txs.at(i); + for (const auto& entries : {tx_iter->GetMemPoolParentsConst(), tx_iter->GetMemPoolChildrenConst()}) { + for (const CTxMemPoolEntry& entry : entries) { + const auto entry_it = mapTx.iterator_to(entry); + if (!visited(entry_it)) { + clustered_txs.push_back(entry_it); + } + } + } + } + return clustered_txs; +} diff --git a/src/txmempool.h b/src/txmempool.h index 2c3cb7e9dbd..769b7f69eac 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -522,9 +522,16 @@ public: /** Returns an iterator to the given hash, if found */ std::optional GetIter(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs); - /** Translate a set of hashes into a set of pool iterators to avoid repeated lookups */ + /** Translate a set of hashes into a set of pool iterators to avoid repeated lookups. + * Does not require that all of the hashes correspond to actual transactions in the mempool, + * only returns the ones that exist. */ setEntries GetIterSet(const std::set& hashes) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Translate a list of hashes into a list of mempool iterators to avoid repeated lookups. + * The nth element in txids becomes the nth element in the returned vector. If any of the txids + * don't actually exist in the mempool, returns an empty vector. */ + std::vector GetIterVec(const std::vector& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Remove a set of transactions from the mempool. * If a transaction is in this set, then all in-mempool descendants must * also be in the set, unless this transaction is being removed for being @@ -585,6 +592,12 @@ public: const Limits& limits, bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Collect the entire cluster of connected transactions for each transaction in txids. + * All txids must correspond to transaction entries in the mempool, otherwise this returns an + * empty vector. This call will also exit early and return an empty vector if it collects 500 or + * more transactions as a DoS protection. */ + std::vector GatherClusters(const std::vector& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and * check ancestor and descendant limits. Heuristics are used to estimate the ancestor and * descendant count of all entries if the package were to be added to the mempool. The limits From 59afcc83548ea67a863dac7b75d000bc8f6a7023 Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 4 Aug 2022 15:37:50 +0100 Subject: [PATCH 2/4] Implement Mini version of BlockAssembler to calculate mining scores Rewrite the same algo instead of reusing BlockAssembler because we have a few extra requirements that would make the changes invasive and difficult to review: - Only operate on the relevant transactions rather than full mempool - Remove transactions that will be replaced so they can't bump their ancestors - Don't hold mempool lock outside of the constructor - Skip things like max block weight and IsFinalTx - Additionally calculate fees to bump remaining ancestor packages to target feerate Co-authored-by: Murch --- src/Makefile.am | 2 + src/node/mini_miner.cpp | 366 ++++++++++++++++++++++++++++++++++++++++ src/node/mini_miner.h | 121 +++++++++++++ 3 files changed, 489 insertions(+) create mode 100644 src/node/mini_miner.cpp create mode 100644 src/node/mini_miner.h diff --git a/src/Makefile.am b/src/Makefile.am index 5830090ada0..3f68ac03f09 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -211,6 +211,7 @@ BITCOIN_CORE_H = \ node/mempool_args.h \ node/mempool_persist_args.h \ node/miner.h \ + node/mini_miner.h \ node/minisketchwrapper.h \ node/psbt.h \ node/transaction.h \ @@ -396,6 +397,7 @@ libbitcoin_node_a_SOURCES = \ node/mempool_args.cpp \ node/mempool_persist_args.cpp \ node/miner.cpp \ + node/mini_miner.cpp \ node/minisketchwrapper.cpp \ node/psbt.cpp \ node/transaction.cpp \ diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp new file mode 100644 index 00000000000..71ae9d23c79 --- /dev/null +++ b/src/node/mini_miner.cpp @@ -0,0 +1,366 @@ +// Copyright (c) 2023 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 + +#include +#include +#include + +namespace node { + +MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector& outpoints) +{ + LOCK(mempool.cs); + // Find which outpoints to calculate bump fees for. + // Anything that's spent by the mempool is to-be-replaced + // Anything otherwise unavailable just has a bump fee of 0 + for (const auto& outpoint : outpoints) { + if (!mempool.exists(GenTxid::Txid(outpoint.hash))) { + // This UTXO is either confirmed or not yet submitted to mempool. + // If it's confirmed, no bump fee is required. + // If it's not yet submitted, we have no information, so return 0. + m_bump_fees.emplace(outpoint, 0); + continue; + } + + // UXTO is created by transaction in mempool, add to map. + // Note: This will either create a missing entry or add the outpoint to an existing entry + m_requested_outpoints_by_txid[outpoint.hash].push_back(outpoint); + + if (const auto ptx{mempool.GetConflictTx(outpoint)}) { + // This outpoint is already being spent by another transaction in the mempool. We + // assume that the caller wants to replace this transaction and its descendants. It + // would be unusual for the transaction to have descendants as the wallet won’t normally + // attempt to replace transactions with descendants. If the outpoint is from a mempool + // transaction, we still need to calculate its ancestors bump fees (added to + // m_requested_outpoints_by_txid below), but after removing the to-be-replaced entries. + // + // Note that the descendants of a transaction include the transaction itself. Also note, + // that this is only calculating bump fees. RBF fee rules should be handled separately. + CTxMemPool::setEntries descendants; + mempool.CalculateDescendants(mempool.GetIter(ptx->GetHash()).value(), descendants); + for (const auto& desc_txiter : descendants) { + m_to_be_replaced.insert(desc_txiter->GetTx().GetHash()); + } + } + } + + // No unconfirmed UTXOs, so nothing mempool-related needs to be calculated. + if (m_requested_outpoints_by_txid.empty()) return; + + // Calculate the cluster and construct the entry map. + std::vector txids_needed; + txids_needed.reserve(m_requested_outpoints_by_txid.size()); + for (const auto& [txid, _]: m_requested_outpoints_by_txid) { + txids_needed.push_back(txid); + } + const auto cluster = mempool.GatherClusters(txids_needed); + if (cluster.empty()) { + // An empty cluster means that at least one of the transactions is missing from the mempool + // (should not be possible given processing above) or DoS limit was hit. + m_ready_to_calculate = false; + return; + } + + // 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)); + m_entries.push_back(mapiter); + } else { + auto outpoints_it = m_requested_outpoints_by_txid.find(txiter->GetTx().GetHash()); + if (outpoints_it != m_requested_outpoints_by_txid.end()) { + // This UTXO is the output of a to-be-replaced transaction. Bump fee is 0; spending + // this UTXO is impossible as it will no longer exist after the replacement. + for (const auto& outpoint : outpoints_it->second) { + m_bump_fees.emplace(outpoint, 0); + } + m_requested_outpoints_by_txid.erase(outpoints_it); + } + } + } + + // Build the m_descendant_set_by_txid cache. + for (const auto& txiter : cluster) { + const auto& txid = txiter->GetTx().GetHash(); + // Cache descendants for future use. Unlike the real mempool, a descendant MiniMinerMempoolEntry + // will not exist without its ancestor MiniMinerMempoolEntry, so these sets won't be invalidated. + std::vector cached_descendants; + const bool remove{m_to_be_replaced.count(txid) > 0}; + CTxMemPool::setEntries descendants; + mempool.CalculateDescendants(txiter, descendants); + Assume(descendants.count(txiter) > 0); + for (const auto& desc_txiter : descendants) { + const auto txid_desc = desc_txiter->GetTx().GetHash(); + const bool remove_desc{m_to_be_replaced.count(txid_desc) > 0}; + auto desc_it{m_entries_by_txid.find(txid_desc)}; + Assume((desc_it == m_entries_by_txid.end()) == remove_desc); + if (remove) Assume(remove_desc); + // It's possible that remove=false but remove_desc=true. + if (!remove && !remove_desc) { + cached_descendants.push_back(desc_it); + } + } + if (remove) { + Assume(cached_descendants.empty()); + } else { + m_descendant_set_by_txid.emplace(txid, cached_descendants); + } + } + + // Release the mempool lock; we now have all the information we need for a subset of the entries + // we care about. We will solely operate on the MiniMinerMempoolEntry map from now on. + Assume(m_in_block.empty()); + Assume(m_requested_outpoints_by_txid.size() <= outpoints.size()); + 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 +// parents do not incentive inclusion of their children. Therefore the mining algorithm only considers +// transactions for inclusion on basis of the minimum of their own feerate or their ancestor feerate. +struct AncestorFeerateComparator +{ + template + bool operator()(const I& a, const I& b) const { + auto min_feerate = [](const MiniMinerMempoolEntry& e) -> CFeeRate { + const CAmount ancestor_fee{e.GetModFeesWithAncestors()}; + const int64_t ancestor_size{e.GetSizeWithAncestors()}; + const CAmount tx_fee{e.GetModifiedFee()}; + const int64_t tx_size{e.GetTxSize()}; + // Comparing ancestor feerate with individual feerate: + // ancestor_fee / ancestor_size <= tx_fee / tx_size + // Avoid division and possible loss of precision by + // multiplying both sides by the sizes: + return ancestor_fee * tx_size < tx_fee * ancestor_size ? + CFeeRate(ancestor_fee, ancestor_size) : + CFeeRate(tx_fee, tx_size); + }; + CFeeRate a_feerate{min_feerate(a->second)}; + CFeeRate b_feerate{min_feerate(b->second)}; + if (a_feerate != b_feerate) { + return a_feerate > b_feerate; + } + // Use txid as tiebreaker for stable sorting + return a->first < b->first; + } +}; + +void MiniMiner::DeleteAncestorPackage(const std::set& ancestors) +{ + Assume(ancestors.size() >= 1); + // "Mine" all transactions in this ancestor set. + for (auto& anc : ancestors) { + Assume(m_in_block.count(anc->first) == 0); + m_in_block.insert(anc->first); + m_total_fees += anc->second.GetModifiedFee(); + m_total_vsize += anc->second.GetTxSize(); + auto it = m_descendant_set_by_txid.find(anc->first); + // Each entry’s descendant set includes itself + Assume(it != m_descendant_set_by_txid.end()); + for (auto& descendant : it->second) { + // If these fail, we must be double-deducting. + Assume(descendant->second.GetModFeesWithAncestors() >= anc->second.GetModifiedFee()); + Assume(descendant->second.vsize_with_ancestors >= anc->second.GetTxSize()); + descendant->second.fee_with_ancestors -= anc->second.GetModifiedFee(); + descendant->second.vsize_with_ancestors -= anc->second.GetTxSize(); + } + } + // Delete these entries. + for (const auto& anc : ancestors) { + m_descendant_set_by_txid.erase(anc->first); + // The above loop should have deducted each ancestor's size and fees from each of their + // respective descendants exactly once. + Assume(anc->second.GetModFeesWithAncestors() == 0); + Assume(anc->second.GetSizeWithAncestors() == 0); + auto vec_it = std::find(m_entries.begin(), m_entries.end(), anc); + Assume(vec_it != m_entries.end()); + m_entries.erase(vec_it); + m_entries_by_txid.erase(anc); + } +} + +void MiniMiner::SanityCheck() const +{ + // m_entries, m_entries_by_txid, and m_descendant_set_by_txid all same size + Assume(m_entries.size() == m_entries_by_txid.size()); + Assume(m_entries.size() == m_descendant_set_by_txid.size()); + // Cached ancestor values should be at least as large as the transaction's own fee and size + Assume(std::all_of(m_entries.begin(), m_entries.end(), [](const auto& entry) { + return entry->second.GetSizeWithAncestors() >= entry->second.GetTxSize() && + entry->second.GetModFeesWithAncestors() >= entry->second.GetModifiedFee();})); + // None of the entries should be to-be-replaced transactions + Assume(std::all_of(m_to_be_replaced.begin(), m_to_be_replaced.end(), + [&](const auto& txid){return m_entries_by_txid.find(txid) == m_entries_by_txid.end();})); +} + +void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate) +{ + 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()); + + // Pick highest ancestor feerate entry. + auto best_iter = m_entries.begin(); + Assume(best_iter != m_entries.end()); + 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)) { + break; + } + + // 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; + { + std::set to_process; + to_process.insert(*best_iter); + while (!to_process.empty()) { + auto iter = to_process.begin(); + Assume(iter != to_process.end()); + ancestors.insert(*iter); + for (const auto& input : (*iter)->second.GetTx().vin) { + if (auto parent_it{m_entries_by_txid.find(input.prevout.hash)}; parent_it != m_entries_by_txid.end()) { + if (ancestors.count(parent_it) == 0) { + to_process.insert(parent_it); + } + } + } + to_process.erase(iter); + } + } + DeleteAncestorPackage(ancestors); + SanityCheck(); + } + 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; +} + +std::map MiniMiner::CalculateBumpFees(const CFeeRate& target_feerate) +{ + if (!m_ready_to_calculate) return {}; + // Build a block template until the target feerate is hit. + BuildMockTemplate(target_feerate); + + // Each transaction that "made it into the block" has a bumpfee of 0, i.e. they are part of an + // ancestor package with at least the target feerate and don't need to be bumped. + for (const auto& txid : m_in_block) { + // Not all of the block transactions were necessarily requested. + auto it = m_requested_outpoints_by_txid.find(txid); + if (it != m_requested_outpoints_by_txid.end()) { + for (const auto& outpoint : it->second) { + m_bump_fees.emplace(outpoint, 0); + } + m_requested_outpoints_by_txid.erase(it); + } + } + + // A transactions and its ancestors will only be picked into a block when + // both the ancestor set feerate and the individual feerate meet the target + // feerate. + // + // We had to convince ourselves that after running the mini miner and + // picking all eligible transactions into our MockBlockTemplate, there + // could still be transactions remaining that have a lower individual + // feerate than their ancestor feerate. So here is an example: + // + // ┌─────────────────┐ + // │ │ + // │ Grandparent │ + // │ 1700 vB │ + // │ 1700 sats │ Target feerate: 10 s/vB + // │ 1 s/vB │ GP Ancestor Set Feerate (ASFR): 1 s/vB + // │ │ P1_ASFR: 9.84 s/vB + // └──────▲───▲──────┘ P2_ASFR: 2.47 s/vB + // │ │ C_ASFR: 10.27 s/vB + // ┌───────────────┐ │ │ ┌──────────────┐ + // │ ├────┘ └────┤ │ ⇒ C_FR < TFR < C_ASFR + // │ Parent 1 │ │ Parent 2 │ + // │ 200 vB │ │ 200 vB │ + // │ 17000 sats │ │ 3000 sats │ + // │ 85 s/vB │ │ 15 s/vB │ + // │ │ │ │ + // └───────────▲───┘ └───▲──────────┘ + // │ │ + // │ ┌───────────┐ │ + // └────┤ ├────┘ + // │ Child │ + // │ 100 vB │ + // │ 900 sats │ + // │ 9 s/vB │ + // │ │ + // └───────────┘ + // + // We therefore calculate both the bump fee that is necessary to elevate + // the individual transaction to the target feerate: + // target_feerate × tx_size - tx_fees + // and the bump fee that is necessary to bump the entire ancestor set to + // the target feerate: + // target_feerate × ancestor_set_size - ancestor_set_fees + // By picking the maximum from the two, we ensure that a transaction meets + // both criteria. + for (const auto& [txid, outpoints] : m_requested_outpoints_by_txid) { + auto it = m_entries_by_txid.find(txid); + Assume(it != m_entries_by_txid.end()); + if (it != m_entries_by_txid.end()) { + Assume(target_feerate.GetFee(it->second.GetSizeWithAncestors()) > std::min(it->second.GetModifiedFee(), it->second.GetModFeesWithAncestors())); + CAmount bump_fee_with_ancestors = target_feerate.GetFee(it->second.GetSizeWithAncestors()) - it->second.GetModFeesWithAncestors(); + CAmount bump_fee_individual = target_feerate.GetFee(it->second.GetTxSize()) - it->second.GetModifiedFee(); + const CAmount bump_fee{std::max(bump_fee_with_ancestors, bump_fee_individual)}; + Assume(bump_fee >= 0); + for (const auto& outpoint : outpoints) { + m_bump_fees.emplace(outpoint, bump_fee); + } + } + } + return m_bump_fees; +} + +std::optional MiniMiner::CalculateTotalBumpFees(const CFeeRate& target_feerate) +{ + if (!m_ready_to_calculate) return std::nullopt; + // Build a block template until the target feerate is hit. + BuildMockTemplate(target_feerate); + + // All remaining ancestors that are not part of m_in_block must be bumped, but no other relatives + std::set ancestors; + std::set to_process; + for (const auto& [txid, outpoints] : m_requested_outpoints_by_txid) { + // Skip any ancestors that already have a miner score higher than the target feerate + // (already "made it" into the block) + if (m_in_block.count(txid)) continue; + auto iter = m_entries_by_txid.find(txid); + if (iter == m_entries_by_txid.end()) continue; + to_process.insert(iter); + ancestors.insert(iter); + } + while (!to_process.empty()) { + auto iter = to_process.begin(); + const CTransaction& tx = (*iter)->second.GetTx(); + for (const auto& input : tx.vin) { + if (auto parent_it{m_entries_by_txid.find(input.prevout.hash)}; parent_it != m_entries_by_txid.end()) { + to_process.insert(parent_it); + ancestors.insert(parent_it); + } + } + to_process.erase(iter); + } + const auto ancestor_package_size = std::accumulate(ancestors.cbegin(), ancestors.cend(), int64_t{0}, + [](int64_t sum, const auto it) {return sum + it->second.GetTxSize();}); + const auto ancestor_package_fee = std::accumulate(ancestors.cbegin(), ancestors.cend(), CAmount{0}, + [](CAmount sum, const auto it) {return sum + it->second.GetModifiedFee();}); + return target_feerate.GetFee(ancestor_package_size) - ancestor_package_fee; +} +} // namespace node diff --git a/src/node/mini_miner.h b/src/node/mini_miner.h new file mode 100644 index 00000000000..db07e6d1bf0 --- /dev/null +++ b/src/node/mini_miner.h @@ -0,0 +1,121 @@ +// Copyright (c) 2022 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_NODE_MINI_MINER_H +#define BITCOIN_NODE_MINI_MINER_H + +#include + +#include +#include +#include + +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; + +// This class must be constructed while holding mempool.cs. After construction, the object's +// methods can be called without holding that lock. +public: + CAmount fee_with_ancestors; + int64_t vsize_with_ancestors; + 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()) + { } + + CAmount GetModifiedFee() const { return fee_individual; } + CAmount GetModFeesWithAncestors() const { return fee_with_ancestors; } + int64_t GetTxSize() const { return vsize_individual; } + int64_t GetSizeWithAncestors() const { return vsize_with_ancestors; } + const CTransaction& GetTx() const LIFETIMEBOUND { return *tx; } +}; + +// Comparator needed for std::set +struct IteratorComparator +{ + template + bool operator()(const I& a, const I& b) const + { + return &(*a) < &(*b); + } +}; + +/** 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. */ +class MiniMiner +{ + // When true, a caller may use CalculateBumpFees(). Becomes false if we failed to retrieve + // mempool entries (i.e. cluster size too large) or bump fees have already been calculated. + bool m_ready_to_calculate{true}; + + // Set once per lifetime, fill in during initialization. + // txids of to-be-replaced transactions + std::set m_to_be_replaced; + + // If multiple argument outpoints correspond to the same transaction, cache them together in + // a single entry indexed by txid. Then we can just work with txids since all outpoints from + // 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. + std::map m_bump_fees; + + // The constructed block template + std::set m_in_block; + + // Information on the current status of the block + CAmount m_total_fees{0}; + int32_t m_total_vsize{0}; + + /** Main data structure holding the entries, can be indexed by txid */ + std::map m_entries_by_txid; + using MockEntryMap = decltype(m_entries_by_txid); + + /** Vector of entries, can be sorted by ancestor feerate. */ + std::vector m_entries; + + /** Map of txid to its descendants. Should be inclusive. */ + std::map> m_descendant_set_by_txid; + + /** Consider this ancestor package "mined" so remove all these entries from our data structures. */ + void DeleteAncestorPackage(const std::set& ancestors); + + /** Perform some checks. */ + void SanityCheck() const; + +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); + + /** Returns set of txids in the block template if one has been constructed. */ + std::set GetMockTemplateTxids() const { return m_in_block; } + + MiniMiner(const CTxMemPool& mempool, const std::vector& outpoints); + + /** 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 + * if they cannot be calculated. */ + std::map CalculateBumpFees(const CFeeRate& target_feerate); + + /** Construct a new block template and, calculate the cost of bumping all transactions that did + * 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 + +#endif // BITCOIN_NODE_MINI_MINER_H From 3f3f2d59ea2946a7b7cc8cb0222fb602d62645d0 Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 11 Aug 2022 09:58:39 +0100 Subject: [PATCH 3/4] [unit test] GatherClusters and MiniMiner unit tests Co-authored-by: Murch Co-authored-by: theStack Co-authored-by: furszy --- src/Makefile.test.include | 1 + src/test/miniminer_tests.cpp | 477 +++++++++++++++++++++++++++++++++++ 2 files changed, 478 insertions(+) create mode 100644 src/test/miniminer_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index d6992640ff2..3e9d6ad9e3d 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -106,6 +106,7 @@ BITCOIN_TESTS =\ test/merkle_tests.cpp \ test/merkleblock_tests.cpp \ test/miner_tests.cpp \ + test/miniminer_tests.cpp \ test/miniscript_tests.cpp \ test/minisketch_tests.cpp \ test/multisig_tests.cpp \ diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp new file mode 100644 index 00000000000..3f4a5fbe747 --- /dev/null +++ b/src/test/miniminer_tests.cpp @@ -0,0 +1,477 @@ +// Copyright (c) 2021 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 +#include +#include + +BOOST_FIXTURE_TEST_SUITE(miniminer_tests, TestingSetup) + +static inline CTransactionRef make_tx(const std::vector& inputs, size_t num_outputs) +{ + CMutableTransaction tx = CMutableTransaction(); + tx.vin.resize(inputs.size()); + tx.vout.resize(num_outputs); + for (size_t i = 0; i < inputs.size(); ++i) { + tx.vin[i].prevout = inputs[i]; + } + for (size_t i = 0; i < num_outputs; ++i) { + tx.vout[i].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + // The actual input and output values of these transactions don't really + // matter, since all accounting will use the entries' cached fees. + tx.vout[i].nValue = COIN; + } + return MakeTransactionRef(tx); +} + +static inline bool sanity_check(const std::vector& transactions, + const std::map& bumpfees) +{ + // No negative bumpfees. + for (const auto& [outpoint, fee] : bumpfees) { + if (fee < 0) return false; + if (fee == 0) continue; + auto outpoint_ = outpoint; // structured bindings can't be captured in C++17, so we need to use a variable + const bool found = std::any_of(transactions.cbegin(), transactions.cend(), [&](const auto& tx) { + return outpoint_.hash == tx->GetHash() && outpoint_.n < tx->vout.size(); + }); + if (!found) return false; + } + for (const auto& tx : transactions) { + // If tx has multiple outputs, they must all have the same bumpfee (if they exist). + if (tx->vout.size() > 1) { + std::set distinct_bumpfees; + for (size_t i{0}; i < tx->vout.size(); ++i) { + const auto bumpfee = bumpfees.find(COutPoint{tx->GetHash(), static_cast(i)}); + if (bumpfee != bumpfees.end()) distinct_bumpfees.insert(bumpfee->second); + } + if (distinct_bumpfees.size() > 1) return false; + } + } + return true; +} + +template +Value Find(const std::map& map, const Key& key) +{ + auto it = map.find(key); + BOOST_CHECK_MESSAGE(it != map.end(), strprintf("Cannot find %s", key.ToString())); + return it->second; +} + +BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) +{ + CTxMemPool& pool = *Assert(m_node.mempool); + 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 tx1 and child tx2 with normal fees: + const auto tx1 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx1)); + const auto tx2 = make_tx({COutPoint{tx1->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx2)); + + // Create a low-feerate parent tx3 and high-feerate child tx4 (cpfp) + const auto tx3 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx3)); + const auto tx4 = make_tx({COutPoint{tx3->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx4)); + + // Create a parent tx5 and child tx6 where both have low fees + const auto tx5 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx5)); + const auto tx6 = make_tx({COutPoint{tx5->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx6)); + // Make tx6'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(tx6->GetHash(), CENT/100); + + // Create a high-feerate parent tx7, low-feerate child tx8 + const auto tx7 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx7)); + const auto tx8 = make_tx({COutPoint{tx7->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx8)); + + std::vector all_unspent_outpoints({ + COutPoint{tx1->GetHash(), 1}, + COutPoint{tx2->GetHash(), 0}, + COutPoint{tx3->GetHash(), 1}, + COutPoint{tx4->GetHash(), 0}, + COutPoint{tx5->GetHash(), 1}, + COutPoint{tx6->GetHash(), 0}, + COutPoint{tx7->GetHash(), 1}, + COutPoint{tx8->GetHash(), 0} + }); + for (const auto& outpoint : all_unspent_outpoints) BOOST_CHECK(!pool.isSpent(outpoint)); + + std::vector all_spent_outpoints({ + COutPoint{tx1->GetHash(), 0}, + COutPoint{tx3->GetHash(), 0}, + COutPoint{tx5->GetHash(), 0}, + COutPoint{tx7->GetHash(), 0} + }); + for (const auto& outpoint : all_spent_outpoints) BOOST_CHECK(pool.GetConflictTx(outpoint) != nullptr); + + std::vector all_parent_outputs({ + COutPoint{tx1->GetHash(), 0}, + COutPoint{tx1->GetHash(), 1}, + COutPoint{tx3->GetHash(), 0}, + COutPoint{tx3->GetHash(), 1}, + COutPoint{tx5->GetHash(), 0}, + COutPoint{tx5->GetHash(), 1}, + COutPoint{tx7->GetHash(), 0}, + COutPoint{tx7->GetHash(), 1} + }); + + + std::vector all_transactions{tx1, tx2, tx3, tx4, tx5, tx6, tx7, tx8}; + struct TxDimensions { + size_t vsize; CAmount mod_fee; CFeeRate feerate; + }; + std::map tx_dims; + for (const auto& tx : all_transactions) { + const auto it = pool.GetIter(tx->GetHash()).value(); + tx_dims.emplace(tx->GetHash(), TxDimensions{it->GetTxSize(), it->GetModifiedFee(), + CFeeRate(it->GetModifiedFee(), it->GetTxSize())}); + } + + const std::vector various_normal_feerates({CFeeRate(0), CFeeRate(500), CFeeRate(999), + CFeeRate(1000), CFeeRate(2000), CFeeRate(2500), + CFeeRate(3333), CFeeRate(7800), CFeeRate(11199), + CFeeRate(23330), CFeeRate(50000), CFeeRate(5*CENT)}); + + // All nonexistent entries have a bumpfee of zero, regardless of feerate + std::vector nonexistent_outpoints({ COutPoint{GetRandHash(), 0}, COutPoint{GetRandHash(), 3} }); + for (const auto& outpoint : nonexistent_outpoints) BOOST_CHECK(!pool.isSpent(outpoint)); + for (const auto& feerate : various_normal_feerates) { + node::MiniMiner mini_miner(pool, nonexistent_outpoints); + BOOST_CHECK(mini_miner.IsReadyToCalculate()); + auto bump_fees = mini_miner.CalculateBumpFees(feerate); + BOOST_CHECK(!mini_miner.IsReadyToCalculate()); + BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + BOOST_CHECK(bump_fees.size() == nonexistent_outpoints.size()); + for (const auto& outpoint: nonexistent_outpoints) { + auto it = bump_fees.find(outpoint); + BOOST_CHECK(it != bump_fees.end()); + BOOST_CHECK_EQUAL(it->second, 0); + } + } + + // Gather bump fees for all available UTXOs. + for (const auto& target_feerate : various_normal_feerates) { + node::MiniMiner mini_miner(pool, all_unspent_outpoints); + BOOST_CHECK(mini_miner.IsReadyToCalculate()); + auto bump_fees = mini_miner.CalculateBumpFees(target_feerate); + BOOST_CHECK(!mini_miner.IsReadyToCalculate()); + BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); + + // Check tx1 bumpfee: no other bumper. + const TxDimensions& tx1_dimensions = tx_dims.find(tx1->GetHash())->second; + CAmount bumpfee1 = Find(bump_fees, COutPoint{tx1->GetHash(), 1}); + if (target_feerate <= tx1_dimensions.feerate) { + BOOST_CHECK_EQUAL(bumpfee1, 0); + } else { + // Difference is fee to bump tx1 from current to target feerate. + BOOST_CHECK_EQUAL(bumpfee1, target_feerate.GetFee(tx1_dimensions.vsize) - tx1_dimensions.mod_fee); + } + + // Check tx3 bumpfee: assisted by tx4. + const TxDimensions& tx3_dimensions = tx_dims.find(tx3->GetHash())->second; + const TxDimensions& tx4_dimensions = tx_dims.find(tx4->GetHash())->second; + const CFeeRate tx3_feerate = CFeeRate(tx3_dimensions.mod_fee + tx4_dimensions.mod_fee, tx3_dimensions.vsize + tx4_dimensions.vsize); + CAmount bumpfee3 = Find(bump_fees, COutPoint{tx3->GetHash(), 1}); + if (target_feerate <= tx3_feerate) { + // As long as target feerate is below tx4's ancestor feerate, there is no bump fee. + BOOST_CHECK_EQUAL(bumpfee3, 0); + } else { + // Difference is fee to bump tx3 from current to target feerate, without tx4. + BOOST_CHECK_EQUAL(bumpfee3, target_feerate.GetFee(tx3_dimensions.vsize) - tx3_dimensions.mod_fee); + } + + // If tx6’s modified fees are sufficient for tx5 and tx6 to be picked + // into the block, our prospective new transaction would not need to + // bump tx5 when using tx5’s second output. If however even tx6’s + // modified fee (which essentially indicates "effective feerate") is + // not sufficient to bump tx5, using the second output of tx5 would + // require our transaction to bump tx5 from scratch since we evaluate + // transaction packages per ancestor sets and do not consider multiple + // children’s fees. + const TxDimensions& tx5_dimensions = tx_dims.find(tx5->GetHash())->second; + const TxDimensions& tx6_dimensions = tx_dims.find(tx6->GetHash())->second; + const CFeeRate tx5_feerate = CFeeRate(tx5_dimensions.mod_fee + tx6_dimensions.mod_fee, tx5_dimensions.vsize + tx6_dimensions.vsize); + CAmount bumpfee5 = Find(bump_fees, COutPoint{tx5->GetHash(), 1}); + if (target_feerate <= tx5_feerate) { + // As long as target feerate is below tx6's ancestor feerate, there is no bump fee. + BOOST_CHECK_EQUAL(bumpfee5, 0); + } else { + // Difference is fee to bump tx5 from current to target feerate, without tx6. + BOOST_CHECK_EQUAL(bumpfee5, target_feerate.GetFee(tx5_dimensions.vsize) - tx5_dimensions.mod_fee); + } + } + // Spent outpoints should usually not be requested as they would not be + // considered available. However, when they are explicitly requested, we + // can calculate their bumpfee to facilitate RBF-replacements + for (const auto& target_feerate : various_normal_feerates) { + node::MiniMiner mini_miner_all_spent(pool, all_spent_outpoints); + BOOST_CHECK(mini_miner_all_spent.IsReadyToCalculate()); + auto bump_fees_all_spent = mini_miner_all_spent.CalculateBumpFees(target_feerate); + BOOST_CHECK(!mini_miner_all_spent.IsReadyToCalculate()); + BOOST_CHECK_EQUAL(bump_fees_all_spent.size(), all_spent_outpoints.size()); + node::MiniMiner mini_miner_all_parents(pool, all_parent_outputs); + BOOST_CHECK(mini_miner_all_parents.IsReadyToCalculate()); + auto bump_fees_all_parents = mini_miner_all_parents.CalculateBumpFees(target_feerate); + BOOST_CHECK(!mini_miner_all_parents.IsReadyToCalculate()); + BOOST_CHECK_EQUAL(bump_fees_all_parents.size(), all_parent_outputs.size()); + for (auto& bump_fees : {bump_fees_all_parents, bump_fees_all_spent}) { + // For all_parents case, both outputs from the parent should have the same bump fee, + // even though only one of them is in a to-be-replaced transaction. + BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + + // Check tx1 bumpfee: no other bumper. + const TxDimensions& tx1_dimensions = tx_dims.find(tx1->GetHash())->second; + CAmount it1_spent = Find(bump_fees, COutPoint{tx1->GetHash(), 0}); + if (target_feerate <= tx1_dimensions.feerate) { + BOOST_CHECK_EQUAL(it1_spent, 0); + } else { + // Difference is fee to bump tx1 from current to target feerate. + BOOST_CHECK_EQUAL(it1_spent, target_feerate.GetFee(tx1_dimensions.vsize) - tx1_dimensions.mod_fee); + } + + // Check tx3 bumpfee: no other bumper, because tx4 is to-be-replaced. + const TxDimensions& tx3_dimensions = tx_dims.find(tx3->GetHash())->second; + const CFeeRate tx3_feerate_unbumped = tx3_dimensions.feerate; + auto it3_spent = Find(bump_fees, COutPoint{tx3->GetHash(), 0}); + if (target_feerate <= tx3_feerate_unbumped) { + BOOST_CHECK_EQUAL(it3_spent, 0); + } else { + // Difference is fee to bump tx3 from current to target feerate, without tx4. + BOOST_CHECK_EQUAL(it3_spent, target_feerate.GetFee(tx3_dimensions.vsize) - tx3_dimensions.mod_fee); + } + + // Check tx5 bumpfee: no other bumper, because tx6 is to-be-replaced. + const TxDimensions& tx5_dimensions = tx_dims.find(tx5->GetHash())->second; + const CFeeRate tx5_feerate_unbumped = tx5_dimensions.feerate; + auto it5_spent = Find(bump_fees, COutPoint{tx5->GetHash(), 0}); + if (target_feerate <= tx5_feerate_unbumped) { + BOOST_CHECK_EQUAL(it5_spent, 0); + } else { + // Difference is fee to bump tx5 from current to target feerate, without tx6. + BOOST_CHECK_EQUAL(it5_spent, target_feerate.GetFee(tx5_dimensions.vsize) - tx5_dimensions.mod_fee); + } + } + } +} + +BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) +{ + CTxMemPool& pool = *Assert(m_node.mempool); + LOCK2(::cs_main, pool.cs); + TestMemPoolEntryHelper entry; + + const CAmount low_fee{CENT/2000}; + const CAmount med_fee{CENT/200}; + const CAmount high_fee{CENT/10}; + + // Create 3 parents of different feerates, and 1 child spending from all 3. + const auto tx1 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx1)); + const auto tx2 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(med_fee).FromTx(tx2)); + const auto tx3 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx3)); + const auto tx4 = make_tx({COutPoint{tx1->GetHash(), 0}, COutPoint{tx2->GetHash(), 0}, COutPoint{tx3->GetHash(), 0}}, /*num_outputs=*/3); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx4)); + + // Create 1 grandparent and 1 parent, then 2 children. + const auto tx5 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx5)); + const auto tx6 = make_tx({COutPoint{tx5->GetHash(), 0}}, /*num_outputs=*/3); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx6)); + const auto tx7 = make_tx({COutPoint{tx6->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(med_fee).FromTx(tx7)); + const auto tx8 = make_tx({COutPoint{tx6->GetHash(), 1}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx8)); + + std::vector all_transactions{tx1, tx2, tx3, tx4, tx5, tx6, tx7, tx8}; + std::vector tx_vsizes; + tx_vsizes.reserve(all_transactions.size()); + for (const auto& tx : all_transactions) tx_vsizes.push_back(GetVirtualTransactionSize(*tx)); + + std::vector all_unspent_outpoints({ + COutPoint{tx1->GetHash(), 1}, + COutPoint{tx2->GetHash(), 1}, + COutPoint{tx3->GetHash(), 1}, + COutPoint{tx4->GetHash(), 0}, + COutPoint{tx4->GetHash(), 1}, + COutPoint{tx4->GetHash(), 2}, + COutPoint{tx5->GetHash(), 1}, + COutPoint{tx6->GetHash(), 2}, + COutPoint{tx7->GetHash(), 0}, + COutPoint{tx8->GetHash(), 0} + }); + for (const auto& outpoint : all_unspent_outpoints) BOOST_CHECK(!pool.isSpent(outpoint)); + + const auto tx3_feerate = CFeeRate(high_fee, tx_vsizes[2]); + const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[3]); + // tx4's feerate is lower than tx3's. same fee, different weight. + BOOST_CHECK(tx3_feerate > tx4_feerate); + const auto tx4_anc_feerate = CFeeRate(low_fee + med_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[3]); + const auto tx5_feerate = CFeeRate(high_fee, tx_vsizes[4]); + const auto tx7_anc_feerate = CFeeRate(low_fee + med_fee, tx_vsizes[5] + tx_vsizes[6]); + const auto tx8_anc_feerate = CFeeRate(low_fee + high_fee, tx_vsizes[5] + tx_vsizes[7]); + BOOST_CHECK(tx5_feerate > tx7_anc_feerate); + BOOST_CHECK(tx5_feerate > tx8_anc_feerate); + + // Extremely high feerate: everybody's bumpfee is from their full ancestor set. + { + node::MiniMiner mini_miner(pool, all_unspent_outpoints); + const CFeeRate very_high_feerate(COIN); + BOOST_CHECK(tx4_anc_feerate < very_high_feerate); + BOOST_CHECK(mini_miner.IsReadyToCalculate()); + auto bump_fees = mini_miner.CalculateBumpFees(very_high_feerate); + BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); + BOOST_CHECK(!mini_miner.IsReadyToCalculate()); + BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + const auto tx1_bumpfee = bump_fees.find(COutPoint{tx1->GetHash(), 1}); + BOOST_CHECK(tx1_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx1_bumpfee->second, very_high_feerate.GetFee(tx_vsizes[0]) - low_fee); + const auto tx4_bumpfee = bump_fees.find(COutPoint{tx4->GetHash(), 0}); + BOOST_CHECK(tx4_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx4_bumpfee->second, + very_high_feerate.GetFee(tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]) - (low_fee + med_fee + high_fee + high_fee)); + const auto tx7_bumpfee = bump_fees.find(COutPoint{tx7->GetHash(), 0}); + BOOST_CHECK(tx7_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx7_bumpfee->second, + very_high_feerate.GetFee(tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]) - (high_fee + low_fee + med_fee)); + const auto tx8_bumpfee = bump_fees.find(COutPoint{tx8->GetHash(), 0}); + BOOST_CHECK(tx8_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx8_bumpfee->second, + very_high_feerate.GetFee(tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]) - (high_fee + low_fee + high_fee)); + // Total fees: if spending multiple outputs from tx4 don't double-count fees. + node::MiniMiner mini_miner_total_tx4(pool, {COutPoint{tx4->GetHash(), 0}, COutPoint{tx4->GetHash(), 1}}); + BOOST_CHECK(mini_miner_total_tx4.IsReadyToCalculate()); + const auto tx4_bump_fee = mini_miner_total_tx4.CalculateTotalBumpFees(very_high_feerate); + BOOST_CHECK(!mini_miner_total_tx4.IsReadyToCalculate()); + BOOST_CHECK(tx4_bump_fee.has_value()); + BOOST_CHECK_EQUAL(tx4_bump_fee.value(), + very_high_feerate.GetFee(tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]) - (low_fee + med_fee + high_fee + high_fee)); + // Total fees: if spending both tx7 and tx8, don't double-count fees. + node::MiniMiner mini_miner_tx7_tx8(pool, {COutPoint{tx7->GetHash(), 0}, COutPoint{tx8->GetHash(), 0}}); + BOOST_CHECK(mini_miner_tx7_tx8.IsReadyToCalculate()); + const auto tx7_tx8_bumpfee = mini_miner_tx7_tx8.CalculateTotalBumpFees(very_high_feerate); + BOOST_CHECK(!mini_miner_tx7_tx8.IsReadyToCalculate()); + BOOST_CHECK(tx7_tx8_bumpfee.has_value()); + BOOST_CHECK_EQUAL(tx7_tx8_bumpfee.value(), + very_high_feerate.GetFee(tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6] + tx_vsizes[7]) - (high_fee + low_fee + med_fee + high_fee)); + } + // Feerate just below tx5: tx7 and tx8 have different bump fees. + { + const auto just_below_tx5 = CFeeRate(tx5_feerate.GetFeePerK() - 5); + node::MiniMiner mini_miner(pool, all_unspent_outpoints); + BOOST_CHECK(mini_miner.IsReadyToCalculate()); + auto bump_fees = mini_miner.CalculateBumpFees(just_below_tx5); + BOOST_CHECK(!mini_miner.IsReadyToCalculate()); + BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); + BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + const auto tx7_bumpfee = bump_fees.find(COutPoint{tx7->GetHash(), 0}); + BOOST_CHECK(tx7_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx7_bumpfee->second, just_below_tx5.GetFee(tx_vsizes[5] + tx_vsizes[6]) - (low_fee + med_fee)); + const auto tx8_bumpfee = bump_fees.find(COutPoint{tx8->GetHash(), 0}); + BOOST_CHECK(tx8_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx8_bumpfee->second, just_below_tx5.GetFee(tx_vsizes[5] + tx_vsizes[7]) - (low_fee + high_fee)); + // Total fees: if spending both tx7 and tx8, don't double-count fees. + node::MiniMiner mini_miner_tx7_tx8(pool, {COutPoint{tx7->GetHash(), 0}, COutPoint{tx8->GetHash(), 0}}); + BOOST_CHECK(mini_miner_tx7_tx8.IsReadyToCalculate()); + const auto tx7_tx8_bumpfee = mini_miner_tx7_tx8.CalculateTotalBumpFees(just_below_tx5); + BOOST_CHECK(!mini_miner_tx7_tx8.IsReadyToCalculate()); + BOOST_CHECK(tx7_tx8_bumpfee.has_value()); + BOOST_CHECK_EQUAL(tx7_tx8_bumpfee.value(), just_below_tx5.GetFee(tx_vsizes[5] + tx_vsizes[6]) - (low_fee + med_fee)); + } + // Feerate between tx7 and tx8's ancestor feerates: don't need to bump tx6 because tx8 already does. + { + const auto just_above_tx7 = CFeeRate(med_fee + 10, tx_vsizes[6]); + BOOST_CHECK(just_above_tx7 <= CFeeRate(low_fee + high_fee, tx_vsizes[5] + tx_vsizes[7])); + node::MiniMiner mini_miner(pool, all_unspent_outpoints); + BOOST_CHECK(mini_miner.IsReadyToCalculate()); + auto bump_fees = mini_miner.CalculateBumpFees(just_above_tx7); + BOOST_CHECK(!mini_miner.IsReadyToCalculate()); + BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); + BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + const auto tx7_bumpfee = bump_fees.find(COutPoint{tx7->GetHash(), 0}); + BOOST_CHECK(tx7_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx7_bumpfee->second, just_above_tx7.GetFee(tx_vsizes[6]) - (med_fee)); + const auto tx8_bumpfee = bump_fees.find(COutPoint{tx8->GetHash(), 0}); + BOOST_CHECK(tx8_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx8_bumpfee->second, 0); + } +} +BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) +{ + CTxMemPool& pool = *Assert(m_node.mempool); + LOCK2(cs_main, pool.cs); + + // Add chain of size 500 + TestMemPoolEntryHelper entry; + std::vector chain_txids; + auto& lasttx = m_coinbase_txns[0]; + for (auto i{0}; i < 500; ++i) { + const auto tx = make_tx({COutPoint{lasttx->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(CENT).FromTx(tx)); + chain_txids.push_back(tx->GetHash()); + lasttx = tx; + } + const auto cluster_500tx = pool.GatherClusters({lasttx->GetHash()}); + CTxMemPool::setEntries cluster_500tx_set{cluster_500tx.begin(), cluster_500tx.end()}; + BOOST_CHECK_EQUAL(cluster_500tx.size(), cluster_500tx_set.size()); + const auto vec_iters_500 = pool.GetIterVec(chain_txids); + for (const auto& iter : vec_iters_500) BOOST_CHECK(cluster_500tx_set.count(iter)); + + // GatherClusters stops at 500 transactions. + const auto tx_501 = make_tx({COutPoint{lasttx->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(CENT).FromTx(tx_501)); + const auto cluster_501 = pool.GatherClusters({tx_501->GetHash()}); + BOOST_CHECK_EQUAL(cluster_501.size(), 0); + + // Zig Zag cluster: + // txp0 txp1 txp2 ... txp48 txp49 + // \ / \ / \ \ / + // txc0 txc1 txc2 ... txc48 + // Note that each transaction's ancestor size is 1 or 3, and each descendant size is 1, 2 or 3. + // However, all of these transactions are in the same cluster. + std::vector zigzag_txids; + for (auto p{0}; p < 50; ++p) { + const auto txp = make_tx({COutPoint{GetRandHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(CENT).FromTx(txp)); + zigzag_txids.push_back(txp->GetHash()); + } + for (auto c{0}; c < 49; ++c) { + const auto txc = make_tx({COutPoint{zigzag_txids[c], 1}, COutPoint{zigzag_txids[c+1], 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(CENT).FromTx(txc)); + zigzag_txids.push_back(txc->GetHash()); + } + const auto vec_iters_zigzag = pool.GetIterVec(zigzag_txids); + // It doesn't matter which tx we calculate cluster for, everybody is in it. + const std::vector indeces{0, 22, 72, zigzag_txids.size() - 1}; + for (const auto index : indeces) { + const auto cluster = pool.GatherClusters({zigzag_txids[index]}); + BOOST_CHECK_EQUAL(cluster.size(), zigzag_txids.size()); + CTxMemPool::setEntries clusterset{cluster.begin(), cluster.end()}; + BOOST_CHECK_EQUAL(cluster.size(), clusterset.size()); + for (const auto& iter : vec_iters_zigzag) BOOST_CHECK(clusterset.count(iter)); + } +} + +BOOST_AUTO_TEST_SUITE_END() From 6b605b91c1faf2c7f7cc0c9d39b4fcfd66dc2965 Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 16 Feb 2023 15:31:44 +0000 Subject: [PATCH 4/4] [fuzz] Add MiniMiner target + diff fuzz against BlockAssembler Co-authored-by: dergoegge Co-authored-by: mzumsande Co-authored-by: Murch --- src/Makefile.test.include | 1 + src/test/fuzz/mini_miner.cpp | 192 +++++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 src/test/fuzz/mini_miner.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 3e9d6ad9e3d..26fd6287708 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -283,6 +283,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/message.cpp \ test/fuzz/miniscript.cpp \ test/fuzz/minisketch.cpp \ + test/fuzz/mini_miner.cpp \ test/fuzz/muhash.cpp \ test/fuzz/multiplication_overflow.cpp \ test/fuzz/net.cpp \ diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp new file mode 100644 index 00000000000..f49d9403931 --- /dev/null +++ b/src/test/fuzz/mini_miner.cpp @@ -0,0 +1,192 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include +#include + +namespace { + +const TestingSetup* g_setup; +std::deque g_available_coins; +void initialize_miner() +{ + static const auto testing_setup = MakeNoLogFileContext(); + g_setup = testing_setup.get(); + for (uint32_t i = 0; i < uint32_t{100}; ++i) { + g_available_coins.push_back(COutPoint{uint256::ZERO, i}); + } +} + +// Test that the MiniMiner can run with various outpoints and feerates. +FUZZ_TARGET_INIT(mini_miner, initialize_miner) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + CTxMemPool pool{CTxMemPool::Options{}}; + std::vector outpoints; + std::deque available_coins = g_available_coins; + LOCK2(::cs_main, pool.cs); + // Cluster size cannot exceed 500 + LIMITED_WHILE(!available_coins.empty(), 500) + { + CMutableTransaction mtx = CMutableTransaction(); + const size_t num_inputs = fuzzed_data_provider.ConsumeIntegralInRange(1, available_coins.size()); + const size_t num_outputs = fuzzed_data_provider.ConsumeIntegralInRange(1, 50); + for (size_t n{0}; n < num_inputs; ++n) { + auto prevout = available_coins.front(); + mtx.vin.push_back(CTxIn(prevout, CScript())); + available_coins.pop_front(); + } + for (uint32_t n{0}; n < num_outputs; ++n) { + mtx.vout.push_back(CTxOut(100, P2WSH_OP_TRUE)); + } + CTransactionRef tx = MakeTransactionRef(mtx); + TestMemPoolEntryHelper entry; + const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)}; + assert(MoneyRange(fee)); + pool.addUnchecked(entry.Fee(fee).FromTx(tx)); + + // All outputs are available to spend + for (uint32_t n{0}; n < num_outputs; ++n) { + if (fuzzed_data_provider.ConsumeBool()) { + available_coins.push_back(COutPoint{tx->GetHash(), n}); + } + } + + if (fuzzed_data_provider.ConsumeBool() && !tx->vout.empty()) { + // Add outpoint from this tx (may or not be spent by a later tx) + outpoints.push_back(COutPoint{tx->GetHash(), + (uint32_t)fuzzed_data_provider.ConsumeIntegralInRange(0, tx->vout.size())}); + } else { + // Add some random outpoint (will be interpreted as confirmed or not yet submitted + // to mempool). + auto outpoint = ConsumeDeserializable(fuzzed_data_provider); + if (outpoint.has_value() && std::find(outpoints.begin(), outpoints.end(), *outpoint) == outpoints.end()) { + outpoints.push_back(*outpoint); + } + } + + } + + const CFeeRate target_feerate{CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/1000)}}; + std::optional total_bumpfee; + CAmount sum_fees = 0; + { + node::MiniMiner mini_miner{pool, outpoints}; + assert(mini_miner.IsReadyToCalculate()); + const auto bump_fees = mini_miner.CalculateBumpFees(target_feerate); + for (const auto& outpoint : outpoints) { + auto it = bump_fees.find(outpoint); + assert(it != bump_fees.end()); + assert(it->second >= 0); + sum_fees += it->second; + } + assert(!mini_miner.IsReadyToCalculate()); + } + { + node::MiniMiner mini_miner{pool, outpoints}; + assert(mini_miner.IsReadyToCalculate()); + total_bumpfee = mini_miner.CalculateTotalBumpFees(target_feerate); + assert(total_bumpfee.has_value()); + assert(!mini_miner.IsReadyToCalculate()); + } + // Overlapping ancestry across multiple outpoints can only reduce the total bump fee. + assert (sum_fees >= *total_bumpfee); +} + +// Test that MiniMiner and BlockAssembler build the same block given the same transactions and constraints. +FUZZ_TARGET_INIT(mini_miner_selection, initialize_miner) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + CTxMemPool pool{CTxMemPool::Options{}}; + // Make a copy to preserve determinism. + std::deque available_coins = g_available_coins; + std::vector transactions; + + LOCK2(::cs_main, pool.cs); + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100) + { + CMutableTransaction mtx = CMutableTransaction(); + const size_t num_inputs = 2; + const size_t num_outputs = fuzzed_data_provider.ConsumeIntegralInRange(2, 5); + for (size_t n{0}; n < num_inputs; ++n) { + auto prevout = available_coins.front(); + mtx.vin.push_back(CTxIn(prevout, CScript())); + available_coins.pop_front(); + } + for (uint32_t n{0}; n < num_outputs; ++n) { + mtx.vout.push_back(CTxOut(100, P2WSH_OP_TRUE)); + } + CTransactionRef tx = MakeTransactionRef(mtx); + + // First 2 outputs are available to spend. The rest are added to outpoints to calculate bumpfees. + // There is no overlap between spendable coins and outpoints passed to MiniMiner because the + // MiniMiner interprets spent coins as to-be-replaced and excludes them. + for (uint32_t n{0}; n < num_outputs - 1; ++n) { + if (fuzzed_data_provider.ConsumeBool()) { + available_coins.push_front(COutPoint{tx->GetHash(), n}); + } else { + available_coins.push_back(COutPoint{tx->GetHash(), n}); + } + } + + // Stop if pool reaches DEFAULT_BLOCK_MAX_WEIGHT because BlockAssembler will stop when the + // block template reaches that, but the MiniMiner will keep going. + if (pool.GetTotalTxSize() + GetVirtualTransactionSize(*tx) >= DEFAULT_BLOCK_MAX_WEIGHT) break; + TestMemPoolEntryHelper entry; + const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)}; + assert(MoneyRange(fee)); + pool.addUnchecked(entry.Fee(fee).FromTx(tx)); + transactions.push_back(tx); + } + std::vector outpoints; + for (const auto& coin : g_available_coins) { + if (!pool.GetConflictTx(coin)) outpoints.push_back(coin); + } + for (const auto& tx : transactions) { + assert(pool.exists(GenTxid::Txid(tx->GetHash()))); + for (uint32_t n{0}; n < tx->vout.size(); ++n) { + COutPoint coin{tx->GetHash(), n}; + if (!pool.GetConflictTx(coin)) outpoints.push_back(coin); + } + } + const CFeeRate target_feerate{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)}; + + node::BlockAssembler::Options miner_options; + miner_options.blockMinFeeRate = target_feerate; + miner_options.nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT; + miner_options.test_block_validity = false; + + node::BlockAssembler miner{g_setup->m_node.chainman->ActiveChainstate(), &pool, miner_options}; + node::MiniMiner mini_miner{pool, outpoints}; + assert(mini_miner.IsReadyToCalculate()); + + CScript spk_placeholder = CScript() << OP_0; + // Use BlockAssembler as oracle. BlockAssembler and MiniMiner should select the same + // transactions, stopping once packages do not meet target_feerate. + const auto blocktemplate{miner.CreateNewBlock(spk_placeholder)}; + mini_miner.BuildMockTemplate(target_feerate); + assert(!mini_miner.IsReadyToCalculate()); + auto mock_template_txids = mini_miner.GetMockTemplateTxids(); + // MiniMiner doesn't add a coinbase tx. + assert(mock_template_txids.count(blocktemplate->block.vtx[0]->GetHash()) == 0); + mock_template_txids.emplace(blocktemplate->block.vtx[0]->GetHash()); + assert(mock_template_txids.size() <= blocktemplate->block.vtx.size()); + assert(mock_template_txids.size() >= blocktemplate->block.vtx.size()); + assert(mock_template_txids.size() == blocktemplate->block.vtx.size()); + for (const auto& tx : blocktemplate->block.vtx) { + assert(mock_template_txids.count(tx->GetHash())); + } +} +} // namespace