Skip to content

Add WnafBase::multiscalar_mul#82

Closed
tarcieri wants to merge 1 commit into
zkcrypto:release-0.14.0from
tarcieri:multiscalar-mul
Closed

Add WnafBase::multiscalar_mul#82
tarcieri wants to merge 1 commit into
zkcrypto:release-0.14.0from
tarcieri:multiscalar-mul

Conversation

@tarcieri
Copy link
Copy Markdown
Contributor

@tarcieri tarcieri commented May 29, 2026

Computes a sum-of-products aA + bB + ... in variable time with w-NAF multi-exponentiation using the interleaved window method, also known as Straus' method.

The key insight is that when computing this sum by means of additions and doublings, the doublings can be shared by performing the additions within an inner loop.

The API and implementation are inspired in part by curve25519-dalek, namely the VartimeMultiscalarMul trait and corresponding implementation in straus.rs.

This results in ~28% speedup on p256 for a 3 scalar/point input:

ProjectivePoint operations/point-scalar lincomb (variable-time)
    time:   [149.13 µs 149.80 µs 150.84 µs]
    change: [−27.999% −27.645% −27.267%] (p = 0.00 < 0.05)

See also: RustCrypto#14

Computes a sum-of-products `aA + bB + ...` in variable time with w-NAF
multi-exponentiation using the interleaved window method, also known
as Straus' method.

The key insight is that when computing this sum by means of additions
and doublings, the doublings can be shared by performing the additions
within an inner loop.

The API and implementation are inspired in part by `curve25519-dalek`,
namely the `VartimeMultiscalarMul` trait and corresponding
implementation in `straus.rs`.

This results in ~28% speedup on `p256` for a 3 scalar/point input:

    ProjectivePoint operations/point-scalar lincomb (variable-time)
        time:   [149.13 µs 149.80 µs 150.84 µs]
        change: [−27.999% −27.645% −27.267%] (p = 0.00 < 0.05)
@tarcieri tarcieri changed the title Add WnafBase::multiscalar_mul (#14) Add WnafBase::multiscalar_mul May 29, 2026
@TalDerei TalDerei mentioned this pull request May 30, 2026
Copy link
Copy Markdown

@TalDerei TalDerei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, cherry picks commit from rustcrypto-group; left some initial observations (but i'm not a crate maintainer). cc @ebfull @str4d

Comment thread src/wnaf.rs
Comment on lines +522 to +523
/// Perform a multiscalar multiplication.
pub fn multiscalar_mul<'a, I, J>(scalars: I, bases: J) -> G
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

referencing dalek (VartimeMultiscalarMul trait and corresponding impl in straus.rs), there's explicit timing language there. suggest a timing note that this runs in variable time and not be used with secret scalars (dalek frames it as operating on "public scalars"). maybe also rename to vartime_multiscalar_mul?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tend to put vartime at the end of the method names BTW.

Copy link
Copy Markdown

@TalDerei TalDerei May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, suggestion was matching dalek's naming convention.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW we also use *_vartime as a suffix in @RustCrypto projects. When looking at something like rustdoc it makes it easier to discover operations have constant time and variable time versions

Comment thread src/wnaf.rs
/// This function must be provided with `tables` and `wnafs` that were constructed with
/// the same window size; otherwise, it may panic or produce invalid results.
pub(crate) fn wnaf_multi_exp<G: Group>(tables: &[&[G]], wnafs: &[&[i64]]) -> G {
debug_assert_eq!(tables.len(), wnafs.len());
Copy link
Copy Markdown

@TalDerei TalDerei May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

promote to release assertion for mismatched lengths.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for now it's not possible to trip this assertion as the equality is enforced at the type system level where WnafBase and WnafScalar have a const generic WINDOW_SIZE that's ensured equal by bounds.

I can promote it, but really it's belt-and-suspenders.

Comment thread src/wnaf.rs
///
/// This function must be provided with `tables` and `wnafs` that were constructed with
/// the same window size; otherwise, it may panic or produce invalid results.
pub(crate) fn wnaf_multi_exp<G: Group>(tables: &[&[G]], wnafs: &[&[i64]]) -> G {
Copy link
Copy Markdown

@TalDerei TalDerei May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK – traced the impl; extends wNAF to MSM via Straus' method: (N * (doublings + adds)) to (doublings + (N * adds)), where N is # LC terms. doubling savings from interleaving windows (and it stacks on existing addition savings that use signed digits). fine for small MSMs, but obviously for larger N a hand rolled Pippenger impl downstream.

generated some of my own test vectors locally, but suggesting multiscalar coverage for wnaf_multi_exp / multiscalar_mul in the diff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wNAF implementations in both curve25519-dalek and libsecp256k1 provide both Straus and Pippenger and select between them at runtime, which would probably be ideal here as well, but that first needs a generic implementation of Pippenger

Comment thread src/wnaf.rs
/// the same window size; otherwise, it may panic or produce invalid results.
pub(crate) fn wnaf_multi_exp<G: Group>(tables: &[&[G]], wnafs: &[&[i64]]) -> G {
debug_assert_eq!(tables.len(), wnafs.len());
let window_size = wnafs.iter().map(|w| w.len()).max().unwrap_or(0);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; window_size is a misnomer here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would you suggest instead?

Comment thread src/wnaf.rs
Comment on lines +524 to +526
where
I: Iterator<Item = &'a WnafScalar<G::Scalar, WINDOW_SIZE>>,
J: Iterator<Item = &'a Self>,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; dalek uses IntoIterator.

@ebfull ebfull deleted the branch zkcrypto:release-0.14.0 June 1, 2026 21:36
@ebfull ebfull closed this Jun 1, 2026
@TalDerei
Copy link
Copy Markdown

TalDerei commented Jun 1, 2026

we can reopen this and target base branch to a zkcrypto:release-0.14.1 per #63 (comment).

@tarcieri tarcieri deleted the multiscalar-mul branch June 2, 2026 12:47
@tarcieri
Copy link
Copy Markdown
Contributor Author

tarcieri commented Jun 2, 2026

@TalDerei do you mean retarget to main? The release-0.14.0 branch was deleted.

Edit: oh, 0.14.1. Every patch release is getting its own branch?

@tarcieri
Copy link
Copy Markdown
Contributor Author

tarcieri commented Jun 2, 2026

Reopened as #85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants