refactor: Make tests generic w.r.t. curve/group #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Weird new clippy warning only in CI
arnaucube
reviewed
Jul 20, 2023
CPerezz
requested changes
Jul 20, 2023
There was a problem hiding this comment.
I'm not in favour of merging this.
It's non-sensical to actually transform the tests into trait-dependant fns and then create new tests that call the old ones passing the type of the generic.
It just adds more code, and no apparent benefit. As whatever passes the tests for pasta, will do for any other cycle.
|
Regarding generic tests, looks like is also being done in upstream, eg: https://github.com/microsoft/Nova/blob/main/src/nifs.rs#L388 |
CPerezz
approved these changes
Jul 20, 2023
arnaucube
approved these changes
Jul 20, 2023
|
Ready to be merged |
|
CI passed before https://github.com/privacy-scaling-explorations/Nova/actions/runs/5577103414 and only comment change, merging now to make it easier for others to rebase |
hero78119
pushed a commit
to hero78119/SuperNova
that referenced
this pull request
Jan 3, 2024
* Add DigestBuilder. * Make digest and claims private. * refactor: Refactor DigestBuilder - Refactored `src/digest.rs` to replace `Vec<u8>` storage with dedicated Write I/O. - Removed optional `hasher` and introduced dedicated factory method. - Reworked digest computation and mapping into separate functions. - Merged build and digest computation to enhance coherence. - Improved type safety with Result error propagation. * Propagate DigestBuilder changes. * Fix tests. * Correct assertion for OutputSize scale. * Remove commented. * Remove dbg!. * Fixup rebase. --------- Co-authored-by: porcuquine <porcuquine@users.noreply.github.com> Co-authored-by: François Garillot <francois@garillot.net>
Ethan-000
pushed a commit
to Ethan-000/Nova
that referenced
this pull request
Jun 2, 2024
* Add DigestBuilder. * Make digest and claims private. * refactor: Refactor DigestBuilder - Refactored `src/digest.rs` to replace `Vec<u8>` storage with dedicated Write I/O. - Removed optional `hasher` and introduced dedicated factory method. - Reworked digest computation and mapping into separate functions. - Merged build and digest computation to enhance coherence. - Improved type safety with Result error propagation. * Propagate DigestBuilder changes. * Fix tests. * Correct assertion for OutputSize scale. * Remove commented. * Remove dbg!. * Fixup rebase. --------- Co-authored-by: porcuquine <porcuquine@users.noreply.github.com> Co-authored-by: François Garillot <francois@garillot.net> feat: add a digest to R1CSShape (privacy-scaling-explorations#49) * refactor: Refactor Digestible trait - Removed `to_bytes` method from the `Digestible` trait in `src/digest.rs` file. * fix: Make bincode serialization in digest.rs more rigorous - Updated `bincode::serialize_into(byte_sink, self)` with a configurable version to enable "little endian" and "fixint encoding" options. - Added a comment in `src/digest.rs` about `bincode`'s recursive length-prefixing during serialization. * refactor: Refactor digest computation using `OnceCell` and `DigestComputer` This gives up on a generic builder and instead uses an idempotent `OnceCell` + a generic digest computer to populate the digest of a structure. - this shows how to set up digest computation so it doesn't depend on the digest field, - the digest can't be set twice, - an erroneous digest can't be inferred from the serialized data. In Details: - Overhauled digest functionality in multiple files by replacing `DigestBuilder` with `DigestComputer`, significantly altering the handling of hashes. - Incorporated `once_cell::sync::OnceCell` and `ff::PrimeField` dependencies to improve performance and simplify code. - Modified `VerifierKey` and `RunningClaims` structures to include a `OnceCell` for digest, leading to a change in function calls and procedures. - Simplified `setup_running_claims` by removing error handling and directly returning `RunningClaims` type. - Adapted test functions according to the changes including the removal of unnecessary unwrapping in certain scenarios. - Updated Cargo.toml with the new dependency `once_cell` version `1.18.0`. * refactor: rename pp digest in VerifierKey to pp_digest * feat: add a digest to R1CSShape * fix: Small issues - Introduced a new assertion within the `write_bytes` method of `src/supernova/mod.rs` for validating whether the `claims` are empty - Improved code comment clarity regarding the creation of a running claim in `src/supernova/mod.rs`. Co-authored-by: porcuquine <1746729+porcuquine@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #34
Should cover all files