diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index cec1cd56edc..b6163ed5544 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -27,7 +27,7 @@ struct SimTxGraph /** Maximum number of transactions to support simultaneously. Set this higher than txgraph's * cluster count, so we can exercise situations with more transactions than fit in one * cluster. */ - static constexpr unsigned MAX_TRANSACTIONS = CLUSTER_COUNT_LIMIT * 2; + static constexpr unsigned MAX_TRANSACTIONS = MAX_CLUSTER_COUNT_LIMIT * 2; /** Set type to use in the simulation. */ using SetType = BitSet; /** Data type for representing positions within SimTxGraph::graph. */ @@ -44,6 +44,31 @@ struct SimTxGraph std::map simrevmap; /** The set of TxGraph::Ref entries that have been removed, but not yet destroyed. */ std::vector> removed; + /** Whether the graph is oversized (true = yes, false = no, std::nullopt = unknown). */ + std::optional oversized; + /** The configured maximum number of transactions per cluster. */ + DepGraphIndex max_cluster_count; + + /** Construct a new SimData with the specified maximum cluster count. */ + explicit SimTxGraph(DepGraphIndex max_cluster) : max_cluster_count(max_cluster) {} + + /** Check whether this graph is oversized (contains a connected component whose number of + * transactions exceeds max_cluster_count. */ + bool IsOversized() + { + if (!oversized.has_value()) { + // Only recompute when oversized isn't already known. + oversized = false; + auto todo = graph.Positions(); + // Iterate over all connected components of the graph. + while (todo.Any()) { + auto component = graph.FindConnectedComponent(todo); + if (component.Count() > max_cluster_count) oversized = true; + todo -= component; + } + } + return *oversized; + } /** Determine the number of (non-removed) transactions in the graph. */ DepGraphIndex GetTransactionCount() const { return graph.TxCount(); } @@ -84,6 +109,8 @@ struct SimTxGraph auto chl_pos = Find(child); if (chl_pos == MISSING) return; graph.AddDependencies(SetType::Singleton(par_pos), chl_pos); + // This may invalidate our cached oversized value. + if (oversized.has_value() && !*oversized) oversized = std::nullopt; } /** Modify the transaction fee of a ref, if it exists. */ @@ -105,6 +132,8 @@ struct SimTxGraph // invoked until the simulation explicitly decided to do so. removed.push_back(std::move(simmap[pos])); simmap[pos].reset(); + // This may invalidate our cached oversized value. + if (oversized.has_value() && *oversized) oversized = std::nullopt; } /** Construct the set with all positions in this graph corresponding to the specified @@ -170,9 +199,12 @@ 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); + // Construct a real and a simulated graph. - auto real = MakeTxGraph(); - SimTxGraph sim; + auto real = MakeTxGraph(max_count); + SimTxGraph sim(max_count); /** Function to pick any Ref (from sim.simmap or sim.removed, or the empty Ref). */ auto pick_fn = [&]() noexcept -> TxGraph::Ref* { @@ -245,17 +277,6 @@ FUZZ_TARGET(txgraph) // Determine if adding this would introduce a cycle (not allowed by TxGraph), // and if so, skip. if (sim.graph.Ancestors(pos_par)[pos_chl]) break; - // Determine if adding this would violate CLUSTER_COUNT_LIMIT, and if so, skip. - auto temp_depgraph = sim.graph; - temp_depgraph.AddDependencies(SimTxGraph::SetType::Singleton(pos_par), pos_chl); - auto todo = temp_depgraph.Positions(); - bool oversize{false}; - while (todo.Any()) { - auto component = temp_depgraph.FindConnectedComponent(todo); - if (component.Count() > CLUSTER_COUNT_LIMIT) oversize = true; - todo -= component; - } - if (oversize) break; } sim.AddDependency(par, chl); real->AddDependency(*par, *chl); @@ -310,6 +331,10 @@ FUZZ_TARGET(txgraph) bool should_exist = sim.Find(ref) != SimTxGraph::MISSING; assert(exists == should_exist); break; + } else if (command-- == 0) { + // IsOversized. + assert(sim.IsOversized() == real->IsOversized()); + break; } else if (command-- == 0) { // GetIndividualFeerate. auto ref = pick_fn(); @@ -321,7 +346,7 @@ FUZZ_TARGET(txgraph) assert(feerate == sim.graph.FeeRate(simpos)); } break; - } else if (command-- == 0) { + } else if (!sim.IsOversized() && command-- == 0) { // GetChunkFeerate. auto ref = pick_fn(); auto feerate = real->GetChunkFeerate(*ref); @@ -334,20 +359,22 @@ FUZZ_TARGET(txgraph) assert(feerate.size >= sim.graph.FeeRate(simpos).size); } break; - } else if (command-- == 0) { + } else if (!sim.IsOversized() && command-- == 0) { // GetAncestors/GetDescendants. auto ref = pick_fn(); - auto result_set = sim.MakeSet(alt ? real->GetDescendants(*ref) : - real->GetAncestors(*ref)); + auto result = alt ? real->GetDescendants(*ref) : real->GetAncestors(*ref); + assert(result.size() <= max_count); + auto result_set = sim.MakeSet(result); + assert(result.size() == result_set.Count()); auto expect_set = sim.GetAncDesc(ref, alt); assert(result_set == expect_set); break; - } else if (command-- == 0) { + } else if (!sim.IsOversized() && command-- == 0) { // GetCluster. auto ref = pick_fn(); auto result = real->GetCluster(*ref); // Check cluster count limit. - assert(result.size() <= CLUSTER_COUNT_LIMIT); + assert(result.size() <= max_count); // Require the result to be topologically valid and not contain duplicates. auto left = sim.graph.Positions(); for (auto refptr : result) { @@ -382,56 +409,62 @@ FUZZ_TARGET(txgraph) real->SanityCheck(); // Compare simple properties of the graph with the simulation. + assert(real->IsOversized() == sim.IsOversized()); assert(real->GetTransactionCount() == sim.GetTransactionCount()); - // Perform a full comparison. - auto todo = sim.graph.Positions(); - // Iterate over all connected components of the resulting (simulated) graph, each of which - // should correspond to a cluster in the real one. - while (todo.Any()) { - auto component = sim.graph.FindConnectedComponent(todo); - todo -= component; - // Iterate over the transactions in that component. - for (auto i : component) { - // Check its individual feerate against simulation. - assert(sim.graph.FeeRate(i) == real->GetIndividualFeerate(*sim.GetRef(i))); - // Check its ancestors against simulation. - auto expect_anc = sim.graph.Ancestors(i); - auto anc = sim.MakeSet(real->GetAncestors(*sim.GetRef(i))); - 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))); - assert(desc == expect_desc); - // Check the cluster the transaction is part of. - auto cluster = real->GetCluster(*sim.GetRef(i)); - assert(sim.MakeSet(cluster) == component); - // Check that the cluster is reported in a valid topological order (its - // linearization). - std::vector simlin; - SimTxGraph::SetType done; - 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); - } - // Construct a chunking object for the simulated graph, using the reported cluster - // linearization as ordering, and compare it against the reported chunk feerates. - cluster_linearize::LinearizationChunking simlinchunk(sim.graph, simlin); - DepGraphIndex idx{0}; - for (unsigned chunknum = 0; chunknum < simlinchunk.NumChunksLeft(); ++chunknum) { - auto chunk = simlinchunk.GetChunk(chunknum); - // Require that the chunks of cluster linearizations are connected (this must - // be the case as all linearizations inside are PostLinearized). - assert(sim.graph.IsConnected(chunk.transactions)); - // Check the chunk feerates of all transactions in the cluster. - while (chunk.transactions.Any()) { - assert(chunk.transactions[simlin[idx]]); - chunk.transactions.Reset(simlin[idx]); - assert(chunk.feerate == real->GetChunkFeerate(*cluster[idx])); - ++idx; + // If the graph (and the simulation) are not oversized, perform a full comparison. + if (!sim.IsOversized()) { + auto todo = sim.graph.Positions(); + // Iterate over all connected components of the resulting (simulated) graph, each of which + // should correspond to a cluster in the real one. + while (todo.Any()) { + auto component = sim.graph.FindConnectedComponent(todo); + todo -= component; + // Iterate over the transactions in that component. + for (auto i : component) { + // Check its individual feerate against simulation. + assert(sim.graph.FeeRate(i) == real->GetIndividualFeerate(*sim.GetRef(i))); + // Check its ancestors against simulation. + auto expect_anc = sim.graph.Ancestors(i); + auto anc = sim.MakeSet(real->GetAncestors(*sim.GetRef(i))); + assert(anc.Count() <= max_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))); + assert(desc.Count() <= max_count); + assert(desc == expect_desc); + // Check the cluster the transaction is part of. + auto cluster = real->GetCluster(*sim.GetRef(i)); + assert(cluster.size() <= max_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; + 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); + } + // Construct a chunking object for the simulated graph, using the reported cluster + // linearization as ordering, and compare it against the reported chunk feerates. + cluster_linearize::LinearizationChunking simlinchunk(sim.graph, simlin); + DepGraphIndex idx{0}; + for (unsigned chunknum = 0; chunknum < simlinchunk.NumChunksLeft(); ++chunknum) { + auto chunk = simlinchunk.GetChunk(chunknum); + // Require that the chunks of cluster linearizations are connected (this must + // be the case as all linearizations inside are PostLinearized). + assert(sim.graph.IsConnected(chunk.transactions)); + // Check the chunk feerates of all transactions in the cluster. + while (chunk.transactions.Any()) { + assert(chunk.transactions[simlin[idx]]); + chunk.transactions.Reset(simlin[idx]); + assert(chunk.feerate == real->GetChunkFeerate(*cluster[idx])); + ++idx; + } } } } diff --git a/src/txgraph.cpp b/src/txgraph.cpp index 4b49cd6c6f5..ae0da8ba0d2 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -49,7 +49,7 @@ class Cluster { friend class TxGraphImpl; using GraphIndex = TxGraph::GraphIndex; - using SetType = BitSet; + using SetType = BitSet; /** The DepGraph for this cluster, holding all feerates, and ancestors/descendants. */ DepGraph m_depgraph; /** m_mapping[i] gives the GraphIndex for the position i transaction in m_depgraph. Values for @@ -160,6 +160,8 @@ class TxGraphImpl final : public TxGraph private: /** Internal RNG. */ FastRandomContext m_rng; + /** This TxGraphImpl's maximum cluster count limit. */ + const DepGraphIndex m_max_cluster_count; /** Information about one group of Clusters to be merged. */ struct GroupEntry @@ -181,6 +183,9 @@ private: std::vector m_groups; /** Which clusters are to be merged. GroupEntry::m_cluster_offset indexes into this. */ std::vector m_group_clusters; + /** Whether at least one of the groups cannot be applied because it would result in a + * Cluster that violates the cluster count limit. */ + bool m_group_oversized; }; /** The vectors of clusters, one vector per quality level. ClusterSetIndex indexes into each. */ @@ -232,8 +237,13 @@ private: std::vector m_unlinked; public: - /** Construct a new TxGraphImpl. */ - explicit TxGraphImpl() noexcept {} + /** Construct a new TxGraphImpl with the specified maximum cluster count. */ + explicit TxGraphImpl(DepGraphIndex max_cluster_count) noexcept : + m_max_cluster_count(max_cluster_count) + { + Assume(max_cluster_count >= 1); + Assume(max_cluster_count <= MAX_CLUSTER_COUNT_LIMIT); + } // Cannot move or copy (would invalidate TxGraphImpl* in Ref, MiningOrder, EvictionOrder). TxGraphImpl(const TxGraphImpl&) = delete; @@ -309,6 +319,7 @@ public: std::vector GetAncestors(const Ref& arg) noexcept final; std::vector GetDescendants(const Ref& arg) noexcept final; GraphIndex GetTransactionCount() noexcept final; + bool IsOversized() noexcept final; void SanityCheck() const final; }; @@ -696,7 +707,7 @@ void TxGraphImpl::GroupClusters() noexcept // Before computing which Clusters need to be merged together, first apply all removals and // split the Clusters into connected components. If we would group first, we might end up - // with inefficient Clusters which just end up being split again anyway. + // with inefficient and/or oversized Clusters which just end up being split again anyway. SplitAll(); /** Annotated clusters: an entry for each Cluster, together with the representative for the @@ -833,6 +844,7 @@ void TxGraphImpl::GroupClusters() noexcept // back to m_deps_to_add. m_group_data = GroupData{}; m_group_data->m_group_clusters.reserve(an_clusters.size()); + m_group_data->m_group_oversized = false; m_deps_to_add.clear(); m_deps_to_add.reserve(an_deps.size()); auto an_deps_it = an_deps.begin(); @@ -846,9 +858,11 @@ void TxGraphImpl::GroupClusters() noexcept new_entry.m_cluster_count = 0; new_entry.m_deps_offset = m_deps_to_add.size(); new_entry.m_deps_count = 0; + uint32_t total_count{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) { m_group_data->m_group_clusters.push_back(an_clusters_it->first); + total_count += an_clusters_it->first->GetTxCount(); ++an_clusters_it; ++new_entry.m_cluster_count; } @@ -858,6 +872,10 @@ void TxGraphImpl::GroupClusters() noexcept ++an_deps_it; ++new_entry.m_deps_count; } + // Detect oversizedness. + if (total_count > m_max_cluster_count) { + m_group_data->m_group_oversized = true; + } } Assume(an_deps_it == an_deps.end()); Assume(an_clusters_it == an_clusters.end()); @@ -898,6 +916,8 @@ void TxGraphImpl::ApplyDependencies() noexcept Assume(m_group_data.has_value()); // Nothing to do if there are no dependencies to be added. if (m_deps_to_add.empty()) return; + // Dependencies cannot be applied if it would result in oversized clusters. + if (m_group_data->m_group_oversized) return; // For each group of to-be-merged Clusters. for (const auto& group_data : m_group_data->m_groups) { @@ -1073,6 +1093,8 @@ std::vector TxGraphImpl::GetAncestors(const Ref& arg) noexcept Assume(GetRefGraph(arg) == this); // Apply all removals and dependencies, as the result might be incorrect otherwise. ApplyDependencies(); + // Ancestry cannot be known if unapplied dependencies remain. + Assume(m_deps_to_add.empty()); // Find the Cluster the argument is in, and return the empty vector if it isn't in any. auto cluster = m_entries[GetRefIndex(arg)].m_locator.cluster; if (cluster == nullptr) return {}; @@ -1087,6 +1109,8 @@ std::vector TxGraphImpl::GetDescendants(const Ref& arg) noexcept Assume(GetRefGraph(arg) == this); // Apply all removals and dependencies, as the result might be incorrect otherwise. ApplyDependencies(); + // Ancestry cannot be known if unapplied dependencies remain. + Assume(m_deps_to_add.empty()); // Find the Cluster the argument is in, and return the empty vector if it isn't in any. auto cluster = m_entries[GetRefIndex(arg)].m_locator.cluster; if (cluster == nullptr) return {}; @@ -1101,6 +1125,8 @@ std::vector TxGraphImpl::GetCluster(const Ref& arg) noexcept Assume(GetRefGraph(arg) == this); // Apply all removals and dependencies, as the result might be incorrect otherwise. ApplyDependencies(); + // Cluster linearization cannot be known if unapplied dependencies remain. + Assume(m_deps_to_add.empty()); // Find the Cluster the argument is in, and return the empty vector if it isn't in any. auto cluster = m_entries[GetRefIndex(arg)].m_locator.cluster; if (cluster == nullptr) return {}; @@ -1136,6 +1162,8 @@ FeePerWeight TxGraphImpl::GetChunkFeerate(const Ref& arg) noexcept Assume(GetRefGraph(arg) == this); // Apply all removals and dependencies, as the result might be inaccurate otherwise. ApplyDependencies(); + // Chunk feerates cannot be accurately known if unapplied dependencies remain. + Assume(m_deps_to_add.empty()); // Find the cluster the argument is in, and return the empty FeePerWeight if it isn't in any. auto cluster = m_entries[GetRefIndex(arg)].m_locator.cluster; if (cluster == nullptr) return {}; @@ -1146,6 +1174,15 @@ FeePerWeight TxGraphImpl::GetChunkFeerate(const Ref& arg) noexcept return entry.m_chunk_feerate; } +bool TxGraphImpl::IsOversized() noexcept +{ + // Find which Clusters will need to be merged together, as that is where the oversize + // property is assessed. + GroupClusters(); + Assume(m_group_data.has_value()); + return m_group_data->m_group_oversized; +} + void Cluster::SetFee(TxGraphImpl& graph, DepGraphIndex idx, int64_t fee) noexcept { // Make sure the specified DepGraphIndex exists in this Cluster. @@ -1180,6 +1217,8 @@ void Cluster::SanityCheck(const TxGraphImpl& graph) const assert(m_depgraph.PositionRange() == m_mapping.size()); // The linearization for this Cluster must contain every transaction once. assert(m_depgraph.TxCount() == m_linearization.size()); + // The number of transactions in a Cluster cannot exceed m_max_cluster_count. + assert(m_linearization.size() <= graph.m_max_cluster_count); // m_quality and m_setindex are checked in TxGraphImpl::SanityCheck. // Compute the chunking of m_linearization. @@ -1321,7 +1360,7 @@ TxGraph::Ref::Ref(Ref&& other) noexcept std::swap(m_index, other.m_index); } -std::unique_ptr MakeTxGraph() noexcept +std::unique_ptr MakeTxGraph(unsigned max_cluster_count) noexcept { - return std::make_unique(); + return std::make_unique(max_cluster_count); } diff --git a/src/txgraph.h b/src/txgraph.h index f92cfbd7950..83411dc8801 100644 --- a/src/txgraph.h +++ b/src/txgraph.h @@ -12,8 +12,7 @@ #ifndef BITCOIN_TXGRAPH_H #define BITCOIN_TXGRAPH_H -/** No connected component within TxGraph is allowed to exceed this number of transactions. */ -static constexpr unsigned CLUSTER_COUNT_LIMIT{64}; +static constexpr unsigned MAX_CLUSTER_COUNT_LIMIT{64}; /** Data structure to encapsulate fees, sizes, and dependencies for a set of transactions. * @@ -83,24 +82,33 @@ public: * removed), this has no effect. */ virtual void SetTransactionFee(const Ref& arg, int64_t fee) noexcept = 0; - /** Determine whether arg exists in this graph (i.e., was not removed). */ + /** Determine whether the graph is oversized (contains a connected component of more than the + * configured maximum cluster count). Some of the functions below are not available + * for oversized graphs. The mutators above are always available. */ + virtual bool IsOversized() noexcept = 0; + /** Determine whether arg exists in this graph (i.e., was not removed). This is available even + * for oversized graphs. */ virtual bool Exists(const Ref& arg) noexcept = 0; /** Get the individual transaction feerate of transaction arg. Returns the empty FeePerWeight - * if arg does not exist. */ + * if arg does not exist. This is available even for oversized graphs. */ virtual FeePerWeight GetIndividualFeerate(const Ref& arg) noexcept = 0; /** Get the feerate of the chunk which transaction arg is in. Returns the empty FeePerWeight if - * arg does not exist. */ + * arg does not exist. The graph must not be oversized. */ virtual FeePerWeight GetChunkFeerate(const Ref& arg) noexcept = 0; /** Get pointers to all transactions in the cluster which arg is in. The transactions will be - * returned in graph order. Returns {} if arg does not exist in the graph. */ + * returned in graph order. The graph must not be oversized. Returns {} if arg does not exist + * in the graph. */ virtual std::vector GetCluster(const Ref& arg) noexcept = 0; /** Get pointers to all ancestors of the specified transaction (including the transaction - * itself), in unspecified order. Returns {} if arg does not exist in the graph. */ + * itself), in unspecified order. The graph must not be oversized. Returns {} if arg does not + * exist in the graph. */ virtual std::vector GetAncestors(const Ref& arg) noexcept = 0; /** Get pointers to all descendants of the specified transaction (including the transaction - * itself), in unspecified order. Returns {} if arg does not exist in the graph. */ + * itself), in unspecified order. The graph must not be oversized. Returns {} if arg does not + * exist in the graph. */ virtual std::vector GetDescendants(const Ref& arg) noexcept = 0; - /** Get the total number of transactions in the graph. */ + /** Get the total number of transactions in the graph. This is available even for oversized + * graphs. */ virtual GraphIndex GetTransactionCount() noexcept = 0; /** Perform an internal consistency check on this object. */ @@ -145,7 +153,8 @@ public: }; }; -/** Construct a new TxGraph. */ -std::unique_ptr MakeTxGraph() noexcept; +/** 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; #endif // BITCOIN_TXGRAPH_H