txgraph: Add ability to configure maximum cluster size/weight (feature)

This is integrated with the oversized property: the graph is oversized when
any connected component within it contains more than the cluster count limit
many transactions, or when their combined size/weight exceeds the cluster size
limit.

It becomes disallowed to call AddTransaction with a size larger than this limit,
though this limit will be lifted in the next commit.

In addition, SetTransactionFeeRate becomes SetTransactionFee, so that we do not
need to deal with the case that a call to this function might affect the
oversizedness.

Author: sipa

src/test/fuzz/txgraph.cpp

@@ -56,9 +56,12 @@ struct SimTxGraph
     /** Which transactions have been modified in the graph since creation, either directly or by
      *  being in a cluster which includes modifications. Only relevant for the staging graph. */
     SetType modified;
+    /** The configured maximum total size of transactions per cluster. */
+    uint64_t max_cluster_size;
 
-    /** Construct a new SimTxGraph with the specified maximum cluster count. */
-    explicit SimTxGraph(DepGraphIndex max_cluster) : max_cluster_count(max_cluster) {}
+    /** Construct a new SimTxGraph with the specified maximum cluster count and size. */
+    explicit SimTxGraph(DepGraphIndex cluster_count, uint64_t cluster_size) :
+        max_cluster_count(cluster_count), max_cluster_size(cluster_size) {}
 
     // Permit copying and moving.
     SimTxGraph(const SimTxGraph&) noexcept = default;
@@ -78,6 +81,9 @@ struct SimTxGraph
             while (todo.Any()) {
                 auto component = graph.FindConnectedComponent(todo);
                 if (component.Count() > max_cluster_count) oversized = true;
+                uint64_t component_size{0};
+                for (auto i : component) component_size += graph.FeeRate(i).size;
+                if (component_size > max_cluster_size) oversized = true;
                 todo -= component;
             }
         }
@@ -260,14 +266,21 @@ FUZZ_TARGET(txgraph)
     /** Variable used whenever an empty TxGraph::Ref is needed. */
     TxGraph::Ref empty_ref;
 
-    // Decide the maximum number of transactions per cluster we will use in this simulation.
-    auto max_count = provider.ConsumeIntegralInRange<DepGraphIndex>(1, MAX_CLUSTER_COUNT_LIMIT);
+    /** The maximum number of transactions per (non-oversized) cluster we will use in this
+     *  simulation. */
+    auto max_cluster_count = provider.ConsumeIntegralInRange<DepGraphIndex>(1, MAX_CLUSTER_COUNT_LIMIT);
+    /** The maximum total size of transactions in a cluster, which also makes it an upper bound
+     *  on the individual size of a transaction (but this restriction will be lifted in a future
+     *  commit). */
+    auto max_cluster_size = provider.ConsumeIntegralInRange<uint64_t>(1, 0x3fffff * MAX_CLUSTER_COUNT_LIMIT);
+    /** The maximum individual transaction size used in this test (not a TxGraph parameter). */
+    auto max_tx_size = std::min<uint64_t>(0x3fffff, max_cluster_size);
 
     // Construct a real graph, and a vector of simulated graphs (main, and possibly staging).
-    auto real = MakeTxGraph(max_count);
+    auto real = MakeTxGraph(max_cluster_count, max_cluster_size);
     std::vector<SimTxGraph> sims;
     sims.reserve(2);
-    sims.emplace_back(max_count);
+    sims.emplace_back(max_cluster_count, max_cluster_size);
 
     /** Struct encapsulating information about a BlockBuilder that's currently live. */
     struct BlockBuilderData
@@ -391,12 +404,12 @@ FUZZ_TARGET(txgraph)
                 if (alt) {
                     // If alt is true, pick fee and size from the entire range.
                     fee = provider.ConsumeIntegralInRange<int64_t>(-0x8000000000000, 0x7ffffffffffff);
-                    size = provider.ConsumeIntegralInRange<int32_t>(1, 0x3fffff);
+                    size = provider.ConsumeIntegralInRange<int32_t>(1, max_tx_size);
                 } else {
                     // Otherwise, use smaller range which consume fewer fuzz input bytes, as just
                     // these are likely sufficient to trigger all interesting code paths already.
                     fee = provider.ConsumeIntegral<uint8_t>();
-                    size = provider.ConsumeIntegral<uint8_t>() + 1;
+                    size = provider.ConsumeIntegralInRange<uint32_t>(1, std::min<uint32_t>(0xff, max_tx_size));
                 }
                 FeePerWeight feerate{fee, size};
                 // Create a real TxGraph::Ref.
@@ -534,7 +547,7 @@ FUZZ_TARGET(txgraph)
                 auto ref = pick_fn();
                 auto result = alt ? real->GetDescendants(*ref, use_main)
                                   : real->GetAncestors(*ref, use_main);
-                assert(result.size() <= max_count);
+                assert(result.size() <= max_cluster_count);
                 auto result_set = sel_sim.MakeSet(result);
                 assert(result.size() == result_set.Count());
                 auto expect_set = sel_sim.GetAncDesc(ref, alt);
@@ -567,16 +580,20 @@ FUZZ_TARGET(txgraph)
                 auto ref = pick_fn();
                 auto result = real->GetCluster(*ref, use_main);
                 // Check cluster count limit.
-                assert(result.size() <= max_count);
+                assert(result.size() <= max_cluster_count);
                 // Require the result to be topologically valid and not contain duplicates.
                 auto left = sel_sim.graph.Positions();
+                uint64_t total_size{0};
                 for (auto refptr : result) {
                     auto simpos = sel_sim.Find(refptr);
+                    total_size += sel_sim.graph.FeeRate(simpos).size;
                     assert(simpos != SimTxGraph::MISSING);
                     assert(left[simpos]);
                     left.Reset(simpos);
                     assert(!sel_sim.graph.Ancestors(simpos).Overlaps(left));
                 }
+                // Check cluster size limit.
+                assert(total_size <= max_cluster_size);
                 // Require the set to be connected.
                 auto result_set = sel_sim.MakeSet(result);
                 assert(sel_sim.graph.IsConnected(result_set));
@@ -941,28 +958,32 @@ FUZZ_TARGET(txgraph)
                     // Check its ancestors against simulation.
                     auto expect_anc = sim.graph.Ancestors(i);
                     auto anc = sim.MakeSet(real->GetAncestors(*sim.GetRef(i), main_only));
-                    assert(anc.Count() <= max_count);
+                    assert(anc.Count() <= max_cluster_count);
                     assert(anc == expect_anc);
                     // Check its descendants against simulation.
                     auto expect_desc = sim.graph.Descendants(i);
                     auto desc = sim.MakeSet(real->GetDescendants(*sim.GetRef(i), main_only));
-                    assert(desc.Count() <= max_count);
+                    assert(desc.Count() <= max_cluster_count);
                     assert(desc == expect_desc);
                     // Check the cluster the transaction is part of.
                     auto cluster = real->GetCluster(*sim.GetRef(i), main_only);
-                    assert(cluster.size() <= max_count);
+                    assert(cluster.size() <= max_cluster_count);
                     assert(sim.MakeSet(cluster) == component);
                     // Check that the cluster is reported in a valid topological order (its
                     // linearization).
                     std::vector<DepGraphIndex> simlin;
                     SimTxGraph::SetType done;
+                    uint64_t total_size{0};
                     for (TxGraph::Ref* ptr : cluster) {
                         auto simpos = sim.Find(ptr);
                         assert(sim.graph.Descendants(simpos).IsSubsetOf(component - done));
                         done.Set(simpos);
                         assert(sim.graph.Ancestors(simpos).IsSubsetOf(done));
                         simlin.push_back(simpos);
+                        total_size += sim.graph.FeeRate(simpos).size;
                     }
+                    // Check cluster size.
+                    assert(total_size <= max_cluster_size);
                     // Construct a chunking object for the simulated graph, using the reported cluster
                     // linearization as ordering, and compare it against the reported chunk feerates.
                     if (sims.size() == 1 || main_only) {

src/txgraph.cpp

@@ -109,6 +109,8 @@ class Cluster
     }
     /** Get the number of transactions in this Cluster. */
     LinearizationIndex GetTxCount() const noexcept { return m_linearization.size(); }
+    /** Get the total size of the transactions in this Cluster. */
+    uint64_t GetTotalTxSize() const noexcept;
     /** Given a DepGraphIndex into this Cluster, find the corresponding GraphIndex. */
     GraphIndex GetClusterEntry(DepGraphIndex index) const noexcept { return m_mapping[index]; }
     /** Only called by Graph::SwapIndexes. */
@@ -199,6 +201,8 @@ class TxGraphImpl final : public TxGraph
     FastRandomContext m_rng;
     /** This TxGraphImpl's maximum cluster count limit. */
     const DepGraphIndex m_max_cluster_count;
+    /** This TxGraphImpl's maximum cluster size limit. */
+    const uint64_t m_max_cluster_size;
 
     /** Information about one group of Clusters to be merged. */
     struct GroupEntry
@@ -401,9 +405,10 @@ class TxGraphImpl final : public TxGraph
     std::vector<GraphIndex> m_unlinked;
 
 public:
-    /** Construct a new TxGraphImpl with the specified maximum cluster count. */
-    explicit TxGraphImpl(DepGraphIndex max_cluster_count) noexcept :
+    /** Construct a new TxGraphImpl with the specified limits. */
+    explicit TxGraphImpl(DepGraphIndex max_cluster_count, uint64_t max_cluster_size) noexcept :
         m_max_cluster_count(max_cluster_count),
+        m_max_cluster_size(max_cluster_size),
         m_main_chunkindex(ChunkOrder(this))
     {
         Assume(max_cluster_count >= 1);
@@ -635,6 +640,15 @@ void TxGraphImpl::CreateChunkData(GraphIndex idx, LinearizationIndex chunk_count
     }
 }
 
+uint64_t Cluster::GetTotalTxSize() const noexcept
+{
+    uint64_t ret{0};
+    for (auto i : m_linearization) {
+        ret += m_depgraph.FeeRate(i).size;
+    }
+    return ret;
+}
+
 void TxGraphImpl::ClearLocator(int level, GraphIndex idx) noexcept
 {
     auto& entry = m_entries[idx];
@@ -1439,10 +1453,12 @@ void TxGraphImpl::GroupClusters(int level) noexcept
         new_entry.m_deps_offset = clusterset.m_deps_to_add.size();
         new_entry.m_deps_count = 0;
         uint32_t total_count{0};
+        uint64_t total_size{0};
         // Add all its clusters to it (copying those from an_clusters to m_group_clusters).
         while (an_clusters_it != an_clusters.end() && an_clusters_it->second == rep) {
             clusterset.m_group_data->m_group_clusters.push_back(an_clusters_it->first);
             total_count += an_clusters_it->first->GetTxCount();
+            total_size += an_clusters_it->first->GetTotalTxSize();
             ++an_clusters_it;
             ++new_entry.m_cluster_count;
         }
@@ -1453,7 +1469,7 @@ void TxGraphImpl::GroupClusters(int level) noexcept
             ++new_entry.m_deps_count;
         }
         // Detect oversizedness.
-        if (total_count > m_max_cluster_count) {
+        if (total_count > m_max_cluster_count || total_size > m_max_cluster_size) {
             clusterset.m_group_data->m_group_oversized = true;
         }
     }
@@ -1587,6 +1603,7 @@ Cluster::Cluster(uint64_t sequence, TxGraphImpl& graph, const FeePerWeight& feer
 TxGraph::Ref TxGraphImpl::AddTransaction(const FeePerWeight& feerate) noexcept
 {
     Assume(m_main_chunkindex_observers == 0 || GetTopLevel() != 0);
+    Assume(feerate.size > 0 && uint64_t(feerate.size) <= m_max_cluster_size);
     // Construct a new Ref.
     Ref ret;
     // Construct a new Entry, and link it with the Ref.
@@ -2124,6 +2141,10 @@ void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const
     assert(m_linearization.size() <= graph.m_max_cluster_count);
     // The level must match the level the Cluster occurs in.
     assert(m_level == level);
+    // The sum of their sizes cannot exceed m_max_cluster_size. Note that groups of to-be-merged
+    // clusters which would exceed this limit are marked oversized, which means they are never
+    // applied.
+    assert(GetTotalTxSize() <= graph.m_max_cluster_size);
     // m_quality and m_setindex are checked in TxGraphImpl::SanityCheck.
 
     // Compute the chunking of m_linearization.
@@ -2500,7 +2521,7 @@ TxGraph::Ref::Ref(Ref&& other) noexcept
     std::swap(m_index, other.m_index);
 }
 
-std::unique_ptr<TxGraph> MakeTxGraph(unsigned max_cluster_count) noexcept
+std::unique_ptr<TxGraph> MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size) noexcept
 {
-    return std::make_unique<TxGraphImpl>(max_cluster_count);
+    return std::make_unique<TxGraphImpl>(max_cluster_count, max_cluster_size);
 }

src/txgraph.h

@@ -63,10 +63,10 @@ class TxGraph
     /** Virtual destructor, so inheriting is safe. */
     virtual ~TxGraph() = default;
     /** Construct a new transaction with the specified feerate, and return a Ref to it.
-     *  If a staging graph exists, the new transaction is only created there. In all
-     *  further calls, only Refs created by AddTransaction() are allowed to be passed to this
-     *  TxGraph object (or empty Ref objects). Ref objects may outlive the TxGraph they were
-     *  created for. */
+     *  If a staging graph exists, the new transaction is only created there. feerate.size must be
+     *  strictly positive, and cannot exceed the graph's max cluster size. In all further calls,
+     *  only Refs created by AddTransaction() are allowed to be passed to this TxGraph object (or
+     *  empty Ref objects). Ref objects may outlive the TxGraph they were created for. */
     [[nodiscard]] virtual Ref AddTransaction(const FeePerWeight& feerate) noexcept = 0;
     /** Remove the specified transaction. If a staging graph exists, the removal only happens
      *  there. This is a no-op if the transaction was already removed.
@@ -240,8 +240,9 @@ class TxGraph
     };
 };
 
-/** Construct a new TxGraph with the specified limit on transactions within a cluster. That
- *  number cannot exceed MAX_CLUSTER_COUNT_LIMIT. */
-std::unique_ptr<TxGraph> MakeTxGraph(unsigned max_cluster_count) noexcept;
+/** Construct a new TxGraph with the specified limit on the number of transactions within a cluster,
+ *  and on the sum of transaction sizes within a cluster. max_cluster_count cannot exceed
+ *  MAX_CLUSTER_COUNT_LIMIT. */
+std::unique_ptr<TxGraph> MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size) noexcept;
 
 #endif // BITCOIN_TXGRAPH_H