feat: sumcheck part of ECCVM recursive verifier instantiated as an UltraCircuit #6413
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…es into mm/eccvm-work
…rotocol/aztec-packages into mm/stdlib-verifier-commitment-key
maramihali
commented
May 28, 2024
| @@ -417,7 +417,8 @@ template <typename Flavor> class SumcheckVerifier { | |||
| bool checked = false; | |||
| //! [Final Verification Step] | |||
| if constexpr (IsRecursiveFlavor<Flavor>) { | |||
| checked = (full_honk_relation_purported_value == round.target_total_sum).get_value(); | |||
| full_honk_relation_purported_value.assert_equal(round.target_total_sum); | |||
There was a problem hiding this comment.
we don't have an == operator in bigfield
maramihali
commented
May 28, 2024
| @@ -9,20 +9,22 @@ namespace bb { | |||
| * | |||
| * @tparam Builder | |||
| */ | |||
| template <typename Builder> class VerifierCommitmentKey<stdlib::bn254<Builder>> { | |||
| template <typename Curve> class VerifierCommitmentKey { | |||
There was a problem hiding this comment.
Getting this to mirror exactly it's native counterpart which is required to construct the vk for the recursive verifier
Rumata888
reviewed
May 28, 2024
| @@ -70,6 +72,59 @@ template <typename BuilderType> class ECCVMRecursiveFlavor_ { | |||
| using Base::Base; | |||
| }; | |||
|
|
|||
| using VerifierCommitmentKey = VerifierCommitmentKey<Curve>; | |||
| /** | |||
| * @brief The verification key is responsible for storing the the commitments to the precomputed (non-witnessk) | |||
Rumata888
reviewed
May 28, 2024
| * polynomials used by the verifier. | ||
| * | ||
| * @note Note the discrepancy with what sort of data is stored here vs in the proving key. We may want to | ||
| * resolve that, and split out separate PrecomputedPolynom ials/Commitments data for clarity but also for |
Rumata888
reviewed
May 28, 2024
| @@ -75,6 +75,16 @@ template <typename Builder, typename T> class bigfield { | |||
| : bigfield(nullptr, uint256_t(native(value))) | |||
| {} | |||
|
|
|||
| // NOLINTNEXTLINE(google-runtime-int) intended behavior | |||
| bigfield(const unsigned long value) | |||
| : bigfield(nullptr, uint256_t(native(value))) | |||
There was a problem hiding this comment.
In the unsigned case there is no need to do native
Rumata888
reviewed
May 28, 2024
| typename G1::element a = generators[0]; | ||
| typename G1::element b = generators[1]; | ||
| typename G1::element c = generators[2]; | ||
| std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>(); |
There was a problem hiding this comment.
Why did you change this?
There was a problem hiding this comment.
To make it uniform to the recursive verifier but can revert
Rumata888
reviewed
May 28, 2024
| explicit ECCVMRecursiveVerifier_(Builder* builder, | ||
| const std::shared_ptr<NativeVerificationKey>& native_verifier_key); | ||
|
|
||
| bool verify_proof(const HonkProof& proof); // return type?! |
There was a problem hiding this comment.
Could you switch it to stdlib_proof
Rumata888
approved these changes
May 28, 2024
ludamad
reviewed
May 28, 2024
|
|
||
| if (circuit_size != key->circuit_size) { | ||
| return false; | ||
| for (auto [comm, label] : zip_view(commitments.get_wires(), commitment_labels.get_wires())) { |
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.
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 | | |
TomAFrench
added a commit
that referenced
this pull request
May 29, 2024
* 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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the ECCVM recursive verifier up to sumcheck (PCS incoming) with the necessary additional functionality in bigfield. It also removes some unnecessary fields in flavors that are obsolete.