Skip to content

Commit e9c5aeb

Browse files
instagibbsinstagibbs
committed
test: Add tests for CompareFeerateDiagram and CheckConflictTopology
1 parent 588a98d commit e9c5aeb

File tree

1 file changed

+177
-40
lines changed

1 file changed

+177
-40
lines changed

‎src/test/rbf_tests.cpp‎

Lines changed: 177 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ static inline CTransactionRef make_tx(const std::vector<CTransactionRef>& inputs
3737
return MakeTransactionRef(tx);
3838
}
3939

40-
static void add_descendants(const CTransactionRef& tx, int32_t num_descendants, CTxMemPool& pool)
40+
static CTransactionRef add_descendants(const CTransactionRef& tx, int32_t num_descendants, CTxMemPool& pool)
4141
EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs)
4242
{
4343
AssertLockHeld(::cs_main);
@@ -50,6 +50,21 @@ static void add_descendants(const CTransactionRef& tx, int32_t num_descendants,
5050
pool.addUnchecked(entry.FromTx(next_tx));
5151
tx_to_spend = next_tx;
5252
}
53+
// Return last created tx
54+
return tx_to_spend;
55+
}
56+
57+
static CTransactionRef add_descendant_to_parents(const std::vector<CTransactionRef>& parents, CTxMemPool& pool)
58+
EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs)
59+
{
60+
AssertLockHeld(::cs_main);
61+
AssertLockHeld(pool.cs);
62+
TestMemPoolEntryHelper entry;
63+
// Assumes this isn't already spent in mempool
64+
auto child_tx = make_tx(/*inputs=*/parents, /*output_values=*/{50 * CENT});
65+
pool.addUnchecked(entry.FromTx(child_tx));
66+
// Return last created tx
67+
return child_tx;
5368
}
5469

5570
BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
@@ -89,58 +104,76 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
89104
const auto tx8 = make_tx(/*inputs=*/ {m_coinbase_txns[4]}, /*output_values=*/ {999 * CENT});
90105
pool.addUnchecked(entry.Fee(high_fee).FromTx(tx8));
91106

92-
const auto entry1 = pool.GetIter(tx1->GetHash()).value();
93-
const auto entry2 = pool.GetIter(tx2->GetHash()).value();
94-
const auto entry3 = pool.GetIter(tx3->GetHash()).value();
95-
const auto entry4 = pool.GetIter(tx4->GetHash()).value();
96-
const auto entry5 = pool.GetIter(tx5->GetHash()).value();
97-
const auto entry6 = pool.GetIter(tx6->GetHash()).value();
98-
const auto entry7 = pool.GetIter(tx7->GetHash()).value();
99-
const auto entry8 = pool.GetIter(tx8->GetHash()).value();
100-
101-
BOOST_CHECK_EQUAL(entry1->GetFee(), normal_fee);
102-
BOOST_CHECK_EQUAL(entry2->GetFee(), normal_fee);
103-
BOOST_CHECK_EQUAL(entry3->GetFee(), low_fee);
104-
BOOST_CHECK_EQUAL(entry4->GetFee(), high_fee);
105-
BOOST_CHECK_EQUAL(entry5->GetFee(), low_fee);
106-
BOOST_CHECK_EQUAL(entry6->GetFee(), low_fee);
107-
BOOST_CHECK_EQUAL(entry7->GetFee(), high_fee);
108-
BOOST_CHECK_EQUAL(entry8->GetFee(), high_fee);
109-
110-
CTxMemPool::setEntries set_12_normal{entry1, entry2};
111-
CTxMemPool::setEntries set_34_cpfp{entry3, entry4};
112-
CTxMemPool::setEntries set_56_low{entry5, entry6};
113-
CTxMemPool::setEntries all_entries{entry1, entry2, entry3, entry4, entry5, entry6, entry7, entry8};
107+
// Normal txs, will chain txns right before CheckConflictTopology test
108+
const auto tx9 = make_tx(/*inputs=*/ {m_coinbase_txns[5]}, /*output_values=*/ {995 * CENT});
109+
pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx9));
110+
const auto tx10 = make_tx(/*inputs=*/ {m_coinbase_txns[6]}, /*output_values=*/ {995 * CENT});
111+
pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx10));
112+
113+
// Will make these two parents of single child
114+
const auto tx11 = make_tx(/*inputs=*/ {m_coinbase_txns[7]}, /*output_values=*/ {995 * CENT});
115+
pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx11));
116+
const auto tx12 = make_tx(/*inputs=*/ {m_coinbase_txns[8]}, /*output_values=*/ {995 * CENT});
117+
pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx12));
118+
119+
const auto entry1_normal = pool.GetIter(tx1->GetHash()).value();
120+
const auto entry2_normal = pool.GetIter(tx2->GetHash()).value();
121+
const auto entry3_low = pool.GetIter(tx3->GetHash()).value();
122+
const auto entry4_high = pool.GetIter(tx4->GetHash()).value();
123+
const auto entry5_low = pool.GetIter(tx5->GetHash()).value();
124+
const auto entry6_low_prioritised = pool.GetIter(tx6->GetHash()).value();
125+
const auto entry7_high = pool.GetIter(tx7->GetHash()).value();
126+
const auto entry8_high = pool.GetIter(tx8->GetHash()).value();
127+
const auto entry9_unchained = pool.GetIter(tx9->GetHash()).value();
128+
const auto entry10_unchained = pool.GetIter(tx10->GetHash()).value();
129+
const auto entry11_unchained = pool.GetIter(tx11->GetHash()).value();
130+
const auto entry12_unchained = pool.GetIter(tx12->GetHash()).value();
131+
132+
BOOST_CHECK_EQUAL(entry1_normal->GetFee(), normal_fee);
133+
BOOST_CHECK_EQUAL(entry2_normal->GetFee(), normal_fee);
134+
BOOST_CHECK_EQUAL(entry3_low->GetFee(), low_fee);
135+
BOOST_CHECK_EQUAL(entry4_high->GetFee(), high_fee);
136+
BOOST_CHECK_EQUAL(entry5_low->GetFee(), low_fee);
137+
BOOST_CHECK_EQUAL(entry6_low_prioritised->GetFee(), low_fee);
138+
BOOST_CHECK_EQUAL(entry7_high->GetFee(), high_fee);
139+
BOOST_CHECK_EQUAL(entry8_high->GetFee(), high_fee);
140+
141+
CTxMemPool::setEntries set_12_normal{entry1_normal, entry2_normal};
142+
CTxMemPool::setEntries set_34_cpfp{entry3_low, entry4_high};
143+
CTxMemPool::setEntries set_56_low{entry5_low, entry6_low_prioritised};
144+
CTxMemPool::setEntries set_78_high{entry7_high, entry8_high};
145+
CTxMemPool::setEntries all_entries{entry1_normal, entry2_normal, entry3_low, entry4_high,
146+
entry5_low, entry6_low_prioritised, entry7_high, entry8_high};
114147
CTxMemPool::setEntries empty_set;
115148

116149
const auto unused_txid{GetRandHash()};
117150

118151
// Tests for PaysMoreThanConflicts
119152
// These tests use feerate, not absolute fee.
120153
BOOST_CHECK(PaysMoreThanConflicts(/*iters_conflicting=*/set_12_normal,
121-
/*replacement_feerate=*/CFeeRate(entry1->GetModifiedFee() + 1, entry1->GetTxSize() + 2),
154+
/*replacement_feerate=*/CFeeRate(entry1_normal->GetModifiedFee() + 1, entry1_normal->GetTxSize() + 2),
122155
/*txid=*/unused_txid).has_value());
123156
// Replacement must be strictly greater than the originals.
124-
BOOST_CHECK(PaysMoreThanConflicts(set_12_normal, CFeeRate(entry1->GetModifiedFee(), entry1->GetTxSize()), unused_txid).has_value());
125-
BOOST_CHECK(PaysMoreThanConflicts(set_12_normal, CFeeRate(entry1->GetModifiedFee() + 1, entry1->GetTxSize()), unused_txid) == std::nullopt);
157+
BOOST_CHECK(PaysMoreThanConflicts(set_12_normal, CFeeRate(entry1_normal->GetModifiedFee(), entry1_normal->GetTxSize()), unused_txid).has_value());
158+
BOOST_CHECK(PaysMoreThanConflicts(set_12_normal, CFeeRate(entry1_normal->GetModifiedFee() + 1, entry1_normal->GetTxSize()), unused_txid) == std::nullopt);
126159
// These tests use modified fees (including prioritisation), not base fees.
127-
BOOST_CHECK(PaysMoreThanConflicts({entry5}, CFeeRate(entry5->GetModifiedFee() + 1, entry5->GetTxSize()), unused_txid) == std::nullopt);
128-
BOOST_CHECK(PaysMoreThanConflicts({entry6}, CFeeRate(entry6->GetFee() + 1, entry6->GetTxSize()), unused_txid).has_value());
129-
BOOST_CHECK(PaysMoreThanConflicts({entry6}, CFeeRate(entry6->GetModifiedFee() + 1, entry6->GetTxSize()), unused_txid) == std::nullopt);
160+
BOOST_CHECK(PaysMoreThanConflicts({entry5_low}, CFeeRate(entry5_low->GetModifiedFee() + 1, entry5_low->GetTxSize()), unused_txid) == std::nullopt);
161+
BOOST_CHECK(PaysMoreThanConflicts({entry6_low_prioritised}, CFeeRate(entry6_low_prioritised->GetFee() + 1, entry6_low_prioritised->GetTxSize()), unused_txid).has_value());
162+
BOOST_CHECK(PaysMoreThanConflicts({entry6_low_prioritised}, CFeeRate(entry6_low_prioritised->GetModifiedFee() + 1, entry6_low_prioritised->GetTxSize()), unused_txid) == std::nullopt);
130163
// PaysMoreThanConflicts checks individual feerate, not ancestor feerate. This test compares
131-
// replacement_feerate and entry4's feerate, which are the same. The replacement_feerate is
132-
// considered too low even though entry4 has a low ancestor feerate.
133-
BOOST_CHECK(PaysMoreThanConflicts(set_34_cpfp, CFeeRate(entry4->GetModifiedFee(), entry4->GetTxSize()), unused_txid).has_value());
164+
// replacement_feerate and entry4_high's feerate, which are the same. The replacement_feerate is
165+
// considered too low even though entry4_high has a low ancestor feerate.
166+
BOOST_CHECK(PaysMoreThanConflicts(set_34_cpfp, CFeeRate(entry4_high->GetModifiedFee(), entry4_high->GetTxSize()), unused_txid).has_value());
134167

135168
// Tests for EntriesAndTxidsDisjoint
136169
BOOST_CHECK(EntriesAndTxidsDisjoint(empty_set, {tx1->GetHash()}, unused_txid) == std::nullopt);
137170
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx3->GetHash()}, unused_txid) == std::nullopt);
138-
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx2->GetHash()}, unused_txid).has_value());
171+
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2_normal}, {tx2->GetHash()}, unused_txid).has_value());
139172
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx1->GetHash()}, unused_txid).has_value());
140173
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx2->GetHash()}, unused_txid).has_value());
141174
// EntriesAndTxidsDisjoint does not calculate descendants of iters_conflicting; it uses whatever
142-
// the caller passed in. As such, no error is returned even though entry2 is a descendant of tx1.
143-
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx1->GetHash()}, unused_txid) == std::nullopt);
175+
// the caller passed in. As such, no error is returned even though entry2_normal is a descendant of tx1.
176+
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2_normal}, {tx1->GetHash()}, unused_txid) == std::nullopt);
144177

145178
// Tests for PaysForRBF
146179
const CFeeRate incremental_relay_feerate{DEFAULT_INCREMENTAL_RELAY_FEE};
@@ -163,8 +196,8 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
163196
BOOST_CHECK(PaysForRBF(low_fee, high_fee + 99999999, 99999999, incremental_relay_feerate, unused_txid) == std::nullopt);
164197

165198
// Tests for GetEntriesForConflicts
166-
CTxMemPool::setEntries all_parents{entry1, entry3, entry5, entry7, entry8};
167-
CTxMemPool::setEntries all_children{entry2, entry4, entry6};
199+
CTxMemPool::setEntries all_parents{entry1_normal, entry3_low, entry5_low, entry7_high, entry8_high};
200+
CTxMemPool::setEntries all_children{entry2_normal, entry4_high, entry6_low_prioritised};
168201
const std::vector<CTransactionRef> parent_inputs({m_coinbase_txns[0], m_coinbase_txns[1], m_coinbase_txns[2],
169202
m_coinbase_txns[3], m_coinbase_txns[4]});
170203
const auto conflicts_with_parents = make_tx(parent_inputs, {50 * CENT});
@@ -215,15 +248,119 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
215248
BOOST_CHECK(HasNoNewUnconfirmed(/*tx=*/ *spends_unconfirmed.get(),
216249
/*pool=*/ pool,
217250
/*iters_conflicting=*/ all_entries) == std::nullopt);
218-
BOOST_CHECK(HasNoNewUnconfirmed(*spends_unconfirmed.get(), pool, {entry2}) == std::nullopt);
251+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_unconfirmed.get(), pool, {entry2_normal}) == std::nullopt);
219252
BOOST_CHECK(HasNoNewUnconfirmed(*spends_unconfirmed.get(), pool, empty_set).has_value());
220253

221254
const auto spends_new_unconfirmed = make_tx({tx1, tx8}, {36 * CENT});
222-
BOOST_CHECK(HasNoNewUnconfirmed(*spends_new_unconfirmed.get(), pool, {entry2}).has_value());
255+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_new_unconfirmed.get(), pool, {entry2_normal}).has_value());
223256
BOOST_CHECK(HasNoNewUnconfirmed(*spends_new_unconfirmed.get(), pool, all_entries).has_value());
224257

225258
const auto spends_conflicting_confirmed = make_tx({m_coinbase_txns[0], m_coinbase_txns[1]}, {45 * CENT});
226-
BOOST_CHECK(HasNoNewUnconfirmed(*spends_conflicting_confirmed.get(), pool, {entry1, entry3}) == std::nullopt);
259+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_conflicting_confirmed.get(), pool, {entry1_normal, entry3_low}) == std::nullopt);
260+
261+
// Tests for CheckConflictTopology
262+
263+
// Tx4 has 23 descendants
264+
BOOST_CHECK(pool.CheckConflictTopology(set_34_cpfp).has_value());
265+
266+
// No descendants yet
267+
BOOST_CHECK(pool.CheckConflictTopology({entry9_unchained}) == std::nullopt);
268+
269+
// Add 1 descendant, still ok
270+
add_descendants(tx9, 1, pool);
271+
BOOST_CHECK(pool.CheckConflictTopology({entry9_unchained}) == std::nullopt);
272+
273+
// N direct conflicts; ok
274+
BOOST_CHECK(pool.CheckConflictTopology({entry9_unchained, entry10_unchained, entry11_unchained}) == std::nullopt);
275+
276+
// Add 1 descendant, still ok, even if it's considered a direct conflict as well
277+
const auto child_tx = add_descendants(tx10, 1, pool);
278+
const auto entry10_child = pool.GetIter(child_tx->GetHash()).value();
279+
BOOST_CHECK(pool.CheckConflictTopology({entry9_unchained, entry10_unchained, entry11_unchained}) == std::nullopt);
280+
BOOST_CHECK(pool.CheckConflictTopology({entry9_unchained, entry10_unchained, entry11_unchained, entry10_child}) == std::nullopt);
281+
282+
// One more, size 3 cluster too much
283+
const auto grand_child_tx = add_descendants(child_tx, 1, pool);
284+
const auto entry10_grand_child = pool.GetIter(grand_child_tx->GetHash()).value();
285+
BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry9_unchained, entry10_unchained, entry11_unchained}).value(), strprintf("%s has 2 descendants, max 1 allowed", entry10_unchained->GetSharedTx()->GetHash().ToString()));
286+
// even if direct conflict is descendent itself
287+
BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry9_unchained, entry10_grand_child, entry11_unchained}).value(), strprintf("%s has 2 ancestors, max 1 allowed", entry10_grand_child->GetSharedTx()->GetHash().ToString()));
288+
289+
// Make a single child from two singleton parents
290+
const auto two_parent_child_tx = add_descendant_to_parents({tx11, tx12}, pool);
291+
const auto entry_two_parent_child = pool.GetIter(two_parent_child_tx->GetHash()).value();
292+
BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry11_unchained}).value(), strprintf("%s is not the only parent of child %s", entry11_unchained->GetSharedTx()->GetHash().ToString(), entry_two_parent_child->GetSharedTx()->GetHash().ToString()));
293+
BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry12_unchained}).value(), strprintf("%s is not the only parent of child %s", entry12_unchained->GetSharedTx()->GetHash().ToString(), entry_two_parent_child->GetSharedTx()->GetHash().ToString()));
294+
BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry_two_parent_child}).value(), strprintf("%s has 2 ancestors, max 1 allowed", entry_two_parent_child->GetSharedTx()->GetHash().ToString()));
295+
}
296+
297+
BOOST_AUTO_TEST_CASE(feerate_diagram_utilities)
298+
{
299+
// Sanity check the correctness of the feerate diagram comparison.
300+
301+
// A strictly better case.
302+
std::vector<FeeFrac> old_diagram{{FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}};
303+
std::vector<FeeFrac> new_diagram{{FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1050, 400}}};
304+
305+
BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
306+
307+
// Incomparable diagrams
308+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
309+
new_diagram = {FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1000, 400}};
310+
311+
BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered);
312+
313+
// Strictly better but smaller size.
314+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
315+
new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 300}};
316+
317+
BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
318+
319+
// New diagram is strictly better due to the first chunk, even though
320+
// second chunk contributes no fees
321+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
322+
new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 100}, FeeFrac{1100, 200}};
323+
324+
BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
325+
326+
// Feerate of first new chunk is better with, but second chunk is worse
327+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
328+
new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{999, 350}, FeeFrac{1150, 1000}};
329+
330+
BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered);
331+
332+
// If we make the second chunk slightly better, the new diagram now wins.
333+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
334+
new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{1000, 350}, FeeFrac{1150, 500}};
335+
336+
BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
337+
338+
// Identical diagrams, cannot be strictly better
339+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
340+
new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
341+
342+
BOOST_CHECK(std::is_eq(CompareFeerateDiagram(old_diagram, new_diagram)));
343+
344+
// Same aggregate fee, but different total size (trigger single tail fee check step)
345+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}};
346+
new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
347+
348+
// No change in evaluation when tail check needed.
349+
BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram)));
350+
351+
// Padding works on either argument
352+
BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram)));
353+
354+
// Trigger multiple tail fee check steps
355+
old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}};
356+
new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}};
357+
358+
BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram)));
359+
BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram)));
360+
361+
// Multiple tail fee check steps, unordered result
362+
new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}, FeeFrac{1051, 403}};
363+
BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered);
227364
}
228365

229366
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)