-
Notifications
You must be signed in to change notification settings - Fork 22
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
New curve interfaces #484
Merged
Merged
New curve interfaces #484
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…for ff::Field and concrete implementations for Fr and Fq
…r ristretto; add benchmarks
I thought the idea was to have a minimal PR that only does the introduction of new abstractions. This seems to have more, e.g., why is there ark-ff as a new dependency? |
abizjak
reviewed
Dec 7, 2023
Co-authored-by: Aleš Bizjak <[email protected]> Co-authored-by: eb-concordium <[email protected]>
applied Emil B's comments Co-authored-by: eb-concordium <[email protected]>
Added some minor comments and suggestions. Otherwise, looks good to me. |
hamiidreza
reviewed
Dec 20, 2023
rust-src/concordium_base/src/curve_arithmetic/ed25519_instance.rs
Outdated
Show resolved
Hide resolved
rust-src/concordium_base/src/curve_arithmetic/ed25519_instance.rs
Outdated
Show resolved
Hide resolved
rust-src/concordium_base/src/curve_arithmetic/ed25519_instance.rs
Outdated
Show resolved
Hide resolved
minor suggestions
eb-concordium
approved these changes
Jan 2, 2024
abizjak
approved these changes
Jan 2, 2024
rust-src/concordium_base/src/curve_arithmetic/bls12_381_instance.rs
Outdated
Show resolved
Hide resolved
rust-src/concordium_base/src/curve_arithmetic/bls12_381_instance.rs
Outdated
Show resolved
Hide resolved
rust-src/concordium_base/src/curve_arithmetic/ed25519_instance.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Aleš Bizjak <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Make the
Curve
trait and related functionality (e.g. finine field interfaces) better suitable for adding new curve implementations. Add thecurve25519
implementation using the updated interface. Add benchmarks of thecurve25519
curve with our bulletproof implementation.Changes
Field
andPrimeField
traits; the idea is that they contain only what we need and don't have extra dependencies; moreover, we can provide implementation for external type, which was not possible withff::Field
;multiexp
algoritm implementation, if a certain curve provides an optimised version;Field
,PrimeField
for theBLS12-381
curve (including a blanket inplementation ofField
for anyF : ff::Field
;Field
,PrimeField
andCurve
for The ristretto representation ofcurve25519-dalek
;curve25519-dalek
exposed through our interface (authored by @hamiidreza);curve25519
;range proof from dalek's(was exluded in the development process)bulletproofs
(authored by @hamiidreza).Checklist
hard-to-understand areas.