Merge bitcoin/bitcoin#22514: psbt: Actually use SIGHASH_DEFAULT for PSBT signing

c0405ee27f rpc: Document that DEFAULT is for Taproot, ALL for everything else (Andrew Chow)
d3992669df psbt: Actually use SIGHASH_DEFAULT (Andrew Chow)
eb9a1a2c59 psbt: Make sighash_type std::optional<int> (Andrew Chow)

Pull request description:

  Make the behavior align with the help text by actually using SIGHASH_DEFAULT as the default sighash for signing PSBTs.

ACKs for top commit:
  Sjors:
    re-utACK c0405ee27f

Tree-SHA512: 5199fb41de416b2f10ac451f824e7c94b428ba11fdb9e50f0027c692e959ce5813a340c34a4e52d7aa128e12008303d80939a693eff36a869720e45442119828
This commit is contained in:
MarcoFalke 2021-12-10 10:17:24 +01:00
commit 011d6e429b
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
8 changed files with 19 additions and 17 deletions

View file

@ -248,7 +248,7 @@ std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strN
int ParseSighashString(const UniValue& sighash)
{
int hash_type = SIGHASH_ALL;
int hash_type = SIGHASH_DEFAULT;
if (!sighash.isNull()) {
static std::map<std::string, int> map_sighash_values = {
{std::string("DEFAULT"), int(SIGHASH_DEFAULT)},

View file

@ -57,7 +57,7 @@ struct PSBTInput
std::map<CPubKey, KeyOriginInfo> hd_keypaths;
std::map<CKeyID, SigPair> partial_sigs;
std::map<std::vector<unsigned char>, std::vector<unsigned char>> unknown;
int sighash_type = 0;
std::optional<int> sighash_type;
bool IsNull() const;
void FillSignatureData(SignatureData& sigdata) const;
@ -86,9 +86,9 @@ struct PSBTInput
}
// Write the sighash type
if (sighash_type > 0) {
if (sighash_type != std::nullopt) {
SerializeToVector(s, PSBT_IN_SIGHASH);
SerializeToVector(s, sighash_type);
SerializeToVector(s, *sighash_type);
}
// Write the redeem script
@ -201,7 +201,9 @@ struct PSBTInput
} else if (key.size() != 1) {
throw std::ios_base::failure("Sighash type key is more than one byte type");
}
UnserializeFromVector(s, sighash_type);
int sighash;
UnserializeFromVector(s, sighash);
sighash_type = sighash;
break;
case PSBT_IN_REDEEMSCRIPT:
{

View file

@ -785,7 +785,7 @@ static RPCHelpMan signrawtransactionwithkey()
},
},
},
{"sighashtype", RPCArg::Type::STR, RPCArg::Default{"DEFAULT"}, "The signature hash type. Must be one of:\n"
{"sighashtype", RPCArg::Type::STR, RPCArg::Default{"DEFAULT for Taproot, ALL otherwise"}, "The signature hash type. Must be one of:\n"
" \"DEFAULT\"\n"
" \"ALL\"\n"
" \"NONE\"\n"
@ -1255,8 +1255,8 @@ static RPCHelpMan decodepsbt()
}
// Sighash
if (input.sighash_type > 0) {
in.pushKV("sighash", SighashToStr((unsigned char)input.sighash_type));
if (input.sighash_type != std::nullopt) {
in.pushKV("sighash", SighashToStr((unsigned char)*input.sighash_type));
}
// Redeem script and witness script

View file

@ -709,7 +709,7 @@ RPCHelpMan signrawtransactionwithwallet()
},
},
},
{"sighashtype", RPCArg::Type::STR, RPCArg::Default{"DEFAULT"}, "The signature hash type. Must be one of\n"
{"sighashtype", RPCArg::Type::STR, RPCArg::Default{"DEFAULT for Taproot, ALL otherwise"}, "The signature hash type. Must be one of\n"
" \"DEFAULT\"\n"
" \"ALL\"\n"
" \"NONE\"\n"
@ -1167,7 +1167,7 @@ RPCHelpMan walletprocesspsbt()
{
{"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction base64 string"},
{"sign", RPCArg::Type::BOOL, RPCArg::Default{true}, "Also sign the transaction when updating (requires wallet to be unlocked)"},
{"sighashtype", RPCArg::Type::STR, RPCArg::Default{"DEFAULT"}, "The signature hash type to sign with if not specified by the PSBT. Must be one of\n"
{"sighashtype", RPCArg::Type::STR, RPCArg::Default{"DEFAULT for Taproot, ALL otherwise"}, "The signature hash type to sign with if not specified by the PSBT. Must be one of\n"
" \"DEFAULT\"\n"
" \"ALL\"\n"
" \"NONE\"\n"

View file

@ -633,7 +633,7 @@ TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psb
}
// Get the Sighash type
if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) {
if (sign && input.sighash_type != std::nullopt && *input.sighash_type != sighash_type) {
return TransactionError::SIGHASH_MISMATCH;
}
@ -2114,7 +2114,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
}
// Get the Sighash type
if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) {
if (sign && input.sighash_type != std::nullopt && *input.sighash_type != sighash_type) {
return TransactionError::SIGHASH_MISMATCH;
}

View file

@ -236,7 +236,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 = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return TransactionError::INVALID_PSBT; }
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 uint256 GetID() const { return uint256(); }
@ -400,7 +400,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 = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) 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;
uint256 GetID() const override;
@ -609,7 +609,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 = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) 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;
uint256 GetID() const override;

View file

@ -561,7 +561,7 @@ public:
*/
TransactionError FillPSBT(PartiallySignedTransaction& psbtx,
bool& complete,
int sighash_type = 1 /* SIGHASH_ALL */,
int sighash_type = SIGHASH_DEFAULT,
bool sign = true,
bool bip32derivs = true,
size_t* n_signed = nullptr,

View file

@ -457,7 +457,7 @@ class PSBTTest(BitcoinTestFramework):
wrpc = self.nodes[2].get_wallet_rpc("wallet{}".format(i))
for key in signer['privkeys']:
wrpc.importprivkey(key)
signed_tx = wrpc.walletprocesspsbt(signer['psbt'])['psbt']
signed_tx = wrpc.walletprocesspsbt(signer['psbt'], True, "ALL")['psbt']
assert_equal(signed_tx, signer['result'])
# Combiner test