Batch EIP-2537 pairing check Miller loops and use affine MSM - #12329
Conversation
…affine MSM Adopt the new Nethermind.Crypto.Bls 1.1.0-preview.197 APIs: - BLS12_PAIRING_CHECK: decode all pairs into contiguous affine buffers (validated in parallel like the MSM precompiles), then compute the product of Miller loops in a single batched MillerLoopN call sharing the Fp12 squarings across pairs, instead of one MillerLoop + GT multiplication per pair. Pairs containing a point at infinity contribute e(x, y) = 1 and are excluded from the batch, but both of their points are still validated and subgroup-checked as before. - BLS12_G1MSM / BLS12_G2MSM: decode points directly into affine layout via the new raw affine Decode (shrinking the rented point buffers from P1.Sz/P2.Sz to P1Affine.Sz/P2Affine.Sz longs per slot) and call MultiMultAffine, skipping the internal Jacobian-to-affine batch conversion that MultiMult performed. Validation order and error results are unchanged. Add regression tests for all-infinity pairing input, subgroup-invalid points in infinity pairs, and all-infinity multi-point MSM. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @Marchhill's task in 6m 39s —— View job Review: Batch EIP-2537 pairing check Miller loops and use affine MSM
I reviewed the full diff against Correctness — verified equivalent to the serial implementation
Concurrency / thread-safety — no consensus risk
Performance / repo rules
FindingsLow — Low — Known blocker (already acknowledged in the PR description) — unpublished dependency. VerdictThe code changes are sound and consensus-safe; I found no defect that should block merge on its own. The only thing preventing merge is the explicitly-acknowledged draft state / unpublished dependency. Once I could not run the build/tests here (the unpublished package makes |
Temporary until the final package ships to nuget.org Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Gas-benchmark result: PR #12329 (batched pairing Miller loop + affine MSM)Config: Pairing check — matches the PR's claimed mechanism almost exactly
Exactly the expected shape for "share the Fp12 squarings across pairs": zero speedup at 1 pair (nothing to share), then a smooth, monotonic increase as pair count grows. The PR description's stated range (1.14×–1.45× ≈ −12% to −31%, for 2–16 pairs) undersells it a bit at the higher end (tested up to 24 pairs here), but direction and shape are a clean match. Zero individual test regressed by more than 5% across the full 130-test set. G1MSM / G2MSM — real but smaller, and non-monotonic
Mostly a modest, real win (~0 to −9%), consistent with "decode directly into affine layout, skip the Jacobian→affine batch conversion" being a smaller, more fixed-cost optimization than pairing's batching. The G1MSM k=64 outlier (−32.5%) is internally solid — reproducible within ~1pp across all 10 gas sizes at that point count — but doesn't fit the surrounding pattern (k=16 and k=128 are far smaller, and G2MSM at the same k=64 only shows −6%). Flagging this as an interesting, real, reproducible data point rather than something explained by the PR's headline optimization — possibly a bucket-size/algorithm-selection threshold in the underlying MSM implementation specific to G1 at that count. Might be worth a look. OverallMaster 1132ms → PR 976ms average across all 130 tests, −13.8% aggregate, driven mainly by pairing. Unlike contention-focused optimizations (e.g. #12284), this shows up cleanly in a serial, single-threaded benchmark because it's a pure compute-path change — no concurrency dependency to obscure the signal. 🤖 Generated with Claude Code gas-benchmark skill |
EXPB Benchmark ComparisonRun: View workflow run superblocksScenario: Client Processing (SSE)
K6 TTFB
realblocksScenario: Client Processing (SSE)
K6 TTFB
|
EXPB reproducible benchmarks — PR vs master (fusaka 1k + fusaka full 6.1k + 10k Cancun realblocks, 5×/side + dotTrace)Master ( Setup: EXPB reproducible-benchmarks workflow (fusaka payload set from
The PR and master 5× iterations interleaved on the runner (alternating PR/master), which makes the A/Bs drift-resistant. TL;DR
fusaka realblocks (1k blocks, ms)
realblocks 10k (Cancun-era, ms)
fusaka full set (6,099 blocks, ms)
The raw AVG gap is dominated by the known flat-layout "Persisting StateId" tier-boundary stalls on longer replay horizons: every run hits 1–6 s outlier blocks (per-run MAX 0.7–6.1 s), and in the paired per-block data the >1 s stalls cluster in two windows (25,492,48x and 25,495,15x–18x) landing on alternating sides per block — e.g. block 25,492,484 stalls on PR (1,106 vs 35 ms) while 25,492,486 stalls on master (44 vs 1,101 ms). Excluding those 13 lottery blocks: PR 39.71 vs master 39.44 (+0.68%), median per-block delta +0.20 ms, delta distribution p25 = −1.1% / p75 = +2.6% — below the resolving power of 5 runs on this noisier horizon, and inconsistent in direction with the other two suites. All 30 measurement runs clean: full payload count (999 / 6,099 / 9,999 SSE-measured per run), no Nethermind dotTrace (1× per image per suite)Full-run sampling profiles, XML reports diffed with
Infra note: the PR-side 10k dotTrace CI run is marked "failure" — that is the workflow's blanket Pre-merge measurement archaeology (why the first comparison was discarded)Before the merge, the PR base was 43 commits behind master and a naive PR-vs-master 5×5 showed a spurious +3.99% AVG / +11.9% P99 "regression" (t = 20.8), reproducible and image-tied per an interleaved A/B/A. A four-arm decomposition on the fusaka suite (merge-base parent / parent+package-bump-only / full PR / current master) attributed all of it to master-side improvements landed after the PR branched (#12368 et al.): PR vs its own parent = +0.16% (t = 0.7, n.s.), package bump alone = neutral, master vs parent = −3.7% AVG / −11% P99. Runs: parent 5× 29130451772, pkgbump 2× 29130456814, pre-merge PR 5× 29128615696, old-master 5× 29128620946, A/B/A 29129880556/29129885744/29129890763. Superseded by the post-merge results above. Run inventory (post-merge)
Helper branches (this workflow version benchmarks the dispatched ref): Merge note for the author: the 🤖 Generated with Claude Code |
# Conflicts: # src/Nethermind/Nethermind.Runner/packages.lock.json
Avoids relying on MillerLoopN(0) writing the identity into the uninitialized stackalloc buffer and skips the final exponentiation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…safe Consensus-critical concurrency invariant: the result write races across Parallel.For workers, but every writer stores a failure and only the atomically written ResultType gates the outcome, so the post-barrier result is a failure iff any pair failed to validate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- merge latest master into the branch (packages.lock.json conflict resolved + regenerated) - Nethermind.Crypto.Bls 1.1.0-preview.199 (nugettest.org) -> 1.1.0 (nuget.org) - drop the temporary nugettest.org package-source mapping for Nethermind.Crypto.Bls Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collapse the multi-line scratch-buffer / pair-compaction / concurrency notes to concise one-liners. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @Marchhill's task in 3m 19s —— View job Review: Batch EIP-2537 pairing check Miller loops and use affine MSM
I re-reviewed the full diff against Prior blockers — both resolved ✅
Correctness — verified equivalent to the serial implementation
Concurrency / thread-safety — no consensus risk
FindingsLow — VerdictSound, consensus-safe, and well-tested (all-infinity → 1; invalid points in infinity slots rejected; all-infinity MSM → infinity; full 1073-vector suite + gas tests + |
The four TryDecodeRaw overloads (G1/G1Affine, G2/G2Affine) had near-identical bodies. Extract the length/field-element/infinity validation into a shared ValidateRawPoint helper, leaving thin per-type wrappers for the Zero/Decode/ OnCurve calls (the point types share no common interface). Behaviour unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
benaadams
left a comment
There was a problem hiding this comment.
(Nit, optional) Four near-identical TryDecodeRaw overloads now exist (G1/G2 × Jacobian/affine); ValidateRawPoint dedupes the bulk and the ref-struct bindings make further dedup awkward, so acceptable as is.
Related PR: NethermindEth/blst-bindings#9
Changes
Adopts new Nethermind.Crypto.Bls
1.1.0-preview.199APIs (NethermindEth/blst-bindings#9) in the EIP-2537 precompiles:MillerLoopNcall that shares the Fp12 squarings across pairs — measured 1.14×–1.45× end-to-end pairing-check speedup for 2–16 pairs — replacing the serial per-pairMillerLoop+ GT multiplication. Pairs containing an infinity point contribute e(x, y) = 1 and are excluded from the batch; their points are still fully validated, preserving exact consensus semantics (validation order, error results, and gas are unchanged).MultiMultAffine, skipping the internal Jacobian→affine batch conversion.Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
New regression tests: all-infinity pairing check input returns 1; subgroup-invalid G1/G2 points in infinity-paired slots are still rejected; all-infinity multi-point MSM returns infinity. Full EIP-2537 vector suites (1073 tests), the EIP-2537 gas tests, and the full Nethermind.Evm.Test suite pass; zkevm configurations verified to compile.
Remarks
The referenced⚠️ Before merging: NethermindEth/blst-bindings#9 must be merged and a final package published to nuget.org, then the version pin,
Nethermind.Crypto.Bls 1.1.0-preview.199is published on the nugettest.org staging feed (already a configured package source in this repo), so CI restores normally.nuget.configmapping, andpackages.lock.jsonupdated accordingly.