perf(ecsm): 7.7x faster witness generation

[diegokingston]

Summary

compute_witness (one ECSM ecall's witness): 20.8ms → 2.7ms measured (high-popcount scalar, ~380 steps). At 4 ecalls per ecrecover that's ~83ms → ~11ms host per recovery; ~8.3s → ~1.1s for a 100-signature block.

Three changes:

1. Hand-rolled Jacobian replay (dbl-2009-l / madd-2007-bl) over k256's public field arithmetic, using the crate's own Montgomery batch-inversion for z-normalization and slope denominators. Replaces k256's ProjectivePoint::batch_normalize, which measured ~5-6ms for the ~760 points of one witness — no better than 760 individual to_affine() calls (its inner BatchInvert is fine at ~60µs; the total never gets the batching benefit).
2. shifted_quotient: one div_rem instead of separate % (divisibility assert) and / — halves the 512/256-bit BigInt divisions (6 → 3 per step).
3. build_step loop on rayon — steps are independent witnesses.

The subtle part (caught by the parity tests)

The first Jacobian version produced wrong points: k256's lazy-magnitude negate(1) is only correct below ~2p, so subtraction chains over unnormalized intermediates miscompute (this is why k256's own formulas call normalize_weak after every add-chain). Fixed with strategic normalize() on intermediates feeding subtractions, documented in the code. Parity with the BigUint reference replay is covered by tests::curve_tests.

Verification

• 15/15 ecsm crate tests (parity vs the reference replay).
• 8/8 prover ECSM tests: full prove+verify of the ecsm guests + the forged-witness rejection tests.
• Timing harness included: cargo run --release --example bench_witness -p ecsm.

[diegokingston]

/bench

[github-actions]

Benchmark — ethrex 20 transfers (median of 3)

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

Metric	main	PR	Δ
Peak heap	51199 MB	51267 MB	+68 MB (+0.1%) ⚪
Prove time	27.291s	26.110s	-1.181s (-4.3%) ⚪

✅ No significant change.

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

_{Commit: 65b8952 · Baseline: cached · Runner: self-hosted bench}

[diegokingston]

/bench

[MauroToscano]

⚠️ AI-generated review. Produced by an automated agent and posted from this account — not written by hand. Findings below are leads to verify, not confirmed conclusions.

This PR swaps the projective double-and-add replay in ECSM witness generation for a hand-rolled Jacobian group law over k256's field arithmetic plus batch inversion, and leaves useful measurement notes (why batch_normalize was dropped). Witness gen is untrusted, so the hand-rolled group law does not move the soundness surface, and the BigUint parity oracle is the right guard. Four issues below, one of which breaks debug builds today.

crypto/ecsm/src/curve.rs:176 and :200 — f - d.double() and r * r - hhh - x1hh.double() negate a magnitude-2 value, which panics in any debug build. k256 0.13.4 lowers Sub to self + -other and Neg to self.negate(1), whose documented precondition is that the supplied magnitude is >= the operand's actual magnitude; double() is add(self, self) and doubles the tracked magnitude. Under cfg(debug_assertions) k256 compiles in the magnitude-tracking FieldElementImpl, whose negate opens with debug_assert!(self.magnitude <= magnitude) — here debug_assert!(2 <= 1). schedule() always emits op = 0 first for k >= 2, so step 0 of every replay hits :176. These are the only two out-of-contract sites; the mul chains, batch_invert, the affine conversion and the denominator/lambda loops all stay within magnitude limits.

CI stays green because everything touching this runs --release (cargo nextest archive --release), so the blast radius is developers: the workspace [profile.dev] (Cargo.toml:24-28) raises opt-level under the comment "Optimized dev builds: tests run fast without --release" but leaves debug-assertions at its default with no per-package override, so make test, make test-fast, make test-prover*, and any dev-profile cargo run -p cli over an ecrecover guest panic inside k256. Release output is correct by construction, not by luck — negate(1)'s per-limb constants are 2*(m+1)*p_limb, ~4x the limb bound versus a double-of-normalized's <=2x, leaving the actual magnitude far inside max_magnitude() = 2047 — but that is correctness resting on unspecified slack, and it disables the one checker for exactly the lazy-reduction bug class the PR description says bit the first draft. Fix is one token each: f - d - d and r * r - hhh - x1hh - x1hh (both keep every negate(1) on a magnitude-1 operand), or d.double().normalize_weak(). Then confirm the ecsm and prover ECSM tests pass without --release.

crypto/ecsm/Cargo.toml:12 — rayon = "1.8.0" as a hard dep breaks the convention of every neighbouring crate: crypto/crypto, crypto/math, crypto/stark and prover declare it optional = true behind parallel = ["dep:rayon"], prover forwards it (default = ["parallel"]), crypto/stark has par.rs abstracting over cfg(feature = "parallel"), and prover/src/tables/trace_builder.rs — the file calling compute_witness at :851 — uses the paired #[cfg(feature = "parallel")] / #[cfg(not(...))] idiom. Two consequences: executor depends on ecsm unconditionally and declares no features, so it now carries a transitive hard rayon dep it never had; and the unguarded .par_iter() at witness.rs:334 means a --no-default-features build still spins up a rayon pool in the ECSM path — a config actively exercised by Makefile:428/:438 (cargo clippy --workspace --all-targets --no-default-features). Nothing breaks today; it is hygiene. Make it optional = true with parallel = ["dep:rayon"], gate witness.rs:334 with a cfg(not(feature = "parallel")) .iter() fallback, and add "ecsm/parallel" to prover's parallel list.

crypto/ecsm/src/tests/curve_tests.rs:30 — the new replay is never exercised with a non-generator base point. Every test reaching it (curve_tests.rs:30/68, witness_tests.rs, ecdas_tests.rs:32, ecsm_tests.rs:41/252, the e2e ECSM guest tests) passes G; the one non-G call, ecsm_tests.rs:223, uses k=1 and returns at the sched.is_empty() early exit before any Jacobian arithmetic runs. Production is the opposite: trace_builder.rs:851 feeds compute_witness a guest-supplied xG, and the ecrecover workload this PR targets (ecsm_lincomb2(GENERATOR, u1, r_point, u2) in crypto/ethrex-crypto/src/lib.rs:121) uses a recovered R as one of the two bases. Nothing pre-merge covers it either — the ethrex CI tests are execution-only and go through scalar_mul_x (untouched k256 native path), both ethrex proving tests are #[ignore], and the merge-queue --run-ignored job whitelists two unrelated tests. The op graph and its magnitude bookkeeping are identical for any base point (schedule(k) is pure bit logic, magnitude is a per-op counter), so this is not a specific hazard — it is the one production input axis the oracle does not sample, and closing it costs one outer loop over a small Vec of bases (e.g. recover_y_canonical(x(5·G))) around the existing sweep.

crypto/ecsm/src/curve.rs:240 — the replay materializes each intermediate twice. Step 1 pushes a_i at 2i and r_i at 2i+1, then carries r_i forward so it is re-pushed bit-identically at 2i+2: pts holds 2n entries when only n+1 are distinct. Step 2 therefore runs the batch_invert chain, the 4-mul Jacobian-to-affine conversion, and two biguint_from_fe calls for ~n-1 exact duplicates — about 2x the necessary work in that block, on a serial stretch of the function this PR exists to speed up. Store the n+1 distinct points and index a_i = pts[i], r_i = pts[i+1]. Allocations are unchanged (StepPts clones an owned a and r per step either way); the win is the redundant inversion-chain muls, conversions, and fe->BigUint parses.

crypto/ecsm/src/curve.rs:258 — related: step 2 computes each affine coordinate as a FieldElement (x * zi2, y * zi2 * zi) and immediately discards the field form, then steps 3-4 decode the same values right back via fe_from_biguint at :258, :260, :271, :274 — to_bytes_be alloc + be32 copy + from_bytes canonical check + .expect, exactly 2n times per witness (~760 for the bench scalar). Keeping a parallel Vec<(FieldElement, FieldElement)> of the normalized coordinates in step 2 and reading it directly is safe on the magnitude rules: to_bytes fully normalizes so the round trip is a pure normalize(), and both retained values are mul outputs (magnitude 1), so gx - ... / gy - ... need no compensating call. The 2 * i indexing stays regardless — it comes from the interleaved step-1 layout and is still needed at :280/:285/:291.

Nits:

• crypto/ecsm/src/curve.rs:65 — the section header still claims the replay batch_normalizes "all points to affine in one shot", but batch_normalize now has zero call sites, its only other mention being the perf note at :212-214 explaining why it was dropped; reword to hand-rolled Jacobian (dbl-2009-l / madd-2007-bl) over k256's field arithmetic plus two Montgomery batch inversions, keeping the projective/k256 mention since ProjectivePoint is still live at :155 backing scalar_mul_affine_x. The same stale sentence sits at crypto/ecsm/Cargo.toml:13-15 ("the projective double-and-add replay + batch inversion"), directly above the lines this PR adds; fix both.

[diegokingston]

Review addressed in 65b8952 (pushed). Point by point:

1. Debug-build magnitude contract (curve.rs:176/:200) — confirmed real: dev-profile cargo test -p ecsm panicked at field_impl.rs:106 (self.magnitude <= magnitude) on the first double. Fixed exactly as suggested: F − 2D and R² − HHH − 2·X1·HH are now two subtractions of the normalized operand (f - d - d, ... - x1hh - x1hh), keeping every negate(1) on a magnitude-1 operand. Dev and release suites both pass (16/16); the panic is gone and release output is unchanged.
2. rayon hard dep — now optional = true behind ecsm/parallel, witness.rs has the paired #[cfg] .iter() fallback, and prover's parallel forwards ecsm/parallel. --no-default-features checks clean for both ecsm and prover (the Makefile clippy config included).
3. Non-generator base — new parity test k256_replay_matches_reference_non_generator_base (base = 5·G via recover_y_canonical) sweeping the same scalar set against the BigUint reference.
4. Duplicate points in the replay — good catch: now stores the n+1 distinct ladder points (a_i = pts[i], r_i = pts[i+1]); serial witness time roughly halved (22.5ms → 11.4ms; parallel unchanged ~2.9ms).
5. FE→BigUint→FE round trips — affine coordinates are kept in FieldElement form for steps 3-4 (mul outputs, so the subtractions stay in contract); each value is converted to BigUint exactly once.
6. Nits — section header + crate description now describe the hand-rolled Jacobian path (dbl-2009-l / madd-2007-bl).

Verified: cargo test -p ecsm (dev) 16/16, --release 16/16, 8/8 prover ECSM tests (prove+verify + forged rejection), --no-default-features clean, bench 2.96ms/call (parallel) / 11.4ms (serial).

[diegokingston]

/bench