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

Check base field #55

Merged
merged 12 commits into from
Jun 2, 2021
Merged

Check base field #55

merged 12 commits into from
Jun 2, 2021

Conversation

wborgeaud
Copy link
Contributor

  • Check that wires are in the base field when working over an extension field.
  • Open Zs polynomials at zeta and g*zeta.
  • Rewrite of the ListPolynomialCommitment API to be more PLONK specific.

@wborgeaud wborgeaud requested a review from dlubarov June 1, 2021 09:34
@@ -47,135 +47,3 @@ fn fri_l(codeword_len: usize, rate_log: usize, conjecture: bool) -> f64 {
1.0 / (2.0 * EPSILON * rate.sqrt())
}
}

#[cfg(test)]
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 a bit annoying having to remove these tests, but since the FRI API is now very PLONK specific it doesn't make sense to update these tests since they'll end up being the same as those in commitment.rs.
If we need FRI for something other that polynomial commitments in the future, we could add some kind of FriInitialData trait and make the FRI API more general.

@@ -142,29 +143,84 @@ fn fri_verify_initial_proof<F: Field>(
fn fri_combine_initial<F: Field + Extendable<D>, const D: usize>(
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel this is getting pretty complicated; I tried to refactor a bit here, though with limited success. I guess there's only so much we can do.

  • I tried using the powers() iterator to supply all the powers of alpha, instead of tracking cur_alpha and poly_count. Since (currently) we multiply a few things by the same power of alpha, I had to clone the iterator in a few places.
  • Added an unsalted_evals helper
  • Converted subgroup_x once at the beginning
  • Added a comment

What do you think? Feel free to pick any changes you like and ignore the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tweaks, they're all great! I've merged them all.

@wborgeaud wborgeaud merged commit c24ad60 into main Jun 2, 2021
@wborgeaud wborgeaud deleted the check_base_field branch June 2, 2021 07:59
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.

2 participants