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

[kimchi][proof] add zero-knowledge rows in witness #197

Merged
merged 4 commits into from
Oct 29, 2021
Merged

Conversation

mimoo
Copy link
Contributor

@mimoo mimoo commented Oct 21, 2021

We currently add zero-knowledge rows to the witness on the OCaml side. This is not great as this library might be used by all sorts of people who might not think about adding the zero-knowledge stuff.

@mimoo mimoo requested a review from imeckler October 21, 2021 23:54
@@ -5,7 +5,7 @@ This source file implements Plonk circuit constraint primitive.
*****************************************************************************************************************/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if #193 goes first, it'll be easier to review this PR as the formatting changes will be gone

dlog/tests/chacha_test.rs Outdated Show resolved Hide resolved
circuits/plonk-15-wires/src/nolookup/constraints.rs Outdated Show resolved Hide resolved
circuits/plonk-15-wires/src/nolookup/constraints.rs Outdated Show resolved Hide resolved
dlog/plonk-15-wires/src/prover.rs Outdated Show resolved Hide resolved
@mimoo
Copy link
Contributor Author

mimoo commented Oct 22, 2021

maybe before I start fixing all the tests, some feedback from @imeckler @mrmr1993 on the approach? Because I want to make sure that we're not overwriting rows, when randomizing the last 3 rows, I expect the witness to be passed non-padded. This change means that we have to manually pad the witness when we're creating proofs, or double-checking the witness.

@mimoo mimoo force-pushed the mimoo/rand branch 2 times, most recently from 8ac3715 to b0fc55e Compare October 27, 2021 05:17

// double-check the witness
if cfg!(test) {
index.cs.verify(&witness).expect("incorrect witness");
Copy link
Contributor

Choose a reason for hiding this comment

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

Does cfg! work in such a way that in production builds this won't run? How is it controlled?

Copy link
Contributor Author

@mimoo mimoo Oct 28, 2021

Choose a reason for hiding this comment

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

yeah that should be replaced to false in production/debug build, and replaced to true when running test. I can remove it if it looks scary though :o

Copy link
Contributor

@imeckler imeckler left a comment

Choose a reason for hiding this comment

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

Approved with some questions and nits

@mimoo mimoo force-pushed the mimoo/rand branch 4 times, most recently from cb7c49c to eb2de0c Compare October 29, 2021 20:02
@mimoo
Copy link
Contributor Author

mimoo commented Oct 29, 2021

rebased, fixing remaining tests now

@mimoo mimoo force-pushed the mimoo/rand branch 2 times, most recently from 2fe31d6 to 7ade0ca Compare October 29, 2021 20:27
@mimoo
Copy link
Contributor Author

mimoo commented Oct 29, 2021

Screen Shot 2021-10-29 at 1 28 04 PM

just so that there are no surprised, added an error in ProofError

this was previously done on the OCaml side. This is better because the caller
doesn't have to think about doing it.

To make sure that the caller didn't mess up this part, we now ask the caller
to send the unpadded witness columns.
@codecov-commenter
Copy link

Codecov Report

Merging #197 (92920f7) into master (160ab8d) will decrease coverage by 0.00%.
The diff coverage is 96.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
- Coverage   88.90%   88.90%   -0.01%     
==========================================
  Files          74       74              
  Lines       18212    18248      +36     
==========================================
+ Hits        16192    16223      +31     
- Misses       2020     2025       +5     
Impacted Files Coverage Δ
oracle/src/rndoracle.rs 0.00% <ø> (ø)
dlog/plonk-15-wires/src/prover.rs 98.66% <93.47%> (-0.34%) ⬇️
...ircuits/plonk-15-wires/src/nolookup/constraints.rs 94.18% <100.00%> (+0.11%) ⬆️
circuits/plonk-15-wires/src/polynomials/chacha.rs 95.84% <100.00%> (ø)
...s/plonk-15-wires/src/polynomials/endomul_scalar.rs 85.78% <100.00%> (+0.34%) ⬆️
...uits/plonk-15-wires/src/polynomials/permutation.rs 97.54% <100.00%> (ø)
dlog/commitment/src/commitment.rs 91.90% <0.00%> (-0.36%) ⬇️

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 160ab8d...92920f7. Read the comment docs.

@mimoo mimoo deleted the mimoo/rand branch October 29, 2021 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants