Skip to content
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.

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

Feat: 4-dimensional fake GLV #1296

Merged
merged 74 commits into from
Oct 25, 2024
Merged

Feat: 4-dimensional fake GLV #1296

merged 74 commits into from
Oct 25, 2024

Conversation

yelhousni
Copy link
Contributor

@yelhousni yelhousni commented Oct 9, 2024

Description

https://ethresear.ch/t/fake-glv-you-dont-need-an-efficient-endomorphism-to-implement-glv-like-scalar-multiplication-in-snark-circuits/20394/7

Type of change

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

How has this been tested?

Correctness tests are implemented in emulated/sw_emulated/point_test.go and native/sw_bls12377/g1_test.go.

How has this been benchmarked?

All benchmarks are in plonkish constraints (scs).

  • Emulated scalar multiplication in a BN254-PLONK:
scalar mul old (scs) new (scs)
Secp256k1 385,461 282,223
BN254 381,467 279,262
BW6-761 1,367,067 1,010,785
BLS12-381 539,973 390,294
  • Native scalar multiplication on BLS12-377 in a BW6-761-PLONK:
scalar mul old (scs) new (scs)
BLS12-377 3,587 3,567
  • Ethereum precompiles circuits:
precompiles old (scs) new (scs)
ECMUL (BN254) 384,782 280,123
  • PLONK recursion:
Recursion old (scs) new (scs)
BW6-761 in BN254 (emulated) 28,206,642 27,137,821

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

Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

Looks very good. From soundness perspective I only had a question that for GLV+fakeGLV should we also check that s decomposition to s1 and s2 is correct.

I'll also try to think if there is a nice way to generalize over all supported curves. It also would help for the tests where we have a lot of duplication.

std/algebra/native/sw_bls12377/g1.go Outdated Show resolved Hide resolved
std/algebra/native/sw_bls12377/g1.go Outdated Show resolved Hide resolved
std/algebra/native/sw_bls12377/g1.go Outdated Show resolved Hide resolved
std/algebra/native/sw_bls12377/g1_test.go Outdated Show resolved Hide resolved
std/algebra/emulated/sw_emulated/point.go Outdated Show resolved Hide resolved
std/math/emulated/emparams/emparams.go Outdated Show resolved Hide resolved
std/algebra/emulated/sw_emulated/point.go Show resolved Hide resolved
std/algebra/native/twistededwards/point.go Outdated Show resolved Hide resolved
std/algebra/native/twistededwards/curve_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

I'm not fully sure about the possible overflow in BLS12-377, but I think with this approach we have a slight problem. Imo it looks good for the emulated 4-dim, but not for 2-chain.

std/algebra/emulated/sw_emulated/point.go Outdated Show resolved Hide resolved
std/algebra/emulated/sw_emulated/point.go Outdated Show resolved Hide resolved
std/algebra/emulated/sw_emulated/point.go Show resolved Hide resolved
std/algebra/native/sw_bls12377/g1.go Show resolved Hide resolved
@yelhousni
Copy link
Contributor Author

I'm not fully sure about the possible overflow in BLS12-377, but I think with this approach we have a slight problem. Imo it looks good for the emulated 4-dim, but not for 2-chain.

Applied the suggestions and resolved conversations. For BLS12-377, we only use the GLV+Fake-GLV for testing purposes as it is currently more expensive constraint-wise compared to classical GLV. If we do the scalars decomposition check in non-native then definitely way more expensive.

@ivokub ivokub removed this from the v0.9.0 milestone Oct 22, 2024
@ivokub
Copy link
Collaborator

ivokub commented Oct 22, 2024

Thanks for all the changes! All good from my side now. I'll postpone the general interfaces for different curves for a separate PR (#1303)

@yelhousni yelhousni added this to the v0.11.N milestone Oct 24, 2024
@yelhousni yelhousni merged commit dd1291d into master Oct 25, 2024
5 checks passed
@yelhousni yelhousni deleted the feat/fake-GLV branch October 25, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants