Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signing/verifying BIP340 Schnorr when the message is not 32 bytes #702

Open
randombit opened this issue Jul 1, 2024 · 7 comments
Open

Comments

@randombit
Copy link

If I'm understanding the API correctly, it's not currently possible to use this crate to sign or verify BIP340 messages that are not exactly 32 bytes long. Is this correct? If so is there any change of adding such support? It seems libsecp256k1 already supports this, using secp256k1_schnorrsig_sign_custom, and for verification the plain secp256k1_schnorrsig_verify accepts any input length.

@elichai
Copy link
Member

elichai commented Jul 1, 2024

We actually talked about it on IRC today,
We should probably change the API to accept &[u8], if we want to reduce breakage we can accept impl AsRef<[u8]> and impl that for Message

@apoelstra
Copy link
Member

I think we may just have to eat the breakage -- we want to discourage people to directly use Message, by instead accepting Into<Message> for the ECDSA signer (and telling people downstream to implement this only for sighashes and nothing else).

We are also planning to drop the ThirtyTwoByteHash trait in favor of Into<Message>...so I think this fits in well to a single "break the signing API" release that hopefully will be beneficial enough for people to be worth the breakage.

@Kixunil
Copy link
Collaborator

Kixunil commented Jul 2, 2024

Please confirm if my understanding is correct. Schnorr signature needs tagged_hash(r || pk || m) where m is the message. And we supply Message so we're effectively forcing pre-hashing as mentioned in the BIP?

@elichai
Copy link
Member

elichai commented Jul 2, 2024

Please confirm if my understanding is correct. Schnorr signature needs tagged_hash(r || pk || m) where m is the message. And we supply Message so we're effectively forcing pre-hashing as mentioned in the BIP?

Basically yes. as a previous version of the BIP only supported pre-hashing

@apoelstra
Copy link
Member

@Kixunil yep, exactly. And as @elichai says, the history here is that the BIP used to only support this mode (and this mode is what Bitcoin uses in the post-Taproot CHECKSIG opcode).

apoelstra added a commit that referenced this issue Jul 5, 2024
…f 32 bytes `Message`

df98b16 Make schnorr sign/verify accept a message slice (Elichai Turkel)

Pull request description:

  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 :)

ACKs for top commit:
  Kixunil:
    ACK df98b16
  apoelstra:
    ACK df98b16 thanks for all the new test vectors\!

Tree-SHA512: bd99ea8e17fcc6fd71ad39a87c7c21761f325006998a61b33b6f2abc9f892f90a4236bd25615cb34dc83214a70dcdd34ce3e7cece7d6f971c3843505356c97c5
@wolfmcnally
Copy link

Please publish the merged fix soon.
@ChristopherA

@apoelstra
Copy link
Member

@wolfmcnally #750

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants