diff --git a/src/common/types.h b/src/common/types.h index 7b5da0fc0b0..c366a847562 100644 --- a/src/common/types.h +++ b/src/common/types.h @@ -13,6 +13,8 @@ #ifndef BITCOIN_COMMON_TYPES_H #define BITCOIN_COMMON_TYPES_H +#include + namespace common { enum class PSBTError { MISSING_INPUTS, @@ -23,6 +25,31 @@ enum class PSBTError { INCOMPLETE, OK, }; +/** + * Instructions for how a PSBT should be signed or filled with information. + */ +struct PSBTFillOptions { + /** + * Whether to sign or not. + */ + bool sign{true}; + + /** + * The sighash type to use when signing (if PSBT does not specify). + */ + std::optional sighash_type{std::nullopt}; + + /** + * Whether to create the final scriptSig or scriptWitness if possible. + */ + bool finalize{true}; + + /** + * Whether to fill in bip32 derivation information if available. + */ + bool bip32_derivs{true}; +}; + } // namespace common #endif // BITCOIN_COMMON_TYPES_H diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 9154ac05dd0..ab142080fc6 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -202,9 +203,7 @@ public: int& num_blocks) = 0; //! Fill PSBT. - virtual std::optional fillPSBT(std::optional sighash_type, - bool sign, - bool bip32derivs, + virtual std::optional fillPSBT(const common::PSBTFillOptions& options, size_t* n_signed, PartiallySignedTransaction& psbtx, bool& complete) = 0; diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index 9d79b42f29a..62382e5602b 100644 --- a/src/node/psbt.cpp +++ b/src/node/psbt.cpp @@ -64,7 +64,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) // Figure out what is missing SignatureData outdata; - bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, std::nullopt, &outdata) == PSBTError::OK; + bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, /*options=*/{}, &outdata) == PSBTError::OK; // Things are missing if (!complete) { @@ -123,7 +123,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) PSBTInput& input = psbtx.inputs[i]; Coin newcoin; - if (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, nullptr, std::nullopt) != PSBTError::OK || !psbtx.GetInputUTXO(newcoin.out, i)) { + if (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, nullptr, /*options=*/{}) != PSBTError::OK || !psbtx.GetInputUTXO(newcoin.out, i)) { success = false; break; } else { diff --git a/src/psbt.cpp b/src/psbt.cpp index 58979152e65..8c39a6ce975 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -399,7 +399,7 @@ PrecomputedTransactionData PrecomputePSBTData(const PartiallySignedTransaction& return txdata; } -PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, std::optional sighash, SignatureData* out_sigdata, bool finalize) +PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, const common::PSBTFillOptions& options, SignatureData* out_sigdata) { PSBTInput& input = psbt.inputs.at(index); const CMutableTransaction& tx = *psbt.tx; @@ -442,8 +442,8 @@ PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransact // If only the parameter is provided, use it and add it to the PSBT if it is other than SIGHASH_DEFAULT // for all input types, and not SIGHASH_ALL for non-taproot input types. // If neither are provided, use SIGHASH_DEFAULT if it is taproot, and SIGHASH_ALL for everything else. - if (!sighash) sighash = utxo.scriptPubKey.IsPayToTaproot() ? SIGHASH_DEFAULT : SIGHASH_ALL; - Assert(sighash.has_value()); + int sighash{options.sighash_type.value_or(utxo.scriptPubKey.IsPayToTaproot() ? SIGHASH_DEFAULT : SIGHASH_ALL)}; + // For user safety, the desired sighash must be provided if the PSBT wants something other than the default set in the previous line. if (input.sighash_type && input.sighash_type != sighash) { return PSBTError::SIGHASH_MISMATCH; @@ -465,14 +465,14 @@ PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransact if (sig.size() != 64) return PSBTError::SIGHASH_MISMATCH; } } else { - if (!input.m_tap_key_sig.empty() && (input.m_tap_key_sig.size() != 65 || input.m_tap_key_sig.back() != *sighash)) { + if (!input.m_tap_key_sig.empty() && (input.m_tap_key_sig.size() != 65 || input.m_tap_key_sig.back() != sighash)) { return PSBTError::SIGHASH_MISMATCH; } for (const auto& [_, sig] : input.m_tap_script_sigs) { - if (sig.size() != 65 || sig.back() != *sighash) return PSBTError::SIGHASH_MISMATCH; + if (sig.size() != 65 || sig.back() != sighash) return PSBTError::SIGHASH_MISMATCH; } for (const auto& [_, sig] : input.partial_sigs) { - if (sig.second.back() != *sighash) return PSBTError::SIGHASH_MISMATCH; + if (sig.second.back() != sighash) return PSBTError::SIGHASH_MISMATCH; } } @@ -481,14 +481,14 @@ PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransact if (txdata == nullptr) { sig_complete = ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, utxo.scriptPubKey, sigdata); } else { - MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, *sighash); + MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, sighash); sig_complete = ProduceSignature(provider, creator, utxo.scriptPubKey, sigdata); } // Verify that a witness signature was produced in case one was required. if (require_witness_sig && !sigdata.witness) return PSBTError::INCOMPLETE; // If we are not finalizing, set sigdata.complete to false to not set the scriptWitness - if (!finalize && sigdata.complete) sigdata.complete = false; + if (!options.finalize && sigdata.complete) sigdata.complete = false; input.FromSignatureData(sigdata); @@ -558,7 +558,7 @@ bool FinalizePSBT(PartiallySignedTransaction& psbtx) const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { PSBTInput& input = psbtx.inputs.at(i); - complete &= (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, input.sighash_type, nullptr, true) == PSBTError::OK); + complete &= (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, {.sighash_type = input.sighash_type, .finalize = true}, nullptr) == PSBTError::OK); } return complete; diff --git a/src/psbt.h b/src/psbt.h index 7fe9967a0c6..6348bd15ea5 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1427,7 +1427,7 @@ bool PSBTInputSignedAndVerified(const PartiallySignedTransaction& psbt, unsigned * txdata should be the output of PrecomputePSBTData (which can be shared across * multiple SignPSBTInput calls). If it is nullptr, a dummy signature will be created. **/ -[[nodiscard]] PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, std::optional sighash = std::nullopt, SignatureData* out_sigdata = nullptr, bool finalize = true); +[[nodiscard]] PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, const common::PSBTFillOptions& options, SignatureData* out_sigdata = nullptr); /** Reduces the size of the PSBT by dropping unnecessary `non_witness_utxos` (i.e. complete previous transactions) from a psbt when all inputs are segwit v1. */ void RemoveUnnecessaryTransactions(PartiallySignedTransaction& psbtx); diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index f5cb077386e..64ce6d2821b 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(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)}; + const auto err{m_wallet_model->wallet().fillPSBT({.sign = false, .bip32_derivs= 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(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)}; + const auto err{m_wallet_model->wallet().fillPSBT({.sign = true, .bip32_derivs = 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(std::nullopt, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)}; + const auto err{m_wallet_model->wallet().fillPSBT({.sign = false, .bip32_derivs = false}, &n_signed, m_transaction_data, complete)}; if (err) { return 0; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 493b4fb7b3e..74b9341779b 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -447,7 +447,7 @@ void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx) bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx, CMutableTransaction& mtx, bool& complete) { std::optional err; try { - err = model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete); + err = model->wallet().fillPSBT({.sign = true, .bip32_derivs = true}, /*n_signed=*/nullptr, psbtx, complete); } catch (const std::runtime_error& e) { QMessageBox::critical(nullptr, tr("Sign failed"), e.what()); return false; @@ -504,7 +504,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked) PartiallySignedTransaction psbtx(mtx); bool complete = false; // Fill without signing - const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)}; + const auto err{model->wallet().fillPSBT({.sign = false, .bip32_derivs = true}, /*n_signed=*/nullptr, psbtx, complete)}; assert(!complete); assert(!err); @@ -520,7 +520,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(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)}; + const auto err{model->wallet().fillPSBT({.sign = false, .bip32_derivs = 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 7a35f8e4d35..67a420f5ce0 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -517,7 +517,7 @@ bool WalletModel::bumpFee(Txid hash, Txid& new_hash) // "Create Unsigned" clicked PartiallySignedTransaction psbtx(mtx); bool complete = false; - const auto err{wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)}; + const auto err{wallet().fillPSBT({.sign = false, .bip32_derivs = 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 a0f93f0c79c..19305ba46a8 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -198,7 +198,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std // We only actually care about those if our signing provider doesn't hide private // information, as is the case with `descriptorprocesspsbt` // Only error for mismatching sighash types as it is critical that the sighash to sign with matches the PSBT's - if (SignPSBTInput(provider, psbtx, /*index=*/i, &txdata, sighash_type, /*out_sigdata=*/nullptr, finalize) == common::PSBTError::SIGHASH_MISMATCH) { + if (SignPSBTInput(provider, psbtx, /*index=*/i, &txdata, {.sighash_type = sighash_type, .finalize = finalize}, /*out_sigdata=*/nullptr) == common::PSBTError::SIGHASH_MISMATCH) { throw JSONRPCPSBTError(common::PSBTError::SIGHASH_MISMATCH); } } diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index 274da09ec6e..f9fa46b9112 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -79,10 +79,10 @@ 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, std::optional sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const +std::optional ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, const common::PSBTFillOptions& options, int* n_signed) const { - if (!sign) { - return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, sighash_type, false, bip32derivs, n_signed, finalize); + if (!options.sign) { + return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, options, n_signed); } // Already complete if every input is now signed @@ -103,7 +103,7 @@ std::optional ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigned LogWarning("Failed to sign: %s\n", failure_reason); return PSBTError::EXTERNAL_SIGNER_FAILED; } - if (finalize) FinalizePSBT(psbt); + if (options.finalize) FinalizePSBT(psbt); // This won't work in a multisig setup return {}; } } // namespace wallet diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 0ccc243c256..634b3bdbaa1 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -36,7 +36,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan */ util::Result DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const; - 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; + std::optional FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, const common::PSBTFillOptions& options, int* n_signed = nullptr) const override; }; } // namespace wallet #endif // BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 7f84c6fa668..85e5f778b1f 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -337,8 +337,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, std::nullopt, /*sign=*/false, /*bip32derivs=*/true); - auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)}; + wallet.FillPSBT(psbtx, {.sign = false, .bip32_derivs = true}, complete); + auto err{wallet.FillPSBT(psbtx, {.sign = true, .bip32_derivs = false}, complete)}; if (err) return false; complete = FinalizeAndExtractPSBT(psbtx, mtx); return complete; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index ae6b9ff1911..4eee155ce21 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -364,14 +364,12 @@ public: } return {}; } - std::optional fillPSBT(std::optional sighash_type, - bool sign, - bool bip32derivs, + std::optional fillPSBT(const common::PSBTFillOptions& options, size_t* n_signed, PartiallySignedTransaction& psbtx, bool& complete) override { - return m_wallet->FillPSBT(psbtx, complete, sighash_type, sign, bip32derivs, n_signed); + return m_wallet->FillPSBT(psbtx, options, complete, n_signed); } WalletBalances getBalances() override { diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 5f9de93a9ff..af121c126c0 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -114,8 +114,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, std::nullopt, /*sign=*/false, /*bip32derivs=*/true); - const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)}; + pwallet->FillPSBT(psbtx, {.sign = false, .bip32_derivs = true}, complete); + const auto err{pwallet->FillPSBT(psbtx, {.sign = true, .bip32_derivs = false}, complete)}; if (err) { throw JSONRPCPSBTError(*err); } @@ -1138,7 +1138,7 @@ static RPCMethod bumpfee_helper(std::string method_name) } else { PartiallySignedTransaction psbtx(mtx); bool complete = false; - const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true)}; + const auto err{pwallet->FillPSBT(psbtx, {.sign = false, .bip32_derivs = true}, complete)}; CHECK_NONFATAL(!err); CHECK_NONFATAL(!complete); DataStream ssTx{}; @@ -1635,7 +1635,7 @@ RPCMethod walletprocesspsbt() if (sign) EnsureWalletIsUnlocked(*pwallet); - const auto err{wallet.FillPSBT(psbtx, complete, nHashType, sign, bip32derivs, nullptr, finalize)}; + const auto err{wallet.FillPSBT(psbtx, {.sign = sign, .sighash_type = nHashType, .finalize = finalize, .bip32_derivs = bip32derivs}, complete)}; if (err) { throw JSONRPCPSBTError(*err); } @@ -1777,7 +1777,7 @@ RPCMethod 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, std::nullopt, /*sign=*/false, /*bip32derivs=*/bip32derivs)}; + const auto err{wallet.FillPSBT(psbtx, {.sign = false, .bip32_derivs = bip32derivs}, complete)}; if (err) { throw JSONRPCPSBTError(*err); } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 8a1c0a45a56..92152e7c751 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1306,7 +1306,7 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message, return SigningResult::OK; } -std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const +std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, const common::PSBTFillOptions& options, int* n_signed) const { if (n_signed) { *n_signed = 0; @@ -1334,7 +1334,7 @@ std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran } std::unique_ptr keys = std::make_unique(); - std::unique_ptr script_keys = GetSigningProvider(script, /*include_private=*/sign); + std::unique_ptr script_keys = GetSigningProvider(script, /*include_private=*/options.sign); if (script_keys) { keys->Merge(std::move(*script_keys)); } else { @@ -1376,13 +1376,13 @@ 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=*/!options.sign, /*hide_origin=*/!options.bip32_derivs), psbtx, i, &txdata, options, nullptr); if (res != PSBTError::OK && res != PSBTError::INCOMPLETE) { return res; } bool signed_one = PSBTInputSigned(input); - if (n_signed && (signed_one || !sign)) { + if (n_signed && (signed_one || !options.sign)) { // If sign is false, we assume that we _could_ sign if we get here. This // will never have false negatives; it is hard to tell under what i // circumstances it could have false positives. @@ -1396,7 +1396,7 @@ std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran if (!keys) { continue; } - UpdatePSBTOutput(HidingSigningProvider(keys.get(), /*hide_secret=*/true, /*hide_origin=*/!bip32derivs), psbtx, i); + UpdatePSBTOutput(HidingSigningProvider(keys.get(), /*hide_secret=*/true, /*hide_origin=*/!options.bip32_derivs), psbtx, i); } return {}; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 4ac09add62b..1bc5249ca81 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -137,7 +137,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, 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 std::optional FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, const common::PSBTFillOptions& options, int* n_signed = nullptr) const { return common::PSBTError::UNSUPPORTED; } virtual uint256 GetID() const { return uint256(); } @@ -378,7 +378,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, std::optional sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; + std::optional FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, const common::PSBTFillOptions& options, int* n_signed = nullptr) const override; uint256 GetID() const override; diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index fa8bc250d07..100db1a6d25 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -185,12 +185,14 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm) } auto psbt{*opt_psbt}; const PrecomputedTransactionData txdata{PrecomputePSBTData(psbt)}; - 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(); - (void)spk_manager->FillPSBT(psbt, txdata, sighash_type, sign, bip32derivs, nullptr, finalize); + common::PSBTFillOptions options{ + .sign = fuzzed_data_provider.ConsumeBool(), + .sighash_type = fuzzed_data_provider.ConsumeIntegralInRange(0, 151), + .finalize = fuzzed_data_provider.ConsumeBool(), + .bip32_derivs = fuzzed_data_provider.ConsumeBool() + }; + if (options.sighash_type == 151) options.sighash_type = std::nullopt; + (void)spk_manager->FillPSBT(psbt, txdata, options); } ); } diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 91b69b9d624..7e2ee6ce74f 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, std::nullopt, false, true)); + BOOST_REQUIRE(!m_wallet.FillPSBT(psbtx, {.sign = false, .bip32_derivs = true}, complete)); // 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, std::nullopt, true, true)); + BOOST_CHECK(m_wallet.FillPSBT(psbtx, {.sign = true, .bip32_derivs = true}, complete)); } BOOST_AUTO_TEST_CASE(parse_hd_keypath) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d25c39128b2..823e1b48211 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2220,7 +2220,7 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, std::optional sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const +std::optional CWallet::FillPSBT(PartiallySignedTransaction& psbtx, const common::PSBTFillOptions& options, bool& complete, size_t* n_signed) const { if (n_signed) { *n_signed = 0; @@ -2253,7 +2253,7 @@ std::optional CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo // Fill in information from ScriptPubKeyMans for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) { int n_signed_this_spkm = 0; - const auto error{spk_man->FillPSBT(psbtx, txdata, sighash_type, sign, bip32derivs, &n_signed_this_spkm, finalize)}; + const auto error{spk_man->FillPSBT(psbtx, txdata, options, &n_signed_this_spkm)}; if (error) { return error; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 9cd78258454..cbf3efc0376 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -670,26 +670,20 @@ public: /** * Fills out a PSBT with information from the wallet. Fills in UTXOs if we have - * them. Tries to sign if sign=true. Sets `complete` if the PSBT is now complete - * (i.e. has all required signatures or signature-parts, and is ready to - * finalize.) Sets `error` and returns false if something goes wrong. + * them. Tries to sign if options.sign=true. + * Sets `complete` if the PSBT is now complete (i.e. has all required + * signatures or signature-parts, and is ready to finalize.) * * @param[in] psbtx PartiallySignedTransaction to fill in + * @param[in] options options for filling or signing * @param[out] complete indicates whether the PSBT is now complete - * @param[in] sighash_type the sighash type to use when signing (if PSBT does not specify) - * @param[in] sign whether to sign or not - * @param[in] bip32derivs whether to fill in bip32 derivation information if available * @param[out] n_signed the number of inputs signed by this wallet - * @param[in] finalize whether to create the final scriptSig or scriptWitness if possible - * return error + * @returns an error if something goes wrong */ std::optional FillPSBT(PartiallySignedTransaction& psbtx, + const common::PSBTFillOptions& options, bool& complete, - std::optional sighash_type = std::nullopt, - bool sign = true, - bool bip32derivs = true, - size_t* n_signed = nullptr, - bool finalize = true) const; + size_t* n_signed = nullptr) const; /** * Submit the transaction to the node's mempool and then relay to peers.