Skip to content

Commit

Permalink
fuzz: txorphan tests fixups
Browse files Browse the repository at this point in the history
Adds the following fixups in txorphan fuzz tests:

- Don't bond the output count of the created orphans based on the number of available coins
- Allow duplicate inputs, when applicable, but don't store duplicate outpoints

Rationale
---------

The way the test is currently written, duplicate inputs are allowed based on a random flag (`duplicate_input`).
If the flag is unset, upon selecting an outpoint as input for a new transaction, the input is popped to prevent re-selection,
and later re-added to the collection (once all inputs have been picked). However, the re-addition to the collection is performed independently of whether the flag was set or not.
This means that, if the flag is set, the selected inputs are duplicated which in turn makes these inputs more likely to be re-picked in the following iteration of the loop.

Additionally, both the input and output count of the transaction and bonded to the number of available outpoints. This makes sense for the former, but the latter shouldn't be.
  • Loading branch information
sr-gi committed Apr 26, 2024
1 parent 3c88eac commit 6998f02
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/test/fuzz/txorphan.cpp
Expand Up @@ -38,6 +38,9 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)

TxOrphanage orphanage;
std::vector<COutPoint> outpoints;
std::vector<COutPoint> selected_outpoints;
auto nSequence = CTxIn::SEQUENCE_FINAL;

// initial outpoints used to construct transactions later
for (uint8_t i = 0; i < 4; i++) {
outpoints.emplace_back(Txid::FromUint256(uint256{i}), 0);
Expand All @@ -51,14 +54,16 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
const CTransactionRef tx = [&] {
CMutableTransaction tx_mut;
const auto num_in = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(1, outpoints.size());
const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(1, outpoints.size());
const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(1, 256);
// pick unique outpoints from outpoints as input
for (uint32_t i = 0; i < num_in; i++) {
auto& prevout = PickValue(fuzzed_data_provider, outpoints);
tx_mut.vin.emplace_back(prevout);
// make every txid unique so multiple iteration don't potentially create the same outpoint
tx_mut.vin.emplace_back(prevout, CScript{}, nSequence--);
// pop the picked outpoint if duplicate input is not allowed
if (!duplicate_input) {
std::swap(prevout, outpoints.back());
selected_outpoints.push_back(outpoints.back());
outpoints.pop_back();
}
}
Expand All @@ -67,8 +72,10 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
tx_mut.vout.emplace_back(CAmount{0}, CScript{});
}
// restore previously popped outpoints
for (auto& in : tx_mut.vin) {
outpoints.push_back(in.prevout);
while (!selected_outpoints.empty()) {
assert(!duplicate_input);
outpoints.push_back(selected_outpoints.back());
selected_outpoints.pop_back();
}
auto new_tx = MakeTransactionRef(tx_mut);
// add newly constructed transaction to outpoints
Expand Down

0 comments on commit 6998f02

Please sign in to comment.