diff options
author | Ryan Ofsky <ryan@ofsky.org> | 2023-12-13 11:43:16 -0500 |
---|---|---|
committer | Ryan Ofsky <ryan@ofsky.org> | 2024-05-16 10:16:08 -0500 |
commit | 02e62c6c9af4beabaeea58fb1ea3ad0dc5094678 (patch) | |
tree | 3ddfb29afa69039b8d2444d3d2cd2e069e823aee /src | |
parent | 0d44c44ae33434f366229c612d6edeedf7658963 (diff) |
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/Makefile.am | 1 | ||||
-rw-r--r-- | src/common/types.h | 26 | ||||
-rw-r--r-- | src/interfaces/wallet.h | 6 | ||||
-rw-r--r-- | src/node/types.h | 6 | ||||
-rw-r--r-- | src/psbt.cpp | 6 | ||||
-rw-r--r-- | src/psbt.h | 4 | ||||
-rw-r--r-- | src/qt/psbtoperationsdialog.cpp | 17 | ||||
-rw-r--r-- | src/qt/sendcoinsdialog.cpp | 17 | ||||
-rw-r--r-- | src/qt/walletmodel.cpp | 4 | ||||
-rw-r--r-- | src/rpc/rawtransaction.cpp | 5 | ||||
-rw-r--r-- | src/rpc/util.cpp | 27 | ||||
-rw-r--r-- | src/rpc/util.h | 4 | ||||
-rw-r--r-- | src/test/fuzz/kitchen_sink.cpp | 5 | ||||
-rw-r--r-- | src/util/error.cpp | 33 | ||||
-rw-r--r-- | src/util/error.h | 5 | ||||
-rw-r--r-- | src/wallet/external_signer_scriptpubkeyman.cpp | 10 | ||||
-rw-r--r-- | src/wallet/external_signer_scriptpubkeyman.h | 2 | ||||
-rw-r--r-- | src/wallet/feebumper.cpp | 4 | ||||
-rw-r--r-- | src/wallet/interfaces.cpp | 3 | ||||
-rw-r--r-- | src/wallet/rpc/spend.cpp | 22 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.cpp | 18 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.h | 7 | ||||
-rw-r--r-- | src/wallet/test/psbt_wallet_tests.cpp | 4 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 11 | ||||
-rw-r--r-- | src/wallet/wallet.h | 6 |
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, |