From bcd716670ba8a189a2e9b8b035318abceb9ce631 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 4 Feb 2021 18:28:45 -0500 Subject: [PATCH] wallet: Move long term feerate setting to CreateTransaction Instead of setting the long term feerate for each SelectCoinsMinConf iteration, set it once during CreateTransaction and let it be shared with each SelectCoinsMinConf through coin_selection_params.m_long_term_feerate. Does not change behavior. Github-Pull: #21083 Rebased-From: 448d04b931f86941903e855f831249ff5ec77485 --- src/bench/coin_selection.cpp | 5 ++++- src/wallet/test/coinselector_tests.cpp | 20 ++++++++++++++++---- src/wallet/wallet.cpp | 15 +++++++-------- src/wallet/wallet.h | 11 ++++++++++- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 99aafd8dfc73e..1ef89a41dd1df 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -50,7 +50,10 @@ static void CoinSelection(benchmark::Bench& bench) } const CoinEligibilityFilter filter_standard(1, 6, 0); - const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 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), + /* tx_no_inputs_size= */ 0); bench.run([&] { std::set setCoinsRet; CAmount nValueRet; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f38ccba38447d..f375ce02a5a4f 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -35,7 +35,10 @@ static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); CoinEligibilityFilter filter_confirmed(1, 1, 0); CoinEligibilityFilter filter_standard_extra(6, 6, 0); -CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 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), + /* tx_no_inputs_size= */ 0); static void add_coin(const CAmount& nValue, int nInput, std::vector& set) { @@ -262,7 +265,10 @@ 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(true, 0, 0, CFeeRate(3000), 0); + 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), + /* tx_no_inputs_size= */ 0); CoinSet setCoinsRet; CAmount nValueRet; bool bnb_used; @@ -632,8 +638,14 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CAmount target = rand.randrange(balance - 1000) + 1000; // Perform selection - CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0); - CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0); + 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), + /* tx_no_inputs_size= */ 0); + 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), + /* tx_no_inputs_size= */ 0); CoinSet out_set; CAmount out_value = 0; bool bnb_used = false; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3e1d80400012c..b1951c0c3d158 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2362,12 +2362,6 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil std::vector utxo_pool; if (coin_selection_params.use_bnb) { - // Get long term estimate - FeeCalculation feeCalc; - CCoinControl temp; - temp.m_confirm_target = 1008; - CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc); - // 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); @@ -2377,9 +2371,9 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil if (coin_selection_params.m_subtract_fee_outputs) { // Set the effective feerate to 0 as we don't want to use the effective value since the fees will be deducted from the output - group.SetFees(CFeeRate(0) /* effective_feerate */, long_term_feerate); + group.SetFees(CFeeRate(0) /* effective_feerate */, coin_selection_params.m_long_term_feerate); } else { - group.SetFees(coin_selection_params.effective_fee, long_term_feerate); + group.SetFees(coin_selection_params.effective_fee, coin_selection_params.m_long_term_feerate); } OutputGroup pos_group = group.GetPositiveOnlyGroup(); @@ -2827,6 +2821,11 @@ bool CWallet::CreateTransactionInternal( return false; } + // Get long term estimate + CCoinControl cc_temp; + cc_temp.m_confirm_target = chain().estimateMaxBlocks(); + coin_selection_params.m_long_term_feerate = GetMinimumFeeRate(*this, cc_temp, nullptr); + nFeeRet = 0; bool pick_new_inputs = true; CAmount nValueIn = 0; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 69cf6b66a4a21..65b57396bfb0d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -607,11 +607,20 @@ struct CoinSelectionParams size_t change_output_size = 0; size_t change_spend_size = 0; CFeeRate effective_fee = CFeeRate(0); + CFeeRate m_long_term_feerate; size_t tx_noinputs_size = 0; //! Indicate that we are subtracting the fee from outputs bool m_subtract_fee_outputs = false; - CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), effective_fee(effective_fee), tx_noinputs_size(tx_noinputs_size) {} + 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) : + 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), + tx_noinputs_size(tx_noinputs_size) + {} CoinSelectionParams() {} };