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

Impl Bandersnatch curve #513

Merged
merged 14 commits into from
Sep 5, 2023
Merged

Impl Bandersnatch curve #513

merged 14 commits into from
Sep 5, 2023

Conversation

0xkitetsu-dinesh
Copy link
Contributor

Bandersnatch Curve

Description

  • Bandersnatch, a curve over BLS12-381 scalar field, with glv has efficient scalar multiplication over jubjub curve
  • This PR implements Twisted Edward form of Bandersnatch curve
  • This PR doesn't implement GLV, only the curve.
  • You can find the reference here: https://eprint.iacr.org/2021/1152.pdf

Type of change

  • New feature

Checklist

  • Linked to Github Issue
  • Unit tests added
  • This change requires new documentation.
  • Documentation has been added/updated.
  • This change is an Optimization
  • Benchmarks added/run

@0xkitetsu-dinesh 0xkitetsu-dinesh requested review from a team, schouhy and ajgara as code owners July 18, 2023 10:04
@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

Merging #513 (6281017) into main (db6ebf1) will increase coverage by 0.03%.
The diff coverage is 97.82%.

@@            Coverage Diff             @@
##             main     #513      +/-   ##
==========================================
+ Coverage   94.49%   94.52%   +0.03%     
==========================================
  Files          62       64       +2     
  Lines        8987     9079      +92     
==========================================
+ Hits         8492     8582      +90     
- Misses        495      497       +2     
Files Changed Coverage Δ
...rve/edwards/curves/bandersnatch/field_extension.rs 75.00% <75.00%> (ø)
...lliptic_curve/edwards/curves/bandersnatch/curve.rs 98.86% <98.86%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dragan2234
Copy link
Contributor

Copy-pasting the comment in case we or somebody continues working on this:

Some conclustion:

As a next step I thought creating a test for adding a point of infinity to some point, can be a generator. That should result in the same point. So like 56 + 0 = 56 if we think in integers.

But there is a problem with lambdaworks affine and/or projective traits. Neither one of those have an example of the representations of the point of infinity. I tried creating one but then got the assert error on this line:
https://github.com/lambdaclass/lambdaworks/blob/main/math/src/elliptic_curve/point.rs#L44

Another task that we have is having a GLV endomorphism implemented which is a big task, but when trying to decompose it into smaller tasks, we realized that as first step we can implement scalar decomposition which uses "Babai's nearest plane algorithm" implemented here:
https://github.com/zhenfeizhang/bandersnatch/blob/main/bandersnatch/src/curves/glv.rs#L97

This looks trivial, but uses bigint and lambdaworks doesn't have bigint (where arkworks has it's own bigint implementation, although i know there exist rust bigint libraries, arkworks uses their own).

@MauroToscano
Copy link
Collaborator

MauroToscano commented Aug 31, 2023

Table with constants used.

Function

A = "0x73EDA753299D7D483339D80809A1D80553BDA402FFFE5BFEFFFFFFFEFFFFFFFC" = "52435875175126190479447740508185965837690552500527637822603658699938581184508"

D = "0x6389C12633C267CBC66E3BF86BE3B6D8CB66677177E54F92B369F2F5188D58E7" = "45022363124591815672509500913686876175488063829319466900776701791074614335719"

Generator

(x,y,z) = ("0x29C132CC2C0B34C5743711777BBE42F32B79C022AD998465E1E71866A252AE18", "0x2A6C669EDA123E0F157D8B50BADCD586358CAD81EEE464605E3167B6CC974166", 1) = ("18886178867200960497001835917649091219057080094937609519140440539760939937304", "19188667384257783945677642223292697773471335439753913231509108946878080696678", 1)

FIeld Order

0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001 = 52435875175126190479447740508185965837690552500527637822603658699938581184513

Copy link
Collaborator

@MauroToscano MauroToscano left a comment

Choose a reason for hiding this comment

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

Fix generators not matching spec

@MauroToscano
Copy link
Collaborator

Fix generators not matching spec

Generators are good. So ignore the previous comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be field, since it's not an extension

Copy link
Contributor

Choose a reason for hiding this comment

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

@MauroToscano I just fixed the name, let me know if all looks good!

@MauroToscano MauroToscano added this pull request to the merge queue Sep 5, 2023
Merged via the queue into lambdaclass:main with commit 5413230 Sep 5, 2023
6 checks passed
@dragan2234
Copy link
Contributor

@MauroToscano Thanks for merging! We could add a "collaborator-friendly" follow-up issue on the repo regarding the GLV endomorphism (I've put some notes on the comment above) because AFAIU that's the main purpose of this curve and what makes it different from jubjub ( https://github.com/zkcrypto/jubjub ) https://github.com/zhenfeizhang/bandersnatch#examples

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.

None yet

7 participants