cleanup(stark/constraints): drop coefficient-form leftovers, dedupe eval

[diegokingston]

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.

[github-actions]

Codex Code Review

No findings in the PR diff.

I reviewed the changed constraint evaluator, boundary cleanup, unsafe-to-expect replacement, and removed tests/helpers. I did not see security, correctness, significant performance, or simplicity issues introduced by these changes.

Verification note: I attempted to run cargo test -p stark --lib, but the environment could not initialize Rust toolchain state due read-only rustup storage, then network DNS failure when redirected to /tmp. git diff --check passed.

[claude]

Review

Clean, well-scoped refactor. The deletions are confirmed dead code (no remaining callers), the eval_row closure typing is correct (auto-deref from &mut Vec<T> → &mut [T] and &mut Frame → &Frame for new_prover), and the parallel/sequential semantics are preserved identically.

Low – inconsistency in transition.rs (not introduced by this PR, but adjacent to the fix)

The PR correctly replaces unsafe { numerator.div(denominator).unwrap_unchecked() } (the exemptions_period branch) with .expect("zerofier denominator is non-zero: z is sampled out-of-domain"). The non-exemptions path immediately below has the same invariant but still uses a bare .unwrap():

// transition.rs ~line 286-289
(-trace_primitive_root.pow(self.offset() * trace_length / self.period())
    + z.pow(trace_length / self.period()))
.inv()
.unwrap()   // ← same invariant; should be .expect(…) for consistency

Worth a one-line follow-up, but not a blocker.

Everything else: LGTM

• No remaining callers for any deleted public API (new_simple_aux, compute_zerofier, steps, steps_for_boundary, cols_for_boundary, generate_roots_of_unity, values, check_boundary_polys_divisibility).
• The no-op debug check (check_boundary_polys_divisibility called with two empty Vecs) is correctly identified and removed.
• std:: → core:: is a trivially correct portability improvement.
• eval_row closure captures are all shared references; rayon Sync requirements were already present in the old code.

[github-actions]

Codex Code Review

No issues found in the PR diff.

Security: no Critical/High/Medium/Low vulnerabilities identified in the changed code. The removal of unsafe { unwrap_unchecked() } in transition zerofier evaluation is a net safety improvement.

I could not run cargo test -p stark: rustup/Cargo attempted to write under read-only /home/runner/.rustup / /home/runner/.cargo, and then needed network/git access, which is unavailable in this sandbox.

[claude]

Clean, behaviour-preserving refactor. No security issues or bugs found.

[claude]

unsafe removal (transition.rs) - Replacing unsafe unwrap_unchecked with .expect() is a genuine safety improvement. The original invokes UB if the invariant ever breaks; the new code panics cleanly instead.

[claude]

Dead debug code removal (evaluator.rs, debug.rs) - check_boundary_polys_divisibility was called with two always-empty Vec::new() vectors so the loop body never executed. Removing it is correct and eliminates the misleading impression that boundary divisibility was being checked in debug builds.

[claude]

eval_row closure extraction (evaluator.rs) - The parallel and sequential arms are structurally identical to their originals. All captured variables are immutable borrows so Sync constraints for the parallel path are unchanged. The coercions from &mut Vec to &mut slice on the call sites are standard.

[claude]

Minor - public API break: BoundaryConstraints::new_simple_aux is pub on a pub struct. Removing it is a semver-breaking change for external consumers. If this crate is purely internal or semver is managed separately, no action needed - just note it in the changelog.