Skip to content

Commit 7f61d31

Browse files
committed
[refactor]: update coin selection algorithms input parameter max_weight name
- This commit renames the coin selection algorithms input parameter `max_weight` to `max_selection_weight` for clarity. The parameter represent the maximum weight of the UTXOs the coin selection algorithm should select, not the transaction maximum weight. - The commit updates the parameter docstring to provide correct description. - Also updates coin selection unit and fuzzing test variables to match the new name.
1 parent b27afb7 commit 7f61d31

File tree

5 files changed

+55
-55
lines changed

5 files changed

+55
-55
lines changed

src/wallet/coinselection.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,14 @@ struct {
8484
* bound of the range.
8585
* @param const CAmount& cost_of_change This is the cost of creating and spending a change output.
8686
* This plus selection_target is the upper bound of the range.
87-
* @param int max_weight The maximum weight available for the input set.
87+
* @param int max_selection_weight The maximum allowed weight for a selection result to be valid.
8888
* @returns The result of this coin selection algorithm, or std::nullopt
8989
*/
9090

9191
static const size_t TOTAL_TRIES = 100000;
9292

9393
util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
94-
int max_weight)
94+
int max_selection_weight)
9595
{
9696
SelectionResult result(selection_target, SelectionAlgorithm::BNB);
9797
CAmount curr_value = 0;
@@ -128,7 +128,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
128128
curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch
129129
(curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing
130130
backtrack = true;
131-
} else if (curr_selection_weight > max_weight) { // Exceeding weight for standard tx, cannot find more solutions by adding more inputs
131+
} else if (curr_selection_weight > max_selection_weight) { // Selected UTXOs weight exceeds the maximum weight allowed, cannot find more solutions by adding more inputs
132132
max_tx_weight_exceeded = true; // at least one selection attempt exceeded the max weight
133133
backtrack = true;
134134
} else if (curr_value >= selection_target) { // Selected value is within range
@@ -319,10 +319,10 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
319319
* group with multiple as a heavier UTXO with the combined amount here.)
320320
* @param const CAmount& selection_target This is the minimum amount that we need for the transaction without considering change.
321321
* @param const CAmount& change_target The minimum budget for creating a change output, by which we increase the selection_target.
322-
* @param int max_weight The maximum permitted weight for the input set.
322+
* @param int max_selection_weight The maximum allowed weight for a selection result to be valid.
323323
* @returns The result of this coin selection algorithm, or std::nullopt
324324
*/
325-
util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight)
325+
util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_selection_weight)
326326
{
327327
std::sort(utxo_pool.begin(), utxo_pool.end(), descending_effval_weight);
328328
// The sum of UTXO amounts after this UTXO index, e.g. lookahead[5] = Σ(UTXO[6+].amount)
@@ -359,7 +359,7 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c
359359

360360
// The weight of the currently selected input set, and the weight of the best selection
361361
int curr_weight = 0;
362-
int best_selection_weight = max_weight; // Tie is fine, because we prefer lower selection amount
362+
int best_selection_weight = max_selection_weight; // Tie is fine, because we prefer lower selection amount
363363

364364
// Whether the input sets generated during this search have exceeded the maximum transaction weight at any point
365365
bool max_tx_weight_exceeded = false;
@@ -436,8 +436,8 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c
436436
// Insufficient funds with lookahead: CUT
437437
should_cut = true;
438438
} else if (curr_weight > best_selection_weight) {
439-
// best_selection_weight is initialized to max_weight
440-
if (curr_weight > max_weight) max_tx_weight_exceeded = true;
439+
// best_selection_weight is initialized to max_selection_weight
440+
if (curr_weight > max_selection_weight) max_tx_weight_exceeded = true;
441441
// Worse weight than best solution. More UTXOs only increase weight:
442442
// CUT if last selected group had minimal weight, else SHIFT
443443
if (utxo_pool[curr_tail].m_weight <= min_tail_weight[curr_tail]) {
@@ -535,7 +535,7 @@ class MinOutputGroupComparator
535535
};
536536

537537
util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng,
538-
int max_weight)
538+
int max_selection_weight)
539539
{
540540
SelectionResult result(target_value, SelectionAlgorithm::SRD);
541541
std::priority_queue<OutputGroup, std::vector<OutputGroup>, MinOutputGroupComparator> heap;
@@ -565,14 +565,14 @@ util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utx
565565

566566
// If the selection weight exceeds the maximum allowed size, remove the least valuable inputs until we
567567
// are below max weight.
568-
if (weight > max_weight) {
568+
if (weight > max_selection_weight) {
569569
max_tx_weight_exceeded = true; // mark it in case we don't find any useful result.
570570
do {
571571
const OutputGroup& to_remove_group = heap.top();
572572
selected_eff_value -= to_remove_group.GetSelectionAmount();
573573
weight -= to_remove_group.m_weight;
574574
heap.pop();
575-
} while (!heap.empty() && weight > max_weight);
575+
} while (!heap.empty() && weight > max_selection_weight);
576576
}
577577

578578
// Now check if we are above the target
@@ -648,7 +648,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v
648648
}
649649

650650
util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue,
651-
CAmount change_target, FastRandomContext& rng, int max_weight)
651+
CAmount change_target, FastRandomContext& rng, int max_selection_weight)
652652
{
653653
SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK);
654654

@@ -709,7 +709,7 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c
709709
}
710710

711711
// If the result exceeds the maximum allowed size, return closest UTXO above the target
712-
if (result.GetWeight() > max_weight) {
712+
if (result.GetWeight() > max_selection_weight) {
713713
// No coin above target, nothing to do.
714714
if (!lowest_larger) return ErrorMaxWeightExceeded();
715715

src/wallet/coinselection.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -440,25 +440,25 @@ struct SelectionResult
440440
};
441441

442442
util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
443-
int max_weight);
443+
int max_selection_weight);
444444

445-
util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight);
445+
util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_selection_weight);
446446

447447
/** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible
448448
* outputs until the target is satisfied
449449
*
450450
* @param[in] utxo_pool The positive effective value OutputGroups eligible for selection
451451
* @param[in] target_value The target value to select for
452452
* @param[in] rng The randomness source to shuffle coins
453-
* @param[in] max_weight The maximum allowed weight for a selection result to be valid
453+
* @param[in] max_selection_weight The maximum allowed weight for a selection result to be valid
454454
* @returns If successful, a valid SelectionResult, otherwise, util::Error
455455
*/
456456
util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng,
457-
int max_weight);
457+
int max_selection_weight);
458458

459459
// Original coin selection algorithm as a fallback
460460
util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue,
461-
CAmount change_target, FastRandomContext& rng, int max_weight);
461+
CAmount change_target, FastRandomContext& rng, int max_selection_weight);
462462
} // namespace wallet
463463

464464
#endif // BITCOIN_WALLET_COINSELECTION_H

src/wallet/spend.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -695,34 +695,34 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
695695
}
696696
};
697697

698-
// Maximum allowed weight
699-
int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR);
698+
// Maximum allowed weight for selected coins.
699+
int max_selection_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR);
700700

701701
// SFFO frequently causes issues in the context of changeless input sets: skip BnB when SFFO is active
702702
if (!coin_selection_params.m_subtract_fee_outputs) {
703-
if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_inputs_weight)}) {
703+
if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_selection_weight)}) {
704704
results.push_back(*bnb_result);
705705
} else append_error(std::move(bnb_result));
706706
}
707707

708708
// As Knapsack and SRD can create change, also deduce change weight.
709-
max_inputs_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR);
709+
max_selection_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR);
710710

711711
// The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here.
712-
if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_inputs_weight)}) {
712+
if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_selection_weight)}) {
713713
results.push_back(*knapsack_result);
714714
} else append_error(std::move(knapsack_result));
715715

716716
if (coin_selection_params.m_effective_feerate > CFeeRate{3 * coin_selection_params.m_long_term_feerate}) { // Minimize input set for feerates of at least 3×LTFRE (default: 30 ṩ/vB+)
717-
if (auto cg_result{CoinGrinder(groups.positive_group, nTargetValue, coin_selection_params.m_min_change_target, max_inputs_weight)}) {
717+
if (auto cg_result{CoinGrinder(groups.positive_group, nTargetValue, coin_selection_params.m_min_change_target, max_selection_weight)}) {
718718
cg_result->RecalculateWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
719719
results.push_back(*cg_result);
720720
} else {
721721
append_error(std::move(cg_result));
722722
}
723723
}
724724

725-
if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight)}) {
725+
if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_selection_weight)}) {
726726
results.push_back(*srd_result);
727727
} else append_error(std::move(srd_result));
728728

0 commit comments

Comments
 (0)