-
Notifications
You must be signed in to change notification settings - Fork 266
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
feat!: Separate public inputs from proof in acir composer #2618
Conversation
barretenberg/cpp/src/barretenberg/dsl/acir_proofs/acir_composer.cpp
Outdated
Show resolved
Hide resolved
Orthogonal to this, Charlie noted that this PR continues the mix usage of camel case and snake case -- I'll run clang-fix in another PR because I can imagine that changing a lot of files that are unrelated to the topic at hand |
Coming back to this: error is possibly because we are reading the public inputs file when there are no public inputs. read_file has a if size < 0 condition which might be getting triggered |
…d_file will throw
Would this resolve #1315 (not just related to)? |
Seems like this has approvals but needs conflict resolution… @kevaundray sorry to tag you again, but as expected we have people at EthOnline dealing with this… |
This should not be affecting people at EthOnline since I changed the interface to uncover any problems here: https://github.com/noir-lang/noir/blob/88682da87ffc9e26da5c9e4b5a4d8e62a6ee43c6/tooling/noir_js_types/src/types.ts#L17 They should receive a ProofData struct and can pull out the public inputs -- is this not the case? |
Benchmark resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Values are compared against data from master at commit L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every circuit run across all benchmarks.
|
// One solution for this, is to have ACIR introduce the concept of a public output | ||
// which would be different to the public input. A public input would be the public values | ||
// which are inputted to the program and the public output would be the public values | ||
// which are returned when the program terminates. |
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.
ACIR does have this concept. It calls the first public_parameters
and the second return_values
.
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.
Should clarify that this would also be exposed as fields, so we can use it in this file
@kevaundray what happened here? |
Closing after talking to Kev, this will be done in a more robust way by crypto team |
Is there a soft ETA? (E.g. Next sprint, Q2, etc.) |
Related to #1315
Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.