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

New Transcript API (and modified commitment scheme) #111

Merged
merged 19 commits into from
Jan 13, 2021
Merged

Conversation

ebfull
Copy link
Contributor

@ebfull ebfull commented Dec 23, 2020

Closes #66.

The goal of this PR is to solve multiple problems:

  1. Currently, we have various Proof objects scattered around the codebase for each of the different subprotocols. Some of these proof objects contain vectors of (vectors of) curve points and scalars. This complicates serialization/deserialization of proofs because the lengths of these vectors depend on the configuration of the circuit. We don't want to encode the lengths of vectors inside of proofs because we'll know them at runtime and they do not change. We should instead treat proof objects as opaque byte streams into the verifier.
  2. It's easy to accidentally put stuff in a Proof object that isn't also placed in the transcript.
  3. In Multi-proof prover #67 we need to be able to create multiple PLONK proofs at the same time and they will share many different substructures when they're for the same circuit.
  4. (Bonus) Now verification also accounts for the cost of deserialization, which isn't negligible due to point compression.

The idea is to refactor the transcript module to provide two separate concepts: a TranscriptWrite trait that represents something we can write to (at proving time) and a TranscriptRead trait that represents something we can read from (at verifying time). Crucially, implementations of these traits are responsible for simultaneously writing to some std::io::Write buffer at the same time that they hash things into the transcript, and similarly for TranscriptRead/std::io::Read.

We can then remove all of the Proof-like structures from the codebase.

In addition, we perform a few refactorings of the PLONK implementation to reflect the fact that the verifier needs to temporarily store values in the proof and cannot simply access a Proof object anymore. This makes the verifier look a lot more like the prover, which does the same thing for other reasons.

In order to make this easier I had to also change the commitment scheme to more closely match the one from the accumulation scheme paper. That involved three changes:

  1. We subtract [v] G_0 from the commitment P that we're opening at x; the argument from then forward becomes "does the polynomial committed at P have a root at x" which will be efficient inside of recursive circuits because it's just a fixed-based scalar multiplication, versus the variable base P' = P + [v] U thing that we do in the Halo paper.
  2. The prover supplies a commitment R to a random polynomial that also has a root at x. The verifier samples random iota and computes P' = P - [v] G_0 + [iota] R and the inner product argument is run on P'. This makes it so that we don't need to do a Schnorr proof at the end because P' is uniformly distributed in the space of polynomials with a root at x.
  3. We don't sample U for checking the values; we use a fixed U and compute a challenge z to compute U' = [z] U and use U' for that purpose. It was never really necessary for U to be sampled from the group, which I didn't realize at the time.

@ebfull
Copy link
Contributor Author

ebfull commented Dec 23, 2020

Highly recommend reviewing with whitespace changes hidden.

@codecov-io
Copy link

codecov-io commented Dec 23, 2020

Codecov Report

Merging #111 (cc6b0bb) into main (fb37172) will increase coverage by 1.02%.
The diff coverage is 97.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
+ Coverage   83.44%   84.46%   +1.02%     
==========================================
  Files          34       36       +2     
  Lines        3738     3862     +124     
==========================================
+ Hits         3119     3262     +143     
+ Misses        619      600      -19     
Impacted Files Coverage Δ
benches/plonk.rs 0.00% <ø> (ø)
src/arithmetic/fields.rs 47.05% <ø> (-25.41%) ⬇️
src/pasta/curves.rs 74.92% <ø> (+3.76%) ⬆️
src/pasta/fields/fp.rs 84.70% <ø> (-0.14%) ⬇️
src/pasta/fields/fq.rs 81.34% <ø> (-0.17%) ⬇️
src/plonk/lookup.rs 85.71% <ø> (ø)
src/plonk/permutation.rs 100.00% <ø> (ø)
src/poly/multiopen.rs 97.94% <ø> (ø)
src/poly/commitment.rs 88.96% <89.55%> (+1.57%) ⬆️
src/poly/commitment/msm.rs 94.02% <93.75%> (-2.05%) ⬇️
... and 26 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 fb37172...cc6b0bb. Read the comment docs.

src/poly/commitment/verifier.rs Outdated Show resolved Hide resolved
src/poly/commitment/prover.rs Outdated Show resolved Hide resolved
src/transcript.rs Outdated Show resolved Hide resolved
src/transcript.rs Outdated Show resolved Hide resolved
src/poly/commitment.rs Outdated Show resolved Hide resolved
src/transcript.rs Outdated Show resolved Hide resolved
src/transcript.rs Outdated Show resolved Hide resolved
src/plonk/verifier.rs Outdated Show resolved Hide resolved
@str4d
Copy link
Contributor

str4d commented Dec 23, 2020

It would be great for this PR to also add a design page (probably book/src/design/implementation.md) describing why we do this / why proofs are read and written via the transcript 😁

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK. I have not checked the protocol changes.

@r3ld3v r3ld3v added this to the Core Sprint 2020-53 milestone Jan 4, 2021
str4d added a commit that referenced this pull request Jan 6, 2021
Previously, `ChallengeScalar` could use the operator traits defined on
the `F: Field` type it wrapped, due to its `impl Deref<Target = F>`.
This was technically ambiguous, and Rust 1.49.0 makes that ambiguity an
error.

We could fix this by adding operator impls with `ChallengeScalar` on the
RHS, but that would conflict with #111. Instead we manually
dereference every challenge scalar when used in an arithmetic operation.
ebfull and others added 4 commits January 12, 2021 07:40
This modifies the scheme to be almost identical to the construction
outlined in Appenix A.2 of "Proof-Carrying Data from Accumulation
Schemes" (https://eprint.iacr.org/2020/499). The only remaining
difference is that we do not compute [v] U but instead subtract
[v] G_0 from the commitment before opening.
Avoid square challenges in inner product argument
// = [a] G + [-a] H + [abz] U + [h] H
// = [a] G + [abz] U + [h - a] H
// but subtracting to get the desired equality
// ... + [-a] G + [-abz] U + [a - h] H = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the ... here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the left hand side of the equality, msm - [v] G_0 + random_poly_commitment * iota + \sum(L_i * u_i^2) + \sum(R_i * u_i^-2).

// = [a] (G + [b * z] U) + [h] H
// except that we wish for the prover to supply G as Commit(g(X); 1) so
// we must substitute to get
// = [a] ((G - H) + [b * z] U) + [h] H
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand how this is a substitution into the previous line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The prover will not be giving us G but rather G + H (the blinding factor is 1), so we have to subtract H from what they give us. We do this because auxillary commitments always have a blinding factor of 1 in the PLONK API, ensuring that under no circumstance will points at infinity emerge inside of a recursive circuit.

src/transcript.rs Outdated Show resolved Hide resolved
daira pushed a commit to daira/halo2 that referenced this pull request Jan 13, 2021
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

I reviewed everything except the first commit from #113.

src/poly/commitment/prover.rs Show resolved Hide resolved
src/poly/multiopen/prover.rs Show resolved Hide resolved
src/plonk.rs Show resolved Hide resolved
src/plonk/lookup/prover.rs Show resolved Hide resolved
src/plonk/lookup/verifier.rs Outdated Show resolved Hide resolved
src/transcript.rs Outdated Show resolved Hide resolved
src/transcript.rs Outdated Show resolved Hide resolved
@@ -122,6 +122,10 @@ pub trait CurveAffine:
/// The base field over which this elliptic curve is constructed.
type Base: FieldExt;

/// Personalization of BLAKE2b hasher used to generate the uniform
/// random string.
const BLAKE2B_PERSONALIZATION: &'static [u8; 16];
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I like this being a property of the curve implementation, but it will do for now. We will likely need to refactor this later.

Comment on lines +705 to +706
new_curve_impl!(Ep, EpAffine, Fp, Fq, b"halo2_____pallas");
new_curve_impl!(Eq, EqAffine, Fq, Fp, b"halo2______vesta");
Copy link
Contributor

Choose a reason for hiding this comment

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

These definitely will need changing to be more descriptive as to their purpose 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

This will change again when we switch to simplified SWU for hash-to-curve.

src/poly/commitment.rs Outdated Show resolved Hide resolved
@ebfull ebfull merged commit ccca639 into main Jan 13, 2021
@str4d str4d deleted the transcript-api-2 branch January 14, 2021 00:31
str4d pushed a commit that referenced this pull request Jan 19, 2022
str4d added a commit that referenced this pull request Jan 19, 2022
[ECC chip] Fixed- and variable-base scalar multiplication
han0110 pushed a commit to han0110/halo2 that referenced this pull request Jan 19, 2023
…ng key into uncompressed Montgomery form (zcash#111)

* feat: read `VerifyingKey` and `ProvingKey` does not require `params` as
long as we serialize `params.k()`

* feat: add features "serde-raw" and "raw-unchecked" to
serialize/deserialize KZG params, verifying key, and proving key
directly into raw bytes in internal memory format.
So field elements are stored in Montgomery form `a * R (mod p)` and
curve points are stored without compression.

* chore: switch to halo2curves 0.3.1 tag

* feat: add enum `SerdeFormat` for user to select
serialization/deserialization format of curve and field elements

Co-authored-by: Jonathan Wang <jonathanpwang@users.noreply.github.com>
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.

Refactor proofs to not require length encodings
5 participants