aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRyan Ofsky <ryan@ofsky.org>2023-12-13 11:43:16 -0500
committerRyan Ofsky <ryan@ofsky.org>2024-05-16 10:16:08 -0500
commit02e62c6c9af4beabaeea58fb1ea3ad0dc5094678 (patch)
tree3ddfb29afa69039b8d2444d3d2cd2e069e823aee
parent0d44c44ae33434f366229c612d6edeedf7658963 (diff)
downloadbitcoin-02e62c6c9af4beabaeea58fb1ea3ad0dc5094678.tar.xz
common: Add PSBTError enum
Add separate PSBTError enum instead of reusing TransactionError enum for PSBT operations, and drop unused error codes. The error codes returned by PSBT operations and transaction broadcast functions mostly do not overlap, so using an unified enum makes it harder to call any of these functions and know which errors actually need to be handled. Define PSBTError in the common library because PSBT functionality is implemented in the common library and used by both the node (for rawtransaction RPCs) and the wallet.
-rw-r--r--src/Makefile.am1
-rw-r--r--src/common/types.h26
-rw-r--r--src/interfaces/wallet.h6
-rw-r--r--src/node/types.h6
-rw-r--r--src/psbt.cpp6
-rw-r--r--src/psbt.h4
-rw-r--r--src/qt/psbtoperationsdialog.cpp17
-rw-r--r--src/qt/sendcoinsdialog.cpp17
-rw-r--r--src/qt/walletmodel.cpp4
-rw-r--r--src/rpc/rawtransaction.cpp5
-rw-r--r--src/rpc/util.cpp27
-rw-r--r--src/rpc/util.h4
-rw-r--r--src/test/fuzz/kitchen_sink.cpp5
-rw-r--r--src/util/error.cpp33
-rw-r--r--src/util/error.h5
-rw-r--r--src/wallet/external_signer_scriptpubkeyman.cpp10
-rw-r--r--src/wallet/external_signer_scriptpubkeyman.h2
-rw-r--r--src/wallet/feebumper.cpp4
-rw-r--r--src/wallet/interfaces.cpp3
-rw-r--r--src/wallet/rpc/spend.cpp22
-rw-r--r--src/wallet/scriptpubkeyman.cpp18
-rw-r--r--src/wallet/scriptpubkeyman.h7
-rw-r--r--src/wallet/test/psbt_wallet_tests.cpp4
-rw-r--r--src/wallet/wallet.cpp11
-rw-r--r--src/wallet/wallet.h6
25 files changed, 156 insertions, 97 deletions
diff --git a/src/Makefile.am b/src/Makefile.am
index c7b193daaf..36b9102712 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -137,6 +137,7 @@ BITCOIN_CORE_H = \
common/bloom.h \
common/init.h \
common/run_command.h \
+ common/types.h \
common/url.h \
compat/assumptions.h \
compat/byteswap.h \
diff --git a/src/common/types.h b/src/common/types.h
new file mode 100644
index 0000000000..0d9cb67ce9
--- /dev/null
+++ b/src/common/types.h
@@ -0,0 +1,26 @@
+// Copyright (c) 2010-2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+//! @file common/types.h is a home for simple enum and struct type definitions
+//! that can be used internally by functions in the libbitcoin_common library,
+//! but also used externally by node, wallet, and GUI code.
+//!
+//! This file is intended to define only simple types that do not have external
+//! dependencies. More complicated types should be defined in dedicated header
+//! files.
+
+#ifndef BITCOIN_COMMON_TYPES_H
+#define BITCOIN_COMMON_TYPES_H
+
+namespace common {
+enum class PSBTError {
+ MISSING_INPUTS,
+ SIGHASH_MISMATCH,
+ EXTERNAL_SIGNER_NOT_FOUND,
+ EXTERNAL_SIGNER_FAILED,
+ UNSUPPORTED,
+};
+} // namespace common
+
+#endif // BITCOIN_COMMON_TYPES_H
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index f1bfffa7d4..1c916aacb0 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -30,9 +30,11 @@ class CFeeRate;
class CKey;
enum class FeeReason;
enum class OutputType;
-enum class TransactionError;
struct PartiallySignedTransaction;
struct bilingual_str;
+namespace common {
+enum class PSBTError;
+} // namespace common
namespace wallet {
class CCoinControl;
class CWallet;
@@ -202,7 +204,7 @@ public:
int& num_blocks) = 0;
//! Fill PSBT.
- virtual TransactionError fillPSBT(int sighash_type,
+ virtual std::optional<common::PSBTError> fillPSBT(int sighash_type,
bool sign,
bool bip32derivs,
size_t* n_signed,
diff --git a/src/node/types.h b/src/node/types.h
index 7febc45a2b..9dd2edebb5 100644
--- a/src/node/types.h
+++ b/src/node/types.h
@@ -17,16 +17,10 @@ enum class TransactionError {
OK, //!< No error
MISSING_INPUTS,
ALREADY_IN_CHAIN,
- P2P_DISABLED,
MEMPOOL_REJECTED,
MEMPOOL_ERROR,
- INVALID_PSBT,
- PSBT_MISMATCH,
- SIGHASH_MISMATCH,
MAX_FEE_EXCEEDED,
MAX_BURN_EXCEEDED,
- EXTERNAL_SIGNER_NOT_FOUND,
- EXTERNAL_SIGNER_FAILED,
INVALID_PACKAGE,
};
diff --git a/src/psbt.cpp b/src/psbt.cpp
index b2ee3ce7a5..9e59b52b41 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -508,17 +508,17 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti
return true;
}
-TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs)
+bool CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs)
{
out = psbtxs[0]; // Copy the first one
// Merge
for (auto it = std::next(psbtxs.begin()); it != psbtxs.end(); ++it) {
if (!out.Merge(*it)) {
- return TransactionError::PSBT_MISMATCH;
+ return false;
}
}
- return TransactionError::OK;
+ return true;
}
std::string PSBTRoleName(PSBTRole role) {
diff --git a/src/psbt.h b/src/psbt.h
index 3f74083717..95f6fdf91a 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -1263,9 +1263,9 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti
*
* @param[out] out the combined PSBT, if successful
* @param[in] psbtxs the PSBTs to combine
- * @return error (OK if we successfully combined the transactions, other error if they were not compatible)
+ * @return True if we successfully combined the transactions, false if they were not compatible
*/
-[[nodiscard]] TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs);
+[[nodiscard]] bool CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs);
//! Decode a base64ed PSBT into a PartiallySignedTransaction
[[nodiscard]] bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error);
diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp
index 353709c7f5..7bbebec46c 100644
--- a/src/qt/psbtoperationsdialog.cpp
+++ b/src/qt/psbtoperationsdialog.cpp
@@ -13,6 +13,7 @@
#include <qt/forms/ui_psbtoperationsdialog.h>
#include <qt/guiutil.h>
#include <qt/optionsmodel.h>
+#include <util/error.h>
#include <util/fs.h>
#include <util/strencodings.h>
@@ -55,10 +56,10 @@ void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx)
bool complete = FinalizePSBT(psbtx); // Make sure all existing signatures are fully combined before checking for completeness.
if (m_wallet_model) {
size_t n_could_sign;
- TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete);
- if (err != TransactionError::OK) {
+ const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)};
+ if (err) {
showStatus(tr("Failed to load transaction: %1")
- .arg(QString::fromStdString(TransactionErrorString(err).translated)),
+ .arg(QString::fromStdString(PSBTErrorString(*err).translated)),
StatusLevel::ERR);
return;
}
@@ -79,11 +80,11 @@ void PSBTOperationsDialog::signTransaction()
WalletModel::UnlockContext ctx(m_wallet_model->requestUnlock());
- TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete);
+ const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)};
- if (err != TransactionError::OK) {
+ if (err) {
showStatus(tr("Failed to sign transaction: %1")
- .arg(QString::fromStdString(TransactionErrorString(err).translated)), StatusLevel::ERR);
+ .arg(QString::fromStdString(PSBTErrorString(*err).translated)), StatusLevel::ERR);
return;
}
@@ -247,9 +248,9 @@ size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &p
size_t n_signed;
bool complete;
- TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete);
+ const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)};
- if (err != TransactionError::OK) {
+ if (err) {
return 0;
}
return n_signed;
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index 0d8c0f7a63..401c79f007 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -37,6 +37,7 @@
#include <QSettings>
#include <QTextDocument>
+using common::PSBTError;
using wallet::CCoinControl;
using wallet::DEFAULT_PAY_TX_FEE;
@@ -442,26 +443,26 @@ void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx)
}
bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx, CMutableTransaction& mtx, bool& complete) {
- TransactionError err;
+ std::optional<PSBTError> err;
try {
err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
} catch (const std::runtime_error& e) {
QMessageBox::critical(nullptr, tr("Sign failed"), e.what());
return false;
}
- if (err == TransactionError::EXTERNAL_SIGNER_NOT_FOUND) {
+ if (err == PSBTError::EXTERNAL_SIGNER_NOT_FOUND) {
//: "External signer" means using devices such as hardware wallets.
const QString msg = tr("External signer not found");
QMessageBox::critical(nullptr, msg, msg);
return false;
}
- if (err == TransactionError::EXTERNAL_SIGNER_FAILED) {
+ if (err == PSBTError::EXTERNAL_SIGNER_FAILED) {
//: "External signer" means using devices such as hardware wallets.
const QString msg = tr("External signer failure");
QMessageBox::critical(nullptr, msg, msg);
return false;
}
- if (err != TransactionError::OK) {
+ if (err) {
tfm::format(std::cerr, "Failed to sign PSBT");
processSendCoinsReturn(WalletModel::TransactionCreationFailed);
return false;
@@ -501,9 +502,9 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
// Fill without signing
- TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
+ const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
assert(!complete);
- assert(err == TransactionError::OK);
+ assert(!err);
// Copy PSBT to clipboard and offer to save
presentPSBT(psbtx);
@@ -517,9 +518,9 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
bool complete = false;
// Always fill without signing first. This prevents an external signer
// from being called prematurely and is not expensive.
- TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
+ const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
assert(!complete);
- assert(err == TransactionError::OK);
+ assert(!err);
send_failure = !signWithExternalSigner(psbtx, mtx, complete);
// Don't broadcast when user rejects it on the device or there's a failure:
broadcast = complete && !send_failure;
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 87ad98a4cc..f7843706e0 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -534,8 +534,8 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
// "Create Unsigned" clicked
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
- const TransactionError err = wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete);
- if (err != TransactionError::OK || complete) {
+ const auto err{wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)};
+ if (err || complete) {
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Can't draft transaction."));
return false;
}
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 634be2f7fb..674f9bfe90 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1485,9 +1485,8 @@ static RPCHelpMan combinepsbt()
}
PartiallySignedTransaction merged_psbt;
- const TransactionError error = CombinePSBTs(merged_psbt, psbtxs);
- if (error != TransactionError::OK) {
- throw JSONRPCTransactionError(error);
+ if (!CombinePSBTs(merged_psbt, psbtxs)) {
+ throw JSONRPCError(RPC_INVALID_PARAMETER, "PSBTs not compatible (different transactions)");
}
DataStream ssTx{};
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index f5a2e9eb63..9ae6dd98e9 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -7,6 +7,7 @@
#include <clientversion.h>
#include <core_io.h>
#include <common/args.h>
+#include <common/types.h>
#include <consensus/amount.h>
#include <script/interpreter.h>
#include <key_io.h>
@@ -30,6 +31,8 @@
#include <tuple>
#include <utility>
+using common::PSBTError;
+
const std::string UNIX_EPOCH_TIME = "UNIX epoch time";
const std::string EXAMPLE_ADDRESS[2] = {"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl", "bc1q02ad21edsxd23d32dfgqqsz4vv4nmtfzuklhy3"};
@@ -364,6 +367,18 @@ unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target)
return unsigned_target;
}
+RPCErrorCode RPCErrorFromPSBTError(PSBTError err)
+{
+ switch (err) {
+ case PSBTError::UNSUPPORTED:
+ return RPC_INVALID_PARAMETER;
+ case PSBTError::SIGHASH_MISMATCH:
+ return RPC_DESERIALIZATION_ERROR;
+ default: break;
+ }
+ return RPC_TRANSACTION_ERROR;
+}
+
RPCErrorCode RPCErrorFromTransactionError(TransactionError terr)
{
switch (terr) {
@@ -371,18 +386,16 @@ RPCErrorCode RPCErrorFromTransactionError(TransactionError terr)
return RPC_TRANSACTION_REJECTED;
case TransactionError::ALREADY_IN_CHAIN:
return RPC_TRANSACTION_ALREADY_IN_CHAIN;
- case TransactionError::P2P_DISABLED:
- return RPC_CLIENT_P2P_DISABLED;
- case TransactionError::INVALID_PSBT:
- case TransactionError::PSBT_MISMATCH:
- return RPC_INVALID_PARAMETER;
- case TransactionError::SIGHASH_MISMATCH:
- return RPC_DESERIALIZATION_ERROR;
default: break;
}
return RPC_TRANSACTION_ERROR;
}
+UniValue JSONRPCPSBTError(PSBTError err)
+{
+ return JSONRPCError(RPCErrorFromPSBTError(err), PSBTErrorString(err).original);
+}
+
UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string)
{
if (err_string.length() > 0) {
diff --git a/src/rpc/util.h b/src/rpc/util.h
index 0e4dcc27b5..5522e346d2 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -37,6 +37,9 @@ enum class OutputType;
enum class TransactionError;
struct FlatSigningProvider;
struct bilingual_str;
+namespace common {
+enum class PSBTError;
+} // namespace common
static constexpr bool DEFAULT_RPC_DOC_CHECK{
#ifdef RPC_DOC_CHECK
@@ -128,6 +131,7 @@ int ParseSighashString(const UniValue& sighash);
unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target);
RPCErrorCode RPCErrorFromTransactionError(TransactionError terr);
+UniValue JSONRPCPSBTError(common::PSBTError err);
UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string = "");
//! Parse a JSON range specified as int64, or [int64, int64]
diff --git a/src/test/fuzz/kitchen_sink.cpp b/src/test/fuzz/kitchen_sink.cpp
index 82f3a306c5..42a6b316c3 100644
--- a/src/test/fuzz/kitchen_sink.cpp
+++ b/src/test/fuzz/kitchen_sink.cpp
@@ -18,15 +18,10 @@
namespace {
constexpr TransactionError ALL_TRANSACTION_ERROR[] = {
- TransactionError::OK,
TransactionError::MISSING_INPUTS,
TransactionError::ALREADY_IN_CHAIN,
- TransactionError::P2P_DISABLED,
TransactionError::MEMPOOL_REJECTED,
TransactionError::MEMPOOL_ERROR,
- TransactionError::INVALID_PSBT,
- TransactionError::PSBT_MISMATCH,
- TransactionError::SIGHASH_MISMATCH,
TransactionError::MAX_FEE_EXCEEDED,
};
}; // namespace
diff --git a/src/util/error.cpp b/src/util/error.cpp
index 309877d067..9d506ee25c 100644
--- a/src/util/error.cpp
+++ b/src/util/error.cpp
@@ -4,12 +4,33 @@
#include <util/error.h>
+#include <common/types.h>
#include <tinyformat.h>
#include <util/translation.h>
#include <cassert>
#include <string>
+using common::PSBTError;
+
+bilingual_str PSBTErrorString(PSBTError err)
+{
+ switch (err) {
+ case PSBTError::MISSING_INPUTS:
+ return Untranslated("Inputs missing or spent");
+ case PSBTError::SIGHASH_MISMATCH:
+ return Untranslated("Specified sighash value does not match value stored in PSBT");
+ case PSBTError::EXTERNAL_SIGNER_NOT_FOUND:
+ return Untranslated("External signer not found");
+ case PSBTError::EXTERNAL_SIGNER_FAILED:
+ return Untranslated("External signer failed to sign");
+ case PSBTError::UNSUPPORTED:
+ return Untranslated("Signer does not support PSBT");
+ // no default case, so the compiler can warn about missing cases
+ }
+ assert(false);
+}
+
bilingual_str TransactionErrorString(const TransactionError err)
{
switch (err) {
@@ -19,26 +40,14 @@ bilingual_str TransactionErrorString(const TransactionError err)
return Untranslated("Inputs missing or spent");
case TransactionError::ALREADY_IN_CHAIN:
return Untranslated("Transaction already in block chain");
- case TransactionError::P2P_DISABLED:
- return Untranslated("Peer-to-peer functionality missing or disabled");
case TransactionError::MEMPOOL_REJECTED:
return Untranslated("Transaction rejected by mempool");
case TransactionError::MEMPOOL_ERROR:
return Untranslated("Mempool internal error");
- case TransactionError::INVALID_PSBT:
- return Untranslated("PSBT is not well-formed");
- case TransactionError::PSBT_MISMATCH:
- return Untranslated("PSBTs not compatible (different transactions)");
- case TransactionError::SIGHASH_MISMATCH:
- return Untranslated("Specified sighash value does not match value stored in PSBT");
case TransactionError::MAX_FEE_EXCEEDED:
return Untranslated("Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)");
case TransactionError::MAX_BURN_EXCEEDED:
return Untranslated("Unspendable output exceeds maximum configured by user (maxburnamount)");
- case TransactionError::EXTERNAL_SIGNER_NOT_FOUND:
- return Untranslated("External signer not found");
- case TransactionError::EXTERNAL_SIGNER_FAILED:
- return Untranslated("External signer failed to sign");
case TransactionError::INVALID_PACKAGE:
return Untranslated("Transaction rejected due to invalid package");
// no default case, so the compiler can warn about missing cases
diff --git a/src/util/error.h b/src/util/error.h
index ee2d8dbf19..5ef833b9ee 100644
--- a/src/util/error.h
+++ b/src/util/error.h
@@ -19,6 +19,11 @@
#include <string>
struct bilingual_str;
+namespace common {
+enum class PSBTError;
+} // namespace common
+
+bilingual_str PSBTErrorString(common::PSBTError err);
bilingual_str TransactionErrorString(const TransactionError error);
diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp
index b5703fa54a..f98ecc3f3c 100644
--- a/src/wallet/external_signer_scriptpubkeyman.cpp
+++ b/src/wallet/external_signer_scriptpubkeyman.cpp
@@ -17,6 +17,8 @@
#include <utility>
#include <vector>
+using common::PSBTError;
+
namespace wallet {
bool ExternalSignerScriptPubKeyMan::SetupDescriptor(WalletBatch& batch, std::unique_ptr<Descriptor> desc)
{
@@ -76,7 +78,7 @@ util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestin
}
// If sign is true, transaction must previously have been filled
-TransactionError ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
+std::optional<PSBTError> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
{
if (!sign) {
return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, sighash_type, false, bip32derivs, n_signed, finalize);
@@ -88,14 +90,14 @@ TransactionError ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransact
// TODO: for multisig wallets, we should only care if all _our_ inputs are signed
complete &= PSBTInputSigned(input);
}
- if (complete) return TransactionError::OK;
+ if (complete) return {};
std::string strFailReason;
if(!GetExternalSigner().SignTransaction(psbt, strFailReason)) {
tfm::format(std::cerr, "Failed to sign: %s\n", strFailReason);
- return TransactionError::EXTERNAL_SIGNER_FAILED;
+ return PSBTError::EXTERNAL_SIGNER_FAILED;
}
if (finalize) FinalizePSBT(psbt); // This won't work in a multisig setup
- return TransactionError::OK;
+ return {};
}
} // namespace wallet
diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h
index 44286456b6..10d67d2ab4 100644
--- a/src/wallet/external_signer_scriptpubkeyman.h
+++ b/src/wallet/external_signer_scriptpubkeyman.h
@@ -35,7 +35,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
*/
util::Result<void> DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const;
- TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
+ std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
};
} // namespace wallet
#endif // BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index 6a8453965b..8966fde1b7 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -343,8 +343,8 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
// so external signers are not asked to sign more than once.
bool complete;
wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */);
- const TransactionError err = wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true /* sign */, false /* bip32derivs */);
- if (err != TransactionError::OK) return false;
+ auto err{wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true /* sign */, false /* bip32derivs */)};
+ if (err) return false;
complete = FinalizeAndExtractPSBT(psbtx, mtx);
return complete;
} else {
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 0c1cae7253..4421f0eeae 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -34,6 +34,7 @@
#include <utility>
#include <vector>
+using common::PSBTError;
using interfaces::Chain;
using interfaces::FoundBlock;
using interfaces::Handler;
@@ -389,7 +390,7 @@ public:
}
return {};
}
- TransactionError fillPSBT(int sighash_type,
+ std::optional<PSBTError> fillPSBT(int sighash_type,
bool sign,
bool bip32derivs,
size_t* n_signed,
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 1a364a75ed..12c59acadf 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -97,9 +97,9 @@ static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const
// so external signers are not asked to sign more than once.
bool complete;
pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true);
- const TransactionError err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/false)};
- if (err != TransactionError::OK) {
- throw JSONRPCTransactionError(err);
+ const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/false)};
+ if (err) {
+ throw JSONRPCPSBTError(*err);
}
CMutableTransaction mtx;
@@ -1153,8 +1153,8 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
} else {
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
- const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true);
- CHECK_NONFATAL(err == TransactionError::OK);
+ const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true)};
+ CHECK_NONFATAL(!err);
CHECK_NONFATAL(!complete);
DataStream ssTx{};
ssTx << psbtx;
@@ -1602,9 +1602,9 @@ RPCHelpMan walletprocesspsbt()
if (sign) EnsureWalletIsUnlocked(*pwallet);
- const TransactionError err{wallet.FillPSBT(psbtx, complete, nHashType, sign, bip32derivs, nullptr, finalize)};
- if (err != TransactionError::OK) {
- throw JSONRPCTransactionError(err);
+ const auto err{wallet.FillPSBT(psbtx, complete, nHashType, sign, bip32derivs, nullptr, finalize)};
+ if (err) {
+ throw JSONRPCPSBTError(*err);
}
UniValue result(UniValue::VOBJ);
@@ -1736,9 +1736,9 @@ RPCHelpMan walletcreatefundedpsbt()
// Fill transaction with out data but don't sign
bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool();
bool complete = true;
- const TransactionError err{wallet.FillPSBT(psbtx, complete, 1, /*sign=*/false, /*bip32derivs=*/bip32derivs)};
- if (err != TransactionError::OK) {
- throw JSONRPCTransactionError(err);
+ const auto err{wallet.FillPSBT(psbtx, complete, 1, /*sign=*/false, /*bip32derivs=*/bip32derivs)};
+ if (err) {
+ throw JSONRPCPSBTError(*err);
}
// Serialize the PSBT
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index b42275fe4b..69f83ae26c 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -20,6 +20,8 @@
#include <optional>
+using common::PSBTError;
+
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;
@@ -627,7 +629,7 @@ SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, con
return SigningResult::SIGNING_FAILED;
}
-TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
+std::optional<PSBTError> LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
{
if (n_signed) {
*n_signed = 0;
@@ -642,13 +644,13 @@ TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psb
// Get the Sighash type
if (sign && input.sighash_type != std::nullopt && *input.sighash_type != sighash_type) {
- return TransactionError::SIGHASH_MISMATCH;
+ return PSBTError::SIGHASH_MISMATCH;
}
// Check non_witness_utxo has specified prevout
if (input.non_witness_utxo) {
if (txin.prevout.n >= input.non_witness_utxo->vout.size()) {
- return TransactionError::MISSING_INPUTS;
+ return PSBTError::MISSING_INPUTS;
}
} else if (input.witness_utxo.IsNull()) {
// There's no UTXO so we can just skip this now
@@ -670,7 +672,7 @@ TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psb
UpdatePSBTOutput(HidingSigningProvider(this, true, !bip32derivs), psbtx, i);
}
- return TransactionError::OK;
+ return {};
}
std::unique_ptr<CKeyMetadata> LegacyScriptPubKeyMan::GetMetadata(const CTxDestination& dest) const
@@ -2485,7 +2487,7 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message,
return SigningResult::OK;
}
-TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
+std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
{
if (n_signed) {
*n_signed = 0;
@@ -2500,7 +2502,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
// Get the Sighash type
if (sign && input.sighash_type != std::nullopt && *input.sighash_type != sighash_type) {
- return TransactionError::SIGHASH_MISMATCH;
+ return PSBTError::SIGHASH_MISMATCH;
}
// Get the scriptPubKey to know which SigningProvider to use
@@ -2509,7 +2511,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
script = input.witness_utxo.scriptPubKey;
} else if (input.non_witness_utxo) {
if (txin.prevout.n >= input.non_witness_utxo->vout.size()) {
- return TransactionError::MISSING_INPUTS;
+ return PSBTError::MISSING_INPUTS;
}
script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey;
} else {
@@ -2580,7 +2582,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
UpdatePSBTOutput(HidingSigningProvider(keys.get(), /*hide_secret=*/true, /*hide_origin=*/!bip32derivs), psbtx, i);
}
- return TransactionError::OK;
+ return {};
}
std::unique_ptr<CKeyMetadata> DescriptorScriptPubKeyMan::GetMetadata(const CTxDestination& dest) const
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index 5ca7d1ff52..470a73e6b3 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -7,6 +7,7 @@
#include <addresstype.h>
#include <common/signmessage.h>
+#include <common/types.h>
#include <logging.h>
#include <psbt.h>
#include <script/descriptor.h>
@@ -243,7 +244,7 @@ public:
/** Sign a message with the given script */
virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; };
/** Adds script and derivation path information to a PSBT, and optionally signs it. */
- virtual TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return TransactionError::INVALID_PSBT; }
+ virtual std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return common::PSBTError::UNSUPPORTED; }
virtual uint256 GetID() const { return uint256(); }
@@ -421,7 +422,7 @@ public:
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override;
SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
- TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
+ std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
uint256 GetID() const override;
@@ -651,7 +652,7 @@ public:
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override;
SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
- TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
+ std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
uint256 GetID() const override;
diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp
index 9f533bf6ed..e924f90630 100644
--- a/src/wallet/test/psbt_wallet_tests.cpp
+++ b/src/wallet/test/psbt_wallet_tests.cpp
@@ -60,7 +60,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
// Fill transaction with our data
bool complete = true;
- BOOST_REQUIRE_EQUAL(TransactionError::OK, m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false, true));
+ BOOST_REQUIRE(!m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false, true));
// Get the final tx
DataStream ssTx{};
@@ -73,7 +73,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
// Try to sign the mutated input
SignatureData sigdata;
- BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true, true) != TransactionError::OK);
+ BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true, true));
}
BOOST_AUTO_TEST_CASE(parse_hd_keypath)
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 0297009a4b..f0c320fcff 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -81,6 +81,7 @@
struct KeyOriginInfo;
+using common::PSBTError;
using interfaces::FoundBlock;
namespace wallet {
@@ -2167,7 +2168,7 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint,
return false;
}
-TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const
+std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const
{
if (n_signed) {
*n_signed = 0;
@@ -2200,9 +2201,9 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp
// Fill in information from ScriptPubKeyMans
for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) {
int n_signed_this_spkm = 0;
- TransactionError res = spk_man->FillPSBT(psbtx, txdata, sighash_type, sign, bip32derivs, &n_signed_this_spkm, finalize);
- if (res != TransactionError::OK) {
- return res;
+ const auto error{spk_man->FillPSBT(psbtx, txdata, sighash_type, sign, bip32derivs, &n_signed_this_spkm, finalize)};
+ if (error) {
+ return error;
}
if (n_signed) {
@@ -2218,7 +2219,7 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp
complete &= PSBTInputSigned(input);
}
- return TransactionError::OK;
+ return {};
}
SigningResult CWallet::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 6a998fa398..5bc888462f 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -58,7 +58,9 @@ class Coin;
class SigningProvider;
enum class MemPoolRemovalReason;
enum class SigningResult;
-enum class TransactionError;
+namespace common {
+enum class PSBTError;
+} // namespace common
namespace interfaces {
class Wallet;
}
@@ -659,7 +661,7 @@ public:
* @param[in] finalize whether to create the final scriptSig or scriptWitness if possible
* return error
*/
- TransactionError FillPSBT(PartiallySignedTransaction& psbtx,
+ std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbtx,
bool& complete,
int sighash_type = SIGHASH_DEFAULT,
bool sign = true,