From 5901cf7100a75c8131223e23b6c90e0e93611eae Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 6 Sep 2024 15:32:48 -0400 Subject: [PATCH 1/7] clusterlin: abstract out DepGraph::GetReduced{Parents,Children} A fuzz test already relies on these operations, and a future commit will need the same logic too. Therefore, abstract them out into proper member functions, with proper testing. --- src/cluster_linearize.h | 42 ++++++++++++++++++++++++++ src/test/fuzz/cluster_linearize.cpp | 16 ++-------- src/test/util/cluster_linearize.h | 47 ++++++++++++++++++++++++++++- 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index e964849f22839..8467f3f08be0c 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -184,6 +184,48 @@ class DepGraph } } + /** Compute the (reduced) set of parents of node i in this graph. + * + * This returns the minimal subset of the parents of i whose ancestors together equal all of + * i's ancestors (unless i is part of a cycle of dependencies). Note that DepGraph does not + * store the set of parents; this information is inferred from the ancestor sets. + * + * Complexity: O(N) where N=Ancestors(i).Count() (which is bounded by TxCount()). + */ + SetType GetReducedParents(ClusterIndex i) const noexcept + { + SetType parents = Ancestors(i); + parents.Reset(i); + for (auto parent : parents) { + if (parents[parent]) { + parents -= Ancestors(parent); + parents.Set(parent); + } + } + return parents; + } + + /** Compute the (reduced) set of children of node i in this graph. + * + * This returns the minimal subset of the children of i whose descendants together equal all of + * i's descendants (unless i is part of a cycle of dependencies). Note that DepGraph does not + * store the set of children; this information is inferred from the descendant sets. + * + * Complexity: O(N) where N=Descendants(i).Count() (which is bounded by TxCount()). + */ + SetType GetReducedChildren(ClusterIndex i) const noexcept + { + SetType children = Descendants(i); + children.Reset(i); + for (auto child : children) { + if (children[child]) { + children -= Descendants(child); + children.Set(child); + } + } + return children; + } + /** Compute the aggregate feerate of a set of nodes in this graph. * * Complexity: O(N) where N=elems.Count(). diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index d91f85d867b14..4976afbaad807 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -896,24 +896,12 @@ FUZZ_TARGET(clusterlin_postlinearize_tree) } if (direction & 1) { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { - auto children = depgraph_gen.Descendants(i) - TestBitSet::Singleton(i); - // Remove descendants that are children of other descendants. - for (auto j : children) { - if (!children[j]) continue; - children -= depgraph_gen.Descendants(j); - children.Set(j); - } + auto children = depgraph_gen.GetReducedChildren(i); if (children.Any()) depgraph_tree.AddDependency(i, children.First()); } } else { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { - auto parents = depgraph_gen.Ancestors(i) - TestBitSet::Singleton(i); - // Remove ancestors that are parents of other ancestors. - for (auto j : parents) { - if (!parents[j]) continue; - parents -= depgraph_gen.Ancestors(j); - parents.Set(j); - } + auto parents = depgraph_gen.GetReducedParents(i); if (parents.Any()) depgraph_tree.AddDependency(parents.First(), i); } } diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index b86ebcd78b24c..377bfa19fbaef 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -264,9 +264,23 @@ void SanityCheck(const DepGraph& depgraph) for (ClusterIndex j = 0; j < depgraph.TxCount(); ++j) { assert(depgraph.Ancestors(i)[j] == depgraph.Descendants(j)[i]); } + // No transaction is a parent or child of itself. + auto parents = depgraph.GetReducedParents(i); + auto children = depgraph.GetReducedChildren(i); + assert(!parents[i]); + assert(!children[i]); + // Parents of a transaction do not have ancestors inside those parents (except itself). + // Note that even the transaction itself may be missing (if it is part of a cycle). + for (auto parent : parents) { + assert((depgraph.Ancestors(parent) & parents).IsSubsetOf(SetType::Singleton(parent))); + } + // Similar for children and descendants. + for (auto child : children) { + assert((depgraph.Descendants(child) & children).IsSubsetOf(SetType::Singleton(child))); + } } - // If DepGraph is acyclic, serialize + deserialize must roundtrip. if (IsAcyclic(depgraph)) { + // If DepGraph is acyclic, serialize + deserialize must roundtrip. std::vector ser; VectorWriter writer(ser, 0); writer << Using(depgraph); @@ -284,6 +298,37 @@ void SanityCheck(const DepGraph& depgraph) reader >> Using(decoded_depgraph); assert(depgraph == decoded_depgraph); assert(reader.empty()); + + // In acyclic graphs, the union of parents with parents of parents etc. yields the + // full ancestor set (and similar for children and descendants). + std::vector parents, children; + for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + parents.push_back(depgraph.GetReducedParents(i)); + children.push_back(depgraph.GetReducedChildren(i)); + } + for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + // Initialize the set of ancestors with just the current transaction itself. + SetType ancestors = SetType::Singleton(i); + // Iteratively add parents of all transactions in the ancestor set to itself. + while (true) { + const auto old_ancestors = ancestors; + for (auto j : ancestors) ancestors |= parents[j]; + // Stop when no more changes are being made. + if (old_ancestors == ancestors) break; + } + assert(ancestors == depgraph.Ancestors(i)); + + // Initialize the set of descendants with just the current transaction itself. + SetType descendants = SetType::Singleton(i); + // Iteratively add children of all transactions in the descendant set to itself. + while (true) { + const auto old_descendants = descendants; + for (auto j : descendants) descendants |= children[j]; + // Stop when no more changes are being made. + if (old_descendants == descendants) break; + } + assert(descendants == depgraph.Descendants(i)); + } } } From eaab55ffc8102140086297877747b7fa07b419eb Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 9 Sep 2024 14:51:08 -0400 Subject: [PATCH 2/7] clusterlin: rework DepGraphFormatter::Unser This commit does not change the serialization format. Its purpose is making a few changes already in order to reduce the diff size of the later commit that introduces support for holes in DepGraph. The previous approach was to immediately construct a transaction as soon as its feerate was known in a preliminary position, and then undo that, and place it in the correct position once the position information is known (such that a deserialization error in between would not result in an inconsistent state). The new approach is to delay the actual transaction creation until all its information is known, avoiding the need to undo and redo. This requires a different means of determining whether dependencies are redundant, but that has the advantage that a later commit can apply all dependencies at once, reducing the complexity of deserialization. --- src/test/util/cluster_linearize.h | 52 ++++++++++++++++++------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index 377bfa19fbaef..1ee0f7c2a7997 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -189,10 +189,17 @@ struct DepGraphFormatter /** Mapping from serialization order to cluster order, used later to reconstruct the * cluster order. */ std::vector reordering; + /** How big the entries vector in the reconstructed depgraph will be (before the + * introduction of holes in a further commit, this always equals reordering.size()). */ + ClusterIndex total_size{0}; // Read transactions in topological order. - try { - while (true) { + while (true) { + FeeFrac new_feerate; //!< The new transaction's fee and size. + SetType new_ancestors; //!< The new transaction's ancestors (excluding itself). + uint64_t diff{0}; //!< How many potential parents/insertions we have to skip. + bool read_error{false}; + try { // Read size. Size 0 signifies the end of the DepGraph. int32_t size; s >> VARINT_MODE(size, VarIntMode::NONNEGATIVE_SIGNED); @@ -204,21 +211,18 @@ struct DepGraphFormatter s >> VARINT(coded_fee); coded_fee &= 0xFFFFFFFFFFFFF; // Enough for fee between -21M...21M BTC. static_assert(0xFFFFFFFFFFFFF > uint64_t{2} * 21000000 * 100000000); - auto fee = UnsignedToSigned(coded_fee); - // Extend topo_depgraph with the new transaction (preliminarily at the end). - auto topo_idx = topo_depgraph.AddTransaction({fee, size}); - reordering.push_back(reordering.size()); + new_feerate = {UnsignedToSigned(coded_fee), size}; // Read dependency information. - uint64_t diff = 0; //!< How many potential parents we have to skip. + auto topo_idx = reordering.size(); s >> VARINT(diff); for (ClusterIndex dep_dist = 0; dep_dist < topo_idx; ++dep_dist) { /** Which topo_depgraph index we are currently considering as parent of topo_idx. */ ClusterIndex dep_topo_idx = topo_idx - 1 - dep_dist; // Ignore transactions which are already known ancestors of topo_idx. - if (topo_depgraph.Descendants(dep_topo_idx)[topo_idx]) continue; + if (new_ancestors[dep_topo_idx]) continue; if (diff == 0) { // When the skip counter has reached 0, add an actual dependency. - topo_depgraph.AddDependency(dep_topo_idx, topo_idx); + new_ancestors |= topo_depgraph.Ancestors(dep_topo_idx); // And read the number of skips after it. s >> VARINT(diff); } else { @@ -226,20 +230,24 @@ struct DepGraphFormatter --diff; } } - // If we reach this point, we can interpret the remaining skip value as how far - // from the end of reordering the new transaction should be placed (wrapping - // around), so remove the preliminary position it was put in above (which was to - // make sure that if a deserialization exception occurs, the new transaction still - // has some entry in reordering). - reordering.pop_back(); - ClusterIndex insert_distance = diff % (reordering.size() + 1); - // And then update reordering to reflect this new transaction's insertion. - for (auto& pos : reordering) { - pos += (pos >= reordering.size() - insert_distance); - } - reordering.push_back(reordering.size() - insert_distance); + } catch (const std::ios_base::failure&) { + // Continue even if a read error was encountered. + read_error = true; } - } catch (const std::ios_base::failure&) {} + // Construct a new transaction whenever we made it past the new_feerate construction. + if (new_feerate.IsEmpty()) break; + assert(reordering.size() < SetType::Size()); + auto topo_idx = topo_depgraph.AddTransaction(new_feerate); + for (auto parent : new_ancestors) topo_depgraph.AddDependency(parent, topo_idx); + diff %= total_size + 1; + // Insert the new transaction at distance diff back from the end. + for (auto& pos : reordering) { + pos += (pos >= total_size - diff); + } + reordering.push_back(total_size++ - diff); + // Stop if a read error was encountered during deserialization. + if (read_error) break; + } // Construct the original cluster order depgraph. depgraph = DepGraph(topo_depgraph, reordering); From abf50649d13018bf40c5803730a03053737efeee Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 8 Sep 2024 17:42:25 -0400 Subject: [PATCH 3/7] clusterlin: simplify DepGraphFormatter::Ser This does not change the serialization format. It turns out that it is unnecessary to keep track of the order of transactions in the so-far reconstructed DepGraph to decide how far from the end to insert a new transaction. --- src/test/util/cluster_linearize.h | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index 1ee0f7c2a7997..a9ae4ff12a828 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -134,9 +134,8 @@ struct DepGraphFormatter }); /** Which transactions the deserializer already knows when it has deserialized what has - * been serialized here so far, and in what order. */ - std::vector rebuilt_order; - rebuilt_order.reserve(depgraph.TxCount()); + * been serialized here so far. */ + SetType done; // Loop over the transactions in topological order. for (ClusterIndex topo_idx = 0; topo_idx < topo_order.size(); ++topo_idx) { @@ -166,14 +165,11 @@ struct DepGraphFormatter } } // Write position information. - ClusterIndex insert_distance = 0; - while (insert_distance < rebuilt_order.size()) { - // Loop to find how far from the end in rebuilt_order to insert. - if (idx > *(rebuilt_order.end() - 1 - insert_distance)) break; - ++insert_distance; - } - rebuilt_order.insert(rebuilt_order.end() - insert_distance, idx); - s << VARINT(diff + insert_distance); + // The new transaction is to be inserted N positions back from the end of the cluster. + // Emit N to indicate that that many insertion choices are skipped. + auto skips = (done - SetType::Fill(idx)).Count(); + s << VARINT(diff + skips); + done.Set(idx); } // Output a final 0 to denote the end of the graph. From 75b5d42419ed1286fc9557bc97ec5bac3f9be837 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 4 Sep 2024 15:14:54 -0400 Subject: [PATCH 4/7] clusterlin: make DepGraph::AddDependency support multiple dependencies at once This changes DepGraph::AddDependency into DepGraph::AddDependencies, which takes in a single child, but a set of parent transactions, making them all dependencies at once. This is important for performance. N transactions can have O(N^2) parents combined, so constructing a full DepGraph using just AddDependency (which is O(N) on its own) could take O(N^3) time, while doing the same with AddDependencies (also O(N) on its own) only takes O(N^2). Notably, this matters for DepGraphFormatter::Unser, which goes from O(N^3) to O(N^2). Co-Authored-By: Greg Sanders --- src/bench/cluster_linearize.cpp | 24 ++++----- src/cluster_linearize.h | 70 +++++++++---------------- src/test/fuzz/cluster_linearize.cpp | 80 +++++++++++++++++++++-------- src/test/util/cluster_linearize.h | 2 +- 4 files changed, 96 insertions(+), 80 deletions(-) diff --git a/src/bench/cluster_linearize.cpp b/src/bench/cluster_linearize.cpp index cf071dda2d1a5..7d011975ddbec 100644 --- a/src/bench/cluster_linearize.cpp +++ b/src/bench/cluster_linearize.cpp @@ -28,7 +28,7 @@ DepGraph MakeLinearGraph(ClusterIndex ntx) DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({-int32_t(i), 1}); - if (i > 0) depgraph.AddDependency(i - 1, i); + if (i > 0) depgraph.AddDependencies(SetType::Singleton(i - 1), i); } return depgraph; } @@ -43,7 +43,7 @@ DepGraph MakeWideGraph(ClusterIndex ntx) DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({int32_t(i) + 1, 1}); - if (i > 0) depgraph.AddDependency(0, i); + if (i > 0) depgraph.AddDependencies(SetType::Singleton(0), i); } return depgraph; } @@ -70,19 +70,19 @@ DepGraph MakeHardGraph(ClusterIndex ntx) depgraph.AddTransaction({1, 2}); } else if (i == 1) { depgraph.AddTransaction({14, 2}); - depgraph.AddDependency(0, 1); + depgraph.AddDependencies(SetType::Singleton(0), 1); } else if (i == 2) { depgraph.AddTransaction({6, 1}); - depgraph.AddDependency(2, 1); + depgraph.AddDependencies(SetType::Singleton(2), 1); } else if (i == 3) { depgraph.AddTransaction({5, 1}); - depgraph.AddDependency(2, 3); + depgraph.AddDependencies(SetType::Singleton(2), 3); } else if ((i & 1) == 0) { depgraph.AddTransaction({7, 1}); - depgraph.AddDependency(i - 1, i); + depgraph.AddDependencies(SetType::Singleton(i - 1), i); } else { depgraph.AddTransaction({5, 1}); - depgraph.AddDependency(i, 4); + depgraph.AddDependencies(SetType::Singleton(i), 4); } } else { // Even cluster size. @@ -98,16 +98,16 @@ DepGraph MakeHardGraph(ClusterIndex ntx) depgraph.AddTransaction({1, 1}); } else if (i == 1) { depgraph.AddTransaction({3, 1}); - depgraph.AddDependency(0, 1); + depgraph.AddDependencies(SetType::Singleton(0), 1); } else if (i == 2) { depgraph.AddTransaction({1, 1}); - depgraph.AddDependency(0, 2); + depgraph.AddDependencies(SetType::Singleton(0), 2); } else if (i & 1) { depgraph.AddTransaction({4, 1}); - depgraph.AddDependency(i - 1, i); + depgraph.AddDependencies(SetType::Singleton(i - 1), i); } else { depgraph.AddTransaction({0, 1}); - depgraph.AddDependency(i, 3); + depgraph.AddDependencies(SetType::Singleton(i), 3); } } } @@ -195,7 +195,7 @@ void BenchMergeLinearizationsWorstCase(ClusterIndex ntx, benchmark::Bench& bench DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({i, 1}); - if (i) depgraph.AddDependency(0, i); + if (i) depgraph.AddDependencies(SetType::Singleton(0), i); } std::vector lin1; std::vector lin2; diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index 8467f3f08be0c..bcc455874d798 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -86,35 +86,13 @@ class DepGraph * * Complexity: O(N^2) where N=cluster.size(). */ - explicit DepGraph(const Cluster& cluster) noexcept : entries(cluster.size()) + explicit DepGraph(const Cluster& cluster) noexcept : DepGraph(cluster.size()) { for (ClusterIndex i = 0; i < cluster.size(); ++i) { // Fill in fee and size. entries[i].feerate = cluster[i].first; - // Fill in direct parents as ancestors. - entries[i].ancestors = cluster[i].second; - // Make sure transactions are ancestors of themselves. - entries[i].ancestors.Set(i); - } - - // Propagate ancestor information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - // At this point, entries[a].ancestors[b] is true iff b is an ancestor of a and there - // is a path from a to b through the subgraph consisting of {a, b} union - // {0, 1, ..., (i-1)}. - SetType to_merge = entries[i].ancestors; - for (ClusterIndex j = 0; j < entries.size(); ++j) { - if (entries[j].ancestors[i]) { - entries[j].ancestors |= to_merge; - } - } - } - - // Fill in descendant information by transposing the ancestor information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - for (auto j : entries[i].ancestors) { - entries[j].descendants.Set(i); - } + // Fill in dependencies. + AddDependencies(cluster[i].second, i); } } @@ -122,21 +100,16 @@ class DepGraph * * Complexity: O(N^2) where N=depgraph.TxCount(). */ - DepGraph(const DepGraph& depgraph, Span mapping) noexcept : entries(depgraph.TxCount()) + DepGraph(const DepGraph& depgraph, Span mapping) noexcept : DepGraph(depgraph.TxCount()) { Assert(mapping.size() == depgraph.TxCount()); - // Fill in fee, size, ancestors. for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - const auto& input = depgraph.entries[i]; - auto& output = entries[mapping[i]]; - output.feerate = input.feerate; - for (auto j : input.ancestors) output.ancestors.Set(mapping[j]); - } - // Fill in descendant information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - for (auto j : entries[i].ancestors) { - entries[j].descendants.Set(i); - } + // Fill in fee and size. + entries[mapping[i]].feerate = depgraph.entries[i].feerate; + // Fill in dependencies by mapping direct parents. + SetType parents; + for (auto j : depgraph.GetReducedParents(i)) parents.Set(mapping[j]); + AddDependencies(parents, mapping[i]); } } @@ -164,21 +137,26 @@ class DepGraph return new_idx; } - /** Modify this transaction graph, adding a dependency between a specified parent and child. + /** Modify this transaction graph, adding multiple parents to a specified child. * * Complexity: O(N) where N=TxCount(). - **/ - void AddDependency(ClusterIndex parent, ClusterIndex child) noexcept + */ + void AddDependencies(const SetType& parents, ClusterIndex child) noexcept { - // Bail out if dependency is already implied. - if (entries[child].ancestors[parent]) return; - // To each ancestor of the parent, add as descendants the descendants of the child. + // Compute the ancestors of parents that are not already ancestors of child. + SetType par_anc; + for (auto par : parents - Ancestors(child)) { + par_anc |= Ancestors(par); + } + par_anc -= Ancestors(child); + // Bail out if there are no such ancestors. + if (par_anc.None()) return; + // To each such ancestor, add as descendants the descendants of the child. const auto& chl_des = entries[child].descendants; - for (auto anc_of_par : Ancestors(parent)) { + for (auto anc_of_par : par_anc) { entries[anc_of_par].descendants |= chl_des; } - // To each descendant of the child, add as ancestors the ancestors of the parent. - const auto& par_anc = entries[parent].ancestors; + // To each descendant of the child, add those ancestors. for (auto dec_of_chl : Descendants(child)) { entries[dec_of_chl].ancestors |= par_anc; } diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index 4976afbaad807..bf7db4482c6c9 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -176,7 +177,7 @@ void MakeConnected(DepGraph& depgraph) while (todo.Any()) { auto nextcomp = depgraph.FindConnectedComponent(todo); Assume(depgraph.IsConnected(nextcomp)); - depgraph.AddDependency(comp.Last(), nextcomp.First()); + depgraph.AddDependencies(BS::Singleton(comp.Last()), nextcomp.First()); todo -= nextcomp; comp = nextcomp; } @@ -240,32 +241,65 @@ std::vector ReadLinearization(const DepGraph& depgraph, SpanRe } // namespace -FUZZ_TARGET(clusterlin_add_dependency) +FUZZ_TARGET(clusterlin_add_dependencies) { - // Verify that computing a DepGraph from a cluster, or building it step by step using AddDependency - // have the same effect. + // Verify that computing a DepGraph from a cluster, or building it step by step using + // AddDependencies has the same effect. - // Construct a cluster of a certain length, with no dependencies. FuzzedDataProvider provider(buffer.data(), buffer.size()); - auto num_tx = provider.ConsumeIntegralInRange(2, 32); + auto rng_seed = provider.ConsumeIntegral(); + InsecureRandomContext rng(rng_seed); + + // Construct a cluster of a certain length, with no dependencies. + auto num_tx = provider.ConsumeIntegralInRange(2, TestBitSet::Size()); Cluster cluster(num_tx, std::pair{FeeFrac{0, 1}, TestBitSet{}}); // Construct the corresponding DepGraph object (also no dependencies). - DepGraph depgraph(cluster); - SanityCheck(depgraph); - // Read (parent, child) pairs, and add them to the cluster and depgraph. - LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size() * TestBitSet::Size()) { - auto parent = provider.ConsumeIntegralInRange(0, num_tx - 1); - auto child = provider.ConsumeIntegralInRange(0, num_tx - 2); - child += (child >= parent); - cluster[child].second.Set(parent); - depgraph.AddDependency(parent, child); - assert(depgraph.Ancestors(child)[parent]); - assert(depgraph.Descendants(parent)[child]); + DepGraph depgraph_batch(cluster); + SanityCheck(depgraph_batch); + // Read (parents, child) pairs, and add the dependencies to the cluster and depgraph. + std::vector> deps_list; + LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size()) { + const auto parents_mask = provider.ConsumeIntegralInRange(0, (uint64_t{1} << num_tx) - 1); + auto child = provider.ConsumeIntegralInRange(0, num_tx - 1); + + auto parents_mask_shifted = parents_mask; + TestBitSet deps; + for (ClusterIndex i = 0; i < num_tx; ++i) { + if (parents_mask_shifted & 1) { + deps.Set(i); + cluster[child].second.Set(i); + } + parents_mask_shifted >>= 1; + } + assert(parents_mask_shifted == 0); + depgraph_batch.AddDependencies(deps, child); + for (auto i : deps) { + deps_list.emplace_back(i, child); + assert(depgraph_batch.Ancestors(child)[i]); + assert(depgraph_batch.Descendants(i)[child]); + } } // Sanity check the result. - SanityCheck(depgraph); + SanityCheck(depgraph_batch); // Verify that the resulting DepGraph matches one recomputed from the cluster. - assert(DepGraph(cluster) == depgraph); + assert(DepGraph(cluster) == depgraph_batch); + + DepGraph depgraph_individual; + // Add all transactions to depgraph_individual. + for (const auto& [feerate, parents] : cluster) { + depgraph_individual.AddTransaction(feerate); + } + SanityCheck(depgraph_individual); + // Add all individual dependencies to depgraph_individual in randomized order. + std::shuffle(deps_list.begin(), deps_list.end(), rng); + for (auto [parent, child] : deps_list) { + depgraph_individual.AddDependencies(TestBitSet::Singleton(parent), child); + assert(depgraph_individual.Ancestors(child)[parent]); + assert(depgraph_individual.Descendants(parent)[child]); + } + // Sanity check and compare again the batched version. + SanityCheck(depgraph_individual); + assert(depgraph_individual == depgraph_batch); } FUZZ_TARGET(clusterlin_cluster_serialization) @@ -897,12 +931,16 @@ FUZZ_TARGET(clusterlin_postlinearize_tree) if (direction & 1) { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto children = depgraph_gen.GetReducedChildren(i); - if (children.Any()) depgraph_tree.AddDependency(i, children.First()); + if (children.Any()) { + depgraph_tree.AddDependencies(TestBitSet::Singleton(i), children.First()); + } } } else { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto parents = depgraph_gen.GetReducedParents(i); - if (parents.Any()) depgraph_tree.AddDependency(parents.First(), i); + if (parents.Any()) { + depgraph_tree.AddDependencies(TestBitSet::Singleton(parents.First()), i); + } } } diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index a9ae4ff12a828..bec4d41d38519 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -234,7 +234,7 @@ struct DepGraphFormatter if (new_feerate.IsEmpty()) break; assert(reordering.size() < SetType::Size()); auto topo_idx = topo_depgraph.AddTransaction(new_feerate); - for (auto parent : new_ancestors) topo_depgraph.AddDependency(parent, topo_idx); + topo_depgraph.AddDependencies(new_ancestors, topo_idx); diff %= total_size + 1; // Insert the new transaction at distance diff back from the end. for (auto& pos : reordering) { From 0606e66fdbb914f984433d8950f0c32b5fb871f3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 9 Sep 2024 15:12:15 -0400 Subject: [PATCH 5/7] clusterlin: add DepGraph::RemoveTransactions and support for holes in DepGraph This commits introduces support in DepGraph for the transaction positions to be non-continuous. Specifically, it adds: * DepGraph::RemoveTransactions which removes 0 or more positions from a DepGraph. * DepGraph::Positions() to get a set of which positions are in use. * DepGraph::PositionRange() to get the highest used position in a DepGraph + 1. In addition, it extends the DepGraphFormatter format to support holes in a compatible way (it serializes non-holey DepGraphs identically to the old code, and deserializes them the same way) --- src/cluster_linearize.h | 120 +++++++++++++++++++++----- src/test/cluster_linearize_tests.cpp | 39 +++++++++ src/test/feefrac_tests.cpp | 2 +- src/test/fuzz/cluster_linearize.cpp | 51 +++++++---- src/test/util/cluster_linearize.h | 124 +++++++++++++++++++-------- src/util/check.h | 2 +- src/util/feefrac.h | 8 +- 7 files changed, 267 insertions(+), 79 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index bcc455874d798..a2450f549084b 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -57,9 +57,20 @@ class DepGraph /** Data for each transaction, in the same order as the Cluster it was constructed from. */ std::vector entries; + /** Which positions are used. */ + SetType m_used; + public: /** Equality operator (primarily for testing purposes). */ - friend bool operator==(const DepGraph&, const DepGraph&) noexcept = default; + friend bool operator==(const DepGraph& a, const DepGraph& b) noexcept + { + if (a.m_used != b.m_used) return false; + // Only compare the used positions within the entries vector. + for (auto idx : a.m_used) { + if (a.entries[idx] != b.entries[idx]) return false; + } + return true; + } // Default constructors. DepGraph() noexcept = default; @@ -80,6 +91,7 @@ class DepGraph entries[i].ancestors = SetType::Singleton(i); entries[i].descendants = SetType::Singleton(i); } + m_used = SetType::Fill(ntx); } /** Construct a DepGraph object given a cluster. @@ -97,24 +109,50 @@ class DepGraph } /** Construct a DepGraph object given another DepGraph and a mapping from old to new. + * + * @param depgraph The original DepGraph that is being remapped. + * + * @param mapping A Span such that mapping[i] gives the position in the new DepGraph + * for position i in the old depgraph. Its size must be equal to + * depgraph.PositionRange(). The value of mapping[i] is ignored if + * position i is a hole in depgraph (i.e., if !depgraph.Positions()[i]). + * + * @param pos_range The PositionRange() for the new DepGraph. It must equal the largest + * value in mapping for any used position in depgraph plus 1, or 0 if + * depgraph.TxCount() == 0. * * Complexity: O(N^2) where N=depgraph.TxCount(). */ - DepGraph(const DepGraph& depgraph, Span mapping) noexcept : DepGraph(depgraph.TxCount()) + DepGraph(const DepGraph& depgraph, Span mapping, ClusterIndex pos_range) noexcept : entries(pos_range) { - Assert(mapping.size() == depgraph.TxCount()); - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + Assume(mapping.size() == depgraph.PositionRange()); + Assume((pos_range == 0) == (depgraph.TxCount() == 0)); + for (ClusterIndex i : depgraph.Positions()) { + auto new_idx = mapping[i]; + Assume(new_idx < pos_range); + // Add transaction. + entries[new_idx].ancestors = SetType::Singleton(new_idx); + entries[new_idx].descendants = SetType::Singleton(new_idx); + m_used.Set(new_idx); // Fill in fee and size. - entries[mapping[i]].feerate = depgraph.entries[i].feerate; + entries[new_idx].feerate = depgraph.entries[i].feerate; + } + for (ClusterIndex i : depgraph.Positions()) { // Fill in dependencies by mapping direct parents. SetType parents; for (auto j : depgraph.GetReducedParents(i)) parents.Set(mapping[j]); AddDependencies(parents, mapping[i]); } + // Verify that the provided pos_range was correct (no unused positions at the end). + Assume(m_used.None() ? (pos_range == 0) : (pos_range == m_used.Last() + 1)); } + /** Get the set of transactions positions in use. Complexity: O(1). */ + const SetType& Positions() const noexcept { return m_used; } + /** Get the range of positions in this DepGraph. All entries in Positions() are in [0, PositionRange() - 1]. */ + ClusterIndex PositionRange() const noexcept { return entries.size(); } /** Get the number of transactions in the graph. Complexity: O(1). */ - auto TxCount() const noexcept { return entries.size(); } + auto TxCount() const noexcept { return m_used.Count(); } /** Get the feerate of a given transaction i. Complexity: O(1). */ const FeeFrac& FeeRate(ClusterIndex i) const noexcept { return entries[i].feerate; } /** Get the mutable feerate of a given transaction i. Complexity: O(1). */ @@ -124,25 +162,59 @@ class DepGraph /** Get the descendants of a given transaction i. Complexity: O(1). */ const SetType& Descendants(ClusterIndex i) const noexcept { return entries[i].descendants; } - /** Add a new unconnected transaction to this transaction graph (at the end), and return its - * ClusterIndex. + /** Add a new unconnected transaction to this transaction graph (in the first available + * position), and return its ClusterIndex. * * Complexity: O(1) (amortized, due to resizing of backing vector). */ ClusterIndex AddTransaction(const FeeFrac& feefrac) noexcept { - Assume(TxCount() < SetType::Size()); - ClusterIndex new_idx = TxCount(); - entries.emplace_back(feefrac, SetType::Singleton(new_idx), SetType::Singleton(new_idx)); + static constexpr auto ALL_POSITIONS = SetType::Fill(SetType::Size()); + auto available = ALL_POSITIONS - m_used; + Assume(available.Any()); + ClusterIndex new_idx = available.First(); + if (new_idx == entries.size()) { + entries.emplace_back(feefrac, SetType::Singleton(new_idx), SetType::Singleton(new_idx)); + } else { + entries[new_idx] = Entry(feefrac, SetType::Singleton(new_idx), SetType::Singleton(new_idx)); + } + m_used.Set(new_idx); return new_idx; } + /** Remove the specified positions from this DepGraph. + * + * The specified positions will no longer be part of Positions(), and dependencies with them are + * removed. Note that due to DepGraph only tracking ancestors/descendants (and not direct + * dependencies), if a parent is removed while a grandparent remains, the grandparent will + * remain an ancestor. + * + * Complexity: O(N) where N=TxCount(). + */ + void RemoveTransactions(const SetType& del) noexcept + { + m_used -= del; + // Remove now-unused trailing entries. + while (!entries.empty() && !m_used[entries.size() - 1]) { + entries.pop_back(); + } + // Remove the deleted transactions from ancestors/descendants of other transactions. Note + // that the deleted positions will retain old feerate and dependency information. This does + // not matter as they will be overwritten by AddTransaction if they get used again. + for (auto& entry : entries) { + entry.ancestors &= m_used; + entry.descendants &= m_used; + } + } + /** Modify this transaction graph, adding multiple parents to a specified child. * * Complexity: O(N) where N=TxCount(). */ void AddDependencies(const SetType& parents, ClusterIndex child) noexcept { + Assume(m_used[child]); + Assume(parents.IsSubsetOf(m_used)); // Compute the ancestors of parents that are not already ancestors of child. SetType par_anc; for (auto par : parents - Ancestors(child)) { @@ -257,7 +329,7 @@ class DepGraph * * Complexity: O(TxCount()). */ - bool IsConnected() const noexcept { return IsConnected(SetType::Fill(TxCount())); } + bool IsConnected() const noexcept { return IsConnected(m_used); } /** Append the entries of select to list in a topologically valid order. * @@ -507,11 +579,11 @@ class AncestorCandidateFinder */ AncestorCandidateFinder(const DepGraph& depgraph LIFETIMEBOUND) noexcept : m_depgraph(depgraph), - m_todo{SetType::Fill(depgraph.TxCount())}, - m_ancestor_set_feerates(depgraph.TxCount()) + m_todo{depgraph.Positions()}, + m_ancestor_set_feerates(depgraph.PositionRange()) { // Precompute ancestor-set feerates. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : m_depgraph.Positions()) { /** The remaining ancestors for transaction i. */ SetType anc_to_add = m_depgraph.Ancestors(i); FeeFrac anc_feerate; @@ -634,22 +706,26 @@ class SearchCandidateFinder SearchCandidateFinder(const DepGraph& depgraph, uint64_t rng_seed) noexcept : m_rng(rng_seed), m_sorted_to_original(depgraph.TxCount()), - m_original_to_sorted(depgraph.TxCount()), - m_todo(SetType::Fill(depgraph.TxCount())) + m_original_to_sorted(depgraph.PositionRange()) { - // Determine reordering mapping, by sorting by decreasing feerate. - std::iota(m_sorted_to_original.begin(), m_sorted_to_original.end(), ClusterIndex{0}); + // Determine reordering mapping, by sorting by decreasing feerate. Unusued positions are + // not included, as they will never be looked up anyway. + ClusterIndex sorted_pos{0}; + for (auto i : depgraph.Positions()) { + m_sorted_to_original[sorted_pos++] = i; + } std::sort(m_sorted_to_original.begin(), m_sorted_to_original.end(), [&](auto a, auto b) { auto feerate_cmp = depgraph.FeeRate(a) <=> depgraph.FeeRate(b); if (feerate_cmp == 0) return a < b; return feerate_cmp > 0; }); // Compute reverse mapping. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i = 0; i < m_sorted_to_original.size(); ++i) { m_original_to_sorted[m_sorted_to_original[i]] = i; } // Compute reordered dependency graph. - m_sorted_depgraph = DepGraph(depgraph, m_original_to_sorted); + m_sorted_depgraph = DepGraph(depgraph, m_original_to_sorted, m_sorted_to_original.size()); + m_todo = m_sorted_depgraph.Positions(); } /** Check whether any unlinearized transactions remain. */ @@ -1161,7 +1237,7 @@ void PostLinearize(const DepGraph& depgraph, Span lineari // During an even pass, the diagram above would correspond to linearization [2,3,0,1], with // groups [2] and [3,0,1]. - std::vector entries(linearization.size() + 1); + std::vector entries(depgraph.PositionRange() + 1); // Perform two passes over the linearization. for (int pass = 0; pass < 2; ++pass) { diff --git a/src/test/cluster_linearize_tests.cpp b/src/test/cluster_linearize_tests.cpp index d15e783ea15d8..cb87dcbffbe12 100644 --- a/src/test/cluster_linearize_tests.cpp +++ b/src/test/cluster_linearize_tests.cpp @@ -18,6 +18,10 @@ using namespace cluster_linearize; namespace { +/** Special magic value that indicates to TestDepGraphSerialization that a cluster entry represents + * a hole. */ +constexpr std::pair HOLE{FeeFrac{0, 0x3FFFFF}, {}}; + template void TestDepGraphSerialization(const Cluster& cluster, const std::string& hexenc) { @@ -26,6 +30,13 @@ void TestDepGraphSerialization(const Cluster& cluster, const std::strin // Run normal sanity and correspondence checks, which includes a round-trip test. VerifyDepGraphFromCluster(cluster, depgraph); + // Remove holes (which are expected to be present as HOLE entries in cluster). + SetType holes; + for (ClusterIndex i = 0; i < cluster.size(); ++i) { + if (cluster[i] == HOLE) holes.Set(i); + } + depgraph.RemoveTransactions(holes); + // There may be multiple serializations of the same graph, but DepGraphFormatter's serializer // only produces one of those. Verify that hexenc matches that canonical serialization. std::vector encoding; @@ -133,6 +144,34 @@ BOOST_AUTO_TEST_CASE(depgraph_ser_tests) skip insertion C): D,A,B,E,C */ "00" /* end of graph */ ); + + // Transactions: A(1,2), B(3,1), C(2,1), D(1,3), E(1,1). Deps: C->A, D->A, D->B, E->D. + // In order: [_, D, _, _, A, _, B, _, _, _, E, _, _, C] (_ being holes). Internally serialized + // in order A,B,C,D,E. + TestDepGraphSerialization( + {HOLE, {{1, 3}, {4, 6}}, HOLE, HOLE, {{1, 2}, {}}, HOLE, {{3, 1}, {}}, HOLE, HOLE, HOLE, {{1, 1}, {1}}, HOLE, HOLE, {{2, 1}, {4}}}, + "02" /* A size */ + "02" /* A fee */ + "03" /* A insertion position (3 holes): _, _, _, A */ + "01" /* B size */ + "06" /* B fee */ + "06" /* B insertion position (skip B->A dependency, skip 4 inserts, add 1 hole): _, _, _, A, _, B */ + "01" /* C size */ + "04" /* C fee */ + "01" /* C->A dependency (skip C->B dependency) */ + "0b" /* C insertion position (skip 6 inserts, add 5 holes): _, _, _, A, _, B, _, _, _, _, _, C */ + "03" /* D size */ + "02" /* D fee */ + "01" /* D->B dependency (skip D->C dependency) */ + "00" /* D->A dependency (no skips) */ + "0b" /* D insertion position (skip 11 inserts): _, D, _, _, A, _, B, _, _, _, _, _, C */ + "01" /* E size */ + "02" /* E fee */ + "00" /* E->D dependency (no skips) */ + "04" /* E insertion position (skip E->C dependency, E->B and E->A are implied, skip 3 + inserts): _, D, _, _, A, _, B, _, _, _, E, _, _, C */ + "00" /* end of graph */ + ); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp index 5af3c3d7ed8ed..41c9c0a633703 100644 --- a/src/test/feefrac_tests.cpp +++ b/src/test/feefrac_tests.cpp @@ -15,7 +15,7 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) FeeFrac sum{1500, 400}; FeeFrac diff{500, -200}; FeeFrac empty{0, 0}; - FeeFrac zero_fee{0, 1}; // zero-fee allowed + [[maybe_unused]] FeeFrac zero_fee{0, 1}; // zero-fee allowed BOOST_CHECK(empty == FeeFrac{}); // same as no-args diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index bf7db4482c6c9..e75883878e4c3 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -37,7 +37,7 @@ class SimpleCandidateFinder public: /** Construct an SimpleCandidateFinder for a given graph. */ SimpleCandidateFinder(const DepGraph& depgraph LIFETIMEBOUND) noexcept : - m_depgraph(depgraph), m_todo{SetType::Fill(depgraph.TxCount())} {} + m_depgraph(depgraph), m_todo{depgraph.Positions()} {} /** Remove a set of transactions from the set of to-be-linearized ones. */ void MarkDone(SetType select) noexcept { m_todo -= select; } @@ -107,7 +107,7 @@ class ExhaustiveCandidateFinder public: /** Construct an ExhaustiveCandidateFinder for a given graph. */ ExhaustiveCandidateFinder(const DepGraph& depgraph LIFETIMEBOUND) noexcept : - m_depgraph(depgraph), m_todo{SetType::Fill(depgraph.TxCount())} {} + m_depgraph(depgraph), m_todo{depgraph.Positions()} {} /** Remove a set of transactions from the set of to-be-linearized ones. */ void MarkDone(SetType select) noexcept { m_todo -= select; } @@ -153,7 +153,7 @@ std::pair, bool> SimpleLinearize(const DepGraph linearization; SimpleCandidateFinder finder(depgraph); - SetType todo = SetType::Fill(depgraph.TxCount()); + SetType todo = depgraph.Positions(); bool optimal = true; while (todo.Any()) { auto [candidate, iterations_done] = finder.FindCandidateSet(max_iterations); @@ -170,7 +170,7 @@ std::pair, bool> SimpleLinearize(const DepGraph void MakeConnected(DepGraph& depgraph) { - auto todo = BS::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); auto comp = depgraph.FindConnectedComponent(todo); Assume(depgraph.IsConnected(comp)); todo -= comp; @@ -206,7 +206,7 @@ template std::vector ReadLinearization(const DepGraph& depgraph, SpanReader& reader) { std::vector linearization; - TestBitSet todo = TestBitSet::Fill(depgraph.TxCount()); + TestBitSet todo = depgraph.Positions(); // In every iteration one topologically-valid transaction is appended to linearization. while (todo.Any()) { // Compute the set of transactions with no not-yet-included ancestors. @@ -327,6 +327,17 @@ FUZZ_TARGET(clusterlin_cluster_serialization) // check for the serialization). DepGraph depgraph(cluster); VerifyDepGraphFromCluster(cluster, depgraph); + + // Remove an arbitrary subset (in order to construct a graph with holes) and verify that it + // still sanity checks (incl. round-tripping serialization). + uint64_t del = provider.ConsumeIntegralInRange(1, (uint64_t{1} << TestBitSet::Size()) - 1); + TestBitSet setdel; + for (ClusterIndex i = 0; i < TestBitSet::Size(); ++i) { + if (del & 1) setdel.Set(i); + del >>= 1; + } + depgraph.RemoveTransactions(setdel); + SanityCheck(depgraph); } FUZZ_TARGET(clusterlin_depgraph_serialization) @@ -356,7 +367,7 @@ FUZZ_TARGET(clusterlin_components) reader >> Using(depgraph); } catch (const std::ios_base::failure&) {} - TestBitSet todo = TestBitSet::Fill(depgraph.TxCount()); + TestBitSet todo = depgraph.Positions(); while (todo.Any()) { // Find a connected component inside todo. auto component = depgraph.FindConnectedComponent(todo); @@ -367,7 +378,7 @@ FUZZ_TARGET(clusterlin_components) // If todo is the entire graph, and the entire graph is connected, then the component must // be the entire graph. - if (todo == TestBitSet::Fill(depgraph.TxCount())) { + if (todo == depgraph.Positions()) { assert((component == todo) == depgraph.IsConnected()); } @@ -404,7 +415,7 @@ FUZZ_TARGET(clusterlin_components) reader >> VARINT(subset_bits); } catch (const std::ios_base::failure&) {} TestBitSet subset; - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : depgraph.Positions()) { if (todo[i]) { if (subset_bits & 1) subset.Set(i); subset_bits >>= 1; @@ -457,7 +468,7 @@ FUZZ_TARGET(clusterlin_chunking) } // Naively recompute the chunks (each is the highest-feerate prefix of what remains). - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); for (const auto& chunk_feerate : chunking) { assert(todo.Any()); SetInfo accumulator, best; @@ -488,7 +499,7 @@ FUZZ_TARGET(clusterlin_ancestor_finder) } catch (const std::ios_base::failure&) {} AncestorCandidateFinder anc_finder(depgraph); - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); while (todo.Any()) { // Call the ancestor finder's FindCandidateSet for what remains of the graph. assert(!anc_finder.AllDone()); @@ -553,7 +564,7 @@ FUZZ_TARGET(clusterlin_search_finder) ExhaustiveCandidateFinder exh_finder(depgraph); AncestorCandidateFinder anc_finder(depgraph); - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); while (todo.Any()) { assert(!src_finder.AllDone()); assert(!smp_finder.AllDone()); @@ -657,7 +668,7 @@ FUZZ_TARGET(clusterlin_linearization_chunking) } catch (const std::ios_base::failure&) {} // Retrieve a topologically-valid subset of depgraph. - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); auto subset = SetInfo(depgraph, ReadTopologicalSet(depgraph, todo, reader)); // Retrieve a valid linearization for depgraph. @@ -840,8 +851,8 @@ FUZZ_TARGET(clusterlin_linearize) // Only for very small clusters, test every topologically-valid permutation. if (depgraph.TxCount() <= 7) { - std::vector perm_linearization(depgraph.TxCount()); - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) perm_linearization[i] = i; + std::vector perm_linearization; + for (ClusterIndex i : depgraph.Positions()) perm_linearization.push_back(i); // Iterate over all valid permutations. do { // Determine whether perm_linearization is topological. @@ -925,9 +936,17 @@ FUZZ_TARGET(clusterlin_postlinearize_tree) // Now construct a new graph, copying the nodes, but leaving only the first parent (even // direction) or the first child (odd direction). DepGraph depgraph_tree; - for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { - depgraph_tree.AddTransaction(depgraph_gen.FeeRate(i)); + for (ClusterIndex i = 0; i < depgraph_gen.PositionRange(); ++i) { + if (depgraph_gen.Positions()[i]) { + depgraph_tree.AddTransaction(depgraph_gen.FeeRate(i)); + } else { + // For holes, add a dummy transaction which is deleted below, so that non-hole + // transactions retain their position. + depgraph_tree.AddTransaction(FeeFrac{}); + } } + depgraph_tree.RemoveTransactions(TestBitSet::Fill(depgraph_gen.PositionRange()) - depgraph_gen.Positions()); + if (direction & 1) { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto children = depgraph_gen.GetReducedChildren(i); diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index bec4d41d38519..44cd6590d381c 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -27,7 +27,7 @@ using TestBitSet = BitSet<32>; template bool IsAcyclic(const DepGraph& depgraph) noexcept { - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : depgraph.Positions()) { if ((depgraph.Ancestors(i) & depgraph.Descendants(i)) != SetType::Singleton(i)) { return false; } @@ -57,11 +57,14 @@ bool IsAcyclic(const DepGraph& depgraph) noexcept * by parent relations that were serialized before it). * - The various insertion positions in the cluster, from the very end of the cluster, to the * front. + * - The appending of 1, 2, 3, ... holes at the end of the cluster, followed by appending the new + * transaction. * - * Let's say you have a 7-transaction cluster, consisting of transactions F,A,C,B,G,E,D, but - * serialized in order A,B,C,D,E,F,G, because that happens to be a topological ordering. By the - * time G gets serialized, what has been serialized already represents the cluster F,A,C,B,E,D (in - * that order). G has B and E as direct parents, and E depends on C. + * Let's say you have a 7-transaction cluster, consisting of transactions F,A,C,B,_,G,E,_,D + * (where _ represent holes; unused positions within the DepGraph) but serialized in order + * A,B,C,D,E,F,G, because that happens to be a topological ordering. By the time G gets serialized, + * what has been serialized already represents the cluster F,A,C,B,_,E,_,D (in that order). G has B + * and E as direct parents, and E depends on C. * * In this case, the possibilities are, in order: * - [ ] the dependency G->F @@ -71,17 +74,23 @@ bool IsAcyclic(const DepGraph& depgraph) noexcept * - [ ] the dependency G->A * - [ ] put G at the end of the cluster * - [ ] put G before D + * - [ ] put G before the hole before D * - [X] put G before E + * - [ ] put G before the hole before E * - [ ] put G before B * - [ ] put G before C * - [ ] put G before A * - [ ] put G before F + * - [ ] add 1 hole at the end of the cluster, followed by G + * - [ ] add 2 holes at the end of the cluster, followed by G + * - [ ] add ... * - * The skip values in this case are 1 (G->F), 1 (G->D), 3 (G->A, G at end, G before D). No skip - * after 3 is needed (or permitted), because there can only be one position for G. Also note that - * G->C is not included in the list of possibilities, as it is implied by the included G->E and - * E->C that came before it. On deserialization, if the last skip value was 8 or larger (putting - * G before the beginning of the cluster), it is interpreted as wrapping around back to the end. + * The skip values in this case are 1 (G->F), 1 (G->D), 4 (G->A, G at end, G before D, G before + * hole). No skip after 4 is needed (or permitted), because there can only be one position for G. + * Also note that G->C is not included in the list of possibilities, as it is implied by the + * included G->E and E->C that came before it. On deserialization, if the last skip value was 8 or + * larger (putting G before the beginning of the cluster), it is interpreted as wrapping around + * back to the end. * * * Rationale: @@ -125,16 +134,17 @@ struct DepGraphFormatter static void Ser(Stream& s, const DepGraph& depgraph) { /** Construct a topological order to serialize the transactions in. */ - std::vector topo_order(depgraph.TxCount()); - std::iota(topo_order.begin(), topo_order.end(), ClusterIndex{0}); + std::vector topo_order; + topo_order.reserve(depgraph.TxCount()); + for (auto i : depgraph.Positions()) topo_order.push_back(i); std::sort(topo_order.begin(), topo_order.end(), [&](ClusterIndex a, ClusterIndex b) { auto anc_a = depgraph.Ancestors(a).Count(), anc_b = depgraph.Ancestors(b).Count(); if (anc_a != anc_b) return anc_a < anc_b; return a < b; }); - /** Which transactions the deserializer already knows when it has deserialized what has - * been serialized here so far. */ + /** Which positions (incl. holes) the deserializer already knows when it has deserialized + * what has been serialized here so far. */ SetType done; // Loop over the transactions in topological order. @@ -165,10 +175,19 @@ struct DepGraphFormatter } } // Write position information. - // The new transaction is to be inserted N positions back from the end of the cluster. - // Emit N to indicate that that many insertion choices are skipped. - auto skips = (done - SetType::Fill(idx)).Count(); - s << VARINT(diff + skips); + auto add_holes = SetType::Fill(idx) - done - depgraph.Positions(); + if (add_holes.None()) { + // The new transaction is to be inserted N positions back from the end of the + // cluster. Emit N to indicate that that many insertion choices are skipped. + auto skips = (done - SetType::Fill(idx)).Count(); + s << VARINT(diff + skips); + } else { + // The new transaction is to be appended at the end of the cluster, after N holes. + // Emit current_cluster_size + N, to indicate all insertion choices are skipped, + // plus N possibilities for the number of holes. + s << VARINT(diff + done.Count() + add_holes.Count()); + done |= add_holes; + } done.Set(idx); } @@ -185,8 +204,7 @@ struct DepGraphFormatter /** Mapping from serialization order to cluster order, used later to reconstruct the * cluster order. */ std::vector reordering; - /** How big the entries vector in the reconstructed depgraph will be (before the - * introduction of holes in a further commit, this always equals reordering.size()). */ + /** How big the entries vector in the reconstructed depgraph will be (including holes). */ ClusterIndex total_size{0}; // Read transactions in topological order. @@ -235,18 +253,43 @@ struct DepGraphFormatter assert(reordering.size() < SetType::Size()); auto topo_idx = topo_depgraph.AddTransaction(new_feerate); topo_depgraph.AddDependencies(new_ancestors, topo_idx); - diff %= total_size + 1; - // Insert the new transaction at distance diff back from the end. - for (auto& pos : reordering) { - pos += (pos >= total_size - diff); + if (total_size < SetType::Size()) { + // Normal case. + diff %= SetType::Size(); + if (diff <= total_size) { + // Insert the new transaction at distance diff back from the end. + for (auto& pos : reordering) { + pos += (pos >= total_size - diff); + } + reordering.push_back(total_size++ - diff); + } else { + // Append diff - total_size holes at the end, plus the new transaction. + total_size = diff; + reordering.push_back(total_size++); + } + } else { + // In case total_size == SetType::Size, it is not possible to insert the new + // transaction without exceeding SetType's size. Instead, interpret diff as an + // index into the holes, and overwrite a position there. This branch is never used + // when deserializing the output of the serializer, but gives meaning to otherwise + // invalid input. + diff %= (SetType::Size() - reordering.size()); + SetType holes = SetType::Fill(SetType::Size()); + for (auto pos : reordering) holes.Reset(pos); + for (auto pos : holes) { + if (diff == 0) { + reordering.push_back(pos); + break; + } + --diff; + } } - reordering.push_back(total_size++ - diff); // Stop if a read error was encountered during deserialization. if (read_error) break; } // Construct the original cluster order depgraph. - depgraph = DepGraph(topo_depgraph, reordering); + depgraph = DepGraph(topo_depgraph, reordering, total_size); } }; @@ -254,8 +297,19 @@ struct DepGraphFormatter template void SanityCheck(const DepGraph& depgraph) { + // Verify Positions and PositionRange consistency. + ClusterIndex num_positions{0}; + ClusterIndex position_range{0}; + for (ClusterIndex i : depgraph.Positions()) { + ++num_positions; + position_range = i + 1; + } + assert(num_positions == depgraph.TxCount()); + assert(position_range == depgraph.PositionRange()); + assert(position_range >= num_positions); + assert(position_range <= SetType::Size()); // Consistency check between ancestors internally. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : depgraph.Positions()) { // Transactions include themselves as ancestors. assert(depgraph.Ancestors(i)[i]); // If a is an ancestor of b, then b's ancestors must include all of a's ancestors. @@ -264,8 +318,8 @@ void SanityCheck(const DepGraph& depgraph) } } // Consistency check between ancestors and descendants. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - for (ClusterIndex j = 0; j < depgraph.TxCount(); ++j) { + for (ClusterIndex i : depgraph.Positions()) { + for (ClusterIndex j : depgraph.Positions()) { assert(depgraph.Ancestors(i)[j] == depgraph.Descendants(j)[i]); } // No transaction is a parent or child of itself. @@ -305,12 +359,12 @@ void SanityCheck(const DepGraph& depgraph) // In acyclic graphs, the union of parents with parents of parents etc. yields the // full ancestor set (and similar for children and descendants). - std::vector parents, children; - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - parents.push_back(depgraph.GetReducedParents(i)); - children.push_back(depgraph.GetReducedChildren(i)); + std::vector parents(depgraph.PositionRange()), children(depgraph.PositionRange()); + for (ClusterIndex i : depgraph.Positions()) { + parents[i] = depgraph.GetReducedParents(i); + children[i] = depgraph.GetReducedChildren(i); } - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (auto i : depgraph.Positions()) { // Initialize the set of ancestors with just the current transaction itself. SetType ancestors = SetType::Singleton(i); // Iteratively add parents of all transactions in the ancestor set to itself. @@ -382,7 +436,7 @@ void SanityCheck(const DepGraph& depgraph, Span lin TestBitSet done; for (auto i : linearization) { // Check transaction position is in range. - assert(i < depgraph.TxCount()); + assert(depgraph.Positions()[i]); // Check topology and lack of duplicates. assert((depgraph.Ancestors(i) - done) == TestBitSet::Singleton(i)); done.Set(i); diff --git a/src/util/check.h b/src/util/check.h index a02a1de8dcb43..8f28f5dc94127 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -40,7 +40,7 @@ void assertion_fail(std::string_view file, int line, std::string_view func, std: /** Helper for Assert()/Assume() */ template -T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) +constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) { if constexpr (IS_ASSERT #ifdef ABORT_ON_FAILED_ASSUME diff --git a/src/util/feefrac.h b/src/util/feefrac.h index 9772162010a6c..161322b50a48e 100644 --- a/src/util/feefrac.h +++ b/src/util/feefrac.h @@ -64,13 +64,13 @@ struct FeeFrac int32_t size; /** Construct an IsEmpty() FeeFrac. */ - inline FeeFrac() noexcept : fee{0}, size{0} {} + constexpr inline FeeFrac() noexcept : fee{0}, size{0} {} /** Construct a FeeFrac with specified fee and size. */ - inline FeeFrac(int64_t f, int32_t s) noexcept : fee{f}, size{s} {} + constexpr inline FeeFrac(int64_t f, int32_t s) noexcept : fee{f}, size{s} {} - inline FeeFrac(const FeeFrac&) noexcept = default; - inline FeeFrac& operator=(const FeeFrac&) noexcept = default; + constexpr inline FeeFrac(const FeeFrac&) noexcept = default; + constexpr inline FeeFrac& operator=(const FeeFrac&) noexcept = default; /** Check if this is empty (size and fee are 0). */ bool inline IsEmpty() const noexcept { From 1c24c625105cd62518d2eee7e95c3b1c74ea9923 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 23 Sep 2024 10:02:59 -0400 Subject: [PATCH 6/7] clusterlin: merge two DepGraph fuzz tests into simulation test This combines the clusterlin_add_dependency and clusterlin_cluster_serialization fuzz tests into a single clusterlin_depgraph_sim fuzz test. This tests starts from an empty DepGraph and performs a arbitrary number of AddTransaction, AddDependencies, and RemoveTransactions operations on it, and compares the resulting state with a naive reimplementation. --- src/test/fuzz/cluster_linearize.cpp | 224 +++++++++++++++++----------- 1 file changed, 139 insertions(+), 85 deletions(-) diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index e75883878e4c3..5b3770636ab07 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -241,103 +241,157 @@ std::vector ReadLinearization(const DepGraph& depgraph, SpanRe } // namespace -FUZZ_TARGET(clusterlin_add_dependencies) +FUZZ_TARGET(clusterlin_depgraph_sim) { - // Verify that computing a DepGraph from a cluster, or building it step by step using - // AddDependencies has the same effect. + // Simulation test to verify the full behavior of DepGraph. FuzzedDataProvider provider(buffer.data(), buffer.size()); - auto rng_seed = provider.ConsumeIntegral(); - InsecureRandomContext rng(rng_seed); - - // Construct a cluster of a certain length, with no dependencies. - auto num_tx = provider.ConsumeIntegralInRange(2, TestBitSet::Size()); - Cluster cluster(num_tx, std::pair{FeeFrac{0, 1}, TestBitSet{}}); - // Construct the corresponding DepGraph object (also no dependencies). - DepGraph depgraph_batch(cluster); - SanityCheck(depgraph_batch); - // Read (parents, child) pairs, and add the dependencies to the cluster and depgraph. - std::vector> deps_list; - LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size()) { - const auto parents_mask = provider.ConsumeIntegralInRange(0, (uint64_t{1} << num_tx) - 1); - auto child = provider.ConsumeIntegralInRange(0, num_tx - 1); - - auto parents_mask_shifted = parents_mask; - TestBitSet deps; - for (ClusterIndex i = 0; i < num_tx; ++i) { - if (parents_mask_shifted & 1) { - deps.Set(i); - cluster[child].second.Set(i); + + /** Real DepGraph being tested. */ + DepGraph real; + /** Simulated DepGraph (sim[i] is std::nullopt if position i does not exist; otherwise, + * sim[i]->first is its individual feerate, and sim[i]->second is its set of ancestors. */ + std::array>, TestBitSet::Size()> sim; + /** The number of non-nullopt position in sim. */ + ClusterIndex num_tx_sim{0}; + + /** Read a valid index of a transaction from the provider. */ + auto idx_fn = [&]() { + auto offset = provider.ConsumeIntegralInRange(0, num_tx_sim - 1); + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if (!sim[i].has_value()) continue; + if (offset == 0) return i; + --offset; + } + assert(false); + return ClusterIndex(-1); + }; + + /** Read a valid subset of the transactions from the provider. */ + auto subset_fn = [&]() { + auto range = (uint64_t{1} << num_tx_sim) - 1; + const auto mask = provider.ConsumeIntegralInRange(0, range); + auto mask_shifted = mask; + TestBitSet subset; + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if (!sim[i].has_value()) continue; + if (mask_shifted & 1) { + subset.Set(i); } - parents_mask_shifted >>= 1; + mask_shifted >>= 1; } - assert(parents_mask_shifted == 0); - depgraph_batch.AddDependencies(deps, child); - for (auto i : deps) { - deps_list.emplace_back(i, child); - assert(depgraph_batch.Ancestors(child)[i]); - assert(depgraph_batch.Descendants(i)[child]); + assert(mask_shifted == 0); + return subset; + }; + + /** Read any set of transactions from the provider (including unused positions). */ + auto set_fn = [&]() { + auto range = (uint64_t{1} << sim.size()) - 1; + const auto mask = provider.ConsumeIntegralInRange(0, range); + TestBitSet set; + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if ((mask >> i) & 1) { + set.Set(i); + } } - } - // Sanity check the result. - SanityCheck(depgraph_batch); - // Verify that the resulting DepGraph matches one recomputed from the cluster. - assert(DepGraph(cluster) == depgraph_batch); - - DepGraph depgraph_individual; - // Add all transactions to depgraph_individual. - for (const auto& [feerate, parents] : cluster) { - depgraph_individual.AddTransaction(feerate); - } - SanityCheck(depgraph_individual); - // Add all individual dependencies to depgraph_individual in randomized order. - std::shuffle(deps_list.begin(), deps_list.end(), rng); - for (auto [parent, child] : deps_list) { - depgraph_individual.AddDependencies(TestBitSet::Singleton(parent), child); - assert(depgraph_individual.Ancestors(child)[parent]); - assert(depgraph_individual.Descendants(parent)[child]); - } - // Sanity check and compare again the batched version. - SanityCheck(depgraph_individual); - assert(depgraph_individual == depgraph_batch); -} + return set; + }; -FUZZ_TARGET(clusterlin_cluster_serialization) -{ - // Verify that any graph of transactions has its ancestry correctly computed by DepGraph, and - // if it is a DAG, that it can be serialized as a DepGraph in a way that roundtrips. This - // guarantees that any acyclic cluster has a corresponding DepGraph serialization. + /** Propagate ancestor information in sim. */ + auto anc_update_fn = [&]() { + while (true) { + bool updates{false}; + for (ClusterIndex chl = 0; chl < sim.size(); ++chl) { + if (!sim[chl].has_value()) continue; + for (auto par : sim[chl]->second) { + if (!sim[chl]->second.IsSupersetOf(sim[par]->second)) { + sim[chl]->second |= sim[par]->second; + updates = true; + } + } + } + if (!updates) break; + } + }; - FuzzedDataProvider provider(buffer.data(), buffer.size()); + /** Compare the state of transaction i in the simulation with the real one. */ + auto check_fn = [&](ClusterIndex i) { + // Compare used positions. + assert(real.Positions()[i] == sim[i].has_value()); + if (sim[i].has_value()) { + // Compare feerate. + assert(real.FeeRate(i) == sim[i]->first); + // Compare ancestors (note that SanityCheck verifies correspondence between ancestors + // and descendants, so we can restrict ourselves to ancestors here). + assert(real.Ancestors(i) == sim[i]->second); + } + }; - // Construct a cluster in a naive way (using a FuzzedDataProvider-based serialization). - Cluster cluster; - auto num_tx = provider.ConsumeIntegralInRange(1, 32); - cluster.resize(num_tx); - for (ClusterIndex i = 0; i < num_tx; ++i) { - cluster[i].first.size = provider.ConsumeIntegralInRange(1, 0x3fffff); - cluster[i].first.fee = provider.ConsumeIntegralInRange(-0x8000000000000, 0x7ffffffffffff); - for (ClusterIndex j = 0; j < num_tx; ++j) { - if (i == j) continue; - if (provider.ConsumeBool()) cluster[i].second.Set(j); + LIMITED_WHILE(provider.remaining_bytes() > 0, 1000) { + uint8_t command = provider.ConsumeIntegral(); + if (num_tx_sim == 0 || ((command % 3) <= 0 && num_tx_sim < TestBitSet::Size())) { + // AddTransaction. + auto fee = provider.ConsumeIntegralInRange(-0x8000000000000, 0x7ffffffffffff); + auto size = provider.ConsumeIntegralInRange(1, 0x3fffff); + FeeFrac feerate{fee, size}; + // Apply to DepGraph. + auto idx = real.AddTransaction(feerate); + // Verify that the returned index is correct. + assert(!sim[idx].has_value()); + for (ClusterIndex i = 0; i < TestBitSet::Size(); ++i) { + if (!sim[i].has_value()) { + assert(idx == i); + break; + } + } + // Update sim. + sim[idx] = {feerate, TestBitSet::Singleton(idx)}; + ++num_tx_sim; + continue; + } + if ((command % 3) <= 1 && num_tx_sim > 0) { + // AddDependencies. + ClusterIndex child = idx_fn(); + auto parents = subset_fn(); + // Apply to DepGraph. + real.AddDependencies(parents, child); + // Apply to sim. + sim[child]->second |= parents; + continue; + } + if (num_tx_sim > 0) { + // Remove transactions. + auto del = set_fn(); + // Propagate all ancestry information before deleting anything in the simulation (as + // intermediary transactions may be deleted which impact connectivity). + anc_update_fn(); + // Compare the state of the transactions being deleted. + for (auto i : del) check_fn(i); + // Apply to DepGraph. + real.RemoveTransactions(del); + // Apply to sim. + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if (sim[i].has_value()) { + if (del[i]) { + --num_tx_sim; + sim[i] = std::nullopt; + } else { + sim[i]->second -= del; + } + } + } + continue; } + // This should be unreachable (one of the 3 above actions should always be possible). + assert(false); } - // Construct dependency graph, and verify it matches the cluster (which includes a round-trip - // check for the serialization). - DepGraph depgraph(cluster); - VerifyDepGraphFromCluster(cluster, depgraph); - - // Remove an arbitrary subset (in order to construct a graph with holes) and verify that it - // still sanity checks (incl. round-tripping serialization). - uint64_t del = provider.ConsumeIntegralInRange(1, (uint64_t{1} << TestBitSet::Size()) - 1); - TestBitSet setdel; - for (ClusterIndex i = 0; i < TestBitSet::Size(); ++i) { - if (del & 1) setdel.Set(i); - del >>= 1; - } - depgraph.RemoveTransactions(setdel); - SanityCheck(depgraph); + // Compare the real obtained depgraph against the simulation. + anc_update_fn(); + for (ClusterIndex i = 0; i < sim.size(); ++i) check_fn(i); + assert(real.TxCount() == num_tx_sim); + // Sanity check the result (which includes round-tripping serialization, if applicable). + SanityCheck(real); } FUZZ_TARGET(clusterlin_depgraph_serialization) From 0b3ec8c59b2b6d598531cd4e688eb276e597c825 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 23 Sep 2024 11:32:58 -0400 Subject: [PATCH 7/7] clusterlin: remove Cluster type --- src/cluster_linearize.h | 39 +--------------------------- src/test/cluster_linearize_tests.cpp | 14 +++++----- src/test/util/cluster_linearize.h | 37 -------------------------- 3 files changed, 8 insertions(+), 82 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index a2450f549084b..757c81f108389 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -19,14 +19,6 @@ namespace cluster_linearize { -/** Data type to represent cluster input. - * - * cluster[i].first is tx_i's fee and size. - * cluster[i].second[j] is true iff tx_i spends one or more of tx_j's outputs. - */ -template -using Cluster = std::vector>; - /** Data type to represent transaction indices in clusters. */ using ClusterIndex = uint32_t; @@ -54,7 +46,7 @@ class DepGraph Entry(const FeeFrac& f, const SetType& a, const SetType& d) noexcept : feerate(f), ancestors(a), descendants(d) {} }; - /** Data for each transaction, in the same order as the Cluster it was constructed from. */ + /** Data for each transaction. */ std::vector entries; /** Which positions are used. */ @@ -79,35 +71,6 @@ class DepGraph DepGraph& operator=(const DepGraph&) noexcept = default; DepGraph& operator=(DepGraph&&) noexcept = default; - /** Construct a DepGraph object for ntx transactions, with no dependencies. - * - * Complexity: O(N) where N=ntx. - **/ - explicit DepGraph(ClusterIndex ntx) noexcept - { - Assume(ntx <= SetType::Size()); - entries.resize(ntx); - for (ClusterIndex i = 0; i < ntx; ++i) { - entries[i].ancestors = SetType::Singleton(i); - entries[i].descendants = SetType::Singleton(i); - } - m_used = SetType::Fill(ntx); - } - - /** Construct a DepGraph object given a cluster. - * - * Complexity: O(N^2) where N=cluster.size(). - */ - explicit DepGraph(const Cluster& cluster) noexcept : DepGraph(cluster.size()) - { - for (ClusterIndex i = 0; i < cluster.size(); ++i) { - // Fill in fee and size. - entries[i].feerate = cluster[i].first; - // Fill in dependencies. - AddDependencies(cluster[i].second, i); - } - } - /** Construct a DepGraph object given another DepGraph and a mapping from old to new. * * @param depgraph The original DepGraph that is being remapped. diff --git a/src/test/cluster_linearize_tests.cpp b/src/test/cluster_linearize_tests.cpp index cb87dcbffbe12..265ccdc805e4b 100644 --- a/src/test/cluster_linearize_tests.cpp +++ b/src/test/cluster_linearize_tests.cpp @@ -23,18 +23,18 @@ namespace { constexpr std::pair HOLE{FeeFrac{0, 0x3FFFFF}, {}}; template -void TestDepGraphSerialization(const Cluster& cluster, const std::string& hexenc) +void TestDepGraphSerialization(const std::vector>& cluster, const std::string& hexenc) { - DepGraph depgraph(cluster); - - // Run normal sanity and correspondence checks, which includes a round-trip test. - VerifyDepGraphFromCluster(cluster, depgraph); - - // Remove holes (which are expected to be present as HOLE entries in cluster). + // Construct DepGraph from cluster argument. + DepGraph depgraph; SetType holes; for (ClusterIndex i = 0; i < cluster.size(); ++i) { + depgraph.AddTransaction(cluster[i].first); if (cluster[i] == HOLE) holes.Set(i); } + for (ClusterIndex i = 0; i < cluster.size(); ++i) { + depgraph.AddDependencies(cluster[i].second, i); + } depgraph.RemoveTransactions(holes); // There may be multiple serializations of the same graph, but DepGraphFormatter's serializer diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index 44cd6590d381c..871aa9d74ede3 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -390,43 +390,6 @@ void SanityCheck(const DepGraph& depgraph) } } -/** Verify that a DepGraph corresponds to the information in a cluster. */ -template -void VerifyDepGraphFromCluster(const Cluster& cluster, const DepGraph& depgraph) -{ - // Sanity check the depgraph, which includes a check for correspondence between ancestors and - // descendants, so it suffices to check just ancestors below. - SanityCheck(depgraph); - // Verify transaction count. - assert(cluster.size() == depgraph.TxCount()); - // Verify feerates. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - assert(depgraph.FeeRate(i) == cluster[i].first); - } - // Verify ancestors. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - // Start with the transaction having itself as ancestor. - auto ancestors = SetType::Singleton(i); - // Add parents of ancestors to the set of ancestors until it stops changing. - while (true) { - const auto old_ancestors = ancestors; - for (auto ancestor : ancestors) { - ancestors |= cluster[ancestor].second; - } - if (old_ancestors == ancestors) break; - } - // Compare against depgraph. - assert(depgraph.Ancestors(i) == ancestors); - // Some additional sanity tests: - // - Every transaction has itself as ancestor. - assert(ancestors[i]); - // - Every transaction has its direct parents as ancestors. - for (auto parent : cluster[i].second) { - assert(ancestors[parent]); - } - } -} - /** Perform a sanity check on a linearization. */ template void SanityCheck(const DepGraph& depgraph, Span linearization)