refactor(math): dead-code & structural cleanup of the math crate

[diegokingston]

Summary

Dead-code and structural cleanup of the math crate. Removes unused public
API, flattens the FFT module layout, and consolidates tests into one place.
No behavior changes — every removed item has zero production callers.

Changes

Dead code removal

• Drop the unused Deserializable trait and the unreachable error variants
  it fed: DeserializationError, PairingError,
  ByteConversionError::{InvalidValue, PointNotInSubgroup, ValueNotCompressed},
  CreationError::InvalidDecString.
• Remove the vestigial helpers module (next_power_of_two,
  resize_to_next_power_of_two) — unused.

Visibility tightening

• Gate Polynomial::{truncate, reverse} and Polynomial::evaluate_fft_bit_reversed
  behind #[cfg(test)]. Every caller is test-only (truncate/reverse are
  reached only through the already-cfg(test) fast_division /
  invert_polynomial_mod; evaluate_fft_bit_reversed only from the FFT
  property tests), so this keeps them out of the production binary.

Structural

• Flatten fft/cpu/* up to fft/* — the cpu layer had no gpu sibling.
• Collapse the polynomial/ folder into a single polynomial.rs; delete the
  stale polynomial/README.md.
• Consolidate scattered inline test modules into the dedicated tests/
  directory.

Folded in (commit 48762cf)

• Delete reference_fft.rs (the reference radix-2 FFT) and its four
  comparison tests. It existed only to cross-check the production Bowers FFT,
  which is already verified directly against a naive O(n^2) DFT at the same
  sizes — the reference oracle was redundant.
• Gate get_powers_of_primitive_root as test-only; production twiddle
  generation goes through bowers_fft::LayerTwiddles.
• Collapse the evaluate_fft_cpu / interpolate_fft_cpu one-line wrappers
  into their sole callers; narrow coset_lde_full_into to pub(crate).
• Relocate the inline test modules from bit_reversing.rs, goldilocks.rs
  and extensions_goldilocks.rs into tests/.

Footprint

25 files in crypto/math, +674 / -1113. The insertions are mostly the
polynomial/mod.rs content relocating into polynomial.rs.

Test plan

• cargo test -p math — 211 pass
• make lint (clippy --workspace --all-targets -D warnings) — clean

[claude]

Review: Cleanup/math dead code

Clean structural refactor — removes ~830 lines of dead code and flattens the fft::cpu layer to fft::. The overall direction is good.

Medium — Breaking API changes

Three previously-public methods are now gated by #[cfg(test)], which silently removes them from non-test builds. Any downstream crate that calls them will fail to compile without a helpful error:

Symbol	File	Line
Polynomial::truncate	crypto/math/src/polynomial.rs	201
Polynomial::reverse	crypto/math/src/polynomial.rs	211
Polynomial::evaluate_fft_bit_reversed	crypto/math/src/polynomial.rs	971

If the goal is to hide these from external crates, pub(crate) is the right tool — it restricts visibility without disappearing from non-test builds. Inline suggestions attached.

Low — Pre-existing panic surfaced by the move

interpolate_offset_fft calls offset.inv().unwrap() (line 1034). The function already returns Result<_, FFTError>, so a zero offset could be propagated as an error instead of panicking. Not introduced by this PR, but this is a good moment to fix it since the code is being touched.

Removed public API (no objection, just noting)

The following public items are removed with no replacement:

• DeserializationError, PairingError (from math::errors)
• Deserializable trait (from math::traits)
• ByteConversionError::InvalidValue / PointNotInSubgroup / ValueNotCompressed
• CreationError::InvalidDecString
• math::helpers::{next_power_of_two, resize_to_next_power_of_two}

The removed Proof::deserialize had a hardcoded chunks(8) regardless of T's actual size — removing it rather than fixing it is the right call.

No issues with the restructuring itself

The fft::cpu -> fft:: path flattening, the polynomial/mod.rs -> polynomial.rs consolidation, and the reference_fft renaming are all clean.

[github-actions]

Codex Code Review

No issues found in the PR diff for security, correctness, significant performance, or simplicity.

I attempted to run tests, but the sandbox could not resolve/fetch Cargo dependencies and also hit read-only Cargo/Rustup cache paths. Static review only.

[github-actions]

Codex Code Review

Findings

1. Potential bug / API regression: roots_of_unity.rs now hides get_powers_of_primitive_root behind #[cfg(test)]. This was previously a public helper and supports non-coset configurations like Natural, BitReverse, and inverse variants. Any non-test downstream code importing it will now fail to compile, and the tests still pass because they compile with cfg(test). Keep it public unless this is an intentional breaking API removal.

Security

No Critical/High/Medium/Low security vulnerabilities found in the diff.

Verification

I attempted cargo check, but it could not run in this environment because rustup/cargo needed writable home cache paths and an unavailable git dependency.

[claude]

Code Review: Cleanup/math dead code

Summary: This PR removes dead code and restructures the math crate's FFT module — flattening fft/cpu/ into fft/, merging fft/polynomial.rs into polynomial.rs, removing the legacy NR Radix-2 FFT implementation, and deleting unused traits (Deserializable, PairingError, DeserializationError). Changes are largely mechanical and improve clarity.

---

Medium — Reduced cross-validation coverage for a security-critical algorithm

The deleted fft/cpu/fft.rs contained the reference NR Radix-2 Cooley-Tukey FFT, and bowers_fft_tests.rs had three tests that proved Bowers FFT agreed with it:

• test_bowers_vs_native_various_sizes (deterministic sweep)
• test_bowers_vs_native_edge_cases (zeros / ones / alternating)
• proptest_bowers_matches_native_fft (property-based)

These cross-checks were the strongest correctness guarantee for the FFT. Removing both the reference implementation and the cross-check tests simultaneously means Bowers FFT is now only validated against the naive O(n²) DFT — a different algorithm family, not an independent implementation of the same algorithm. For a ZK proving system, FFT correctness is directly tied to proof soundness. Consider keeping at least the proptest cross-check by extracting the reference NR FFT into a test-only helper instead of deleting it entirely.

---

Low — Panic on zero offset in public API (interpolate_offset_fft)

Ok(scaled.scale(&offset.inv().unwrap()))

Panics when offset == FieldElement::zero(). Pre-existing, but since this function is being touched in the merge, it is worth propagating the error instead. A zero coset offset is a degenerate input that should return Err, not panic.

---

Low — evaluate_fft_cpu / interpolate_fft_cpu removed without a deprecation note

These were pub module-level functions in fft/polynomial.rs. They disappear silently. Given this is an internal crate this is probably fine, but worth a CHANGELOG or semver note if any downstream crate consumed them directly.

---

Correctness note — Proof::deserialize removal is the right call

The deleted impl used a hardcoded chunks(8) for a generic T:

for elem in bytes[0..].chunks(8) {
    let node = T::deserialize(elem)?;

This would silently corrupt for any T whose serialized size ≠ 8 bytes. Removing it is correct; if proof deserialization is ever needed the element size must come from the type.

[diegokingston]

/bench 5

[github-actions]

Benchmark — fib_iterative_8M (median of 5)

_{Table parallelism: auto (cores / 3)}

Metric	main	PR	Δ
Peak heap	51851 MB	51844 MB	-7 MB (+-0.0%) ⚪
Prove time	24.966s	25.046s	+0.080s (+0.3%) ⚪

✅ No significant change.

✅ Low variance (time: 1.1%, heap: 0.3%)

_{Commit: 54aac43 · Baseline: cached · Runner: self-hosted bench}