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

feat(schnorr): add support for schnorr signatures #189

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kewde
Copy link

@kewde kewde commented Feb 16, 2022

I won't be continuing this work but it's a good base for anyone else.

  • Needs a fix for elliptic - reject overflowing secret keys.
  • Only on test vector was ported - should be all of them.
  • Browser fallback for schnorr with elliptic is not included in this PR.

helps solve #164

schnorrVerify (sig, msg32, pubkey) {
isUint8Array('signature', sig, 64)
isUint8Array('message', msg32, 32)
isUint8Array('public key', pubkey, [32, 33, 65])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This will allow an odd-Y DER pubkey to verify true against a schnorr signature when technically xonly pubkeys are always assumed to be even. Currently this is achieved by throwing away the parity bit in the code.

While this is pretty understandable for signing (the C library will automatically negate the private key if Y is odd) verification should be stricter IMO.

I am asking the library "Does this pubkey verify the signature X" and if the Y is odd, we're flipping the pubkey to a different pubkey before returning true... we would need to widen the definition of what "pubkey" means in the context of this function alone to mean "Either the given pubkey or its inverse"

To reduce confusion on this point, I think only the xonly (32 length) pubkey should be accepted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we could check the DER pubkeys for evenness and return false/throw. (check first byte is 0x02 for 33 length, check that pubkey[64] & 1 === 0 for 65 length)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense - test vector 3 requires negation of the seckey therefore I assumed it was fine.
https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.py#L73-L85

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That outputs a 32 byte xonly pubkey hex to the csv.
Like I said, automatically flipping to even when signing is fine. It's during verification that you need to be more strict.

Copy link

@Selarun15 Selarun15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0xa170ecfc733473C7c757185fEb279Bf282D83CDa

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

Successfully merging this pull request may close these issues.

3 participants