Skip to content

Commit

Permalink
refactor: Avoid copying util::Result values
Browse files Browse the repository at this point in the history
Copying util::Result values is less efficient than moving them because they
allocate memory and contain strings. Also this is needed to avoid compile
errors in #25722 which adds a
std::unique_ptr member to util::Result which implicity disables copying.
  • Loading branch information
ryanofsky committed Mar 25, 2024
1 parent d293d0b commit c45cb13
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
16 changes: 7 additions & 9 deletions src/wallet/spend.cpp
Expand Up @@ -818,7 +818,7 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
// 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.
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
Expand Down Expand Up @@ -866,9 +866,9 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
if (CAmount total_amount = available_coins.GetTotalAmount() - total_discarded < value_to_select) {
// Special case, too-long-mempool cluster.
if (total_amount + total_unconf_long_chain > value_to_select) {
return util::Result<SelectionResult>({_("Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool")});
return util::Error{_("Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool")};
}
return util::Result<SelectionResult>(util::Error()); // General "Insufficient Funds"
return util::Error{}; // General "Insufficient Funds"
}

// Walk-through the filters until the solution gets found.
Expand All @@ -885,19 +885,17 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
// 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);
if (HasErrorMsg(res)) res_detailed_errors.emplace_back(std::move(res));
}
}

// Return right away if we have a detailed error
if (!res_detailed_errors.empty()) return res_detailed_errors.front();
if (!res_detailed_errors.empty()) return std::move(res_detailed_errors.front());


// General "Insufficient Funds"
return util::Result<SelectionResult>(util::Error());
}();

return res;
return util::Error{};
}
}

static bool IsCurrentForAntiFeeSniping(interfaces::Chain& chain, const uint256& block_hash)
Expand Down
4 changes: 3 additions & 1 deletion src/wallet/test/fuzz/coinselection.cpp
Expand Up @@ -291,7 +291,9 @@ FUZZ_TARGET(coinselection)
}

std::vector<COutput> utxos;
std::vector<util::Result<SelectionResult>> results{result_srd, result_knapsack, result_bnb};
std::vector<util::Result<SelectionResult>> results{
std::move(result_srd), std::move(result_knapsack), std::move(result_bnb)
};
CAmount new_total_balance{CreateCoins(fuzzed_data_provider, utxos, coin_params, next_locktime)};
if (new_total_balance > 0) {
std::set<std::shared_ptr<COutput>> new_utxo_pool;
Expand Down

0 comments on commit c45cb13

Please sign in to comment.