fix: tests and benches (#17)

Recent performance improvements require additional send + sync
constraints on rng amongst other things, this update fills in those
requirements for tests !

.github/workflows/lints-stable.yml

@@ -5,7 +5,7 @@ on: pull_request
 
 jobs:
   clippy:
-    name: Clippy (1.56.1)
+    name: Clippy 
     timeout-minutes: 30
     runs-on: ubuntu-latest
 
@@ -18,6 +18,6 @@ jobs:
       - name: Run clippy
         uses: actions-rs/clippy-check@v1
         with:
-          name: Clippy (1.56.1)
+          name: Clippy 
           token: ${{ secrets.GITHUB_TOKEN }}
           args: --all-features --all-targets -- -D warnings

halo2_gadgets/src/lib.rs

@@ -16,7 +16,7 @@
 
 #![cfg_attr(docsrs, feature(doc_cfg))]
 // Temporary until we have more of the crate implemented.
-#![allow(dead_code)]
+#![allow(dead_code, clippy::single_range_in_vec_init)]
 // Catch documentation errors caused by code changes.
 #![deny(rustdoc::broken_intra_doc_links)]
 #![deny(missing_debug_implementations)]

halo2_proofs/Cargo.toml

@@ -85,6 +85,7 @@ gumdrop = "0.8"
 proptest = "1"
 rand_core = { version = "0.6", default-features = false, features = ["getrandom"] }
 serde_json = "1"
+ark-std = "0.4.0"
 
 [target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dev-dependencies]
 getrandom = { version = "0.2", features = ["js"] }
@@ -112,6 +113,7 @@ cost-estimator = ["serde_derive"]
 derive_serde = ["halo2curves/derive_serde"]
 parallel-poly-read = []
 precompute-coset = []
+multicore = []
 
 [lib]
 bench = false

halo2_proofs/benches/arithmetic.rs

@@ -1,7 +1,7 @@
 #[macro_use]
 extern crate criterion;
 
-use crate::arithmetic::small_multiexp;
+use crate::arithmetic::best_multiexp_cpu;
 use crate::halo2curves::pasta::{EqAffine, Fp};
 use group::ff::Field;
 use halo2_proofs::*;
@@ -27,7 +27,7 @@ fn criterion_benchmark(c: &mut Criterion) {
         c.bench_function("double-and-add", |b| {
             b.iter(|| {
                 for (g_lo, g_hi) in g_lo.iter().zip(g_hi.iter()) {
-                    small_multiexp(&[black_box(coeff_1), black_box(coeff_2)], &[*g_lo, *g_hi]);
+                    best_multiexp_cpu(&[black_box(coeff_1), black_box(coeff_2)], &[*g_lo, *g_hi]);
                 }
             })
         });

halo2_proofs/benches/fft.rs

@@ -7,16 +7,24 @@ use halo2_proofs::*;
 use halo2curves::pasta::Fp;
 
 use criterion::{BenchmarkId, Criterion};
+use poly::EvaluationDomain;
 use rand_core::OsRng;
 
 fn criterion_benchmark(c: &mut Criterion) {
     let mut group = c.benchmark_group("fft");
     for k in 3..19 {
         group.bench_function(BenchmarkId::new("k", k), |b| {
-            let mut a = (0..(1 << k)).map(|_| Fp::random(OsRng)).collect::<Vec<_>>();
+            let domain = EvaluationDomain::<Fp>::new(1, k);
+            let n = domain.get_n() as usize;
+
+            let input = vec![Fp::random(OsRng); n];
+
+            let mut a = input.clone();
+            let l_a = a.len();
+
             let omega = Fp::random(OsRng); // would be weird if this mattered
             b.iter(|| {
-                best_fft(&mut a, omega, k as u32);
+                best_fft(&mut a, omega, k, domain.get_fft_data(l_a), false);
             });
         });
     }

halo2_proofs/benches/plonk.rs

@@ -29,6 +29,7 @@ use criterion::{BenchmarkId, Criterion};
 fn criterion_benchmark(c: &mut Criterion) {
     /// This represents an advice column at a certain row in the ConstraintSystem
     #[derive(Copy, Clone, Debug)]
+    #[allow(dead_code)]
     pub struct Variable(Column<Advice>, usize);
 
     #[derive(Clone)]

halo2_proofs/examples/simple-lookup-unblinded.rs

@@ -1,4 +1,3 @@
-#[macro_use]
 extern crate criterion;
 
 use ff::PrimeField;

halo2_proofs/examples/simple-lookup.rs

@@ -1,4 +1,3 @@
-#[macro_use]
 extern crate criterion;
 
 use ff::PrimeField;

halo2_proofs/src/circuit/floor_planner/v1/strategy.rs

@@ -217,16 +217,8 @@ pub fn slot_in_biggest_advice_first(
     // We now use `sort_by_cached_key` with non-unique keys, and rely on `region_shapes`
     // being sorted by region index (which we also rely on below to return `RegionStart`s
     // in the correct order).
-    #[cfg(not(feature = "floor-planner-v1-legacy-pdqsort"))]
     sorted_regions.sort_by_cached_key(sort_key);
 
-    // To preserve compatibility, when the "floor-planner-v1-legacy-pdqsort" feature is enabled,
-    // we use a copy of the pdqsort implementation from the Rust 1.56.1 standard library, fixed
-    // to its behaviour on 64-bit platforms.
-    // https://github.com/rust-lang/rust/blob/1.56.1/library/core/src/slice/mod.rs#L2365-L2402
-    #[cfg(feature = "floor-planner-v1-legacy-pdqsort")]
-    halo2_legacy_pdqsort::sort::quicksort(&mut sorted_regions, |a, b| sort_key(a).lt(&sort_key(b)));
-
     sorted_regions.reverse();
 
     // Lay out the sorted regions.

halo2_proofs/src/dev.rs

@@ -22,9 +22,11 @@ use crate::{
 
 use maybe_rayon::prelude::{
     IndexedParallelIterator, IntoParallelIterator, IntoParallelRefIterator, ParallelIterator,
-    ParallelSliceMut,
 };
 
+#[cfg(not(feature = "mv-lookup"))]
+use maybe_rayon::prelude::ParallelSliceMut;
+
 pub mod metadata;
 use metadata::Column as ColumnMetadata;
 mod util;

halo2_proofs/src/fft.rs

@@ -42,7 +42,7 @@ mod tests {
 
         let input = vec![Scalar::random(OsRng); n];
 
-        let num_threads = multicore::current_num_threads();
+        let _num_threads = multicore::current_num_threads();
 
         let mut a = input.clone();
         let l_a = a.len();

halo2_proofs/src/helpers.rs

@@ -89,7 +89,7 @@ pub trait SerdePrimeField: PrimeField + SerdeObject {
 
     /// Writes a field element as bytes to the buffer according to the `format`:
     /// - `Processed`: Writes a field element in standard form, with endianness specified by the
-    /// `PrimeField` implementation.
+    ///  `PrimeField` implementation.
     /// - Otherwise: Writes a field element into raw bytes in its internal Montgomery representation,
     /// WITHOUT performing the expensive Montgomery reduction.
     fn write<W: io::Write>(&self, writer: &mut W, format: SerdeFormat) -> io::Result<()> {

halo2_proofs/src/lib.rs

@@ -2,7 +2,13 @@
 
 #![cfg_attr(docsrs, feature(doc_cfg))]
 // The actual lints we want to disable.
-#![allow(clippy::op_ref, clippy::many_single_char_names)]
+#![allow(
+    clippy::op_ref,
+    clippy::many_single_char_names,
+    clippy::empty_docs,
+    clippy::doc_lazy_continuation,
+    clippy::single_range_in_vec_init
+)]
 #![deny(rustdoc::broken_intra_doc_links)]
 #![deny(missing_debug_implementations)]
 #![deny(missing_docs)]

halo2_proofs/src/plonk.rs

@@ -25,7 +25,9 @@ mod circuit;
 mod error;
 mod evaluation;
 mod keygen;
+#[cfg(not(feature = "mv-lookup"))]
 mod lookup;
+#[cfg(feature = "mv-lookup")]
 mod mv_lookup;
 pub mod permutation;
 mod shuffle;

halo2_proofs/src/plonk/circuit.rs

@@ -1374,11 +1374,6 @@ impl<F: Field> Product<Self> for Expression<F> {
     }
 }
 
-/// Represents an index into a vector where each entry corresponds to a distinct
-/// point that polynomials are queried at.
-#[derive(Copy, Clone, Debug)]
-pub(crate) struct PointIndex(pub usize);
-
 /// A "virtual cell" is a PLONK cell that has been queried at a particular relative offset
 /// within a custom gate.
 #[derive(Clone, Debug)]
@@ -1554,6 +1549,7 @@ impl<F: Field> Gate<F> {
 
 /// TODO doc
 #[derive(Debug, Clone)]
+#[allow(dead_code)]
 pub struct LookupTracker<F: Field> {
     pub(crate) table: Vec<Expression<F>>,
     pub(crate) inputs: Vec<Vec<Expression<F>>>,

halo2_proofs/src/plonk/evaluation.rs

@@ -15,6 +15,7 @@ use crate::{
 };
 
 use group::ff::{Field, PrimeField, WithSmallOrderMulGroup};
+#[cfg(feature = "mv-lookup")]
 use maybe_rayon::iter::IndexedParallelIterator;
 use maybe_rayon::iter::IntoParallelRefIterator;
 use maybe_rayon::iter::ParallelIterator;

halo2_proofs/src/plonk/lookup/prover.rs

@@ -45,6 +45,7 @@ pub(in crate::plonk) struct Permuted<C: CurveAffine> {
 }
 
 #[derive(Debug)]
+#[allow(dead_code)]
 pub(in crate::plonk) struct Committed<C: CurveAffine> {
     pub(in crate::plonk) permuted_input_poly: Polynomial<C::Scalar, Coeff>,
     permuted_input_blind: Blind<C::Scalar>,
@@ -56,6 +57,7 @@ pub(in crate::plonk) struct Committed<C: CurveAffine> {
 }
 
 impl<C: SerdeCurveAffine> Committed<C> {
+    #[allow(dead_code)]
     pub fn write<W: std::io::Write>(
         &self,
         writer: &mut W,
@@ -74,6 +76,7 @@ impl<C: SerdeCurveAffine> Committed<C> {
         self.commitment.write(writer, format)
     }
 
+    #[allow(dead_code)]
     pub fn read<R: std::io::Read>(reader: &mut R, format: SerdeFormat) -> std::io::Result<Self>
     where
         <C as CurveAffine>::ScalarExt: crate::helpers::SerdePrimeField,

halo2_proofs/src/plonk/mv_lookup/prover.rs

@@ -43,6 +43,7 @@ pub(in crate::plonk) struct Committed<C: CurveAffine> {
 }
 
 impl<C: SerdeCurveAffine> Committed<C> {
+    #[allow(dead_code)]
     pub fn write<W: std::io::Write>(
         &self,
         writer: &mut W,
@@ -56,6 +57,7 @@ impl<C: SerdeCurveAffine> Committed<C> {
         self.commitment.write(writer, format)
     }
 
+    #[allow(dead_code)]
     pub fn read<R: std::io::Read>(reader: &mut R, format: SerdeFormat) -> std::io::Result<Self>
     where
         <C as CurveAffine>::ScalarExt: crate::helpers::SerdePrimeField,

halo2_proofs/src/plonk/prover.rs

@@ -1,7 +1,5 @@
 use ff::{Field, FromUniformBytes, WithSmallOrderMulGroup};
 use group::Curve;
-use halo2curves::serde::SerdeObject;
-
 use instant::Instant;
 use rand_core::RngCore;
 use rustc_hash::FxBuildHasher;
@@ -20,18 +18,16 @@ use super::{
     permutation, shuffle, vanishing, ChallengeBeta, ChallengeGamma, ChallengeTheta, ChallengeX,
     ChallengeY, Error, ProvingKey,
 };
-use maybe_rayon::iter::IndexedParallelIterator;
-use maybe_rayon::iter::ParallelIterator;
+#[cfg(feature = "mv-lookup")]
+use maybe_rayon::iter::{IndexedParallelIterator, ParallelIterator};
 
 #[cfg(not(feature = "mv-lookup"))]
 use super::lookup;
 #[cfg(feature = "mv-lookup")]
 use super::mv_lookup as lookup;
 
 #[cfg(feature = "mv-lookup")]
-use maybe_rayon::iter::{
-    IntoParallelIterator, IntoParallelRefIterator, IntoParallelRefMutIterator,
-};
+use maybe_rayon::iter::{IntoParallelIterator, IntoParallelRefIterator};
 
 use crate::{
     arithmetic::{eval_polynomial, CurveAffine},
@@ -69,8 +65,7 @@ pub fn create_proof<
     transcript: &mut T,
 ) -> Result<(), Error>
 where
-    Scheme::Scalar: WithSmallOrderMulGroup<3> + FromUniformBytes<64> + SerdeObject,
-    Scheme::Curve: SerdeObject,
+    Scheme::Scalar: WithSmallOrderMulGroup<3> + FromUniformBytes<64>,
     Scheme::ParamsProver: Send + Sync,
 {
     #[cfg(feature = "counter")]
@@ -498,11 +493,13 @@ where
         .collect::<Result<Vec<_>, _>>()?;
 
     #[cfg(feature = "mv-lookup")]
-    lookups.iter().for_each(|lookups| {
-        lookups.iter().for_each(|lookup| {
-            transcript.write_point(lookup.commitment);
-        });
-    });
+    {
+        for lookups_ in &lookups {
+            for lookup in lookups_.iter() {
+                transcript.write_point(lookup.commitment)?;
+            }
+        }
+    }
 
     #[cfg(not(feature = "mv-lookup"))]
     let lookups: Vec<Vec<lookup::prover::Permuted<Scheme::Curve>>> = instance
@@ -603,11 +600,13 @@ where
     let lookups = commit_lookups()?;
 
     #[cfg(feature = "mv-lookup")]
-    lookups.iter().for_each(|lookups| {
-        lookups.iter().for_each(|lookup| {
-            transcript.write_point(lookup.commitment);
-        });
-    });
+    {
+        for lookups_ in &lookups {
+            for lookup in lookups_.iter() {
+                transcript.write_point(lookup.commitment)?;
+            }
+        }
+    }
 
     log::trace!("Lookup commitment: {:?}", start.elapsed());
 

halo2_proofs/src/poly/commitment.rs

@@ -199,7 +199,8 @@ impl<F: Field> Default for Blind<F> {
 }
 
 impl<F: Field + SerdePrimeField> Blind<F> {
-    /// Writes polynomial to buffer using `SerdePrimeField::write`.  
+    /// Writes blind to buffer using `SerdePrimeField::write`.  
+    #[allow(dead_code)]
     pub(crate) fn write<W: io::Write>(
         &self,
         writer: &mut W,
@@ -209,7 +210,8 @@ impl<F: Field + SerdePrimeField> Blind<F> {
         Ok(())
     }
 
-    /// Reads polynomial from buffer using `SerdePrimeField::read`.
+    /// Reads blind from buffer using `SerdePrimeField::read`.
+    #[allow(dead_code)]
     pub(crate) fn read<R: io::Read>(
         reader: &mut R,
         format: crate::SerdeFormat,

halo2_proofs/src/poly/domain.rs

@@ -597,7 +597,7 @@ impl<F: WithSmallOrderMulGroup<3>> EvaluationDomain<F> {
         });
     }
 
-    fn ifft(&self, a: &mut Vec<F>, omega_inv: F, log_n: u32, divisor: F) {
+    fn ifft(&self, a: &mut [F], omega_inv: F, log_n: u32, divisor: F) {
         let fft_data = self.get_fft_data(a.len());
         crate::fft::parallel::fft(a, omega_inv, log_n, fft_data, true);
         // self.fft_inner(a, omega_inv, log_n, true);
@@ -609,7 +609,7 @@ impl<F: WithSmallOrderMulGroup<3>> EvaluationDomain<F> {
         });
     }
 
-    fn fft_inner(&self, a: &mut Vec<F>, omega: F, log_n: u32, inverse: bool) {
+    fn fft_inner(&self, a: &mut [F], omega: F, log_n: u32, inverse: bool) {
         let fft_data = self.get_fft_data(a.len());
         best_fft(a, omega, log_n, fft_data, inverse)
     }
@@ -846,8 +846,8 @@ fn test_coeff_to_extended_part() {
 #[test]
 fn bench_coeff_to_extended_parts() {
     use halo2curves::pasta::pallas::Scalar;
-    use rand_core::OsRng;
     use instant::Instant;
+    use rand_core::OsRng;
 
     let k = 20;
     let domain = EvaluationDomain::<Scalar>::new(3, k);
@@ -934,8 +934,8 @@ fn test_lagrange_vecs_to_extended() {
 #[test]
 fn bench_lagrange_vecs_to_extended() {
     use halo2curves::pasta::pallas::Scalar;
-    use rand_core::OsRng;
     use instant::Instant;
+    use rand_core::OsRng;
 
     let rng = OsRng;
     let domain = EvaluationDomain::<Scalar>::new(8, 10);