aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/Makefile.test.include1
-rw-r--r--src/bench/wallet_loading.cpp5
-rw-r--r--src/interfaces/wallet.h6
-rw-r--r--src/qt/addresstablemodel.cpp2
-rw-r--r--src/qt/walletcontroller.cpp4
-rw-r--r--src/qt/walletmodel.cpp4
-rw-r--r--src/test/result_tests.cpp96
-rw-r--r--src/test/util/wallet.cpp6
-rw-r--r--src/util/result.h87
-rw-r--r--src/wallet/feebumper.cpp4
-rw-r--r--src/wallet/interfaces.cpp16
-rw-r--r--src/wallet/rpc/addresses.cpp8
-rw-r--r--src/wallet/rpc/spend.cpp6
-rw-r--r--src/wallet/scriptpubkeyman.cpp20
-rw-r--r--src/wallet/scriptpubkeyman.h6
-rw-r--r--src/wallet/spend.cpp50
-rw-r--r--src/wallet/spend.h2
-rw-r--r--src/wallet/test/availablecoins_tests.cpp18
-rw-r--r--src/wallet/test/coinselector_tests.cpp4
-rw-r--r--src/wallet/test/fuzz/notifications.cpp5
-rw-r--r--src/wallet/test/spend_tests.cpp2
-rw-r--r--src/wallet/test/wallet_tests.cpp6
-rw-r--r--src/wallet/wallet.cpp10
-rw-r--r--src/wallet/wallet.h4
24 files changed, 247 insertions, 125 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index f52964b033..9ed5731af1 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -119,6 +119,7 @@ BITCOIN_TESTS =\
test/random_tests.cpp \
test/rbf_tests.cpp \
test/rest_tests.cpp \
+ test/result_tests.cpp \
test/reverselock_tests.cpp \
test/rpc_tests.cpp \
test/sanity_tests.cpp \
diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index a10f7ff7d1..27e4dd015d 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -45,11 +45,8 @@ static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
static void AddTx(CWallet& wallet)
{
- const auto& dest = wallet.GetNewDestination(OutputType::BECH32, "");
- assert(dest.HasRes());
-
CMutableTransaction mtx;
- mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())});
+ mtx.vout.push_back({COIN, GetScriptForDestination(*Assert(wallet.GetNewDestination(OutputType::BECH32, "")))});
mtx.vin.push_back(CTxIn());
wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{});
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index e29fefae56..9bd033d991 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -88,7 +88,7 @@ public:
virtual std::string getWalletName() = 0;
// Get a new address.
- virtual BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) = 0;
+ virtual util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string label) = 0;
//! Get public key.
virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;
@@ -139,7 +139,7 @@ public:
virtual void listLockedCoins(std::vector<COutPoint>& outputs) = 0;
//! Create transaction.
- virtual BResult<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients,
+ virtual util::Result<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients,
const wallet::CCoinControl& coin_control,
bool sign,
int& change_pos,
@@ -329,7 +329,7 @@ public:
virtual std::string getWalletDir() = 0;
//! Restore backup wallet
- virtual BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;
+ virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;
//! Return available wallets in wallet directory.
virtual std::vector<std::string> listWalletDir() = 0;
diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index bb50b5384a..8b5da7f9f0 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -384,7 +384,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
return QString();
}
}
- strAddress = EncodeDestination(op_dest.GetObj());
+ strAddress = EncodeDestination(*op_dest);
}
else
{
diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
index 01d84624e8..6e07dc09a0 100644
--- a/src/qt/walletcontroller.cpp
+++ b/src/qt/walletcontroller.cpp
@@ -393,8 +393,8 @@ void RestoreWalletActivity::restore(const fs::path& backup_file, const std::stri
QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] {
auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name, m_warning_message)};
- m_error_message = wallet ? bilingual_str{} : wallet.GetError();
- if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());
+ m_error_message = util::ErrorString(wallet);
+ if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
QTimer::singleShot(0, this, &RestoreWalletActivity::finish);
});
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index fde136b727..ac6c8bea46 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -207,7 +207,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
auto& newTx = transaction.getWtx();
const auto& res = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired);
- newTx = res ? res.GetObj() : nullptr;
+ newTx = res ? *res : nullptr;
transaction.setTransactionFee(nFeeRequired);
if (fSubtractFeeFromAmount && newTx)
transaction.reassignAmounts(nChangePosRet);
@@ -218,7 +218,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
{
return SendCoinsReturn(AmountWithFeeExceedsBalance);
}
- Q_EMIT message(tr("Send Coins"), QString::fromStdString(res.GetError().translated),
+ Q_EMIT message(tr("Send Coins"), QString::fromStdString(util::ErrorString(res).translated),
CClientUIInterface::MSG_ERROR);
return TransactionCreationFailed;
}
diff --git a/src/test/result_tests.cpp b/src/test/result_tests.cpp
new file mode 100644
index 0000000000..6a23a7b895
--- /dev/null
+++ b/src/test/result_tests.cpp
@@ -0,0 +1,96 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <util/result.h>
+
+#include <boost/test/unit_test.hpp>
+
+inline bool operator==(const bilingual_str& a, const bilingual_str& b)
+{
+ return a.original == b.original && a.translated == b.translated;
+}
+
+inline std::ostream& operator<<(std::ostream& os, const bilingual_str& s)
+{
+ return os << "bilingual_str('" << s.original << "' , '" << s.translated << "')";
+}
+
+BOOST_AUTO_TEST_SUITE(result_tests)
+
+struct NoCopy {
+ NoCopy(int n) : m_n{std::make_unique<int>(n)} {}
+ std::unique_ptr<int> m_n;
+};
+
+bool operator==(const NoCopy& a, const NoCopy& b)
+{
+ return *a.m_n == *b.m_n;
+}
+
+std::ostream& operator<<(std::ostream& os, const NoCopy& o)
+{
+ return os << "NoCopy(" << *o.m_n << ")";
+}
+
+util::Result<int> IntFn(int i, bool success)
+{
+ if (success) return i;
+ return util::Error{Untranslated(strprintf("int %i error.", i))};
+}
+
+util::Result<bilingual_str> StrFn(bilingual_str s, bool success)
+{
+ if (success) return s;
+ return util::Error{strprintf(Untranslated("str %s error."), s.original)};
+}
+
+util::Result<NoCopy> NoCopyFn(int i, bool success)
+{
+ if (success) return {i};
+ return util::Error{Untranslated(strprintf("nocopy %i error.", i))};
+}
+
+template <typename T>
+void ExpectResult(const util::Result<T>& result, bool success, const bilingual_str& str)
+{
+ BOOST_CHECK_EQUAL(bool(result), success);
+ BOOST_CHECK_EQUAL(util::ErrorString(result), str);
+}
+
+template <typename T, typename... Args>
+void ExpectSuccess(const util::Result<T>& result, const bilingual_str& str, Args&&... args)
+{
+ ExpectResult(result, true, str);
+ BOOST_CHECK_EQUAL(result.has_value(), true);
+ BOOST_CHECK_EQUAL(result.value(), T{std::forward<Args>(args)...});
+ BOOST_CHECK_EQUAL(&result.value(), &*result);
+}
+
+template <typename T, typename... Args>
+void ExpectFail(const util::Result<T>& result, const bilingual_str& str)
+{
+ ExpectResult(result, false, str);
+}
+
+BOOST_AUTO_TEST_CASE(check_returned)
+{
+ ExpectSuccess(IntFn(5, true), {}, 5);
+ ExpectFail(IntFn(5, false), Untranslated("int 5 error."));
+ ExpectSuccess(NoCopyFn(5, true), {}, 5);
+ ExpectFail(NoCopyFn(5, false), Untranslated("nocopy 5 error."));
+ ExpectSuccess(StrFn(Untranslated("S"), true), {}, Untranslated("S"));
+ ExpectFail(StrFn(Untranslated("S"), false), Untranslated("str S error."));
+}
+
+BOOST_AUTO_TEST_CASE(check_value_or)
+{
+ BOOST_CHECK_EQUAL(IntFn(10, true).value_or(20), 10);
+ BOOST_CHECK_EQUAL(IntFn(10, false).value_or(20), 20);
+ BOOST_CHECK_EQUAL(NoCopyFn(10, true).value_or(20), 10);
+ BOOST_CHECK_EQUAL(NoCopyFn(10, false).value_or(20), 20);
+ BOOST_CHECK_EQUAL(StrFn(Untranslated("A"), true).value_or(Untranslated("B")), Untranslated("A"));
+ BOOST_CHECK_EQUAL(StrFn(Untranslated("A"), false).value_or(Untranslated("B")), Untranslated("B"));
+}
+
+BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp
index 7a00ac9e1f..b54774cbb9 100644
--- a/src/test/util/wallet.cpp
+++ b/src/test/util/wallet.cpp
@@ -8,6 +8,7 @@
#include <outputtype.h>
#include <script/standard.h>
#ifdef ENABLE_WALLET
+#include <util/check.h>
#include <util/translation.h>
#include <wallet/wallet.h>
#endif
@@ -20,10 +21,7 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq
std::string getnewaddress(CWallet& w)
{
constexpr auto output_type = OutputType::BECH32;
- auto op_dest = w.GetNewDestination(output_type, "");
- assert(op_dest.HasRes());
-
- return EncodeDestination(op_dest.GetObj());
+ return EncodeDestination(*Assert(w.GetNewDestination(output_type, "")));
}
#endif // ENABLE_WALLET
diff --git a/src/util/result.h b/src/util/result.h
index 2f586a4c9b..972b1aada0 100644
--- a/src/util/result.h
+++ b/src/util/result.h
@@ -5,45 +5,80 @@
#ifndef BITCOIN_UTIL_RESULT_H
#define BITCOIN_UTIL_RESULT_H
+#include <attributes.h>
#include <util/translation.h>
#include <variant>
-/*
- * 'BResult' is a generic class useful for wrapping a return object
- * (in case of success) or propagating the error cause.
-*/
-template<class T>
-class BResult {
+namespace util {
+
+struct Error {
+ bilingual_str message;
+};
+
+//! The util::Result class provides a standard way for functions to return
+//! either error messages or result values.
+//!
+//! It is intended for high-level functions that need to report error strings to
+//! end users. Lower-level functions that don't need this error-reporting and
+//! only need error-handling should avoid util::Result and instead use standard
+//! classes like std::optional, std::variant, and std::tuple, or custom structs
+//! and enum types to return function results.
+//!
+//! Usage examples can be found in \example ../test/result_tests.cpp, but in
+//! general code returning `util::Result<T>` values is very similar to code
+//! returning `std::optional<T>` values. Existing functions returning
+//! `std::optional<T>` can be updated to return `util::Result<T>` and return
+//! error strings usually just replacing `return std::nullopt;` with `return
+//! util::Error{error_string};`.
+template <class T>
+class Result
+{
private:
std::variant<bilingual_str, T> m_variant;
-public:
- BResult() : m_variant{Untranslated("")} {}
- BResult(T obj) : m_variant{std::move(obj)} {}
- BResult(bilingual_str error) : m_variant{std::move(error)} {}
+ template <typename FT>
+ friend bilingual_str ErrorString(const Result<FT>& result);
- /* Whether the function succeeded or not */
- bool HasRes() const { return std::holds_alternative<T>(m_variant); }
+public:
+ 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)} {}
- /* In case of success, the result object */
- const T& GetObj() const {
- assert(HasRes());
- return std::get<T>(m_variant);
+ //! std::optional methods, so functions returning optional<T> can change to
+ //! return Result<T> with minimal changes to existing code, and vice versa.
+ bool has_value() const noexcept { return m_variant.index() == 1; }
+ const T& value() const LIFETIMEBOUND
+ {
+ assert(has_value());
+ return std::get<1>(m_variant);
}
- T ReleaseObj()
+ T& value() LIFETIMEBOUND
{
- assert(HasRes());
- return std::move(std::get<T>(m_variant));
+ assert(has_value());
+ return std::get<1>(m_variant);
}
-
- /* In case of failure, the error cause */
- const bilingual_str& GetError() const {
- assert(!HasRes());
- return std::get<bilingual_str>(m_variant);
+ template <class U>
+ T value_or(U&& default_value) const&
+ {
+ return has_value() ? value() : std::forward<U>(default_value);
}
-
- explicit operator bool() const { return HasRes(); }
+ template <class U>
+ T value_or(U&& default_value) &&
+ {
+ return has_value() ? std::move(value()) : std::forward<U>(default_value);
+ }
+ explicit operator bool() const noexcept { return has_value(); }
+ const T* operator->() const LIFETIMEBOUND { return &value(); }
+ const T& operator*() const LIFETIMEBOUND { return value(); }
+ T* operator->() LIFETIMEBOUND { return &value(); }
+ T& operator*() LIFETIMEBOUND { return value(); }
};
+template <typename T>
+bilingual_str ErrorString(const Result<T>& result)
+{
+ return result ? bilingual_str{} : std::get<0>(result.m_variant);
+}
+} // namespace util
+
#endif // BITCOIN_UTIL_RESULT_H
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index 43fdcee48d..bda4c76bea 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -221,11 +221,11 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
constexpr int RANDOM_CHANGE_POSITION = -1;
auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false);
if (!res) {
- errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + res.GetError());
+ errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res));
return Result::WALLET_ERROR;
}
- const auto& txr = res.GetObj();
+ const auto& txr = *res;
// Write back new fee if successful
new_fee = txr.fee;
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 4f30060e29..91b188a59c 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -148,7 +148,7 @@ public:
void abortRescan() override { m_wallet->AbortRescan(); }
bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); }
std::string getWalletName() override { return m_wallet->GetName(); }
- BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) override
+ util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string label) override
{
LOCK(m_wallet->cs_wallet);
return m_wallet->GetNewDestination(type, label);
@@ -251,17 +251,17 @@ public:
LOCK(m_wallet->cs_wallet);
return m_wallet->ListLockedCoins(outputs);
}
- BResult<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients,
+ util::Result<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients,
const CCoinControl& coin_control,
bool sign,
int& change_pos,
CAmount& fee) override
{
LOCK(m_wallet->cs_wallet);
- const auto& res = CreateTransaction(*m_wallet, recipients, change_pos,
+ auto res = CreateTransaction(*m_wallet, recipients, change_pos,
coin_control, sign);
- if (!res) return res.GetError();
- const auto& txr = res.GetObj();
+ if (!res) return util::Error{util::ErrorString(res)};
+ const auto& txr = *res;
fee = txr.fee;
change_pos = txr.change_pos;
@@ -571,12 +571,12 @@ public:
options.require_existing = true;
return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings));
}
- BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override
+ util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override
{
DatabaseStatus status;
bilingual_str error;
- BResult<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))};
- if (!wallet) return error;
+ util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))};
+ if (!wallet) return util::Error{error};
return wallet;
}
std::string getWalletDir() override
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index 2a6eb96871..148343a8b0 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -60,10 +60,10 @@ RPCHelpMan getnewaddress()
auto op_dest = pwallet->GetNewDestination(output_type, label);
if (!op_dest) {
- throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
+ throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original);
}
- return EncodeDestination(op_dest.GetObj());
+ return EncodeDestination(*op_dest);
},
};
}
@@ -107,9 +107,9 @@ RPCHelpMan getrawchangeaddress()
auto op_dest = pwallet->GetNewChangeDestination(output_type);
if (!op_dest) {
- throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
+ throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original);
}
- return EncodeDestination(op_dest.GetObj());
+ return EncodeDestination(*op_dest);
},
};
}
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index e27cb1139e..628bf3cc99 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -158,14 +158,14 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
constexpr int RANDOM_CHANGE_POSITION = -1;
auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, true);
if (!res) {
- throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original);
+ throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, util::ErrorString(res).original);
}
- const CTransactionRef& tx = res.GetObj().tx;
+ const CTransactionRef& tx = res->tx;
wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */);
if (verbose) {
UniValue entry(UniValue::VOBJ);
entry.pushKV("txid", tx->GetHash().GetHex());
- entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason));
+ entry.pushKV("fee_reason", StringForFeeReason(res->fee_calc.reason));
return entry;
}
return tx->GetHash().GetHex();
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index bba31dfe0b..2f242901ab 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -21,10 +21,10 @@ namespace wallet {
//! Value for the first BIP 32 hardened derivation. Can be used as a bit mask and as a value. See BIP 32 for more details.
const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000;
-BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type)
+util::Result<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type)
{
if (LEGACY_OUTPUT_TYPES.count(type) == 0) {
- return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");;
+ return util::Error{_("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types")};
}
assert(type != OutputType::BECH32M);
@@ -33,7 +33,7 @@ BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputTyp
// Generate a new key that is added to wallet
CPubKey new_key;
if (!GetKeyFromPool(new_key, type)) {
- return _("Error: Keypool ran out, please call keypoolrefill first");
+ return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")};
}
LearnRelatedScripts(new_key, type);
return GetDestinationForKey(new_key, type);
@@ -1654,11 +1654,11 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const
return set_address;
}
-BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type)
+util::Result<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type)
{
// Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later
if (!CanGetAddresses()) {
- return _("No addresses available");
+ return util::Error{_("No addresses available")};
}
{
LOCK(cs_desc_man);
@@ -1676,11 +1676,11 @@ BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const Outpu
std::vector<CScript> scripts_temp;
if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) {
// We can't generate anymore keys
- return _("Error: Keypool ran out, please call keypoolrefill first");
+ return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")};
}
if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) {
// We can't generate anymore keys
- return _("Error: Keypool ran out, please call keypoolrefill first");
+ return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")};
}
CTxDestination dest;
@@ -1766,11 +1766,11 @@ bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bo
auto op_dest = GetNewDestination(type);
index = m_wallet_descriptor.next_index - 1;
if (op_dest) {
- address = op_dest.GetObj();
+ address = *op_dest;
} else {
- error = op_dest.GetError();
+ error = util::ErrorString(op_dest);
}
- return op_dest.HasRes();
+ return bool(op_dest);
}
void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr)
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index eebc05330f..b772dde533 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -172,7 +172,7 @@ protected:
public:
explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {}
virtual ~ScriptPubKeyMan() {};
- virtual BResult<CTxDestination> GetNewDestination(const OutputType type) { return Untranslated("Not supported"); }
+ virtual util::Result<CTxDestination> GetNewDestination(const OutputType type) { return util::Error{Untranslated("Not supported")}; }
virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; }
//! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it.
@@ -360,7 +360,7 @@ private:
public:
using ScriptPubKeyMan::ScriptPubKeyMan;
- BResult<CTxDestination> GetNewDestination(const OutputType type) override;
+ util::Result<CTxDestination> GetNewDestination(const OutputType type) override;
isminetype IsMine(const CScript& script) const override;
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
@@ -568,7 +568,7 @@ public:
mutable RecursiveMutex cs_desc_man;
- BResult<CTxDestination> GetNewDestination(const OutputType type) override;
+ util::Result<CTxDestination> GetNewDestination(const OutputType type) override;
isminetype IsMine(const CScript& script) const override;
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index c00a2cd023..5d320feff0 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -758,7 +758,7 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng
}
}
-static BResult<CreatedTransactionResult> CreateTransactionInternal(
+static util::Result<CreatedTransactionResult> CreateTransactionInternal(
CWallet& wallet,
const std::vector<CRecipient>& vecSend,
int change_pos,
@@ -844,11 +844,11 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
// Do not, ever, assume that it's fine to change the fee rate if the user has explicitly
// provided one
if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) {
- return strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB));
+ return util::Error{strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB))};
}
if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) {
// eventually allow a fallback fee
- return _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.");
+ return util::Error{_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.")};
}
// Calculate the cost of change
@@ -874,7 +874,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
}
if (IsDust(txout, wallet.chain().relayDustFee())) {
- return _("Transaction amount too small");
+ return util::Error{_("Transaction amount too small")};
}
txNew.vout.push_back(txout);
}
@@ -895,7 +895,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
// Choose coins to use
std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
if (!result) {
- return _("Insufficient funds");
+ return util::Error{_("Insufficient funds")};
}
TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue());
@@ -910,7 +910,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
nChangePosInOut = rng_fast.randrange(txNew.vout.size() + 1);
}
else if ((unsigned int)nChangePosInOut > txNew.vout.size()) {
- return _("Transaction change output index out of range");
+ return util::Error{_("Transaction change output index out of range")};
}
assert(nChangePosInOut != -1);
@@ -937,7 +937,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control);
int nBytes = tx_sizes.vsize;
if (nBytes == -1) {
- return _("Missing solving data for estimating transaction size");
+ return util::Error{_("Missing solving data for estimating transaction size")};
}
nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes);
@@ -997,9 +997,9 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
// Error if this output is reduced to be below dust
if (IsDust(txout, wallet.chain().relayDustFee())) {
if (txout.nValue < 0) {
- return _("The transaction amount is too small to pay the fee");
+ return util::Error{_("The transaction amount is too small to pay the fee")};
} else {
- return _("The transaction amount is too small to send after the fee has been deducted");
+ return util::Error{_("The transaction amount is too small to send after the fee has been deducted")};
}
}
}
@@ -1010,11 +1010,11 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
// Give up if change keypool ran out and change is required
if (scriptChange.empty() && nChangePosInOut != -1) {
- return error;
+ return util::Error{error};
}
if (sign && !wallet.SignTransaction(txNew)) {
- return _("Signing transaction failed");
+ return util::Error{_("Signing transaction failed")};
}
// Return the constructed transaction data.
@@ -1024,17 +1024,17 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
if ((sign && GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT) ||
(!sign && tx_sizes.weight > MAX_STANDARD_TX_WEIGHT))
{
- return _("Transaction too large");
+ return util::Error{_("Transaction too large")};
}
if (nFeeRet > wallet.m_default_max_tx_fee) {
- return TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);
+ return util::Error{TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED)};
}
if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) {
// Lastly, ensure this tx will pass the mempool's chain limits
if (!wallet.chain().checkChainLimits(tx)) {
- return _("Transaction has too long of a mempool chain");
+ return util::Error{_("Transaction has too long of a mempool chain")};
}
}
@@ -1053,7 +1053,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(
return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut, feeCalc);
}
-BResult<CreatedTransactionResult> CreateTransaction(
+util::Result<CreatedTransactionResult> CreateTransaction(
CWallet& wallet,
const std::vector<CRecipient>& vecSend,
int change_pos,
@@ -1061,28 +1061,26 @@ BResult<CreatedTransactionResult> CreateTransaction(
bool sign)
{
if (vecSend.empty()) {
- return _("Transaction must have at least one recipient");
+ return util::Error{_("Transaction must have at least one recipient")};
}
if (std::any_of(vecSend.cbegin(), vecSend.cend(), [](const auto& recipient){ return recipient.nAmount < 0; })) {
- return _("Transaction amounts must not be negative");
+ return util::Error{_("Transaction amounts must not be negative")};
}
LOCK(wallet.cs_wallet);
auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign);
- TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res.HasRes(),
- res ? res.GetObj().fee : 0, res ? res.GetObj().change_pos : 0);
+ TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), bool(res),
+ res ? res->fee : 0, res ? res->change_pos : 0);
if (!res) return res;
- const auto& txr_ungrouped = res.GetObj();
+ const auto& txr_ungrouped = *res;
// try with avoidpartialspends unless it's enabled already
if (txr_ungrouped.fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) {
TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str());
CCoinControl tmp_cc = coin_control;
tmp_cc.m_avoid_partial_spends = true;
- auto res_tx_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign);
- // Helper optional class for now
- std::optional<CreatedTransactionResult> txr_grouped{res_tx_grouped.HasRes() ? std::make_optional(res_tx_grouped.GetObj()) : std::nullopt};
+ auto txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign);
// if fee of this alternative one is within the range of the max fee, we use this one
const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false};
TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(),
@@ -1090,7 +1088,7 @@ BResult<CreatedTransactionResult> CreateTransaction(
if (txr_grouped) {
wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n",
txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped");
- if (use_aps) return res_tx_grouped;
+ if (use_aps) return txr_grouped;
}
}
return res;
@@ -1131,10 +1129,10 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
auto res = CreateTransaction(wallet, vecSend, nChangePosInOut, coinControl, false);
if (!res) {
- error = res.GetError();
+ error = util::ErrorString(res);
return false;
}
- const auto& txr = res.GetObj();
+ const auto& txr = *res;
CTransactionRef tx_new = txr.tx;
nFeeRet = txr.fee;
nChangePosInOut = txr.change_pos;
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 96ecd690be..fdb5113ba4 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -155,7 +155,7 @@ struct CreatedTransactionResult
* selected by SelectCoins(); Also create the change output, when needed
* @note passing change_pos as -1 will result in setting a random position
*/
-BResult<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true);
+util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true);
/**
* Insert additional inputs into the transaction by
diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp
index 01d24da981..3356128112 100644
--- a/src/wallet/test/availablecoins_tests.cpp
+++ b/src/wallet/test/availablecoins_tests.cpp
@@ -33,7 +33,7 @@ public:
constexpr int RANDOM_CHANGE_POSITION = -1;
auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy);
BOOST_CHECK(res);
- tx = res.GetObj().tx;
+ tx = res->tx;
}
wallet->CommitTransaction(tx, {}, {});
CMutableTransaction blocktx;
@@ -57,7 +57,7 @@ public:
BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup)
{
CoinsResult available_coins;
- BResult<CTxDestination> dest;
+ util::Result<CTxDestination> dest{util::Error{}};
LOCK(wallet->cs_wallet);
// Verify our wallet has one usable coinbase UTXO before starting
@@ -75,28 +75,28 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup)
// Bech32m
dest = wallet->GetNewDestination(OutputType::BECH32M, "");
- BOOST_ASSERT(dest.HasRes());
- AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 1 * COIN, /*fSubtractFeeFromAmount=*/true});
+ BOOST_ASSERT(dest);
+ AddTx(CRecipient{{GetScriptForDestination(*dest)}, 1 * COIN, /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.bech32m.size(), 2U);
// Bech32
dest = wallet->GetNewDestination(OutputType::BECH32, "");
- BOOST_ASSERT(dest.HasRes());
- AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 2 * COIN, /*fSubtractFeeFromAmount=*/true});
+ BOOST_ASSERT(dest);
+ AddTx(CRecipient{{GetScriptForDestination(*dest)}, 2 * COIN, /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.bech32.size(), 2U);
// P2SH-SEGWIT
dest = wallet->GetNewDestination(OutputType::P2SH_SEGWIT, "");
- AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 3 * COIN, /*fSubtractFeeFromAmount=*/true});
+ AddTx(CRecipient{{GetScriptForDestination(*dest)}, 3 * COIN, /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.P2SH_segwit.size(), 2U);
// Legacy (P2PKH)
dest = wallet->GetNewDestination(OutputType::LEGACY, "");
- BOOST_ASSERT(dest.HasRes());
- AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 4 * COIN, /*fSubtractFeeFromAmount=*/true});
+ BOOST_ASSERT(dest);
+ AddTx(CRecipient{{GetScriptForDestination(*dest)}, 4 * COIN, /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U);
}
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index cd7fd3f4dd..a4a7216436 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -74,9 +74,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
if (spendable) {
- auto op_dest = wallet.GetNewDestination(OutputType::BECH32, "");
- assert(op_dest.HasRes());
- tx.vout[nInput].scriptPubKey = GetScriptForDestination(op_dest.GetObj());
+ tx.vout[nInput].scriptPubKey = GetScriptForDestination(*Assert(wallet.GetNewDestination(OutputType::BECH32, "")));
}
uint256 txid = tx.GetHash();
diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp
index 5c173773e4..5e9cd4001b 100644
--- a/src/wallet/test/fuzz/notifications.cpp
+++ b/src/wallet/test/fuzz/notifications.cpp
@@ -69,14 +69,13 @@ struct FuzzedWallet {
CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider)
{
auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)};
- BResult<CTxDestination> op_dest;
+ util::Result<CTxDestination> op_dest{util::Error{}};
if (fuzzed_data_provider.ConsumeBool()) {
op_dest = wallet->GetNewDestination(type, "");
} else {
op_dest = wallet->GetNewChangeDestination(type);
}
- assert(op_dest.HasRes());
- return GetScriptForDestination(op_dest.GetObj());
+ return GetScriptForDestination(*Assert(op_dest));
}
};
diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index 53c3b5d2ae..dc935a1a04 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -35,7 +35,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
coin_control.m_change_type = OutputType::LEGACY;
auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, coin_control);
BOOST_CHECK(res);
- const auto& txr = res.GetObj();
+ const auto& txr = *res;
BOOST_CHECK_EQUAL(txr.tx->vout.size(), 1);
BOOST_CHECK_EQUAL(txr.tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - txr.fee);
BOOST_CHECK_GT(txr.fee, 0);
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index d3a760742d..1d2235be3d 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -536,7 +536,7 @@ public:
constexpr int RANDOM_CHANGE_POSITION = -1;
auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy);
BOOST_CHECK(res);
- tx = res.GetObj().tx;
+ tx = res->tx;
}
wallet->CommitTransaction(tx, {}, {});
CMutableTransaction blocktx;
@@ -918,8 +918,8 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup)
// Add tx to wallet
const auto& op_dest = wallet.GetNewDestination(OutputType::BECH32M, "");
- BOOST_ASSERT(op_dest.HasRes());
- const CTxDestination& dest = op_dest.GetObj();
+ BOOST_ASSERT(op_dest);
+ const CTxDestination& dest = *op_dest;
CMutableTransaction mtx;
mtx.vout.push_back({COIN, GetScriptForDestination(dest)});
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 9a48a1ca21..e750cd5a2c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2330,24 +2330,24 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
return res;
}
-BResult<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label)
+util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label)
{
LOCK(cs_wallet);
auto spk_man = GetScriptPubKeyMan(type, false /* internal */);
if (!spk_man) {
- return strprintf(_("Error: No %s addresses available."), FormatOutputType(type));
+ return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))};
}
spk_man->TopUp();
auto op_dest = spk_man->GetNewDestination(type);
if (op_dest) {
- SetAddressBook(op_dest.GetObj(), label, "receive");
+ SetAddressBook(*op_dest, label, "receive");
}
return op_dest;
}
-BResult<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type)
+util::Result<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type)
{
LOCK(cs_wallet);
@@ -2355,7 +2355,7 @@ BResult<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type)
bilingual_str error;
ReserveDestination reservedest(this, type);
if (!reservedest.GetReservedDestination(dest, true, error)) {
- return error;
+ return util::Error{error};
}
reservedest.KeepDestination();
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 20f0c3579c..f2b9723840 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -670,8 +670,8 @@ public:
*/
void MarkDestinationsDirty(const std::set<CTxDestination>& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
- BResult<CTxDestination> GetNewDestination(const OutputType type, const std::string label);
- BResult<CTxDestination> GetNewChangeDestination(const OutputType type);
+ util::Result<CTxDestination> GetNewDestination(const OutputType type, const std::string label);
+ util::Result<CTxDestination> GetNewChangeDestination(const OutputType type);
isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-22 03:48:57 +0000