Skip to content

feat: add Pectra BLS12-381 elliptic curve precompiles #1447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ivokub merged 62 commits into from
Apr 15, 2025
Merged

feat: add Pectra BLS12-381 elliptic curve precompiles #1447

ivokub merged 62 commits into from
Apr 15, 2025

Conversation

@yelhousni
Copy link
Contributor

@yelhousni yelhousni commented Mar 14, 2025 •
edited
Loading

Description

This PR adds gnark circuits corresponding to precompiles for BLS12-381 curve operations https://eips.ethereum.org/EIPS/eip-2537.

TODO:

  • BLS12_PAIRING_CHECK: inlcude G2 memerbship in Miller loop (lines computation).
  • BLS12_G2ADD: check the two addends are on G2.
  • BLS12_G2MSM: optimized ScalarMul() with GLV
    • GLS? fakeGLS? GLV/S+fakeGLV/S?
  • BLS12_G1MSM and BLS12_G2MSM: optimize MSM algorithm.

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Tests are implemented in std/evmprecompiles/bls_test.go.

How has this been benchmarked?

PLONK (on BLS12-377) number of constraints (scs):

precompile curve membership test? subgroup membership test? constraints (scs)
BLS12_G1ADD Yes No 9 641
BLS12_G2ADD Yes No 23 668
precompile curve membership test? subgroup membership test? constraints (scs)
BLS12_G1MSM (1 pair)
Scalar Multiplication
 Yes Yes 590 418
BLS12_G1MSM (2 pairs) Yes Yes 1 132 727
BLS12_G1MSM (3 pairs) Yes Yes 1 569 303
BLS12_G1MSM (5 pairs) Yes Yes 2 442 455
BLS12_G1MSM (10 pairs) Yes Yes 4 625 335
BLS12_G1MSM (20 pairs) Yes Yes 8 991 095
BLS12_G1MSM (100 pairs) Yes Yes 43 917 175
precompile curve membership test? subgroup membership test? constraints (scs)
BLS12_G2MSM (1 pair)
Scalar Multiplication
 Yes Yes 1 225 743
BLS12_G2MSM (2 pairs) Yes Yes 2 196 208
BLS12_G2MSM (3 pairs) Yes Yes 3 166 673
BLS12_G2MSM (5 pairs) Yes Yes 5 107 603
BLS12_G2MSM (10 pairs) Yes Yes 9 959 981
BLS12_G2MSM (20 pairs) Yes Yes 19 665 567
BLS12_G2MSM (50 pairs) Yes Yes 48 794 236
precompile curve membership test? subgroup membership test? constraints (scs)
BLS12_PAIRING_CHECK (2 pairs) Yes Yes 2 821 323
BLS12_PAIRING_CHECK (4 pairs) Yes Yes 4 993 571
BLS12_PAIRING_CHECK (9 pairs) Yes Yes 10 424 481
BLS12_PAIRING_CHECK (20 pairs) Yes Yes 22 371 551

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@yelhousni yelhousni added type: new feature dep: linea Issues affecting Linea downstream labels Mar 14, 2025
@yelhousni yelhousni added this to the v0.11.N milestone Mar 14, 2025
@yelhousni yelhousni self-assigned this Mar 14, 2025
@OlivierBBB OlivierBBB mentioned this pull request Mar 17, 2025
Closed
3 tasks
yelhousni and others added 19 commits March 17, 2025 15:03
@yelhousni yelhousni requested a review from ivokub April 8, 2025 16:11
@yelhousni
Copy link
Contributor Author

yelhousni commented Apr 8, 2025

I added Consensys/gnark-crypto#674 - I'll go over G2 map to curve to see that it is also useful for #1040.

Otherwise the map to curve also looks good. @yelhousni - please re-request review when you have addressed all the comments and I'll re-review. I had to merge master because there were some changes due to gnark-crypto API changes.

Addressed all the points except the AssertIsOnCurve and AssertIsOnG1 in the std/algebra.Curve interface. No strong opinion here.

@ivokub
Copy link
Collaborator

ivokub commented Apr 9, 2025

I added Consensys/gnark-crypto#674 - I'll go over G2 map to curve to see that it is also useful for #1040.
Otherwise the map to curve also looks good. @yelhousni - please re-request review when you have addressed all the comments and I'll re-review. I had to merge master because there were some changes due to gnark-crypto API changes.

Addressed all the points except the AssertIsOnCurve and AssertIsOnG1 in the std/algebra.Curve interface. No strong opinion here.

Yup - I think we could try to keep it simple for now. The only thing I'm worried about when having duplicate implementations (in sw_emulated and sw_bls12381) is that it may cause confusion when auditing or debugging bugs. I'll have another look, maybe I can come up with some idea while @ThomasPiellard is addressing the missing test comment.

ivokub
ivokub reviewed Apr 9, 2025
View reviewed changes
@ivokub ivokub self-requested a review April 13, 2025 23:39
ivokub
ivokub approved these changes Apr 13, 2025
View reviewed changes
Copy link
Collaborator

@ivokub ivokub left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good from my side now. But before merging we need to wait for Consensys/gnark-crypto#674 to be merged to gnark-crypto master and then we need to update the dependency in this PR. Otherwise we may have non-linear dependencies and depend on go package cache in case the gnark-crypto PR is deleted.

I'll try moving AssertIsOnCurve and AssertIsOnG1 to interfaces and de-duplicate in separate PR.

@ivokub ivokub mentioned this pull request Apr 14, 2025
Merged
13 tasks
@ivokub ivokub merged commit 289cf1a into master Apr 15, 2025
5 checks passed
@ivokub ivokub deleted the pectra/precompiles branch April 15, 2025 09:04
This was referenced Apr 17, 2025
Closed
Closed
Closed
AlexandreBelling pushed a commit that referenced this pull request Apr 30, 2025
@yelhousni @ThomasPiellard
@ivokub @Tabaie @AlexandreBelling
feat: add Pectra BLS12-381 elliptic curve precompiles (#1447)
b43853b 
Co-authored-by: Thomas Piellard <[email protected]>
Co-authored-by: Ivo Kubjas <[email protected]>
Co-authored-by: Arya Tabaie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ThomasPiellard ThomasPiellard ThomasPiellard left review comments

@ivokub ivokub ivokub approved these changes

Assignees

@yelhousni yelhousni

Labels

dep: linea Issues affecting Linea downstream type: new feature

Projects

None yet

Milestone

v0.11.N

Development

Successfully merging this pull request may close these issues.

5 participants

@yelhousni @ivokub @ThomasPiellard @Tabaie