-
Notifications
You must be signed in to change notification settings - Fork 219
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
fix: Account for missing indices in flattened witness map #1907
Conversation
What's the reasoning about committing bberg flattened witnesses here rather than using the ACVM format and flattening inside the bberg repo like we do for ACIR? We could have a binary similar to |
On second thought there's still some bberg dependency through the black box function support but it's much reduced. |
Yeah I think we can reduce the bberg dependency as we go along, with this change, we can confidently start removing bberg from Noir itself as bberg will be running these compiled tests on master. Long term, we want these to be uploaded to github storage and then bberg can be changed to download from that instead |
I currently have no idea how to parse the witness format output by Noir. If the more complicated format is desired its serialization format needs to be documented (or I need to be pointed to it). IIRC it was also using DEFLATE but in a pretty non standard way. I was unable to trivially get a unix cmd line tool to decompress it. Can we not at least use GzEncoder so its trivially simple to decompress (if compression is needed at all)? 1m witnesses is a 32mb file which doesn't seem too bad. |
We can switch to using GzEncoder for compression and do something like: let file = File::create("witness.gz")?;
let mut gz = GzEncoder::new(file, Compression::default()); |
Description
Fixes the flat witness format to include 32 0's in place of witness index map holes.
Includes all compiled test vectors json output, and flat witness output.
Problem*
Barretenberg wants to run a bunch of acir test vectors through it's bb and bb.js impls.
Documentation
This PR requires documentation updates when merged.
Additional Context
PR Checklist*
cargo fmt
on default settings.