Remove Groth16/SnarkPack+. #774
Conversation
porcuquine
dismissed stale reviews from samuelburnham and arthurpaulino
via
d87c5c0
porcuquine
force-pushed
the
remove-snarkpack
branch
from
d87c5c0 to
3d3791e
Compare
porcuquine
force-pushed
the
remove-snarkpack
branch
from
3d3791e to
a44c11b
Compare
|
Hmmmm, this is now failing on the computed constraint count in the LEM It's not obvious to me why this is suddenly showing up now, especially since I thought LEM circuit generation was self-contained. @arthurpaulino @gabriel-barrett Any ideas? |
|
AAh! The cost of For more context: we count constraints in two ways. There's the normal way, synthesizing a circuit with a blank frame and then using the API to get the number of constraints. And we count constraints with a function that also serves as a live documentation for how the number of constraints in a LEM grows. Both counts must match the hardcoded value. |
arthurpaulino
force-pushed
the
remove-snarkpack
branch
2 times, most recently
from
fc90ebd to
e74150d
Compare
arthurpaulino
force-pushed
the
remove-snarkpack
branch
from
e74150d to
24aeba6
Compare
| @@ -18,7 +18,6 @@ base64 = { workspace = true } | |||
| base-x = "0.2.11" | |||
There was a problem hiding this comment.
The removal of memmap isn't there, but I think it doesn't matter as #777 is about to re-introduce it.
| let bit_decomp_cost = match F::FIELD { | ||
| LanguageField::Pallas => 298, | ||
| LanguageField::Vesta => 301, | ||
| LanguageField::BLS12_381 => 388, |
There was a problem hiding this comment.
Why is this still a thing if we remove BLS12-381. If there are leftover bits that are not here just for testing, could we list them in an issue @arthurpaulino ?
There was a problem hiding this comment.
I'm not fully aware of all the places BLS12-381 can be found. This one is easy because LanguageField is defined in lurk::field, which is pretty central
- Follow-up of lurk-lab#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`
) * chore:: Refactor field tests to use pasta_curves instead of blstrs - Follow-up of #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` * refactor: Remove BLS12-381 support from the LanguageField enum - 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.
This PR removes support for Groth16/SnarkPack+. Now that we have SuperNova, we no longer needed the vestigial original backend in order to preserve and demonstrate multiple backend support. Moreover, the differences in capabilities and interfaces between the folding and proof-aggregation based backends have become a source of friction and maintenance overhead.
This removal will allow us to extend the
Provertrait to more fully cover the range of behaviors provided by bothNovaandSuperNova.Although it could have been removed outright, for the time being I left the BLS12-381 uses related to field genericity, so we continue to exercise it. However, this likely should be dropped in the future when that need is otherwise met. This will allow dropping the
blstrsdependency.With some more work, we can and should also remove the
bellpepper_core::Circuitimplementation forMultiFrame. It's no longer required for creating proofs, so is somewhat confusing. It is, however, still used in some value-providing tests.