diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index df1ced48a71..8ecb2a7429b 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -207,7 +207,7 @@ public: int& num_blocks) = 0; //! Fill PSBT. - virtual std::optional fillPSBT(int sighash_type, + virtual std::optional fillPSBT(std::optional sighash_type, bool sign, bool bip32derivs, size_t* n_signed, diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index 5a4b4442f3d..afe6a8999ac 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -59,7 +59,7 @@ 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; - const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)}; + const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)}; if (err) { showStatus(tr("Failed to load transaction: %1") .arg(QString::fromStdString(PSBTErrorString(*err).translated)), @@ -83,7 +83,7 @@ void PSBTOperationsDialog::signTransaction() WalletModel::UnlockContext ctx(m_wallet_model->requestUnlock()); - const auto 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(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)}; if (err) { showStatus(tr("Failed to sign transaction: %1") @@ -251,7 +251,7 @@ size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &p size_t n_signed; bool complete; - const auto 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(std::nullopt, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)}; if (err) { return 0; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 0ee1b359fa9..8c3797603e5 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -446,7 +446,7 @@ void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx) bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx, CMutableTransaction& mtx, bool& complete) { std::optional err; try { - err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete); + err = model->wallet().fillPSBT(std::nullopt, /*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; @@ -503,7 +503,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked) PartiallySignedTransaction psbtx(mtx); bool complete = false; // Fill without signing - const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)}; + const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)}; assert(!complete); assert(!err); @@ -519,7 +519,7 @@ 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. - const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)}; + const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)}; assert(!complete); assert(!err); send_failure = !signWithExternalSigner(psbtx, mtx, complete); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 0a01c0a45b1..eb68ee7142e 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -535,7 +535,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) // "Create Unsigned" clicked PartiallySignedTransaction psbtx(mtx); bool complete = false; - const auto err{wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)}; + const auto err{wallet().fillPSBT(std::nullopt, /*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/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index 32e9941453b..7268354f1a9 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -79,7 +79,7 @@ util::Result ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestin } // If sign is true, transaction must previously have been filled -std::optional ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const +std::optional ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional 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); diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 10d67d2ab47..b44452e495e 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 DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const; - std::optional 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 FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional 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 b875461c9f2..170941b1daf 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -343,8 +343,8 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) { // First fill transaction with our data without signing, // so external signers are not asked to sign more than once. bool complete; - wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */); - auto err{wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true /* sign */, false /* bip32derivs */)}; + wallet.FillPSBT(psbtx, complete, std::nullopt, false /* sign */, true /* bip32derivs */); + auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, true /* sign */, false /* bip32derivs */)}; if (err) return false; complete = FinalizeAndExtractPSBT(psbtx, mtx); return complete; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 21e8a0b3bd2..d477c34d69e 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -391,7 +391,7 @@ public: } return {}; } - std::optional fillPSBT(int sighash_type, + std::optional fillPSBT(std::optional 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 bea9b2eec18..7410ab1f8a9 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -103,8 +103,8 @@ static UniValue FinishTransaction(const std::shared_ptr pwallet, const // First fill transaction with our data without signing, // so external signers are not asked to sign more than once. bool complete; - pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true); - const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/false)}; + pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true); + const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)}; if (err) { throw JSONRPCPSBTError(*err); } @@ -1169,7 +1169,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) } else { PartiallySignedTransaction psbtx(mtx); bool complete = false; - const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true)}; + const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true)}; CHECK_NONFATAL(!err); CHECK_NONFATAL(!complete); DataStream ssTx{}; @@ -1767,7 +1767,7 @@ 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 auto err{wallet.FillPSBT(psbtx, complete, 1, /*sign=*/false, /*bip32derivs=*/bip32derivs)}; + const auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/bip32derivs)}; if (err) { throw JSONRPCPSBTError(*err); } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 9f72e91b85d..88efb0becea 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -638,11 +638,12 @@ SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, con return SigningResult::SIGNING_FAILED; } -std::optional LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const +std::optional LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const { if (n_signed) { *n_signed = 0; } + if (!sighash_type) sighash_type = SIGHASH_DEFAULT; for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { const CTxIn& txin = psbtx.tx->vin[i]; PSBTInput& input = psbtx.inputs.at(i); @@ -665,7 +666,7 @@ std::optional LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransact // There's no UTXO so we can just skip this now continue; } - PSBTError res = SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, &txdata, sighash_type, nullptr, finalize); + PSBTError res = SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, &txdata, *sighash_type, nullptr, finalize); if (res != PSBTError::OK && res != PSBTError::INCOMPLETE) { return res; } @@ -2548,11 +2549,12 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message, return SigningResult::OK; } -std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const +std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const { if (n_signed) { *n_signed = 0; } + if (!sighash_type) sighash_type = SIGHASH_DEFAULT; for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { const CTxIn& txin = psbtx.tx->vin[i]; PSBTInput& input = psbtx.inputs.at(i); @@ -2623,7 +2625,7 @@ std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran } } - PSBTError res = SignPSBTInput(HidingSigningProvider(keys.get(), /*hide_secret=*/!sign, /*hide_origin=*/!bip32derivs), psbtx, i, &txdata, sighash_type, nullptr, finalize); + PSBTError res = SignPSBTInput(HidingSigningProvider(keys.get(), /*hide_secret=*/!sign, /*hide_origin=*/!bip32derivs), psbtx, i, &txdata, *sighash_type, nullptr, finalize); if (res != PSBTError::OK && res != PSBTError::INCOMPLETE) { return res; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 0d19eb92229..a5d2d695b6a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -246,7 +246,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 std::optional 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 std::optional FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return common::PSBTError::UNSUPPORTED; } virtual uint256 GetID() const { return uint256(); } @@ -492,7 +492,7 @@ public: bool SignTransaction(CMutableTransaction& tx, const std::map& coins, int sighash, std::map& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; - std::optional 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 FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; uint256 GetID() const override; @@ -681,7 +681,7 @@ public: bool SignTransaction(CMutableTransaction& tx, const std::map& coins, int sighash, std::map& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; - std::optional 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 FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional sighash_type = std::nullopt, 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/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index 0c17a6bf7a9..18b3c829a46 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -188,7 +188,8 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm) } auto psbt{*opt_psbt}; const PrecomputedTransactionData txdata{PrecomputePSBTData(psbt)}; - const int sighash_type{fuzzed_data_provider.ConsumeIntegralInRange(0, 150)}; + std::optional sighash_type{fuzzed_data_provider.ConsumeIntegralInRange(0, 151)}; + if (sighash_type == 151) sighash_type = std::nullopt; auto sign = fuzzed_data_provider.ConsumeBool(); auto bip32derivs = fuzzed_data_provider.ConsumeBool(); auto finalize = fuzzed_data_provider.ConsumeBool(); diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 09057114a0b..d6cc0805100 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) // Fill transaction with our data bool complete = true; - BOOST_REQUIRE(!m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false, true)); + BOOST_REQUIRE(!m_wallet.FillPSBT(psbtx, complete, std::nullopt, false, true)); // Get the final tx DataStream ssTx{}; @@ -77,7 +77,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)); + BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, std::nullopt, true, true)); } BOOST_AUTO_TEST_CASE(parse_hd_keypath) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ea59f1a9d57..74987cb29dc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2197,11 +2197,12 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const +std::optional CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, std::optional sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const { if (n_signed) { *n_signed = 0; } + if (!sighash_type) sighash_type = SIGHASH_DEFAULT; LOCK(cs_wallet); // Get all of the previous transactions for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { @@ -2240,7 +2241,7 @@ std::optional CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo } } - RemoveUnnecessaryTransactions(psbtx, sighash_type); + RemoveUnnecessaryTransactions(psbtx, *sighash_type); // Complete if every input is now signed complete = true; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f8b914dd8ff..f1dce204f96 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -663,7 +663,7 @@ public: */ std::optional FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, - int sighash_type = SIGHASH_DEFAULT, + std::optional sighash_type = std::nullopt, bool sign = true, bool bip32derivs = true, size_t* n_signed = nullptr,