Skip to content

Commit 7524fcf

Browse files
committed
Merge bitcoin#28372: fuzz: coinselection, improve min_viable_change/change_output_size
cd81007 fuzz: coinselection, improve `min_viable_change`/`change_output_size` (brunoerg) Pull request description: Instead of "randomly" fuzzing `min_viable_change` and `change_output_size`, and since they're correlated, this PR changes the approach to fuzz them according to the logic in `CreateTransactionInternal`. ACKs for top commit: murchandamus: ACK cd81007 achow101: ACK cd81007 furszy: Code ACK cd81007 Tree-SHA512: 4539b469f00cdf666078d80c07ed062726f804e390400348148cd3092db9cdc178c6d00ead39aef19acf97badfb6576ce23546d8967387e81c5398d52d7f4404
2 parents 3a0f54d + cd81007 commit 7524fcf

File tree

1 file changed

+7
-5
lines changed

1 file changed

+7
-5
lines changed

src/wallet/test/fuzz/coinselection.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ FUZZ_TARGET(coinselection)
8686
const CFeeRate effective_fee_rate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)};
8787
// Discard feerate must be at least dust relay feerate
8888
const CFeeRate discard_fee_rate{fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(DUST_RELAY_TX_FEE, COIN)};
89-
const CAmount min_viable_change{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)};
9089
const CAmount target{fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(1, MAX_MONEY)};
9190
const bool subtract_fee_outputs{fuzzed_data_provider.ConsumeBool()};
9291

@@ -95,12 +94,15 @@ FUZZ_TARGET(coinselection)
9594
coin_params.m_subtract_fee_outputs = subtract_fee_outputs;
9695
coin_params.m_long_term_feerate = long_term_fee_rate;
9796
coin_params.m_effective_feerate = effective_fee_rate;
98-
coin_params.min_viable_change = min_viable_change;
99-
coin_params.change_output_size = fuzzed_data_provider.ConsumeIntegralInRange<int>(10, 1000);
97+
coin_params.change_output_size = fuzzed_data_provider.ConsumeIntegralInRange(1, MAX_SCRIPT_SIZE);
10098
coin_params.m_change_fee = effective_fee_rate.GetFee(coin_params.change_output_size);
10199
coin_params.m_discard_feerate = discard_fee_rate;
102100
coin_params.change_spend_size = fuzzed_data_provider.ConsumeIntegralInRange<int>(41, 1000);
103-
coin_params.m_cost_of_change = coin_params.m_change_fee + coin_params.m_discard_feerate.GetFee(coin_params.change_spend_size);
101+
const auto change_spend_fee{coin_params.m_discard_feerate.GetFee(coin_params.change_spend_size)};
102+
coin_params.m_cost_of_change = coin_params.m_change_fee + change_spend_fee;
103+
CScript change_out_script = CScript() << std::vector<unsigned char>(coin_params.change_output_size, OP_TRUE);
104+
const auto dust{GetDustThreshold(CTxOut{/*nValueIn=*/0, change_out_script}, coin_params.m_discard_feerate)};
105+
coin_params.min_viable_change = std::max(change_spend_fee + 1, dust);
104106

105107
int next_locktime{0};
106108
CAmount total_balance{CreateCoins(fuzzed_data_provider, utxo_pool, coin_params, next_locktime)};
@@ -119,7 +121,7 @@ FUZZ_TARGET(coinselection)
119121
auto result_bnb = coin_params.m_subtract_fee_outputs ? util::Error{Untranslated("BnB disabled when SFFO is enabled")} :
120122
SelectCoinsBnB(group_pos, target, coin_params.m_cost_of_change, MAX_STANDARD_TX_WEIGHT);
121123
if (result_bnb) {
122-
assert(result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0);
124+
assert(result_bnb->GetChange(coin_params.min_viable_change, coin_params.m_change_fee) == 0);
123125
assert(result_bnb->GetSelectedValue() >= target);
124126
(void)result_bnb->GetShuffledInputVector();
125127
(void)result_bnb->GetInputSet();

0 commit comments

Comments
 (0)