Skip to content

Commit

Permalink
wallet: Move discard feerate fetching to CreateTransaction
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@achow101
achow101 committed Mar 16, 2021
1 parent 362a735 commit cca8b00
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/bench/coin_selection.cpp
View file
Expand Up @@ -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<CInputCoin> setCoinsRet;
Expand Down
8 changes: 4 additions & 4 deletions src/wallet/test/coinselector_tests.cpp
View file
Expand Up @@ -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<CInputCoin>& set)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion src/wallet/wallet.cpp
View file
Expand Up @@ -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 */);

// 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);
Expand Down Expand Up @@ -2819,6 +2819,9 @@ bool CWallet::CreateTransactionInternal(
cc_temp.m_confirm_target = 1008;
coin_selection_params.m_long_term_feerate = GetMinimumFeeRate(*this, cc_temp, nullptr);

// Set discard feerate
coin_selection_params.m_discard_feerate = GetDiscardRate(*this);

nFeeRet = 0;
bool pick_new_inputs = true;
CAmount nValueIn = 0;
Expand Down
4 changes: 3 additions & 1 deletion src/wallet/wallet.h
View file
Expand Up @@ -608,18 +608,20 @@ struct CoinSelectionParams
size_t change_spend_size = 0;
CFeeRate effective_fee = CFeeRate(0);
CFeeRate m_long_term_feerate = CFeeRate(0);
CFeeRate m_discard_feerate = CFeeRate(0);
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)
{}
Expand Down

0 comments on commit cca8b00

Please sign in to comment.