Make schnorr sign/verify accept a message slice instead of 32 bytes Message
#706
Conversation
| @@ -959,8 +959,8 @@ impl Keypair { | |||
| /// Constructs an schnorr signature for `msg` using the global [`SECP256K1`] context. | |||
| #[inline] | |||
| #[cfg(all(feature = "global-context", feature = "rand-std"))] | |||
| pub fn sign_schnorr(&self, msg: Message) -> schnorr::Signature { | |||
| SECP256K1.sign_schnorr(&msg, self) | |||
| pub fn sign_schnorr(&self, msg: &[u8]) -> schnorr::Signature { | |||
There was a problem hiding this comment.
I'm not really sure about this. On one hand Message worked like a lint on the other this API makes much more sense in the context of schnorr sigs. I'm tentatively ACK on this but I'll take more time to think about it and possibly hear other opinions.
Other than that, the PR looks OK.
There was a problem hiding this comment.
Message forced the user to pre-hash their message, which is not required by the upstream API, has no security benefit, and cannot be worked around (it's not just a lint, it's a meaningful API restriction).
There was a problem hiding this comment.
I was thinking of having two methods or something. But I guess this is fine. I can't find a good enough reason to do it differently.
There was a problem hiding this comment.
Please publish this change soon. We'd rather not have to fork the crate to support BIP-340 signatures.
@ChristopherA
|
Can you rebase to get CI passing? |
Done :) |
|
Ok, now the CI failures look real -- formatting and MSRV. |
|
I've realized that this adds another reason to have a first-class signing API in |
|
For sure -- in Bitcoin we want a signing API that takes the sighash types (or something equivalent) and does the right thing. And which doesn't let you sign arbitrary crap, at least not without first extracting a rust-secp key. |
|
Looking forward to seeing this released! |
Migrated `from_slice` usage to `from_byte_array` with proper error forwarding when needed. Converted to Message::as_ref<[u8]> for signing due to API change of receiving arbitrary byte arrays in lieu of Message hashes only. Updated lockfiles. `from_slice` has been deprecated by rust-bitcoin#3102 due to better support for arrays in Rust. BIP340 supports arbitrary byte arrays as discussed on rust-bitcoin/rust-secp256k1#706
Migrated `from_slice` usage to `from_byte_array` with proper error forwarding when needed. Converted to Message::as_ref<[u8]> for signing due to API change of receiving arbitrary byte arrays in lieu of Message hashes only. Updated lockfiles. `from_slice` has been deprecated by rust-bitcoin#3102 due to better support for arrays in Rust. BIP340 supports arbitrary byte arrays as discussed on rust-bitcoin/rust-secp256k1#706 Closes rust-bitcoin#3482
Migrated `from_slice` usage to `from_byte_array` with proper error forwarding when needed. Converted to Message::as_ref<[u8]> for signing due to API change of receiving arbitrary byte arrays in lieu of Message hashes only. Updated lockfiles. `from_slice` has been deprecated by rust-bitcoin#3102 due to better support for arrays in Rust. BIP340 supports arbitrary byte arrays as discussed on rust-bitcoin/rust-secp256k1#706 Closes rust-bitcoin#3482
As discussed on #702 and on IRC,
BIP340 has evolved from supporting only "pre-hashed" 32 byte messages, to supporting messages of "any length" and as such we should allow the users to pass a message of any length.
Note that passing exactly 32 bytes will make the API behave exactly as before (ie it will produce the same signatures).
I added all the test vectors from: https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv To make sure the API is correct even for empty messages and shorter/longer ones :)