Skip to content

perf(precompiles): add support for GLV strategy - #3426

Open
Nashtare wants to merge 1 commit into
nextfrom
robin/precompile_glv
Open

perf(precompiles): add support for GLV strategy #3426
Nashtare wants to merge 1 commit into
nextfrom
robin/precompile_glv

Conversation

@Nashtare

@Nashtare Nashtare commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Describe your changes

Add support for GLV strategy when computing MSMs.

Move the secp256k1 GLV endomorphism split from the ECDSA verifier's MASM/advice ABI into the precompiles prover's addition-chain strategy. ecdsa_k256_keccak::verify now always logs a plain u1*G + u2*Q claim
(Straus-shaped, no algorithm selector, no GLV advice), and the deferred prover satisfies it internally with a GLV-decomposed wNAF chain -- phi(P) is certified in-circuit via its value relation (x_phi = beta*x_P, y_phi = y_P), never trusted host advice.

Performance comparison between the two algorithms: https://claude.ai/code/artifact/b2e7a755-9313-4397-9810-eba22ab4a39e?via=auto_preview

MASM overhead is negligible (12 cycles for verify).

Checklist before requesting a review

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Commits are signed.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.
  • Updated CHANGELOG.md

@Nashtare
Nashtare requested review from adr1anh and huitseeker July 25, 2026 12:28
@Nashtare
Nashtare force-pushed the robin/precompile_glv branch from 34daf0d to 5e40746 Compare July 25, 2026 15:01

@huitseeker huitseeker left a comment

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.

Thanks! overall this looks super promising!!

Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak.masm Outdated
Comment thread crates/lib/core/src/dsa.rs Outdated

@adr1anh adr1anh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The signed GLV algebra looks correct to me:

u ≡ sa·a + λ·sb·b mod n

and the implementation consistently applies each sign both to the MSM base and to the scalar-field recomposition. Here λ is the scalar-field eigenvalue; β is only used to construct φ(P) = (βx, y).

I have two main blocking concerns:

  1. Valid zero GLV halves are always included in the 4-term MSM, but the curve precompile rejects zero-scalar terms. This can reject valid signatures.
  2. I don’t think GLV hints should become part of encode_signature. Generating them through an event from the already-derived U1/U2 digests would preserve the signature API and avoid duplicating hashing, inversion, and scalar arithmetic.

There is also a mismatch between the PR description and the current branch: the description says selection remains manual and the benchmark uses Straus, while the implementation is now GLV-only and Straus is archived.

The remaining comments are documentation and MASM cleanup suggestions. There are also unrelated duplicate changelog entries, and the branch should be rebased rather than carrying the merge from next.

This was a static review; I did not compile or run tests.

Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak_glv.masm Outdated
Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak_glv.masm Outdated
Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak_glv.masm Outdated
Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak_glv.masm Outdated
Comment thread crates/lib/core/src/dsa.rs Outdated
Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak_glv.masm Outdated
Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak_glv.masm Outdated
Comment thread crates/lib/core/asm/crypto/dsa/ecdsa_k256_keccak_glv.masm Outdated
Comment thread precompiles/src/math/curve/glv.rs Outdated
@Nashtare
Nashtare force-pushed the robin/precompile_glv branch 2 times, most recently from dccc6c6 to 2509066 Compare August 3, 2026 15:13
Move the secp256k1 GLV endomorphism scalar decomposition from the
ECDSA verifier's MASM/advice ABI into the precompiles prover's
addition-chain strategy: ecdsa_k256_keccak::verify logs a plain
u1*G + u2*Q claim, and the deferred prover satisfies it with a
GLV-decomposed chain, certified in-circuit (phi(P) proven via its
value relation, lambda pinned by pointer, never an AIR constant).

This also cleans up the CHANGELOG duplication left by the earlier
merge with next, and drops the now-dead scalar_inv_mod_n/reduce_mod_n
helpers.
@Nashtare
Nashtare force-pushed the robin/precompile_glv branch from 2509066 to 22e3253 Compare August 3, 2026 15:23
@Nashtare

Nashtare commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Updated comparison against baseline. We now have GLV outperforming Straus in almost all cases and the delta is even larger for heavy batches (up to 36%)

Screenshot 2026-08-03 at 17 57 38

@Nashtare
Nashtare requested a review from adr1anh August 4, 2026 06:55
@Nashtare

Nashtare commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@adr1anh I've revamped the approach to account for our latest discussion. The initial overhead of GLV is now basically removed (and the gains are even bigger). This should also make it much easier / cleaner to integrate non-GLV curves like secp256r1 or Curve25519 in the future.

@Nashtare
Nashtare requested a review from Al-Kindi-0 August 7, 2026 09:59
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