-
Notifications
You must be signed in to change notification settings - Fork 615
chore: fixing the sizes of VMs in CIVC #11793
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
Changes from 14 commits
9e12eb4
0d7d3c2
8c401ee
5ad09c4
0d38b3b
34383c7
ccc9dc1
7a99d97
1dea9ad
dd2f56a
5631090
c310c8a
69cf76b
e3dc49a
f47c3d4
3ff127b
6a60788
59abff0
1b5788d
b4065bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,18 +13,20 @@ | |
| namespace bb { | ||
|
|
||
| ECCVMProver::ECCVMProver(CircuitBuilder& builder, | ||
| const bool fixed_size, | ||
| const std::shared_ptr<Transcript>& transcript, | ||
| const std::shared_ptr<Transcript>& ipa_transcript) | ||
| : transcript(transcript) | ||
| , ipa_transcript(ipa_transcript) | ||
| , fixed_size(fixed_size) | ||
| { | ||
| PROFILE_THIS_NAME("ECCVMProver(CircuitBuilder&)"); | ||
|
|
||
| // TODO(https://github.com/AztecProtocol/barretenberg/issues/939): Remove redundancy between | ||
| // ProvingKey/ProverPolynomials and update the model to reflect what's done in all other proving systems. | ||
|
|
||
| // Construct the proving key; populates all polynomials except for witness polys | ||
| key = std::make_shared<ProvingKey>(builder); | ||
| key = std::make_shared<ProvingKey>(builder, fixed_size); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the compiler was choosing the wrong constructor, so I added this condition |
||
|
|
||
| key->commitment_key = std::make_shared<CommitmentKey>(key->circuit_size); | ||
| } | ||
|
|
@@ -45,10 +47,19 @@ void ECCVMProver::execute_preamble_round() | |
| */ | ||
| void ECCVMProver::execute_wire_commitments_round() | ||
| { | ||
| auto wire_polys = key->polynomials.get_wires(); | ||
| auto labels = commitment_labels.get_wires(); | ||
| for (size_t idx = 0; idx < wire_polys.size(); ++idx) { | ||
| transcript->send_to_verifier(labels[idx], key->commitment_key->commit(wire_polys[idx])); | ||
| // Commit to wires whose length is bounded by the real size of the ECCVM | ||
| for (const auto& [wire, label] : zip_view(key->polynomials.get_wires_without_accumulators(), | ||
| commitment_labels.get_wires_without_accumulators())) { | ||
| // TODO(https://github.com/AztecProtocol/barretenberg/issues/1240) Structured Polynomials in | ||
| // ECCVM/Translator/MegaZK | ||
| PolynomialSpan<FF> wire_span = wire; | ||
| transcript->send_to_verifier(label, key->commitment_key->commit(wire_span.subspan(0, key->real_size))); | ||
| } | ||
|
|
||
| // The accumulators are populated until the 2^{CONST_ECCVM_LOG_N}, therefore we commit to a full-sized polynomial | ||
| for (const auto& [wire, label] : | ||
| zip_view(key->polynomials.get_accumulators(), commitment_labels.get_accumulators())) { | ||
| transcript->send_to_verifier(label, key->commitment_key->commit(wire)); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -58,6 +69,7 @@ void ECCVMProver::execute_wire_commitments_round() | |
| */ | ||
| void ECCVMProver::execute_log_derivative_commitments_round() | ||
| { | ||
|
|
||
| // Compute and add beta to relation parameters | ||
| auto [beta, gamma] = transcript->template get_challenges<FF>("beta", "gamma"); | ||
|
|
||
|
|
@@ -95,6 +107,7 @@ void ECCVMProver::execute_grand_product_computation_round() | |
| */ | ||
| void ECCVMProver::execute_relation_check_rounds() | ||
| { | ||
|
|
||
| using Sumcheck = SumcheckProver<Flavor>; | ||
|
|
||
| auto sumcheck = Sumcheck(key->circuit_size, transcript); | ||
|
|
@@ -199,8 +212,6 @@ void ECCVMProver::execute_pcs_rounds() | |
|
|
||
| // Produce another challenge passed as input to the translator verifier | ||
| translation_batching_challenge_v = transcript->template get_challenge<FF>("Translation:batching_challenge"); | ||
|
|
||
| vinfo("computed opening proof"); | ||
| } | ||
|
|
||
| ECCVMProof ECCVMProver::export_proof() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,13 @@ template <typename FF_> class ECCVMSetRelationImpl { | |
| 3 // left-shiftable polynomial sub-relation | ||
| }; | ||
|
|
||
| template <typename AllEntities> inline static bool skip(const AllEntities& in) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into the other relations - does not seem there are any simple skipping conditions |
||
| { | ||
| // If z_perm == z_perm_shift, this implies that none of the wire values for the present input are involved in | ||
| // non-trivial copy constraints. | ||
| return (in.z_perm - in.z_perm_shift).is_zero(); | ||
| } | ||
|
|
||
| template <typename Accumulator> static Accumulator convert_to_wnaf(const auto& s0, const auto& s1) | ||
| { | ||
| auto t = s0 + s0; | ||
|
|
||
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 is a nicely designed test but just checking the selectors here is in general not sufficient. For example, this would not catch a difference in copy constraints between the two circuits. The best thing to do is to simply check equality on the VKs themselves. At least for Mega, this requires first setting the
pcs_verification_key = nullptrin both since in general that wont point to the same address. (See for example the test GenerateResetKernelVKFromConstraints). I assume there is a similar issue for the eccvm/translatorThere 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.
thanks for pointing this out