From 13aad26b78481f2abd6d5e3ae6218e46e4d0060a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 10 Dec 2025 15:06:21 -0500 Subject: [PATCH] clusterlin: randomize various decisions in SFL (feature) This introduces a local RNG inside the SFL state, which is used to randomize various decisions inside the algorithm, in order to make it hard to create pathological clusters which predictably have bad performance. The decisions being randomized are: * When deciding what chunk to attempt to split, the queue order is randomized. * When deciding which dependency to split on, a uniformly random one is chosen among those with higher top feerate than bottom feerate within the chosen chunk. * When deciding which chunks to merge, a uniformly random one among those with the higher feerate difference is picked. * When merging two chunks, a uniformly random dependency between them is now activated. * When making the state topological, the queue of chunks to process is randomized. --- src/cluster_linearize.h | 116 ++++++++++++++++++++-------- src/test/fuzz/cluster_linearize.cpp | 2 +- src/test/util/cluster_linearize.h | 16 ++-- 3 files changed, 91 insertions(+), 43 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index 4d0d7c55d44..6639244f35e 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -682,25 +682,28 @@ public: * - How to decide which chunks to merge: * - The merge upwards and downward rules specify that the lowest-feerate respectively * highest-feerate candidate chunk is merged with, but if there are multiple equal-feerate - * candidates, the chunk with the highest-index transaction involving a relevant dependency is - * picked (this will be changed in a later commit). + * candidates, a uniformly random one among them is picked. * * - How to decide what dependency to activate (when merging chunks): - * - After picking two chunks to be merged (see above), the dependency with the lowest-index - * transaction in the other chunk is activated (this will be changed in a later commit). + * - After picking two chunks to be merged (see above), a uniformly random dependency between the + * two chunks is activated. * * - How to decide which chunk to find a dependency to split in: - * - A round-robin queue of chunks to improve is maintained. + * - A round-robin queue of chunks to improve is maintained. The initial ordering of this queue + * is uniformly randomly permuted. * * - How to decide what dependency to deactivate (when splitting chunks): * - Inside the selected chunk (see above), among the dependencies whose top feerate is strictly - * higher than its bottom feerate in the selected chunk, if any, the one with the lowest-index - * child is deactivated (this will be changed in a later commit). + * higher than its bottom feerate in the selected chunk, if any, a uniformly random dependency + * is deactivated. */ template class SpanningForestState { private: + /** Internal RNG. */ + InsecureRandomContext m_rng; + /** Data type to represent indexing into m_tx_data. */ using TxIdx = uint32_t; /** Data type to represent indexing into m_dep_data. */ @@ -876,20 +879,30 @@ private: Assume(top_chunk.chunk_rep == top_rep); auto& bottom_chunk = m_tx_data[bottom_rep]; Assume(bottom_chunk.chunk_rep == bottom_rep); - // Activate the first dependency between bottom_chunk and top_chunk. + // Count the number of dependencies between bottom_chunk and top_chunk. + TxIdx num_deps{0}; for (auto tx : top_chunk.chunk_setinfo.transactions) { auto& tx_data = m_tx_data[tx]; - // As an optimization, only iterate over transactions which have dependencies in the - // bottom chunk. - if (tx_data.children.Overlaps(bottom_chunk.chunk_setinfo.transactions)) { + num_deps += (tx_data.children & bottom_chunk.chunk_setinfo.transactions).Count(); + } + Assume(num_deps > 0); + // Uniformly randomly pick one of them and activate it. + TxIdx pick = m_rng.randrange(num_deps); + for (auto tx : top_chunk.chunk_setinfo.transactions) { + auto& tx_data = m_tx_data[tx]; + auto intersect = tx_data.children & bottom_chunk.chunk_setinfo.transactions; + auto count = intersect.Count(); + if (pick < count) { for (auto dep : tx_data.child_deps) { auto& dep_data = m_dep_data[dep]; if (bottom_chunk.chunk_setinfo.transactions[dep_data.child]) { - return Activate(dep); + if (pick == 0) return Activate(dep); + --pick; } } break; } + pick -= count; } Assume(false); return TxIdx(-1); @@ -906,7 +919,7 @@ private: // Iterate over all transactions in the chunk, figuring out which other chunk each // depends on, but only testing each other chunk once. For those depended-on chunks, // remember the highest-feerate (if DownWard) or lowest-feerate (if !DownWard) one. - // If multiple equal-feerate candidate chunks to merge with exist, pick the last one + // If multiple equal-feerate candidate chunks to merge with exist, pick a random one // among them. /** Which transactions have been reached from this chunk already. Initialize with the @@ -918,6 +931,9 @@ private: FeeFrac best_other_chunk_feerate = chunk_data.chunk_setinfo.feerate; /** The representative for the best candidate chunk to merge with. -1 if none. */ TxIdx best_other_chunk_rep = TxIdx(-1); + /** We generate random tiebreak values to pick between equal-feerate candidate chunks. + * This variable stores the tiebreak of the current best candidate. */ + uint64_t best_other_chunk_tiebreak{0}; for (auto tx : chunk_txn) { auto& tx_data = m_tx_data[tx]; /** The transactions reached by following dependencies from tx that have not been @@ -932,9 +948,12 @@ private: // See if it has an acceptable feerate. auto cmp = DownWard ? FeeRateCompare(best_other_chunk_feerate, reached_chunk.feerate) : FeeRateCompare(reached_chunk.feerate, best_other_chunk_feerate); - if (cmp <= 0) { + if (cmp > 0) continue; + uint64_t tiebreak = m_rng.rand64(); + if (cmp < 0 || tiebreak >= best_other_chunk_tiebreak) { best_other_chunk_feerate = reached_chunk.feerate; best_other_chunk_rep = reached_chunk_rep; + best_other_chunk_tiebreak = tiebreak; } } } @@ -989,7 +1008,7 @@ private: public: /** Construct a spanning forest for the given DepGraph, with every transaction in its own chunk * (not topological). */ - explicit SpanningForestState(const DepGraph& depgraph) noexcept + explicit SpanningForestState(const DepGraph& depgraph, uint64_t rng_seed) noexcept : m_rng(rng_seed) { m_transaction_idxs = depgraph.Positions(); auto num_transactions = m_transaction_idxs.Count(); @@ -1050,6 +1069,11 @@ public: auto& tx_data = m_tx_data[tx]; if (tx_data.chunk_rep == tx) { m_suboptimal_chunks.emplace_back(tx); + // Randomize the initial order of suboptimal chunks in the queue. + TxIdx j = m_rng.randrange(m_suboptimal_chunks.size()); + if (j != m_suboptimal_chunks.size() - 1) { + std::swap(m_suboptimal_chunks.back(), m_suboptimal_chunks[j]); + } } } while (!m_suboptimal_chunks.empty()) { @@ -1060,17 +1084,23 @@ public: // If what was popped is not currently a chunk representative, continue. This may // happen when it was merged with something else since being added. if (chunk_data.chunk_rep != chunk) continue; - // Attempt to merge the chunk upwards. - auto result_up = MergeStep(chunk); - if (result_up != TxIdx(-1)) { - m_suboptimal_chunks.push_back(result_up); - continue; - } - // Attempt to merge the chunk downwards. - auto result_down = MergeStep(chunk); - if (result_down != TxIdx(-1)) { - m_suboptimal_chunks.push_back(result_down); - continue; + int flip = m_rng.randbool(); + for (int i = 0; i < 2; ++i) { + if (i ^ flip) { + // Attempt to merge the chunk upwards. + auto result_up = MergeStep(chunk); + if (result_up != TxIdx(-1)) { + m_suboptimal_chunks.push_back(result_up); + break; + } + } else { + // Attempt to merge the chunk downwards. + auto result_down = MergeStep(chunk); + if (result_down != TxIdx(-1)) { + m_suboptimal_chunks.push_back(result_down); + break; + } + } } } } @@ -1083,6 +1113,11 @@ public: auto& tx_data = m_tx_data[tx]; if (tx_data.chunk_rep == tx) { m_suboptimal_chunks.push_back(tx); + // Randomize the initial order of suboptimal chunks in the queue. + TxIdx j = m_rng.randrange(m_suboptimal_chunks.size()); + if (j != m_suboptimal_chunks.size() - 1) { + std::swap(m_suboptimal_chunks.back(), m_suboptimal_chunks[j]); + } } } } @@ -1099,7 +1134,10 @@ public: // happen when a split chunk merges in Improve() with one or more existing chunks that // are themselves on the suboptimal queue already. if (chunk_data.chunk_rep != chunk) continue; - // Iterate over all transactions of the chunk. + // Remember the best dependency seen so far. + DepIdx candidate_dep = DepIdx(-1); + uint64_t candidate_tiebreak = 0; + // Iterate over all transactions. for (auto tx : chunk_data.chunk_setinfo.transactions) { const auto& tx_data = m_tx_data[tx]; // Iterate over all active child dependencies of the transaction. @@ -1109,13 +1147,24 @@ public: if (!dep_data.active) continue; // Skip if this dependency is ineligible (the top chunk that would be created // does not have higher feerate than the chunk it is currently part of). - if (!(dep_data.top_setinfo.feerate >> chunk_data.chunk_setinfo.feerate)) continue; - // Otherwise, deactivate it and then make the state topological again with a - // sequence of merges. - Improve(dep_idx); - return true; + auto cmp = FeeRateCompare(dep_data.top_setinfo.feerate, chunk_data.chunk_setinfo.feerate); + if (cmp <= 0) continue; + // Generate a random tiebreak for this dependency, and reject it if its tiebreak + // is worse than the best so far. This means that among all eligible + // dependencies, a uniformly random one will be chosen. + uint64_t tiebreak = m_rng.rand64(); + if (tiebreak < candidate_tiebreak) continue; + // Remember this as our (new) candidate dependency. + candidate_dep = dep_idx; + candidate_tiebreak = tiebreak; } } + // If a candidate with positive gain was found, deactivate it and then make the state + // topological again with a sequence of merges. + if (candidate_dep != DepIdx(-1)) Improve(candidate_dep); + // Stop processing for now, even if nothing was activated, as the loop above may have + // had a nontrivial cost. + return !m_suboptimal_chunks.empty(); } // No improvable chunk was found, we are done. return false; @@ -1397,9 +1446,8 @@ public: template std::tuple, bool, uint64_t> Linearize(const DepGraph& depgraph, uint64_t max_iterations, uint64_t rng_seed, std::span old_linearization = {}) noexcept { - (void)rng_seed; // Unused for now. /** Initialize a spanning forest data structure for this cluster. */ - SpanningForestState forest(depgraph); + SpanningForestState forest(depgraph, rng_seed); if (!old_linearization.empty()) { forest.LoadLinearization(old_linearization); } else { diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index fe18407dbb4..a32fc169b89 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -1021,7 +1021,7 @@ FUZZ_TARGET(clusterlin_sfl) // Initialize SFL state. if (make_connected) MakeConnected(depgraph); - SpanningForestState sfl(depgraph); + SpanningForestState sfl(depgraph, rng.rand64()); // Function to test the state. std::vector last_diagram; diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index 2e73032e2c3..f57c486bd97 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -402,14 +402,14 @@ inline uint64_t MaxOptimalLinearizationIters(DepGraphIndex cluster_count) // *some* reasonable cost bound, optimal linearizations are always found. static constexpr uint64_t ITERS[65] = { 0, - 0, 2, 8, 21, 51, 99, 162, 208, - 300, 349, 489, 627, 776, 867, 982, 1204, - 1414, 1473, 1770, 2045, 2285, 2417, 3669, 3953, - 3816, 5720, 4103, 5934, 5443, 5323, 6338, 6407, - 7671, 11625, 11799, 10104, 9631, 11203, 12487, 15262, - 17800, 14132, 21915, 16495, 23350, 21304, 22221, 22230, - 26119, 22182, 31118, 30848, 32166, 37174, 39708, 36189, - 42747, 43689, 46555, 39818, 51077, 58489, 72633, 59756 + 0, 2, 8, 21, 51, 96, 162, 200, + 273, 323, 413, 506, 602, 788, 883, 948, + 1153, 1187, 1367, 1619, 1854, 2271, 2257, 2707, + 2904, 3275, 3342, 4209, 4648, 4146, 4273, 4905, + 5358, 5767, 5977, 6777, 7812, 7689, 8478, 8425, + 9561, 11765, 10743, 11806, 12812, 12838, 15421, 16778, + 16661, 19393, 17995, 23947, 23314, 24564, 26209, 29267, + 24719, 31065, 31794, 29185, 32465, 35432, 39986, 36865 }; assert(cluster_count < std::size(ITERS)); // Multiply the table number by two, to account for the fact that they are not absolutes.