Skip to content

Commit adc3491

Browse files
committed
dash(G1): deterministic equal-feerate tx selection for byte-parity
Block-template tx ordering was the next non-deterministic seam after the curtime seam (#622): get_sorted_txs_with_fees streamed a std::multimap<double,uint256>, 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.
1 parent 083071d commit adc3491

2 files changed

Lines changed: 84 additions & 12 deletions

File tree

src/impl/dash/coin/mempool.hpp

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include <cstdint>
5050
#include <ctime>
5151
#include <map>
52+
#include <set>
5253
#include <mutex>
5354
#include <optional>
5455
#include <utility>
@@ -72,6 +73,21 @@ struct MempoolEntry {
7273
}
7374
};
7475

76+
/// Deterministic block-template selection key. Sorts highest-feerate
77+
/// first; ties broken by txid ascending. This makes
78+
/// get_sorted_txs_with_fees byte-reproducible across nodes/runs
79+
/// (equal-feerate txs no longer depend on mempool insertion order) and
80+
/// matches dashcore BlockAssembler ordering
81+
/// (CompareTxMemPoolEntryByAncestorFee breaks feerate ties by GetHash()).
82+
struct FeeKey {
83+
double feerate;
84+
uint256 txid;
85+
bool operator<(const FeeKey& o) const {
86+
if (feerate != o.feerate) return feerate > o.feerate; // higher feerate first
87+
return txid < o.txid; // txid asc tiebreak (oracle-conformant)
88+
}
89+
};
90+
7591
class Mempool {
7692
public:
7793
static constexpr size_t DEFAULT_MAX_BYTES = 300ULL * 1024 * 1024;
@@ -139,7 +155,7 @@ class Mempool {
139155
// recompute_unknown_fees() resolves them after a UTXO update.
140156
// Uses negative feerate as the multimap key so begin() = best.
141157
if (stored.fee_known) {
142-
m_feerate_index.emplace(stored.feerate_satvb(), txid);
158+
m_feerate_index.insert(FeeKey{stored.feerate_satvb(), txid});
143159
}
144160

145161
// Periodic stats — every 100 entries + first 5 + every eviction.
@@ -243,7 +259,7 @@ class Mempool {
243259
for (auto& [txid, entry] : m_pool) {
244260
if (entry.fee_known) continue;
245261
if (compute_fee_locked(entry, utxo)) {
246-
m_feerate_index.emplace(entry.feerate_satvb(), txid);
262+
m_feerate_index.insert(FeeKey{entry.feerate_satvb(), txid});
247263
resolved_fees += entry.fee;
248264
++resolved;
249265
} else {
@@ -346,8 +362,8 @@ class Mempool {
346362
uint32_t total_bytes = 0;
347363
auto* utxo = m_utxo.load();
348364

349-
for (const auto& [feerate, txid] : m_feerate_index) {
350-
(void)feerate;
365+
for (const auto& fk : m_feerate_index) {
366+
const uint256& txid = fk.txid;
351367
auto pit = m_pool.find(txid);
352368
if (pit == m_pool.end()) continue;
353369
const auto& entry = pit->second;
@@ -388,7 +404,7 @@ class Mempool {
388404
std::map<std::pair<uint256, uint32_t>, uint256> m_spent_outputs;
389405
// Step 2: feerate-sorted index. greater<double> means begin() =
390406
// highest feerate, so iteration is best-first.
391-
std::multimap<double, uint256, std::greater<double>> m_feerate_index;
407+
std::set<FeeKey> m_feerate_index;
392408
size_t m_total_bytes{0};
393409
size_t m_max_bytes;
394410
time_t m_expiry_sec;
@@ -415,13 +431,7 @@ class Mempool {
415431

416432
// Drop from feerate index (only present if fee was known).
417433
if (it->second.fee_known) {
418-
auto frng = m_feerate_index.equal_range(it->second.feerate_satvb());
419-
for (auto rit = frng.first; rit != frng.second; ++rit) {
420-
if (rit->second == txid) {
421-
m_feerate_index.erase(rit);
422-
break;
423-
}
424-
}
434+
m_feerate_index.erase(FeeKey{it->second.feerate_satvb(), txid});
425435
}
426436

427437
// Drop from spent-outputs index.

test/test_dash_mempool.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
#include <core/uint256.hpp>
2626
#include <core/coin/utxo_view_cache.hpp>
2727

28+
#include <algorithm>
2829
#include <cstdint>
30+
#include <vector>
2931

3032
using dash::coin::Mempool;
3133
using dash::coin::MutableTransaction;
@@ -254,3 +256,63 @@ TEST(DashMempool, SortedSelectionHighestFeerateFirst)
254256
EXPECT_EQ(dash_txid(selected[1].tx), dash_txid(tx_lo));
255257
EXPECT_EQ(total_fees, 11'000u);
256258
}
259+
260+
// ─── G1 byte-parity: equal-feerate selection is deterministic ────────────────
261+
//
262+
// The embedded GBT template builder serializes txs in the order
263+
// get_sorted_txs_with_fees() returns them. For txs sharing the SAME
264+
// feerate the old std::multimap<double,uint256> kept them in mempool
265+
// INSERTION order, so two nodes with the same mempool contents but a
266+
// different arrival order produced different template bytes — a
267+
// non-deterministic seam that breaks G1 byte-parity against the
268+
// p2pool-dash / dashcore oracle. FeeKey now breaks feerate ties by txid
269+
// ascending (matches dashcore CompareTxMemPoolEntryByAncestorFee's
270+
// GetHash() tiebreak), so the projection is byte-reproducible.
271+
//
272+
// This KAT pins that: identical equal-feerate sets added in OPPOSITE
273+
// orders must yield the SAME selection, ordered by txid ascending.
274+
static std::vector<uint256> equal_feerate_selection(bool reverse_insertion)
275+
{
276+
UTXOViewCache utxo(nullptr);
277+
// 5 distinct prevouts, identical value ⇒ identical fee & base_size
278+
// ⇒ identical feerate for every spend below.
279+
constexpr int N = 5;
280+
std::vector<MutableTransaction> txs;
281+
for (int i = 0; i < N; ++i) {
282+
uint256 prev = mint_hash(200 + i);
283+
utxo.add_coin(Outpoint(prev, 0), Coin(100'000, {}, 1, false));
284+
txs.push_back(make_spend(prev, 0, /*out=*/95'000, /*salt=*/300 + i)); // fee 5000
285+
}
286+
287+
Mempool mp;
288+
mp.set_utxo(&utxo);
289+
if (reverse_insertion)
290+
for (auto it = txs.rbegin(); it != txs.rend(); ++it) EXPECT_TRUE(mp.add_tx(*it));
291+
else
292+
for (auto& t : txs) EXPECT_TRUE(mp.add_tx(t));
293+
294+
auto [selected, total_fees] = mp.get_sorted_txs_with_fees(/*max_bytes=*/1u << 20);
295+
EXPECT_EQ(total_fees, static_cast<uint64_t>(N) * 5'000u);
296+
std::vector<uint256> out;
297+
for (auto& s : selected) out.push_back(dash_txid(s.tx));
298+
return out;
299+
}
300+
301+
TEST(DashMempool, EqualFeerateSelectionIsTxidAscendingAndInsertionOrderIndependent)
302+
{
303+
auto forward = equal_feerate_selection(/*reverse_insertion=*/false);
304+
auto reverse = equal_feerate_selection(/*reverse_insertion=*/true);
305+
306+
ASSERT_EQ(forward.size(), 5u);
307+
ASSERT_EQ(reverse.size(), 5u);
308+
309+
// Insertion order must not affect the projected order.
310+
EXPECT_EQ(forward, reverse)
311+
<< "equal-feerate tx order must be independent of mempool arrival order";
312+
313+
// And that stable order is txid ascending (dashcore GetHash() tiebreak).
314+
auto sorted = forward;
315+
std::sort(sorted.begin(), sorted.end());
316+
EXPECT_EQ(forward, sorted)
317+
<< "equal-feerate ties must resolve to txid-ascending, oracle-conformant order";
318+
}

0 commit comments

Comments
 (0)