diff options
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, |