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<FeeKey> index. Fenced to src/impl/dash/coin/mempool.hpp +
test/test_dash_mempool.cpp; no shared/other-coin/consensus reach.

Author: frstrtr

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<double>(fee)   * o.base_size;
+        const double f2 = static_cast<double>(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.

test/test_dash_mempool.cpp

@@ -27,12 +27,14 @@
 
 #include <algorithm>
 #include <cstdint>
+#include <set>
 #include <vector>
 
 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<double>(fa) * sb;
+        const double f2 = static_cast<double>(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<double>(fa) * sb, static_cast<double>(fb) * sa)
+        << "vector must be a genuine cross-multiply feerate tie";
+    EXPECT_LT(static_cast<double>(fa) / sa, static_cast<double>(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<B)";
+    EXPECT_FALSE(B < A);
+
+    // FeeKey ordering must agree with the independent oracle on this pair.
+    EXPECT_EQ(A < B, oracle_less(fa, sa, ta, fb, sb, tb));
+    EXPECT_EQ(B < A, oracle_less(fb, sb, tb, fa, sa, ta));
+
+    // The real index type (std::set<FeeKey>) must iterate A best-first.
+    std::set<FeeKey> 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);
+}