From adc3491b1e011eca81f0d82687ddbd6f02fbbfbd Mon Sep 17 00:00:00 2001 From: frstrtr Date: Fri, 3 Jul 2026 22:49:56 +0000 Subject: [PATCH 1/2] dash(G1): deterministic equal-feerate tx selection for byte-parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Block-template tx ordering was the next non-deterministic seam after the curtime seam (#622): get_sorted_txs_with_fees streamed a std::multimap, so equal-feerate txs kept mempool INSERTION order — node-dependent, breaking G1 byte-parity of the embedded-GBT template projection against the p2pool-dash/dashcore oracle. Replace the index with a FeeKey set sorting feerate-desc then txid-asc, matching dashcore CompareTxMemPoolEntryByAncestorFee (GetHash tiebreak). Selection is now byte-reproducible regardless of arrival order. Fenced to src/impl/dash/coin/mempool.hpp + test. No shared/other-coin tree, no consensus-value change (same txs, deterministic order). KAT pins equal-feerate ties resolve txid-ascending, insertion-independent. --- src/impl/dash/coin/mempool.hpp | 34 ++++++++++++------- test/test_dash_mempool.cpp | 62 ++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/src/impl/dash/coin/mempool.hpp b/src/impl/dash/coin/mempool.hpp index ba124c87c..963b2c3b9 100644 --- a/src/impl/dash/coin/mempool.hpp +++ b/src/impl/dash/coin/mempool.hpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -72,6 +73,21 @@ struct MempoolEntry { } }; +/// Deterministic block-template selection key. Sorts highest-feerate +/// first; ties broken by txid ascending. This makes +/// get_sorted_txs_with_fees byte-reproducible across nodes/runs +/// (equal-feerate txs no longer depend on mempool insertion order) and +/// matches dashcore BlockAssembler ordering +/// (CompareTxMemPoolEntryByAncestorFee breaks feerate ties by GetHash()). +struct FeeKey { + double feerate; + uint256 txid; + bool operator<(const FeeKey& o) const { + if (feerate != o.feerate) return feerate > o.feerate; // higher feerate first + return txid < o.txid; // txid asc tiebreak (oracle-conformant) + } +}; + class Mempool { public: static constexpr size_t DEFAULT_MAX_BYTES = 300ULL * 1024 * 1024; @@ -139,7 +155,7 @@ class Mempool { // recompute_unknown_fees() resolves them after a UTXO update. // Uses negative feerate as the multimap key so begin() = best. if (stored.fee_known) { - m_feerate_index.emplace(stored.feerate_satvb(), txid); + m_feerate_index.insert(FeeKey{stored.feerate_satvb(), txid}); } // Periodic stats — every 100 entries + first 5 + every eviction. @@ -243,7 +259,7 @@ class Mempool { for (auto& [txid, entry] : m_pool) { if (entry.fee_known) continue; if (compute_fee_locked(entry, utxo)) { - m_feerate_index.emplace(entry.feerate_satvb(), txid); + m_feerate_index.insert(FeeKey{entry.feerate_satvb(), txid}); resolved_fees += entry.fee; ++resolved; } else { @@ -346,8 +362,8 @@ class Mempool { uint32_t total_bytes = 0; auto* utxo = m_utxo.load(); - for (const auto& [feerate, txid] : m_feerate_index) { - (void)feerate; + for (const auto& fk : m_feerate_index) { + const uint256& txid = fk.txid; auto pit = m_pool.find(txid); if (pit == m_pool.end()) continue; const auto& entry = pit->second; @@ -388,7 +404,7 @@ class Mempool { std::map, uint256> m_spent_outputs; // Step 2: feerate-sorted index. greater means begin() = // highest feerate, so iteration is best-first. - std::multimap> m_feerate_index; + std::set m_feerate_index; size_t m_total_bytes{0}; size_t m_max_bytes; time_t m_expiry_sec; @@ -415,13 +431,7 @@ class Mempool { // Drop from feerate index (only present if fee was known). if (it->second.fee_known) { - auto frng = m_feerate_index.equal_range(it->second.feerate_satvb()); - for (auto rit = frng.first; rit != frng.second; ++rit) { - if (rit->second == txid) { - m_feerate_index.erase(rit); - break; - } - } + m_feerate_index.erase(FeeKey{it->second.feerate_satvb(), txid}); } // Drop from spent-outputs index. diff --git a/test/test_dash_mempool.cpp b/test/test_dash_mempool.cpp index 615892703..f62f7c114 100644 --- a/test/test_dash_mempool.cpp +++ b/test/test_dash_mempool.cpp @@ -25,7 +25,9 @@ #include #include +#include #include +#include using dash::coin::Mempool; using dash::coin::MutableTransaction; @@ -254,3 +256,63 @@ TEST(DashMempool, SortedSelectionHighestFeerateFirst) EXPECT_EQ(dash_txid(selected[1].tx), dash_txid(tx_lo)); EXPECT_EQ(total_fees, 11'000u); } + +// ─── G1 byte-parity: equal-feerate selection is deterministic ──────────────── +// +// The embedded GBT template builder serializes txs in the order +// get_sorted_txs_with_fees() returns them. For txs sharing the SAME +// feerate the old std::multimap kept them in mempool +// INSERTION order, so two nodes with the same mempool contents but a +// different arrival order produced different template bytes — a +// non-deterministic seam that breaks G1 byte-parity against the +// p2pool-dash / dashcore oracle. FeeKey now breaks feerate ties by txid +// ascending (matches dashcore CompareTxMemPoolEntryByAncestorFee's +// GetHash() tiebreak), so the projection is byte-reproducible. +// +// This KAT pins that: identical equal-feerate sets added in OPPOSITE +// orders must yield the SAME selection, ordered by txid ascending. +static std::vector equal_feerate_selection(bool reverse_insertion) +{ + UTXOViewCache utxo(nullptr); + // 5 distinct prevouts, identical value ⇒ identical fee & base_size + // ⇒ identical feerate for every spend below. + constexpr int N = 5; + std::vector txs; + for (int i = 0; i < N; ++i) { + uint256 prev = mint_hash(200 + i); + utxo.add_coin(Outpoint(prev, 0), Coin(100'000, {}, 1, false)); + txs.push_back(make_spend(prev, 0, /*out=*/95'000, /*salt=*/300 + i)); // fee 5000 + } + + Mempool mp; + mp.set_utxo(&utxo); + if (reverse_insertion) + for (auto it = txs.rbegin(); it != txs.rend(); ++it) EXPECT_TRUE(mp.add_tx(*it)); + else + for (auto& t : txs) EXPECT_TRUE(mp.add_tx(t)); + + auto [selected, total_fees] = mp.get_sorted_txs_with_fees(/*max_bytes=*/1u << 20); + EXPECT_EQ(total_fees, static_cast(N) * 5'000u); + std::vector out; + for (auto& s : selected) out.push_back(dash_txid(s.tx)); + return out; +} + +TEST(DashMempool, EqualFeerateSelectionIsTxidAscendingAndInsertionOrderIndependent) +{ + auto forward = equal_feerate_selection(/*reverse_insertion=*/false); + auto reverse = equal_feerate_selection(/*reverse_insertion=*/true); + + ASSERT_EQ(forward.size(), 5u); + ASSERT_EQ(reverse.size(), 5u); + + // Insertion order must not affect the projected order. + EXPECT_EQ(forward, reverse) + << "equal-feerate tx order must be independent of mempool arrival order"; + + // And that stable order is txid ascending (dashcore GetHash() tiebreak). + auto sorted = forward; + std::sort(sorted.begin(), sorted.end()); + EXPECT_EQ(forward, sorted) + << "equal-feerate ties must resolve to txid-ascending, oracle-conformant order"; +} From 23dd87e0c7e04b445c4c9d081a73e870fa8ff0c3 Mon Sep 17 00:00:00 2001 From: frstrtr Date: Fri, 3 Jul 2026 23:22:57 +0000 Subject: [PATCH 2/2] dash(G1): feerate compare is division-free cross-multiply, not pre-divided double FeeKey keyed the sorted index on a pre-divided double (fee/base_size). That division rounds, so it can collapse a strict dashcore order into a tie -- or split a dashcore tie into a strict order -- diverging the selection order (and thus template byte-serialization) from the p2pool-dash/dashcore oracle. dashcore CompareTxMemPoolEntryByAncestorFee avoids division by cross-multiplying (fee, size) as doubles; FeeKey now carries (fee, base_size) and reproduces that exact comparison. Equal feerate still breaks by txid ascending (GetHash), as the oracle does. Adds an exhaustive-search divergence-vector KAT that pins the cross-multiply against an independent oracle reimplementation and the real std::set index. Fenced to src/impl/dash/coin/mempool.hpp + test/test_dash_mempool.cpp; no shared/other-coin/consensus reach. --- src/impl/dash/coin/mempool.hpp | 35 +++++++++----- test/test_dash_mempool.cpp | 85 ++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 12 deletions(-) diff --git a/src/impl/dash/coin/mempool.hpp b/src/impl/dash/coin/mempool.hpp index 963b2c3b9..fd5730bf8 100644 --- a/src/impl/dash/coin/mempool.hpp +++ b/src/impl/dash/coin/mempool.hpp @@ -74,17 +74,28 @@ struct MempoolEntry { }; /// Deterministic block-template selection key. Sorts highest-feerate -/// first; ties broken by txid ascending. This makes -/// get_sorted_txs_with_fees byte-reproducible across nodes/runs -/// (equal-feerate txs no longer depend on mempool insertion order) and -/// matches dashcore BlockAssembler ordering -/// (CompareTxMemPoolEntryByAncestorFee breaks feerate ties by GetHash()). +/// first; ties broken by txid ascending. Byte-reproducible across +/// nodes/runs AND bit-for-bit conformant with dashcore BlockAssembler +/// ordering. dashcore CompareTxMemPoolEntryByAncestorFee compares two +/// entries by cross-multiplication of (fee, size) as doubles -- +/// f1 = a.fee * b.size vs f2 = b.fee * a.size -- explicitly "avoid +/// division by rewriting (a/b > c/d) as (a*d > c*b)". A pre-divided +/// fee/base_size double rounds where the cross-multiply does not, so the +/// two representations can order a tie-pair differently. We therefore +/// carry (fee, base_size) and reproduce the exact double cross-multiply; +/// equal feerate is broken by GetHash()/txid ascending, as the oracle +/// does. (No ancestor packages in this simplified mempool, so +/// GetModFeeAndSize reduces to the entry's own (fee, base_size).) struct FeeKey { - double feerate; - uint256 txid; + uint64_t fee; // satoshi + uint32_t base_size; // serialized bytes (>0 for every indexed entry) + uint256 txid; bool operator<(const FeeKey& o) const { - if (feerate != o.feerate) return feerate > o.feerate; // higher feerate first - return txid < o.txid; // txid asc tiebreak (oracle-conformant) + // dashcore CompareTxMemPoolEntryByAncestorFee, division-free form. + const double f1 = static_cast(fee) * o.base_size; + const double f2 = static_cast(o.fee) * base_size; + if (f1 != f2) return f1 > f2; // higher feerate first + return txid < o.txid; // txid asc tiebreak (oracle-conformant) } }; @@ -155,7 +166,7 @@ class Mempool { // recompute_unknown_fees() resolves them after a UTXO update. // Uses negative feerate as the multimap key so begin() = best. if (stored.fee_known) { - m_feerate_index.insert(FeeKey{stored.feerate_satvb(), txid}); + m_feerate_index.insert(FeeKey{stored.fee, stored.base_size, txid}); } // Periodic stats — every 100 entries + first 5 + every eviction. @@ -259,7 +270,7 @@ class Mempool { for (auto& [txid, entry] : m_pool) { if (entry.fee_known) continue; if (compute_fee_locked(entry, utxo)) { - m_feerate_index.insert(FeeKey{entry.feerate_satvb(), txid}); + m_feerate_index.insert(FeeKey{entry.fee, entry.base_size, txid}); resolved_fees += entry.fee; ++resolved; } else { @@ -431,7 +442,7 @@ class Mempool { // Drop from feerate index (only present if fee was known). if (it->second.fee_known) { - m_feerate_index.erase(FeeKey{it->second.feerate_satvb(), txid}); + m_feerate_index.erase(FeeKey{it->second.fee, it->second.base_size, txid}); } // Drop from spent-outputs index. diff --git a/test/test_dash_mempool.cpp b/test/test_dash_mempool.cpp index f62f7c114..d2885e9e7 100644 --- a/test/test_dash_mempool.cpp +++ b/test/test_dash_mempool.cpp @@ -27,12 +27,14 @@ #include #include +#include #include using dash::coin::Mempool; using dash::coin::MutableTransaction; using dash::coin::dash_txid; using dash::coin::BlockType; +using dash::coin::FeeKey; using ::core::coin::UTXOViewCache; using ::core::coin::Outpoint; using ::core::coin::Coin; @@ -316,3 +318,86 @@ TEST(DashMempool, EqualFeerateSelectionIsTxidAscendingAndInsertionOrderIndepende EXPECT_EQ(forward, sorted) << "equal-feerate ties must resolve to txid-ascending, oracle-conformant order"; } + + +// --- G1 byte-parity: feerate compare is dashcore division-free cross-multiply +// +// dashcore CompareTxMemPoolEntryByAncestorFee compares two entries by +// cross-multiplication -- f1 = a.fee * b.size vs f2 = b.fee * a.size -- +// explicitly to "avoid division by rewriting (a/b > c/d) as (a*d > c*b)". +// c2pool previously keyed the sorted index on a PRE-DIVIDED double +// (fee / base_size). That division rounds, so it can collapse a strict +// dashcore order into a tie -- or split a dashcore tie into a strict +// order -- making the two representations disagree on the selection +// order of certain (fee, size) pairs. A different selection order is a +// different template byte-serialization: a latent G1 byte-parity seam +// against the p2pool-dash / dashcore oracle. FeeKey now carries +// (fee, base_size) and reproduces the exact double cross-multiply. +// +// Divergence vector (found by exhaustive search). The disagreement only +// manifests at fee magnitudes >~1e14 sat, where the fee/size division +// loses ULPs the cross-multiply keeps -- for realistic magnitudes the +// two representations agree, so this fix is exact-oracle-conformance +// hardening, not a realistic-value bug: +// A = (fee 182912374030878, size 3535) +// B = (fee 4415613369921651, size 85337) +// Pre-divided doubles: A -> 51743245836.174819946 < B -> 51743245836.174827576, +// i.e. the OLD code ranks B strictly ABOVE A. +// Cross-multiply: A.fee*B.size == B.fee*A.size exactly -> a genuine +// feerate TIE, resolved by txid ascending (dashcore GetHash()). +TEST(DashMempool, FeerateCompareIsDivisionFreeCrossMultiplyNotPreDividedDouble) +{ + // Independent dashcore-style reference (division-free cross-multiply). + auto oracle_less = [](uint64_t fa, uint32_t sa, const uint256& ta, + uint64_t fb, uint32_t sb, const uint256& tb) { + const double f1 = static_cast(fa) * sb; + const double f2 = static_cast(fb) * sa; + if (f1 != f2) return f1 > f2; // higher feerate first + return ta < tb; // txid ascending + }; + + // Two distinct txids; assign the SMALLER to A so a correct + // (cross-multiply => tie => txid-asc) key orders A before B, whereas + // the old pre-divide code would have put B first ("B higher feerate"). + const uint256 t0 = mint_hash(9001); + const uint256 t1 = mint_hash(9002); + const uint256 ta = std::min(t0, t1); + const uint256 tb = std::max(t0, t1); + ASSERT_TRUE(ta < tb); + + const uint64_t fa = 182912374030878ULL; + const uint32_t sa = 3535u; + const uint64_t fb = 4415613369921651ULL; + const uint32_t sb = 85337u; + + // Precondition on the vector: a genuine cross-multiply tie that the + // buggy pre-divide would (wrongly) have seen as a strict order. + EXPECT_EQ(static_cast(fa) * sb, static_cast(fb) * sa) + << "vector must be a genuine cross-multiply feerate tie"; + EXPECT_LT(static_cast(fa) / sa, static_cast(fb) / sb) + << "vector must be a strict (wrong) order under the old pre-divide"; + + const FeeKey A{fa, sa, ta}; + const FeeKey B{fb, sb, tb}; + + // FeeKey resolves the cross-multiply tie by txid ascending => A first. + EXPECT_TRUE(A < B) << "cross-multiply tie must fall to txid-ascending (A) must iterate A best-first. + std::set idx{B, A}; + ASSERT_EQ(idx.size(), 2u); + EXPECT_EQ(idx.begin()->txid, ta) + << "best-first iteration must yield A (smaller txid) on the tie"; + + // Sanity: the feerate arm still dominates. Strictly higher feerate + // must precede regardless of a smaller-txid competitor. + const FeeKey hi{20000u, 250u, tb}; // 80 sat/byte + const FeeKey lo{10000u, 250u, ta}; // 40 sat/byte, but smaller txid + EXPECT_TRUE(hi < lo) << "higher feerate must precede regardless of txid"; + EXPECT_FALSE(lo < hi); +}