wallet: use GetChange() when computing waste

This commit is contained in:
S3RK 2022-07-06 09:03:01 +02:00
parent 87e0ef9031
commit 4fef534428
5 changed files with 25 additions and 14 deletions

View file

@ -156,7 +156,7 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo
for (const size_t& i : best_selection) {
result.AddInput(utxo_pool.at(i));
}
result.ComputeAndSetWaste(CAmount{0});
result.ComputeAndSetWaste(cost_of_change, cost_of_change, CAmount{0});
assert(best_waste == result.GetWaste());
return result;
@ -406,9 +406,15 @@ CAmount GenerateChangeTarget(const CAmount payment_value, const CAmount change_f
}
}
void SelectionResult::ComputeAndSetWaste(CAmount change_cost)
void SelectionResult::ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee)
{
m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective);
const CAmount change = GetChange(min_viable_change, change_fee);
if (change > 0) {
m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective);
} else {
m_waste = GetSelectionWaste(m_selected_inputs, 0, m_target, m_use_effective);
}
}
CAmount SelectionResult::GetWaste() const

View file

@ -310,7 +310,7 @@ public:
void AddInput(const OutputGroup& group);
/** Calculates and stores the waste for this selection via GetSelectionWaste */
void ComputeAndSetWaste(CAmount change_cost);
void ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee);
[[nodiscard]] CAmount GetWaste() const;
void Merge(const SelectionResult& other);

View file

@ -497,12 +497,12 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
// 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.
std::vector<OutputGroup> all_groups = GroupOutputs(wallet, available_coins, coin_selection_params, eligibility_filter, /*positive_only=*/false);
if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) {
knapsack_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
knapsack_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
results.push_back(*knapsack_result);
}
if (auto srd_result{SelectCoinsSRD(positive_groups, nTargetValue, coin_selection_params.rng_fast)}) {
srd_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
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);
}
@ -574,7 +574,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL);
result.AddInput(preset_inputs);
if (result.GetSelectedValue() < nTargetValue) return std::nullopt;
result.ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
result.ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
return result;
}
@ -671,7 +671,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
// Add preset inputs to result
res->Merge(preselected);
if (res->GetAlgo() == SelectionAlgorithm::MANUAL) {
res->ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
res->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
}
return res;

View file

@ -248,9 +248,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
// Iteration exhaustion test
CAmount target = make_hard_case(17, utxo_pool);
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 0)); // Should exhaust
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 1)); // Should exhaust
target = make_hard_case(14, utxo_pool);
const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 0); // Should not exhaust
const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 1); // Should not exhaust
BOOST_CHECK(result7);
// Test same value early bailout optimization
@ -289,8 +289,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
// Make sure that effective value is working in AttemptSelection when BnB is used
CoinSelectionParams coin_selection_params_bnb{
rand,
/*change_output_size=*/ 0,
/*change_spend_size=*/ 0,
/*change_output_size=*/ 31,
/*change_spend_size=*/ 68,
/*min_change_target=*/ 0,
/*effective_feerate=*/ CFeeRate(3000),
/*long_term_feerate=*/ CFeeRate(1000),
@ -298,6 +298,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
/*tx_noinputs_size=*/ 0,
/*avoid_partial=*/ false,
};
coin_selection_params_bnb.m_change_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_output_size);
coin_selection_params_bnb.m_cost_of_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size) + coin_selection_params_bnb.m_change_fee;
coin_selection_params_bnb.min_viable_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size);
{
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
wallet->LoadWallet();
@ -777,6 +780,8 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test)
/*tx_noinputs_size=*/ 0,
/*avoid_partial=*/ false,
};
cs_params.m_cost_of_change = 1;
cs_params.min_viable_change = 1;
CCoinControl cc;
const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params);
BOOST_CHECK(result);

View file

@ -85,11 +85,11 @@ FUZZ_TARGET(coinselection)
const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change);
auto result_srd = SelectCoinsSRD(group_pos, target, fast_random_context);
if (result_srd) result_srd->ComputeAndSetWaste(cost_of_change);
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)};
auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context);
if (result_knapsack) result_knapsack->ComputeAndSetWaste(cost_of_change);
if (result_knapsack) result_knapsack->ComputeAndSetWaste(cost_of_change, cost_of_change, 0);
// If the total balance is sufficient for the target and we are not using
// effective values, Knapsack should always find a solution.