diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index d7f17f3353b..460e6c53a52 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -134,6 +134,8 @@ struct SimTxGraph simmap[simpos] = std::make_shared(); 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(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(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(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(-0x8000000000000, 0x7ffffffffffff); - size = provider.ConsumeIntegralInRange(1, max_tx_size); + size = provider.ConsumeIntegralInRange(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(); - size = provider.ConsumeIntegralInRange(1, std::min(0xff, max_tx_size)); + size = provider.ConsumeIntegralInRange(1, 0xff); } FeePerWeight feerate{fee, size}; // Create a real TxGraph::Ref. diff --git a/src/txgraph.cpp b/src/txgraph.cpp index 2629e8a2e88..0853e26e743 100644 --- a/src/txgraph.cpp +++ b/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 @@ public: { 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 @@ private: /** Which clusters are to be merged. GroupEntry::m_cluster_offset indexes into this. */ std::vector 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 @@ private: /** 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 m_oversized{false}; ClusterSet() noexcept = default; @@ -446,8 +456,10 @@ public: /** 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 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& 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& 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, 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(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 diff --git a/src/txgraph.h b/src/txgraph.h index 95bdecc3ddf..077b2a4e852 100644 --- a/src/txgraph.h +++ b/src/txgraph.h @@ -64,9 +64,9 @@ public: 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.