Skip to content

Commit 295a1ca

Browse files
committed
txgraph: Expose ability to compare transactions (feature)
In order to make it possible for higher layers to compare transaction quality (ordering within the implicit total ordering on the mempool), expose a comparison function and test it.
1 parent 22c68cd commit 295a1ca

File tree

3 files changed

+112
-4
lines changed

3 files changed

+112
-4
lines changed

src/test/fuzz/txgraph.cpp

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,13 +508,79 @@ FUZZ_TARGET(txgraph)
508508
// cause it to be re-evaluated in TxGraph).
509509
sims.back().oversized = std::nullopt;
510510
break;
511+
} else if (!main_sim.IsOversized() && command-- == 0) {
512+
// CompareMainOrder.
513+
auto ref_a = pick_fn();
514+
auto ref_b = pick_fn();
515+
auto sim_a = main_sim.Find(ref_a);
516+
auto sim_b = main_sim.Find(ref_b);
517+
// Both transactions must exist in the main graph.
518+
if (sim_a == SimTxGraph::MISSING || sim_b == SimTxGraph::MISSING) break;
519+
auto cmp = real->CompareMainOrder(*ref_a, *ref_b);
520+
// Distinct transactions have distinct places.
521+
if (sim_a != sim_b) assert(cmp != 0);
522+
// Ancestors go before descendants.
523+
if (main_sim.graph.Ancestors(sim_a)[sim_b]) assert(cmp >= 0);
524+
if (main_sim.graph.Descendants(sim_a)[sim_b]) assert(cmp <= 0);
525+
// Do not verify consistency with chunk feerates, as we cannot easily determine
526+
// these here without making more calls to real, which could affect its internal
527+
// state. A full comparison is done at the end.
528+
break;
511529
}
512530
}
513531
}
514532

515533
// After running all modifications, perform an internal sanity check (before invoking
516534
// inspectors that may modify the internal state).
517535
real->SanityCheck();
536+
537+
if (!sims[0].IsOversized()) {
538+
// If the main graph is not oversized, verify the total ordering implied by
539+
// CompareMainOrder.
540+
// First construct two distinct randomized permutations of the positions in sims[0].
541+
std::vector<SimTxGraph::Pos> vec1;
542+
for (auto i : sims[0].graph.Positions()) vec1.push_back(i);
543+
std::shuffle(vec1.begin(), vec1.end(), rng);
544+
auto vec2 = vec1;
545+
std::shuffle(vec2.begin(), vec2.end(), rng);
546+
if (vec1 == vec2) std::next_permutation(vec2.begin(), vec2.end());
547+
// Sort both according to CompareMainOrder. By having randomized starting points, the order
548+
// of CompareMainOrder invocations is somewhat randomized as well.
549+
auto cmp = [&](SimTxGraph::Pos a, SimTxGraph::Pos b) noexcept {
550+
return real->CompareMainOrder(*sims[0].GetRef(a), *sims[0].GetRef(b)) < 0;
551+
};
552+
std::sort(vec1.begin(), vec1.end(), cmp);
553+
std::sort(vec2.begin(), vec2.end(), cmp);
554+
555+
// Verify the resulting orderings are identical. This could only fail if the ordering was
556+
// not total.
557+
assert(vec1 == vec2);
558+
559+
// Verify that the ordering is topological.
560+
auto todo = sims[0].graph.Positions();
561+
for (auto i : vec1) {
562+
todo.Reset(i);
563+
assert(!sims[0].graph.Ancestors(i).Overlaps(todo));
564+
}
565+
assert(todo.None());
566+
567+
// For every transaction in the total ordering, find a random one before it and after it,
568+
// and compare their chunk feerates, which must be consistent with the ordering.
569+
for (size_t pos = 0; pos < vec1.size(); ++pos) {
570+
auto pos_feerate = real->GetMainChunkFeerate(*sims[0].GetRef(vec1[pos]));
571+
if (pos > 0) {
572+
size_t before = rng.randrange<size_t>(pos);
573+
auto before_feerate = real->GetMainChunkFeerate(*sims[0].GetRef(vec1[before]));
574+
assert(FeeRateCompare(before_feerate, pos_feerate) >= 0);
575+
}
576+
if (pos + 1 < vec1.size()) {
577+
size_t after = pos + 1 + rng.randrange<size_t>(vec1.size() - 1 - pos);
578+
auto after_feerate = real->GetMainChunkFeerate(*sims[0].GetRef(vec1[after]));
579+
assert(FeeRateCompare(after_feerate, pos_feerate) <= 0);
580+
}
581+
}
582+
}
583+
518584
assert(real->HaveStaging() == (sims.size() > 1));
519585

520586
// Try to run a full comparison, for both main_only=false and main_only=true in TxGraph

src/txgraph.cpp

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,8 @@ class TxGraphImpl final : public TxGraph
303303
Locator m_locator[MAX_LEVELS];
304304
/** The chunk feerate of this transaction in main (if present in m_locator[0]). */
305305
FeePerWeight m_main_chunk_feerate;
306+
/** The position this transaction has in the main linearization (if present). */
307+
LinearizationIndex m_main_lin_index;
306308
};
307309

308310
/** The set of all transactions (in all levels combined). GraphIndex values index into this. */
@@ -447,6 +449,7 @@ class TxGraphImpl final : public TxGraph
447449
std::vector<Ref*> GetDescendants(const Ref& arg, bool main_only = false) noexcept final;
448450
GraphIndex GetTransactionCount(bool main_only = false) noexcept final;
449451
bool IsOversized(bool main_only = false) noexcept final;
452+
std::strong_ordering CompareMainOrder(const Ref& a, const Ref& b) noexcept final;
450453

451454
void SanityCheck() const final;
452455
};
@@ -499,9 +502,10 @@ void Cluster::Updated(TxGraphImpl& graph) noexcept
499502
entry.m_locator[m_level].SetPresent(this, idx);
500503
}
501504
// If this is for the main graph (level = 0), and the Cluster's quality is ACCEPTABLE or
502-
// OPTIMAL, compute its chunking and store its information in the Entry's m_main_chunk_feerate.
503-
// These fields are only accessed after making the entire graph ACCEPTABLE, so it is pointless
504-
// to compute these if we haven't reached that quality level yet.
505+
// OPTIMAL, compute its chunking and store its information in the Entry's m_main_lin_index
506+
// and m_main_chunk_feerate. These fields are only accessed after making the entire graph
507+
// ACCEPTABLE, so it is pointless to compute these if we haven't reached that quality level
508+
// yet.
505509
if (m_level == 0 && IsAcceptable()) {
506510
LinearizationChunking chunking(m_depgraph, m_linearization);
507511
LinearizationIndex lin_idx{0};
@@ -511,9 +515,10 @@ void Cluster::Updated(TxGraphImpl& graph) noexcept
511515
Assume(chunk.transactions.Any());
512516
// Iterate over the transactions in the linearization, which must match those in chunk.
513517
do {
514-
DepGraphIndex idx = m_linearization[lin_idx++];
518+
DepGraphIndex idx = m_linearization[lin_idx];
515519
GraphIndex graph_idx = m_mapping[idx];
516520
auto& entry = graph.m_entries[graph_idx];
521+
entry.m_main_lin_index = lin_idx++;
517522
entry.m_main_chunk_feerate = FeePerWeight::FromFeeFrac(chunk.feerate);
518523
Assume(chunk.transactions[idx]);
519524
chunk.transactions.Reset(idx);
@@ -594,6 +599,10 @@ void Cluster::ApplyRemovals(TxGraphImpl& graph, std::span<GraphIndex>& to_remove
594599
// are just never accessed, but set it to -1 here to increase the ability to detect a bug
595600
// that causes it to be accessed regardless.
596601
m_mapping[locator.index] = GraphIndex(-1);
602+
// - Remove its linearization index from the Entry (if in main).
603+
if (m_level == 0) {
604+
entry.m_main_lin_index = LinearizationIndex(-1);
605+
}
597606
// - Mark it as missing/removed in the Entry's locator.
598607
graph.ClearLocator(m_level, idx);
599608
to_remove = to_remove.subspan(1);
@@ -1730,6 +1739,33 @@ void TxGraphImpl::SetTransactionFee(const Ref& ref, int64_t fee) noexcept
17301739
}
17311740
}
17321741

1742+
std::strong_ordering TxGraphImpl::CompareMainOrder(const Ref& a, const Ref& b) noexcept
1743+
{
1744+
// The references must not be empty.
1745+
Assume(GetRefGraph(a) == this);
1746+
Assume(GetRefGraph(b) == this);
1747+
// Apply dependencies in main.
1748+
ApplyDependencies(0);
1749+
Assume(m_main_clusterset.m_deps_to_add.empty());
1750+
// Make both involved Clusters acceptable, so chunk feerates are relevant.
1751+
const auto& entry_a = m_entries[GetRefIndex(a)];
1752+
const auto& entry_b = m_entries[GetRefIndex(b)];
1753+
const auto& locator_a = entry_a.m_locator[0];
1754+
const auto& locator_b = entry_b.m_locator[0];
1755+
Assume(locator_a.IsPresent());
1756+
Assume(locator_b.IsPresent());
1757+
MakeAcceptable(*locator_a.cluster);
1758+
MakeAcceptable(*locator_b.cluster);
1759+
// Compare chunk feerates, and return result if it differs.
1760+
auto feerate_cmp = FeeRateCompare(entry_b.m_main_chunk_feerate, entry_a.m_main_chunk_feerate);
1761+
if (feerate_cmp < 0) return std::strong_ordering::less;
1762+
if (feerate_cmp > 0) return std::strong_ordering::greater;
1763+
// Compare Cluster* as tie-break for equal chunk feerates.
1764+
if (locator_a.cluster != locator_b.cluster) return locator_a.cluster <=> locator_b.cluster;
1765+
// As final tie-break, compare position within cluster linearization.
1766+
return entry_a.m_main_lin_index <=> entry_b.m_main_lin_index;
1767+
}
1768+
17331769
void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const
17341770
{
17351771
// There must be an m_mapping for each m_depgraph position (including holes).
@@ -1747,6 +1783,7 @@ void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const
17471783

17481784
// Verify m_linearization.
17491785
SetType m_done;
1786+
LinearizationIndex linindex{0};
17501787
assert(m_depgraph.IsAcyclic());
17511788
for (auto lin_pos : m_linearization) {
17521789
assert(lin_pos < m_mapping.size());
@@ -1759,6 +1796,8 @@ void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const
17591796
assert(entry.m_locator[level].index == lin_pos);
17601797
// For main-level entries, check linearization position and chunk feerate.
17611798
if (level == 0 && IsAcceptable()) {
1799+
assert(entry.m_main_lin_index == linindex);
1800+
++linindex;
17621801
if (!linchunking.GetChunk(0).transactions[lin_pos]) {
17631802
linchunking.MarkDone(linchunking.GetChunk(0).transactions);
17641803
}

src/txgraph.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ class TxGraph
141141
* graph exists, it is queried; otherwise the main graph is queried. This is available even
142142
* for oversized graphs. */
143143
virtual GraphIndex GetTransactionCount(bool main_only = false) noexcept = 0;
144+
/** Compare two transactions according to their order in the main graph. Both transactions must
145+
* be in the main graph. The main graph must not be oversized. */
146+
virtual std::strong_ordering CompareMainOrder(const Ref& a, const Ref& b) noexcept = 0;
144147

145148
/** Perform an internal consistency check on this object. */
146149
virtual void SanityCheck() const = 0;

0 commit comments

Comments
 (0)