diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index e4649970384..d7f17f3353b 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -56,9 +56,12 @@ struct SimTxGraph /** Which transactions have been modified in the graph since creation, either directly or by * being in a cluster which includes modifications. Only relevant for the staging graph. */ SetType modified; + /** The configured maximum total size of transactions per cluster. */ + uint64_t max_cluster_size; - /** Construct a new SimTxGraph with the specified maximum cluster count. */ - explicit SimTxGraph(DepGraphIndex max_cluster) : max_cluster_count(max_cluster) {} + /** Construct a new SimTxGraph with the specified maximum cluster count and size. */ + explicit SimTxGraph(DepGraphIndex cluster_count, uint64_t cluster_size) : + max_cluster_count(cluster_count), max_cluster_size(cluster_size) {} // Permit copying and moving. SimTxGraph(const SimTxGraph&) noexcept = default; @@ -78,6 +81,9 @@ struct SimTxGraph while (todo.Any()) { auto component = graph.FindConnectedComponent(todo); if (component.Count() > max_cluster_count) oversized = true; + uint64_t component_size{0}; + for (auto i : component) component_size += graph.FeeRate(i).size; + if (component_size > max_cluster_size) oversized = true; todo -= component; } } @@ -260,14 +266,21 @@ FUZZ_TARGET(txgraph) /** Variable used whenever an empty TxGraph::Ref is needed. */ TxGraph::Ref empty_ref; - // Decide the maximum number of transactions per cluster we will use in this simulation. - auto max_count = provider.ConsumeIntegralInRange(1, MAX_CLUSTER_COUNT_LIMIT); + /** The maximum number of transactions per (non-oversized) cluster we will use in this + * simulation. */ + auto max_cluster_count = provider.ConsumeIntegralInRange(1, MAX_CLUSTER_COUNT_LIMIT); + /** The maximum total size of transactions in a cluster, which also makes it an upper bound + * on the individual size of a transaction (but this restriction will be lifted in a future + * commit). */ + auto max_cluster_size = provider.ConsumeIntegralInRange(1, 0x3fffff * MAX_CLUSTER_COUNT_LIMIT); + /** The maximum individual transaction size used in this test (not a TxGraph parameter). */ + auto max_tx_size = std::min(0x3fffff, max_cluster_size); // Construct a real graph, and a vector of simulated graphs (main, and possibly staging). - auto real = MakeTxGraph(max_count); + auto real = MakeTxGraph(max_cluster_count, max_cluster_size); std::vector sims; sims.reserve(2); - sims.emplace_back(max_count); + sims.emplace_back(max_cluster_count, max_cluster_size); /** Struct encapsulating information about a BlockBuilder that's currently live. */ struct BlockBuilderData @@ -391,12 +404,12 @@ FUZZ_TARGET(txgraph) if (alt) { // If alt is true, pick fee and size from the entire range. fee = provider.ConsumeIntegralInRange(-0x8000000000000, 0x7ffffffffffff); - size = provider.ConsumeIntegralInRange(1, 0x3fffff); + size = provider.ConsumeIntegralInRange(1, max_tx_size); } else { // Otherwise, use smaller range which consume fewer fuzz input bytes, as just // these are likely sufficient to trigger all interesting code paths already. fee = provider.ConsumeIntegral(); - size = provider.ConsumeIntegral() + 1; + size = provider.ConsumeIntegralInRange(1, std::min(0xff, max_tx_size)); } FeePerWeight feerate{fee, size}; // Create a real TxGraph::Ref. @@ -534,7 +547,7 @@ FUZZ_TARGET(txgraph) auto ref = pick_fn(); auto result = alt ? real->GetDescendants(*ref, use_main) : real->GetAncestors(*ref, use_main); - assert(result.size() <= max_count); + assert(result.size() <= max_cluster_count); auto result_set = sel_sim.MakeSet(result); assert(result.size() == result_set.Count()); auto expect_set = sel_sim.GetAncDesc(ref, alt); @@ -567,16 +580,20 @@ FUZZ_TARGET(txgraph) auto ref = pick_fn(); auto result = real->GetCluster(*ref, use_main); // Check cluster count limit. - assert(result.size() <= max_count); + assert(result.size() <= max_cluster_count); // Require the result to be topologically valid and not contain duplicates. auto left = sel_sim.graph.Positions(); + uint64_t total_size{0}; for (auto refptr : result) { auto simpos = sel_sim.Find(refptr); + total_size += sel_sim.graph.FeeRate(simpos).size; assert(simpos != SimTxGraph::MISSING); assert(left[simpos]); left.Reset(simpos); assert(!sel_sim.graph.Ancestors(simpos).Overlaps(left)); } + // Check cluster size limit. + assert(total_size <= max_cluster_size); // Require the set to be connected. auto result_set = sel_sim.MakeSet(result); assert(sel_sim.graph.IsConnected(result_set)); @@ -941,28 +958,32 @@ FUZZ_TARGET(txgraph) // Check its ancestors against simulation. auto expect_anc = sim.graph.Ancestors(i); auto anc = sim.MakeSet(real->GetAncestors(*sim.GetRef(i), main_only)); - assert(anc.Count() <= max_count); + assert(anc.Count() <= max_cluster_count); assert(anc == expect_anc); // Check its descendants against simulation. auto expect_desc = sim.graph.Descendants(i); auto desc = sim.MakeSet(real->GetDescendants(*sim.GetRef(i), main_only)); - assert(desc.Count() <= max_count); + assert(desc.Count() <= max_cluster_count); assert(desc == expect_desc); // Check the cluster the transaction is part of. auto cluster = real->GetCluster(*sim.GetRef(i), main_only); - assert(cluster.size() <= max_count); + assert(cluster.size() <= max_cluster_count); assert(sim.MakeSet(cluster) == component); // Check that the cluster is reported in a valid topological order (its // linearization). std::vector simlin; SimTxGraph::SetType done; + uint64_t total_size{0}; for (TxGraph::Ref* ptr : cluster) { auto simpos = sim.Find(ptr); assert(sim.graph.Descendants(simpos).IsSubsetOf(component - done)); done.Set(simpos); assert(sim.graph.Ancestors(simpos).IsSubsetOf(done)); simlin.push_back(simpos); + total_size += sim.graph.FeeRate(simpos).size; } + // Check cluster size. + assert(total_size <= max_cluster_size); // Construct a chunking object for the simulated graph, using the reported cluster // linearization as ordering, and compare it against the reported chunk feerates. if (sims.size() == 1 || main_only) { diff --git a/src/txgraph.cpp b/src/txgraph.cpp index 33afca68b0b..2629e8a2e88 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -109,6 +109,8 @@ public: } /** Get the number of transactions in this Cluster. */ LinearizationIndex GetTxCount() const noexcept { return m_linearization.size(); } + /** Get the total size of the transactions in this Cluster. */ + uint64_t GetTotalTxSize() const noexcept; /** Given a DepGraphIndex into this Cluster, find the corresponding GraphIndex. */ GraphIndex GetClusterEntry(DepGraphIndex index) const noexcept { return m_mapping[index]; } /** Only called by Graph::SwapIndexes. */ @@ -199,6 +201,8 @@ private: FastRandomContext m_rng; /** This TxGraphImpl's maximum cluster count limit. */ const DepGraphIndex m_max_cluster_count; + /** This TxGraphImpl's maximum cluster size limit. */ + const uint64_t m_max_cluster_size; /** Information about one group of Clusters to be merged. */ struct GroupEntry @@ -401,9 +405,10 @@ private: std::vector m_unlinked; public: - /** Construct a new TxGraphImpl with the specified maximum cluster count. */ - explicit TxGraphImpl(DepGraphIndex max_cluster_count) noexcept : + /** Construct a new TxGraphImpl with the specified limits. */ + explicit TxGraphImpl(DepGraphIndex max_cluster_count, uint64_t max_cluster_size) noexcept : m_max_cluster_count(max_cluster_count), + m_max_cluster_size(max_cluster_size), m_main_chunkindex(ChunkOrder(this)) { Assume(max_cluster_count >= 1); @@ -635,6 +640,15 @@ void TxGraphImpl::CreateChunkData(GraphIndex idx, LinearizationIndex chunk_count } } +uint64_t Cluster::GetTotalTxSize() const noexcept +{ + uint64_t ret{0}; + for (auto i : m_linearization) { + ret += m_depgraph.FeeRate(i).size; + } + return ret; +} + void TxGraphImpl::ClearLocator(int level, GraphIndex idx) noexcept { auto& entry = m_entries[idx]; @@ -1439,10 +1453,12 @@ void TxGraphImpl::GroupClusters(int level) noexcept new_entry.m_deps_offset = clusterset.m_deps_to_add.size(); new_entry.m_deps_count = 0; uint32_t total_count{0}; + uint64_t total_size{0}; // Add all its clusters to it (copying those from an_clusters to m_group_clusters). while (an_clusters_it != an_clusters.end() && an_clusters_it->second == rep) { clusterset.m_group_data->m_group_clusters.push_back(an_clusters_it->first); total_count += an_clusters_it->first->GetTxCount(); + total_size += an_clusters_it->first->GetTotalTxSize(); ++an_clusters_it; ++new_entry.m_cluster_count; } @@ -1453,7 +1469,7 @@ void TxGraphImpl::GroupClusters(int level) noexcept ++new_entry.m_deps_count; } // Detect oversizedness. - if (total_count > m_max_cluster_count) { + if (total_count > m_max_cluster_count || total_size > m_max_cluster_size) { clusterset.m_group_data->m_group_oversized = true; } } @@ -1587,6 +1603,7 @@ Cluster::Cluster(uint64_t sequence, TxGraphImpl& graph, const FeePerWeight& feer TxGraph::Ref TxGraphImpl::AddTransaction(const FeePerWeight& feerate) noexcept { Assume(m_main_chunkindex_observers == 0 || GetTopLevel() != 0); + Assume(feerate.size > 0 && uint64_t(feerate.size) <= m_max_cluster_size); // Construct a new Ref. Ref ret; // Construct a new Entry, and link it with the Ref. @@ -2124,6 +2141,10 @@ void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const assert(m_linearization.size() <= graph.m_max_cluster_count); // The level must match the level the Cluster occurs in. assert(m_level == level); + // The sum of their sizes cannot exceed m_max_cluster_size. Note that groups of to-be-merged + // clusters which would exceed this limit are marked oversized, which means they are never + // applied. + assert(GetTotalTxSize() <= graph.m_max_cluster_size); // m_quality and m_setindex are checked in TxGraphImpl::SanityCheck. // Compute the chunking of m_linearization. @@ -2500,7 +2521,7 @@ TxGraph::Ref::Ref(Ref&& other) noexcept std::swap(m_index, other.m_index); } -std::unique_ptr MakeTxGraph(unsigned max_cluster_count) noexcept +std::unique_ptr MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size) noexcept { - return std::make_unique(max_cluster_count); + return std::make_unique(max_cluster_count, max_cluster_size); } diff --git a/src/txgraph.h b/src/txgraph.h index 5779397f82f..95bdecc3ddf 100644 --- a/src/txgraph.h +++ b/src/txgraph.h @@ -63,10 +63,10 @@ public: /** Virtual destructor, so inheriting is safe. */ virtual ~TxGraph() = default; /** Construct a new transaction with the specified feerate, and return a Ref to it. - * If a staging graph exists, the new transaction is only created there. In all - * further calls, only Refs created by AddTransaction() are allowed to be passed to this - * TxGraph object (or empty Ref objects). Ref objects may outlive the TxGraph they were - * created for. */ + * If a staging graph exists, the new transaction is only created there. feerate.size must be + * strictly positive, and cannot exceed the graph's max cluster size. In all further calls, + * only Refs created by AddTransaction() are allowed to be passed to this TxGraph object (or + * empty Ref objects). Ref objects may outlive the TxGraph they were created for. */ [[nodiscard]] virtual Ref AddTransaction(const FeePerWeight& feerate) noexcept = 0; /** Remove the specified transaction. If a staging graph exists, the removal only happens * there. This is a no-op if the transaction was already removed. @@ -240,8 +240,9 @@ public: }; }; -/** Construct a new TxGraph with the specified limit on transactions within a cluster. That - * number cannot exceed MAX_CLUSTER_COUNT_LIMIT. */ -std::unique_ptr MakeTxGraph(unsigned max_cluster_count) noexcept; +/** Construct a new TxGraph with the specified limit on the number of transactions within a cluster, + * and on the sum of transaction sizes within a cluster. max_cluster_count cannot exceed + * MAX_CLUSTER_COUNT_LIMIT. */ +std::unique_ptr MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size) noexcept; #endif // BITCOIN_TXGRAPH_H