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

MapToScalarField() added for Banderwagon points #278

Merged
merged 8 commits into from
Oct 6, 2023

Conversation

advaita-saha
Copy link
Collaborator

@advaita-saha advaita-saha commented Sep 24, 2023

Fixes #277

Refere the issue #277 for the detailed specs

  • Write the mapToBaseField() function for computing $\frac{x}{y}$ and returning as an $F_p$ element
  • MapToScalarField() for converting the $F_p$ element to $F_r$, in between it is done by first converting $F_p$ -> Bytes -> $F_r$
  • Tests for mapToBaseField() & MapToScalarField()
  • Commenting and linking specs documents in the code
  • Adding the BatchMapToField(), this does the same operations but for a batch of Banderwagon points together
  • Tests for the Batch operations
  • Final checks, fixes and commenting the codebase

@advaita-saha advaita-saha changed the title MapToScalarField added for Banderwagon points MapToScalarField() added for Banderwagon points Sep 24, 2023
@advaita-saha advaita-saha marked this pull request as ready for review October 1, 2023 22:39
Copy link
Owner

@mratsim mratsim left a comment

Choose a reason for hiding this comment

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

OK overall,

I would change the array input to use openArray instead because:

  • This is more flexible, many times the input size isn't known at compile-time
  • Code size, this creates a proc per N (monomorphization), which isn't worth it for such a slow code.

batchAffine has an example on how to do this properly:

constantine/ethereum_verkle_primitives.nim Outdated Show resolved Hide resolved
constantine/ethereum_verkle_primitives.nim Outdated Show resolved Hide resolved
constantine/math/elliptic/ec_twistededwards_batch_ops.nim Outdated Show resolved Hide resolved
constantine/math/elliptic/ec_twistededwards_batch_ops.nim Outdated Show resolved Hide resolved
@advaita-saha
Copy link
Collaborator Author

I have also added the tests to nimble file

@mratsim
Copy link
Owner

mratsim commented Oct 6, 2023

Perfect, thanks!

@mratsim mratsim merged commit c97036d into mratsim:master Oct 6, 2023
6 checks passed
mratsim added a commit that referenced this pull request Jun 11, 2024
* PCS: Add vector Pedersen commitments

* IPA: unfortunately FR[Banderwagon] does not have a high-enough 2-adicity for optimized Lagrange polynomial

* polynomials: rename PolyDomainEval to PolyRootsDomainEval as it's specialized to roots of unity domain

* polynomials: introduce formal derivatives and vanishing polynomials

* CRS / protocol setups: rename truted_setups folder to prepare for transparent IPA setup

* fix: batch inversion zero edge cases, introduced in #278

* feat: refactor and add new lagrange polynomial primitives

* refactor(eth-verkle-ipa): delete barycentric_form and use polynomials impl as IPA backend
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.

Map To Scalar Field
2 participants