Merge d25e28c20b into merged_master (Bitcoin PR #21083)

This commit is contained in:
Andrew Poelstra 2021-06-27 14:26:06 +00:00
commit b1fab9a7e6
4 changed files with 53 additions and 35 deletions

View file

@ -51,7 +51,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, false);
const CoinSelectionParams coin_selection_params(/* use_bnb= */ true, /* change_output_size= */ 34,
/* change_spend_size= */ 148, /* effective_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;
CAmountMap mapValueRet;

View file

@ -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, false);
CoinSelectionParams coin_selection_params(/* use_bnb= */ false, /* change_output_size= */ 0,
/* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(0),
/* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
/* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
// ELEMENTS: helper function wrapping a single-asset call to SelectCoinsMinConf
static bool SimpleSelectCoinsMinConf(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins,
@ -284,7 +287,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, false);
CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 0,
/* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(3000),
/* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000),
/* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
CoinSet setCoinsRet;
CAmount nValueRet;
bool bnb_used;
@ -316,7 +322,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);
CAmountMap mapTargetValue;
mapTargetValue[CAsset()] = 10 * CENT;
CAmountMap mapValueRet;
@ -657,8 +663,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, false);
CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0, false);
CoinSelectionParams coin_selection_params_knapsack(/* use_bnb= */ false, /* change_output_size= */ 34,
/* change_spend_size= */ 148, /* effective_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_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;
bool bnb_used = false;

View file

@ -2486,20 +2486,14 @@ bool CWallet::SelectCoinsMinConf(const CAmountMap& mapTargetValue, const CoinEli
mapValueRet.clear();
if (coin_selection_params.use_bnb && mapTargetValue.size() == 1) {
// Get long term estimate
FeeCalculation feeCalc;
CCoinControl temp;
temp.m_confirm_target = 1008;
CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc);
// Get the feerate for effective value.
// When subtracting the fee from the outputs, we want the effective feerate to be 0
CFeeRate effective_feerate{0};
if (!coin_selection_params.m_subtract_fee_outputs) {
effective_feerate = coin_selection_params.effective_fee;
effective_feerate = coin_selection_params.m_effective_feerate;
}
std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, long_term_feerate, eligibility_filter, true /* positive_only */);
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 */);
// ELEMENTS:
CAsset asset = mapTargetValue.begin()->first;
@ -2522,10 +2516,10 @@ bool CWallet::SelectCoinsMinConf(const CAmountMap& mapTargetValue, const CoinEli
// END ELEMENTS
// 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.m_effective_feerate.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);
CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size);
bnb_used = true;
CAmount nValueRet;
bool ret = SelectCoinsBnB(groups, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees);
@ -2628,7 +2622,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
coin_selection_params.use_bnb = false;
coin.m_input_bytes = 0;
}
coin.effective_value = coin.value - coin_selection_params.effective_fee.GetFee(coin.m_input_bytes);
coin.effective_value = coin.value - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes);
if (coin_selection_params.use_bnb) {
value_to_select[coin.asset] -= coin.effective_value;
} else {
@ -3396,16 +3390,27 @@ bool CWallet::CreateTransactionInternal(
coin_selection_params.change_output_size += (MAX_RANGEPROOF_SIZE + DEFAULT_SURJECTIONPROOF_SIZE + WITNESS_SCALE_FACTOR - 1)/WITNESS_SCALE_FACTOR;
}
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
CFeeRate nFeeRateNeeded = 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 && 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.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) {
// eventually allow a fallback fee
error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.");
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);
// ELEMENTS:
// Start with tiny non-zero fee for issuance entropy and loop until there is enough fee
@ -3523,7 +3528,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, mapValueToSelect, setCoins, mapValueIn, 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.
@ -3560,7 +3564,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 (assetChange.first == policyAsset && (IsDust(newTxOut, discard_rate) || bnb_used))
if (assetChange.first == policyAsset && (IsDust(newTxOut, coin_selection_params.m_discard_feerate) || bnb_used))
{
vChangePosInOut.erase(assetChange.first);
nFeeRet += assetChange.second;
@ -3743,13 +3747,7 @@ bool CWallet::CreateTransactionInternal(
txNew = blind_details->tx_unblinded_unsigned;
}
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.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
@ -3763,8 +3761,8 @@ 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 minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate);
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;
nFeeRet = fee_needed_with_change;

View file

@ -674,17 +674,22 @@ 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;
bool m_avoid_partial_spends = false;
CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size, bool avoid_partial) :
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, bool avoid_partial) :
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),
m_avoid_partial_spends(avoid_partial)
{}