txgraph: Permit transactions that exceed cluster size limit (feature)

This removes the restriction added in the previous commit that individual
transactions do not exceed the max cluster size limit.

With this change, the responsibility for enforcing cluster size limits can
be localized purely in TxGraph, without callers (and in particular, tests)
needing to duplicate the enforcement for individual transactions.

Author: sipa

src/test/fuzz/txgraph.cpp

@@ -134,6 +134,8 @@ struct SimTxGraph
         simmap[simpos] = std::make_shared<TxGraph::Ref>();
         auto ptr = simmap[simpos].get();
         simrevmap[ptr] = simpos;
+        // This may invalidate our cached oversized value.
+        if (oversized.has_value() && !*oversized) oversized = std::nullopt;
         return ptr;
     }
 
@@ -269,12 +271,8 @@ FUZZ_TARGET(txgraph)
     /** 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). */
+    /** The maximum total size of transactions in a (non-oversized) cluster. */
     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_cluster_count, max_cluster_size);
@@ -404,12 +402,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, max_tx_size);
+                    size = provider.ConsumeIntegralInRange<int32_t>(1, 0x3fffff);
                 } 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.ConsumeIntegralInRange<uint32_t>(1, std::min<uint32_t>(0xff, max_tx_size));
+                    size = provider.ConsumeIntegralInRange<uint32_t>(1, 0xff);
                 }
                 FeePerWeight feerate{fee, size};
                 // Create a real TxGraph::Ref.

src/txgraph.cpp

@@ -35,6 +35,9 @@ using ClusterSetIndex = uint32_t;
 /** Quality levels for cached cluster linearizations. */
 enum class QualityLevel
 {
+    /** This is a singleton cluster consisting of a transaction that individually exceeds the
+     *  cluster size limit. It cannot be merged with anything. */
+    OVERSIZED_SINGLETON,
     /** This cluster may have multiple disconnected components, which are all NEEDS_RELINEARIZE. */
     NEEDS_SPLIT,
     /** This cluster may have multiple disconnected components, which are all ACCEPTABLE. */
@@ -101,6 +104,10 @@ class Cluster
     {
         return m_quality == QualityLevel::OPTIMAL;
     }
+    /** Whether this cluster is oversized. Note that no changes that can cause oversizedness are
+     *  ever applied, so the only way a materialized Cluster object can be oversized is by being
+     *  an individually oversized transaction singleton. */
+    bool IsOversized() const noexcept { return m_quality == QualityLevel::OVERSIZED_SINGLETON; }
     /** Whether this cluster requires splitting. */
     bool NeedsSplitting() const noexcept
     {
@@ -225,7 +232,7 @@ class TxGraphImpl final : public TxGraph
         /** Which clusters are to be merged. GroupEntry::m_cluster_offset indexes into this. */
         std::vector<Cluster*> m_group_clusters;
         /** Whether at least one of the groups cannot be applied because it would result in a
-         *  Cluster that violates the cluster count limit. */
+         *  Cluster that violates the max cluster count or size limit. */
         bool m_group_oversized;
     };
 
@@ -248,8 +255,11 @@ class TxGraphImpl final : public TxGraph
         /** Total number of transactions in this graph (sum of all transaction counts in all
          *  Clusters, and for staging also those inherited from the main ClusterSet). */
         GraphIndex m_txcount{0};
+        /** Total number of individually oversized transactions in the graph. */
+        GraphIndex m_txcount_oversized{0};
         /** Whether this graph is oversized (if known). This roughly matches
-         *  m_group_data->m_group_oversized, but may be known even if m_group_data is not. */
+         *  m_group_data->m_group_oversized || (m_txcount_oversized > 0), but may be known even if
+         *  m_group_data is not. */
         std::optional<bool> m_oversized{false};
 
         ClusterSet() noexcept = default;
@@ -446,8 +456,10 @@ class TxGraphImpl final : public TxGraph
     /** Get a reference to the ClusterSet at the specified level (which must exist). */
     ClusterSet& GetClusterSet(int level) noexcept;
     const ClusterSet& GetClusterSet(int level) const noexcept;
-    /** Make a transaction not exist at a specified level. It must currently exist there. */
-    void ClearLocator(int level, GraphIndex index) noexcept;
+    /** Make a transaction not exist at a specified level. It must currently exist there.
+     *  oversized_tx indicates whether the transaction is an individually-oversized one
+     *  (OVERSIZED_SINGLETON). */
+    void ClearLocator(int level, GraphIndex index, bool oversized_tx) noexcept;
     /** Find which Clusters in main conflict with ones in staging. */
     std::vector<Cluster*> GetConflicts() const noexcept;
     /** Clear an Entry's ChunkData. */
@@ -649,7 +661,7 @@ uint64_t Cluster::GetTotalTxSize() const noexcept
     return ret;
 }
 
-void TxGraphImpl::ClearLocator(int level, GraphIndex idx) noexcept
+void TxGraphImpl::ClearLocator(int level, GraphIndex idx, bool oversized_tx) noexcept
 {
     auto& entry = m_entries[idx];
     auto& clusterset = GetClusterSet(level);
@@ -663,12 +675,14 @@ void TxGraphImpl::ClearLocator(int level, GraphIndex idx) noexcept
     }
     // Update the transaction count.
     --clusterset.m_txcount;
+    clusterset.m_txcount_oversized -= oversized_tx;
     // If clearing main, adjust the status of Locators of this transaction in staging, if it exists.
     if (level == 0 && GetTopLevel() == 1) {
         if (entry.m_locator[1].IsRemoved()) {
             entry.m_locator[1].SetMissing();
         } else if (!entry.m_locator[1].IsPresent()) {
             --m_staging_clusterset->m_txcount;
+            m_staging_clusterset->m_txcount_oversized -= oversized_tx;
         }
     }
     if (level == 0) ClearChunkData(entry);
@@ -799,7 +813,7 @@ void Cluster::ApplyRemovals(TxGraphImpl& graph, std::span<GraphIndex>& to_remove
             entry.m_main_lin_index = LinearizationIndex(-1);
         }
         // - Mark it as missing/removed in the Entry's locator.
-        graph.ClearLocator(m_level, idx);
+        graph.ClearLocator(m_level, idx, m_quality == QualityLevel::OVERSIZED_SINGLETON);
         to_remove = to_remove.subspan(1);
     } while(!to_remove.empty());
 
@@ -838,7 +852,7 @@ void Cluster::ApplyRemovals(TxGraphImpl& graph, std::span<GraphIndex>& to_remove
 void Cluster::Clear(TxGraphImpl& graph) noexcept
 {
     for (auto i : m_linearization) {
-        graph.ClearLocator(m_level, m_mapping[i]);
+        graph.ClearLocator(m_level, m_mapping[i], m_quality == QualityLevel::OVERSIZED_SINGLETON);
     }
     m_depgraph = {};
     m_linearization.clear();
@@ -1298,6 +1312,17 @@ void TxGraphImpl::GroupClusters(int level) noexcept
      *  to-be-merged group). */
     std::vector<std::pair<std::pair<GraphIndex, GraphIndex>, uint64_t>> an_deps;
 
+    // Construct an an_clusters entry for every oversized cluster, including ones from levels below,
+    // as they may be inherited in this one.
+    for (int level_iter = 0; level_iter <= level; ++level_iter) {
+        for (auto& cluster : GetClusterSet(level_iter).m_clusters[int(QualityLevel::OVERSIZED_SINGLETON)]) {
+            auto graph_idx = cluster->GetClusterEntry(0);
+            auto cur_cluster = FindCluster(graph_idx, level);
+            if (cur_cluster == nullptr) continue;
+            an_clusters.emplace_back(cur_cluster, cur_cluster->m_sequence);
+        }
+    }
+
     // Construct a an_clusters entry for every parent and child in the to-be-applied dependencies,
     // and an an_deps entry for each dependency to be applied.
     an_deps.reserve(clusterset.m_deps_to_add.size());
@@ -1314,7 +1339,7 @@ void TxGraphImpl::GroupClusters(int level) noexcept
         an_deps.emplace_back(std::pair{par, chl}, chl_cluster->m_sequence);
     }
     // Sort and deduplicate an_clusters, so we end up with a sorted list of all involved Clusters
-    // to which dependencies apply.
+    // to which dependencies apply, or which are oversized.
     std::sort(an_clusters.begin(), an_clusters.end(), [](auto& a, auto& b) noexcept { return a.second < b.second; });
     an_clusters.erase(std::unique(an_clusters.begin(), an_clusters.end()), an_clusters.end());
     // Sort an_deps by applying the same order to the involved child cluster.
@@ -1517,7 +1542,7 @@ void TxGraphImpl::ApplyDependencies(int level) noexcept
     // Nothing to do if there are no dependencies to be added.
     if (clusterset.m_deps_to_add.empty()) return;
     // Dependencies cannot be applied if it would result in oversized clusters.
-    if (clusterset.m_group_data->m_group_oversized) return;
+    if (clusterset.m_oversized == true) return;
 
     // For each group of to-be-merged Clusters.
     for (const auto& group_entry : clusterset.m_group_data->m_groups) {
@@ -1573,7 +1598,7 @@ void Cluster::Relinearize(TxGraphImpl& graph, uint64_t max_iters) noexcept
 void TxGraphImpl::MakeAcceptable(Cluster& cluster) noexcept
 {
     // Relinearize the Cluster if needed.
-    if (!cluster.NeedsSplitting() && !cluster.IsAcceptable()) {
+    if (!cluster.NeedsSplitting() && !cluster.IsAcceptable() && !cluster.IsOversized()) {
         cluster.Relinearize(*this, 10000);
     }
 }
@@ -1603,7 +1628,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);
+    Assume(feerate.size > 0);
     // Construct a new Ref.
     Ref ret;
     // Construct a new Entry, and link it with the Ref.
@@ -1615,13 +1640,20 @@ TxGraph::Ref TxGraphImpl::AddTransaction(const FeePerWeight& feerate) noexcept
     GetRefGraph(ret) = this;
     GetRefIndex(ret) = idx;
     // Construct a new singleton Cluster (which is necessarily optimally linearized).
+    bool oversized = uint64_t(feerate.size) > m_max_cluster_size;
     auto cluster = std::make_unique<Cluster>(m_next_sequence_counter++, *this, feerate, idx);
     auto cluster_ptr = cluster.get();
     int level = GetTopLevel();
     auto& clusterset = GetClusterSet(level);
-    InsertCluster(level, std::move(cluster), QualityLevel::OPTIMAL);
+    InsertCluster(level, std::move(cluster), oversized ? QualityLevel::OVERSIZED_SINGLETON : QualityLevel::OPTIMAL);
     cluster_ptr->Updated(*this);
     ++clusterset.m_txcount;
+    // Deal with individually oversized transactions.
+    if (oversized) {
+        ++clusterset.m_txcount_oversized;
+        clusterset.m_oversized = true;
+        clusterset.m_group_data = std::nullopt;
+    }
     // Return the Ref.
     return ret;
 }
@@ -1934,11 +1966,15 @@ bool TxGraphImpl::IsOversized(bool main_only) noexcept
         // Return cached value if known.
         return *clusterset.m_oversized;
     }
-    // Find which Clusters will need to be merged together, as that is where the oversize
-    // property is assessed.
-    GroupClusters(level);
-    Assume(clusterset.m_group_data.has_value());
-    clusterset.m_oversized = clusterset.m_group_data->m_group_oversized;
+    ApplyRemovals(level);
+    if (clusterset.m_txcount_oversized > 0) {
+        clusterset.m_oversized = true;
+    } else {
+        // Find which Clusters will need to be merged together, as that is where the oversize
+        // property is assessed.
+        GroupClusters(level);
+    }
+    Assume(clusterset.m_oversized.has_value());
     return *clusterset.m_oversized;
 }
 
@@ -1958,6 +1994,7 @@ void TxGraphImpl::StartStaging() noexcept
     // Copy statistics, precomputed data, and to-be-applied dependencies (only if oversized) to
     // the new graph. To-be-applied removals will always be empty at this point.
     m_staging_clusterset->m_txcount = m_main_clusterset.m_txcount;
+    m_staging_clusterset->m_txcount_oversized = m_main_clusterset.m_txcount_oversized;
     m_staging_clusterset->m_deps_to_add = m_main_clusterset.m_deps_to_add;
     m_staging_clusterset->m_group_data = m_main_clusterset.m_group_data;
     m_staging_clusterset->m_oversized = m_main_clusterset.m_oversized;
@@ -1985,7 +2022,13 @@ void TxGraphImpl::AbortStaging() noexcept
     if (!m_main_clusterset.m_group_data.has_value()) {
         // In case m_oversized in main was kept after a Ref destruction while staging exists, we
         // need to re-evaluate m_oversized now.
-        m_main_clusterset.m_oversized = std::nullopt;
+        if (m_main_clusterset.m_to_remove.empty() && m_main_clusterset.m_txcount_oversized > 0) {
+            // It is possible that a Ref destruction caused a removal in main while staging existed.
+            // In this case, m_txcount_oversized may be inaccurate.
+            m_main_clusterset.m_oversized = true;
+        } else {
+            m_main_clusterset.m_oversized = std::nullopt;
+        }
     }
 }
 
@@ -2019,6 +2062,7 @@ void TxGraphImpl::CommitStaging() noexcept
     m_main_clusterset.m_group_data = std::move(m_staging_clusterset->m_group_data);
     m_main_clusterset.m_oversized = std::move(m_staging_clusterset->m_oversized);
     m_main_clusterset.m_txcount = std::move(m_staging_clusterset->m_txcount);
+    m_main_clusterset.m_txcount_oversized = std::move(m_staging_clusterset->m_txcount_oversized);
     // Delete the old staging graph, after all its information was moved to main.
     m_staging_clusterset.reset();
     Compact();
@@ -2033,7 +2077,9 @@ void Cluster::SetFee(TxGraphImpl& graph, DepGraphIndex idx, int64_t fee) noexcep
     // Update the fee, remember that relinearization will be necessary, and update the Entries
     // in the same Cluster.
     m_depgraph.FeeRate(idx).fee = fee;
-    if (!NeedsSplitting()) {
+    if (m_quality == QualityLevel::OVERSIZED_SINGLETON) {
+        // Nothing to do.
+    } else if (!NeedsSplitting()) {
         graph.SetClusterQuality(m_level, m_quality, m_setindex, QualityLevel::NEEDS_RELINEARIZE);
     } else {
         graph.SetClusterQuality(m_level, m_quality, m_setindex, QualityLevel::NEEDS_SPLIT);
@@ -2141,12 +2187,15 @@ 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);
+    // The sum of their sizes cannot exceed m_max_cluster_size, unless it is an individually
+    // oversized transaction singleton. Note that groups of to-be-merged clusters which would
+    // exceed this limit are marked oversized, which means they are never applied.
+    assert(m_quality == QualityLevel::OVERSIZED_SINGLETON || GetTotalTxSize() <= graph.m_max_cluster_size);
     // m_quality and m_setindex are checked in TxGraphImpl::SanityCheck.
 
+    // OVERSIZED clusters are singletons.
+    assert(m_quality != QualityLevel::OVERSIZED_SINGLETON || m_linearization.size() == 1);
+
     // Compute the chunking of m_linearization.
     LinearizationChunking linchunking(m_depgraph, m_linearization);
 
@@ -2317,9 +2366,11 @@ void TxGraphImpl::SanityCheck() const
         if (!clusterset.m_to_remove.empty()) compact_possible = false;
         if (!clusterset.m_removed.empty()) compact_possible = false;
 
-        // If m_group_data exists, its m_group_oversized must match m_oversized.
-        if (clusterset.m_group_data.has_value()) {
-            assert(clusterset.m_oversized == clusterset.m_group_data->m_group_oversized);
+        // If m_group_data exists, and no outstanding removals remain, m_group_oversized must match
+        // m_group_oversized || (m_txcount_oversized > 0).
+        if (clusterset.m_group_data.has_value() && clusterset.m_to_remove.empty()) {
+            assert(clusterset.m_oversized ==
+                   (clusterset.m_group_data->m_group_oversized || (clusterset.m_txcount_oversized > 0)));
         }
 
         // For non-top levels, m_oversized must be known (as it cannot change until the level

src/txgraph.h

@@ -64,9 +64,9 @@ class TxGraph
     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. 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. */
+     *  strictly positive. 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.