aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/init.cpp6
-rw-r--r--src/qt/addresstablemodel.cpp11
-rw-r--r--src/test/mempool_tests.cpp16
-rw-r--r--src/util/result.h12
-rw-r--r--src/validation.cpp13
-rw-r--r--src/wallet/spend.cpp30
-rw-r--r--src/wallet/test/fuzz/coinselection.cpp5
-rw-r--r--src/wallet/test/fuzz/notifications.cpp6
-rw-r--r--src/wallet/test/spend_tests.cpp6
9 files changed, 60 insertions, 45 deletions
diff --git a/src/init.cpp b/src/init.cpp
index c19d596c7f..47d649be3d 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -992,10 +992,8 @@ bool AppInitParameterInteraction(const ArgsManager& args)
InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections));
// ********************************************************* Step 3: parameter-to-internal-flags
- auto result = init::SetLoggingCategories(args);
- if (!result) return InitError(util::ErrorString(result));
- result = init::SetLoggingLevel(args);
- if (!result) return InitError(util::ErrorString(result));
+ if (auto result{init::SetLoggingCategories(args)}; !result) return InitError(util::ErrorString(result));
+ if (auto result{init::SetLoggingLevel(args)}; !result) return InitError(util::ErrorString(result));
nConnectTimeout = args.GetIntArg("-timeout", DEFAULT_CONNECT_TIMEOUT);
if (nConnectTimeout <= 0) {
diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index c52ef7cd67..efdc3966d1 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -369,21 +369,22 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
else if(type == Receive)
{
// Generate a new address to associate with given label
- auto op_dest = walletModel->wallet().getNewDestination(address_type, strLabel);
- if (!op_dest) {
+ if (auto dest{walletModel->wallet().getNewDestination(address_type, strLabel)}) {
+ strAddress = EncodeDestination(*dest);
+ } else {
WalletModel::UnlockContext ctx(walletModel->requestUnlock());
if (!ctx.isValid()) {
// Unlock wallet failed or was cancelled
editStatus = WALLET_UNLOCK_FAILURE;
return QString();
}
- op_dest = walletModel->wallet().getNewDestination(address_type, strLabel);
- if (!op_dest) {
+ if (auto dest_retry{walletModel->wallet().getNewDestination(address_type, strLabel)}) {
+ strAddress = EncodeDestination(*dest_retry);
+ } else {
editStatus = KEY_GENERATION_FAILURE;
return QString();
}
}
- strAddress = EncodeDestination(*op_dest);
}
else
{
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index 217e4a6d22..9f8d434213 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -202,9 +202,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
tx7.vout[1].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
tx7.vout[1].nValue = 1 * COIN;
- auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), CTxMemPool::Limits::NoLimits())};
- BOOST_REQUIRE(ancestors_calculated.has_value());
- BOOST_CHECK(*ancestors_calculated == setAncestors);
+ {
+ auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), CTxMemPool::Limits::NoLimits())};
+ BOOST_REQUIRE(ancestors_calculated.has_value());
+ BOOST_CHECK(*ancestors_calculated == setAncestors);
+ }
pool.addUnchecked(entry.FromTx(tx7), setAncestors);
BOOST_CHECK_EQUAL(pool.size(), 7U);
@@ -260,9 +262,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
tx10.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
tx10.vout[0].nValue = 10 * COIN;
- ancestors_calculated = pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(NodeSeconds{4s}).FromTx(tx10), CTxMemPool::Limits::NoLimits());
- BOOST_REQUIRE(ancestors_calculated);
- BOOST_CHECK(*ancestors_calculated == setAncestors);
+ {
+ auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(NodeSeconds{4s}).FromTx(tx10), CTxMemPool::Limits::NoLimits())};
+ BOOST_REQUIRE(ancestors_calculated);
+ BOOST_CHECK(*ancestors_calculated == setAncestors);
+ }
pool.addUnchecked(entry.FromTx(tx10), setAncestors);
diff --git a/src/util/result.h b/src/util/result.h
index b99995c7e5..122a7638fa 100644
--- a/src/util/result.h
+++ b/src/util/result.h
@@ -39,6 +39,16 @@ private:
std::variant<bilingual_str, T> m_variant;
+ //! Disallow copy constructor, require Result to be moved for efficiency.
+ Result(const Result&) = delete;
+
+ //! Disallow operator= to avoid confusion in the future when the Result
+ //! class gains support for richer error reporting, and callers should have
+ //! ability to set a new result value without clearing existing error
+ //! messages.
+ Result& operator=(const Result&) = delete;
+ Result& operator=(Result&&) = delete;
+
template <typename FT>
friend bilingual_str ErrorString(const Result<FT>& result);
@@ -46,6 +56,8 @@ public:
Result() : m_variant{std::in_place_index_t<1>{}, std::monostate{}} {} // constructor for void
Result(T obj) : m_variant{std::in_place_index_t<1>{}, std::move(obj)} {}
Result(Error error) : m_variant{std::in_place_index_t<0>{}, std::move(error.message)} {}
+ Result(Result&&) = default;
+ ~Result() = default;
//! std::optional methods, so functions returning optional<T> can change to
//! return Result<T> with minimal changes to existing code, and vice versa.
diff --git a/src/validation.cpp b/src/validation.cpp
index 903f9caf13..f57851b4f7 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -946,8 +946,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
maybe_rbf_limits.descendant_size_vbytes += conflict->GetSizeWithDescendants();
}
- auto ancestors{m_pool.CalculateMemPoolAncestors(*entry, maybe_rbf_limits)};
- if (!ancestors) {
+ if (auto ancestors{m_pool.CalculateMemPoolAncestors(*entry, maybe_rbf_limits)}) {
+ ws.m_ancestors = std::move(*ancestors);
+ } else {
// If CalculateMemPoolAncestors fails second time, we want the original error string.
// Contracting/payment channels CPFP carve-out:
// If the new transaction is relatively small (up to 40k weight)
@@ -970,11 +971,13 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message);
}
- ancestors = m_pool.CalculateMemPoolAncestors(*entry, cpfp_carve_out_limits);
- if (!ancestors) return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message);
+ if (auto ancestors_retry{m_pool.CalculateMemPoolAncestors(*entry, cpfp_carve_out_limits)}) {
+ ws.m_ancestors = std::move(*ancestors_retry);
+ } else {
+ return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message);
+ }
}
- ws.m_ancestors = *ancestors;
// Even though just checking direct mempool parents for inheritance would be sufficient, we
// check using the full ancestor set here because it's more convenient to use what we have
// already calculated.
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 5d23ebd35a..9a7e166e68 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -683,11 +683,11 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
// Vector of results. We will choose the best one based on waste.
std::vector<SelectionResult> results;
std::vector<util::Result<SelectionResult>> errors;
- auto append_error = [&] (const util::Result<SelectionResult>& result) {
+ auto append_error = [&] (util::Result<SelectionResult>&& result) {
// If any specific error message appears here, then something different from a simple "no selection found" happened.
// Let's save it, so it can be retrieved to the user if no other selection algorithm succeeded.
if (HasErrorMsg(result)) {
- errors.emplace_back(result);
+ errors.emplace_back(std::move(result));
}
};
@@ -698,7 +698,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
if (!coin_selection_params.m_subtract_fee_outputs) {
if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_inputs_weight)}) {
results.push_back(*bnb_result);
- } else append_error(bnb_result);
+ } else append_error(std::move(bnb_result));
}
// As Knapsack and SRD can create change, also deduce change weight.
@@ -707,25 +707,25 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
// The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here.
if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_inputs_weight)}) {
results.push_back(*knapsack_result);
- } else append_error(knapsack_result);
+ } else append_error(std::move(knapsack_result));
if (coin_selection_params.m_effective_feerate > CFeeRate{3 * coin_selection_params.m_long_term_feerate}) { // Minimize input set for feerates of at least 3×LTFRE (default: 30 ṩ/vB+)
if (auto cg_result{CoinGrinder(groups.positive_group, nTargetValue, coin_selection_params.m_min_change_target, max_inputs_weight)}) {
cg_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
results.push_back(*cg_result);
} else {
- append_error(cg_result);
+ append_error(std::move(cg_result));
}
}
if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight)}) {
results.push_back(*srd_result);
- } else append_error(srd_result);
+ } else append_error(std::move(srd_result));
if (results.empty()) {
// No solution found, retrieve the first explicit error (if any).
// future: add 'severity level' to errors so the worst one can be retrieved instead of the first one.
- return errors.empty() ? util::Error() : errors.front();
+ return errors.empty() ? util::Error() : std::move(errors.front());
}
// If the chosen input set has unconfirmed inputs, check for synergies from overlapping ancestry
@@ -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
@@ -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.
@@ -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)
diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp
index 297432de9e..331590df7f 100644
--- a/src/wallet/test/fuzz/coinselection.cpp
+++ b/src/wallet/test/fuzz/coinselection.cpp
@@ -291,7 +291,10 @@ 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;
+ results.emplace_back(std::move(result_srd));
+ results.emplace_back(std::move(result_knapsack));
+ results.emplace_back(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;
diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp
index 9a515828fe..792079e6c6 100644
--- a/src/wallet/test/fuzz/notifications.cpp
+++ b/src/wallet/test/fuzz/notifications.cpp
@@ -106,13 +106,11 @@ struct FuzzedWallet {
CTxDestination GetDestination(FuzzedDataProvider& fuzzed_data_provider)
{
auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)};
- util::Result<CTxDestination> op_dest{util::Error{}};
if (fuzzed_data_provider.ConsumeBool()) {
- op_dest = wallet->GetNewDestination(type, "");
+ return *Assert(wallet->GetNewDestination(type, ""));
} else {
- op_dest = wallet->GetNewChangeDestination(type);
+ return *Assert(wallet->GetNewChangeDestination(type));
}
- return *Assert(op_dest);
}
CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { return GetScriptForDestination(GetDestination(fuzzed_data_provider)); }
void FundTx(FuzzedDataProvider& fuzzed_data_provider, CMutableTransaction tx)
diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index 3509bc116f..963c0f838b 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -97,13 +97,11 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup)
// so that the recipient's amount is no longer equal to the user's selected target of 299 BTC.
// First case, use 'subtract_fee_from_outputs=true'
- util::Result<CreatedTransactionResult> res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control);
- BOOST_CHECK(!res_tx.has_value());
+ BOOST_CHECK(!CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control));
// Second case, don't use 'subtract_fee_from_outputs'.
recipients[0].fSubtractFeeFromAmount = false;
- res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control);
- BOOST_CHECK(!res_tx.has_value());
+ BOOST_CHECK(!CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control));
}
BOOST_AUTO_TEST_SUITE_END()
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-27 14:15:14 +0000