Skip to content

Commit

Permalink
wallet: Move long term feerate setting to CreateTransaction
Browse files Browse the repository at this point in the history 
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: bitcoin#21083
Rebased-From: 448d04b
  • Loading branch information
@achow101
achow101 committed Mar 24, 2021
1 parent 34c89f9 commit bcd7166
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 14 deletions.
5 changes: 4 additions & 1 deletion src/bench/coin_selection.cpp
View file
Expand Up @@ -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<CInputCoin> setCoinsRet;
CAmount nValueRet;
Expand Down
20 changes: 16 additions & 4 deletions src/wallet/test/coinselector_tests.cpp
View file
Expand Up @@ -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<CInputCoin>& set)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 7 additions & 8 deletions src/wallet/wallet.cpp
View file
Expand Up @@ -2362,12 +2362,6 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil

std::vector<OutputGroup> 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);

Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion src/wallet/wallet.h
View file
Expand Up @@ -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() {}
};

Expand Down

0 comments on commit bcd7166

Please sign in to comment.