Skip to content

Commit 41205bf

Browse files
glozowglozow
committed
Merge bitcoin/bitcoin#25674: add unit tests for RBF rules in isolation
c320cdd [unit tests] individual RBF Rules in isolation (glozow) Pull request description: Test each RBF rule more thoroughly and in isolation so we're not relying on things like overall mempool acceptance logic, ordering of mempool checks, RPC results, etc. RBF was pretty recently refactored out, so there isn't much unit test coverage. From https://marcofalke.github.io/btc_cov/test_bitcoin.coverage/src/policy/rbf.cpp.gcov.html: ![image](https://user-images.githubusercontent.com/25183001/180783280-6777f4b4-ef95-462a-b414-1a9e268836a6.png) ACKs for top commit: instagibbs: reACK bitcoin/bitcoin@c320cdd jonatack: ACK c320cdd w0xlt: ACK bitcoin/bitcoin@c320cdd Tree-SHA512: dab555214496255801b9ea92b7bf708bba1ff23edf055c85e29be5eab7d7a863440ee19588aacdce54b2c03feaa4b5963eb159ed89473560bd228737cbfec160
2 parents 62c8646 + c320cdd commit 41205bf

File tree

2 files changed

+231
-0
lines changed

2 files changed

+231
-0
lines changed

‎src/Makefile.test.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ BITCOIN_TESTS =\
117117
test/prevector_tests.cpp \
118118
test/raii_event_tests.cpp \
119119
test/random_tests.cpp \
120+
test/rbf_tests.cpp \
120121
test/rest_tests.cpp \
121122
test/reverselock_tests.cpp \
122123
test/rpc_tests.cpp \

‎src/test/rbf_tests.cpp

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
// Copyright (c) 2021 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
#include <policy/rbf.h>
5+
#include <random.h>
6+
#include <txmempool.h>
7+
#include <util/system.h>
8+
#include <util/time.h>
9+
10+
#include <test/util/setup_common.h>
11+
12+
#include <boost/test/unit_test.hpp>
13+
#include <optional>
14+
#include <vector>
15+
16+
BOOST_FIXTURE_TEST_SUITE(rbf_tests, TestingSetup)
17+
18+
static inline CTransactionRef make_tx(const std::vector<CTransactionRef>& inputs,
19+
const std::vector<CAmount>& output_values)
20+
{
21+
CMutableTransaction tx = CMutableTransaction();
22+
tx.vin.resize(inputs.size());
23+
tx.vout.resize(output_values.size());
24+
for (size_t i = 0; i < inputs.size(); ++i) {
25+
tx.vin[i].prevout.hash = inputs[i]->GetHash();
26+
tx.vin[i].prevout.n = 0;
27+
// Add a witness so wtxid != txid
28+
CScriptWitness witness;
29+
witness.stack.push_back(std::vector<unsigned char>(i + 10));
30+
tx.vin[i].scriptWitness = witness;
31+
}
32+
for (size_t i = 0; i < output_values.size(); ++i) {
33+
tx.vout[i].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
34+
tx.vout[i].nValue = output_values[i];
35+
}
36+
return MakeTransactionRef(tx);
37+
}
38+
39+
static void add_descendants(const CTransactionRef& tx, int32_t num_descendants, CTxMemPool& pool)
40+
EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs)
41+
{
42+
AssertLockHeld(::cs_main);
43+
AssertLockHeld(pool.cs);
44+
TestMemPoolEntryHelper entry;
45+
// Assumes this isn't already spent in mempool
46+
auto tx_to_spend = tx;
47+
for (int32_t i{0}; i < num_descendants; ++i) {
48+
auto next_tx = make_tx(/*inputs=*/{tx_to_spend}, /*output_values=*/{(50 - i) * CENT});
49+
pool.addUnchecked(entry.FromTx(next_tx));
50+
tx_to_spend = next_tx;
51+
}
52+
}
53+
54+
BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
55+
{
56+
CTxMemPool& pool = *Assert(m_node.mempool);
57+
LOCK2(::cs_main, pool.cs);
58+
TestMemPoolEntryHelper entry;
59+
60+
const CAmount low_fee{CENT/100};
61+
const CAmount normal_fee{CENT/10};
62+
const CAmount high_fee{CENT};
63+
64+
// Create a parent tx1 and child tx2 with normal fees:
65+
const auto tx1 = make_tx(/*inputs=*/ {m_coinbase_txns[0]}, /*output_values=*/ {10 * COIN});
66+
pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx1));
67+
const auto tx2 = make_tx(/*inputs=*/ {tx1}, /*output_values=*/ {995 * CENT});
68+
pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx2));
69+
70+
// Create a low-feerate parent tx3 and high-feerate child tx4 (cpfp)
71+
const auto tx3 = make_tx(/*inputs=*/ {m_coinbase_txns[1]}, /*output_values=*/ {1099 * CENT});
72+
pool.addUnchecked(entry.Fee(low_fee).FromTx(tx3));
73+
const auto tx4 = make_tx(/*inputs=*/ {tx3}, /*output_values=*/ {999 * CENT});
74+
pool.addUnchecked(entry.Fee(high_fee).FromTx(tx4));
75+
76+
// Create a parent tx5 and child tx6 where both have very low fees
77+
const auto tx5 = make_tx(/*inputs=*/ {m_coinbase_txns[2]}, /*output_values=*/ {1099 * CENT});
78+
pool.addUnchecked(entry.Fee(low_fee).FromTx(tx5));
79+
const auto tx6 = make_tx(/*inputs=*/ {tx3}, /*output_values=*/ {1098 * CENT});
80+
pool.addUnchecked(entry.Fee(low_fee).FromTx(tx6));
81+
// Make tx6's modified fee much higher than its base fee. This should cause it to pass
82+
// the fee-related checks despite being low-feerate.
83+
pool.PrioritiseTransaction(tx6->GetHash(), 1 * COIN);
84+
85+
// Two independent high-feerate transactions, tx7 and tx8
86+
const auto tx7 = make_tx(/*inputs=*/ {m_coinbase_txns[3]}, /*output_values=*/ {999 * CENT});
87+
pool.addUnchecked(entry.Fee(high_fee).FromTx(tx7));
88+
const auto tx8 = make_tx(/*inputs=*/ {m_coinbase_txns[4]}, /*output_values=*/ {999 * CENT});
89+
pool.addUnchecked(entry.Fee(high_fee).FromTx(tx8));
90+
91+
const auto entry1 = pool.GetIter(tx1->GetHash()).value();
92+
const auto entry2 = pool.GetIter(tx2->GetHash()).value();
93+
const auto entry3 = pool.GetIter(tx3->GetHash()).value();
94+
const auto entry4 = pool.GetIter(tx4->GetHash()).value();
95+
const auto entry5 = pool.GetIter(tx5->GetHash()).value();
96+
const auto entry6 = pool.GetIter(tx6->GetHash()).value();
97+
const auto entry7 = pool.GetIter(tx7->GetHash()).value();
98+
const auto entry8 = pool.GetIter(tx8->GetHash()).value();
99+
100+
BOOST_CHECK_EQUAL(entry1->GetFee(), normal_fee);
101+
BOOST_CHECK_EQUAL(entry2->GetFee(), normal_fee);
102+
BOOST_CHECK_EQUAL(entry3->GetFee(), low_fee);
103+
BOOST_CHECK_EQUAL(entry4->GetFee(), high_fee);
104+
BOOST_CHECK_EQUAL(entry5->GetFee(), low_fee);
105+
BOOST_CHECK_EQUAL(entry6->GetFee(), low_fee);
106+
BOOST_CHECK_EQUAL(entry7->GetFee(), high_fee);
107+
BOOST_CHECK_EQUAL(entry8->GetFee(), high_fee);
108+
109+
CTxMemPool::setEntries set_12_normal{entry1, entry2};
110+
CTxMemPool::setEntries set_34_cpfp{entry3, entry4};
111+
CTxMemPool::setEntries set_56_low{entry5, entry6};
112+
CTxMemPool::setEntries all_entries{entry1, entry2, entry3, entry4, entry5, entry6, entry7, entry8};
113+
CTxMemPool::setEntries empty_set;
114+
115+
const auto unused_txid{GetRandHash()};
116+
117+
// Tests for PaysMoreThanConflicts
118+
// These tests use feerate, not absolute fee.
119+
BOOST_CHECK(PaysMoreThanConflicts(/*iters_conflicting=*/set_12_normal,
120+
/*replacement_feerate=*/CFeeRate(entry1->GetModifiedFee() + 1, entry1->GetTxSize() + 2),
121+
/*txid=*/unused_txid).has_value());
122+
// Replacement must be strictly greater than the originals.
123+
BOOST_CHECK(PaysMoreThanConflicts(set_12_normal, CFeeRate(entry1->GetModifiedFee(), entry1->GetTxSize()), unused_txid).has_value());
124+
BOOST_CHECK(PaysMoreThanConflicts(set_12_normal, CFeeRate(entry1->GetModifiedFee() + 1, entry1->GetTxSize()), unused_txid) == std::nullopt);
125+
// These tests use modified fees (including prioritisation), not base fees.
126+
BOOST_CHECK(PaysMoreThanConflicts({entry5}, CFeeRate(entry5->GetModifiedFee() + 1, entry5->GetTxSize()), unused_txid) == std::nullopt);
127+
BOOST_CHECK(PaysMoreThanConflicts({entry6}, CFeeRate(entry6->GetFee() + 1, entry6->GetTxSize()), unused_txid).has_value());
128+
BOOST_CHECK(PaysMoreThanConflicts({entry6}, CFeeRate(entry6->GetModifiedFee() + 1, entry6->GetTxSize()), unused_txid) == std::nullopt);
129+
// PaysMoreThanConflicts checks individual feerate, not ancestor feerate. This test compares
130+
// replacement_feerate and entry4's feerate, which are the same. The replacement_feerate is
131+
// considered too low even though entry4 has a low ancestor feerate.
132+
BOOST_CHECK(PaysMoreThanConflicts(set_34_cpfp, CFeeRate(entry4->GetModifiedFee(), entry4->GetTxSize()), unused_txid).has_value());
133+
134+
// Tests for EntriesAndTxidsDisjoint
135+
BOOST_CHECK(EntriesAndTxidsDisjoint(empty_set, {tx1->GetHash()}, unused_txid) == std::nullopt);
136+
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx3->GetHash()}, unused_txid) == std::nullopt);
137+
// EntriesAndTxidsDisjoint uses txids, not wtxids.
138+
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx2->GetWitnessHash()}, unused_txid) == std::nullopt);
139+
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx2->GetHash()}, unused_txid).has_value());
140+
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx1->GetHash()}, unused_txid).has_value());
141+
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx2->GetHash()}, unused_txid).has_value());
142+
// EntriesAndTxidsDisjoint does not calculate descendants of iters_conflicting; it uses whatever
143+
// the caller passed in. As such, no error is returned even though entry2 is a descendant of tx1.
144+
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx1->GetHash()}, unused_txid) == std::nullopt);
145+
146+
// Tests for PaysForRBF
147+
const CFeeRate incremental_relay_feerate{DEFAULT_INCREMENTAL_RELAY_FEE};
148+
const CFeeRate higher_relay_feerate{2 * DEFAULT_INCREMENTAL_RELAY_FEE};
149+
// Must pay at least as much as the original.
150+
BOOST_CHECK(PaysForRBF(/*original_fees=*/high_fee,
151+
/*replacement_fees=*/high_fee,
152+
/*replacement_vsize=*/1,
153+
/*relay_fee=*/CFeeRate(0),
154+
/*txid=*/unused_txid)
155+
== std::nullopt);
156+
BOOST_CHECK(PaysForRBF(high_fee, high_fee - 1, 1, CFeeRate(0), unused_txid).has_value());
157+
BOOST_CHECK(PaysForRBF(high_fee + 1, high_fee, 1, CFeeRate(0), unused_txid).has_value());
158+
// Additional fees must cover the replacement's vsize at incremental relay fee
159+
BOOST_CHECK(PaysForRBF(high_fee, high_fee + 1, 2, incremental_relay_feerate, unused_txid).has_value());
160+
BOOST_CHECK(PaysForRBF(high_fee, high_fee + 2, 2, incremental_relay_feerate, unused_txid) == std::nullopt);
161+
BOOST_CHECK(PaysForRBF(high_fee, high_fee + 2, 2, higher_relay_feerate, unused_txid).has_value());
162+
BOOST_CHECK(PaysForRBF(high_fee, high_fee + 4, 2, higher_relay_feerate, unused_txid) == std::nullopt);
163+
BOOST_CHECK(PaysForRBF(low_fee, high_fee, 99999999, incremental_relay_feerate, unused_txid).has_value());
164+
BOOST_CHECK(PaysForRBF(low_fee, high_fee + 99999999, 99999999, incremental_relay_feerate, unused_txid) == std::nullopt);
165+
166+
// Tests for GetEntriesForConflicts
167+
CTxMemPool::setEntries all_parents{entry1, entry3, entry5, entry7, entry8};
168+
CTxMemPool::setEntries all_children{entry2, entry4, entry6};
169+
const std::vector<CTransactionRef> parent_inputs({m_coinbase_txns[0], m_coinbase_txns[1], m_coinbase_txns[2],
170+
m_coinbase_txns[3], m_coinbase_txns[4]});
171+
const auto conflicts_with_parents = make_tx(parent_inputs, {50 * CENT});
172+
CTxMemPool::setEntries all_conflicts;
173+
BOOST_CHECK(GetEntriesForConflicts(/*tx=*/ *conflicts_with_parents.get(),
174+
/*pool=*/ pool,
175+
/*iters_conflicting=*/ all_parents,
176+
/*all_conflicts=*/ all_conflicts) == std::nullopt);
177+
BOOST_CHECK(all_conflicts == all_entries);
178+
auto conflicts_size = all_conflicts.size();
179+
all_conflicts.clear();
180+
181+
add_descendants(tx2, 23, pool);
182+
BOOST_CHECK(GetEntriesForConflicts(*conflicts_with_parents.get(), pool, all_parents, all_conflicts) == std::nullopt);
183+
conflicts_size += 23;
184+
BOOST_CHECK_EQUAL(all_conflicts.size(), conflicts_size);
185+
all_conflicts.clear();
186+
187+
add_descendants(tx4, 23, pool);
188+
BOOST_CHECK(GetEntriesForConflicts(*conflicts_with_parents.get(), pool, all_parents, all_conflicts) == std::nullopt);
189+
conflicts_size += 23;
190+
BOOST_CHECK_EQUAL(all_conflicts.size(), conflicts_size);
191+
all_conflicts.clear();
192+
193+
add_descendants(tx6, 23, pool);
194+
BOOST_CHECK(GetEntriesForConflicts(*conflicts_with_parents.get(), pool, all_parents, all_conflicts) == std::nullopt);
195+
conflicts_size += 23;
196+
BOOST_CHECK_EQUAL(all_conflicts.size(), conflicts_size);
197+
all_conflicts.clear();
198+
199+
add_descendants(tx7, 23, pool);
200+
BOOST_CHECK(GetEntriesForConflicts(*conflicts_with_parents.get(), pool, all_parents, all_conflicts) == std::nullopt);
201+
conflicts_size += 23;
202+
BOOST_CHECK_EQUAL(all_conflicts.size(), conflicts_size);
203+
BOOST_CHECK_EQUAL(all_conflicts.size(), 100);
204+
all_conflicts.clear();
205+
206+
// Exceeds maximum number of conflicts.
207+
add_descendants(tx8, 1, pool);
208+
BOOST_CHECK(GetEntriesForConflicts(*conflicts_with_parents.get(), pool, all_parents, all_conflicts).has_value());
209+
210+
// Tests for HasNoNewUnconfirmed
211+
const auto spends_unconfirmed = make_tx({tx1}, {36 * CENT});
212+
for (const auto& input : spends_unconfirmed->vin) {
213+
// Spends unconfirmed inputs.
214+
BOOST_CHECK(pool.exists(GenTxid::Txid(input.prevout.hash)));
215+
}
216+
BOOST_CHECK(HasNoNewUnconfirmed(/*tx=*/ *spends_unconfirmed.get(),
217+
/*pool=*/ pool,
218+
/*iters_conflicting=*/ all_entries) == std::nullopt);
219+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_unconfirmed.get(), pool, {entry2}) == std::nullopt);
220+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_unconfirmed.get(), pool, empty_set).has_value());
221+
222+
const auto spends_new_unconfirmed = make_tx({tx1, tx8}, {36 * CENT});
223+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_new_unconfirmed.get(), pool, {entry2}).has_value());
224+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_new_unconfirmed.get(), pool, all_entries).has_value());
225+
226+
const auto spends_conflicting_confirmed = make_tx({m_coinbase_txns[0], m_coinbase_txns[1]}, {45 * CENT});
227+
BOOST_CHECK(HasNoNewUnconfirmed(*spends_conflicting_confirmed.get(), pool, {entry1, entry3}) == std::nullopt);
228+
}
229+
230+
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)