paulmillr / noble-secp256k1

Fastest 4KB JS implementation of secp256k1 signatures and ECDH
https://paulmillr.com/noble
MIT License
757 stars 114 forks source link

Flesh out Schnorr/BIP340 functionality (version2) #52

Closed brandonblack closed 2 years ago

brandonblack commented 2 years ago

This is an alternative implementation to #50, using the technique from https://github.com/loganfsmyth/gensync to reduce the copy paste needed to implement synchronous and asynchronous versions of functions.

The added cognitive overhead of this technique is definitely a downside, but it helps keep the code from growing unnecessarily large as functionality is added, and makes it more natural to hew close to the specifications of cryptographic protocols rather than having the competing pressure to break things down to reduce copy/paste.

I went looking for this solution while I was working on implementing MuSig2* (https://github.com/ElementsProject/secp256k1-zkp/blob/10dd7e676d2930ba16d21e93a94a70b880bef312/doc/musig-spec.mediawiki) in noble-secp256k1. That being a somewhat more complex protocol, involving many tagged hashes, the overhead to having sync and async was growing.

brandonblack commented 2 years ago

Here's the WIP MuSig2* implementation I mentioned: https://github.com/brandonblack/noble-secp256k1/pull/1

brandonblack commented 2 years ago

If this style of sync/async handling gets merged, we can also combine the 3-parts of ECDSA sign into a single function, which will make it easier to validate against the spec.

paulmillr commented 2 years ago

Thanks for the work Brandon, this is a really nice concept!

I'd keep the code duplicated / less complex to not depend on async generators. Reason is: extremely security-conscious code and we need to think of all the possible outcomes of the generators. Maybe later!

Let's continue in previous PR and sorry you've wasted time on this.

brandonblack commented 2 years ago

No worries. If at some point you want MuSig2 key aggregation / signing upstreamed, we may want to revisit this. It was getting to be a real bear to implement sync/async MuSig2 w/o something like this.