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

Reorder R1CS constraints + CompactPolynomial refactor #561

Merged
merged 40 commits into from
Jan 17, 2025

Conversation

moodlezoup
Copy link
Collaborator

@moodlezoup moodlezoup commented Jan 13, 2025

This is a stack of two separate changes:

  1. Reorder R1CS constraints #554 by @jprider63, which reorders Jolt's R1CS witnesses to be in "constraint-major" order (rather than "step-major" order). This is incremental progress toward a "streaming" Spartan prover algorithm.
  2. A significant overhaul/optimization of Jolt's data structures for multilinear polynomials, described in further detail below.

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 the CompactPolynomial 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; the SpartanInterleavedPolynomial refactor brings the time back to parity with main. The polynomial implementation itself is sort of a combination of the old SparsePolynomial and the SparseInterleavedPolynomial used sparse grand products.

Large(usize),
}

impl MsmType {
Copy link
Contributor

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.

Copy link
Collaborator Author

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

@moodlezoup moodlezoup force-pushed the feat/compact-polys branch 2 times, most recently from b20cc1e to 8bca301 Compare January 15, 2025 16:49
Comment on lines -89 to -96
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;
Copy link
Collaborator Author

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

@moodlezoup moodlezoup changed the title Feat/compact polys Reorder R1CS constraints + CompactPolynomial refactor Jan 16, 2025
@moodlezoup moodlezoup marked this pull request as ready for review January 16, 2025 22:11
@moodlezoup moodlezoup merged commit b8d6f0a into main Jan 17, 2025
10 checks passed
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.

3 participants