diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 3fd0280b8b5..9cf61dbe514 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -387,13 +387,6 @@ void OutputGroupTypeMap::Push(const OutputGroup& group, OutputType type, bool in } } -std::optional OutputGroupTypeMap::Find(OutputType type) -{ - auto it_by_type = groups_by_type.find(type); - if (it_by_type == groups_by_type.end()) return std::nullopt; - return it_by_type->second; -} - CAmount GetSelectionWaste(const std::set>& inputs, CAmount change_cost, CAmount target, bool use_effective_value) { // This function should not be called with empty inputs as that would mean the selection failed diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 3abd22c2071..5a7b748be15 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -153,6 +153,11 @@ struct CoinSelectionParams { * associated with the same address. This helps reduce privacy leaks resulting from address * reuse. Dust outputs are not eligible to be added to output groups and thus not considered. */ bool m_avoid_partial_spends = false; + /** + * When true, allow unsafe coins to be selected during Coin Selection. This may spend unconfirmed outputs: + * 1) Received from other wallets, 2) replacing other txs, 3) that have been replaced. + */ + bool m_include_unsafe_inputs = false; CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size, CAmount min_change_target, CFeeRate effective_feerate, @@ -222,8 +227,6 @@ struct OutputGroup CAmount effective_value{0}; /** The fee to spend these UTXOs at the effective feerate. */ CAmount fee{0}; - /** The target feerate of the transaction we're trying to build. */ - CFeeRate m_effective_feerate{0}; /** The fee to spend these UTXOs at the long term feerate. */ CAmount long_term_fee{0}; /** The feerate for spending a created change output eventually (i.e. not urgently, and thus at @@ -238,7 +241,6 @@ struct OutputGroup OutputGroup() {} OutputGroup(const CoinSelectionParams& params) : - m_effective_feerate(params.m_effective_feerate), m_long_term_feerate(params.m_long_term_feerate), m_subtract_fee_outputs(params.m_subtract_fee_outputs) {} @@ -266,8 +268,6 @@ struct OutputGroupTypeMap // Based on the insert flag; appends group to the 'mixed_group' and, if value > 0, to the 'positive_group'. // This affects both; the groups filtered by type and the overall groups container. void Push(const OutputGroup& group, OutputType type, bool insert_positive, bool insert_mixed); - // Retrieves 'Groups' filtered by type - std::optional Find(OutputType type); // Different output types count size_t TypesCount() { return groups_by_type.size(); } }; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index a8ecce119af..5771d33b7a8 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -415,9 +415,6 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, // Allowing partial spends means no grouping. Each COutput gets its own OutputGroup for (const auto& [type, outputs] : coins.coins) { for (const COutput& output : outputs) { - // Skip outputs we cannot spend - if (!output.spendable) continue; - // Get mempool info size_t ancestors, descendants; wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); @@ -444,11 +441,10 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, // to the last OutputGroup in the vector for the scriptPubKey. When the last OutputGroup has // OUTPUT_GROUP_MAX_ENTRIES COutputs, a new OutputGroup is added to the end of the vector. typedef std::map, std::vector> ScriptPubKeyToOutgroup; - const auto& group_outputs = []( - const COutput& output, OutputType type, size_t ancestors, size_t descendants, - ScriptPubKeyToOutgroup& groups_map, const CoinSelectionParams& coin_sel_params, - bool positive_only) { - std::vector& groups = groups_map[std::make_pair(output.txout.scriptPubKey,type)]; + const auto& insert_output = [&]( + const std::shared_ptr& output, OutputType type, size_t ancestors, size_t descendants, + ScriptPubKeyToOutgroup& groups_map) { + std::vector& groups = groups_map[std::make_pair(output->txout.scriptPubKey,type)]; if (groups.size() == 0) { // No OutputGroups for this scriptPubKey yet, add one @@ -467,25 +463,24 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, group = &groups.back(); } - // Filter for positive only before adding the output to group - if (!positive_only || output.GetEffectiveValue() > 0) { - group->Insert(std::make_shared(output), ancestors, descendants); - } + group->Insert(output, ancestors, descendants); }; ScriptPubKeyToOutgroup spk_to_groups_map; ScriptPubKeyToOutgroup spk_to_positive_groups_map; for (const auto& [type, outs] : coins.coins) { for (const COutput& output : outs) { - // Skip outputs we cannot spend - if (!output.spendable) continue; - size_t ancestors, descendants; wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); - group_outputs(output, type, ancestors, descendants, spk_to_groups_map, coin_sel_params, /*positive_only=*/ false); - group_outputs(output, type, ancestors, descendants, spk_to_positive_groups_map, - coin_sel_params, /*positive_only=*/ true); + const auto& shared_output = std::make_shared(output); + // Filter for positive only before adding the output + if (output.GetEffectiveValue() > 0) { + insert_output(shared_output, type, ancestors, descendants, spk_to_positive_groups_map); + } + + // 'All' groups + insert_output(shared_output, type, ancestors, descendants, spk_to_groups_map); } } @@ -622,7 +617,7 @@ util::Result SelectCoins(const CWallet& wallet, CoinsResult& av } // Start wallet Coin Selection procedure - auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_control, coin_selection_params); + auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_selection_params); if (!op_selection_result) return op_selection_result; // If needed, add preset inputs to the automatic coin selection result @@ -637,7 +632,7 @@ util::Result SelectCoins(const CWallet& wallet, CoinsResult& av return op_selection_result; } -util::Result AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) +util::Result AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CoinSelectionParams& coin_selection_params) { unsigned int limit_ancestor_count = 0; unsigned int limit_descendant_count = 0; @@ -646,12 +641,10 @@ util::Result AutomaticCoinSelection(const CWallet& wallet, Coin const size_t max_descendants = (size_t)std::max(1, limit_descendant_count); const bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); - // form groups from remaining coins; note that preset coins will not - // automatically have their associated (same address) coins included - if (coin_control.m_avoid_partial_spends && available_coins.Size() > OUTPUT_GROUP_MAX_ENTRIES) { - // Cases where we have 101+ outputs all pointing to the same destination may result in - // privacy leaks as they will potentially be deterministically sorted. We solve that by - // explicitly shuffling the outputs before processing + // Cases where we have 101+ outputs all pointing to the same destination may result in + // privacy leaks as they will potentially be deterministically sorted. We solve that by + // explicitly shuffling the outputs before processing + if (coin_selection_params.m_avoid_partial_spends && available_coins.Size() > OUTPUT_GROUP_MAX_ENTRIES) { available_coins.Shuffle(coin_selection_params.rng_fast); } @@ -678,7 +671,7 @@ util::Result AutomaticCoinSelection(const CWallet& wallet, Coin ordered_filters.push_back({CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, /*include_partial=*/true)}); // Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs // received from other wallets. - if (coin_control.m_include_unsafe_inputs) { + if (coin_selection_params.m_include_unsafe_inputs) { ordered_filters.push_back({CoinEligibilityFilter(/*conf_mine=*/0, /*conf_theirs*/0, max_ancestors-1, max_descendants-1, /*include_partial=*/true)}); } // Try with unlimited ancestors/descendants. The transaction will still need to meet @@ -809,6 +802,7 @@ static util::Result CreateTransactionInternal( CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends; + coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs; // Set the long term feerate estimate to the wallet's consolidate feerate coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index b8bc82db7aa..78c2c5f22b9 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -191,7 +191,7 @@ util::Result FetchSelectedInputs(const CWallet& wallet, const * or (2) an specific error message if there was something particularly wrong (e.g. a selection * result that surpassed the tx max weight size). */ -util::Result AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, +util::Result AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); /** diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 16eee1e050a..e8c18dbb675 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -592,12 +592,6 @@ public: bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool DummySignTx(CMutableTransaction &txNew, const std::set &txouts, const CCoinControl* coin_control = nullptr) const - { - std::vector v_txouts(txouts.size()); - std::copy(txouts.begin(), txouts.end(), v_txouts.begin()); - return DummySignTx(txNew, v_txouts, coin_control); - } bool DummySignTx(CMutableTransaction &txNew, const std::vector &txouts, const CCoinControl* coin_control = nullptr) const; bool ImportScripts(const std::set scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);