highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-21 15:50:07 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fold GetSelectionWaste() into ComputeAndSetWaste()

Browse Source 
Both `GetSelectionWaste()` and `ComputeAndSetWaste()` now are part of
`SelectionResult`. Instead of `ComputeAndSetWaste()` being a wrapper for
`GetSelectionWaste()`, we combine them to a new function
`RecalculateWaste()`.

As I was combining the logic of the two functions, I noticed that
`GetSelectionWaste()` was making the odd assumption that the
`change_cost` being set to zero means that no change is created.
However, if we build transactions at a feerate of zero with the
`discard_feerate` also set to zero, we'd organically have a
`change_cost` of zero, even when we create change on a transaction.

This commit cleans up this duplicate meaning of `change_cost` and relies
on `GetChange()` to figure out whether there is change on basis of the
`min_viable_change` and whatever is left after deducting fees.

Since this broke a bunch of tests that relied on the double-meaning of
`change_cost` a bunch of tests had to be fixed.
This commit is contained in:
Murch
2023-08-21 15:27:23 -04:00
parent 5a5ab1d544
commit 7aa7e30441
6 changed files with 83 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
src/wallet/coinselection.cpp
View File

@@ -194,7 +194,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
for (const size_t& i : best_selection) {
result.AddInput(utxo_pool.at(i));
}
result.ComputeAndSetWaste(cost_of_change, cost_of_change, CAmount{0});
result.RecalculateWaste(cost_of_change, cost_of_change, CAmount{0});
assert(best_waste == result.GetWaste());
return result;
@@ -792,35 +792,6 @@ void OutputGroupTypeMap::Push(const OutputGroup& group, OutputType type, bool in
}
}
CAmount SelectionResult::GetSelectionWaste(CAmount change_cost, CAmount target, bool use_effective_value)
{
// This function should not be called with empty inputs as that would mean the selection failed
assert(!m_selected_inputs.empty());
// Always consider the cost of spending an input now vs in the future.
CAmount waste = 0;
for (const auto& coin_ptr : m_selected_inputs) {
const COutput& coin = *coin_ptr;
waste += coin.GetFee() - coin.long_term_fee;
}
// Bump fee of whole selection may diverge from sum of individual bump fees
waste -= bump_fee_group_discount;
if (change_cost) {
// Consider the cost of making change and spending it in the future
// If we aren't making change, the caller should've set change_cost to 0
assert(change_cost > 0);
waste += change_cost;
} else {
// When we are not making change (change_cost == 0), consider the excess we are throwing away to fees
CAmount selected_effective_value = use_effective_value ? GetSelectedEffectiveValue() : GetSelectedValue();
assert(selected_effective_value >= target);
waste += selected_effective_value - target;
}
return waste;
}
CAmount GenerateChangeTarget(const CAmount payment_value, const CAmount change_fee, FastRandomContext& rng)
{
if (payment_value <= CHANGE_LOWER / 2) {
@@ -839,16 +810,32 @@ void SelectionResult::SetBumpFeeDiscount(const CAmount discount)
bump_fee_group_discount = discount;
}
void SelectionResult::ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee)
void SelectionResult::RecalculateWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee)
{
const CAmount change = GetChange(min_viable_change, change_fee);
// This function should not be called with empty inputs as that would mean the selection failed
assert(!m_selected_inputs.empty());
if (change > 0) {
m_waste = GetSelectionWaste(change_cost, m_target, m_use_effective);
} else {
m_waste = GetSelectionWaste(0, m_target, m_use_effective);
// Always consider the cost of spending an input now vs in the future.
CAmount waste = 0;
for (const auto& coin_ptr : m_selected_inputs) {
const COutput& coin = *coin_ptr;
waste += coin.GetFee() - coin.long_term_fee;
}
// Bump fee of whole selection may diverge from sum of individual bump fees
waste -= bump_fee_group_discount;
if (GetChange(min_viable_change, change_fee)) {
// if we have a minimum viable amount after deducting fees, account for
// cost of creating and spending change
waste += change_cost;
} else {
// When we are not making change (GetChange(…) == 0), consider the excess we are throwing away to fees
CAmount selected_effective_value = m_use_effective ? GetSelectedEffectiveValue() : GetSelectedValue();
assert(selected_effective_value >= m_target);
waste += selected_effective_value - m_target;
}
m_waste = waste;
}
void SelectionResult::SetAlgoCompleted(bool algo_completed)