aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/bench/coin_selection.cpp2
-rw-r--r--src/wallet/coinselection.cpp40
-rw-r--r--src/wallet/coinselection.h5
-rw-r--r--src/wallet/test/coinselector_tests.cpp4
-rw-r--r--src/wallet/wallet.cpp26
-rw-r--r--src/wallet/wallet.h2
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);
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-01 07:39:54 +0000