Merge bitcoin/bitcoin#30857: cluster mempool: extend DepGraph functionality

0b3ec8c59b clusterlin: remove Cluster type (Pieter Wuille)
1c24c62510 clusterlin: merge two DepGraph fuzz tests into simulation test (Pieter Wuille)
0606e66fdb clusterlin: add DepGraph::RemoveTransactions and support for holes in DepGraph (Pieter Wuille)
75b5d42419 clusterlin: make DepGraph::AddDependency support multiple dependencies at once (Pieter Wuille)
abf50649d1 clusterlin: simplify DepGraphFormatter::Ser (Pieter Wuille)
eaab55ffc8 clusterlin: rework DepGraphFormatter::Unser (Pieter Wuille)
5901cf7100 clusterlin: abstract out DepGraph::GetReduced{Parents,Children} (Pieter Wuille)

Pull request description:

  Part of cluster mempool: #30289

  This adds:
  * `DepGraph::AddDependencies` to add 0 or more dependencies to a single transaction at once (identical to calling `DepGraph::AddDependency` once for each, but more efficient).
  * `DepGraph::RemoveTransactions` to remove 0 or more transactions from a depgraph.
  * `DepGraph::GetReducedParents` (and `DepGraph::GetReducedChildren`) to get the (reduced) direct parents and children of a transaction in a depgraph.

  After which, the `Cluster` type is removed.

  This is the result of fleshing out the design for the "intermediate layer" ("TxGraph", no PR yet) between the cluster linearization layer and the mempool layer. My earlier thinking was that TxGraph would store `Cluster` objects (vectors of pairs of `FeeFrac`s and sets of parents), and convert them to `DepGraph` on the fly whenever needed. However, after more consideration, it seems better to have TxGraph store `DepGraph` objects, and manipulate them directly without constantly re-creating them. This requires `DepGraph` to have some additional functionality.

  The bulk of the complexity here is the addition of `DepGraph::RemoveTransactions`, which leaves the remaining transactions' positions within the `DepGraph` untouched (we want existing identifiers to remain valid), so this implies that graphs can now have "holes" (positions that are unused, but followed by positions that are used). To enable that, an extension of the fuzz/test serialization format `DepGraphFormatter` is included to deal with such holes.

ACKs for top commit:
  sdaftuar:
    reACK 0b3ec8c59b
  instagibbs:
    reACK 0b3ec8c59b
  ismaelsadeeq:
    reACK 0b3ec8c59b
  glozow:
    ACK 0b3ec8c59b, reviewed range-diff from  aab53ddcd8fcbc3c0be0da9383f8e06abe5badda and `clusterlin_depgraph_sim`

Tree-SHA512: a804b7f26d544c5cb0847322e235c810525cb0607737be6116c3156d582da3ba3352af8ea48e74eed5268f9c3eca63b30181d01b23a6dd0be1b99191f81cceb0

src/bench/cluster_linearize.cpp

@@ -28,7 +28,7 @@ DepGraph<SetType> MakeLinearGraph(ClusterIndex ntx)
     DepGraph<SetType> 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<SetType> MakeWideGraph(ClusterIndex ntx)
     DepGraph<SetType> 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<SetType> 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<SetType> 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<SetType> 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<ClusterIndex> lin1;
     std::vector<ClusterIndex> lin2;

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<typename SetType>
-using Cluster = std::vector<std::pair<FeeFrac, SetType>>;
-
 /** 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<Entry> 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 @@ public:
     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);
-        }
-    }
-
-    /** Construct a DepGraph object given a cluster.
-     *
-     * Complexity: O(N^2) where N=cluster.size().
-     */
-    explicit DepGraph(const Cluster<SetType>& 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 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<SetType>& depgraph, Span<const ClusterIndex> mapping) noexcept : entries(depgraph.TxCount())
+    DepGraph(const DepGraph<SetType>& depgraph, Span<const ClusterIndex> mapping, ClusterIndex pos_range) noexcept : entries(pos_range)
     {
-        Assert(mapping.size() == depgraph.TxCount());
+        Assume(mapping.size() == depgraph.PositionRange());
-        // Fill in fee, size, ancestors.
+        Assume((pos_range == 0) == (depgraph.TxCount() == 0));
-        for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) {
+        for (ClusterIndex i : depgraph.Positions()) {
-            const auto& input = depgraph.entries[i];
+            auto new_idx = mapping[i];
-            auto& output = entries[mapping[i]];
+            Assume(new_idx < pos_range);
-            output.feerate = input.feerate;
+            // Add transaction.
-            for (auto j : input.ancestors) output.ancestors.Set(mapping[j]);
+            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 : depgraph.Positions()) {
-        for (ClusterIndex i = 0; i < entries.size(); ++i) {
+            // Fill in dependencies by mapping direct parents.
-            for (auto j : entries[i].ancestors) {
+            SetType parents;
-                entries[j].descendants.Set(i);
+            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 @@ public:
     /** 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
+    /** Add a new unconnected transaction to this transaction graph (in the first available
-     *  ClusterIndex.
+     *  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());
+        static constexpr auto ALL_POSITIONS = SetType::Fill(SetType::Size());
-        ClusterIndex new_idx = TxCount();
+        auto available = ALL_POSITIONS - m_used;
-        entries.emplace_back(feefrac, SetType::Singleton(new_idx), SetType::Singleton(new_idx));
+        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
     {
-        // Bail out if dependency is already implied.
+        m_used -= del;
-        if (entries[child].ancestors[parent]) return;
+        // Remove now-unused trailing entries.
-        // To each ancestor of the parent, add as descendants the descendants of the child.
+        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)) {
+            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.
+        // To each descendant of the child, add those ancestors.
-        const auto& par_anc = entries[parent].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 @@ public:
      *
      * 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 @@ public:
      */
     AncestorCandidateFinder(const DepGraph<SetType>& depgraph LIFETIMEBOUND) noexcept :
         m_depgraph(depgraph),
-        m_todo{SetType::Fill(depgraph.TxCount())},
+        m_todo{depgraph.Positions()},
-        m_ancestor_set_feerates(depgraph.TxCount())
+        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 @@ public:
     SearchCandidateFinder(const DepGraph<SetType>& depgraph, uint64_t rng_seed) noexcept :
         m_rng(rng_seed),
         m_sorted_to_original(depgraph.TxCount()),
-        m_original_to_sorted(depgraph.TxCount()),
+        m_original_to_sorted(depgraph.PositionRange())
-        m_todo(SetType::Fill(depgraph.TxCount()))
     {
-        // Determine reordering mapping, by sorting by decreasing feerate.
+        // Determine reordering mapping, by sorting by decreasing feerate. Unusued positions are
-        std::iota(m_sorted_to_original.begin(), m_sorted_to_original.end(), ClusterIndex{0});
+        // 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<SetType>& depgraph, Span<ClusterIndex> 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<TxEntry> entries(linearization.size() + 1);
+    std::vector<TxEntry> entries(depgraph.PositionRange() + 1);
 
     // Perform two passes over the linearization.
     for (int pass = 0; pass < 2; ++pass) {

src/test/cluster_linearize_tests.cpp

@@ -18,13 +18,24 @@ using namespace cluster_linearize;
 
 namespace {
 
-template<typename SetType>
+/** Special magic value that indicates to TestDepGraphSerialization that a cluster entry represents
-void TestDepGraphSerialization(const Cluster<SetType>& cluster, const std::string& hexenc)
+ *  a hole. */
-{
+constexpr std::pair<FeeFrac, TestBitSet> HOLE{FeeFrac{0, 0x3FFFFF}, {}};
-    DepGraph depgraph(cluster);
 
-    // Run normal sanity and correspondence checks, which includes a round-trip test.
+template<typename SetType>
-    VerifyDepGraphFromCluster(cluster, depgraph);
+void TestDepGraphSerialization(const std::vector<std::pair<FeeFrac, SetType>>& cluster, const std::string& hexenc)
+{
+    // Construct DepGraph from cluster argument.
+    DepGraph<SetType> 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<TestBitSet>(
+        {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()

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
 

src/test/fuzz/cluster_linearize.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <cluster_linearize.h>
+#include <random.h>
 #include <serialize.h>
 #include <streams.h>
 #include <test/fuzz/fuzz.h>
@@ -36,7 +37,7 @@ class SimpleCandidateFinder
 public:
     /** Construct an SimpleCandidateFinder for a given graph. */
     SimpleCandidateFinder(const DepGraph<SetType>& 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<SetType>& 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<std::vector<ClusterIndex>, bool> SimpleLinearize(const DepGraph<SetTyp
 {
     std::vector<ClusterIndex> 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<std::vector<ClusterIndex>, bool> SimpleLinearize(const DepGraph<SetTyp
 template<typename BS>
 void MakeConnected(DepGraph<BS>& 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<typename BS>
 std::vector<ClusterIndex> ReadLinearization(const DepGraph<BS>& depgraph, SpanReader& reader)
 {
     std::vector<ClusterIndex> 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<ClusterIndex> ReadLinearization(const DepGraph<BS>& 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
+    // Simulation test to verify the full behavior of DepGraph.
-    // have the same effect.
-
-    // Construct a cluster of a certain length, with no dependencies.
-    FuzzedDataProvider provider(buffer.data(), buffer.size());
-    auto num_tx = provider.ConsumeIntegralInRange<ClusterIndex>(2, 32);
-    Cluster<TestBitSet> 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<ClusterIndex>(0, num_tx - 1);
-        auto child = provider.ConsumeIntegralInRange<ClusterIndex>(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.
 
     FuzzedDataProvider provider(buffer.data(), buffer.size());
 
-    // Construct a cluster in a naive way (using a FuzzedDataProvider-based serialization).
+    /** Real DepGraph being tested. */
-    Cluster<TestBitSet> cluster;
+    DepGraph<TestBitSet> real;
-    auto num_tx = provider.ConsumeIntegralInRange<ClusterIndex>(1, 32);
+    /** Simulated DepGraph (sim[i] is std::nullopt if position i does not exist; otherwise,
-    cluster.resize(num_tx);
+     *  sim[i]->first is its individual feerate, and sim[i]->second is its set of ancestors. */
-    for (ClusterIndex i = 0; i < num_tx; ++i) {
+    std::array<std::optional<std::pair<FeeFrac, TestBitSet>>, TestBitSet::Size()> sim;
-        cluster[i].first.size = provider.ConsumeIntegralInRange<int32_t>(1, 0x3fffff);
+    /** The number of non-nullopt position in sim. */
-        cluster[i].first.fee = provider.ConsumeIntegralInRange<int64_t>(-0x8000000000000, 0x7ffffffffffff);
+    ClusterIndex num_tx_sim{0};
-        for (ClusterIndex j = 0; j < num_tx; ++j) {
+
-            if (i == j) continue;
+    /** Read a valid index of a transaction from the provider. */
-            if (provider.ConsumeBool()) cluster[i].second.Set(j);
+    auto idx_fn = [&]() {
+        auto offset = provider.ConsumeIntegralInRange<ClusterIndex>(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<uint64_t>(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;
+    };
+
+    /** 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<uint64_t>(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<uint8_t>();
+        if (num_tx_sim == 0 || ((command % 3) <= 0 && num_tx_sim < TestBitSet::Size())) {
+            // AddTransaction.
+            auto fee = provider.ConsumeIntegralInRange<int64_t>(-0x8000000000000, 0x7ffffffffffff);
+            auto size = provider.ConsumeIntegralInRange<int32_t>(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
+    // Compare the real obtained depgraph against the simulation.
-    // check for the serialization).
+    anc_update_fn();
-    DepGraph depgraph(cluster);
+    for (ClusterIndex i = 0; i < sim.size(); ++i) check_fn(i);
-    VerifyDepGraphFromCluster(cluster, depgraph);
+    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<DepGraphFormatter>(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<TestBitSet> 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<ClusterIndex> perm_linearization(depgraph.TxCount());
+            std::vector<ClusterIndex> perm_linearization;
-            for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) perm_linearization[i] = i;
+            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<TestBitSet> depgraph_tree;
-    for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) {
+    for (ClusterIndex i = 0; i < depgraph_gen.PositionRange(); ++i) {
-        depgraph_tree.AddTransaction(depgraph_gen.FeeRate(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);
+            auto children = depgraph_gen.GetReducedChildren(i);
-            // Remove descendants that are children of other descendants.
+            if (children.Any()) {
-            for (auto j : children) {
+                depgraph_tree.AddDependencies(TestBitSet::Singleton(i), children.First());
-                if (!children[j]) continue;
-                children -= depgraph_gen.Descendants(j);
-                children.Set(j);
             }
-            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);
+            auto parents = depgraph_gen.GetReducedParents(i);
-            // Remove ancestors that are parents of other ancestors.
+            if (parents.Any()) {
-            for (auto j : parents) {
+                depgraph_tree.AddDependencies(TestBitSet::Singleton(parents.First()), i);
-                if (!parents[j]) continue;
-                parents -= depgraph_gen.Ancestors(j);
-                parents.Set(j);
             }
-            if (parents.Any()) depgraph_tree.AddDependency(parents.First(), i);
         }
     }
 

src/test/util/cluster_linearize.h

@@ -27,7 +27,7 @@ using TestBitSet = BitSet<32>;
 template<typename SetType>
 bool IsAcyclic(const DepGraph<SetType>& 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<SetType>& 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
+ * Let's say you have a 7-transaction cluster, consisting of transactions F,A,C,B,_,G,E,_,D
- * serialized in order A,B,C,D,E,F,G, because that happens to be a topological ordering. By the
+ * (where _ represent holes; unused positions within the DepGraph) but serialized in order
- * time G gets serialized, what has been serialized already represents the cluster F,A,C,B,E,D (in
+ * A,B,C,D,E,F,G, because that happens to be a topological ordering. By the time G gets serialized,
- * that order). G has B and E as direct parents, and E depends on C.
+ * 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<SetType>& 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
+ * The skip values in this case are 1 (G->F), 1 (G->D), 4 (G->A, G at end, G before D, G before
- * after 3 is needed (or permitted), because there can only be one position for G. Also note that
+ * hole). No skip after 4 is needed (or permitted), because there can only be one position for G.
- * G->C is not included in the list of possibilities, as it is implied by the included G->E and
+ * Also note that G->C is not included in the list of possibilities, as it is implied by the
- * E->C that came before it. On deserialization, if the last skip value was 8 or larger (putting
+ * included G->E and E->C that came before it. On deserialization, if the last skip value was 8 or
- * G before the beginning of the cluster), it is interpreted as wrapping around back to the end.
+ * 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<SetType>& depgraph)
     {
         /** Construct a topological order to serialize the transactions in. */
-        std::vector<ClusterIndex> topo_order(depgraph.TxCount());
+        std::vector<ClusterIndex> topo_order;
-        std::iota(topo_order.begin(), topo_order.end(), ClusterIndex{0});
+        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
+        /** Which positions (incl. holes) the deserializer already knows when it has deserialized
-         *  been serialized here so far, and in what order. */
+         *  what has been serialized here so far. */
-        std::vector<ClusterIndex> rebuilt_order;
+        SetType done;
-        rebuilt_order.reserve(depgraph.TxCount());
 
         // 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;
+            auto add_holes = SetType::Fill(idx) - done - depgraph.Positions();
-            while (insert_distance < rebuilt_order.size()) {
+            if (add_holes.None()) {
-                // Loop to find how far from the end in rebuilt_order to insert.
+                // The new transaction is to be inserted N positions back from the end of the
-                if (idx > *(rebuilt_order.end() - 1 - insert_distance)) break;
+                // cluster. Emit N to indicate that that many insertion choices are skipped.
-                ++insert_distance;
+                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);
+            done.Set(idx);
-            s << VARINT(diff + insert_distance);
         }
 
         // 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<ClusterIndex> 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);
+                new_feerate = {UnsignedToSigned(coded_fee), size};
-                // Extend topo_depgraph with the new transaction (preliminarily at the end).
-                auto topo_idx = topo_depgraph.AddTransaction({fee, size});
-                reordering.push_back(reordering.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
+            } catch (const std::ios_base::failure&) {
-                // from the end of reordering the new transaction should be placed (wrapping
+                // Continue even if a read error was encountered.
-                // around), so remove the preliminary position it was put in above (which was to
+                read_error = true;
-                // 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&) {}
+            // 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;
+                }
+            }
+            // 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<typename SetType>
 void SanityCheck(const DepGraph<SetType>& 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<SetType>& depgraph)
         }
     }
     // Consistency check between ancestors and descendants.
-    for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) {
+    for (ClusterIndex i : depgraph.Positions()) {
-        for (ClusterIndex j = 0; j < depgraph.TxCount(); ++j) {
+        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<unsigned char> ser;
         VectorWriter writer(ser, 0);
         writer << Using<DepGraphFormatter>(depgraph);
@@ -284,42 +356,36 @@ void SanityCheck(const DepGraph<SetType>& depgraph)
         reader >> Using<DepGraphFormatter>(decoded_depgraph);
         assert(depgraph == decoded_depgraph);
         assert(reader.empty());
-    }
-}
 
-/** Verify that a DepGraph corresponds to the information in a cluster. */
+        // In acyclic graphs, the union of parents with parents of parents etc. yields the
-template<typename SetType>
+        // full ancestor set (and similar for children and descendants).
-void VerifyDepGraphFromCluster(const Cluster<SetType>& cluster, const DepGraph<SetType>& depgraph)
+        std::vector<SetType> parents(depgraph.PositionRange()), children(depgraph.PositionRange());
-{
+        for (ClusterIndex i : depgraph.Positions()) {
-    // Sanity check the depgraph, which includes a check for correspondence between ancestors and
+            parents[i] = depgraph.GetReducedParents(i);
-    // descendants, so it suffices to check just ancestors below.
+            children[i] = depgraph.GetReducedChildren(i);
-    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.
+        for (auto i : depgraph.Positions()) {
-        assert(depgraph.Ancestors(i) == ancestors);
+            // Initialize the set of ancestors with just the current transaction itself.
-        // Some additional sanity tests:
+            SetType ancestors = SetType::Singleton(i);
-        // - Every transaction has itself as ancestor.
+            // Iteratively add parents of all transactions in the ancestor set to itself.
-        assert(ancestors[i]);
+            while (true) {
-        // - Every transaction has its direct parents as ancestors.
+                const auto old_ancestors = ancestors;
-        for (auto parent : cluster[i].second) {
+                for (auto j : ancestors) ancestors |= parents[j];
-            assert(ancestors[parent]);
+                // 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<SetType>& depgraph, Span<const ClusterIndex> 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);

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 <bool IS_ASSERT, typename T>
-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

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;
+    constexpr inline FeeFrac(const FeeFrac&) noexcept = default;
-    inline FeeFrac& operator=(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 {