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

chore:: Refactor field tests to use pasta_curves instead of blstrs #845

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

huitseeker
Copy link
Member

  • Follow-up of Remove Groth16/SnarkPack+. #774
  • Swapped out blstrs::Scalar implementation in src/field.rs with pasta_curves::pallas::Scalar
  • Readjusted tests to now run on the pasta_curve as opposed to the blstrs
  • Updated Cargo.toml by removing blstrs from both the workspace and dev dependencies
  • Removed the association of blstrs/portable from the portable feature in Cargo.toml

- Follow-up of argumentcomputer#774
- Swapped out `blstrs::Scalar` implementation in `src/field.rs` with pasta_curves::pallas::Scalar
- Readjusted tests to now run on the `pasta_curve` as opposed to the `blstrs`
- Updated `Cargo.toml` by removing `blstrs` from both the workspace and dev dependencies
- Removed the association of `blstrs/portable` from the `portable` feature in `Cargo.toml`
@huitseeker huitseeker requested a review from a team as a code owner November 3, 2023 17:06
@arthurpaulino
Copy link
Member

Some benchmarks mention the wrong field name, referring to BLS12 instead of Vesta. Should we change it here?

- Removed `BLS12_381` option across the project, affecting the enumeration of `LanguageField`, use in `src/lem/circuit.rs`, and representation in `src/cli/mod.rs`.
- Updated test and benchmarking data in `src/field.rs` and `end2end_lem.rs` to reflect these changes.
- Reestructured  `src/cli/circom.rs` directory for simplicity and updated the logic generating `r1cs` and `witness`.
- Removed "bls" feature from neptune in Cargo.toml, and updated documentation in README.md to reflect these changes.
Copy link
Member

@arthurpaulino arthurpaulino 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 💯 sure this is exhaustive since there can be overlooked typos. If we find more, we can fix in other PRs

@huitseeker huitseeker added this pull request to the merge queue Nov 3, 2023
Merged via the queue into argumentcomputer:master with commit 6d0f79c Nov 3, 2023
15 checks passed
@huitseeker huitseeker deleted the remove_blstrs branch November 3, 2023 18:27
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.

3 participants