privacy-scaling-explorations / zk-kit

A monorepo of reusable libraries for zero-knowledge technologies.
https://zkkit.pse.dev/
MIT License
256 stars 52 forks source link

Use noble cryptography libraries #238

Open 0xbok opened 3 weeks ago

0xbok commented 3 weeks ago

Noble is a set of libraries providing "Audited & minimal JS implementation" of several cryptographic primitives implemented in zk-kit.

Some links: https://github.com/paulmillr/noble-curves, https://github.com/paulmillr/noble-hashes, https://paulmillr.com/noble/

It can be used to define babyjubjub curve, import poseidon, blake and sha hashes.

As discussed with @cedoor already, it's worthwhile to use these libraries as they are audited and optimized, and it can fix known and unkown bugs (or devation from standard) in zk-kit: like how eddsa secret key is derived currently.

cedoor commented 3 weeks ago

@artwyman, @ctrlc03 tagging you here as this would be a big change that will affect the following packages (used by MACI and some PCDs):

Those shouldn't be breaking changes, but there could be other issues related to this one, to follow the EdDSA standard, e.g. using the Noble sha-3 hash (audited) rather than the current blake1.

Curious to know what you think about this.

artwyman commented 3 weeks ago

For non-breaking changes, my primary concerns to understand would be the impact on performance and bundle download size. circomlibjs has a very large set of poseidon constants for all sizes, so switching to poseidon-lite was a big win for bundle size. I'd be curious if these noble libraries have been optimized for that.

For breaking changes, I'm most concerned about cryptographic compatibility, rather than simple API changes. We have lots of EdDSA tickets (containing Poseidon hashes and EdDSA signatures) already in the wild which need to remain verifiable. Those were created using circomlibjs, but I've tested compatibility and intend to switch to zk-kit for them. There aren't yet any PODs (new PCD format for tickets and other things) in the wild, but there will be soon (early May) so changes to the cryptographic algorithms would need to either happen quickly, or we'd need a backward-compatible option there as well. These two concerns together might argue for providing both hash options (blake1 vs. sha-3) as different configurations in the library. It would be okay if we had to make "breaking" API changes to opt for the blake1 hash in use cases which require it, but it would be important to be able to support both hashes in the same codebase.

Side note just to confirm my understanding: I think the hashing happens only in the JS code, while verification circuits operate on the secret scalar post-hash. So I'm only considering our TypeScript code for the need to be explicitly backward compatible, while assuming our EdDSAPoseidon verification circuits should still work as-is for signatures generated with either hash. Let me know if I'm misunderstanding the impact here.

cedoor commented 3 weeks ago

@artwyman thanks for your input!

I'd be curious if these noble libraries have been optimized for that.

I agree, that's what I'd like to test first.

For breaking changes, I'm most concerned about cryptographic compatibility, rather than simple API changes.

So, I forgot to add some info here. We're going to have a breaking change anyway due to this bug: https://github.com/privacy-scaling-explorations/zk-kit/issues/239, which would make public keys different regardless of the hash used. We'll add more information there as soon as possible. In the meantime, I'll ping you to add context.

I see your concerts tho!