From 56684bc3f6f1adc5e70042d7792cb5e9d65bace0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 14 Nov 2024 16:16:59 -0500 Subject: [PATCH] txgraph: (feature) expose ability to compare transactions 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. --- src/test/fuzz/txgraph.cpp | 66 +++++++++++++++++++++++++++++++++++++++ src/txgraph.cpp | 48 +++++++++++++++++++++++++--- src/txgraph.h | 4 +++ 3 files changed, 114 insertions(+), 4 deletions(-) diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index e537f546120..45d7f7d02fe 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -503,6 +503,24 @@ FUZZ_TARGET(txgraph) sims.erase(sims.begin()); } break; + } else if (main_sim.GetTransactionCount() > 0 && !main_sim.IsOversized() && command-- == 0) { + // CompareMainOrder. + auto& ref_a = pick_fn(); + auto& ref_b = pick_fn(); + auto sim_a = main_sim.Find(ref_a); + auto sim_b = main_sim.Find(ref_b); + // Both transactions must exist in the main graph. + if (sim_a == SimTxGraph::MISSING || sim_b == SimTxGraph::MISSING) break; + auto cmp = real->CompareMainOrder(ref_a, ref_b); + // Distinct transactions have distinct places. + if (sim_a != sim_b) assert(cmp != 0); + // Ancestors go before descendants. + if (main_sim.graph.Ancestors(sim_a)[sim_b]) assert(cmp >= 0); + if (main_sim.graph.Descendants(sim_a)[sim_b]) assert(cmp <= 0); + // Do not verify consistency with chunk feerates, as we cannot easily determine + // these here without making more calls to real, which could affect its internal + // state. A full comparison is done at the end. + break; } } } @@ -510,6 +528,54 @@ FUZZ_TARGET(txgraph) // After running all modifications, perform an internal sanity check (before invoking // inspectors that may modify the internal state). real->SanityCheck(); + + if (!sims[0].IsOversized()) { + // If the main graph is not oversized, verify the total ordering implied by + // CompareMainOrder. + // First construct two distinct randomized permutations of the positions in sims[0]. + std::vector vec1; + for (auto i : sims[0].graph.Positions()) vec1.push_back(i); + std::shuffle(vec1.begin(), vec1.end(), rng); + auto vec2 = vec1; + std::shuffle(vec2.begin(), vec2.end(), rng); + if (vec1 == vec2) std::next_permutation(vec2.begin(), vec2.end()); + // Sort both according to CompareMainOrder. By having randomized starting points, the order + // of CompareMainOrder invocations is somewhat randomized as well. + auto cmp = [&](SimTxGraph::Pos a, SimTxGraph::Pos b) noexcept { + return real->CompareMainOrder(sims[0].GetRef(a), sims[0].GetRef(b)) < 0; + }; + std::sort(vec1.begin(), vec1.end(), cmp); + std::sort(vec2.begin(), vec2.end(), cmp); + + // Verify the resulting orderings are identical. This could only fail if the ordering was + // not total. + assert(vec1 == vec2); + + // Verify that the ordering is topological. + auto todo = sims[0].graph.Positions(); + for (auto i : vec1) { + todo.Reset(i); + assert(!sims[0].graph.Ancestors(i).Overlaps(todo)); + } + assert(todo.None()); + + // For every transaction in the total ordering, find a random one before it and after it, + // and compare their chunk feerates, which must be consistent with the ordering. + for (size_t pos = 0; pos < vec1.size(); ++pos) { + auto pos_feerate = real->GetMainChunkFeerate(sims[0].GetRef(vec1[pos])); + if (pos > 0) { + size_t before = rng.randrange(pos); + auto before_feerate = real->GetMainChunkFeerate(sims[0].GetRef(vec1[before])); + assert(FeeRateCompare(before_feerate, pos_feerate) >= 0); + } + if (pos + 1 < vec1.size()) { + size_t after = pos + 1 + rng.randrange(vec1.size() - 1 - pos); + auto after_feerate = real->GetMainChunkFeerate(sims[0].GetRef(vec1[after])); + assert(FeeRateCompare(after_feerate, pos_feerate) <= 0); + } + } + } + assert(real->HaveStaging() == (sims.size() > 1)); // Try to run a full comparison, for both main_only=false and main_only=true in TxGraph diff --git a/src/txgraph.cpp b/src/txgraph.cpp index 4d7eb4ac1a0..d85d5b9ab90 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -242,6 +242,8 @@ private: Locator m_locator[MAX_LEVELS]; /** The chunk feerate of this transaction in main (if present in m_locator[0]) */ FeeFrac m_main_chunk_feerate; + /** The position this transaction in the main linearization (if present). /*/ + LinearizationIndex m_main_lin_index; /** Check whether this Entry is not present in any Cluster. */ bool IsWiped() const noexcept @@ -358,6 +360,7 @@ public: std::vector GetDescendants(const Ref& arg, bool main_only = false) noexcept final; GraphIndex GetTransactionCount(bool main_only = false) noexcept final; bool IsOversized(bool main_only = false) noexcept final; + std::strong_ordering CompareMainOrder(const Ref& a, const Ref& b) noexcept final; void SanityCheck() const final; }; @@ -399,9 +402,10 @@ void Cluster::Updated(TxGraphImpl& graph) noexcept entry.m_locator[m_level].SetPresent(this, idx); } // If this is for the main graph (level = 0), and the Cluster's quality is ACCEPTABLE or - // OPTIMAL, compute its chunking and store its information in the Entry's m_main_chunk_feerate. - // These fields are only accessed after making the entire graph ACCEPTABLE, so it is pointless - // to compute these if we haven't reached that quality level yet. + // OPTIMAL, compute its chunking and store its information in the Entry's m_main_lin_index + // and m_main_chunk_feerate. These fields are only accessed after making the entire graph + // ACCEPTABLE, so it is pointless to compute these if we haven't reached that quality level + // yet. if (m_level == 0 && (m_quality == QualityLevel::OPTIMAL || m_quality == QualityLevel::ACCEPTABLE)) { LinearizationChunking chunking(m_depgraph, m_linearization); LinearizationIndex lin_idx{0}; @@ -410,9 +414,10 @@ void Cluster::Updated(TxGraphImpl& graph) noexcept auto chunk = chunking.GetChunk(chunk_idx); // Iterate over the transactions in the linearization, which must match those in chunk. while (true) { - ClusterIndex idx = m_linearization[lin_idx++]; + ClusterIndex idx = m_linearization[lin_idx]; GraphIndex graph_idx = m_mapping[idx]; auto& entry = graph.m_entries[graph_idx]; + entry.m_main_lin_index = lin_idx++; entry.m_main_chunk_feerate = chunk.feerate; chunk.transactions.Reset(idx); if (chunk.transactions.None()) break; @@ -487,6 +492,10 @@ void Cluster::ApplyRemovals(TxGraphImpl& graph, std::span& to_remove todo.Set(locator.index); // - Remove from m_mapping. m_mapping[locator.index] = GraphIndex(-1); + // - Remove its linearization index from the Entry (if in main). + if (m_level == 0) { + entry.m_main_lin_index = LinearizationIndex(-1); + } // - Mark it as missing/removed in the Entry's locator. graph.ClearLocator(m_level, idx); to_remove = to_remove.subspan(1); @@ -1611,6 +1620,35 @@ void TxGraphImpl::SetTransactionFee(Ref& ref, int64_t fee) noexcept } } +std::strong_ordering TxGraphImpl::CompareMainOrder(const Ref& a, const Ref& b) noexcept +{ + // The references must not be empty. + Assume(GetRefGraph(a) == this); + Assume(GetRefGraph(b) == this); + // Apply dependencies if main is the only level (in every other case, they will have been + // applied already prior to the creating of staging, or main is oversized). + SplitAll(0); + if (m_clustersets.size() == 1) ApplyDependencies(); + Assume(!m_clustersets[0].m_oversized); + // Make both involved Clusters acceptable, so chunk feerates are relevant. + const auto& entry_a = m_entries[GetRefIndex(a)]; + const auto& entry_b = m_entries[GetRefIndex(b)]; + const auto& locator_a = entry_a.m_locator[0]; + const auto& locator_b = entry_b.m_locator[0]; + Assume(locator_a.IsPresent()); + Assume(locator_b.IsPresent()); + MakeAcceptable(*locator_a.cluster); + MakeAcceptable(*locator_b.cluster); + // Compare chunk feerates, and return result if it differs. + auto feerate_cmp = FeeRateCompare(entry_b.m_main_chunk_feerate, entry_a.m_main_chunk_feerate); + if (feerate_cmp < 0) return std::strong_ordering::less; + if (feerate_cmp > 0) return std::strong_ordering::greater; + // Compare Cluster* as tie-break for equal chunk feerates. + if (locator_a.cluster != locator_b.cluster) return locator_a.cluster <=> locator_b.cluster; + // As final tie-break, compare position within cluster linearization. + return entry_a.m_main_lin_index <=> entry_b.m_main_lin_index; +} + void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const { // There must be an m_mapping for each m_depgraph position (including holes). @@ -1628,6 +1666,7 @@ void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const // Verify m_linearization. SetType m_done; + LinearizationIndex linindex{0}; assert(m_depgraph.IsAcyclic()); for (auto lin_pos : m_linearization) { assert(lin_pos < m_mapping.size()); @@ -1640,6 +1679,7 @@ void Cluster::SanityCheck(const TxGraphImpl& graph, int level) const assert(entry.m_locator[level].index == lin_pos); // For top-level entries, check linearization position and chunk feerate. if (level == 0 && (m_quality == QualityLevel::OPTIMAL || m_quality == QualityLevel::ACCEPTABLE)) { + assert(entry.m_main_lin_index == linindex++); if (!linchunking.GetChunk(0).transactions[lin_pos]) { linchunking.MarkDone(linchunking.GetChunk(0).transactions); } diff --git a/src/txgraph.h b/src/txgraph.h index 4042da0a8e6..ab0815a2aca 100644 --- a/src/txgraph.h +++ b/src/txgraph.h @@ -150,6 +150,10 @@ public: * graph exists, it is queried; otherwise the main graph is queried. This is available even * for oversized graphs. */ virtual GraphIndex GetTransactionCount(bool main_only = false) noexcept = 0; + /** Compare two transactions according to the total order in the main graph (topological, and + * from high to low chunk feerate). Both transactions must be in the main graph. The main + * graph must not be oversized. */ + virtual std::strong_ordering CompareMainOrder(const Ref& a, const Ref& b) noexcept = 0; /** Perform an internal consistency check on this object. */ virtual void SanityCheck() const = 0;