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

Rebase to original Plonk #685

Merged
merged 1 commit into from
May 24, 2022
Merged

Rebase to original Plonk #685

merged 1 commit into from
May 24, 2022

Conversation

xevisalle
Copy link
Member

@xevisalle xevisalle commented May 23, 2022

This PR rebases the code to the original Plonk implementation, but keeping all the recent improvements. It does the following:

  • Modify the prover and the verifier according to the original Plonk.
  • Remove all the Plonkup related code.

@xevisalle xevisalle added the area:cryptography anything related to cryptographic primitives, hashing and ZKP label May 23, 2022
@xevisalle xevisalle self-assigned this May 23, 2022
@xevisalle xevisalle linked an issue May 23, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented May 23, 2022

Codecov Report

Merging #685 (0d1b85e) into master (71e1f6e) will decrease coverage by 1.31%.
The diff coverage is 95.73%.

❗ Current head 0d1b85e differs from pull request most recent head 716d159. Consider uploading reports for the commit 716d159 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #685      +/-   ##
==========================================
- Coverage   82.05%   80.73%   -1.32%     
==========================================
  Files          54       47       -7     
  Lines        4418     3503     -915     
==========================================
- Hits         3625     2828     -797     
+ Misses        793      675     -118     
Impacted Files Coverage Δ
src/constraint_system/arithmetic.rs 80.95% <ø> (ø)
src/constraint_system/constraint.rs 32.91% <ø> (+0.81%) ⬆️
src/constraint_system/helper.rs 88.23% <ø> (+0.73%) ⬆️
src/error.rs 0.00% <ø> (-7.15%) ⬇️
src/lib.rs 100.00% <ø> (ø)
src/proof_system/linearization_poly.rs 99.30% <ø> (-0.20%) ⬇️
src/proof_system/quotient_poly.rs 98.52% <ø> (-0.35%) ⬇️
src/proof_system/verifier.rs 53.33% <ø> (ø)
src/permutation.rs 89.78% <90.90%> (-0.01%) ⬇️
src/constraint_system/logic.rs 93.02% <94.28%> (-2.48%) ⬇️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77c2377...716d159. Read the comment docs.

@xevisalle xevisalle marked this pull request as ready for review May 23, 2022 13:08
@xevisalle xevisalle requested review from ZER0, moCello and vlopes11 May 23, 2022 13:08
Copy link
Member

@moCello moCello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell it LGTM, just one comment that you might want to add but that is up to you

}

impl Serializable<{ 24 * BlsScalar::SIZE }> for ProofEvaluations {
impl Serializable<{ 16 * BlsScalar::SIZE }> for ProofEvaluations {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can improve the code-base here by adding a comment that describes the magic number

@xevisalle xevisalle merged commit 2774e15 into master May 24, 2022
@xevisalle xevisalle deleted the rebase branch May 24, 2022 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:cryptography anything related to cryptographic primitives, hashing and ZKP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rebase the repository to the original Plonk
2 participants