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.

Author: ryanofsky

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 \

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

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 @@ class Wallet
         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,

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,
 };
 

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) {

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);

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;

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;

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;
         }

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{};