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 e964849f22839..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,12 +46,23 @@ 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. */ + 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; @@ -68,80 +71,51 @@ class DepGraph DepGraph& operator=(const DepGraph&) noexcept = default; DepGraph& operator=(DepGraph&&) noexcept = default; - /** Construct a DepGraph object for ntx transactions, with no dependencies. + /** Construct a DepGraph object given another DepGraph and a mapping from old to new. * - * 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); - } - } - - /** Construct a DepGraph object given a cluster. + * @param depgraph The original DepGraph that is being remapped. * - * Complexity: O(N^2) where N=cluster.size(). - */ - explicit DepGraph(const Cluster& cluster) noexcept : entries(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); - } - } - } - - /** Construct a DepGraph object given another DepGraph and a mapping from old to new. + * @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 : entries(depgraph.TxCount()) + DepGraph(const DepGraph& depgraph, Span mapping, ClusterIndex pos_range) noexcept : entries(pos_range) { - 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]); + 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[new_idx].feerate = depgraph.entries[i].feerate; } - // Fill in descendant information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - for (auto j : entries[i].ancestors) { - entries[j].descendants.Set(i); - } + 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). */ @@ -151,39 +125,120 @@ 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; } - /** Modify this transaction graph, adding a dependency between a specified parent and child. + /** 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 AddDependency(ClusterIndex parent, ClusterIndex child) noexcept + */ + 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 { - // 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. + 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)) { + 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; } } + /** 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(). @@ -237,7 +292,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. * @@ -487,11 +542,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; @@ -614,22 +669,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. */ @@ -1141,7 +1200,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..265ccdc805e4b 100644 --- a/src/test/cluster_linearize_tests.cpp +++ b/src/test/cluster_linearize_tests.cpp @@ -18,13 +18,24 @@ 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) +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); + // 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 // only produces one of those. Verify that hexenc matches that canonical serialization. @@ -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 d91f85d867b14..5b3770636ab07 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 @@ -36,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; } @@ -106,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; } @@ -152,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); @@ -169,14 +170,14 @@ 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; 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; } @@ -205,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. @@ -240,59 +241,157 @@ std::vector ReadLinearization(const DepGraph& depgraph, SpanRe } // namespace -FUZZ_TARGET(clusterlin_add_dependency) +FUZZ_TARGET(clusterlin_depgraph_sim) { - // Verify that computing a DepGraph from a cluster, or building it step by step using AddDependency - // have the same effect. + // Simulation test to verify the full behavior of DepGraph. - // Construct a cluster of a certain length, with no dependencies. FuzzedDataProvider provider(buffer.data(), buffer.size()); - auto num_tx = provider.ConsumeIntegralInRange(2, 32); - 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]); - } - // Sanity check the result. - SanityCheck(depgraph); - // Verify that the resulting DepGraph matches one recomputed from the cluster. - assert(DepGraph(cluster) == depgraph); -} -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. + /** 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); + }; - FuzzedDataProvider provider(buffer.data(), buffer.size()); + /** 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); + } + mask_shifted >>= 1; + } + assert(mask_shifted == 0); + return subset; + }; - // 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); + /** 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); + } } + return set; + }; + + /** 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; + } + }; + + /** 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); + } + }; + + 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); + // 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) @@ -322,7 +421,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); @@ -333,7 +432,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()); } @@ -370,7 +469,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; @@ -423,7 +522,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; @@ -454,7 +553,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()); @@ -519,7 +618,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()); @@ -623,7 +722,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. @@ -806,8 +905,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. @@ -891,30 +990,30 @@ 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.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.AddDependencies(TestBitSet::Singleton(i), children.First()); } - 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.AddDependencies(TestBitSet::Singleton(parents.First()), 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..871aa9d74ede3 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,18 +134,18 @@ 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, and in what order. */ - std::vector rebuilt_order; - rebuilt_order.reserve(depgraph.TxCount()); + /** 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. for (ClusterIndex topo_idx = 0; topo_idx < topo_order.size(); ++topo_idx) { @@ -166,14 +175,20 @@ 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; + 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; } - rebuilt_order.insert(rebuilt_order.end() - insert_distance, idx); - s << VARINT(diff + insert_distance); + done.Set(idx); } // Output a final 0 to denote the end of the graph. @@ -189,10 +204,16 @@ 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 (including holes). */ + 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 +225,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,23 +244,52 @@ 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); + } catch (const std::ios_base::failure&) { + // Continue even if a read error was encountered. + read_error = true; + } + // 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); + topo_depgraph.AddDependencies(new_ancestors, topo_idx); + 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(reordering.size() - insert_distance); } - } catch (const std::ios_base::failure&) {} + // 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); } }; @@ -250,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. @@ -260,13 +318,27 @@ 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. + 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,42 +356,36 @@ void SanityCheck(const DepGraph& depgraph) reader >> Using(decoded_depgraph); assert(depgraph == decoded_depgraph); assert(reader.empty()); - } -} -/** 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; + // 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(depgraph.PositionRange()), children(depgraph.PositionRange()); + for (ClusterIndex i : depgraph.Positions()) { + parents[i] = depgraph.GetReducedParents(i); + children[i] = depgraph.GetReducedChildren(i); } - // 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]); + 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. + 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)); } } } @@ -333,7 +399,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 {