wallet: Move discard feerate fetching to CreateTransaction

Instead of fetching the discard feerate for each SelectCoinsMinConf
iteration, fetch and cache it once during CreateTransaction so that it
is shared for each SelectCoinsMinConf through
coin_selection_params.m_discard_feerate.

Does not change behavior.
This commit is contained in:
Andrew Chow committed 2021-03-16 16:33:27 -04:00
1 parent 448d04b931
commit bdd0c2934b
4 files changed
+13 -10

No files matched your search

+1 -1
View File
@@ -51,7 +51,7 @@ static void CoinSelection(benchmark::Bench& bench)
const CoinEligibilityFilter filter_standard(1, 6, 0); const CoinEligibilityFilter filter_standard(1, 6, 0);
const CoinSelectionParams coin_selection_params(/* use_bnb= */ true, /* change_output_size= */ 34, const CoinSelectionParams coin_selection_params(/* use_bnb= */ true, /* change_output_size= */ 34,
/* change_spend_size= */ 148, /* effective_fee= */ CFeeRate(0), /* change_spend_size= */ 148, /* effective_fee= */ CFeeRate(0),
/* long_term_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
/* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
bench.run([&] { bench.run([&] {
std::set<CInputCoin> setCoinsRet; std::set<CInputCoin> setCoinsRet;
+4 -4
View File
@@ -37,7 +37,7 @@ CoinEligibilityFilter filter_confirmed(1, 1, 0);
CoinEligibilityFilter filter_standard_extra(6, 6, 0); CoinEligibilityFilter filter_standard_extra(6, 6, 0);
CoinSelectionParams coin_selection_params(/* use_bnb= */ false, /* change_output_size= */ 0, CoinSelectionParams coin_selection_params(/* use_bnb= */ false, /* change_output_size= */ 0,
/* change_spend_size= */ 0, /* effective_fee= */ CFeeRate(0), /* change_spend_size= */ 0, /* effective_fee= */ CFeeRate(0),
/* long_term_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
/* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set)
@@ -274,7 +274,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
// Make sure that effective value is working in SelectCoinsMinConf when BnB is used // Make sure that effective value is working in SelectCoinsMinConf when BnB is used
CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 0, CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 0,
/* change_spend_size= */ 0, /* effective_fee= */ CFeeRate(3000), /* change_spend_size= */ 0, /* effective_fee= */ CFeeRate(3000),
/* long_term_feerate= */ CFeeRate(1000), /* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000),
/* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
CoinSet setCoinsRet; CoinSet setCoinsRet;
CAmount nValueRet; CAmount nValueRet;
@@ -647,11 +647,11 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test)
// Perform selection // Perform selection
CoinSelectionParams coin_selection_params_knapsack(/* use_bnb= */ false, /* change_output_size= */ 34, CoinSelectionParams coin_selection_params_knapsack(/* use_bnb= */ false, /* change_output_size= */ 34,
/* change_spend_size= */ 148, /* effective_fee= */ CFeeRate(0), /* change_spend_size= */ 148, /* effective_fee= */ CFeeRate(0),
/* long_term_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
/* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 34, CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 34,
/* change_spend_size= */ 148, /* effective_fee= */ CFeeRate(0), /* change_spend_size= */ 148, /* effective_fee= */ CFeeRate(0),
/* long_term_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
/* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
CoinSet out_set; CoinSet out_set;
CAmount out_value = 0; CAmount out_value = 0;
+5 -4
View File
@@ -2374,7 +2374,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, true /* positive_only */); std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, true /* positive_only */);
// Calculate cost of change // Calculate cost of change
CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); CAmount cost_of_change = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size);
// Calculate the fees for things that aren't inputs // Calculate the fees for things that aren't inputs
CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size);
@@ -2798,7 +2798,8 @@ bool CWallet::CreateTransactionInternal(
CTxOut change_prototype_txout(0, scriptChange); CTxOut change_prototype_txout(0, scriptChange);
coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout); coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout);
CFeeRate discard_rate = GetDiscardRate(*this); // Set discard feerate
coin_selection_params.m_discard_feerate = GetDiscardRate(*this);
// Get the fee rate to use effective values in coin selection // Get the fee rate to use effective values in coin selection
coin_selection_params.effective_fee = GetMinimumFeeRate(*this, coin_control, &feeCalc); coin_selection_params.effective_fee = GetMinimumFeeRate(*this, coin_control, &feeCalc);
@@ -2917,7 +2918,7 @@ bool CWallet::CreateTransactionInternal(
// Never create dust outputs; if we would, just // Never create dust outputs; if we would, just
// add the dust to the fee. // add the dust to the fee.
// The nChange when BnB is used is always going to go to fees. // The nChange when BnB is used is always going to go to fees.
if (IsDust(newTxOut, discard_rate) || bnb_used) if (IsDust(newTxOut, coin_selection_params.m_discard_feerate) || bnb_used)
{ {
nChangePosInOut = -1; nChangePosInOut = -1;
nFeeRet += nChange; nFeeRet += nChange;
@@ -2969,7 +2970,7 @@ bool CWallet::CreateTransactionInternal(
if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) {
unsigned int tx_size_with_change = nBytes + coin_selection_params.change_output_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size unsigned int tx_size_with_change = nBytes + coin_selection_params.change_output_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size
CAmount fee_needed_with_change = coin_selection_params.effective_fee.GetFee(tx_size_with_change); CAmount fee_needed_with_change = coin_selection_params.effective_fee.GetFee(tx_size_with_change);
CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, coin_selection_params.m_discard_feerate);
if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) {
pick_new_inputs = false; pick_new_inputs = false;
nFeeRet = fee_needed_with_change; nFeeRet = fee_needed_with_change;
+3 -1
View File
@@ -608,18 +608,20 @@ struct CoinSelectionParams
size_t change_spend_size = 0; size_t change_spend_size = 0;
CFeeRate effective_fee = CFeeRate(0); CFeeRate effective_fee = CFeeRate(0);
CFeeRate m_long_term_feerate; CFeeRate m_long_term_feerate;
CFeeRate m_discard_feerate;
size_t tx_noinputs_size = 0; size_t tx_noinputs_size = 0;
//! Indicate that we are subtracting the fee from outputs //! Indicate that we are subtracting the fee from outputs
bool m_subtract_fee_outputs = false; bool m_subtract_fee_outputs = false;
bool m_avoid_partial_spends = false; bool m_avoid_partial_spends = false;
CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee,
CFeeRate long_term_feerate, size_t tx_noinputs_size, bool avoid_partial) : CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial) :
use_bnb(use_bnb), use_bnb(use_bnb),
change_output_size(change_output_size), change_output_size(change_output_size),
change_spend_size(change_spend_size), change_spend_size(change_spend_size),
effective_fee(effective_fee), effective_fee(effective_fee),
m_long_term_feerate(long_term_feerate), m_long_term_feerate(long_term_feerate),
m_discard_feerate(discard_feerate),
tx_noinputs_size(tx_noinputs_size), tx_noinputs_size(tx_noinputs_size),
m_avoid_partial_spends(avoid_partial) m_avoid_partial_spends(avoid_partial)
{} {}