search

celo-org / celo-bls-snark-rs

Implements SNARK-friendly BLS signatures
https://celo.org
Apache License 2.0
83 stars 24 forks source link

HashToCurve and Hasher: Add new() method #203

Open ValarDragon opened 3 years ago

ValarDragon commented 3 years ago

The current HashToCurve / Hasher API's don't let one template over the Hash type. The issue at the moment is that the hash functions require operating on a struct instantiated elsewhere.

I suggest refactoring both of these to provide a new() -> Result<Self, Self::Error> API. The digest Trait, which allows for generalizing over cryptographic hash function, does something similar with just having a new() method.

I'm currently working on such a change, and would be happy to upstream it EDIT: Its done in this branch (https://github.com/sikkatech/celo-bls-snark-rs/tree/make_hashes_have_new_fn ), sans for fixing the same doctests that are blocking #202

kobigurk commented 3 years ago

I like the idea in general a lot, though keep in mind the following (which was the main reason it was designed like this) - when instantiating a large BH hasher, we need to generate a lot of base points, which can take noticable time.

ValarDragon commented 3 years ago

That makes sense. In those cases, I believe that we could make them return copies of a lazily evaluated singleton instance.

Another question is should the new() method take in the domain? I imagine in the general case, you can cache the work needed for domains / personalizations. (For instance if you set a personalization string to be the first block of what you hash.

kobigurk commented 3 years ago

That makes sense. In those cases, I believe that we could make them return copies of a lazily evaluated singleton instance.

Another question is should the new() method take in the domain? I imagine in the general case, you can cache the work needed for domains / personalizations. (For instance if you set a personalization string to be the first block of what you hash.

Yep, singleton instances might work here, which is similar to what we do now, but more explicitly.

About domain, I don't have a strong preference either way. In the use-cases I'm imagining this optimization would be hard to apply anyway.