-
Notifications
You must be signed in to change notification settings - Fork 122
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
Update arkworks
dependencies
#207
Conversation
Fr, G1Projective, G2Projective, Fqk move pairing imports and associated traits pairing stuff
`short_weierstrass_jacobian` → `short_weierstrass` `twisted_edwards_extend` → `twisted_edwards`
te/sw curve config imports
no longer under `group` submodule
ark-serialize work on bls sigs ark-serialization for aead structs use the new ark-serialize interface in `to_bytes!` macro remove mistaken serde import
group/projective rename
use the inner value on PairingOutput for checking if ==1
normalize batch
fix more clippy clippy warnings fix clippy: avoid empty init
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, for serialization, arkworks currently does the same whether it's compressed/uncompreseed or checked/unchecked.
plonk/src/proof_system/structs.rs
Outdated
@@ -100,7 +100,7 @@ where | |||
.chunks_exact(2) | |||
.map(|chunk| { | |||
if chunk.len() == 2 { | |||
Commitment(Affine::new(chunk[0], chunk[1], false)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should use this constructor or Affine::new_unchecked
( cuz the former include is_on_curve
and is_in_correct_subgroup
by default)
(same for other changes in this commit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also gone through it. LGTM! 🚀
Again, really appreciate the work Marti @tessico
I'm glad to see this merged! Just saw that since EspressoSystems/tagged-base64#47 hasn't been merged and tagged yet, the master here now depends on a branch of tagged-base64, which probably isn't ideal for long term, sth to keep in mind :) |
Description
Pay special attention to:
TODO
s I leftcloses: #45
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
Pending
section inCHANGELOG.md
Files changed
in the GitHub PR explorer