aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/Makefile.am1
-rw-r--r--src/bench/wallet_loading.cpp7
-rw-r--r--src/interfaces/wallet.h8
-rw-r--r--src/qt/addresstablemodel.cpp14
-rw-r--r--src/qt/walletmodel.cpp6
-rw-r--r--src/test/util/wallet.cpp7
-rw-r--r--src/util/result.h43
-rw-r--r--src/wallet/feebumper.cpp13
-rw-r--r--src/wallet/interfaces.cpp24
-rw-r--r--src/wallet/rpc/addresses.cpp18
-rw-r--r--src/wallet/rpc/spend.cpp12
-rw-r--r--src/wallet/scriptpubkeyman.cpp35
-rw-r--r--src/wallet/scriptpubkeyman.h7
-rw-r--r--src/wallet/spend.cpp103
-rw-r--r--src/wallet/spend.h9
-rw-r--r--src/wallet/test/coinselector_tests.cpp8
-rw-r--r--src/wallet/test/fuzz/notifications.cpp11
-rw-r--r--src/wallet/test/spend_tests.cpp15
-rw-r--r--src/wallet/test/wallet_tests.cpp18
-rw-r--r--src/wallet/wallet.cpp29
-rw-r--r--src/wallet/wallet.h6
21 files changed, 201 insertions, 193 deletions
diff --git a/src/Makefile.am b/src/Makefile.am
index 3fbbe180fc..a9e9db0a7d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -277,6 +277,7 @@ BITCOIN_CORE_H = \
util/overloaded.h \
util/rbf.h \
util/readwritefile.h \
+ util/result.h \
util/serfloat.h \
util/settings.h \
util/sock.h \
diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index f611383788..d258d7a29e 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -47,12 +47,11 @@ static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
static void AddTx(CWallet& wallet)
{
- bilingual_str error;
- CTxDestination dest;
- wallet.GetNewDestination(OutputType::BECH32, "", dest, error);
+ const auto& dest = wallet.GetNewDestination(OutputType::BECH32, "");
+ assert(dest.HasRes());
CMutableTransaction mtx;
- mtx.vout.push_back({COIN, GetScriptForDestination(dest)});
+ mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())});
mtx.vin.push_back(CTxIn());
wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{});
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index b3cb0ae387..fe198c999b 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -12,6 +12,7 @@
#include <script/standard.h> // For CTxDestination
#include <support/allocators/secure.h> // For SecureString
#include <util/message.h>
+#include <util/result.h>
#include <util/ui_change_type.h>
#include <cstdint>
@@ -87,7 +88,7 @@ public:
virtual std::string getWalletName() = 0;
// Get a new address.
- virtual bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) = 0;
+ virtual BResult<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;
@@ -138,12 +139,11 @@ public:
virtual void listLockedCoins(std::vector<COutPoint>& outputs) = 0;
//! Create transaction.
- virtual CTransactionRef createTransaction(const std::vector<wallet::CRecipient>& recipients,
+ virtual BResult<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients,
const wallet::CCoinControl& coin_control,
bool sign,
int& change_pos,
- CAmount& fee,
- bilingual_str& fail_reason) = 0;
+ CAmount& fee) = 0;
//! Commit transaction.
virtual void commitTransaction(CTransactionRef tx,
diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index 27ee9509e6..bb50b5384a 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -370,23 +370,21 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
else if(type == Receive)
{
// Generate a new address to associate with given label
- CTxDestination dest;
- if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest))
- {
+ auto op_dest = walletModel->wallet().getNewDestination(address_type, strLabel);
+ if (!op_dest) {
WalletModel::UnlockContext ctx(walletModel->requestUnlock());
- if(!ctx.isValid())
- {
+ if (!ctx.isValid()) {
// Unlock wallet failed or was cancelled
editStatus = WALLET_UNLOCK_FAILURE;
return QString();
}
- if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest))
- {
+ op_dest = walletModel->wallet().getNewDestination(address_type, strLabel);
+ if (!op_dest) {
editStatus = KEY_GENERATION_FAILURE;
return QString();
}
}
- strAddress = EncodeDestination(dest);
+ strAddress = EncodeDestination(op_dest.GetObj());
}
else
{
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index ab4d1cae3f..bb6079afee 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -204,10 +204,10 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
{
CAmount nFeeRequired = 0;
int nChangePosRet = -1;
- bilingual_str error;
auto& newTx = transaction.getWtx();
- newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error);
+ const auto& res = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired);
+ newTx = res ? res.GetObj() : 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(error.translated),
+ Q_EMIT message(tr("Send Coins"), QString::fromStdString(res.GetError().translated),
CClientUIInterface::MSG_ERROR);
return TransactionCreationFailed;
}
diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp
index 52aaeabccf..7a00ac9e1f 100644
--- a/src/test/util/wallet.cpp
+++ b/src/test/util/wallet.cpp
@@ -20,11 +20,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq
std::string getnewaddress(CWallet& w)
{
constexpr auto output_type = OutputType::BECH32;
- CTxDestination dest;
- bilingual_str error;
- if (!w.GetNewDestination(output_type, "", dest, error)) assert(false);
+ auto op_dest = w.GetNewDestination(output_type, "");
+ assert(op_dest.HasRes());
- return EncodeDestination(dest);
+ return EncodeDestination(op_dest.GetObj());
}
#endif // ENABLE_WALLET
diff --git a/src/util/result.h b/src/util/result.h
new file mode 100644
index 0000000000..dcf5edaa5b
--- /dev/null
+++ b/src/util/result.h
@@ -0,0 +1,43 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_RESULT_H
+#define BITCOIN_UTIL_RESULT_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 {
+private:
+ std::variant<bilingual_str, T> m_variant;
+
+public:
+ BResult() : m_variant(Untranslated("")) {}
+ BResult(const T& _obj) : m_variant(_obj) {}
+ BResult(const bilingual_str& error) : m_variant(error) {}
+
+ /* Whether the function succeeded or not */
+ bool HasRes() const { return std::holds_alternative<T>(m_variant); }
+
+ /* In case of success, the result object */
+ const T& GetObj() const {
+ assert(HasRes());
+ return std::get<T>(m_variant);
+ }
+
+ /* In case of failure, the error cause */
+ const bilingual_str& GetError() const {
+ assert(!HasRes());
+ return std::get<bilingual_str>(m_variant);
+ }
+
+ explicit operator bool() const { return HasRes(); }
+};
+
+#endif // BITCOIN_UTIL_RESULT_H
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index 5e70ed4a30..43fdcee48d 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -219,19 +219,18 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
new_coin_control.m_min_depth = 1;
constexpr int RANDOM_CHANGE_POSITION = -1;
- bilingual_str fail_reason;
- FeeCalculation fee_calc_out;
- std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, fail_reason, new_coin_control, fee_calc_out, false);
- if (!txr) {
- errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + fail_reason);
+ 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());
return Result::WALLET_ERROR;
}
+ const auto& txr = res.GetObj();
// Write back new fee if successful
- new_fee = txr->fee;
+ new_fee = txr.fee;
// Write back transaction
- mtx = CMutableTransaction(*txr->tx);
+ mtx = CMutableTransaction(*txr.tx);
return Result::OK;
}
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 327276f2ed..547d972c8d 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -146,11 +146,10 @@ 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(); }
- bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) override
+ BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) override
{
LOCK(m_wallet->cs_wallet);
- bilingual_str error;
- return m_wallet->GetNewDestination(type, label, dest, error);
+ return m_wallet->GetNewDestination(type, label);
}
bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override
{
@@ -250,22 +249,21 @@ public:
LOCK(m_wallet->cs_wallet);
return m_wallet->ListLockedCoins(outputs);
}
- CTransactionRef createTransaction(const std::vector<CRecipient>& recipients,
+ BResult<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients,
const CCoinControl& coin_control,
bool sign,
int& change_pos,
- CAmount& fee,
- bilingual_str& fail_reason) override
+ CAmount& fee) override
{
LOCK(m_wallet->cs_wallet);
- FeeCalculation fee_calc_out;
- std::optional<CreatedTransactionResult> txr = CreateTransaction(*m_wallet, recipients, change_pos,
- fail_reason, coin_control, fee_calc_out, sign);
- if (!txr) return {};
- fee = txr->fee;
- change_pos = txr->change_pos;
+ const auto& res = CreateTransaction(*m_wallet, recipients, change_pos,
+ coin_control, sign);
+ if (!res) return res.GetError();
+ const auto& txr = res.GetObj();
+ fee = txr.fee;
+ change_pos = txr.change_pos;
- return txr->tx;
+ return txr.tx;
}
void commitTransaction(CTransactionRef tx,
WalletValueMap value_map,
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index da4cc44ee6..9428d049de 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -58,13 +58,12 @@ RPCHelpMan getnewaddress()
output_type = parsed.value();
}
- CTxDestination dest;
- bilingual_str error;
- if (!pwallet->GetNewDestination(output_type, label, dest, error)) {
- throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original);
+ auto op_dest = pwallet->GetNewDestination(output_type, label);
+ if (!op_dest) {
+ throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
}
- return EncodeDestination(dest);
+ return EncodeDestination(op_dest.GetObj());
},
};
}
@@ -106,12 +105,11 @@ RPCHelpMan getrawchangeaddress()
output_type = parsed.value();
}
- CTxDestination dest;
- bilingual_str error;
- if (!pwallet->GetNewChangeDestination(output_type, dest, error)) {
- throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original);
+ auto op_dest = pwallet->GetNewChangeDestination(output_type);
+ if (!op_dest) {
+ throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
}
- return EncodeDestination(dest);
+ return EncodeDestination(op_dest.GetObj());
},
};
}
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 0e8cee5db8..22c1f2bdc2 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -156,18 +156,16 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
// Send
constexpr int RANDOM_CHANGE_POSITION = -1;
- bilingual_str error;
- FeeCalculation fee_calc_out;
- std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc_out, true);
- if (!txr) {
- throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original);
+ auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, true);
+ if (!res) {
+ throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original);
}
- CTransactionRef tx = txr->tx;
+ const CTransactionRef& tx = res.GetObj().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(fee_calc_out.reason));
+ entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason));
return entry;
}
return tx->GetHash().GetHex();
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 1fec82a485..bba31dfe0b 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -21,26 +21,22 @@ 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;
-bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error)
+BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type)
{
if (LEGACY_OUTPUT_TYPES.count(type) == 0) {
- error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");
- return false;
+ return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");;
}
assert(type != OutputType::BECH32M);
LOCK(cs_KeyStore);
- error.clear();
// Generate a new key that is added to wallet
CPubKey new_key;
if (!GetKeyFromPool(new_key, type)) {
- error = _("Error: Keypool ran out, please call keypoolrefill first");
- return false;
+ return _("Error: Keypool ran out, please call keypoolrefill first");
}
LearnRelatedScripts(new_key, type);
- dest = GetDestinationForKey(new_key, type);
- return true;
+ return GetDestinationForKey(new_key, type);
}
typedef std::vector<unsigned char> valtype;
@@ -1658,12 +1654,11 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const
return set_address;
}
-bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error)
+BResult<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()) {
- error = _("No addresses available");
- return false;
+ return _("No addresses available");
}
{
LOCK(cs_desc_man);
@@ -1681,15 +1676,14 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest
std::vector<CScript> scripts_temp;
if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) {
// We can't generate anymore keys
- error = _("Error: Keypool ran out, please call keypoolrefill first");
- return false;
+ return _("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
- error = _("Error: Keypool ran out, please call keypoolrefill first");
- return false;
+ return _("Error: Keypool ran out, please call keypoolrefill first");
}
+ CTxDestination dest;
std::optional<OutputType> out_script_type = m_wallet_descriptor.descriptor->GetOutputType();
if (out_script_type && out_script_type == type) {
ExtractDestination(scripts_temp[0], dest);
@@ -1698,7 +1692,7 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest
}
m_wallet_descriptor.next_index++;
WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor);
- return true;
+ return dest;
}
}
@@ -1769,9 +1763,14 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle
bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error)
{
LOCK(cs_desc_man);
- bool result = GetNewDestination(type, address, error);
+ auto op_dest = GetNewDestination(type);
index = m_wallet_descriptor.next_index - 1;
- return result;
+ if (op_dest) {
+ address = op_dest.GetObj();
+ } else {
+ error = op_dest.GetError();
+ }
+ return op_dest.HasRes();
}
void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr)
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index ad924791af..eebc05330f 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -11,6 +11,7 @@
#include <script/standard.h>
#include <util/error.h>
#include <util/message.h>
+#include <util/result.h>
#include <util/time.h>
#include <wallet/crypter.h>
#include <wallet/ismine.h>
@@ -171,7 +172,7 @@ protected:
public:
explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {}
virtual ~ScriptPubKeyMan() {};
- virtual bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) { return false; }
+ virtual BResult<CTxDestination> GetNewDestination(const OutputType type) { return 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.
@@ -359,7 +360,7 @@ private:
public:
using ScriptPubKeyMan::ScriptPubKeyMan;
- bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override;
+ BResult<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;
@@ -567,7 +568,7 @@ public:
mutable RecursiveMutex cs_desc_man;
- bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override;
+ BResult<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 21007f5a92..9be29c4709 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -652,19 +652,16 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng
}
}
-static std::optional<CreatedTransactionResult> CreateTransactionInternal(
+static BResult<CreatedTransactionResult> CreateTransactionInternal(
CWallet& wallet,
const std::vector<CRecipient>& vecSend,
int change_pos,
- bilingual_str& error,
const CCoinControl& coin_control,
- FeeCalculation& fee_calc_out,
bool sign) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
AssertLockHeld(wallet.cs_wallet);
// out variables, to be packed into returned result structure
- CTransactionRef tx;
CAmount nFeeRet;
int nChangePosInOut = change_pos;
@@ -693,6 +690,7 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
// Create change script that will be used if we need change
CScript scriptChange;
+ bilingual_str error; // possible error str
// coin control: send change to custom address
if (!std::get_if<CNoDestination>(&coin_control.destChange)) {
@@ -740,13 +738,11 @@ static std::optional<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) {
- 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));
- return std::nullopt;
+ 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));
}
if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) {
// eventually allow a fallback fee
- error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.");
- return std::nullopt;
+ return _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.");
}
// Calculate the cost of change
@@ -771,10 +767,8 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION);
}
- if (IsDust(txout, wallet.chain().relayDustFee()))
- {
- error = _("Transaction amount too small");
- return std::nullopt;
+ if (IsDust(txout, wallet.chain().relayDustFee())) {
+ return _("Transaction amount too small");
}
txNew.vout.push_back(txout);
}
@@ -795,8 +789,7 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
// Choose coins to use
std::optional<SelectionResult> result = SelectCoins(wallet, res_available_coins.coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
if (!result) {
- error = _("Insufficient funds");
- return std::nullopt;
+ return _("Insufficient funds");
}
TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue());
@@ -810,10 +803,8 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
// Insert change txn at random position:
nChangePosInOut = rng_fast.randrange(txNew.vout.size() + 1);
}
- else if ((unsigned int)nChangePosInOut > txNew.vout.size())
- {
- error = _("Transaction change output index out of range");
- return std::nullopt;
+ else if ((unsigned int)nChangePosInOut > txNew.vout.size()) {
+ return _("Transaction change output index out of range");
}
assert(nChangePosInOut != -1);
@@ -840,8 +831,7 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control);
int nBytes = tx_sizes.vsize;
if (nBytes == -1) {
- error = _("Missing solving data for estimating transaction size");
- return std::nullopt;
+ return _("Missing solving data for estimating transaction size");
}
nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes);
@@ -901,11 +891,10 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
// Error if this output is reduced to be below dust
if (IsDust(txout, wallet.chain().relayDustFee())) {
if (txout.nValue < 0) {
- error = _("The transaction amount is too small to pay the fee");
+ return _("The transaction amount is too small to pay the fee");
} else {
- error = _("The transaction amount is too small to send after the fee has been deducted");
+ return _("The transaction amount is too small to send after the fee has been deducted");
}
- return std::nullopt;
}
}
++i;
@@ -915,42 +904,37 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
// Give up if change keypool ran out and change is required
if (scriptChange.empty() && nChangePosInOut != -1) {
- return std::nullopt;
+ return error;
}
if (sign && !wallet.SignTransaction(txNew)) {
- error = _("Signing transaction failed");
- return std::nullopt;
+ return _("Signing transaction failed");
}
// Return the constructed transaction data.
- tx = MakeTransactionRef(std::move(txNew));
+ CTransactionRef tx = MakeTransactionRef(std::move(txNew));
// Limit size
if ((sign && GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT) ||
(!sign && tx_sizes.weight > MAX_STANDARD_TX_WEIGHT))
{
- error = _("Transaction too large");
- return std::nullopt;
+ return _("Transaction too large");
}
if (nFeeRet > wallet.m_default_max_tx_fee) {
- error = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);
- return std::nullopt;
+ return 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)) {
- error = _("Transaction has too long of a mempool chain");
- return std::nullopt;
+ return _("Transaction has too long of a mempool chain");
}
}
// Before we return success, we assume any change key will be used to prevent
// accidental re-use.
reservedest.KeepDestination();
- fee_calc_out = feeCalc;
wallet.WalletLogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay,
@@ -960,52 +944,50 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
feeCalc.est.fail.start, feeCalc.est.fail.end,
(feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) > 0.0 ? 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) : 0.0,
feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool);
- return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut);
+ return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut, feeCalc);
}
-std::optional<CreatedTransactionResult> CreateTransaction(
+BResult<CreatedTransactionResult> CreateTransaction(
CWallet& wallet,
const std::vector<CRecipient>& vecSend,
int change_pos,
- bilingual_str& error,
const CCoinControl& coin_control,
- FeeCalculation& fee_calc_out,
bool sign)
{
if (vecSend.empty()) {
- error = _("Transaction must have at least one recipient");
- return std::nullopt;
+ return _("Transaction must have at least one recipient");
}
if (std::any_of(vecSend.cbegin(), vecSend.cend(), [](const auto& recipient){ return recipient.nAmount < 0; })) {
- error = _("Transaction amounts must not be negative");
- return std::nullopt;
+ return _("Transaction amounts must not be negative");
}
LOCK(wallet.cs_wallet);
- std::optional<CreatedTransactionResult> txr_ungrouped = CreateTransactionInternal(wallet, vecSend, change_pos, error, coin_control, fee_calc_out, sign);
- TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), txr_ungrouped.has_value(),
- txr_ungrouped.has_value() ? txr_ungrouped->fee : 0, txr_ungrouped.has_value() ? txr_ungrouped->change_pos : 0);
- if (!txr_ungrouped) return std::nullopt;
+ 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);
+ if (!res) return res;
+ const auto& txr_ungrouped = res.GetObj();
// 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) {
+ 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;
- bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results
- std::optional<CreatedTransactionResult> txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, error2, tmp_cc, fee_calc_out, sign);
+ 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};
// 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};
+ 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(),
txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() ? txr_grouped->change_pos : 0);
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 txr_grouped;
+ txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped");
+ if (use_aps) return res_tx_grouped;
}
}
- return txr_ungrouped;
+ return res;
}
bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl)
@@ -1041,12 +1023,15 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
}
}
- FeeCalculation fee_calc_out;
- std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, vecSend, nChangePosInOut, error, coinControl, fee_calc_out, false);
- if (!txr) return false;
- CTransactionRef tx_new = txr->tx;
- nFeeRet = txr->fee;
- nChangePosInOut = txr->change_pos;
+ auto res = CreateTransaction(wallet, vecSend, nChangePosInOut, coinControl, false);
+ if (!res) {
+ error = res.GetError();
+ return false;
+ }
+ const auto& txr = res.GetObj();
+ CTransactionRef tx_new = txr.tx;
+ nFeeRet = txr.fee;
+ nChangePosInOut = txr.change_pos;
if (nChangePosInOut != -1) {
tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]);
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 21f0095e77..ab0ff1ee58 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -6,6 +6,8 @@
#define BITCOIN_WALLET_SPEND_H
#include <consensus/amount.h>
+#include <policy/fees.h> // for FeeCalculation
+#include <util/result.h>
#include <wallet/coinselection.h>
#include <wallet/transaction.h>
#include <wallet/wallet.h>
@@ -100,10 +102,11 @@ struct CreatedTransactionResult
{
CTransactionRef tx;
CAmount fee;
+ FeeCalculation fee_calc;
int change_pos;
- CreatedTransactionResult(CTransactionRef tx, CAmount fee, int change_pos)
- : tx(tx), fee(fee), change_pos(change_pos) {}
+ CreatedTransactionResult(CTransactionRef _tx, CAmount _fee, int _change_pos, const FeeCalculation& _fee_calc)
+ : tx(_tx), fee(_fee), fee_calc(_fee_calc), change_pos(_change_pos) {}
};
/**
@@ -111,7 +114,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
*/
-std::optional<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true);
+BResult<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/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 9dd17c8e48..a418105ee1 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -74,11 +74,9 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
if (spendable) {
- CTxDestination dest;
- bilingual_str error;
- const bool destination_ok = wallet.GetNewDestination(OutputType::BECH32, "", dest, error);
- assert(destination_ok);
- tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest);
+ auto op_dest = wallet.GetNewDestination(OutputType::BECH32, "");
+ assert(op_dest.HasRes());
+ tx.vout[nInput].scriptPubKey = GetScriptForDestination(op_dest.GetObj());
}
uint256 txid = tx.GetHash();
diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp
index 9089c8ff46..816bef6148 100644
--- a/src/wallet/test/fuzz/notifications.cpp
+++ b/src/wallet/test/fuzz/notifications.cpp
@@ -69,15 +69,14 @@ struct FuzzedWallet {
CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider)
{
auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)};
- CTxDestination dest;
- bilingual_str error;
+ BResult<CTxDestination> op_dest;
if (fuzzed_data_provider.ConsumeBool()) {
- assert(wallet->GetNewDestination(type, "", dest, error));
+ op_dest = wallet->GetNewDestination(type, "");
} else {
- assert(wallet->GetNewChangeDestination(type, dest, error));
+ op_dest = wallet->GetNewChangeDestination(type);
}
- assert(error.empty());
- return GetScriptForDestination(dest);
+ assert(op_dest.HasRes());
+ return GetScriptForDestination(op_dest.GetObj());
}
};
diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index bdc148afb4..53c3b5d2ae 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -28,19 +28,18 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
auto check_tx = [&wallet](CAmount leftover_input_amount) {
CRecipient recipient{GetScriptForRawPubKey({}), 50 * COIN - leftover_input_amount, true /* subtract fee */};
constexpr int RANDOM_CHANGE_POSITION = -1;
- bilingual_str error;
CCoinControl coin_control;
coin_control.m_feerate.emplace(10000);
coin_control.fOverrideFeeRate = true;
// We need to use a change type with high cost of change so that the leftover amount will be dropped to fee instead of added as a change output
coin_control.m_change_type = OutputType::LEGACY;
- FeeCalculation fee_calc;
- std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc);
- BOOST_CHECK(txr.has_value());
- 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);
- return txr->fee;
+ auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, coin_control);
+ BOOST_CHECK(res);
+ const auto& txr = res.GetObj();
+ 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);
+ return txr.fee;
};
// Send full input amount to recipient, check that only nonzero fee is
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 4093392ff9..81c01e5023 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -4,7 +4,6 @@
#include <wallet/wallet.h>
-#include <any>
#include <future>
#include <memory>
#include <stdint.h>
@@ -13,7 +12,6 @@
#include <interfaces/chain.h>
#include <key_io.h>
#include <node/blockstorage.h>
-#include <node/context.h>
#include <policy/policy.h>
#include <rpc/server.h>
#include <test/util/logging.h>
@@ -536,14 +534,12 @@ public:
CWalletTx& AddTx(CRecipient recipient)
{
CTransactionRef tx;
- bilingual_str error;
CCoinControl dummy;
- FeeCalculation fee_calc_out;
{
constexpr int RANDOM_CHANGE_POSITION = -1;
- std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, dummy, fee_calc_out);
- BOOST_CHECK(txr.has_value());
- tx = txr->tx;
+ auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy);
+ BOOST_CHECK(res);
+ tx = res.GetObj().tx;
}
wallet->CommitTransaction(tx, {}, {});
CMutableTransaction blocktx;
@@ -629,9 +625,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
wallet->SetMinVersion(FEATURE_LATEST);
wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
BOOST_CHECK(!wallet->TopUpKeyPool(1000));
- CTxDestination dest;
- bilingual_str error;
- BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error));
+ BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, ""));
}
{
const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase());
@@ -639,9 +633,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
wallet->SetMinVersion(FEATURE_LATEST);
wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
- CTxDestination dest;
- bilingual_str error;
- BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error));
+ BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, ""));
}
}
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 79ec29f19a..de4a8a6eb7 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2318,37 +2318,36 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
return res;
}
-bool CWallet::GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, bilingual_str& error)
+BResult<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label)
{
LOCK(cs_wallet);
- error.clear();
- bool result = false;
auto spk_man = GetScriptPubKeyMan(type, false /* internal */);
- if (spk_man) {
- spk_man->TopUp();
- result = spk_man->GetNewDestination(type, dest, error);
- } else {
- error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type));
+ if (!spk_man) {
+ return strprintf(_("Error: No %s addresses available."), FormatOutputType(type));
}
- if (result) {
- SetAddressBook(dest, label, "receive");
+
+ spk_man->TopUp();
+ auto op_dest = spk_man->GetNewDestination(type);
+ if (op_dest) {
+ SetAddressBook(op_dest.GetObj(), label, "receive");
}
- return result;
+ return op_dest;
}
-bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& dest, bilingual_str& error)
+BResult<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type)
{
LOCK(cs_wallet);
- error.clear();
+ CTxDestination dest;
+ bilingual_str error;
ReserveDestination reservedest(this, type);
if (!reservedest.GetReservedDestination(dest, true, error)) {
- return false;
+ return error;
}
reservedest.KeepDestination();
- return true;
+ return dest;
}
std::optional<int64_t> CWallet::GetOldestKeyPoolTime() const
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 865ce92311..e2c3d76438 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -15,6 +15,7 @@
#include <psbt.h>
#include <tinyformat.h>
#include <util/message.h>
+#include <util/result.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/system.h>
@@ -45,7 +46,6 @@ using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wall
class CScript;
enum class FeeEstimateMode;
-struct FeeCalculation;
struct bilingual_str;
namespace wallet {
@@ -666,8 +666,8 @@ public:
*/
void MarkDestinationsDirty(const std::set<CTxDestination>& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
- bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, bilingual_str& error);
- bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, bilingual_str& error);
+ BResult<CTxDestination> GetNewDestination(const OutputType type, const std::string label);
+ BResult<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-08-26 19:17:15 +0000