diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 9fc8ab0ef51..89477aa16d1 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -204,7 +204,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 454d9287aea..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_DEFAULT, /*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 b8a9854f0e0..c2ebb875de6 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -450,7 +450,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; @@ -507,7 +507,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); @@ -523,7 +523,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 07d5b9413c3..4483144582f 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -519,7 +519,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..66c2dd9d170 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 = std::nullopt, 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 0a58285a969..f4c8be938e2 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -338,8 +338,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, /*sign=*/false, /*bip32derivs=*/true); + auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)}; if (err) return false; complete = FinalizeAndExtractPSBT(psbtx, mtx); return complete; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 5308b4418d7..0f8925f77ee 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -383,7 +383,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 a59d274b176..4626a02da32 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -100,8 +100,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); } @@ -1175,7 +1175,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{}; @@ -1777,7 +1777,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 c9d52c18dff..0d20a73871b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1311,11 +1311,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); @@ -1386,7 +1387,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 d8aac3ed02f..13b8233879c 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -153,7 +153,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(); } @@ -382,7 +382,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 63768c89afc..6a1c51290b3 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -192,7 +192,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 c44ea496339..63153fb55db 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -65,7 +65,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{}; @@ -78,7 +78,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 3c5df177679..215d7b505cd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2101,11 +2101,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) { @@ -2144,7 +2145,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 e32b8c7272b..c6b56bd9ef0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -659,7 +659,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,