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

Switch to @noble/curves #25

Closed
paulmillr opened this issue Feb 17, 2023 · 12 comments
Closed

Switch to @noble/curves #25

paulmillr opened this issue Feb 17, 2023 · 12 comments
Labels
good first issue Good for newcomers

Comments

@paulmillr
Copy link

curves recently got out. This could be an opportunity to:

  1. Make API synchronous
  2. Improve compatibility with environments where EC is not available in native webcrypto
  3. Add support for additional curves, such as secp256k1, or any other curve, including custom ones.

As a side note, you're using hash-to-curve which is also implemented in curves. We will probably publish an audit of the library some time soon.

I have started working on the pull request for voprf-ts, and now half-way there. Would you folks accept it?

@armfazh
Copy link
Contributor

armfazh commented Feb 17, 2023

Hi, what about providing a group interface (./src/group.ts) so we can plug different implementations of group, say sjcl and noble.

@armfazh armfazh added the good first issue Good for newcomers label Feb 17, 2023
@paulmillr
Copy link
Author

@armfazh right now my fork has all calls to sjcl replaced with calls to curves. The interfaces (Scalar, Elt) are similar, however i've replaced calls to sjcl.bn with native bigint, which means the type are not exact.

How should this be resolved?

Also, should I continue? I don't want to continue if you aren't going to merge this.

@paulmillr
Copy link
Author

paulmillr commented Feb 17, 2023

sjcl seems like piece of shit library, probably 20x slower than noble while having much more code

@paulmillr
Copy link
Author

I'm talking about this:

    private constructor(public readonly g: Group, private readonly k: sjcl.bn) {

getting replaced with k: bigint.

@paulmillr
Copy link
Author

@armfazh OK, just understood what you've meant. You want to have voprf-ts not depend on any crypto library, right?

This makes sense and could allow easy noble plug-in. However, where would you place noble-based group.ts though? Is it this repo? I don't want to place an abstraction into noble-curves directly because we have enough abstractions of our own and the new one would only be useful for voprf-ts.

@armfazh
Copy link
Contributor

armfazh commented Feb 17, 2023

The PR like any other piece of code must pass through a code review process. So I cannot guarantee anything in advance.

OTOH, I like the idea of using something that is faster, and also secure. The properties of sjcl are well-known.

We can work together to get the changes needed to have noble as a plug-able lib. Feel free to email me for details.

@paulmillr
Copy link
Author

After our discussion: what's the decision? Should I pursue this, and if everything is ok after pr review, you would merge it; or you would prefer to keep sjcl?

@armfazh
Copy link
Contributor

armfazh commented Feb 21, 2023

Please go for it, it's ok moving to noble.

@paulmillr
Copy link
Author

=> #26

@sublimator
Copy link
Contributor

@armfazh

We can work together to get the changes needed to have noble as a plug-able lib. Feel free to email me for details.

I am potentially down to help with that

@sublimator
Copy link
Contributor

@armfazh

Feel free to email me for details.

I emailed you ;)

@armfazh
Copy link
Contributor

armfazh commented Sep 2, 2023

Completed in #38

@armfazh armfazh closed this as completed Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants