-
Notifications
You must be signed in to change notification settings - Fork 234
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
refactor: share verifier rounds #4849
Conversation
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. 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.
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 processing duration by data writes.
|
inst->relation_parameters = | ||
RelationParameters<FF>{ eta, beta, gamma, public_input_delta, lookup_grand_product_delta }; | ||
auto& key = inst->verification_key; | ||
OinkVerifier<Flavor> oink_verifier{ key, transcript, domain_separator + '_' }; |
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.
I don't really like this domain_separator hack, but it works
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.
I had to add '_'
to the domain separator so that normally we wouldn't add '_'
to the manifest strings when domain_separator was empty (in the ultra_verifier case)
const auto pub_inputs_offset = | ||
transcript->template receive_from_prover<uint32_t>(domain_separator + "pub_inputs_offset"); | ||
|
||
ASSERT(circuit_size == key->circuit_size); |
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 could also be turn into if (circuit_size != key->circuit_size) return false;
instead of assert.
@@ -31,7 +32,6 @@ UltraVerifier_<Flavor>::UltraVerifier_(UltraVerifier_&& other) | |||
template <typename Flavor> UltraVerifier_<Flavor>& UltraVerifier_<Flavor>::operator=(UltraVerifier_&& other) | |||
{ | |||
key = other.key; | |||
commitments.clear(); |
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.
commitments wasn't being used...
|
||
namespace bb { | ||
|
||
template <IsUltraFlavor Flavor> struct OinkOutput { |
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.
output state from Oink verifier
@@ -0,0 +1,140 @@ | |||
#include "barretenberg/ultra_honk/oink_verifier.hpp" |
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 file is largely copied, just split up into different functions
…/aztec-packages into lx/verifiers-round-sharing
|
||
ASSERT(circuit_size == key->circuit_size); | ||
ASSERT(public_input_size == key->num_public_inputs); | ||
ASSERT(pub_inputs_offset == key->pub_inputs_offset); |
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.
probably not needed as well. This should just be overwritten by transcript pub_inputs_offset?
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.
LGTM
return false; | ||
// Copy the witness_commitments over to the VerifierCommitments | ||
for (auto [wit_comm_1, wit_comm_2] : zip_view(commitments.get_witness(), witness_commitments.get_all())) { | ||
wit_comm_1 = wit_comm_2; |
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.
Prob no reason not to std::move
these but it's also not much data.
Addresses AztecProtocol/barretenberg#882, but doesn't fully close it since recursive verifiers aren't shared.
Creates OinkVerifier which executes all of the pre-sumcheck logic in the folding and ultra honk verifiers. This does not handle the recursive verifiers.
I also create OinkOutput which serves as the output state that is passed from OinkVerifier back to UltraVerifier or ProtogalaxyVerifier.
Please read contributing guidelines and remove this line.