From 48fc675163a657e615fd4b2680fc3accba12f95d Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 4 Feb 2021 18:21:26 -0500 Subject: wallet: Use existing feerate instead of getting a new one During each loop of CreateTransaction, instead of constantly getting a new feerate, use the feerate that we have already fetched for all fee calculations. Thix fixes a race condition where the feerate required changes during each iteration of the loop. This commit changes behavior as the "Fee estimation failed" error will now take priority over "Signing transaction failed". Github-Pull: #21083 Rebased-From: 1a6a0b0dfb90f9ebd4b86d7934c6aa5594974f5f --- src/wallet/wallet.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ff8bfff872..12947ea017 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2821,6 +2821,11 @@ bool CWallet::CreateTransactionInternal( error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), nFeeRateNeeded.ToString(FeeEstimateMode::SAT_VB)); return false; } + if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) { + // eventually allow a fallback fee + error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); + return false; + } nFeeRet = 0; bool pick_new_inputs = true; @@ -2958,13 +2963,7 @@ bool CWallet::CreateTransactionInternal( return false; } - nFeeNeeded = GetMinimumFee(*this, nBytes, coin_control, &feeCalc); - if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) { - // eventually allow a fallback fee - error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); - return false; - } - + nFeeNeeded = coin_selection_params.effective_fee.GetFee(nBytes); if (nFeeRet >= nFeeNeeded) { // Reduce fee to only the needed amount if possible. This // prevents potential overpayment in fees if the coins @@ -2978,7 +2977,7 @@ bool CWallet::CreateTransactionInternal( // change output. Only try this once. 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 = GetMinimumFee(*this, tx_size_with_change, coin_control, nullptr); + 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); if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { pick_new_inputs = false; -- cgit v1.2.3 From 34c89f92f34b5ca12da95d5f0b0240682c5a1c1f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 4 Feb 2021 18:23:51 -0500 Subject: wallet: Replace nFeeRateNeeded with effective_fee Make sure that all fee calculations use the same feerate. coin_selection_params.effective_fee is the variable we use for all fee calculations, so get rid of remaining nFeeRateNeeded usages and just directly set coin_selection_params.effective_fee. Does not change behavior. Github-Pull: #21083 Rebased-From: e2f429e6bbf7098f278c0247b954ecd3ba53cf37 --- src/wallet/wallet.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 12947ea017..3e1d804000 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2814,11 +2814,11 @@ bool CWallet::CreateTransactionInternal( CFeeRate discard_rate = GetDiscardRate(*this); // Get the fee rate to use effective values in coin selection - CFeeRate nFeeRateNeeded = GetMinimumFeeRate(*this, coin_control, &feeCalc); + coin_selection_params.effective_fee = GetMinimumFeeRate(*this, coin_control, &feeCalc); // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly // provided one - if (coin_control.m_feerate && nFeeRateNeeded > *coin_control.m_feerate) { - error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), nFeeRateNeeded.ToString(FeeEstimateMode::SAT_VB)); + if (coin_control.m_feerate && coin_selection_params.effective_fee > *coin_control.m_feerate) { + error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.effective_fee.ToString(FeeEstimateMode::SAT_VB)); return false; } if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) { @@ -2900,7 +2900,6 @@ bool CWallet::CreateTransactionInternal( } else { coin_selection_params.change_spend_size = (size_t)change_spend_size; } - coin_selection_params.effective_fee = nFeeRateNeeded; if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coin_control, coin_selection_params, bnb_used)) { // If BnB was used, it was the first pass. No longer the first pass and continue loop with knapsack. -- cgit v1.2.3 From bcd716670ba8a189a2e9b8b035318abceb9ce631 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 4 Feb 2021 18:28:45 -0500 Subject: 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 99aafd8dfc..1ef89a41dd 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 f38ccba384..f375ce02a5 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 3e1d804000..b1951c0c3d 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 69cf6b66a4..65b57396bf 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() {} }; -- cgit v1.2.3 From 5fc381e443d6d967e6f7f8bc88a4fd66e18379eb Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 4 Feb 2021 19:11:24 -0500 Subject: 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. Github-Pull: #21083 Rebased-From: bdd0c2934b7f389ffcfae3b602ee3ecee8581acd --- 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 1ef89a41dd..df277dc2f6 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -52,7 +52,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); bench.run([&] { std::set setCoinsRet; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f375ce02a5..4686ecdf89 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); static void add_coin(const CAmount& nValue, int nInput, std::vector& set) @@ -267,7 +267,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); CoinSet setCoinsRet; CAmount nValueRet; @@ -640,11 +640,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); 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); CoinSet out_set; CAmount out_value = 0; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b1951c0c3d..a3216a33ae 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2363,7 +2363,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil std::vector utxo_pool; if (coin_selection_params.use_bnb) { // 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); // Filter by the min conf specs and add to utxo_pool and calculate effective value for (OutputGroup& group : groups) { @@ -2805,7 +2805,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); @@ -2924,7 +2925,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; @@ -2976,7 +2977,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 65b57396bf..20ee63e4ac 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -608,17 +608,19 @@ 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; 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) : + CFeeRate long_term_feerate, CFeeRate discard_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), + m_discard_feerate(discard_feerate), tx_noinputs_size(tx_noinputs_size) {} CoinSelectionParams() {} -- cgit v1.2.3 From d61fb07da7c12e4a1f68cf645f32d563a657a506 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 16 Mar 2021 16:19:03 -0400 Subject: Rename CoinSelectionParams::effective_fee to m_effective_feerate It's a feerate, not a fee. Also follow the style guide for member names. Github-Pull: #21083 Rebased-From: f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3 --- src/bench/coin_selection.cpp | 2 +- src/wallet/test/coinselector_tests.cpp | 10 +++++----- src/wallet/wallet.cpp | 18 +++++++++--------- src/wallet/wallet.h | 6 +++--- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index df277dc2f6..74ba98f8c2 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), + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), /* tx_no_inputs_size= */ 0); bench.run([&] { diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 4686ecdf89..46c4b5d697 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -36,7 +36,7 @@ CoinEligibilityFilter filter_standard(1, 6, 0); 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), + /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), /* tx_no_inputs_size= */ 0); @@ -266,7 +266,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), + /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(3000), /* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000), /* tx_no_inputs_size= */ 0); CoinSet setCoinsRet; @@ -300,7 +300,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CCoinControl coin_control; coin_control.fAllowOtherInputs = true; coin_control.Select(COutPoint(vCoins.at(0).tx->GetHash(), vCoins.at(0).i)); - coin_selection_params_bnb.effective_fee = CFeeRate(0); + coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); BOOST_CHECK(wallet->SelectCoins(vCoins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used)); BOOST_CHECK(bnb_used); BOOST_CHECK(coin_selection_params_bnb.use_bnb); @@ -639,11 +639,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), + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_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), + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), /* tx_no_inputs_size= */ 0); CoinSet out_set; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a3216a33ae..5c257aa7d8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2363,7 +2363,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil std::vector utxo_pool; if (coin_selection_params.use_bnb) { // Calculate cost of change - 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); + CAmount cost_of_change = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size) + coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.change_output_size); // Filter by the min conf specs and add to utxo_pool and calculate effective value for (OutputGroup& group : groups) { @@ -2373,14 +2373,14 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil // 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 */, coin_selection_params.m_long_term_feerate); } else { - group.SetFees(coin_selection_params.effective_fee, coin_selection_params.m_long_term_feerate); + group.SetFees(coin_selection_params.m_effective_feerate, coin_selection_params.m_long_term_feerate); } OutputGroup pos_group = group.GetPositiveOnlyGroup(); if (pos_group.effective_value > 0) utxo_pool.push_back(pos_group); } // 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.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); bnb_used = true; return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); } else { @@ -2437,7 +2437,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm if (coin.m_input_bytes <= 0) { return false; // Not solvable, can't estimate size for fee } - coin.effective_value = coin.txout.nValue - coin_selection_params.effective_fee.GetFee(coin.m_input_bytes); + 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 { @@ -2809,11 +2809,11 @@ bool CWallet::CreateTransactionInternal( 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); + coin_selection_params.m_effective_feerate = GetMinimumFeeRate(*this, coin_control, &feeCalc); // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly // provided one - if (coin_control.m_feerate && coin_selection_params.effective_fee > *coin_control.m_feerate) { - error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.effective_fee.ToString(FeeEstimateMode::SAT_VB)); + if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) { + error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB)); return false; } if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) { @@ -2962,7 +2962,7 @@ bool CWallet::CreateTransactionInternal( return false; } - nFeeNeeded = coin_selection_params.effective_fee.GetFee(nBytes); + nFeeNeeded = coin_selection_params.m_effective_feerate.GetFee(nBytes); if (nFeeRet >= nFeeNeeded) { // Reduce fee to only the needed amount if possible. This // prevents potential overpayment in fees if the coins @@ -2976,7 +2976,7 @@ bool CWallet::CreateTransactionInternal( // change output. Only try this once. 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 fee_needed_with_change = coin_selection_params.m_effective_feerate.GetFee(tx_size_with_change); 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; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 20ee63e4ac..6fde2fa79d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -606,19 +606,19 @@ struct CoinSelectionParams bool use_bnb = true; size_t change_output_size = 0; size_t change_spend_size = 0; - CFeeRate effective_fee = CFeeRate(0); + CFeeRate m_effective_feerate; 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; - 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_feerate, CFeeRate long_term_feerate, CFeeRate discard_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_effective_feerate(effective_feerate), m_long_term_feerate(long_term_feerate), m_discard_feerate(discard_feerate), tx_noinputs_size(tx_noinputs_size) -- cgit v1.2.3