From d0e45a0992a613fdd7deee9c314760e4a246b1e6 Mon Sep 17 00:00:00 2001 From: diegokingston Date: Thu, 21 May 2026 16:30:34 -0300 Subject: [PATCH] cleanup(stark/constraints): drop coefficient-form leftovers, dedupe eval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behaviour-preserving cleanup of `crypto/stark/src/constraints` (plan items 1-5). The eval-form STARK migration left several pieces of coefficient-form scaffolding behind. boundary.rs — remove vestigial pre-eval-form code - The eval-form prover evaluates boundaries on-the-fly in `evaluator.rs` (`trace[col] - value`); it never interpolates boundary polynomials. Deleted the now-unused `steps`, `steps_for_boundary`, `cols_for_boundary`, `generate_roots_of_unity`, `values`, `compute_zerofier`, and the dead `new_simple_aux`. Production keeps only the `BoundaryConstraint` / `BoundaryConstraints` structs, `new_main`/`new_aux`/`new_simple_main` and `from_constraints`. 153 -> 57 LOC; drops the `itertools` use. - `tests/boundary_tests.rs` only exercised `compute_zerofier`; removed. evaluator.rs — remove a no-op debug check - `check_boundary_polys_divisibility` was called with two always-empty vecs, so it iterated nothing — a check that checked nothing. Removed the dead `#[cfg(debug_assertions)]` block (`boundary_polys`, `boundary_zerofiers`, the unused `_transition_evaluations`), the function from `debug.rs` (no other caller), and the imports it needed. This also clears the `unused import: Polynomial` warning in release builds. evaluator.rs — deduplicate `evaluate_transitions` - The parallel and sequential arms repeated ~32 lines of identical per-row accumulation. Extracted a shared `eval_row` closure; only the iterator (`into_par_iter().map_init` vs `into_iter().map`) now differs. Per-thread buffer reuse is unchanged. transition.rs — hygiene - `evaluate_zerofier` used `unsafe { …unwrap_unchecked() }` while the sibling branch used a safe `.unwrap()` on the same invariant. Replaced with `.expect(...)` — no `unsafe` for skipping one zero-check. - `std::ops::Div` / `std::iter::zip` -> `core::` to match the module. 123 stark lib tests pass (124 minus the deleted compute_zerofier test); 273 prover lib tests pass; all build configs + lint clean. --- crypto/stark/src/constraints/boundary.rs | 109 +----------- crypto/stark/src/constraints/evaluator.rs | 187 ++++++++------------- crypto/stark/src/constraints/transition.rs | 12 +- crypto/stark/src/debug.rs | 16 -- crypto/stark/src/tests/boundary_tests.rs | 36 ---- crypto/stark/src/tests/mod.rs | 1 - 6 files changed, 84 insertions(+), 277 deletions(-) delete mode 100644 crypto/stark/src/tests/boundary_tests.rs diff --git a/crypto/stark/src/constraints/boundary.rs b/crypto/stark/src/constraints/boundary.rs index 6030e3dfe..b34b6afec 100644 --- a/crypto/stark/src/constraints/boundary.rs +++ b/crypto/stark/src/constraints/boundary.rs @@ -1,15 +1,10 @@ -use itertools::Itertools; -use math::{ - field::{element::FieldElement, traits::IsField}, - polynomial::Polynomial, -}; +use math::field::{element::FieldElement, traits::IsField}; -#[derive(Debug)] -/// Represents a boundary constraint that must hold in an execution -/// trace: +/// Represents a boundary constraint that must hold in an execution trace: /// * col: The column of the trace where the constraint must hold /// * step: The step (or row) of the trace where the constraint must hold /// * value: The value the constraint must have in that column and step +#[derive(Debug)] pub struct BoundaryConstraint { pub col: usize, pub step: usize, @@ -36,7 +31,7 @@ impl BoundaryConstraint { } } - /// Used for creating boundary constraints for a trace with only one column + /// Boundary constraint for a trace with a single main column. pub fn new_simple_main(step: usize, value: FieldElement) -> Self { Self { col: 0, @@ -45,109 +40,17 @@ impl BoundaryConstraint { is_aux: false, } } - - /// Used for creating boundary constraints for a trace with only one column - pub fn new_simple_aux(step: usize, value: FieldElement) -> Self { - Self { - col: 0, - step, - value, - is_aux: true, - } - } } -/// Data structure that stores all the boundary constraints that must -/// hold for the execution trace +/// All the boundary constraints that must hold for an execution trace. #[derive(Default, Debug)] pub struct BoundaryConstraints { pub constraints: Vec>, } impl BoundaryConstraints { - /// To instantiate from a vector of BoundaryConstraint elements + /// Instantiate from a vector of `BoundaryConstraint` elements. pub fn from_constraints(constraints: Vec>) -> Self { Self { constraints } } - - /// Returns all the steps where boundary conditions exist for the given column - pub fn steps(&self, col: usize) -> Vec { - self.constraints - .iter() - .filter(|v| v.col == col) - .map(|c| c.step) - .collect() - } - - pub fn steps_for_boundary(&self) -> Vec { - self.constraints - .iter() - .unique_by(|elem| elem.step) - .map(|v| v.step) - .collect() - } - - pub fn cols_for_boundary(&self) -> Vec { - self.constraints - .iter() - .unique_by(|elem| elem.col) - .map(|v| v.col) - .collect() - } - - /// Given the primitive root of some domain, returns the domain values corresponding - /// to the steps where the boundary conditions hold. This is useful when interpolating - /// the boundary conditions, since we must know the x values - pub fn generate_roots_of_unity( - &self, - primitive_root: &FieldElement, - cols_trace: &[usize], - ) -> Vec>> { - cols_trace - .iter() - .map(|i| { - self.steps(*i) - .into_iter() - .map(|s| primitive_root.pow(s)) - .collect::>>() - }) - .collect::>>>() - } - - /// For every trace column, give all the values the trace must be equal to in - /// the steps where the boundary constraints hold - pub fn values(&self, cols_trace: &[usize]) -> Vec>> { - cols_trace - .iter() - .map(|i| { - self.constraints - .iter() - .filter(|c| c.col == *i) - .map(|c| c.value.clone()) - .collect() - }) - .collect() - } - - /// Computes the zerofier of the boundary quotient. The result is the - /// multiplication of each binomial that evaluates to zero in the domain - /// values where the boundary constraints must hold. - /// - /// Example: If there are boundary conditions in the third and fifth steps, - /// then the zerofier will be (x - w^3) * (x - w^5) - pub fn compute_zerofier( - &self, - primitive_root: &FieldElement, - col: usize, - ) -> Polynomial> { - self.steps(col).into_iter().fold( - Polynomial::new_monomial(FieldElement::::one(), 0), - |zerofier, step| { - let binomial = - Polynomial::new(&[-primitive_root.pow(step), FieldElement::::one()]); - // TODO: Implement the MulAssign trait for Polynomials? - zerofier * binomial - }, - ) - } } diff --git a/crypto/stark/src/constraints/evaluator.rs b/crypto/stark/src/constraints/evaluator.rs index 39d9af2a0..6e94473b7 100644 --- a/crypto/stark/src/constraints/evaluator.rs +++ b/crypto/stark/src/constraints/evaluator.rs @@ -1,14 +1,10 @@ use super::boundary::BoundaryConstraints; -#[cfg(all(debug_assertions, not(feature = "parallel")))] -use crate::debug::check_boundary_polys_divisibility; use crate::domain::Domain; use crate::lookup::{BusPublicInputs, LOGUP_CHALLENGE_ALPHA, PackingShifts, compute_alpha_powers}; use crate::trace::LDETraceTable; use crate::traits::{AIR, TransitionEvaluationContext, ZerofierEvaluations}; use crate::{frame::Frame, prover::evaluate_polynomial_on_lde_domain}; use math::field::traits::{IsFFTField, IsField, IsSubFieldOf}; -#[cfg(not(feature = "parallel"))] -use math::polynomial::Polynomial; use math::{fft::errors::FFTError, field::element::FieldElement}; #[cfg(feature = "parallel")] use rayon::{ @@ -78,9 +74,69 @@ where let num_aux_cols = lde_trace.num_aux_cols(); let num_offsets = offsets.len(); + // Per-row evaluation, shared by the parallel and sequential paths below: + // fill the frame, evaluate transition constraints, accumulate with zerofiers. + let eval_row = |i: usize, + boundary: FieldElement, + transition_buf: &mut [FieldElement], + base_buf: &mut [FieldElement], + periodic_buf: &mut [FieldElement], + frame: &mut Frame| + -> FieldElement { + frame.fill_from_lde(lde_trace, i, offsets); + + for (j, col) in lde_periodic_columns.iter().enumerate() { + periodic_buf[j] = col[i].clone(); + } + + let ctx = TransitionEvaluationContext::new_prover( + frame, + periodic_buf, + rap_challenges, + &logup_alpha_powers, + logup_table_offset, + &packing_shifts, + ); + air.compute_transition_prover(&ctx, base_buf, transition_buf); + + let acc_transition = if is_uniform { + // All constraints share one zerofier: factor it out of the sum. + let z = zerofier_data.get_uniform(i); + // F×E inner product for base constraints (3 muls per term) + let mut sum = base_buf + .iter() + .zip(&transition_coefficients[..num_base]) + .fold(FieldElement::zero(), |acc, (eval, beta)| acc + eval * beta); + // E×E for extension constraints (9 muls per term) + sum = transition_buf[num_base..] + .iter() + .zip(&transition_coefficients[num_base..]) + .fold(sum, |acc, (eval, beta)| acc + eval * beta); + z * &sum + } else { + let mut sum = base_buf + .iter() + .enumerate() + .zip(&transition_coefficients[..num_base]) + .fold(FieldElement::zero(), |acc, ((c_idx, eval), beta)| { + acc + zerofier_data.get(c_idx, i) * eval * beta + }); + sum = transition_buf[num_base..] + .iter() + .enumerate() + .zip(&transition_coefficients[num_base..]) + .fold(sum, |acc, ((j, eval), beta)| { + acc + zerofier_data.get(num_base + j, i) * eval * beta + }); + sum + }; + + acc_transition + boundary + }; + #[cfg(feature = "parallel")] { - let evaluations_t: Vec<_> = boundary_evaluation + boundary_evaluation .into_par_iter() .enumerate() .map_init( @@ -98,59 +154,10 @@ where ) }, |(transition_buf, base_buf, periodic_buf, frame), (i, boundary)| { - frame.fill_from_lde(lde_trace, i, offsets); - - for (j, col) in lde_periodic_columns.iter().enumerate() { - periodic_buf[j] = col[i].clone(); - } - - let ctx = TransitionEvaluationContext::new_prover( - frame, - periodic_buf, - rap_challenges, - &logup_alpha_powers, - logup_table_offset, - &packing_shifts, - ); - air.compute_transition_prover(&ctx, base_buf, transition_buf); - - let acc_transition = if is_uniform { - // All constraints share one zerofier: factor it out of the sum. - let z = zerofier_data.get_uniform(i); - // F×E inner product for base constraints (3 muls per term) - let mut sum = base_buf - .iter() - .zip(&transition_coefficients[..num_base]) - .fold(FieldElement::zero(), |acc, (eval, beta)| acc + eval * beta); - // E×E for extension constraints (9 muls per term) - sum = transition_buf[num_base..] - .iter() - .zip(&transition_coefficients[num_base..]) - .fold(sum, |acc, (eval, beta)| acc + eval * beta); - z * &sum - } else { - let mut sum = base_buf - .iter() - .enumerate() - .zip(&transition_coefficients[..num_base]) - .fold(FieldElement::zero(), |acc, ((c_idx, eval), beta)| { - acc + zerofier_data.get(c_idx, i) * eval * beta - }); - sum = transition_buf[num_base..] - .iter() - .enumerate() - .zip(&transition_coefficients[num_base..]) - .fold(sum, |acc, ((j, eval), beta)| { - acc + zerofier_data.get(num_base + j, i) * eval * beta - }); - sum - }; - - acc_transition + boundary + eval_row(i, boundary, transition_buf, base_buf, periodic_buf, frame) }, ) - .collect(); - evaluations_t + .collect() } #[cfg(not(feature = "parallel"))] @@ -165,54 +172,14 @@ where .into_iter() .enumerate() .map(|(i, boundary)| { - frame.fill_from_lde(lde_trace, i, offsets); - - for (j, col) in lde_periodic_columns.iter().enumerate() { - periodic_buf[j] = col[i].clone(); - } - - let ctx = TransitionEvaluationContext::new_prover( - &frame, - &periodic_buf, - rap_challenges, - &logup_alpha_powers, - logup_table_offset, - &packing_shifts, - ); - air.compute_transition_prover(&ctx, &mut base_buf, &mut transition_buf); - - let acc_transition = if is_uniform { - let z = zerofier_data.get_uniform(i); - // F×E inner product for base constraints (3 muls per term) - let mut sum = base_buf - .iter() - .zip(&transition_coefficients[..num_base]) - .fold(FieldElement::zero(), |acc, (eval, beta)| acc + eval * beta); - // E×E for extension constraints (9 muls per term) - sum = transition_buf[num_base..] - .iter() - .zip(&transition_coefficients[num_base..]) - .fold(sum, |acc, (eval, beta)| acc + eval * beta); - z * &sum - } else { - let mut sum = base_buf - .iter() - .enumerate() - .zip(&transition_coefficients[..num_base]) - .fold(FieldElement::zero(), |acc, ((c_idx, eval), beta)| { - acc + zerofier_data.get(c_idx, i) * eval * beta - }); - sum = transition_buf[num_base..] - .iter() - .enumerate() - .zip(&transition_coefficients[num_base..]) - .fold(sum, |acc, ((j, eval), beta)| { - acc + zerofier_data.get(num_base + j, i) * eval * beta - }); - sum - }; - - acc_transition + boundary + eval_row( + i, + boundary, + &mut transition_buf, + &mut base_buf, + &mut periodic_buf, + &mut frame, + ) }) .collect() } @@ -280,9 +247,6 @@ where }) .collect::>>>(); - #[cfg(all(debug_assertions, not(feature = "parallel")))] - let boundary_polys: Vec>> = Vec::new(); - let trace_length = domain.interpolation_domain_size; let lde_periodic_columns = air .get_periodic_column_polynomials(trace_length) @@ -327,15 +291,6 @@ where }) .collect(); - #[cfg(all(debug_assertions, not(feature = "parallel")))] - let boundary_zerofiers = Vec::new(); - - #[cfg(all(debug_assertions, not(feature = "parallel")))] - check_boundary_polys_divisibility(boundary_polys, boundary_zerofiers); - - #[cfg(all(debug_assertions, not(feature = "parallel")))] - let _transition_evaluations: Vec> = Vec::new(); - let zerofier_data = air.transition_zerofier_evaluations_grouped(domain); // Iterate over all LDE domain and compute the part of the composition polynomial diff --git a/crypto/stark/src/constraints/transition.rs b/crypto/stark/src/constraints/transition.rs index 5fbae50a0..af86ed7e6 100644 --- a/crypto/stark/src/constraints/transition.rs +++ b/crypto/stark/src/constraints/transition.rs @@ -1,4 +1,4 @@ -use std::ops::Div; +use core::ops::Div; use crate::domain::Domain; use crate::prover::evaluate_polynomial_on_lde_domain; @@ -205,7 +205,7 @@ where .cycle() .take(end_exemption_evaluations.len()); - std::iter::zip(cycled_evaluations, end_exemption_evaluations) + core::iter::zip(cycled_evaluations, end_exemption_evaluations) .map(|(eval, exemption_eval)| eval * exemption_eval) .collect() @@ -246,7 +246,7 @@ where .cycle() .take(end_exemption_evaluations.len()); - std::iter::zip(cycled_evaluations, end_exemption_evaluations) + core::iter::zip(cycled_evaluations, end_exemption_evaluations) .map(|(eval, exemption_eval)| eval * exemption_eval) .collect() } @@ -276,8 +276,10 @@ where let denominator = -trace_primitive_root .pow(self.offset() * trace_length / self.period()) + z.pow(trace_length / self.period()); - // The denominator isn't zero because z is sampled outside the set of primitive roots. - return unsafe { numerator.div(denominator).unwrap_unchecked() } + // The denominator is non-zero: z is sampled outside the set of primitive roots. + return numerator + .div(denominator) + .expect("zerofier denominator is non-zero: z is sampled out-of-domain") * end_exemptions_poly.evaluate(z); } diff --git a/crypto/stark/src/debug.rs b/crypto/stark/src/debug.rs index 906b8f317..d62557b88 100644 --- a/crypto/stark/src/debug.rs +++ b/crypto/stark/src/debug.rs @@ -152,22 +152,6 @@ pub fn validate_trace< ret } -pub fn check_boundary_polys_divisibility( - boundary_polys: Vec>>, - boundary_zerofiers: Vec>>, -) { - for (i, (poly, z)) in boundary_polys - .iter() - .zip(boundary_zerofiers.iter()) - .enumerate() - { - let (_, b) = poly.clone().long_division_with_remainder(z); - if b != Polynomial::zero() { - error!("Boundary poly {i} is not divisible by its zerofier"); - } - } -} - /// Validates that the one-dimensional array `data` can be interpreted as two-dimensional /// array, returning a true when valid and false when not. pub fn validate_2d_structure(data: &[FieldElement], width: usize) -> bool diff --git a/crypto/stark/src/tests/boundary_tests.rs b/crypto/stark/src/tests/boundary_tests.rs deleted file mode 100644 index 7ccafc163..000000000 --- a/crypto/stark/src/tests/boundary_tests.rs +++ /dev/null @@ -1,36 +0,0 @@ -use math::field::{element::FieldElement, goldilocks::GoldilocksField, traits::IsFFTField}; -use math::polynomial::Polynomial; - -use crate::constraints::boundary::{BoundaryConstraint, BoundaryConstraints}; - -type PrimeField = GoldilocksField; - -#[test] -fn zerofier_is_the_correct_one() { - let one = FieldElement::::one(); - - // Fibonacci constraints: - // * a0 = 1 - // * a1 = 1 - // * a7 = 32 - let a0 = BoundaryConstraint::new_simple_main(0, one); - let a1 = BoundaryConstraint::new_simple_main(1, one); - let result = BoundaryConstraint::new_simple_main(7, FieldElement::::from(32)); - - let constraints = BoundaryConstraints::from_constraints(vec![a0, a1, result]); - - let primitive_root = PrimeField::get_primitive_root_of_unity(3).unwrap(); - - // P_0(x) = (x - 1) - let a0_zerofier = Polynomial::new(&[-one, one]); - // P_1(x) = (x - w^1) - let a1_zerofier = Polynomial::new(&[-primitive_root.pow(1u32), one]); - // P_res(x) = (x - w^7) - let res_zerofier = Polynomial::new(&[-primitive_root.pow(7u32), one]); - - let expected_zerofier = a0_zerofier * a1_zerofier * res_zerofier; - - let zerofier = constraints.compute_zerofier(&primitive_root, 0); - - assert_eq!(expected_zerofier, zerofier); -} diff --git a/crypto/stark/src/tests/mod.rs b/crypto/stark/src/tests/mod.rs index 7b3743407..0ce31d256 100644 --- a/crypto/stark/src/tests/mod.rs +++ b/crypto/stark/src/tests/mod.rs @@ -1,5 +1,4 @@ pub mod air_tests; -pub mod boundary_tests; pub mod bus_tests; pub mod fri_tests; pub mod proof_options_tests;