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

DO NOT MERGE: GLV Implementation #211

Open
wants to merge 50 commits into
base: main
Choose a base branch
from
Open

DO NOT MERGE: GLV Implementation #211

wants to merge 50 commits into from

Conversation

missac213
Copy link

@missac213 missac213 commented Aug 23, 2022

DO NOT MERGE: Move to OpenZL.

Changes:

  • manta-crypto/src/glv: GLV scalar product implementation, including sage scripts which compute the GLV parameters for BLS and BN and correctness tests.
  • manta-benchmark: Added Benchmark trait and implemented it for GLV, which is now benchmarked. It shows the expected performance improvement. TODO: implement Benchmark trait for the other benchmarked functions, I plan to do this as part of DO NOT MERGE: Feat/test benchmark suite crypto primitives #208. Also planning to expand benchmark/ecc to the BN curve in that PR.

Performance Benefits: 31% less constraints in R1CS. See https://www.notion.so/mantanetwork/Scalar-multiplication-of-elliptic-curves-194a4e70a4f943a7acbab549d4331a4d for the full details.

Closes #209


Before we can merge this PR, please make sure that all the following items have been checked off:

  • Linked to an issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Added one line describing your change in CHANGELOG.md and added the appropriate changelog label to the PR.
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Checked that changes and commits conform to the standards outlined in CONTRIBUTING.md.

@bhgomes bhgomes marked this pull request as draft August 23, 2022 02:08
@bhgomes bhgomes added the P-low Priority: Low label Aug 29, 2022
SupremoUGH and others added 16 commits September 7, 2022 17:02
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Copy link
Contributor

@bhgomes bhgomes left a comment

Choose a reason for hiding this comment

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

Another comment:

The file structure for the benchmarking library is a bit complicated. I would recommend the following flatter structure:

    - manta-benchmark
      - src
          - lib.rs (contains Benchmark trait)
          - glv.rs (contains GLV impl of Benchmark trait)
          - ecc.rs (contains ECC benchmark definitions)

Also, see my notes in the manta-crypto/src/glv/mod.rs file on moving the AffineCurveExt trait.

manta-crypto/Cargo.toml Outdated Show resolved Hide resolved
manta-crypto/src/arkworks/glv/mod.rs Outdated Show resolved Hide resolved
manta-crypto/src/arkworks/glv/mod.rs Outdated Show resolved Hide resolved
manta-crypto/src/arkworks/glv/mod.rs Outdated Show resolved Hide resolved
manta-crypto/src/arkworks/glv/mod.rs Outdated Show resolved Hide resolved
manta-benchmark/src/benchmark/glv.rs Outdated Show resolved Hide resolved
manta-benchmark/Cargo.toml Outdated Show resolved Hide resolved
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
SupremoUGH and others added 4 commits October 20, 2022 22:28
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
bhgomes
bhgomes previously approved these changes Oct 27, 2022
SupremoUGH
SupremoUGH previously approved these changes Oct 27, 2022
@SupremoUGH SupremoUGH removed the request for review from BoyuanFeng October 27, 2022 22:42
@SupremoUGH SupremoUGH dismissed BoyuanFeng’s stale review October 27, 2022 22:44

all changes implemented, merging

Signed-off-by: Francisco Hernandez Iglesias <f.hernandeziglesias@outlook.com>
@SupremoUGH SupremoUGH dismissed stale reviews from bhgomes and themself via 8a17ad8 December 5, 2022 15:40
Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
@bhgomes bhgomes added the DO-NOT-MERGE Labels a PR that should not be merged label Dec 7, 2022
@bhgomes bhgomes changed the title GLV Implementation DO NOT MERGE: GLV Implementation Dec 7, 2022
@bhgomes bhgomes mentioned this pull request Dec 7, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cryptography Area: Issues and PRs related to Cryptography A-performance Area: Performance-related Issues and PRs changelog:added Changelog: add these changes to the `added` section of the changelog DO-NOT-MERGE Labels a PR that should not be merged P-low Priority: Low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Arkworks GLV Implementation
4 participants