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: add tweakHash and extraEntropy optional params to signSchnorr() #6

Conversation

motorina0
Copy link
Member

Related to: bitcoinjs/bitcoinjs-lib#1742 (comment)

Taproot requires that the Schnorr Signature is produced from the tweaked private key (corresponding to the Output Key)

Changes:

  • add the tweakHash optional parameter to signSchnorr()
  • add the extraEntropy optional parameter to signSchnorr()
    • this one should have already been here

@motorina0
Copy link
Member Author

motorina0 commented Jan 29, 2022

Q: maybe verifySchnorr() should also take a tweakHash optional param?

@motorina0
Copy link
Member Author

  • add tweak for verifySchnorr()
  • move tweak as the last (optional) parameter

@motorina0
Copy link
Member Author

Another option is to add thetweak field to ECPairOptions.
If the tweak field is present then we can either:

  1. directly generate tweaked private and public keys, OR
  2. only use the tweak for signSchnorr()

src/ecpair.d.ts Outdated
verifySchnorr(hash: Buffer, signature: Buffer): boolean;
signSchnorr(hash: Buffer): Buffer;
verifySchnorr(hash: Buffer, signature: Buffer, tweak?: Buffer): boolean;
signSchnorr(hash: Buffer, extraEntropy?: Buffer, teak?: Buffer): Buffer;
Copy link
Member

Choose a reason for hiding this comment

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

Tweak?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed typo.

@junderw
Copy link
Member

junderw commented Feb 3, 2022

Another option is to add thetweak field to ECPairOptions. If the tweak field is present then we can either:

  1. directly generate tweaked private and public keys, OR
  2. only use the tweak for signSchnorr()

The question to ask is "will the interface users need to pass in data?"

  1. If the tweak is something that will only ever be created from info contained in the ECPair (ie. the hash of it's own pubkey) then there is no need for users of the interface to pass in tweaks.
  2. If the tweak is something that could possibly be passed in, then we should allow it to be passed as data to the method.

Since we will be signing tweaked with various things based on the hashing of scripts done outside ECPair keeping it as a parameter to the method rather than an instantiation parameter seems wiser.

@motorina0
Copy link
Member Author

Since we will be signing tweaked with various things based on the hashing of scripts done outside ECPair keeping it as a parameter to the method rather than an instantiation parameter seems wiser.

  • I agree. This is the current implementation in this PR
  • I've mentioned the other options for sake of comparing advantages/disadvantages

Please let me know if there is something I should change to this PR.

@motorina0

This comment was marked as outdated.

@junderw

This comment was marked as outdated.

@motorina0

This comment was marked as outdated.

@junderw

This comment was marked as outdated.

@motorina0

This comment was marked as outdated.

@motorina0

This comment was marked as outdated.

@junderw

This comment was marked as outdated.

@motorina0

This comment was marked as outdated.

@motorina0
Copy link
Member Author

motorina0 commented Jun 1, 2022

I actually think Option 2 is a good approach for the psbt scenario.

cc: @tiero, @brandonblack

@tiero

This comment was marked as resolved.

@motorina0

This comment was marked as outdated.

@tiero
Copy link

tiero commented Jun 1, 2022

What if we add a verifySchnorrSafe (or similar name)

We want to keep the changes to the Signer interface to a minimum. This would not be backwards compatible.

Okok makes sense. So would go with option 2 as well.

Also what if verifying with pubkey only happens in validareSignatureOfInput and we ask optional tweak pubkey there as parameter? (Or also this we cant change?)

@brandonblack
Copy link

I think I'm in favor of a tweak function interface rather than adding the argument to signSchnorr:

  • Can support both ordinary and x-only tweaking. tweak(tweak: Buffer, xOnly: boolean) There's at least some reason to believe that future Bitcoin versions will bring back compact keys due to the complications introduced by x-only keys.
  • The interface signSchnorr(msg, e, tweak) makes it easy to mess up and put tweak where e goes. Obviously post-signing verification in psbt protects against that, but we could avoid that (likely double) verification also.
  • It's more consistent to have individual functions that do individual things on a lower level interface like this.

@junderw
Copy link
Member

junderw commented Jun 1, 2022

I think I'm in favor of a tweak function interface rather than adding the argument

👍

@motorina0
Copy link
Member Author

Will be done in a different PR.

@motorina0 motorina0 closed this Jun 1, 2022
@motorina0 motorina0 mentioned this pull request Jun 7, 2022
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.

4 participants