From bdd0c2934b7f389ffcfae3b602ee3ecee8581acd Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 4 Feb 2021 19:11:24 -0500 Subject: [PATCH] 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. --- src/bench/coin_selection.cpp | 2 +- src/wallet/test/coinselector_tests.cpp | 8 ++++---- src/wallet/wallet.cpp | 9 +++++---- src/wallet/wallet.h | 4 +++- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 0cbd4bfad4..56fd16e0fd 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -51,7 +51,7 @@ static void CoinSelection(benchmark::Bench& bench) const CoinEligibilityFilter filter_standard(1, 6, 0); const CoinSelectionParams coin_selection_params(/* use_bnb= */ true, /* change_output_size= */ 34, /* 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); bench.run([&] { std::set setCoinsRet; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 901b2e8ccf..09b3b3e116 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -37,7 +37,7 @@ CoinEligibilityFilter filter_confirmed(1, 1, 0); CoinEligibilityFilter filter_standard_extra(6, 6, 0); CoinSelectionParams coin_selection_params(/* use_bnb= */ false, /* change_output_size= */ 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); static void add_coin(const CAmount& nValue, int nInput, std::vector& 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 CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 0, /* 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); CoinSet setCoinsRet; CAmount nValueRet; @@ -647,11 +647,11 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) // Perform selection CoinSelectionParams coin_selection_params_knapsack(/* use_bnb= */ false, /* change_output_size= */ 34, /* 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); CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 34, /* 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); CoinSet out_set; CAmount out_value = 0; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ce0c458c64..a835aaa2fb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2374,7 +2374,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil std::vector 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 - 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 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); 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 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 // add the dust to the fee. // 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; nFeeRet += nChange; @@ -2969,7 +2970,7 @@ bool CWallet::CreateTransactionInternal( 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 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) { pick_new_inputs = false; nFeeRet = fee_needed_with_change; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 67d51de30f..e1a567989f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -608,18 +608,20 @@ struct CoinSelectionParams size_t change_spend_size = 0; CFeeRate effective_fee = CFeeRate(0); CFeeRate m_long_term_feerate; + CFeeRate m_discard_feerate; size_t tx_noinputs_size = 0; //! Indicate that we are subtracting the fee from outputs bool m_subtract_fee_outputs = false; bool m_avoid_partial_spends = false; 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), change_output_size(change_output_size), change_spend_size(change_spend_size), effective_fee(effective_fee), m_long_term_feerate(long_term_feerate), + m_discard_feerate(discard_feerate), tx_noinputs_size(tx_noinputs_size), m_avoid_partial_spends(avoid_partial) {}