txgraph: pass fallback_order to TxGraph (preparation)
What changed, and why it matters
This commit is a pure internal refactoring in Bitcoin Core. It adds a new optional tie-breaker function (called fallback_order) to the transaction graph (TxGraph) code, but explicitly does not use it yet. It is preparation for future work and does not change any network behavior, consensus rules, or security properties.
No security action needed. Treat as normal code-review item; verify the fallback comparator is stable and deterministic as future commits begin to use it.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change modifies MakeTxGraph to accept an std::function
Changed components
src/txgraph.hsrc/txgraph.cppsrc/txmempool.cppsrc/bench/txgraph.cppsrc/test/fuzz/txgraph.cppsrc/test/txgraph_tests.cppsrc/primitives/transaction_identifier.hInspect captured patch +100 / −23
diff --git a/src/bench/txgraph.cpp b/src/bench/txgraph.cpp
index 7257a043..00074db6 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<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) {
diff --git a/src/primitives/transaction_identifier.h b/src/primitives/transaction_identifier.h
index 9b518d7d..a1109591 100644
--- a/src/primitives/transaction_identifier.h
+++ b/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}; }
diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp
index 51546f61..5352e2c1 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<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);
diff --git a/src/test/txgraph_tests.cpp b/src/test/txgraph_tests.cpp
index 31fefd9c..f069e5bd 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<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);
diff --git a/src/txgraph.cpp b/src/txgraph.cpp
index 492356a8..2c024c96 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<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. */
- 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<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);
}
diff --git a/src/txgraph.h b/src/txgraph.h
index 385db014..8c7a9174 100644
--- a/src/txgraph.h
+++ b/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
- * 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<TxGraph> 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<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
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 58262e7f..b8be3e08 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<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
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.