txgraph: pass fallback_order to TxGraph (preparation)

This adds an std::function<strong_ordering(Ref&,Ref&)> argument to the
MakeTxGraph function, which can be used by the caller (e.g., mempool
code) to provide a fallback order to TxGraph.

This is just preparation; TxGraph does not yet use this fallback order
for anything.

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<size_t> 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) {

src/primitives/transaction_identifier.h

@@ -9,6 +9,7 @@
 #include <uint256.h>
 #include <util/types.h>
 
+#include <compare>
 #include <cstddef>
 #include <optional>
 #include <string>
@@ -42,7 +43,7 @@ public:
     template <typename Other>
     bool operator==(const Other& other) const { return Compare(other) == 0; }
     template <typename Other>
-    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}; }

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<SimTxObject>();
+        simmap[simpos] = std::make_shared<SimTxObject>(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<uint64_t>(0, 10000);
 
+    /** The set of uint64_t "txid"s that have been assigned before. */
+    std::set<uint64_t> 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<const SimTxObject&>(a).m_txid;
+        uint64_t txid_b = static_cast<const SimTxObject&>(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<SimTxGraph> sims;
     sims.reserve(2);
     sims.emplace_back(max_cluster_count, max_cluster_size);
@@ -460,8 +479,14 @@ FUZZ_TARGET(txgraph)
                     size = provider.ConsumeIntegralInRange<uint32_t>(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);

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<TxGraph::Ref> 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<TxGraph::Ref> refs;
@@ -188,7 +197,7 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_huge)
     std::vector<size_t> 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<TxGraph::Ref> 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<std::vector<TxGraph::Ref*>> expected_chunks) {
         std::vector<std::vector<TxGraph::Ref*>> 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<TxGraph::Ref> refs;
     refs.reserve(2);

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<std::strong_ordering(const TxGraph::Ref&, const TxGraph::Ref&)> m_fallback_order;
 
     /** Information about one group of Clusters to be merged. */
     struct GroupEntry
@@ -621,11 +623,17 @@ private:
     std::vector<GraphIndex> m_unlinked;
 
 public:
-    /** Construct a new TxGraphImpl with the specified limits. */
+    /** 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) noexcept :
+    explicit TxGraphImpl(
+        DepGraphIndex max_cluster_count,
+        uint64_t max_cluster_size,
+        uint64_t acceptable_iters,
+        const std::function<std::strong_ordering(const TxGraph::Ref&, const TxGraph::Ref&)>& 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<TxGraph> MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size, uint64_t acceptable_iters) noexcept
+std::unique_ptr<TxGraph> MakeTxGraph(
+    unsigned max_cluster_count,
+    uint64_t max_cluster_size,
+    uint64_t acceptable_iters,
+    const std::function<std::strong_ordering(const TxGraph::Ref&, const TxGraph::Ref&)>& fallback_order) noexcept
 {
-    return std::make_unique<TxGraphImpl>(max_cluster_count, max_cluster_size, acceptable_iters);
+    return std::make_unique<TxGraphImpl>(max_cluster_count, max_cluster_size, acceptable_iters, fallback_order);
 }

src/txgraph.h

@@ -4,6 +4,7 @@
 
 #include <compare>
 #include <cstdint>
+#include <functional>
 #include <memory>
 #include <optional>
 #include <utility>
@@ -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
+ *  and on the sum of transaction sizes within a cluster.
- *  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. */
+ * - max_cluster_count cannot exceed MAX_CLUSTER_COUNT_LIMIT.
-std::unique_ptr<TxGraph> MakeTxGraph(unsigned max_cluster_count, uint64_t max_cluster_size, uint64_t acceptable_iters) noexcept;
+ * - 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<TxGraph> MakeTxGraph(
+    unsigned max_cluster_count,
+    uint64_t max_cluster_size,
+    uint64_t acceptable_iters,
+    const std::function<std::strong_ordering(const TxGraph::Ref&, const TxGraph::Ref&)>& fallback_order
+) noexcept;
 
 #endif // BITCOIN_TXGRAPH_H

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<const CTxMemPoolEntry&>(a).GetTx().GetHash();
+            const Txid& txid_b = static_cast<const CTxMemPoolEntry&>(b).GetTx().GetHash();
+            return txid_a <=> txid_b;
+        });
 }
 
 bool CTxMemPool::isSpent(const COutPoint& outpoint) const