highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-10 14:48:46 +02:00
Code Issues Packages Projects Releases Wiki Activity

feefrac: drop comparison and operator{<<,>>} for sorted wrappers

Browse Source 
Instead of having an unintuitive but total implicit sort order on
FeeFrac (first increasing feerate, then decreasing size), and separate
overloaded operator<< and operator>> for a weak ordering that only looks
at feerate, replace these with explicit wrapper classes which make the
behavior more explicit.

This allows for things like ByRatio{a} <= ByRatio{b}, instead of the
earlier !(a >> b). It also supports usage inside std::max and
std::greater, so one can use:
* std::max<ByRatioNegSize<FeeFrac>>(a, b)
* std::sort(v.begin(), v.end(), std::greater<ByRatioNegSize<FeeFrac>>{})
This commit is contained in:
Pieter Wuille
2026-02-24 17:09:59 -05:00
committed by Pieter Wuille
parent 3a8b4e89f6
commit 747da25360
15 changed files with 215 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
src/cluster_linearize.h
View File

@@ -432,7 +432,7 @@ std::vector<SetInfo<SetType>> ChunkLinearizationInfo(const DepGraph<SetType>& de
/** The new chunk to be added, initially a singleton. */
SetInfo<SetType> new_chunk(depgraph, i);
// As long as the new chunk has a higher feerate than the last chunk so far, absorb it.
while (!ret.empty() && new_chunk.feerate >> ret.back().feerate) {
while (!ret.empty() && ByRatio{new_chunk.feerate} > ByRatio{ret.back().feerate}) {
new_chunk |= ret.back();
ret.pop_back();
}
@@ -452,7 +452,7 @@ std::vector<FeeFrac> ChunkLinearization(const DepGraph<SetType>& depgraph, std::
/** The new chunk to be added, initially a singleton. */
auto new_chunk = depgraph.FeeRate(i);
// As long as the new chunk has a higher feerate than the last chunk so far, absorb it.
while (!ret.empty() && new_chunk >> ret.back()) {
while (!ret.empty() && ByRatio{new_chunk} > ByRatio{ret.back()}) {
new_chunk += ret.back();
ret.pop_back();
}
@@ -1027,8 +1027,8 @@ private:
auto& reached_chunk_info = m_set_info[reached_chunk_idx];
todo -= reached_chunk_info.transactions;
// See if it has an acceptable feerate.
auto cmp = DownWard ? FeeRateCompare(best_other_chunk_feerate, reached_chunk_info.feerate)
: FeeRateCompare(reached_chunk_info.feerate, best_other_chunk_feerate);
auto cmp = DownWard ? ByRatio{best_other_chunk_feerate} <=> ByRatio{reached_chunk_info.feerate}
: ByRatio{reached_chunk_info.feerate} <=> ByRatio{best_other_chunk_feerate};
if (cmp > 0) continue;
uint64_t tiebreak = m_rng.rand64();
if (cmp < 0 || tiebreak >= best_other_chunk_tiebreak) {
@@ -1150,7 +1150,7 @@ private:
auto& dep_top_info = m_set_info[tx_data.dep_top_idx[child_idx]];
// Skip if this dependency is ineligible (the top chunk that would be created
// does not have higher feerate than the chunk it is currently part of).
auto cmp = FeeRateCompare(dep_top_info.feerate, chunk_info.feerate);
auto cmp = ByRatio{dep_top_info.feerate} <=> ByRatio{chunk_info.feerate};
if (cmp <= 0) continue;
// Generate a random tiebreak for this dependency, and reject it if its tiebreak
// is worse than the best so far. This means that among all eligible
@@ -1377,7 +1377,7 @@ public:
// Skip if this dependency does not have equal top and bottom set feerates. Note
// that the top cannot have higher feerate than the bottom, or OptimizeSteps would
// have dealt with it.
if (dep_top_info.feerate << chunk_info.feerate) continue;
if (ByRatio{dep_top_info.feerate} < ByRatio{chunk_info.feerate}) continue;
have_any = true;
// Skip if this dependency does not have pivot in the right place.
if (move_pivot_down == dep_top_info.transactions[pivot_idx]) continue;
@@ -1506,7 +1506,7 @@ public:
// First sort by increasing transaction feerate.
auto& a_feerate = m_depgraph.FeeRate(a);
auto& b_feerate = m_depgraph.FeeRate(b);
auto feerate_cmp = FeeRateCompare(a_feerate, b_feerate);
auto feerate_cmp = ByRatio{a_feerate} <=> ByRatio{b_feerate};
if (feerate_cmp != 0) return feerate_cmp < 0;
// Then by decreasing transaction size.
if (a_feerate.size != b_feerate.size) {
@@ -1528,7 +1528,7 @@ public:
// First sort by increasing chunk feerate.
auto& chunk_feerate_a = m_set_info[a.first].feerate;
auto& chunk_feerate_b = m_set_info[b.first].feerate;
auto feerate_cmp = FeeRateCompare(chunk_feerate_a, chunk_feerate_b);
auto feerate_cmp = ByRatio{chunk_feerate_a} <=> ByRatio{chunk_feerate_b};
if (feerate_cmp != 0) return feerate_cmp < 0;
// Then by decreasing chunk size.
if (chunk_feerate_a.size != chunk_feerate_b.size) {
@@ -1618,7 +1618,7 @@ public:
for (auto chunk_idx : m_chunk_idxs) {
ret.push_back(m_set_info[chunk_idx].feerate);
}
std::sort(ret.begin(), ret.end(), std::greater{});
std::sort(ret.begin(), ret.end(), std::greater<ByRatioNegSize<FeeFrac>>{});
return ret;
}
@@ -1972,7 +1972,7 @@ void PostLinearize(const DepGraph<SetType>& depgraph, std::span<DepGraphIndex> l
DepGraphIndex next_group = SENTINEL; // We inserted at the end, so next group is sentinel.
DepGraphIndex prev_group = entries[cur_group].prev_group;
// Continue as long as the current group has higher feerate than the previous one.
while (entries[cur_group].feerate >> entries[prev_group].feerate) {
while (ByRatio{entries[cur_group].feerate} > ByRatio{entries[prev_group].feerate}) {
// prev_group/cur_group/next_group refer to (the last transactions of) 3
// consecutive entries in groups list.
Assume(cur_group == entries[next_group].prev_group);

2
src/node/miner.cpp
View File

@@ -295,7 +295,7 @@ void BlockAssembler::addChunks()
while (selected_transactions.size() > 0) {
// Check to see if min fee rate is still respected.
if (chunk_feerate_vsize << m_options.blockMinFeeRate.GetFeePerVSize()) {
if (ByRatio{chunk_feerate_vsize} < ByRatio{m_options.blockMinFeeRate.GetFeePerVSize()}) {
// Everything else we might consider has a lower feerate
return;
}

4
src/node/mini_miner.cpp
View File

@@ -184,12 +184,12 @@ struct AncestorFeerateComparator
auto min_feerate = [](const MiniMinerMempoolEntry& e) -> FeeFrac {
FeeFrac self_feerate(e.GetModifiedFee(), e.GetTxSize());
FeeFrac ancestor_feerate(e.GetModFeesWithAncestors(), e.GetSizeWithAncestors());
return std::min(ancestor_feerate, self_feerate);
return std::min<ByRatioNegSize<FeeFrac>>(ancestor_feerate, self_feerate);
};
FeeFrac a_feerate{min_feerate(a->second)};
FeeFrac b_feerate{min_feerate(b->second)};
if (a_feerate != b_feerate) {
return a_feerate > b_feerate;
return ByRatioNegSize{a_feerate} > ByRatioNegSize{b_feerate};
}
// Use txid as tiebreaker for stable sorting
return a->first < b->first;

20
src/node/txorphanage.cpp
View File

@@ -164,7 +164,7 @@ class TxOrphanageImpl final : public TxOrphanage {
assert(max_peer_memory > 0);
const FeeFrac latency_score(m_total_latency_score, max_peer_latency_score);
const FeeFrac mem_score(m_total_usage, max_peer_memory);
return std::max<FeeFrac>(latency_score, mem_score);
return std::max<ByRatioNegSize<FeeFrac>>(latency_score, mem_score);
}
};
/** Store per-peer statistics. Used to determine each peer's DoS score. The size of this map is used to determine the
@@ -457,12 +457,18 @@ void TxOrphanageImpl::LimitOrphans()
for (const auto& [nodeid, entry] : m_peer_orphanage_info) {
// Performance optimization: only consider peers with a DoS score > 1.
const auto dos_score = entry.GetDosScore(max_lat, max_mem);
if (dos_score >> FeeFrac{1, 1}) {
if (ByRatio{dos_score} > ByRatio{FeeFrac{1, 1}}) {
heap_peer_dos.emplace_back(nodeid, dos_score);
}
}
static constexpr auto compare_score = [](const auto& left, const auto& right) {
if (left.second != right.second) return left.second < right.second;
if (left.second != right.second) {
// Note: if ratios are the same, this tiebreaks by denominator. In practice, since the
// latency denominator (number of announcements and inputs) is always lower, this means
// that a peer with only high latency scores will be targeted before a peer using a lot
// of memory, even if they have the same ratios.
return ByRatioNegSize{left.second} < ByRatioNegSize{right.second};
}
// Tiebreak by considering the more recent peer (higher NodeId) to be worse.
return left.first < right.first;
};
@@ -472,9 +478,6 @@ void TxOrphanageImpl::LimitOrphans()
// This outer loop finds the peer with the highest DoS score, which is a fraction of memory and latency scores
// over the respective allowances. We continue until the orphanage is within global limits. That means some peers
// might still have a DoS score > 1 at the end.
// Note: if ratios are the same, FeeFrac tiebreaks by denominator. In practice, since the latency denominator (number of
// announcements and inputs) is always lower, this means that a peer with only high latency scores will be targeted
// before a peer using a lot of memory, even if they have the same ratios.
do {
Assume(!heap_peer_dos.empty());
// This is a max-heap, so the worst peer is at the front. pop_heap()
@@ -484,7 +487,7 @@ void TxOrphanageImpl::LimitOrphans()
heap_peer_dos.pop_back();
// If needs trim, then at least one peer has a DoS score higher than 1.
Assume(dos_score >> (FeeFrac{1, 1}));
Assume(ByRatio{dos_score} > ByRatio{FeeFrac(1, 1)});
auto it_worst_peer = m_peer_orphanage_info.find(worst_peer);
@@ -506,7 +509,8 @@ void TxOrphanageImpl::LimitOrphans()
// If we erased the last orphan from this peer, it_worst_peer will be invalidated.
it_worst_peer = m_peer_orphanage_info.find(worst_peer);
if (it_worst_peer == m_peer_orphanage_info.end() || it_worst_peer->second.GetDosScore(max_lat, max_mem) <= dos_threshold) break;
if (it_worst_peer == m_peer_orphanage_info.end() ||
ByRatioNegSize{it_worst_peer->second.GetDosScore(max_lat, max_mem)} <= ByRatioNegSize{dos_threshold}) break;
}
LogDebug(BCLog::TXPACKAGES, "peer=%d orphanage overflow, removed %u of %u announcements\n", worst_peer, num_erased_this_round, starting_num_ann);

6
src/policy/feerate.h
View File

@@ -60,13 +60,13 @@ public:
* Return the fee in satoshis for a vsize of 1000 vbytes
*/
CAmount GetFeePerK() const { return CAmount(m_feerate.EvaluateFeeDown(1000)); }
friend std::weak_ordering operator<=>(const CFeeRate& a, const CFeeRate& b) noexcept
friend std::strong_ordering operator<=>(const CFeeRate& a, const CFeeRate& b) noexcept
{
return FeeRateCompare(a.m_feerate, b.m_feerate);
return ByRatio{a.m_feerate} <=> ByRatio{b.m_feerate};
}
friend bool operator==(const CFeeRate& a, const CFeeRate& b) noexcept
{
return FeeRateCompare(a.m_feerate, b.m_feerate) == std::weak_ordering::equivalent;
return ByRatio{a.m_feerate} == ByRatio{b.m_feerate};
}
CFeeRate& operator+=(const CFeeRate& a) {
m_feerate = FeePerVSize(GetFeePerK() + a.GetFeePerK(), 1000);

60
src/test/feefrac_tests.cpp
View File

@@ -70,40 +70,40 @@ BOOST_AUTO_TEST_CASE(feefrac_operators)
BOOST_CHECK(p1 + p3 == p4);
// Fee-rate comparison
BOOST_CHECK(p1 > p2);
BOOST_CHECK(p1 >= p2);
BOOST_CHECK(p1 >= p4-p3);
BOOST_CHECK(!(p1 >> p3)); // not strictly better
BOOST_CHECK(p1 >> p2); // strictly greater feerate
BOOST_CHECK(ByRatioNegSize{p1} > ByRatioNegSize{p2});
BOOST_CHECK(ByRatioNegSize{p1} >= ByRatioNegSize{p2});
BOOST_CHECK(ByRatioNegSize{p1} >= ByRatioNegSize{p4-p3});
BOOST_CHECK(!(ByRatio{p1} > ByRatio{p3})); // not strictly better
BOOST_CHECK(ByRatio{p1} > ByRatio{p2}); // strictly greater feerate
BOOST_CHECK(p2 < p1);
BOOST_CHECK(p2 <= p1);
BOOST_CHECK(p1 <= p4-p3);
BOOST_CHECK(!(p3 << p1)); // not strictly worse
BOOST_CHECK(p2 << p1); // strictly lower feerate
BOOST_CHECK(ByRatioNegSize{p2} < ByRatioNegSize{p1});
BOOST_CHECK(ByRatioNegSize{p2} <= ByRatioNegSize{p1});
BOOST_CHECK(ByRatioNegSize{p1} <= ByRatioNegSize{p4-p3});
BOOST_CHECK(!(ByRatio{p3} < ByRatio{p1})); // not strictly worse
BOOST_CHECK(ByRatio{p2} < ByRatio{p1}); // strictly lower feerate
// "empty" comparisons
BOOST_CHECK(!(p1 >> empty)); // << will always result in false
BOOST_CHECK(!(p1 << empty));
BOOST_CHECK(!(empty >> empty));
BOOST_CHECK(!(empty << empty));
BOOST_CHECK(!(ByRatio{p1} > ByRatio{empty})); // << will always result in false
BOOST_CHECK(!(ByRatio{p1} < ByRatio{empty}));
BOOST_CHECK(!(ByRatio{empty} > ByRatio{empty}));
BOOST_CHECK(!(ByRatio{empty} < ByRatio{empty}));
// empty is always bigger than everything else
BOOST_CHECK(empty > p1);
BOOST_CHECK(empty > p2);
BOOST_CHECK(empty > p3);
BOOST_CHECK(empty >= p1);
BOOST_CHECK(empty >= p2);
BOOST_CHECK(empty >= p3);
BOOST_CHECK(ByRatioNegSize{empty} > ByRatioNegSize{p1});
BOOST_CHECK(ByRatioNegSize{empty} > ByRatioNegSize{p2});
BOOST_CHECK(ByRatioNegSize{empty} > ByRatioNegSize{p3});
BOOST_CHECK(ByRatioNegSize{empty} >= ByRatioNegSize{p1});
BOOST_CHECK(ByRatioNegSize{empty} >= ByRatioNegSize{p2});
BOOST_CHECK(ByRatioNegSize{empty} >= ByRatioNegSize{p3});
// check "max" values for comparison
FeeFrac oversized_1{4611686000000, 4000000};
FeeFrac oversized_2{184467440000000, 100000};
BOOST_CHECK(oversized_1 < oversized_2);
BOOST_CHECK(oversized_1 <= oversized_2);
BOOST_CHECK(oversized_1 << oversized_2);
BOOST_CHECK(oversized_1 != oversized_2);
BOOST_CHECK(ByRatioNegSize{oversized_1} < ByRatioNegSize{oversized_2});
BOOST_CHECK(ByRatioNegSize{oversized_1} <= ByRatioNegSize{oversized_2});
BOOST_CHECK(ByRatio{oversized_1} < ByRatio{oversized_2});
BOOST_CHECK(ByRatioNegSize{oversized_1} != ByRatioNegSize{oversized_2});
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(0), 0);
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(1), 1152921);
@@ -124,13 +124,13 @@ BOOST_AUTO_TEST_CASE(feefrac_operators)
// Tests paths that use double arithmetic
FeeFrac busted{(static_cast<int64_t>(INT32_MAX)) + 1, INT32_MAX};
BOOST_CHECK(!(busted < busted));
BOOST_CHECK(!(ByRatioNegSize{busted} < ByRatioNegSize{busted}));
FeeFrac max_fee{2100000000000000, INT32_MAX};
BOOST_CHECK(!(max_fee < max_fee));
BOOST_CHECK(!(max_fee > max_fee));
BOOST_CHECK(max_fee <= max_fee);
BOOST_CHECK(max_fee >= max_fee);
BOOST_CHECK(!(ByRatioNegSize{max_fee} < ByRatioNegSize{max_fee}));
BOOST_CHECK(!(ByRatioNegSize{max_fee} > ByRatioNegSize{max_fee}));
BOOST_CHECK(ByRatioNegSize{max_fee} <= ByRatioNegSize{max_fee});
BOOST_CHECK(ByRatioNegSize{max_fee} >= ByRatioNegSize{max_fee});
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(0), 0);
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(1), 977888);
@@ -146,7 +146,7 @@ BOOST_AUTO_TEST_CASE(feefrac_operators)
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(INT32_MAX), 2100000000000000);
FeeFrac max_fee2{1, 1};
BOOST_CHECK(max_fee >= max_fee2);
BOOST_CHECK(ByRatioNegSize{max_fee} >= ByRatioNegSize{max_fee2});
// Test for integer overflow issue (https://github.com/bitcoin/bitcoin/issues/32294)
BOOST_CHECK_EQUAL((FeeFrac{0x7ffffffdfffffffb, 0x7ffffffd}.EvaluateFeeDown(0x7fffffff)), 0x7fffffffffffffff);

20
src/test/fuzz/cluster_linearize.cpp
View File

@@ -126,7 +126,7 @@ public:
// Add a queue entry with split excluded.
queue.emplace_back(inc, und - m_depgraph.Descendants(split));
// Update statistics to account for the candidate new_inc.
if (new_inc.feerate > best.feerate) best = new_inc;
if (ByRatioNegSize{new_inc.feerate} > ByRatioNegSize{best.feerate}) best = new_inc;
break;
}
}
@@ -181,7 +181,7 @@ public:
x_shifted >>= 1;
}
SetInfo cur(m_depgraph, txn & m_todo);
if (cur.feerate > best.feerate) best = cur;
if (ByRatioNegSize{cur.feerate} > ByRatioNegSize{best.feerate}) best = cur;
}
return best;
}
@@ -737,7 +737,7 @@ FUZZ_TARGET(clusterlin_chunking)
// Verify that chunk feerates are monotonically non-increasing.
for (size_t i = 1; i < chunking.size(); ++i) {
assert(!(chunking[i] >> chunking[i - 1]));
assert(ByRatio{chunking[i]} <= ByRatio{chunking[i - 1]});
}
// Naively recompute the chunks (each is the highest-feerate prefix of what remains).
@@ -748,7 +748,7 @@ FUZZ_TARGET(clusterlin_chunking)
for (DepGraphIndex idx : linearization) {
if (todo[idx]) {
accumulator.Set(depgraph, idx);
if (best.feerate.IsEmpty() || accumulator.feerate >> best.feerate) {
if (best.feerate.IsEmpty() || ByRatio{accumulator.feerate} > ByRatio{best.feerate}) {
best = accumulator;
}
}
@@ -825,7 +825,7 @@ FUZZ_TARGET(clusterlin_simple_finder)
// Compare with a non-empty topological set read from the fuzz input (comparing with an
// empty set is not interesting).
auto read_topo = ReadTopologicalSet(depgraph, todo, reader, /*non_empty=*/true);
assert(found.feerate >= depgraph.FeeRate(read_topo));
assert(ByRatioNegSize{found.feerate} >= ByRatioNegSize{depgraph.FeeRate(read_topo)});
}
// Find a non-empty topologically valid subset of transactions to remove from the graph.
@@ -1110,9 +1110,9 @@ FUZZ_TARGET(clusterlin_linearize)
// Check whether tx2 only depends on transactions that precede tx1.
if ((depgraph.Ancestors(tx2) - done).Count() == 1) {
// tx2 could take position pos1.
// Verify that individual transaction feerate is decreasing (note that >=
// tie-breaks by size).
assert(depgraph.FeeRate(tx1) >= depgraph.FeeRate(tx2));
// Verify that individual transaction feerate is decreasing (tie-breaking by
// size).
assert(ByRatioNegSize{depgraph.FeeRate(tx1)} >= ByRatioNegSize{depgraph.FeeRate(tx2)});
// If feerate and size are equal, compare by DepGraphIndex.
if (depgraph.FeeRate(tx1) == depgraph.FeeRate(tx2)) {
assert(tx1 < tx2);
@@ -1139,8 +1139,8 @@ FUZZ_TARGET(clusterlin_linearize)
// Check whether chunk2 only depends on transactions that precede chunk1.
if ((chunk2_ancestors - done).IsSubsetOf(chunk2.transactions)) {
// chunk2 could take position chunk_num1.
// Verify that chunk feerate is decreasing (note that >= tie-breaks by size).
assert(chunk1.feerate >= chunk2.feerate);
// Verify that chunk feerate is decreasing (tie-breaking by size).
assert(ByRatioNegSize{chunk1.feerate} >= ByRatioNegSize{chunk2.feerate});
// If feerate and size are equal, compare by maximum DepGraphIndex element.
if (chunk1.feerate == chunk2.feerate) {
assert(chunk1.transactions.Last() < chunk2.transactions.Last());

20
src/test/fuzz/feefrac.cpp
View File

@@ -84,9 +84,9 @@ FUZZ_TARGET(feefrac)
// Feerate comparisons
auto cmp_feerate = MulCompare(f1, s2, f2, s1);
assert(FeeRateCompare(fr1, fr2) == cmp_feerate);
assert((fr1 << fr2) == std::is_lt(cmp_feerate));
assert((fr1 >> fr2) == std::is_gt(cmp_feerate));
assert((ByRatio{fr1} <=> ByRatio{fr2}) == cmp_feerate);
assert((ByRatio{fr1} < ByRatio{fr2}) == std::is_lt(cmp_feerate));
assert((ByRatio{fr1} > ByRatio{fr2}) == std::is_gt(cmp_feerate));
// Compare with manual invocation of FeeFrac::Mul.
auto cmp_mul = FeeFrac::Mul(f1, s2) <=> FeeFrac::Mul(f2, s1);
@@ -98,13 +98,13 @@ FUZZ_TARGET(feefrac)
// Total order comparisons
auto cmp_total = std::is_eq(cmp_feerate) ? (s2 <=> s1) : cmp_feerate;
assert((fr1 <=> fr2) == cmp_total);
assert((fr1 < fr2) == std::is_lt(cmp_total));
assert((fr1 > fr2) == std::is_gt(cmp_total));
assert((fr1 <= fr2) == std::is_lteq(cmp_total));
assert((fr1 >= fr2) == std::is_gteq(cmp_total));
assert((fr1 == fr2) == std::is_eq(cmp_total));
assert((fr1 != fr2) == std::is_neq(cmp_total));
assert((ByRatioNegSize{fr1} <=> ByRatioNegSize{fr2}) == cmp_total);
assert((ByRatioNegSize{fr1} < ByRatioNegSize{fr2}) == std::is_lt(cmp_total));
assert((ByRatioNegSize{fr1} > ByRatioNegSize{fr2}) == std::is_gt(cmp_total));
assert((ByRatioNegSize{fr1} <= ByRatioNegSize{fr2}) == std::is_lteq(cmp_total));
assert((ByRatioNegSize{fr1} >= ByRatioNegSize{fr2}) == std::is_gteq(cmp_total));
assert((ByRatioNegSize{fr1} == ByRatioNegSize{fr2}) == std::is_eq(cmp_total));
assert((ByRatioNegSize{fr1} != ByRatioNegSize{fr2}) == std::is_neq(cmp_total));
}
FUZZ_TARGET(feefrac_div_fallback)

6
src/test/fuzz/feeratediagram.cpp
View File

@@ -63,9 +63,9 @@ FeeFrac EvaluateDiagram(int32_t size, std::span<const FeeFrac> diagram)
return {point_a.fee * dir_coef.size + dir_coef.fee * (size - point_a.size), dir_coef.size};
}
std::weak_ordering CompareFeeFracWithDiagram(const FeeFrac& ff, std::span<const FeeFrac> diagram)
std::strong_ordering CompareFeeFracWithDiagram(const FeeFrac& ff, std::span<const FeeFrac> diagram)
{
return FeeRateCompare(FeeFrac{ff.fee, 1}, EvaluateDiagram(ff.size, diagram));
return ByRatio{FeeFrac{ff.fee, 1}} <=> ByRatio{EvaluateDiagram(ff.size, diagram)};
}
std::partial_ordering CompareDiagrams(std::span<const FeeFrac> dia1, std::span<const FeeFrac> dia2)
@@ -126,7 +126,7 @@ FUZZ_TARGET(build_and_compare_feerate_diagram)
int32_t size = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(0, diagram2.back().size);
auto eval1 = EvaluateDiagram(size, diagram1);
auto eval2 = EvaluateDiagram(size, diagram2);
auto cmp = FeeRateCompare(eval1, eval2);
auto cmp = ByRatio{eval1} <=> ByRatio{eval2};
if (std::is_lt(cmp)) assert(!std::is_gt(real));
if (std::is_gt(cmp)) assert(!std::is_lt(real));
}

4
src/test/fuzz/rbf.cpp
View File

@@ -210,12 +210,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
FeeFrac first_sum;
for (size_t i = 0; i < calc_results->first.size(); ++i) {
first_sum += calc_results->first[i];
if (i) assert(!(calc_results->first[i - 1] << calc_results->first[i]));
if (i) assert(ByRatio{calc_results->first[i - 1]} >= ByRatio{calc_results->first[i]});
}
FeeFrac second_sum;
for (size_t i = 0; i < calc_results->second.size(); ++i) {
second_sum += calc_results->second[i];
if (i) assert(!(calc_results->second[i - 1] << calc_results->second[i]));
if (i) assert(ByRatio{calc_results->second[i - 1]} >= ByRatio{calc_results->second[i]});
}
FeeFrac replaced;

26
src/test/fuzz/txgraph.cpp
View File

@@ -433,7 +433,7 @@ FUZZ_TARGET(txgraph)
assert(num_tx == sim.GetTransactionCount());
// Sort by feerate only, since violating topological constraints within same-feerate
// chunks won't affect diagram comparisons.
std::sort(chunk_feerates.begin(), chunk_feerates.end(), std::greater{});
std::sort(chunk_feerates.begin(), chunk_feerates.end(), std::greater<ByRatioNegSize<FeeFrac>>{});
return chunk_feerates;
};
@@ -806,10 +806,10 @@ FUZZ_TARGET(txgraph)
assert(sim_gain == real_gain);
// Check that the feerates in each diagram are monotonically decreasing.
for (size_t i = 1; i < real_main_diagram.size(); ++i) {
assert(FeeRateCompare(real_main_diagram[i], real_main_diagram[i - 1]) <= 0);
assert(ByRatio{real_main_diagram[i]} <= ByRatio{real_main_diagram[i - 1]});
}
for (size_t i = 1; i < real_staged_diagram.size(); ++i) {
assert(FeeRateCompare(real_staged_diagram[i], real_staged_diagram[i - 1]) <= 0);
assert(ByRatio{real_staged_diagram[i]} <= ByRatio{real_staged_diagram[i - 1]});
}
break;
} else if (block_builders.size() < 4 && !main_sim.IsOversized() && command-- == 0) {
@@ -829,7 +829,7 @@ FUZZ_TARGET(txgraph)
if (chunk) {
// Chunk feerates must be monotonously decreasing.
if (!builder_data.last_feerate.IsEmpty()) {
assert(!(chunk->second >> builder_data.last_feerate));
assert(ByRatio{chunk->second} <= ByRatio{builder_data.last_feerate});
}
builder_data.last_feerate = chunk->second;
// Verify the contents of GetCurrentChunk.
@@ -1118,7 +1118,7 @@ FUZZ_TARGET(txgraph)
std::pair<int32_t, uint64_t> max_chunk_tiebreak{0, 0};
for (const auto& chunk : real_chunking) {
// If this is the first chunk with a strictly lower feerate, reset.
if (chunk.feerate << last_chunk_feerate) {
if (ByRatio{chunk.feerate} < ByRatio{last_chunk_feerate}) {
comp_prefix_sizes.clear();
max_chunk_tiebreak = {0, 0};
}
@@ -1212,12 +1212,12 @@ FUZZ_TARGET(txgraph)
if (pos > 0) {
size_t before = rng.randrange<size_t>(pos);
auto before_feerate = real->GetMainChunkFeerate(*sims[0].GetRef(vec1[before]));
assert(FeeRateCompare(before_feerate, pos_feerate) >= 0);
assert(ByRatio{before_feerate} >= ByRatio{pos_feerate});
}
if (pos + 1 < vec1.size()) {
size_t after = pos + 1 + rng.randrange<size_t>(vec1.size() - 1 - pos);
auto after_feerate = real->GetMainChunkFeerate(*sims[0].GetRef(vec1[after]));
assert(FeeRateCompare(after_feerate, pos_feerate) <= 0);
assert(ByRatio{after_feerate} <= ByRatio{pos_feerate});
}
}
@@ -1265,17 +1265,17 @@ FUZZ_TARGET(txgraph)
auto [main_cmp_diagram, stage_cmp_diagram] = real->GetMainStagingDiagrams();
// Check that the feerates in each diagram are monotonically decreasing.
for (size_t i = 1; i < main_cmp_diagram.size(); ++i) {
assert(FeeRateCompare(main_cmp_diagram[i], main_cmp_diagram[i - 1]) <= 0);
assert(ByRatio{main_cmp_diagram[i]} <= ByRatio{main_cmp_diagram[i - 1]});
}
for (size_t i = 1; i < stage_cmp_diagram.size(); ++i) {
assert(FeeRateCompare(stage_cmp_diagram[i], stage_cmp_diagram[i - 1]) <= 0);
assert(ByRatio{stage_cmp_diagram[i]} <= ByRatio{stage_cmp_diagram[i - 1]});
}
// Treat the diagrams as sets of chunk feerates, and sort them in the same way so that
// std::set_difference can be used on them below. The exact ordering does not matter
// here, but it has to be consistent with the one used in main_real_diagram and
// stage_real_diagram).
std::sort(main_cmp_diagram.begin(), main_cmp_diagram.end(), std::greater{});
std::sort(stage_cmp_diagram.begin(), stage_cmp_diagram.end(), std::greater{});
std::sort(main_cmp_diagram.begin(), main_cmp_diagram.end(), std::greater<ByRatioNegSize<FeeFrac>>{});
std::sort(stage_cmp_diagram.begin(), stage_cmp_diagram.end(), std::greater<ByRatioNegSize<FeeFrac>>{});
// Find the chunks that appear in main_diagram but are missing from main_cmp_diagram.
// This is allowed, because GetMainStagingDiagrams omits clusters in main unaffected
// by staging.
@@ -1283,7 +1283,7 @@ FUZZ_TARGET(txgraph)
std::set_difference(main_real_diagram.begin(), main_real_diagram.end(),
main_cmp_diagram.begin(), main_cmp_diagram.end(),
std::inserter(missing_main_cmp, missing_main_cmp.end()),
std::greater{});
std::greater<ByRatioNegSize<FeeFrac>>{});
assert(main_cmp_diagram.size() + missing_main_cmp.size() == main_real_diagram.size());
// Do the same for chunks in stage_diagram missing from stage_cmp_diagram.
auto stage_real_diagram = get_diagram_fn(TxGraph::Level::TOP);
@@ -1291,7 +1291,7 @@ FUZZ_TARGET(txgraph)
std::set_difference(stage_real_diagram.begin(), stage_real_diagram.end(),
stage_cmp_diagram.begin(), stage_cmp_diagram.end(),
std::inserter(missing_stage_cmp, missing_stage_cmp.end()),
std::greater{});
std::greater<ByRatioNegSize<FeeFrac>>{});
assert(stage_cmp_diagram.size() + missing_stage_cmp.size() == stage_real_diagram.size());
// The missing chunks must be equal across main & staging (otherwise they couldn't have
// been omitted).

6
src/test/fuzz/txorphan.cpp
View File

@@ -554,7 +554,7 @@ FUZZ_TARGET(txorphanage_sim)
count += 1 + (txn[ann.tx]->vin.size() / 10);
usage += GetTransactionWeight(*txn[ann.tx]);
}
return std::max(FeeFrac{count, max_count}, FeeFrac{usage, max_usage});
return std::max<ByRatioNegSize<FeeFrac>>(FeeFrac{count, max_count}, FeeFrac{usage, max_usage});
};
//
@@ -706,13 +706,13 @@ FUZZ_TARGET(txorphanage_sim)
auto dos_score = dos_score_fn(peer, max_ann, max_mem);
// Use >= so that the more recent peer (higher NodeId) wins in case of
// ties.
if (dos_score >= worst_dos_score) {
if (ByRatioNegSize{dos_score} >= ByRatioNegSize{worst_dos_score}) {
worst_dos_score = dos_score;
worst_peer = peer;
}
}
assert(worst_peer != unsigned(-1));
assert(worst_dos_score >> FeeFrac(1, 1));
assert(ByRatio{worst_dos_score} > ByRatio{FeeFrac(1, 1)});
// Find oldest announcement from worst_peer, preferring non-reconsiderable ones.
bool done{false};
for (int reconsider = 0; reconsider < 2; ++reconsider) {

19
src/txgraph.cpp
View File

@@ -496,9 +496,8 @@ private:
const auto& entry_a = m_entries[a];
const auto& entry_b = m_entries[b];
// 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;
auto feerate_cmp = ByRatio{entry_b.m_main_chunk_feerate} <=> ByRatio{entry_a.m_main_chunk_feerate};
if (feerate_cmp != 0) return feerate_cmp;
// Compare equal-feerate chunk prefix size for comparing equal chunk feerates. This does two
// things: it distinguishes equal-feerate chunks within the same cluster (because later
// ones will always have a higher prefix size), and it may distinguish equal-feerate chunks
@@ -1100,7 +1099,7 @@ void GenericClusterImpl::Updated(TxGraphImpl& graph, int level, bool rename) noe
Assume(chunk_count > 0);
// Update equal_feerate_chunk_feerate to include this chunk, starting over when the
// feerate changed.
if (chunk.feerate << equal_feerate_chunk_feerate) {
if (ByRatio{chunk.feerate} < ByRatio{equal_feerate_chunk_feerate}) {
equal_feerate_chunk_feerate = chunk.feerate;
} else {
// Note that this is adding fees to fees, and sizes to sizes, so the overall
@@ -2828,8 +2827,8 @@ std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>> TxGraphImpl::GetMainStagin
}
}
// Sort both by decreasing feerate to obtain diagrams, and return them.
std::sort(main_feerates.begin(), main_feerates.end(), [](auto& a, auto& b) { return a > b; });
std::sort(staging_feerates.begin(), staging_feerates.end(), [](auto& a, auto& b) { return a > b; });
std::sort(main_feerates.begin(), main_feerates.end(), std::greater<ByRatioNegSize<FeeFrac>>{});
std::sort(staging_feerates.begin(), staging_feerates.end(), std::greater<ByRatioNegSize<FeeFrac>>{});
return std::make_pair(std::move(main_feerates), std::move(staging_feerates));
}
@@ -2875,10 +2874,10 @@ void GenericClusterImpl::SanityCheck(const TxGraphImpl& graph, int level) const
++chunk_num;
assert(chunk_num < linchunking.size());
chunk_pos = 0;
if (linchunking[chunk_num].feerate << equal_feerate_prefix) {
if (ByRatio{linchunking[chunk_num].feerate} < ByRatio{equal_feerate_prefix}) {
equal_feerate_prefix = linchunking[chunk_num].feerate;
} else {
assert(!(linchunking[chunk_num].feerate >> equal_feerate_prefix));
assert(ByRatio{linchunking[chunk_num].feerate} == ByRatio{equal_feerate_prefix});
equal_feerate_prefix += linchunking[chunk_num].feerate;
}
}
@@ -3103,7 +3102,7 @@ void TxGraphImpl::SanityCheck() const
actual_chunkindex.insert(idx);
auto chunk_feerate = m_entries[idx].m_main_chunk_feerate;
if (!last_chunk_feerate.IsEmpty()) {
assert(FeeRateCompare(last_chunk_feerate, chunk_feerate) >= 0);
assert(ByRatio{last_chunk_feerate} >= ByRatio{FeeFrac{chunk_feerate}});
}
last_chunk_feerate = chunk_feerate;
}
@@ -3338,7 +3337,7 @@ std::vector<TxGraph::Ref*> TxGraphImpl::Trim() noexcept
// We do not need to sort by cluster or within clusters, because due to the implicit
// dependency between consecutive linearization elements, no two transactions from the
// same Cluster will ever simultaneously be in the heap.
return a->m_chunk_feerate < b->m_chunk_feerate;
return ByRatioNegSize{a->m_chunk_feerate} < ByRatioNegSize{b->m_chunk_feerate};
};
/** Given a TrimTxData entry, find the representative of the partition it is in. */

6
src/util/feefrac.cpp
View File

@@ -42,17 +42,17 @@ std::partial_ordering CompareChunks(std::span<const FeeFrac> chunks0, std::span<
const auto slope_ap = point_p - point_a;
Assume(slope_ap.size > 0);
std::weak_ordering cmp = std::weak_ordering::equivalent;
auto cmp = std::strong_ordering::equivalent;
if (done_0 || done_1) {
// If a single side has no points left, act as if AB has slope tail_feerate(of 0).
Assume(!(done_0 && done_1));
cmp = FeeRateCompare(slope_ap, FeeFrac(0, 1));
cmp = ByRatio{slope_ap} <=> ByRatio{FeeFrac(0, 1)};
} else {
// If both sides have points left, compute B, and the slope of AB explicitly.
const FeeFrac& point_b = next_point(!unproc_side);
const auto slope_ab = point_b - point_a;
Assume(slope_ab.size >= slope_ap.size);
cmp = FeeRateCompare(slope_ap, slope_ab);
cmp = ByRatio{slope_ap} <=> ByRatio{slope_ab};
// If B and P have the same size, B can be marked as processed (in addition to P, see
// below), as we've already performed a comparison at this size.

154
src/util/feefrac.h
View File

@@ -12,29 +12,9 @@
#include <cstdint>
#include <vector>
/** Data structure storing a fee and size, ordered by increasing fee/size.
/** Data structure storing a fee and size.
*
* The size of a FeeFrac cannot be zero unless the fee is also zero.
*
* FeeFracs have a total ordering, first by increasing feerate (ratio of fee over size), and then
* by decreasing size. The empty FeeFrac (fee and size both 0) sorts last. So for example, the
* following FeeFracs are in sorted order:
*
* - fee=0 size=1 (feerate 0)
* - fee=1 size=2 (feerate 0.5)
* - fee=2 size=3 (feerate 0.667...)
* - fee=2 size=2 (feerate 1)
* - fee=1 size=1 (feerate 1)
* - fee=3 size=2 (feerate 1.5)
* - fee=2 size=1 (feerate 2)
* - fee=0 size=0 (undefined feerate)
*
* A FeeFrac is considered "better" if it sorts after another, by this ordering. All standard
* comparison operators (<=>, ==, !=, >, <, >=, <=) respect this ordering.
*
* The FeeRateCompare, and >> and << operators only compare feerate and treat equal feerate but
* different size as equivalent. The empty FeeFrac is neither lower or higher in feerate than any
* other.
*/
struct FeeFrac
{
@@ -153,35 +133,6 @@ struct FeeFrac
return a.fee == b.fee && a.size == b.size;
}
/** Compare two FeeFracs just by feerate. */
friend inline std::weak_ordering FeeRateCompare(const FeeFrac& a, const FeeFrac& b) noexcept
{
auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size);
return cross_a <=> cross_b;
}
/** Check if a FeeFrac object has strictly lower feerate than another. */
friend inline bool operator<<(const FeeFrac& a, const FeeFrac& b) noexcept
{
auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size);
return cross_a < cross_b;
}
/** Check if a FeeFrac object has strictly higher feerate than another. */
friend inline bool operator>>(const FeeFrac& a, const FeeFrac& b) noexcept
{
auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size);
return cross_a > cross_b;
}
/** Compare two FeeFracs. <, >, <=, and >= are auto-generated from this. */
friend inline std::strong_ordering operator<=>(const FeeFrac& a, const FeeFrac& b) noexcept
{
auto cross_a = Mul(a.fee, b.size), cross_b = Mul(b.fee, a.size);
if (cross_a == cross_b) return b.size <=> a.size;
return cross_a <=> cross_b;
}
/** Swap two FeeFracs. */
friend inline void swap(FeeFrac& a, FeeFrac& b) noexcept
{
@@ -255,4 +206,107 @@ using FeePerVSize = FeePerUnit<VSizeTag>;
struct WeightTag {};
using FeePerWeight = FeePerUnit<WeightTag>;
/** Wrapper around FeeFrac & derived types, which adds a feerate-based ordering which treats
* equal-feerate but distinct-size FeeFracs as equals.
*
* This is not included inside FeeFrac itself, because it is not a total ordering (as would be
* expected for built-in comparison operators).
*/
template<std::derived_from<FeeFrac> T>
class ByRatio
{
const T& m_feefrac;
public:
constexpr ByRatio(const T& feefrac) noexcept : m_feefrac{feefrac} {}
friend bool operator==(const ByRatio& a, const ByRatio& b) noexcept
{
auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size);
auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size);
return cross_a == cross_b;
}
// Note that we can use std::strong_ordering here, because even though FeeFrac{1,2} and
// FeeFrac{2,4} are distinct as FeeFracs, they are indistinguishable from ByRatio's perspective
// (operator== also treats them as equal).
friend std::strong_ordering operator<=>(const ByRatio& a, const ByRatio& b) noexcept
{
auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size);
auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size);
return cross_a <=> cross_b;
}
// Specialized versions for efficiency. GCC 15+ and Clang 11+ produce operator<=>-derived
// versions that are equally efficient as this at -O2, but earlier versions do not.
friend bool operator<(const ByRatio& a, const ByRatio& b) noexcept
{
auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size);
auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size);
return cross_a < cross_b;
}
friend bool operator>(const ByRatio& a, const ByRatio& b) noexcept
{
auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size);
auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size);
return cross_a > cross_b;
}
friend bool operator<=(const ByRatio& a, const ByRatio& b) noexcept
{
auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size);
auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size);
return cross_a <= cross_b;
}
friend bool operator>=(const ByRatio& a, const ByRatio& b) noexcept
{
auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size);
auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size);
return cross_a >= cross_b;
}
};
/** Wrapper around FeeFrac & derived types, which adds a total ordering which first sorts by feerate
* and then by reversed size (i.e., larger sizes come first).
*
* This is not included inside FeeFrac itself, because it is not the most natural behavior, so it
* is better to make code using it invoke this explicitly.
*
* The empty FeeFrac (fee and size both 0) sorts last. So for example, the following FeeFracs are
* in sorted order:
*
* - fee=0 size=1 (feerate 0)
* - fee=1 size=2 (feerate 0.5)
* - fee=2 size=3 (feerate 0.667...)
* - fee=2 size=2 (feerate 1)
* - fee=1 size=1 (feerate 1)
* - fee=3 size=2 (feerate 1.5)
* - fee=2 size=1 (feerate 2)
* - fee=0 size=0 (undefined feerate)
*/
template<std::derived_from<FeeFrac> T>
class ByRatioNegSize
{
const T& m_feefrac;
public:
constexpr ByRatioNegSize(const T& feefrac) noexcept : m_feefrac{feefrac} {}
friend bool operator==(const ByRatioNegSize& a, const ByRatioNegSize& b) noexcept
{
return a.m_feefrac == b.m_feefrac;
}
friend std::strong_ordering operator<=>(const ByRatioNegSize& a, const ByRatioNegSize& b) noexcept
{
auto cross_a = T::Mul(a.m_feefrac.fee, b.m_feefrac.size);
auto cross_b = T::Mul(b.m_feefrac.fee, a.m_feefrac.size);
auto cmp = cross_a <=> cross_b;
if (cmp != 0) return cmp;
return b.m_feefrac.size <=> a.m_feefrac.size;
}
// Support conversion back to underlying FeeFrac, which allows using std::max().
operator const T&() const noexcept { return m_feefrac; }
};
#endif // BITCOIN_UTIL_FEEFRAC_H