foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

Have KnapsackSolver actually use effective values

Browse source 
Although the CreateTransaction loop currently remains, it should be
largely unused. KnapsackSolver will now account for transaction fees
when doing its selection.

In the previous commit, SelectCoinsMinConf was refactored to have some
calculations become shared for KnapsackSolver and SelectCoinsBnB. In
this commit, KnapsackSolver will now use the not_input_fees and
effective_feerate so that it include the fee for non-input things
(excluding a change output) so that the algorithm will select enough to
cover those fees. This is necessary for selecting on effective values.

Additionally, the OutputGroups
created for KnapsackSolver will actually have their effective values
calculated and set, and KnapsackSolver will do its selection on those
effective values.

Lastly, SelectCoins is modified to use the same value for preselected
inputs for BnB and KnapsackSolver. While it will still use the real
value when subtracting the fee from outputs, this behavior will be
the same regardless of the algo used for selecting additional inputs.
This commit is contained in:
Andrew Chow 2021-05-06 13:54:22 -04:00
parent bf26e018de
commit 01dc8ebda5
2 changed files with 10 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/wallet/coinselection.cpp
View file

@ -227,14 +227,14 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group
Shuffle(groups.begin(), groups.end(), FastRandomContext());
for (const OutputGroup& group : groups) {
if (group.m_value == nTargetValue) {
if (group.effective_value == nTargetValue) {
util::insert(setCoinsRet, group.m_outputs);
nValueRet += group.m_value;
return true;
} else if (group.m_value < nTargetValue + MIN_CHANGE) {
} else if (group.effective_value < nTargetValue + MIN_CHANGE) {
applicable_groups.push_back(group);
nTotalLower += group.m_value;
} else if (!lowest_larger || group.m_value < lowest_larger->m_value) {
nTotalLower += group.effective_value;
} else if (!lowest_larger || group.effective_value < lowest_larger->effective_value) {
lowest_larger = group;
}
}
@ -267,7 +267,7 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group
// If we have a bigger coin and (either the stochastic approximation didn't find a good solution,
// or the next bigger coin is closer), return the bigger coin
if (lowest_larger &&
((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || lowest_larger->m_value <= nBest)) {
((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || lowest_larger->effective_value <= nBest)) {
util::insert(setCoinsRet, lowest_larger->m_outputs);
nValueRet += lowest_larger->m_value;
} else {

18
src/wallet/wallet.cpp
View file

@ -2413,13 +2413,11 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
return SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change, setCoinsRet, nValueRet);
} else {
// 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.
// The knapsack solver currently does not use effective values, so we give GroupOutputs feerates of 0 so it sets the effective values to be the same as the real value.
std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, CFeeRate(0), CFeeRate(0), eligibility_filter, false /* positive_only */);
std::vector<OutputGroup> all_groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, false /* positive_only */);
bnb_used = false;
// While nTargetValue includes the transaction fees for non-input things, it does not include the fee for creating a change output.
// So we need to include that for KnapsackSolver as well, as we are expecting to create a change output.
return KnapsackSolver(nTargetValue + coin_selection_params.m_change_fee, groups, setCoinsRet, nValueRet);
return KnapsackSolver(nTargetValue + coin_selection_params.m_change_fee, all_groups, setCoinsRet, nValueRet);
}
}
@ -2467,10 +2465,10 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
return false; // Not solvable, can't estimate size for fee
}
coin.effective_value = coin.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes);
if (coin_selection_params.use_bnb) {
value_to_select -= coin.effective_value;
} else {
if (coin_selection_params.m_subtract_fee_outputs) {
value_to_select -= coin.txout.nValue;
} else {
value_to_select -= coin.effective_value;
}
setPresetCoins.insert(coin);
} else {
@ -2956,12 +2954,6 @@ bool CWallet::CreateTransactionInternal(
const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size);
CAmount nValueToSelect = nValue + not_input_fees;
// For KnapsackSolver, when we are not subtracting the fee from the recipients, we also want to include the fees for the
// inputs that we found in the previous iteration.
if (!coin_selection_params.use_bnb && nSubtractFeeFromAmount == 0) {
nValueToSelect += std::max(CAmount(0), nFeeRet - not_input_fees);
}
// Choose coins to use
bool bnb_used = false;
nValueIn = 0;