[bug] Increase SRD target by change_fee

I discovered via fuzzing of another coin selection approach that at
extremely high feerates SRD may find input sets that lead to
transactions without change outputs. This is an unintended outcome since
SRD is meant to always produce a transaction with a change output—we use
other algorithms to specifically search for changeless solutions.

The issue occures when the flat allowance of 50,000 ṩ for change is
insufficient to pay for the creation of a change output with a non-dust
amount, at and above 1,613 ṩ/vB. Increasing the change budget by
change_fees makes SRD behave as expected at any feerates.
This commit is contained in:
Murch 2023-06-02 16:27:36 -04:00
parent a36134fcc7
commit 941b8c6539
No known key found for this signature in database
GPG key ID: 7BA035CA5B901713
5 changed files with 6 additions and 6 deletions

View file

@ -191,7 +191,7 @@ public:
}
};
util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, FastRandomContext& rng,
util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng,
int max_weight)
{
SelectionResult result(target_value, SelectionAlgorithm::SRD);
@ -201,7 +201,7 @@ util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utx
// barely meets the target. Just use the lower bound change target instead of the randomly
// generated one, since SRD will result in a random change amount anyway; avoid making the
// target needlessly large.
target_value += CHANGE_LOWER;
target_value += CHANGE_LOWER + change_fee;
std::vector<size_t> indexes;
indexes.resize(utxo_pool.size());

View file

@ -421,7 +421,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
* @param[in] max_weight The maximum allowed weight for a selection result to be valid
* @returns If successful, a valid SelectionResult, otherwise, util::Error
*/
util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, FastRandomContext& rng,
util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng,
int max_weight);
// Original coin selection algorithm as a fallback

View file

@ -583,7 +583,7 @@ util::Result<SelectionResult> ChooseSelectionResult(const CAmount& nTargetValue,
results.push_back(*knapsack_result);
} else append_error(knapsack_result);
if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.rng_fast, max_inputs_weight)}) {
if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight)}) {
srd_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
results.push_back(*srd_result);
} else append_error(srd_result);

View file

@ -968,7 +968,7 @@ static util::Result<SelectionResult> SelectCoinsSRD(const CAmount& target,
std::unique_ptr<CWallet> wallet = NewWallet(m_node);
CoinEligibilityFilter filter(0, 0, 0); // accept all coins without ancestors
Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups;
return SelectCoinsSRD(group.positive_group, target, cs_params.rng_fast, max_weight);
return SelectCoinsSRD(group.positive_group, target, cs_params.m_change_fee, cs_params.rng_fast, max_weight);
}
BOOST_AUTO_TEST_CASE(srd_tests)

View file

@ -90,7 +90,7 @@ FUZZ_TARGET(coinselection)
// Run coinselection algorithms
const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change, MAX_STANDARD_TX_WEIGHT);
auto result_srd = SelectCoinsSRD(group_pos, target, fast_random_context, MAX_STANDARD_TX_WEIGHT);
auto result_srd = SelectCoinsSRD(group_pos, target, coin_params.m_change_fee, fast_random_context, MAX_STANDARD_TX_WEIGHT);
if (result_srd) result_srd->ComputeAndSetWaste(cost_of_change, cost_of_change, 0);
CAmount change_target{GenerateChangeTarget(target, coin_params.m_change_fee, fast_random_context)};