feat: extend functionality of SmallSubgroupIPA #12244

iakovenkos · 2025-02-24T23:24:12Z

To fix the issue with translation evaluations when the masking is enabled in ECCVM, we need to prove the batched evaluation of the masking term, where by masking term we mean a small table (5 columns, 4 rows) added at the end of the Transcript polynomials.

Fortunately, we could use the SmallSubgroupIPA protocol to do this efficiently and faciliatate the integration of this fix into Goblin.

This is the first batch of changes:

Introduction of a class TranslationData designed to process the masked wires and commit to the masking term
Extension of SmallSubgroupIPA functionality - now it could accept Translation data and produce a proof of the batch opening of the masking term
Added corresponding tests to SmallSubgroupIPATests
Moved implementations to .cpp

iakovenkos · 2025-02-24T23:29:57Z

barretenberg/cpp/src/barretenberg/eccvm/CMakeLists.txt

needed to add this after moving SmallSubgroupIPA stuff to .cpp + in two other CMake files below

…om:AztecProtocol/aztec-packages into si/small-subgroup-ipa-supports-translation

iakovenkos · 2025-02-25T12:12:54Z

barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplemini.hpp

@@ -31,7 +31,7 @@ template <typename Curve> class ShpleminiProver_ {
                              std::span<FF> multilinear_challenge,
                              const std::shared_ptr<CommitmentKey<Curve>>& commitment_key,
                              const std::shared_ptr<Transcript>& transcript,
-                              const std::array<Polynomial, NUM_LIBRA_EVALUATIONS>& libra_polynomials = {},
+                              const std::array<Polynomial, NUM_SMALL_IPA_EVALUATIONS>& libra_polynomials = {},


this global const is rather a SmallSubgroupIPA constant. I haven't thought the argument would be used elsewehere, so initially called it NUM_LIBRA_EVALUATIONS

iakovenkos · 2025-02-25T12:15:01Z

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp

@@ -1,7 +1,9 @@
 #pragma once



moved impls and some docs to cpp

iakovenkos · 2025-02-25T12:16:30Z

...nberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.test.cpp

+// Simulate the interaction between the prover and the verifier leading to the consistency check performed by the
+// verifier.
+TYPED_TEST(SmallSubgroupIPATest, TranslationMaskingTermConsistency)


don't know how to name it better

iakovenkos · 2025-02-25T12:20:32Z

barretenberg/cpp/src/barretenberg/eccvm/eccvm_translation_data.hpp

+    }
+    /**
+     * @brief   Let m_0, ..., m_4 be the vectors of last 4 coeffs in each transcript poly,  we compute the concatenation
+     * \f$ (0 || m_0 || ... || m_4)\f$ in Lagrange and monomial basis and mask the latter.


here constant term is 0 because we don't need extra masking as opposed to the ZKSumcheckData-usecase (there is redundant randomness in the masking entries of the ECCVM transcript wires)

Can you codify your PR comment here into a comment in the code?

ledwards2225

Over this looks good to me. As I mentioned, it would have been a lot easier to review if the hpp/cpp separation had happened in a separate PR - something to keep in mind for next time. I wound up just skimming through everything rather than trying to figure out what was new so my review may not have the depth it otherwise would have. I left some comments around things that I think could be clarified a bit either in code or comments. I think there is sufficient code sharing here so as not to warrant something like separate classes for the different use cases here but we should monitor things to ensure that the single class remains readable if any more complexity is introduced.

ledwards2225 · 2025-02-26T18:49:15Z

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.cpp

+template <typename Flavor>
+SmallSubgroupIPAProver<Flavor>::SmallSubgroupIPAProver(std::shared_ptr<typename Flavor::Transcript>& transcript)
+    : interpolation_domain{}
+    , concatenated_polynomial(Polynomial<FF>(SUBGROUP_SIZE + 2))


Not terribly important but it seems a little funny to me to initialize some of these members in the constructor given that SUBGROUP_SIZE is static. Maybe this is just a matter of preference? Curious if @ludamad has an opinion.

I remember trying to not initialize them in the constructor and hitting memory errors

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.cpp

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.cpp

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp

iakovenkos · 2025-02-27T16:58:23Z

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp

-                                                  commitment_key->commit(batched_quotient));
-        }
-    }
+    std::array<Polynomial<FF>, 2> static compute_lagrange_polynomials(


I guess, this constant is fine, cause it's clear that we're computing 2 Lagranges

…om:AztecProtocol/aztec-packages into si/small-subgroup-ipa-supports-translation

iakovenkos · 2025-02-28T14:09:26Z

Changed big_sum to grand_sum everywhere
Gave names to magic constants 2 and 3
Removed confusing zero coefficient in front of (m_0 || m_1 || ... || m_4)
Isolated Lagrange to monomial transformation
Expanded docs + fixed doxygen markdown issues

ledwards2225

Looks great, thanks for the thorough docs and for making those updates

ledwards2225 · 2025-02-28T14:27:42Z

barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.cpp

+{
+    // Reallocate the commitment key if necessary. This is an edge case with SmallSubgroupIPA since it has
+    // polynomials that may exceed the circuit size.
+    if (commitment_key->dyadic_size < MASKED_GRAND_SUM_LENGTH) {


this is fine but just thinking out loud, maybe the right thing to do is to have each flavor define how to compute the required srs size..

good point, I'll do a small separate PR to move it to the ck allocation step in oink and Translator/ECCVM provers

After [extending SmallSubgroupIPA functionality](#12244), we integrated the changes into the ECCVM logic. Specifically, the Prover and Verifier methods `compute_translation_opening_claim` were modified to establish the correctness of the value `translation_masking_term_eval`= $\sum_i m_i(x) v^i $, where $m_i$ are univariate polynomials of degree `MASKING_OFFSET` used to mask the wires `op`, `Px`, `Py`, `z1`, and `z2`. Detailed documentation has been included in the `ECCVMProver`'s method `compute_translation_opening_claims`. The integration into Goblin/ClientIVC required minimal changes - namely, the TranslatorVerifier accepts `translation_masking_term_eval` and subtracts it from the `eccvm_opening`. Note that currently the masking is not turned on in ECCVM, so `translation_masking_term_eval`= 0.

iakovenkos added 2 commits February 24, 2025 22:30

almost compiling

a9531ec

added failure test + docs

60537ca

iakovenkos commented Feb 24, 2025

View reviewed changes

iakovenkos and others added 3 commits February 24, 2025 23:40

eccvm translation data docs

64d09df

Merge branch 'master' into si/small-subgroup-ipa-supports-translation

63799f6

Merge branch 'si/small-subgroup-ipa-supports-translation' of github.c…

a7e63c5

…om:AztecProtocol/aztec-packages into si/small-subgroup-ipa-supports-translation

iakovenkos marked this pull request as ready for review February 24, 2025 23:41

iakovenkos self-assigned this Feb 24, 2025

iakovenkos and others added 3 commits February 25, 2025 00:44

ck fix (?)

f0f8778

fix transcript tests + docs

be5c7bf

Merge branch 'master' into si/small-subgroup-ipa-supports-translation

7a97ee3

iakovenkos commented Feb 25, 2025

View reviewed changes

iakovenkos requested review from maramihali and ledwards2225 February 25, 2025 12:21

ledwards2225 removed the request for review from maramihali February 26, 2025 14:54

iakovenkos mentioned this pull request Feb 26, 2025

feat: translation evaluations: refactor + soundness fix #12051

Merged

ledwards2225 requested changes Feb 26, 2025

View reviewed changes

iakovenkos commented Feb 27, 2025

View reviewed changes

iakovenkos and others added 9 commits February 27, 2025 18:01

review addressed

ee07343

Merge branch 'si/small-subgroup-ipa-supports-translation' of github.c…

6e2d0c3

…om:AztecProtocol/aztec-packages into si/small-subgroup-ipa-supports-translation

Merge branch 'master' into si/small-subgroup-ipa-supports-translation

f54a046

Merge branch 'master' into si/small-subgroup-ipa-supports-translation

d001c13

ck fix

b886bcd

Merge branch 'si/small-subgroup-ipa-supports-translation' of github.c…

417c8cb

…om:AztecProtocol/aztec-packages into si/small-subgroup-ipa-supports-translation

Merge branch 'master' into si/small-subgroup-ipa-supports-translation

5811213

fix ck (2)

14a79db

Merge branch 'si/small-subgroup-ipa-supports-translation' of github.c…

ce5ef82

…om:AztecProtocol/aztec-packages into si/small-subgroup-ipa-supports-translation

iakovenkos requested a review from ledwards2225 February 28, 2025 14:01

ledwards2225 approved these changes Feb 28, 2025

View reviewed changes

iakovenkos merged commit 050ce57 into master Feb 28, 2025
6 checks passed

iakovenkos deleted the si/small-subgroup-ipa-supports-translation branch February 28, 2025 14:58

iakovenkos mentioned this pull request Mar 4, 2025

feat: translation evaluations with zk #12222

Merged

iakovenkos mentioned this pull request Mar 6, 2025

Translation evaluation correctness in the presence of ZK AztecProtocol/barretenberg#1174

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: extend functionality of SmallSubgroupIPA #12244

feat: extend functionality of SmallSubgroupIPA #12244

iakovenkos commented Feb 24, 2025 •

edited

Loading

iakovenkos Feb 24, 2025

iakovenkos Feb 25, 2025

iakovenkos Feb 25, 2025

iakovenkos Feb 25, 2025

iakovenkos Feb 25, 2025

ledwards2225 Feb 26, 2025

ledwards2225 left a comment

ledwards2225 Feb 26, 2025

iakovenkos Feb 27, 2025

iakovenkos Feb 27, 2025

iakovenkos commented Feb 28, 2025

ledwards2225 left a comment

ledwards2225 Feb 28, 2025

iakovenkos Feb 28, 2025

feat: extend functionality of SmallSubgroupIPA #12244

feat: extend functionality of SmallSubgroupIPA #12244

Conversation

iakovenkos commented Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ledwards2225 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iakovenkos commented Feb 28, 2025

ledwards2225 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iakovenkos commented Feb 24, 2025 •

edited

Loading