diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/bench/coin_selection.cpp | 2 | ||||
-rw-r--r-- | src/wallet/coinselection.cpp | 40 | ||||
-rw-r--r-- | src/wallet/coinselection.h | 5 | ||||
-rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 4 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 26 | ||||
-rw-r--r-- | src/wallet/wallet.h | 2 |
6 files changed, 27 insertions, 52 deletions
diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 6cb45ec7a5..6af7e785de 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -75,7 +75,7 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup> tx.vout[nInput].nValue = nValue; std::unique_ptr<CWalletTx> wtx = MakeUnique<CWalletTx>(&testWallet, MakeTransactionRef(std::move(tx))); set.emplace_back(); - set.back().Insert(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0); + set.back().Insert(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0, false); wtxn.emplace_back(std::move(wtx)); } // Copied from src/wallet/test/coinselector_tests.cpp diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index f4843dd0ce..8032deb2fa 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -300,16 +300,24 @@ 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, bool positive_only) { + // Compute the effective value first + const CAmount coin_fee = output.m_input_bytes < 0 ? 0 : m_effective_feerate.GetFee(output.m_input_bytes); + const CAmount ev = output.txout.nValue - coin_fee; + + // Filter for positive only here before adding the coin + if (positive_only && ev <= 0) return; + 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); + + coin.m_fee = coin_fee; 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; + coin.effective_value = ev; effective_value += coin.effective_value; m_from_me &= from_me; @@ -324,35 +332,9 @@ void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size m_descendants = std::max(m_descendants, descendants); } -std::vector<CInputCoin>::iterator OutputGroup::Discard(const CInputCoin& output) { - auto it = m_outputs.begin(); - while (it != m_outputs.end() && it->outpoint != output.outpoint) ++it; - if (it == m_outputs.end()) return it; - m_value -= output.txout.nValue; - effective_value -= output.effective_value; - fee -= output.m_fee; - long_term_fee -= output.m_long_term_fee; - return m_outputs.erase(it); -} - bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const { return m_depth >= (m_from_me ? eligibility_filter.conf_mine : eligibility_filter.conf_theirs) && m_ancestors <= eligibility_filter.max_ancestors && m_descendants <= eligibility_filter.max_descendants; } - -OutputGroup OutputGroup::GetPositiveOnlyGroup() -{ - OutputGroup group(*this); - for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) { - const CInputCoin& coin = *it; - // Only include outputs that are positive effective value (i.e. not dust) - if (coin.effective_value <= 0) { - it = group.Discard(coin); - } else { - ++it; - } - } - return group; -} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 721f1a22eb..6e3b8c3915 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -87,11 +87,8 @@ struct OutputGroup m_long_term_feerate(long_term_feerate) {} - void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants); - std::vector<CInputCoin>::iterator Discard(const CInputCoin& output); + void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only); bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; - - OutputGroup GetPositiveOnlyGroup(); }; bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 9af6ddd7cf..b5c9e62b5c 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -116,7 +116,7 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<CInputCoin>& coins static_groups.clear(); for (auto& coin : coins) { static_groups.emplace_back(); - static_groups.back().Insert(coin, 0, true, 0, 0); + static_groups.back().Insert(coin, 0, true, 0, 0, false); } return static_groups; } @@ -127,7 +127,7 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins) static_groups.clear(); for (auto& coin : coins) { static_groups.emplace_back(); - static_groups.back().Insert(coin.GetInputCoin(), coin.nDepth, coin.tx->m_amounts[CWalletTx::DEBIT].m_cached[ISMINE_SPENDABLE] && coin.tx->m_amounts[CWalletTx::DEBIT].m_value[ISMINE_SPENDABLE] == 1 /* HACK: we can't figure out the is_me flag so we use the conditions defined above; perhaps set safe to false for !fIsFromMe in add_coin() */, 0, 0); + static_groups.back().Insert(coin.GetInputCoin(), coin.nDepth, coin.tx->m_amounts[CWalletTx::DEBIT].m_cached[ISMINE_SPENDABLE] && coin.tx->m_amounts[CWalletTx::DEBIT].m_value[ISMINE_SPENDABLE] == 1 /* HACK: we can't figure out the is_me flag so we use the conditions defined above; perhaps set safe to false for !fIsFromMe in add_coin() */, 0, 0, false); } return static_groups; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f2c035da14..df0e39b301 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2374,7 +2374,6 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil nValueRet = 0; if (coin_selection_params.use_bnb) { - std::vector<OutputGroup> utxo_pool; // Get long term estimate FeeCalculation feeCalc; CCoinControl temp; @@ -2388,22 +2387,17 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil 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, eligibility_filter); + std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, eligibility_filter.max_ancestors, effective_feerate, 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); - // Filter by the min conf specs and add to utxo_pool and calculate effective value - for (OutputGroup& group : groups) { - 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); bnb_used = true; - return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); + return SelectCoinsBnB(groups, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); } else { - std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, eligibility_filter.max_ancestors, CFeeRate(0), CFeeRate(0), eligibility_filter); + std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, eligibility_filter.max_ancestors, CFeeRate(0), CFeeRate(0), eligibility_filter, false /* positive_only */); bnb_used = false; return KnapsackSolver(nTargetValue, groups, setCoinsRet, nValueRet); @@ -4199,7 +4193,7 @@ bool CWalletTx::IsImmatureCoinBase() const return GetBlocksToMaturity() > 0; } -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 CoinEligibilityFilter& filter) 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 CoinEligibilityFilter& filter, bool positive_only) const { std::vector<OutputGroup> groups; std::map<CTxDestination, OutputGroup> gmap; std::set<CTxDestination> full_groups; @@ -4224,16 +4218,17 @@ std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outpu it->second = OutputGroup{effective_feerate, long_term_feerate}; 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, positive_only); } else { 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); + ins.first->second.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants, positive_only); } } else { // This is for if each output gets it's own OutputGroup OutputGroup coin(effective_feerate, long_term_feerate); - coin.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); - if (coin.EligibleForSpending(filter)) groups.push_back(coin); + coin.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants, positive_only); + if (positive_only && coin.effective_value <= 0) continue; + if (coin.m_outputs.size() > 0 && coin.EligibleForSpending(filter)) groups.push_back(coin); } } } @@ -4245,7 +4240,8 @@ std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outpu group.m_ancestors = max_ancestors - 1; } // If the OutputGroup is not eligible, don't add it - if (group.EligibleForSpending(filter)) groups.push_back(group); + if (positive_only && group.effective_value <= 0) continue; + if (group.m_outputs.size() > 0 && group.EligibleForSpending(filter)) groups.push_back(group); } } return groups; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1c523d86b8..da14b0108c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -841,7 +841,7 @@ public: 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); - 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 CoinEligibilityFilter& filter) 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 CoinEligibilityFilter& filter, bool positive_only) const; bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); |