Add Bindings for Schnorr Adaptor Signatures #89
Conversation
updated the secp256k1-zkp-sys vendor with the following commands: ```bash export SECP_VENDOR_SECP_REPO="https://github.com/siv2r/secp256k1-zkp.git" ./vendor-secp.sh schnorr-adaptor-module ````
|
Nice, thanks!! I reviewed the code, though I don't think we should merge it until upstream merges theirs and we can vendor it from the main repo. |
|
|
||
| impl str::FromStr for SchnorrAdaptorPreSignature { | ||
| type Err = Error; | ||
| fn from_str(s: &str) -> Result<SchnorrAdaptorPreSignature, Error> { |
There was a problem hiding this comment.
In 28fd225:
Can you use Self rather than SchnorrAdaptorPreSignature everywhere you can? It helps to keep track of which functions are manipulating an object and which ones are converting it to other types.
| secp: &Secp256k1<C>, | ||
| msg: &Message, | ||
| keypair: &Keypair, | ||
| adaptor: &PublicKey, |
There was a problem hiding this comment.
In 28fd225:
I think we should introduce a new type PublicAdaptor. Fine to have From<PublicKey> and vice-versa on it, but it helps to keep the types separate. Also fine, I think, to keep using SecretKey for the secret adaptor since that's only used in one function and it's one you've gotta be careful about anyway.
There was a problem hiding this comment.
Oh, actually I see that we have methods that both create a signature from a secret adaptor and extract it. Now I think that we should create a new SecretAdaptor (or maybe just Adaptor) type for the secret half as well.
There was a problem hiding this comment.
Okay, I'll introduce the SecretAdaptor and Adaptor types. The Adaptor type for the public adaptor, to keep it consistent with the zkp module naming convention.
| } | ||
|
|
||
| /// Extracts the adaptor point from a Schnorr adaptor pre-signature. | ||
| pub fn extract_adaptor(&self, msg: &Message, pubkey: &XOnlyPublicKey) -> Result<PublicKey, Error> { |
There was a problem hiding this comment.
In 28fd225:
In this method, extract_secret_adaptor, and adapt, I'm tempted to take self by value so that the user can't use the same presignature with multiple adaptors or signatures unless they call .clone to explicitly duplicate it. This is a mild inconvenience, but the clone call is an opportunity for the user to comment what it is they're doing using the same (pre)sig in two different contexts, and for simple protocols it might prevent accidents.
On the other hand, this might be seen as just "making the API hard to use for no good reason" since a presignature is a public object and there's no actual harm in trying to do multiple things with it. (Plus, this entire adaptor signature module is designed for "roll your own crypto" applications.)
@jonasnick what do you think?
There was a problem hiding this comment.
I don't really have an opinion on this. It seems like given that the module is designed to "roll your own crypto", the accidents that can happen by using the same presig in two contexts seem to be not very severe in comparison.
There was a problem hiding this comment.
Yeah, that's how I'm leaning. Ok, let's not take self by value.
This PR introduces Rust bindings for the Schnorr adaptor signature functionality added in: BlockstreamResearch/secp256k1-zkp#299. The secp256k1-zkp PR is pending review from the maintainers, but I don’t expect the user-facing APIs to change much. Hence, I’m opening the bindings PR here.
Changes:
secp256k1_schnorr_adaptor_presignsecp256k1_schnorr_adaptor_extractsecp256k1_schnorr_adaptor_adaptsecp256k1_schnorr_adaptor_extract_seczkp::schnorr_adaptor::SchnorrAdaptorPreSignaturewrapper type.presign.extract_adaptor.adapt.extract_secadaptor.Note: The overall diff might appear large because the first commit (528f610) updates the secp256k1-zkp-sys vendor with the Schnorr adaptor module (since those changes haven’t been merged upstream yet). The code changes relevant to the bindings are in commits 2 and 3.