Move fee setting of OutputGroup to Insert

OutputGroup will handle the fee and effective value computations
inside of Insert. It now needs to take the effective feerate and long
term feerates as arguments to its constructor.
This commit is contained in:
Andrew Chow 2020-08-31 15:56:30 -04:00
parent 6148a8acda
commit 99b399aba5
4 changed files with 33 additions and 38 deletions

View file

@ -302,6 +302,16 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group
void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) { void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) {
m_outputs.push_back(output); m_outputs.push_back(output);
CInputCoin& coin = m_outputs.back();
coin.m_fee = coin.m_input_bytes < 0 ? 0 : m_effective_feerate.GetFee(coin.m_input_bytes);
fee += coin.m_fee;
coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : m_long_term_feerate.GetFee(coin.m_input_bytes);
long_term_fee += coin.m_long_term_fee;
coin.effective_value = coin.txout.nValue - coin.m_fee;
effective_value += coin.effective_value;
m_from_me &= from_me; m_from_me &= from_me;
m_value += output.txout.nValue; m_value += output.txout.nValue;
m_depth = std::min(m_depth, depth); m_depth = std::min(m_depth, depth);
@ -312,9 +322,6 @@ void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size
// descendants is the count as seen from the top ancestor, not the descendants as seen from the // descendants is the count as seen from the top ancestor, not the descendants as seen from the
// coin itself; thus, this value is counted as the max, not the sum // coin itself; thus, this value is counted as the max, not the sum
m_descendants = std::max(m_descendants, descendants); m_descendants = std::max(m_descendants, descendants);
effective_value += output.effective_value;
fee += output.m_fee;
long_term_fee += output.m_long_term_fee;
} }
std::vector<CInputCoin>::iterator OutputGroup::Discard(const CInputCoin& output) { std::vector<CInputCoin>::iterator OutputGroup::Discard(const CInputCoin& output) {
@ -335,23 +342,6 @@ bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_f
&& m_descendants <= eligibility_filter.max_descendants; && m_descendants <= eligibility_filter.max_descendants;
} }
void OutputGroup::SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate)
{
fee = 0;
long_term_fee = 0;
effective_value = 0;
for (CInputCoin& coin : m_outputs) {
coin.m_fee = coin.m_input_bytes < 0 ? 0 : effective_feerate.GetFee(coin.m_input_bytes);
fee += coin.m_fee;
coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes);
long_term_fee += coin.m_long_term_fee;
coin.effective_value = coin.txout.nValue - coin.m_fee;
effective_value += coin.effective_value;
}
}
OutputGroup OutputGroup::GetPositiveOnlyGroup() OutputGroup OutputGroup::GetPositiveOnlyGroup()
{ {
OutputGroup group(*this); OutputGroup group(*this);

View file

@ -6,11 +6,10 @@
#define BITCOIN_WALLET_COINSELECTION_H #define BITCOIN_WALLET_COINSELECTION_H
#include <amount.h> #include <amount.h>
#include <policy/feerate.h>
#include <primitives/transaction.h> #include <primitives/transaction.h>
#include <random.h> #include <random.h>
class CFeeRate;
//! target minimum change amount //! target minimum change amount
static constexpr CAmount MIN_CHANGE{COIN / 100}; static constexpr CAmount MIN_CHANGE{COIN / 100};
//! final minimum change amount after paying for fees //! final minimum change amount after paying for fees
@ -78,15 +77,20 @@ struct OutputGroup
size_t m_descendants{0}; size_t m_descendants{0};
CAmount effective_value{0}; CAmount effective_value{0};
CAmount fee{0}; CAmount fee{0};
CFeeRate m_effective_feerate{0};
CAmount long_term_fee{0}; CAmount long_term_fee{0};
CFeeRate m_long_term_feerate{0};
OutputGroup() {} OutputGroup() {}
OutputGroup(const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate) :
m_effective_feerate(effective_feerate),
m_long_term_feerate(long_term_feerate)
{}
void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants); void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants);
std::vector<CInputCoin>::iterator Discard(const CInputCoin& output); std::vector<CInputCoin>::iterator Discard(const CInputCoin& output);
bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const;
//! Update the OutputGroup's fee, long_term_fee, and effective_value based on the given feerates
void SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate);
OutputGroup GetPositiveOnlyGroup(); OutputGroup GetPositiveOnlyGroup();
}; };

View file

@ -2381,7 +2381,14 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
temp.m_confirm_target = 1008; temp.m_confirm_target = 1008;
CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc); CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc);
std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, eligibility_filter.max_ancestors); // 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;
}
std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, eligibility_filter.max_ancestors, effective_feerate, long_term_feerate);
// Calculate cost of change // 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 = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size);
@ -2390,13 +2397,6 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
for (OutputGroup& group : groups) { for (OutputGroup& group : groups) {
if (!group.EligibleForSpending(eligibility_filter)) continue; if (!group.EligibleForSpending(eligibility_filter)) continue;
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);
} else {
group.SetFees(coin_selection_params.effective_fee, long_term_feerate);
}
OutputGroup pos_group = group.GetPositiveOnlyGroup(); OutputGroup pos_group = group.GetPositiveOnlyGroup();
if (pos_group.effective_value > 0) utxo_pool.push_back(pos_group); if (pos_group.effective_value > 0) utxo_pool.push_back(pos_group);
} }
@ -2405,7 +2405,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
bnb_used = true; bnb_used = true;
return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees);
} else { } else {
std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, eligibility_filter.max_ancestors); std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, eligibility_filter.max_ancestors, CFeeRate(0), CFeeRate(0));
// Filter by the min conf specs and add to utxo_pool // Filter by the min conf specs and add to utxo_pool
for (const OutputGroup& group : groups) { for (const OutputGroup& group : groups) {
@ -4206,7 +4206,7 @@ bool CWalletTx::IsImmatureCoinBase() const
return GetBlocksToMaturity() > 0; return GetBlocksToMaturity() > 0;
} }
std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const { std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate) const {
std::vector<OutputGroup> groups; std::vector<OutputGroup> groups;
std::map<CTxDestination, OutputGroup> gmap; std::map<CTxDestination, OutputGroup> gmap;
std::set<CTxDestination> full_groups; std::set<CTxDestination> full_groups;
@ -4228,15 +4228,16 @@ std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outpu
// high amount of fees. // high amount of fees.
if (it->second.m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) { if (it->second.m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) {
groups.push_back(it->second); groups.push_back(it->second);
it->second = OutputGroup{}; it->second = OutputGroup{effective_feerate, long_term_feerate};
full_groups.insert(dst); full_groups.insert(dst);
} }
it->second.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); it->second.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants);
} else { } else {
gmap[dst].Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); auto ins = gmap.emplace(dst, OutputGroup{effective_feerate, long_term_feerate});
ins.first->second.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants);
} }
} else { } else {
groups.emplace_back(); groups.emplace_back(effective_feerate, long_term_feerate);
groups.back().Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); groups.back().Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants);
} }
} }

View file

@ -841,7 +841,7 @@ public:
bool IsSpentKey(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsSpentKey(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const; std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate) const;
bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);