mirror/bitcoin
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

wallet: make SelectCoins flow return util::Result

Browse Source
This commit is contained in:
furszy committed 2022-12-21 23:14:50 -03:00
1 parent e5e147fe97
commit 7e8340ab1a
3 files changed
+29 -28
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/wallet/spend.cpp
+22 -21
View File
@@ -510,7 +510,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
return groups_out; return groups_out;
} }
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) const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types)
{ {
// Run coin selection on each OutputType and compute the Waste Metric // Run coin selection on each OutputType and compute the Waste Metric
@@ -534,10 +534,10 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
} }
// 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 // 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 // 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. // Vector of results. We will choose the best one based on waste.
std::vector<SelectionResult> results; std::vector<SelectionResult> results;
@@ -561,7 +561,7 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
if (results.empty()) { if (results.empty()) {
// No solution found // No solution found
return std::nullopt; return util::Error();
} }
std::vector<SelectionResult> eligible_results; std::vector<SelectionResult> eligible_results;
@@ -571,7 +571,7 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
}); });
if (eligible_results.empty()) { if (eligible_results.empty()) {
return std::nullopt; return util::Error();
} }
// Choose the result with the least waste // Choose the result with the least waste
@@ -580,15 +580,15 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
return best_result; return best_result;
} }
std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
const CAmount& nTargetValue, const CCoinControl& coin_control, const CAmount& nTargetValue, const CCoinControl& coin_control,
const CoinSelectionParams& coin_selection_params) const CoinSelectionParams& coin_selection_params)
{ {
// Deduct preset inputs amount from the search target // Deduct preset inputs amount from the search target
CAmount selection_target = nTargetValue - pre_set_inputs.total_amount; CAmount selection_target = nTargetValue - pre_set_inputs.total_amount;
// Return if automatic coin selection is disabled, and we don't cover the selection target // 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();
// Return if we can cover the target only with the preset inputs // Return if we can cover the target only with the preset inputs
if (selection_target <= 0) { if (selection_target <= 0) {
@@ -603,7 +603,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() : CAmount available_coins_total_amount = coin_selection_params.m_subtract_fee_outputs ? available_coins.GetTotalAmount() :
(available_coins.GetEffectiveTotalAmount().has_value() ? *available_coins.GetEffectiveTotalAmount() : 0); (available_coins.GetEffectiveTotalAmount().has_value() ? *available_coins.GetEffectiveTotalAmount() : 0);
if (selection_target > available_coins_total_amount) { if (selection_target > available_coins_total_amount) {
return std::nullopt; // Insufficient funds return util::Error(); // Insufficient funds
} }
// Start wallet Coin Selection procedure // Start wallet Coin Selection procedure
@@ -627,7 +627,7 @@ struct SelectionFilter {
bool allow_mixed_output_types{true}; bool allow_mixed_output_types{true};
}; };
std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) 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_ancestor_count = 0;
unsigned int limit_descendant_count = 0; unsigned int limit_descendant_count = 0;
@@ -648,7 +648,7 @@ std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coi
// Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the // 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 // transaction at a target feerate. If an attempt fails, more attempts may be made using a more
// permissive CoinEligibilityFilter. // permissive CoinEligibilityFilter.
std::optional<SelectionResult> res = [&] { util::Result<SelectionResult> res = [&] {
// Place coins eligibility filters on a scope increasing order. // Place coins eligibility filters on a scope increasing order.
std::vector<SelectionFilter> ordered_filters{ std::vector<SelectionFilter> ordered_filters{
// If possible, fund the transaction with confirmed UTXOs only. Prefer at least six // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six
@@ -687,7 +687,7 @@ std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coi
coin_selection_params, select_filter.allow_mixed_output_types)}) return res; coin_selection_params, select_filter.allow_mixed_output_types)}) return res;
} }
// Coin Selection failed. // Coin Selection failed.
return std::optional<SelectionResult>(); return util::Result<SelectionResult>(util::Error());
}(); }();
return res; return res;
@@ -914,13 +914,14 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
} }
// Choose coins to use // Choose coins to use
std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); auto select_coins_res = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
if (!result) { if (!select_coins_res) {
return util::Error{_("Insufficient funds")}; return util::Error{_("Insufficient funds")};
} }
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) { if (change_amount > 0) {
CTxOut newTxOut(change_amount, scriptChange); CTxOut newTxOut(change_amount, scriptChange);
if (nChangePosInOut == -1) { if (nChangePosInOut == -1) {
@@ -935,7 +936,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
} }
// Shuffle selected coins and fill in final vin // 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 // The sequence number is set to non-maxint so that DiscourageFeeSniping
// works. // works.
@@ -960,7 +961,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes);
const CAmount output_value = CalculateOutputValue(txNew); const CAmount output_value = CalculateOutputValue(txNew);
Assume(recipients_sum + change_amount == output_value); 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 // Sanity check that the fee cannot be negative as that means we have more output value than input value
if (current_fee < 0) { if (current_fee < 0) {
@@ -971,7 +972,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
if (nChangePosInOut != -1 && fee_needed < current_fee) { if (nChangePosInOut != -1 && fee_needed < current_fee) {
auto& change = txNew.vout.at(nChangePosInOut); auto& change = txNew.vout.at(nChangePosInOut);
change.nValue += current_fee - fee_needed; change.nValue += current_fee - fee_needed;
current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew); current_fee = result.GetSelectedValue() - CalculateOutputValue(txNew);
if (fee_needed != current_fee) { if (fee_needed != current_fee) {
return util::Error{Untranslated(STR_INTERNAL_BUG("Change adjustment: Fee needed != fee paid"))}; return util::Error{Untranslated(STR_INTERNAL_BUG("Change adjustment: Fee needed != fee paid"))};
} }
@@ -1010,7 +1011,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
} }
++i; ++i;
} }
current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew); current_fee = result.GetSelectedValue() - CalculateOutputValue(txNew);
if (fee_needed != current_fee) { if (fee_needed != current_fee) {
return util::Error{Untranslated(STR_INTERNAL_BUG("SFFO: Fee needed != fee paid"))}; return util::Error{Untranslated(STR_INTERNAL_BUG("SFFO: Fee needed != fee paid"))};
} }
src/wallet/spend.h
+6 -6
View File
@@ -121,7 +121,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
* returns If successful, a SelectionResult containing the input set * returns If successful, a SelectionResult containing the input set
* If failed, a nullopt * If failed, a nullopt
*/ */
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); const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types);
/** /**
@@ -137,7 +137,7 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
* returns If successful, a SelectionResult containing the input set * returns If successful, a SelectionResult containing the input set
* If failed, a nullopt * If failed, a nullopt
*/ */
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); const CoinSelectionParams& coin_selection_params);
// User manually selected inputs that must be part of the transaction // User manually selected inputs that must be part of the transaction
@@ -177,16 +177,16 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
* returns If successful, a SelectionResult containing the selected coins * returns If successful, a SelectionResult containing the selected coins
* If failed, a nullopt. * If failed, a nullopt.
*/ */
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); 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 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. * 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, util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
const CAmount& nTargetValue, const CCoinControl& coin_control, const CAmount& nTargetValue, const CCoinControl& coin_control,
const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
struct CreatedTransactionResult struct CreatedTransactionResult
{ {
src/wallet/test/coinselector_tests.cpp
+1 -1
View File
@@ -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 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()); std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(chain, "", args, CreateMockWalletDatabase());
wallet->LoadWallet(); wallet->LoadWallet();