diff --git a/src/bench/txgraph.cpp b/src/bench/txgraph.cpp index 7257a043777..00074db658f 100644 --- a/src/bench/txgraph.cpp +++ b/src/bench/txgraph.cpp @@ -12,6 +12,11 @@ namespace { +std::strong_ordering PointerComparator(const TxGraph::Ref& a, const TxGraph::Ref& b) noexcept +{ + return (&a) <=> (&b); +} + void BenchTxGraphTrim(benchmark::Bench& bench) { // The from-block transactions consist of 1000 fully linear clusters, each with 64 @@ -60,7 +65,7 @@ void BenchTxGraphTrim(benchmark::Bench& bench) std::vector top_components; InsecureRandomContext rng(11); - auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS); + auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS, PointerComparator); // Construct the top chains. for (int chain = 0; chain < NUM_TOP_CHAINS; ++chain) { diff --git a/src/primitives/transaction_identifier.h b/src/primitives/transaction_identifier.h index 9b518d7d795..a1109591277 100644 --- a/src/primitives/transaction_identifier.h +++ b/src/primitives/transaction_identifier.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -42,7 +43,7 @@ public: template bool operator==(const Other& other) const { return Compare(other) == 0; } template - bool operator<(const Other& other) const { return Compare(other) < 0; } + std::strong_ordering operator<=>(const Other& other) const { return Compare(other) <=> 0; } const uint256& ToUint256() const LIFETIMEBOUND { return m_wrapped; } static transaction_identifier FromUint256(const uint256& id) { return {id}; } diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index 51546f612ec..5352e2c1e30 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -25,6 +25,10 @@ namespace { struct SimTxObject : public TxGraph::Ref { + // Use random uint64_t as txids for this simulation (0 = empty object). + const uint64_t m_txid{0}; + SimTxObject() noexcept = default; + explicit SimTxObject(uint64_t txid) noexcept : m_txid(txid) {} }; /** Data type representing a naive simulated TxGraph, keeping all transactions (even from @@ -142,14 +146,14 @@ struct SimTxGraph } /** Add a new transaction to the simulation and the specified real graph. */ - void AddTransaction(TxGraph& txgraph, const FeePerWeight& feerate) + void AddTransaction(TxGraph& txgraph, const FeePerWeight& feerate, uint64_t txid) { assert(graph.TxCount() < MAX_TRANSACTIONS); auto simpos = graph.AddTransaction(feerate); real_is_optimal = false; MakeModified(simpos); assert(graph.Positions()[simpos]); - simmap[simpos] = std::make_shared(); + simmap[simpos] = std::make_shared(txid); txgraph.AddTransaction(*simmap[simpos], feerate); auto ptr = simmap[simpos].get(); simrevmap[ptr] = simpos; @@ -324,8 +328,23 @@ FUZZ_TARGET(txgraph) /** The number of iterations to consider a cluster acceptably linearized. */ auto acceptable_iters = provider.ConsumeIntegralInRange(0, 10000); + /** The set of uint64_t "txid"s that have been assigned before. */ + std::set assigned_txids; + // Construct a real graph, and a vector of simulated graphs (main, and possibly staging). - auto real = MakeTxGraph(max_cluster_count, max_cluster_size, acceptable_iters); + auto fallback_order = [&](const TxGraph::Ref& a, const TxGraph::Ref& b) noexcept { + uint64_t txid_a = static_cast(a).m_txid; + uint64_t txid_b = static_cast(b).m_txid; + assert(assigned_txids.contains(txid_a)); + assert(assigned_txids.contains(txid_b)); + return txid_a <=> txid_b; + }; + auto real = MakeTxGraph( + /*max_cluster_count=*/max_cluster_count, + /*max_cluster_size=*/max_cluster_size, + /*acceptable_iters=*/acceptable_iters, + /*fallback_order=*/fallback_order); + std::vector sims; sims.reserve(2); sims.emplace_back(max_cluster_count, max_cluster_size); @@ -460,8 +479,14 @@ FUZZ_TARGET(txgraph) size = provider.ConsumeIntegralInRange(1, 0xff); } FeePerWeight feerate{fee, size}; + // Pick a novel txid (and not 0, which is reserved for empty_ref). + uint64_t txid; + do { + txid = rng.rand64(); + } while (txid == 0 || assigned_txids.contains(txid)); + assigned_txids.insert(txid); // Create the transaction in the simulation and the real graph. - top_sim.AddTransaction(*real, feerate); + top_sim.AddTransaction(*real, feerate, txid); break; } else if ((block_builders.empty() || sims.size() > 1) && top_sim.GetTransactionCount() + top_sim.removed.size() > 1 && command-- == 0) { // AddDependency. @@ -1069,7 +1094,12 @@ FUZZ_TARGET(txgraph) // that calling Linearize on it does not improve it further. if (sims[0].real_is_optimal) { auto real_diagram = ChunkLinearization(sims[0].graph, vec1); - auto [sim_lin, sim_optimal, _cost] = Linearize(sims[0].graph, 300000, rng.rand64(), IndexTxOrder{}, vec1); + auto fallback_order_sim = [&](DepGraphIndex a, DepGraphIndex b) noexcept { + auto txid_a = sims[0].GetRef(a)->m_txid; + auto txid_b = sims[0].GetRef(b)->m_txid; + return txid_a <=> txid_b; + }; + auto [sim_lin, sim_optimal, _cost] = Linearize(sims[0].graph, 300000, rng.rand64(), fallback_order_sim, vec1); PostLinearize(sims[0].graph, sim_lin); auto sim_diagram = ChunkLinearization(sims[0].graph, sim_lin); auto cmp = CompareChunks(real_diagram, sim_diagram); diff --git a/src/test/txgraph_tests.cpp b/src/test/txgraph_tests.cpp index 31fefd9ce18..f069e5bdc75 100644 --- a/src/test/txgraph_tests.cpp +++ b/src/test/txgraph_tests.cpp @@ -13,9 +13,18 @@ BOOST_AUTO_TEST_SUITE(txgraph_tests) +namespace { + /** The number used as acceptable_iters argument in these tests. High enough that everything * should be optimal, always. */ -static constexpr uint64_t NUM_ACCEPTABLE_ITERS = 100'000'000; +constexpr uint64_t NUM_ACCEPTABLE_ITERS = 100'000'000; + +std::strong_ordering PointerComparator(const TxGraph::Ref& a, const TxGraph::Ref& b) noexcept +{ + return (&a) <=> (&b); +} + +} // namespace BOOST_AUTO_TEST_CASE(txgraph_trim_zigzag) { @@ -39,7 +48,7 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_zigzag) static constexpr int32_t MAX_CLUSTER_SIZE = 100'000 * 100; // Create a new graph for the test. - auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS); + auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS, PointerComparator); // Add all transactions and store their Refs. std::vector refs; @@ -102,7 +111,7 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_flower) /** Set a very large cluster size limit so that only the count limit is triggered. */ static constexpr int32_t MAX_CLUSTER_SIZE = 100'000 * 100; - auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS); + auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS, PointerComparator); // Add all transactions and store their Refs. std::vector refs; @@ -188,7 +197,7 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_huge) std::vector top_components; FastRandomContext rng; - auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS); + auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS, PointerComparator); // Construct the top chains. for (int chain = 0; chain < NUM_TOP_CHAINS; ++chain) { @@ -261,7 +270,7 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_big_singletons) static constexpr int NUM_TOTAL_TX = 100; // Create a new graph for the test. - auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS); + auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS, PointerComparator); // Add all transactions and store their Refs. std::vector refs; @@ -295,7 +304,7 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_big_singletons) BOOST_AUTO_TEST_CASE(txgraph_chunk_chain) { // Create a new graph for the test. - auto graph = MakeTxGraph(50, 1000, NUM_ACCEPTABLE_ITERS); + auto graph = MakeTxGraph(50, 1000, NUM_ACCEPTABLE_ITERS, PointerComparator); auto block_builder_checker = [&graph](std::vector> expected_chunks) { std::vector> chunks; @@ -374,7 +383,7 @@ BOOST_AUTO_TEST_CASE(txgraph_staging) /* Create a new graph for the test. * The parameters are max_cluster_count, max_cluster_size, acceptable_iters */ - auto graph = MakeTxGraph(10, 1000, NUM_ACCEPTABLE_ITERS); + auto graph = MakeTxGraph(10, 1000, NUM_ACCEPTABLE_ITERS, PointerComparator); std::vector refs; refs.reserve(2); diff --git a/src/txgraph.cpp b/src/txgraph.cpp index 492356a899d..2c024c969a7 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -403,6 +403,8 @@ private: /** The number of linearization improvement steps needed per cluster to be considered * acceptable. */ const uint64_t m_acceptable_iters; + /** Fallback ordering for transactions. */ + const std::function m_fallback_order; /** Information about one group of Clusters to be merged. */ struct GroupEntry @@ -621,11 +623,17 @@ private: std::vector m_unlinked; public: - /** Construct a new TxGraphImpl with the specified limits. */ - explicit TxGraphImpl(DepGraphIndex max_cluster_count, uint64_t max_cluster_size, uint64_t acceptable_iters) noexcept : + /** Construct a new TxGraphImpl with the specified limits and fallback order. */ + explicit TxGraphImpl( + DepGraphIndex max_cluster_count, + uint64_t max_cluster_size, + uint64_t acceptable_iters, + const std::function& fallback_order + ) noexcept : m_max_cluster_count(max_cluster_count), m_max_cluster_size(max_cluster_size), m_acceptable_iters(acceptable_iters), + m_fallback_order(fallback_order), m_main_chunkindex(ChunkOrder(this)) { Assume(max_cluster_count >= 1); @@ -3523,7 +3531,11 @@ TxGraph::Ref::Ref(Ref&& other) noexcept std::swap(m_index, other.m_index); } -std::unique_ptr MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size, uint64_t acceptable_iters) noexcept +std::unique_ptr MakeTxGraph( + unsigned max_cluster_count, + uint64_t max_cluster_size, + uint64_t acceptable_iters, + const std::function& fallback_order) noexcept { - return std::make_unique(max_cluster_count, max_cluster_size, acceptable_iters); + return std::make_unique(max_cluster_count, max_cluster_size, acceptable_iters, fallback_order); } diff --git a/src/txgraph.h b/src/txgraph.h index 385db0147cc..8c7a91744fd 100644 --- a/src/txgraph.h +++ b/src/txgraph.h @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -253,9 +254,20 @@ public: }; /** 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. acceptable_iters controls how many linearization optimization - * steps will be performed per cluster before they are considered to be of acceptable quality. */ -std::unique_ptr MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size, uint64_t acceptable_iters) noexcept; + * and on the sum of transaction sizes within a cluster. + * + * - max_cluster_count cannot exceed MAX_CLUSTER_COUNT_LIMIT. + * - acceptable_iters controls how many linearization optimization steps will be performed per + * cluster before they are considered to be of acceptable quality. + * - fallback_order determines how to break tie-breaks between transactions: + * fallback_order(a, b) < 0 means a is "better" than b, and will (in case of ties) be placed + * first. This ordering must be stable over the transactions' lifetimes. + */ +std::unique_ptr MakeTxGraph( + unsigned max_cluster_count, + uint64_t max_cluster_size, + uint64_t acceptable_iters, + const std::function& fallback_order +) noexcept; #endif // BITCOIN_TXGRAPH_H diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 58262e7f2f9..b8be3e080e7 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -176,7 +176,15 @@ static CTxMemPool::Options&& Flatten(CTxMemPool::Options&& opts, bilingual_str& CTxMemPool::CTxMemPool(Options opts, bilingual_str& error) : m_opts{Flatten(std::move(opts), error)} { - m_txgraph = MakeTxGraph(m_opts.limits.cluster_count, m_opts.limits.cluster_size_vbytes * WITNESS_SCALE_FACTOR, ACCEPTABLE_ITERS); + m_txgraph = MakeTxGraph( + /*max_cluster_count=*/m_opts.limits.cluster_count, + /*max_cluster_size=*/m_opts.limits.cluster_size_vbytes * WITNESS_SCALE_FACTOR, + /*acceptable_iters=*/ACCEPTABLE_ITERS, + /*fallback_order=*/[&](const TxGraph::Ref& a, const TxGraph::Ref& b) noexcept { + const Txid& txid_a = static_cast(a).GetTx().GetHash(); + const Txid& txid_b = static_cast(b).GetTx().GetHash(); + return txid_a <=> txid_b; + }); } bool CTxMemPool::isSpent(const COutPoint& outpoint) const