-
Notifications
You must be signed in to change notification settings - Fork 310
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: prepare circuit output for validation #6678
Conversation
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
just merged master as I noticed it had a fix for your CI failure :) |
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Proof generationEach column represents the number of threads used in proof generation. | Metric | | 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 kernel circuit run across all benchmarks.
Stats on running time collected for app circuits
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method | Metric | | |
…to lw/circuit_output_validation
@@ -38,10 +42,8 @@ impl PrivateKernelCircuitPublicInputsComposer { | |||
tx_request.tx_context, | |||
); | |||
|
|||
public_inputs.min_revertible_side_effect_counter = private_call_public_inputs.min_revertible_side_effect_counter; |
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.
Assigning the min counter in propagate_from_private_call
instead, which is run by both init and inner.
} | ||
|
||
#[test] | ||
fn propagate_max_block_number_request() { |
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.
Tests for propagating data are done in the tests for PrivateKernelCircuitPublicInputsComposer
.
assert( | ||
note_logs[i].note_hash_counter | ||
== self.public_inputs.end.new_note_hashes.get_unchecked(note_index).counter(), "Could not find note hash linked to note log." | ||
assert(note_index < new_note_hashes.len(), "Could not find note hash linked to note log"); |
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.
Need to check against the number of non-empty items. Otherwise, we would be allowing a log to link to counter 0.
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.
Looks great! And the reduction in constraints is enormous! Only some very small nits 🥳
for i in 0..dest.len() { // Loop through dest instead of source because we also need to check that dest is appended with empty items. | ||
should_check |= i == num_prepended_items; // Prepended items have been checked in validate_array_prepended() and can be skipped here. | ||
if should_check { | ||
is_non_empty_item &= i != items_propagated; |
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.
We can probably extract this one out of the if should_check and maybe same some constraints 🤓
for i in 0..dest.len() { | ||
should_check |= i == num_prepended_items; | ||
if should_check { | ||
is_non_empty_item &= i != items_propagated; |
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.
Same on this one!
noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr
Outdated
Show resolved
Hide resolved
* master: feat: prepare circuit output for validation (#6678) chore: stop building/publishing `acvm_backend.wasm` (#6584) chore: add bench programs (#6566) chore: make public data update requests, note hashes, and unencrypted logs readonly in TS (#6658) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg feat: update honk recursion constraint (#6545) feat: Add code-workspace and update build dirs (#6723) feat: Sync from noir (#6717) feat: folding acir programs (#6685) feat: sumcheck part of ECCVM recursive verifier instantiated as an UltraCircuit (#6413)
Changing private kernel init and inner to:
The checks are done in a new component:
private_kernel_circuit_output_validator.nr
.