chore: turn on masking in eccvm by iakovenkos · Pull Request #12467 · AztecProtocol/aztec-packages

iakovenkos · 2025-03-05T09:54:46Z

All ECCVM wires are masked now.
Redefined lagrange_last in ECCVM to keep it sound.
Used commit_structured with active ranges to commit to randomized wires, as they have a huge 0 region from real_size to circuit_size - MASKING_OFFSET.

It closes the translation evaluations arc:

Goblin: verify_translation passes when ECCVM wires are masked thanks to ECCVMVerifier propagating the translation_masking_term_eval to TranslatorVerifier

Closes AztecProtocol/barretenberg#1238

…rifier

…ocol/aztec-packages into si/fix-translation-evaluations

…-evaluations-pt2

…sking term eval method

iakovenkos · 2025-03-06T13:39:39Z

barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp

    Fr& operator[](size_t index)
    {
-        ASSERT(index >= start_index && index < end_index());
+        ASSERT(index >= start_index && index <= end_index());


I was hitting this assert in commit_structured with active_regions. I think the inequality should be non-strict

it's meant to be [start, end) so I'm suspicious this is a bug

as far as i understand, it's consistent with this

auto poly_end = &polynomial[second]; scalars.insert(scalars.end(), poly_start, poly_end);

so the pointer is to the the element at position circuit_size, and the insertion excludes the poly_end

changed back to strict inequalities

iakovenkos · 2025-03-06T14:00:16Z

barretenberg/cpp/src/barretenberg/plonk_honk_shared/library/grand_product_library.hpp

 template <typename Flavor>
 void compute_grand_products(typename Flavor::ProverPolynomials& full_polynomials,
-                            bb::RelationParameters<typename Flavor::FF>& relation_parameters)
+                            bb::RelationParameters<typename Flavor::FF>& relation_parameters,


needed to modify the computation of z_perm in ECCVMProver

maramihali

Generally looks good, thanks, some nits and waiting to review the update to active_ranges that would remove the changes from Polynomial class in this PR.

barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp

maramihali · 2025-03-07T12:17:16Z

barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp

 */
 void ECCVMProver::execute_wire_commitments_round()
 {
+    // We mask the last MASKING_OFFSET coefficients of each wire polynomial. To commit to the masked wires when


this is a good idea! do you have plans to integrate it in Mega as a follow-up? if not, it would be good to add an issue or detail in the existing issues the approach taken in ECCVM

I'll expand some of the existing issues

barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp

maramihali · 2025-03-07T12:43:43Z

barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp

    // 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())) {
+        wire.mask();


in oink you introduced a function commit_to_witness_polynomials, it would be probably nice to mirror it here as well

I'd keep it as is, cause the commitment logic here is waaaay less involved than in Oink

I kinda like having the masking functionality abstracted in a function so noone in the future forgets to call mask() but not feeling very strongly about it

agree, added this method

iakovenkos · 2025-03-07T16:41:48Z

barretenberg/cpp/src/barretenberg/commitment_schemes/sparse_commitment.test.cpp

    EXPECT_EQ(result, expected_result);
 }

+/**


here's a test to catch the edge case when the last active region contains the last coeff of a poly

iakovenkos · 2025-03-07T16:42:49Z

barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp

+            // Pointer to the first element past the active range. Accessing `&polynomial[second]` directly can trigger
+            // an assertion when `second == polynomial_size`, so we compute the pointer using `polynomial.data()`
+            // to ensure safe range handling.
+            auto poly_end = polynomial.data() + (second - polynomial.start_index);


not beautiful, but don't have to do &polynomial[circuit_size]

maramihali · 2025-03-07T16:53:54Z

barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp

 */
 void ECCVMProver::execute_wire_commitments_round()
 {
+    // We mask the last MASKING_OFFSET coefficients of each wire polynomial. To commit to the masked wires when


Maybe: We add MASKING_OFFSET random values to the coefficients of each wire polynomial to not leak information via the commitment and evaluation.

.. or something along these lines

maramihali

thank you for making the changes!

maramihali · 2025-03-07T16:56:27Z

barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp

    // 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())) {
+        wire.mask();


I kinda like having the masking functionality abstracted in a function so noone in the future forgets to call mask() but not feeling very strongly about it

🤖 I have created a new Aztec Packages release --- ## [0.79.0](v0.78.1...v0.79.0) (2025-03-11) ### ⚠ BREAKING CHANGES * aggregate data for batch calls ([#12562](#12562)) ### Features * add extra attributes to target_info ([#12583](#12583)) ([c296422](c296422)) * add optional oracle resolver url in `acvm_cli` (noir-lang/noir#7630) ([cc6cdbb](cc6cdbb)) * allow to pay via sponsored fpc from cli ([#12598](#12598)) ([877de5c](877de5c)) * array concat method (noir-lang/noir#7199) ([cc6cdbb](cc6cdbb)) * **avm:** ToRadix gadget ([#12528](#12528)) ([02a7171](02a7171)) * aztec-up -v flag ([#12590](#12590)) ([6a41565](6a41565)) * **bb:** consider polynomial end_index when constructing partially evaluated multivariates ([#12530](#12530)) ([abd22cd](abd22cd)) * **config:** add fallbacks ([#12593](#12593)) ([f2f9ef3](f2f9ef3)) * **p2p:** add trusted peers mechanics ([#12447](#12447)) ([d67f7e8](d67f7e8)) * **p2p:** peer manager peer count metrics ([#12575](#12575)) ([b4891c1](b4891c1)) * provision alerts ([#12561](#12561)) ([2ea1767](2ea1767)) * Resolve callstacks in protocol circuit errors on wasm ([#12573](#12573)) ([657299b](657299b)) ### Bug Fixes * aggregate data for batch calls ([#12562](#12562)) ([bd0b3b6](bd0b3b6)) * broken kind transfer test ([#12611](#12611)) ([6e91934](6e91934)) * Cl/release fixes 2 ([#12595](#12595)) ([fc597f4](fc597f4)) * Cl/release noir refs ([#12597](#12597)) ([fdcfcaf](fdcfcaf)) * demote log ([#12626](#12626)) ([bec8953](bec8953)) * deploy method test ([#12609](#12609)) ([f2c06c2](f2c06c2)) * Do not report epoch as complete until blocks have synced ([#12638](#12638)) ([2ddfa76](2ddfa76)), closes [#12625](#12625) * Error on infinitely recursive types (noir-lang/noir#7579) ([cc6cdbb](cc6cdbb)) * get L1 tx utils config from env ([#12620](#12620)) ([d930c01](d930c01)) * Log overflow handling in reset ([#12579](#12579)) ([283b624](283b624)) * metrics update ([#12571](#12571)) ([80a5df2](80a5df2)) * **sandbox:** query release please manifest for version if in a docker container ([#12591](#12591)) ([db8ebc6](db8ebc6)) * **spartan:** setup needs kubectl ([#12580](#12580)) ([753cb33](753cb33)) * update dead partial notes link ([#12629](#12629)) ([5a1dc4c](5a1dc4c)) * update error message to display 128 bits as valid bit size (noir-lang/noir#7626) ([cc6cdbb](cc6cdbb)) * update fallback transport ([#12470](#12470)) ([88f0711](88f0711)) ### Miscellaneous * bump external pinned commits (noir-lang/noir#7640) ([cc6cdbb](cc6cdbb)) * **ci3:** add helper for uncached test introspection ([#12618](#12618)) ([9ac518b](9ac518b)) * **ci3:** better memsuspend_limit comment ([#12622](#12622)) ([de84187](de84187)) * clean up upgrade test and other small things ([#12558](#12558)) ([c28abe1](c28abe1)) * cleanup eth artifacts + misc aztec.js reorg ([#12563](#12563)) ([6623244](6623244)) * **docs:** Updated accounts page ([#12019](#12019)) ([d45dac9](d45dac9)) * Fix mac build ([#12610](#12610)) ([adceed6](adceed6)) * gemini soundness regression test ([#12570](#12570)) ([c654106](c654106)) * more sane e2e_prover/full timeout ([#12619](#12619)) ([add9d35](add9d35)) * reactivate acir_test for `regression_5045` ([#12548](#12548)) ([c89f89c](c89f89c)) * remove unnecessary trait bounds (noir-lang/noir#7635) ([cc6cdbb](cc6cdbb)) * Rename `StructDefinition` to `TypeDefinition` (noir-lang/noir#7614) ([cc6cdbb](cc6cdbb)) * replace relative paths to noir-protocol-circuits ([4f7f5c3](4f7f5c3)) * replace relative paths to noir-protocol-circuits ([0f68d11](0f68d11)) * replace relative paths to noir-protocol-circuits ([8f593ce](8f593ce)) * replace relative paths to noir-protocol-circuits ([251ae38](251ae38)) * rollup library cleanup ([#12621](#12621)) ([361fc59](361fc59)) * **sandbox:** drop cheat-codes log level ([#12586](#12586)) ([24f04c7](24f04c7)) * **sandbox:** expose anvil port ([#12599](#12599)) ([955f1b0](955f1b0)) * **testnet:** updating script for ignition, change naming ([#12566](#12566)) ([2d7b69d](2d7b69d)) * turn on masking in eccvm ([#12467](#12467)) ([aacb91a](aacb91a)) * Update Bb line counting script ([#12350](#12350)) ([7a41843](7a41843)) * update docs to reflect u128 type (noir-lang/noir#7623) ([cc6cdbb](cc6cdbb)) * Validate blobs posted to sink belong to our L2 ([#12587](#12587)) ([9578f1e](9578f1e)), closes [#12497](#12497) ### Documentation * update cli-wallet commands in profiler doc ([#12568](#12568)) ([239a4fb](239a4fb)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

iakovenkos and others added 30 commits February 17, 2025 17:03

small refactor of translation evals logic in eccvm

ead1afc

+ ECCVMTranslation Class; extended functionality in SmallSubgroupIPA

d9d9a1f

test added

5504006

test passes

acc8a51

cleaning up

4d4349e

small subgroup ipa test clean-up

3aa169b

propagate translation challenges from eccvm verifier to translator ve…

6628ec7

…rifier

slightly more sound

a51d492

IndependentVKHash test added to Goblin

bfaab51

Merge branch 'master' into si/fix-translation-evaluations

b4eaa6b

fix build

b21a2b2

translation labels no longer constexpr to fix build

bcc3284

Merge branch 'master' into si/fix-translation-evaluations

ae74a81

first steps: extending the eccvm prover

8af3dbf

renamed constants + docs

7d8e932

Merge branch 'master' into si/fix-translation-evaluations

a18fc20

fix build

98f29b0

Merge branch 'si/fix-translation-evaluations' of github.com:AztecProt…

9eec4c3

…ocol/aztec-packages into si/fix-translation-evaluations

Merge branch 'si/fix-translation-evaluations' into si/fix-translation…

7d2a2ee

…-evaluations-pt2

integration + clean-up

bfe9f09

wip: recursive verifiers

896cd45

Merge branch 'master' into si/fix-translation-evaluations-pt2

8731224

build fixed

e08c1fe

fix tests

790a71d

cleaning up

c9b6d5f

ECCVMProver docs explaining the SmallSubgroupIPA usage + shift the ma…

44ea7b1

…sking term eval method

more consistency in consts

e1704c7

more clean-up

f449343

polishing

2b498f0

Merge branch 'master' into si/fix-translation-evaluations-pt2

77fc71d

iakovenkos and others added 3 commits March 6, 2025 12:13

Merge branch 'master' into si/fix-translation-evaluations-pt2

233b355

Merge branch 'si/fix-translation-evaluations-pt2' into si/eccvm-zk-on

ce6a6ae

Merge branch 'si/fix-translation-evaluations-pt2' into si/eccvm-zk-on

cc78cdb

iakovenkos marked this pull request as ready for review March 6, 2025 11:20

iakovenkos added 3 commits March 6, 2025 12:21

Merge branch 'master' into si/fix-translation-evaluations-pt2

06efe1f

fix build

471a9ad

Merge branch 'si/fix-translation-evaluations-pt2' into si/eccvm-zk-on

bc416fa

Base automatically changed from si/fix-translation-evaluations-pt2 to master March 6, 2025 13:21

iakovenkos self-assigned this Mar 6, 2025

Merge branch 'master' into si/eccvm-zk-on

947bbad

iakovenkos commented Mar 6, 2025

View reviewed changes

use mask() + better constants naming

8ddc501

iakovenkos commented Mar 6, 2025

View reviewed changes

iakovenkos requested a review from maramihali March 6, 2025 14:00

iakovenkos and others added 2 commits March 6, 2025 15:47

fix tests

12b7d41

Merge branch 'master' into si/eccvm-zk-on

3243380

maramihali reviewed Mar 7, 2025

View reviewed changes

iakovenkos and others added 2 commits March 7, 2025 15:57

Merge branch 'master' into si/eccvm-zk-on

f57d219

resolve review comments

539003a

iakovenkos commented Mar 7, 2025

View reviewed changes

maramihali reviewed Mar 7, 2025

View reviewed changes

maramihali approved these changes Mar 7, 2025

View reviewed changes

iakovenkos and others added 2 commits March 7, 2025 17:42

add commit to witness utility

141df18

Merge branch 'master' into si/eccvm-zk-on

d2c46fc

iakovenkos merged commit aacb91a into master Mar 7, 2025
7 checks passed

iakovenkos deleted the si/eccvm-zk-on branch March 7, 2025 18:26

AztecBot mentioned this pull request Mar 7, 2025

chore(master): release 0.79.0 #12578

Merged

iakovenkos mentioned this pull request Jul 25, 2025

Ensure ECCVM polynomial commitments are never points at infinity AztecProtocol/barretenberg#820

Closed

Conversation

iakovenkos commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iakovenkos Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iakovenkos Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maramihali left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maramihali left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

iakovenkos commented Mar 5, 2025 •

edited

Loading

iakovenkos Mar 6, 2025 •

edited

Loading

iakovenkos Mar 7, 2025 •

edited

Loading