gui, psbt: Use SIGHASH_DEFAULT when signing PSBTs

[achow101]

SIGHASH_DEFAULT should be used to indicate SIGHASH_DEFAULT for taproot inputs, and SIGHASH_ALL for all other input types. This avoids adding an unnecessary byte to the end of all Taproot signatures added to PSBTs signed in the GUI.

See also bitcoin/bitcoin#22514

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	Sjors, pablomartin4btc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #832 (Improve user dialog when signing multisig psbts by mjdietzx)
• #bitcoin/bitcoin/31622 (psbt: add non-default sighash types to PSBTs and unify sighash type match checking by achow101)
• #bitcoin/bitcoin/29675 (wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[luke-jr]

This avoids adding an unnecessary byte to the end of all Taproot signatures added to PSBTs signed in the GUI.

Two bytes, isn't it? For the PSBT_IN_SIGHASH keytype

[achow101]

This avoids adding an unnecessary byte to the end of all Taproot signatures added to PSBTs signed in the GUI.

Two bytes, isn't it? For the PSBT_IN_SIGHASH keytype

No, that field is not added. This is referring to the actual signature that will show up in the scriptWitness, and as appears in PSBT_IN_TAPROOT_*_SIG fields beforehand.

[Sjors]

utACK 3e97ff9

It would be useful for the FillPSBT doc in wallet.h to explain that passing SIGHASH_DEFAULT will use SIGHASH_ALL for pre-taproot inputs.

You have to crawl all the way to MutableTransactionSignatureCreator::CreateSig to find this out:

// BASE/WITNESS_V0 signatures don't support explicit SIGHASH_DEFAULT, use SIGHASH_ALL instead.
const int hashtype = nHashType == SIGHASH_DEFAULT ? SIGHASH_ALL : nHashType;

bitcoin/bitcoin@e94bb4e makes this problem go away though.

---

Additional background:
https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#taproot-key-path-spending-signature-validation

A Taproot signature is a 64-byte Schnorr signature, as defined in BIP340, with the sighash byte appended in the usual Bitcoin fashion. This sighash byte is optional. If omitted, the resulting signatures are 64 bytes, and a SIGHASH_DEFAULT mode is implied.

[pablomartin4btc]

utACK 3e97ff9

I agree with @Sjors but is that not covered by doc at the /** Signature hash types/flags */ enum definition (src/script/interpreter.h)?

bitcoin/bitcoin@e94bb4e makes this problem go away though.

is that part of a PR?

[Sjors]

@pablomartin4btc it's part of #31622.

The description in interpreter.h is ambiguous. It says "Taproot only". It also says "equivalent to SIGHASH_ALL", but doesn't say that it gets converted (and obviously a constant can't guarantee that actually happens).