-
Notifications
You must be signed in to change notification settings - Fork 191
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
Reorder R1CS constraints + CompactPolynomial refactor #561
Conversation
3b03d67
to
36ed25c
Compare
Large(usize), | ||
} | ||
|
||
impl MsmType { |
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 thought having this around made it easier to read/maintain.
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 removed it because it gets weird with CompactPolynomial
–– the Medium
size straddles the u8
and u16
size
4993457
to
08a2e79
Compare
b20cc1e
to
8bca301
Compare
8bca301
to
c691c63
Compare
fn batch_prove( | ||
setup: &Self::Setup, | ||
polynomials: &[&DensePolynomial<Self::Field>], | ||
opening_point: &[Self::Field], | ||
openings: &[Self::Field], | ||
batch_type: BatchType, | ||
transcript: &mut ProofTranscript, | ||
) -> Self::BatchedProof; |
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.
Deleting batch_prove
and batch_verify
from trait because we now use the batched opening proof protocol in opening_proof.rs
instead
53fe211
to
b72983c
Compare
This is a stack of two separate changes:
The main observation behind the optimizations/refactor is that most of Jolt's polynomials have small (relative to a 256-bit field) coefficients. Before, ~all polynomials were represented by the
DensePolynomial
struct, which uses a field element per coefficient. With this PR we introduce theCompactPolynomial
struct, which instead uses Rust's primitive integer types (u8
,u16
, etc.) to represent coefficients.This reduces prover memory usage and leads to some speed improvements as well. Note that once a CompactPolynomial is bound, its coefficients will need to be represented as field elements (but there will be half as many as there are unbound coefficients).
A side effect of representing coefficients as primitive integers is that we need some tricks to avoid spending too much time converting the coefficients to Montgomery form when doing (field) arithmetic with them. These new tricks are described in the changes to
ark.rs
The final main change in this PR is the introduction of
SpartanInterleavedPolynomial
, which is used to represent the Az, Bz, and Cz polynomials used in the first Spartan sumcheck. The R1CS reordering changes slowed down the computation of Az, Bz, and Cz; theSpartanInterleavedPolynomial
refactor brings the time back to parity with main. The polynomial implementation itself is sort of a combination of the oldSparsePolynomial
and theSparseInterleavedPolynomial
used sparse grand products.