chore: ECCVM shiftable column audit at lagrange_first row#22442
Open
notnotraju wants to merge 5 commits intosi/eccvm-lagrange-last-integerfrom
Open
chore: ECCVM shiftable column audit at lagrange_first row#22442notnotraju wants to merge 5 commits intosi/eccvm-lagrange-last-integerfrom
notnotraju wants to merge 5 commits intosi/eccvm-lagrange-last-integerfrom
Conversation
added 5 commits
April 9, 2026 14:33
Document why each of the 26 shiftable columns in the ECCVM is safe (or requires an explicit constraint) when masking moves lagrange_first away from the PCS-enforced zero row. Only z_perm is vulnerable, fixed by the Z_PERM_INIT subrelation. Add corruption tests for the 3 Category 3 columns (precompute_round, precompute_scalar_sum, precompute_s1hi) that are constrained by lagrange_first-activated initialization relations. TODO for the 21 Category 1/2 columns once masking-at-top lands.
Add corruption tests for shiftable columns at the lagrange_first row (row k = NUM_DISABLED_ROWS_IN_SUMCHECK with masking-at-top). - ShiftableInitializationConstraints: verify ROUND_SHIFT_ZERO, SCALAR_SUM_SHIFT_ZERO, and FIRST_SLICE_POSITIVE catch bad shifted values at exactly the lagrange_first row via single-row evaluation. - AccumulatorNotEmptyUnconstrainedAtLagrangeFirst: proves that corrupting transcript_accumulator_not_empty (+ accumulator_x/y) at the lagrange_first row is NOT caught by any existing relation. TODO: add lagrange_first * transcript_accumulator_not_empty = 0.
…cy tests - Fix doc: 5 columns were misclassified as "harmless" but are actually caught by self-consistency constraints (precompute_pc, precompute_select, transcript_msm_count, transcript_accumulator_x/y). - Add HarmlessColumnsUnconstrainedAtLagrangeFirst test: verifies the 11 truly harmless columns are dead values at the lagrange_first row (also serves as regression guard for future relation changes). - Use single-row evaluation in all new tests to avoid false positives from masking-region relation failures.
- Add SelfConsistencyColumnsCaughtAtLagrangeFirst test for 11 columns (transcript_mul, transcript_msm_count, msm_transition, msm_add, msm_double, msm_skew, msm_add1, precompute_pc, precompute_select, transcript_accumulator_x/y) — verifies at least one relation catches corruption at the lagrange_first row. - Add BoolsRelation and SetRelation to harmless and gap tests (now checks all 6 relation families except LookupRelation). - Move msm_transition from harmless to self-consistency (BoolsRelation catches non-boolean values). - Clarify transcript_accumulator_x/y dependency on accumulator_not_empty fix in the doc. - Extract shared helpers (eval_relation_at_row, expect_no_new_nonzero, has_new_nonzero, ColumnSpec) to reduce duplication.
Address review feedback: "harmless" conflated two different safety mechanisms. Now split into: - Dead (6 cols): not referenced by any relation at row k, including the set relation. Corruption is invisible. - Multiset-constrained (4 cols): appear in set relation num/den at row k. Corruption changes the multiset entry, caught by the global terminal condition (z_perm_shift = 0 at lagrange_last), not per-row. Note: single-row test can't detect this since z_perm is recomputed from corrupted data. Both categories pass the per-row harmless test (all 6 relation families). The distinction is documented in eccvm_flavor.hpp and annotated in the test column list ([dead] vs [multiset]).
3 tasks
notnotraju
added a commit
that referenced
this pull request
Apr 10, 2026
…22461) ## Summary Add `lagrange_first * transcript_accumulator_not_empty = 0` subrelation to `ECCVMTranscriptRelation`. This is a prerequisite for #22334 (masking at top of ECCVM circuit). The audit in #22442 identified that when `lagrange_first` moves to row k (away from the PCS-enforced zero row), `transcript_accumulator_not_empty` is the only shiftable column where a malicious prover can potentially set a non-zero value at the `lagrange_first` row without any existing relation catching it. Setting it to 1 disables `INFINITY_ACC_X/Y`, allowing arbitrary accumulator coordinates to be injected. ## Changes - New subrelation `ACCUMULATOR_NOT_EMPTY_INIT` in `ecc_transcript_relation.hpp` (degree 2) - Gate count updates (+174 gates from the new subrelation): - `ECCVM_RECURSIVE_VERIFIER_GATE_COUNT`: 224336 → 224510 - `CHONK_RECURSION_GATES`: 1491593 → 1491767 ## Test plan - [x] `eccvm_tests` — all 44 tests pass - [x] `stdlib_eccvm_verifier_tests` — `SingleRecursiveVerification` passes with updated gate count - [x] `dsl_tests` — `GateCountChonkRecursion` passes with updated gate count Co-authored-by: notnotraju <raju@aztec-labs.com>
critesjosh
pushed a commit
that referenced
this pull request
Apr 14, 2026
…22461) ## Summary Add `lagrange_first * transcript_accumulator_not_empty = 0` subrelation to `ECCVMTranscriptRelation`. This is a prerequisite for #22334 (masking at top of ECCVM circuit). The audit in #22442 identified that when `lagrange_first` moves to row k (away from the PCS-enforced zero row), `transcript_accumulator_not_empty` is the only shiftable column where a malicious prover can potentially set a non-zero value at the `lagrange_first` row without any existing relation catching it. Setting it to 1 disables `INFINITY_ACC_X/Y`, allowing arbitrary accumulator coordinates to be injected. ## Changes - New subrelation `ACCUMULATOR_NOT_EMPTY_INIT` in `ecc_transcript_relation.hpp` (degree 2) - Gate count updates (+174 gates from the new subrelation): - `ECCVM_RECURSIVE_VERIFIER_GATE_COUNT`: 224336 → 224510 - `CHONK_RECURSION_GATES`: 1491593 → 1491767 ## Test plan - [x] `eccvm_tests` — all 44 tests pass - [x] `stdlib_eccvm_verifier_tests` — `SingleRecursiveVerification` passes with updated gate count - [x] `dsl_tests` — `GateCountChonkRecursion` passes with updated gate count Co-authored-by: notnotraju <raju@aztec-labs.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
lagrange_firstrow under masking-at-topeccvm_flavor.hpp) why each of the 26 shiftable columns is safe or needs an explicit constraint whenlagrange_firstmoves away from the PCS-enforced zero rowtranscript_accumulator_not_emptyis unconstrained at thelagrange_firstrow — a malicious prover can set it to 1, disablingINFINITY_ACC_X/Yand injecting arbitrary accumulator coordinates undetected. Fix: addlagrange_first * transcript_accumulator_not_empty = 0New tests
ShiftableInitializationConstraints: VerifiesROUND_SHIFT_ZERO,SCALAR_SUM_SHIFT_ZERO, andFIRST_SLICE_POSITIVEcatch corruption at exactly thelagrange_firstrow (single-row evaluation)AccumulatorNotEmptyUnconstrainedAtLagrangeFirst: Proves the gap — no existing relation catches the corruption (TODO to add the constraint)Existing constraints verified
z_perm: Already fixed byZ_PERM_INIT(tested inSetRelationFailsOnZPermNonZeroAtFirstRow)precompute_round,precompute_scalar_sum,precompute_s1hi: Caught by WNAF initialization subrelationsTest plan
eccvm_tests --gtest_filter="*RelationCorruption*"— all 7 tests pass