fix: BLS Lagrange interpolation via fr-domain + Pippenger MSM - #660
fix: BLS Lagrange interpolation via fr-domain + Pippenger MSM#660varex83agent wants to merge 3 commits into
Conversation
…penger MSM Ported from closed PR #551 (first optimization pass), split out as an isolated change. - Lagrange signature interpolation now uses blst Pippenger multi-scalar multiplication with a single final affine conversion, instead of one scalar mult plus one field-inversion-costing affine conversion per share. - Polynomial evaluation (threshold_split) and secret interpolation (recover_secret) moved to fr-domain Horner/dot-product arithmetic; fr copies of secret material are volatile-wiped via a SecretFrVec drop guard. - Error semantics preserved throughout. Measured (Apple M3 Pro, vs charon v1.7.1 herumi): - tbls/threshold_aggregate 3-of-4: 407us -> 205us (charon 300us) - tbls/threshold_aggregate 7-of-10: 951us -> 404us (charon 792us) - tbls/threshold_split 7-of-10: 15.1us -> 10.0us - tbls/recover_secret 7-of-10: 14.5us -> 10.1us Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
iamquang95
left a comment
There was a problem hiding this comment.
Math seems fine, but I suggest that we should add more tests that pass old and new code to make sure it well covered.
| result = scalar_add_secret(&result, &term)?; | ||
| unsafe { | ||
| for (share, coeff) in shares_fr.0.iter().zip(&coeffs) { | ||
| let mut term = blst::blst_fr::default(); |
There was a problem hiding this comment.
term = share_i·λ_i retains recoverable secret material; raw blst_fr is Copy with no zeroizing Drop.
There was a problem hiding this comment.
Fixed in 787bdc3 — term = share_i·λ_i is now wiped with wipe_fr each loop iteration, since blst_fr is Copy with no zeroizing Drop.
| // Multi-scalar multiplication (Pippenger) of all signatures by their | ||
| // Lagrange coefficients in one pass, with a single final affine | ||
| // conversion (each affine conversion costs a field inversion). | ||
| let points: Vec<blst::blst_p2_affine> = signatures |
There was a problem hiding this comment.
signatures
.as_slice()
.mult(&scalar_bytes, SCALAR_BITS)
.to_signature()
blst already implements MultiPoint for Signature
There was a problem hiding this comment.
Done in 787bdc3. Now uses signatures.mult(&scalar_bytes, SCALAR_BITS).to_signature() via blst's MultiPoint for [Signature], dropping the manual affine extraction and blst_p2_to_affine conversion. The blst impl transmutes to the same affine slice and runs the identical MSM.
Address PR review on the threshold BLS interpolation: - Wipe `term = share_i·λ_i` each iteration in `lagrange_interpolate_secret`; `blst_fr` is `Copy` with no zeroizing `Drop`, so the intermediate secret product must be wiped explicitly. - Aggregate partial signatures via blst's `MultiPoint for [Signature]` instead of hand-rolling the affine extraction and final conversion; it transmutes to the same affine slice and runs the identical MSM. - Add subset-coverage tests exercising both interpolation paths across every threshold-sized subset of shares/signatures for several (t, n) configurations. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
|
Added test coverage in 787bdc3 for the concern about validating the new math: |
…gation main refactored the BLS backend from `blst_impl.rs` (the `Tbls` trait impl) into free functions split across `tbls.rs` (orchestration) and `tbls/math.rs` (all blst/unsafe arithmetic). This PR's fr-domain + Pippenger optimizations lived in the now-deleted `blst_impl.rs`. Resolution: accept main's module split and port this PR's changes into it. - `tbls/math.rs`: fr-domain `evaluate_polynomial`/`lagrange_interpolate_secret` (with per-term and accumulator zeroization), Pippenger MSM signature interpolation via blst's `MultiPoint for [Signature]`, fr-domain Lagrange coefficients, and the `fr_from_scalar`/`scalar_from_fr`/`secret_from_fr`/ `wipe_fr`/`SecretFrVec` helpers. `evaluate_polynomial` keeps its `&[BlstSecretKey] -> BlstSecretKey` signature so `tbls.rs` is unchanged. - `tbls.rs` tests: ported the every-threshold-subset coverage tests as free-function calls. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Summary
This is an isolated port of the BLS aggregation performance optimization from closed PR #551, split out as a standalone change touching only
crates/crypto/src/blst_impl.rs.What changed
threshold_aggregate): replaced per-share scalar-multiply + affine conversion with blst Pippenger multi-scalar multiplication (MSM), performing a single affine conversion at the end.threshold_split): moved from point-space to fr-domain Horner evaluation.recover_secret): moved to fr-domain dot-product arithmetic.SecretFrVecRAII guard.Measured performance (Apple M3 Pro, vs charon v1.7.1 herumi)
threshold_aggregateis now ~2× faster and beats charon's herumi-based implementation at both thresholds.🤖 Generated with Claude Code