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

Implement u256 prime field #137

Merged
merged 22 commits into from
Mar 4, 2023

Conversation

GianfrancoBazzani
Copy link
Contributor

@GianfrancoBazzani GianfrancoBazzani commented Mar 2, 2023

Implement u256 prime field

Description

solves #126
solves #95

Implemented U256PrimeField and generalized Montgomery backed for avoiding code repetition and allow easy and fast future implementations of other fields with different limb lengths.

Example of the UX while using the mod:
Screenshot from 2023-03-02 18-25-44

trait aliases are experimental see issue #41517 rust-lang/rust#41517, but in the future the use of trait alias may upgrade the UX to something like this, note that instead of use general trait IsMontgomeryConfiguration<"limbs"> a trait alias would be defined, for example U256IsMontgomeryConfiguration = IsMontgomeryConfiguration<4>:

Screenshot from 2023-03-02 18-28-53

Type of change

  • New feature

Checklist

  • Linked to Github Issue
  • Unit tests added
  • This change requires new documentation.

@GianfrancoBazzani GianfrancoBazzani requested review from a team, schouhy and ajgara as code owners March 2, 2023 08:18
@codecov-commenter
Copy link

codecov-commenter commented Mar 2, 2023

Codecov Report

Merging #137 (70984d3) into main (3c681a3) will decrease coverage by 0.70%.
The diff coverage is 91.25%.

@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
- Coverage   95.71%   95.02%   -0.70%     
==========================================
  Files          49       49              
  Lines        5251     5382     +131     
==========================================
+ Hits         5026     5114      +88     
- Misses        225      268      +43     
Impacted Files Coverage Δ
crypto/src/hash/poseidon/mod.rs 96.81% <ø> (ø)
...rt_weierstrass/curves/bls12_377/field_extension.rs 75.00% <ø> (ø)
...rt_weierstrass/curves/bls12_381/field_extension.rs 10.00% <ø> (ø)
...tic_curve/short_weierstrass/curves/test_curve_2.rs 88.00% <ø> (ø)
proving-system/stark/src/lib.rs 96.99% <ø> (ø)
...src/field/fields/montgomery_backed_prime_fields.rs 91.25% <91.25%> (ø)
math/src/field/element.rs 87.71% <0.00%> (-4.39%) ⬇️

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

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.

Looks good!

As a side note, use git move when renaming files, so git realizes it's the same one edited.

@ilitteri ilitteri added this pull request to the merge queue Mar 4, 2023
Merged via the queue into lambdaclass:main with commit f9ef87e Mar 4, 2023
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.

5 participants