psbt: use DER encoded + sighash byte for PSBT_IN_PARTIAL_SIG items #5307
Conversation
|
That is likely because we were using the PSBT format as an internal transport format only, and didn't use Thank you very much @jgriffiths, and sorry for causing a false alarm for |
|
ACK d8bde0a |
Per BIP-0171, the signature map is of pubkey to "The signature as would be pushed to the stack from a scriptSig or witness". Fixes 5298 Changelog-Fixed: PSBT: Fix signature encoding to comply with BIP-0171. Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
jgriffiths
force-pushed
the
psbt_partial_sig_fix
branch
from
d8bde0a
to
5af53b1
Compare
|
I've squashed the test commit into the actual change for git-bisectability, no further changes. |
|
Hurray, the bot actually did recognize that there were no content changes between the PR versions and reapplied the ACKs :-) |
| @@ -268,18 +268,20 @@ bool psbt_input_set_signature(struct wally_psbt *psbt, size_t in, | |||
| const struct bitcoin_signature *sig) | |||
| { | |||
| u8 pk_der[PUBKEY_CMPR_LEN]; | |||
| u8 sig_der[73]; | |||
There was a problem hiding this comment.
is this constant defined anywhere by any chance?
There was a problem hiding this comment.
Wally has EC_SIGNATURE_DER_MAX_LEN so EC_SIGNATURE_DER_MAX_LEN + 1 could be used (1 for the sighash byte).
That should be a separate commit if done IMO, also changing the other places below (I just followed the existing examples). Note that this would require wally be exposed in the interface of bitcoin/signature.h which it currently isn't. Perhaps a c-lightning specific define?
bitcoin/script.c: u8 der[73];
bitcoin/script.c: u8 der[73];
bitcoin/signature.c:size_t signature_to_der(u8 der[73], const struct bitcoin_signature *sig)
bitcoin/signature.c: u8 der[73];
bitcoin/signature.h:size_t signature_to_der(u8 der[73], const struct bitcoin_signature *sig);
There was a problem hiding this comment.
just a nit, doesn't block this PR
ACK
|
confirmed compatible with rust-bitcoin 0.28.1 |
This fixes the incorrect signature encoding as reported in #5298.
I don't see that the old code could ever have worked: whether signing through wally or any other PSBT signer, the resulting signature in the witness would be of an incorrect form and thus should be rejected by core.
Its probably worth getting submission/validation of a tx produced from signing one of these PSBTs under test.