diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index 079c7e2d4e2..3933202e0c8 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, 1, &outdata) == PSBTError::OK; + bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, std::nullopt, &outdata) == PSBTError::OK; // Things are missing if (!complete) { @@ -124,7 +124,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) PSBTInput& input = psbtx.inputs[i]; Coin newcoin; - if (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, nullptr, 1) != PSBTError::OK || !psbtx.GetInputUTXO(newcoin.out, i)) { + if (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, nullptr, std::nullopt) != PSBTError::OK || !psbtx.GetInputUTXO(newcoin.out, i)) { success = false; break; } else { diff --git a/src/psbt.cpp b/src/psbt.cpp index c89066c1497..c4d3e0e227f 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -375,7 +375,7 @@ PrecomputedTransactionData PrecomputePSBTData(const PartiallySignedTransaction& return txdata; } -PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, int sighash, SignatureData* out_sigdata, bool finalize) +PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, std::optional sighash, SignatureData* out_sigdata, bool finalize) { PSBTInput& input = psbt.inputs.at(index); const CMutableTransaction& tx = *psbt.tx; @@ -413,12 +413,24 @@ PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransact return PSBTError::MISSING_INPUTS; } + // Get the sighash type + // If both the field and the parameter are provided, they must match + // 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()); + // 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; + } + sigdata.witness = false; bool sig_complete; 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. @@ -448,10 +460,11 @@ PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransact return sig_complete ? PSBTError::OK : PSBTError::INCOMPLETE; } -void RemoveUnnecessaryTransactions(PartiallySignedTransaction& psbtx, const int& sighash_type) +void RemoveUnnecessaryTransactions(PartiallySignedTransaction& psbtx, std::optional sighash_type) { + if (!sighash_type) sighash_type = SIGHASH_DEFAULT; // Only drop non_witness_utxos if sighash_type != SIGHASH_ANYONECANPAY - if ((sighash_type & 0x80) != SIGHASH_ANYONECANPAY) { + if ((*sighash_type & 0x80) != SIGHASH_ANYONECANPAY) { // Figure out if any non_witness_utxos should be dropped std::vector to_drop; for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { @@ -489,7 +502,7 @@ bool FinalizePSBT(PartiallySignedTransaction& psbtx) bool complete = true; const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { - complete &= (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, SIGHASH_ALL, nullptr, true) == PSBTError::OK); + complete &= (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, std::nullopt, nullptr, true) == PSBTError::OK); } return complete; diff --git a/src/psbt.h b/src/psbt.h index a443bce7d54..69bfd646cc1 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1404,10 +1404,10 @@ 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, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool finalize = true); +[[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); /** 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, const int& sighash_type); +void RemoveUnnecessaryTransactions(PartiallySignedTransaction& psbtx, std::optional sighash_type); /** Counts the unsigned inputs of a PSBT. */ size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction& psbt); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 3599d493d39..59b0aea7edd 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -163,7 +163,7 @@ static std::vector CreateTxDoc() // Update PSBT with information from the mempool, the UTXO set, the txindex, and the provided descriptors. // Optionally, sign the inputs that we can using information from the descriptors. -PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std::any& context, const HidingSigningProvider& provider, int sighash_type, bool finalize) +PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std::any& context, const HidingSigningProvider& provider, std::optional sighash_type, bool finalize) { // Unserialize the transactions PartiallySignedTransaction psbtx; @@ -244,7 +244,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std UpdatePSBTOutput(provider, psbtx, i); } - RemoveUnnecessaryTransactions(psbtx, /*sighash_type=*/1); + RemoveUnnecessaryTransactions(psbtx, /*sighash_type=*/std::nullopt); return psbtx; } @@ -1796,7 +1796,7 @@ static RPCHelpMan utxoupdatepsbt() request.params[0].get_str(), request.context, HidingSigningProvider(&provider, /*hide_secret=*/true, /*hide_origin=*/false), - /*sighash_type=*/SIGHASH_ALL, + /*sighash_type=*/std::nullopt, /*finalize=*/false); DataStream ssTx{}; @@ -2063,7 +2063,7 @@ RPCHelpMan descriptorprocesspsbt() EvalDescriptorStringOrObject(descs[i], provider, /*expand_priv=*/true); } - int sighash_type = ParseSighashString(request.params[2]); + std::optional sighash_type = ParseSighashString(request.params[2]); bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool(); bool finalize = request.params[4].isNull() ? true : request.params[4].get_bool(); diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 53f943bb9e0..17a111ae0bc 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -304,12 +304,15 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FlatSigningProvider* keystore, void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map& coins, const UniValue& hashType, UniValue& result) { - int nHashType = ParseSighashString(hashType); + std::optional nHashType = ParseSighashString(hashType); + if (!nHashType) { + nHashType = SIGHASH_DEFAULT; + } // Script verification errors std::map input_errors; - bool complete = SignTransaction(mtx, keystore, coins, nHashType, input_errors); + bool complete = SignTransaction(mtx, keystore, coins, *nHashType, input_errors); SignTransactionResultToJSON(mtx, complete, coins, input_errors, result); } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 4c83517c8da..27fe1f197b8 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -378,10 +378,10 @@ UniValue DescribeAddress(const CTxDestination& dest) * * @pre The sighash argument should be string or null. */ -int ParseSighashString(const UniValue& sighash) +std::optional ParseSighashString(const UniValue& sighash) { if (sighash.isNull()) { - return SIGHASH_DEFAULT; + return std::nullopt; } const auto result{SighashFromStr(sighash.get_str())}; if (!result) { diff --git a/src/rpc/util.h b/src/rpc/util.h index 4eafa1eb6e4..0c48f8aa292 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -138,7 +138,7 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto UniValue DescribeAddress(const CTxDestination& dest); /** Parse a sighash string representation and raise an RPC error if it is invalid. */ -int ParseSighashString(const UniValue& sighash); +std::optional ParseSighashString(const UniValue& sighash); //! Parse a confirm target option and raise an RPC error if it is invalid. unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 4626a02da32..9cc5227988a 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -956,12 +956,15 @@ RPCHelpMan signrawtransactionwithwallet() // Parse the prevtxs array ParsePrevouts(request.params[1], nullptr, coins); - int nHashType = ParseSighashString(request.params[2]); + std::optional nHashType = ParseSighashString(request.params[2]); + if (!nHashType) { + nHashType = SIGHASH_DEFAULT; + } // Script verification errors std::map input_errors; - bool complete = pwallet->SignTransaction(mtx, coins, nHashType, input_errors); + bool complete = pwallet->SignTransaction(mtx, coins, *nHashType, input_errors); UniValue result(UniValue::VOBJ); SignTransactionResultToJSON(mtx, complete, coins, input_errors, result); return result; @@ -1629,7 +1632,7 @@ RPCHelpMan walletprocesspsbt() } // Get the sighash type - int nHashType = ParseSighashString(request.params[2]); + std::optional nHashType = ParseSighashString(request.params[2]); // Fill transaction with our data and also sign bool sign = request.params[1].isNull() ? true : request.params[1].get_bool(); diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 0d20a73871b..136c44d0837 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1316,7 +1316,6 @@ std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran 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); @@ -1387,7 +1386,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/wallet.cpp b/src/wallet/wallet.cpp index 215d7b505cd..de74000977d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2106,7 +2106,6 @@ std::optional CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo 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) { @@ -2145,7 +2144,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;