Skip to content

Commit ba7c67f

Browse files
committed
Merge bitcoin/bitcoin#29879: fuzz: explicitly cap the vsize of RBFs for diagram checks
016ed24 fuzz: explicitly cap the vsize of RBFs for diagram checks (Greg Sanders) Pull request description: In master we are hitting a case where vsize transactions much larger than max standard size are causing an overflow in not-yet-exposed RBF diagram checking code: bitcoin/bitcoin#29757 (comment) `ConsumeTxMemPoolEntry` is creating entries with tens of thousands of sigops cost, causing the resulting RBFs to be "overly large". To fix this I cause the fuzz test to stop adding transactions to the mempool when we reach a potential overflow of `int32_t`. ACKs for top commit: glozow: ACK 016ed24 marcofleon: ACK 016ed24. I ran libFuzzer on `package_rbf` on the current master branch until the overflow was encountered. Then I built the PR branch and ran the fuzzer using the crash input. Tree-SHA512: b3ffc98d2c4598eb3010edd58b9370aab1441aafbb1044c83b2b90c17dfe9135b8de9dba475dd0108863c1ffedede443cd978e95231a41cf1f0715629197fa51
2 parents 67c0d93 + 016ed24 commit ba7c67f

File tree

1 file changed

+23
-4
lines changed

1 file changed

+23
-4
lines changed

src/test/fuzz/rbf.cpp

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,26 +107,46 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
107107
std::vector<CTransaction> mempool_txs;
108108
size_t iter{0};
109109

110+
int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(1, 1000000);
111+
112+
// Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow
113+
// Add replacement_vsize since this is added to new diagram during RBF check
114+
int64_t running_vsize_total{replacement_vsize};
115+
110116
LOCK2(cs_main, pool.cs);
111117

112118
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS)
113119
{
114120
// Make sure txns only have one input, and that a unique input is given to avoid circular references
115121
std::optional<CMutableTransaction> parent = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS);
116122
if (!parent) {
117-
continue;
123+
return;
118124
}
119125
assert(iter <= g_outpoints.size());
120126
parent->vin.resize(1);
121127
parent->vin[0].prevout = g_outpoints[iter++];
122128

123129
mempool_txs.emplace_back(*parent);
124-
pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()));
130+
const auto parent_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back());
131+
running_vsize_total += parent_entry.GetTxSize();
132+
if (running_vsize_total > std::numeric_limits<int32_t>::max()) {
133+
// We aren't adding this final tx to mempool, so we don't want to conflict with it
134+
mempool_txs.pop_back();
135+
break;
136+
}
137+
pool.addUnchecked(parent_entry);
125138
if (fuzzed_data_provider.ConsumeBool() && !child->vin.empty()) {
126139
child->vin[0].prevout = COutPoint{mempool_txs.back().GetHash(), 0};
127140
}
128141
mempool_txs.emplace_back(*child);
129-
pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()));
142+
const auto child_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back());
143+
running_vsize_total += child_entry.GetTxSize();
144+
if (running_vsize_total > std::numeric_limits<int32_t>::max()) {
145+
// We aren't adding this final tx to mempool, so we don't want to conflict with it
146+
mempool_txs.pop_back();
147+
break;
148+
}
149+
pool.addUnchecked(child_entry);
130150

131151
if (fuzzed_data_provider.ConsumeBool()) {
132152
pool.PrioritiseTransaction(mempool_txs.back().GetHash().ToUint256(), fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(-100000, 100000));
@@ -149,7 +169,6 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
149169

150170
// Calculate the feerate diagrams for a replacement.
151171
CAmount replacement_fees = ConsumeMoney(fuzzed_data_provider);
152-
int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(1, 1000000);
153172
auto calc_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)};
154173

155174
if (calc_results.has_value()) {

0 commit comments

Comments
 (0)