-
Notifications
You must be signed in to change notification settings - Fork 99
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
Support optimized witness generation for circuit2. #190
Conversation
21013c6
to
4eee9a4
Compare
bf300d3
to
86b2111
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other TODO: this line should be updated when bellperson releases:
gbench/Cargo.toml
10:bellperson = { version = "0.25.0", default-features = false }
I pushed a commit that makes the witness-generation optimization automatically available to SAFE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM apart for the bellperson dependency of course (to be updated in 2 places). The rebase should be pretty trivial.
This is a nit: #200
FYI: #199 is changing the required tests upon merging (removing “rustfmt”, which is now integrated in “clippy” and adding “msrv”, which requires source compatibility with the declared rust-version). The PR in its current form has not run the second check, hence the “Some checks haven’t completed yet”, but that should resolve when you rebase. |
This is nothing but a rebased and refreshed argumentcomputer#190 Please refer to the authoritative commit message there. - Import and implement witness-generation for optimal Poseidon hash circuit into new file `circuit2_witness.rs` and update file `circuit2.rs` to accommodate these changes. - Private methods in `circuit2.rs` are now public and deprecated function descriptions have been updated. - The `BenchCircuit` struct in `synthesis.rs` has been refactored to include new functionalities and handle more cases. - A module `circuit2_witness` is added to `lib.rs` for exclusively witness generation.. - The `bellpepper` dependency is moved from dev-dependencies to dependencies in `Cargo.toml` (due to using witness generation). - Unit tests are introduced for the new functionalities in `circuit2_witness.rs`.
This is nothing but a rebased and refreshed argumentcomputer#190 Please refer to the authoritative commit message there. - Import and implement witness-generation for optimal Poseidon hash circuit into new file `circuit2_witness.rs` and update file `circuit2.rs` to accommodate these changes. - Private methods in `circuit2.rs` are now public and deprecated function descriptions have been updated. - The `BenchCircuit` struct in `synthesis.rs` has been refactored to include new functionalities and handle more cases. - A module `circuit2_witness` is added to `lib.rs` for exclusively witness generation.. - The `bellpepper` dependency is moved from dev-dependencies to dependencies in `Cargo.toml` (due to using witness generation). - Unit tests are introduced for the new functionalities in `circuit2_witness.rs`.
This is nothing but a rebased and refreshed #190 Please refer to the authoritative commit message there. - Import and implement witness-generation for optimal Poseidon hash circuit into new file `circuit2_witness.rs` and update file `circuit2.rs` to accommodate these changes. - Private methods in `circuit2.rs` are now public and deprecated function descriptions have been updated. - The `BenchCircuit` struct in `synthesis.rs` has been refactored to include new functionalities and handle more cases. - A module `circuit2_witness` is added to `lib.rs` for exclusively witness generation.. - The `bellpepper` dependency is moved from dev-dependencies to dependencies in `Cargo.toml` (due to using witness generation). - Unit tests are introduced for the new functionalities in `circuit2_witness.rs`.
Made obsolete by #219. |
[Do not merge until updated for bellpepper.]
UPDATE: This PR now uses bellpepper. However, we intend to retain bellperson compatibilty for those requiring it. This PR switches to bellpepper to expedite upstream changes. Therefore it should also add a mechanism for using bellperson instead, when needed.
UPDATE2: I have added a
bellperson
feature, so now this only needs to be updated to exercise the feature flag on CI.UPDATE3: Actually, the new work still depends on
bellperson
git dependency, so maybe the answer is to make the new work unsupported whenbellperson
feature is selected.This PR makes use of filecoin-project/bellperson#309 to implement
SizedWitness
forposeidon
. The resulting witness is for thecircuit2
variant, with final result allocated. Several entry points are provided, to support usages where the witness should be, for example, generated directly into the constraint system, or returned for later use.The synthesis benchmark is extended to allow comparison with previous synthesis (using
BenchCS
, which carries minimal overhead). Note that thehash_allocated_witness
runs are about 16x faster than theOptimal Allocated Poseidon Circuit
runs.