diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/coinselection.cpp | 12 | ||||
-rw-r--r-- | src/wallet/coinselection.h | 14 | ||||
-rw-r--r-- | src/wallet/spend.cpp | 131 | ||||
-rw-r--r-- | src/wallet/spend.h | 24 | ||||
-rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 2 |
5 files changed, 110 insertions, 73 deletions
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 59c6958d6b..e6ba89627c 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -448,7 +448,8 @@ void SelectionResult::Clear() void SelectionResult::AddInput(const OutputGroup& group) { - util::insert(m_selected_inputs, group.m_outputs); + // As it can fail, combine inputs first + InsertInputs(group.m_outputs); m_use_effective = !group.m_subtract_fee_outputs; m_weight += group.m_weight; @@ -456,7 +457,8 @@ void SelectionResult::AddInput(const OutputGroup& group) void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs) { - util::insert(m_selected_inputs, inputs); + // As it can fail, combine inputs first + InsertInputs(inputs); m_use_effective = !subtract_fee_outputs; m_weight += std::accumulate(inputs.cbegin(), inputs.cend(), 0, [](int sum, const auto& coin) { @@ -466,16 +468,14 @@ void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_f void SelectionResult::Merge(const SelectionResult& other) { - // Obtain the expected selected inputs count after the merge (for now, duplicates are not allowed) - const size_t expected_count = m_selected_inputs.size() + other.m_selected_inputs.size(); + // As it can fail, combine inputs first + InsertInputs(other.m_selected_inputs); m_target += other.m_target; m_use_effective |= other.m_use_effective; if (m_algo == SelectionAlgorithm::MANUAL) { m_algo = other.m_algo; } - util::insert(m_selected_inputs, other.m_selected_inputs); - assert(m_selected_inputs.size() == expected_count); m_weight += other.m_weight; } diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 2efeb8476d..9ff2011ce3 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -10,6 +10,8 @@ #include <policy/feerate.h> #include <primitives/transaction.h> #include <random.h> +#include <util/system.h> +#include <util/check.h> #include <optional> @@ -186,6 +188,7 @@ struct CoinEligibilityFilter /** When avoid_reuse=true and there are full groups (OUTPUT_GROUP_MAX_ENTRIES), whether or not to use any partial groups.*/ const bool m_include_partial_groups{false}; + CoinEligibilityFilter() = delete; CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_ancestors) {} CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants) {} CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants, bool include_partial) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants), m_include_partial_groups(include_partial) {} @@ -301,6 +304,17 @@ private: /** Total weight of the selected inputs */ int m_weight{0}; + template<typename T> + void InsertInputs(const T& inputs) + { + // Store sum of combined input sets to check that the results have no shared UTXOs + const size_t expected_count = m_selected_inputs.size() + inputs.size(); + util::insert(m_selected_inputs, inputs); + if (m_selected_inputs.size() != expected_count) { + throw std::runtime_error(STR_INTERNAL_BUG("Shared UTXOs among selection results")); + } + } + public: explicit SelectionResult(const CAmount target, SelectionAlgorithm algo) : m_target(target), m_algo(algo) {} diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 4d2182e079..0ec9e5411a 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -510,15 +510,20 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C return groups_out; } -std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins, +// Returns true if the result contains an error and the message is not empty +static bool HasErrorMsg(const util::Result<SelectionResult>& res) { return !util::ErrorString(res).empty(); } + +util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins, const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types) { // Run coin selection on each OutputType and compute the Waste Metric std::vector<SelectionResult> results; for (const auto& it : available_coins.coins) { - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}) { - results.push_back(*result); - } + auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}; + // If any specific error message appears here, then something particularly wrong happened. + if (HasErrorMsg(result)) return result; // So let's return the specific error. + // Append the favorable result. + if (result) results.push_back(*result); } // If we have at least one solution for funding the transaction without mixing, choose the minimum one according to waste metric // and return the result @@ -528,16 +533,14 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm // over all available coins, which would allow mixing. // If TypesCount() <= 1, there is nothing to mix. if (allow_mixed_output_types && available_coins.TypesCount() > 1) { - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) { - return result; - } + return ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params); } // Either mixing is not allowed and we couldn't find a solution from any single OutputType, or mixing was allowed and we still couldn't // find a solution using all available coins - return std::nullopt; + return util::Error(); }; -std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, const CoinSelectionParams& coin_selection_params) +util::Result<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, const CoinSelectionParams& coin_selection_params) { // Vector of results. We will choose the best one based on waste. std::vector<SelectionResult> results; @@ -561,7 +564,7 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons if (results.empty()) { // No solution found - return std::nullopt; + return util::Error(); } std::vector<SelectionResult> eligible_results; @@ -571,7 +574,8 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons }); if (eligible_results.empty()) { - return std::nullopt; + return util::Error{_("The inputs size exceeds the maximum weight. " + "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")}; } // Choose the result with the least waste @@ -580,15 +584,18 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons return best_result; } -std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, - const CAmount& nTargetValue, const CCoinControl& coin_control, - const CoinSelectionParams& coin_selection_params) +util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, + const CAmount& nTargetValue, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) { // Deduct preset inputs amount from the search target CAmount selection_target = nTargetValue - pre_set_inputs.total_amount; // Return if automatic coin selection is disabled, and we don't cover the selection target - if (!coin_control.m_allow_other_inputs && selection_target > 0) return std::nullopt; + if (!coin_control.m_allow_other_inputs && selection_target > 0) { + return util::Error{_("The preselected coins total amount does not cover the transaction target. " + "Please allow other inputs to be automatically selected or include more coins manually")}; + } // Return if we can cover the target only with the preset inputs if (selection_target <= 0) { @@ -603,7 +610,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a CAmount available_coins_total_amount = coin_selection_params.m_subtract_fee_outputs ? available_coins.GetTotalAmount() : (available_coins.GetEffectiveTotalAmount().has_value() ? *available_coins.GetEffectiveTotalAmount() : 0); if (selection_target > available_coins_total_amount) { - return std::nullopt; // Insufficient funds + return util::Error(); // Insufficient funds } // Start wallet Coin Selection procedure @@ -622,7 +629,12 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a return op_selection_result; } -std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) +struct SelectionFilter { + CoinEligibilityFilter filter; + bool allow_mixed_output_types{true}; +}; + +util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) { unsigned int limit_ancestor_count = 0; unsigned int limit_descendant_count = 0; @@ -643,54 +655,56 @@ std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coi // Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the // transaction at a target feerate. If an attempt fails, more attempts may be made using a more // permissive CoinEligibilityFilter. - std::optional<SelectionResult> res = [&] { - // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six - // confirmations on outputs received from other wallets and only spend confirmed change. - if (auto r1{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/false)}) return r1; - // Allow mixing only if no solution from any single output type can be found - if (auto r2{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 1, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) return r2; - + util::Result<SelectionResult> res = [&] { + // Place coins eligibility filters on a scope increasing order. + std::vector<SelectionFilter> ordered_filters{ + // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six + // confirmations on outputs received from other wallets and only spend confirmed change. + {CoinEligibilityFilter(1, 6, 0), /*allow_mixed_output_types=*/false}, + {CoinEligibilityFilter(1, 1, 0)}, + }; // Fall back to using zero confirmation change (but with as few ancestors in the mempool as // possible) if we cannot fund the transaction otherwise. if (wallet.m_spend_zero_conf_change) { - if (auto r3{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, 2), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) return r3; - if (auto r4{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), - available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) { - return r4; - } - if (auto r5{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), - available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) { - return r5; - } + ordered_filters.push_back({CoinEligibilityFilter(0, 1, 2)}); + ordered_filters.push_back({CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3))}); + ordered_filters.push_back({CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2)}); // If partial groups are allowed, relax the requirement of spending OutputGroups (groups // of UTXOs sent to the same address, which are obviously controlled by a single wallet) // in their entirety. - if (auto r6{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, /*include_partial=*/true), - available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) { - return r6; - } + 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 (auto r7{AttemptSelection(wallet, value_to_select, - CoinEligibilityFilter(/*conf_mine=*/0, /*conf_theirs=*/0, max_ancestors-1, max_descendants-1, /*include_partial=*/true), - available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) { - return r7; - } + 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 // mempool ancestor/descendant policy to be accepted to mempool and broadcasted, but // OutputGroups use heuristics that may overestimate ancestor/descendant counts. if (!fRejectLongChains) { - if (auto r8{AttemptSelection(wallet, value_to_select, - CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), std::numeric_limits<uint64_t>::max(), /*include_partial=*/true), - available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) { - return r8; - } + ordered_filters.push_back({CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), + std::numeric_limits<uint64_t>::max(), + /*include_partial=*/true)}); + } + } + + // Walk-through the filters until the solution gets found. + // If no solution is found, return the first detailed error (if any). + // future: add "error level" so the worst one can be picked instead. + std::vector<util::Result<SelectionResult>> res_detailed_errors; + for (const auto& select_filter : ordered_filters) { + if (auto res{AttemptSelection(wallet, value_to_select, select_filter.filter, available_coins, + coin_selection_params, select_filter.allow_mixed_output_types)}) { + return res; // result found + } else { + // If any specific error message appears here, then something particularly wrong might have happened. + // Save the error and continue the selection process. So if no solutions gets found, we can return + // the detailed error to the upper layers. + if (HasErrorMsg(res)) res_detailed_errors.emplace_back(res); } } // Coin Selection failed. - return std::optional<SelectionResult>(); + return res_detailed_errors.empty() ? util::Result<SelectionResult>(util::Error()) : res_detailed_errors.front(); }(); return res; @@ -917,13 +931,16 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( } // Choose coins to use - std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); - if (!result) { - return util::Error{_("Insufficient funds")}; + auto select_coins_res = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); + if (!select_coins_res) { + // 'SelectCoins' either returns a specific error message or, if empty, means a general "Insufficient funds". + const bilingual_str& err = util::ErrorString(select_coins_res); + return util::Error{err.empty() ?_("Insufficient funds") : err}; } - TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->GetAlgo()).c_str(), result->GetTarget(), result->GetWaste(), result->GetSelectedValue()); + const SelectionResult& result = *select_coins_res; + TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result.GetAlgo()).c_str(), result.GetTarget(), result.GetWaste(), result.GetSelectedValue()); - const CAmount change_amount = result->GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee); + const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee); if (change_amount > 0) { CTxOut newTxOut(change_amount, scriptChange); if (nChangePosInOut == -1) { @@ -938,7 +955,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( } // Shuffle selected coins and fill in final vin - std::vector<COutput> selected_coins = result->GetShuffledInputVector(); + std::vector<COutput> selected_coins = result.GetShuffledInputVector(); // The sequence number is set to non-maxint so that DiscourageFeeSniping // works. @@ -963,7 +980,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); const CAmount output_value = CalculateOutputValue(txNew); Assume(recipients_sum + change_amount == output_value); - CAmount current_fee = result->GetSelectedValue() - output_value; + CAmount current_fee = result.GetSelectedValue() - output_value; // Sanity check that the fee cannot be negative as that means we have more output value than input value if (current_fee < 0) { @@ -974,7 +991,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( if (nChangePosInOut != -1 && fee_needed < current_fee) { auto& change = txNew.vout.at(nChangePosInOut); change.nValue += current_fee - fee_needed; - current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew); + current_fee = result.GetSelectedValue() - CalculateOutputValue(txNew); if (fee_needed != current_fee) { return util::Error{Untranslated(STR_INTERNAL_BUG("Change adjustment: Fee needed != fee paid"))}; } @@ -1013,7 +1030,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( } ++i; } - current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew); + current_fee = result.GetSelectedValue() - CalculateOutputValue(txNew); if (fee_needed != current_fee) { return util::Error{Untranslated(STR_INTERNAL_BUG("SFFO: Fee needed != fee paid"))}; } diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 281a6ca9e8..5ffdd11813 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -121,9 +121,11 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C * param@[in] coin_selection_params Parameters for the coin selection * param@[in] allow_mixed_output_types Relax restriction that SelectionResults must be of the same OutputType * returns If successful, a SelectionResult containing the input set - * If failed, a nullopt + * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds") + * or (2) an specific error message if there was something particularly wrong (e.g. a selection + * result that surpassed the tx max weight size). */ -std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins, +util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins, const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types); /** @@ -137,9 +139,11 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm * param@[in] available_coins The struct of coins, organized by OutputType, available for selection prior to filtering * param@[in] coin_selection_params Parameters for the coin selection * returns If successful, a SelectionResult containing the input set - * If failed, a nullopt + * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds") + * or (2) an specific error message if there was something particularly wrong (e.g. a selection + * result that surpassed the tx max weight size). */ -std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, +util::Result<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, const CoinSelectionParams& coin_selection_params); // User manually selected inputs that must be part of the transaction @@ -177,18 +181,20 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const * param@[in] coin_selection_params Parameters for this coin selection such as feerates, whether to avoid partial spends, * and whether to subtract the fee from the outputs. * returns If successful, a SelectionResult containing the selected coins - * If failed, a nullopt. + * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds") + * or (2) an specific error message if there was something particularly wrong (e.g. a selection + * result that surpassed the tx max weight size). */ -std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, +util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); /** * Select all coins from coin_control, and if coin_control 'm_allow_other_inputs=true', call 'AutomaticCoinSelection' to * select a set of coins such that nTargetValue - pre_set_inputs.total_amount is met. */ -std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, - const CAmount& nTargetValue, const CCoinControl& coin_control, - const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, + const CAmount& nTargetValue, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); struct CreatedTransactionResult { diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index c30209002d..ae6d1deb8f 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -931,7 +931,7 @@ BOOST_AUTO_TEST_CASE(effective_value_test) BOOST_CHECK_EQUAL(output5.GetEffectiveValue(), nValue); // The effective value should be equal to the absolute value if input_bytes is -1 } -static std::optional<SelectionResult> select_coins(const CAmount& target, const CoinSelectionParams& cs_params, const CCoinControl& cc, std::function<CoinsResult(CWallet&)> coin_setup, interfaces::Chain* chain, const ArgsManager& args) +static util::Result<SelectionResult> select_coins(const CAmount& target, const CoinSelectionParams& cs_params, const CCoinControl& cc, std::function<CoinsResult(CWallet&)> coin_setup, interfaces::Chain* chain, const ArgsManager& args) { std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(chain, "", args, CreateMockWalletDatabase()); wallet->LoadWallet(); |