diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp index e435fecd5b7d..2ff946444a6c 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp @@ -362,6 +362,90 @@ class ECCVMFlavor { /** * @brief Represents polynomials shifted by 1 or their evaluations, defined relative to WitnessEntities. + * + * @details + * ## Shiftable columns and the lagrange_first row + * + * The PCS enforces P[0] = 0 for shiftable columns. With masking at the top of the circuit, + * `lagrange_first` activates at row k = NUM_DISABLED_ROWS_IN_SUMCHECK, not row 0. The value P[k] is + * NOT constrained by the PCS — a malicious prover can freely choose it. + * + * For soundness, every shiftable column must either: + * (a) be explicitly constrained to zero at the lagrange_first row, or + * (b) have its unconstrained value be unable to pollute subsequent computation. + * + * The builder sets all shiftable columns to 0 at the lagrange_first row (it is the empty first row of + * each subtable). Relations enforce this in three ways: + * + * ### Explicitly constrained at lagrange_first (relations force P[k] or P[k+1] = 0) + * + * - z_perm (col 25): Z_PERM_INIT enforces `lagrange_first * z_perm = 0`. + * - precompute_round (col 20): ROUND_SHIFT_ZERO forces round_shift = 0 at lagrange_first. + * - precompute_scalar_sum (col 2): SCALAR_SUM_SHIFT_ZERO forces scalar_sum_shift = 0. + * - precompute_s1hi (col 3): FIRST_SLICE_POSITIVE constrains s1hi_shift ∈ {2, 3}. + * - transcript_accumulator_not_empty (col 22): **UNCONSTRAINED — needs fix.** See TODO in + * eccvm_relation_corruption.test.cpp. Without `lagrange_first * accumulator_not_empty = 0`, a + * malicious prover can disable INFINITY_ACC_X/Y and inject arbitrary accumulator coordinates. + * + * ### Dead: P[k] is not read by any relation at any active row + * + * These columns are not referenced by any non-trivially-active subrelation at row k. A malicious + * prover can set P[k] to anything and no per-row relation fires. This is safe because: + * - P[k] as an unshifted value at row k: every relation term vanishes (gated by + * `is_not_first_row = 0` or by selectors that are zero at the empty first row). + * - P[k] as a shifted value at row k-1: row k-1 is in the masking region where sumcheck does not + * evaluate relations. + * - P[k+1] = P_shift(k) is unconstrained by row k's relations (same gating), but IS constrained + * at row k+1 by that row's own relations (hiding row, etc.). + * + * These columns also do not appear in the set relation's numerator/denominator at row k. + * Verified by `HarmlessColumnsUnconstrainedAtLagrangeFirst` (checks all 6 relation families + * including SetRelation at row k). + * + * - precompute_dx/dy (cols 4-5), precompute_tx/ty (cols 6-7): gated by (-lagrange_first + 1). + * - msm_accumulator_x/y (cols 12-13): gated by (-lagrange_first + 1) in IDLE_ROW_PRESERVES_ACC + * and by operation selectors elsewhere. Set relation reads msm_accumulator_x/y_shift (not + * unshifted), gated by (-lagrange_first + 1) * msm_transition_shift. + * + * ### Multiset-constrained: not caught per-row, but caught by global multiset balance + * + * These columns appear in the set relation's numerator or denominator at row k. Corrupting P[k] + * changes the multiset entry at row k, which requires a matching entry in the precompute/MSM + * subtables that doesn't exist in the honest trace. The set relation catches this via its terminal + * condition (z_perm_shift = 0 at lagrange_last), not per-row. + * + * Note: our single-row test (`HarmlessColumnsUnconstrainedAtLagrangeFirst`) evaluates SetRelation + * at row k but does NOT detect this, because the test recomputes z_perm from the corrupted trace. + * The global multiset mismatch only manifests at lagrange_last. + * + * - msm_count (col 14), msm_round (col 15), msm_pc (col 17): appear in set relation denominator + * (first term: wnaf slice tuples). Not referenced by MSM/transcript/wnaf/point-table relations + * at row k (gated by is_not_first_row or operation selectors). + * - transcript_pc (col 19): appears in set relation denominator (second and third terms). Not + * referenced by transcript relation at row k (gated by is_not_first_row in PC_UPDATE; gated by + * transcript_mul = 0 and transcript_msm_transition = 0 in the set relation terms). + * + * ### Self-consistency: corruption is caught by existing relations + * + * These columns participate in relations that are NOT gated off at the lagrange_first row. + * Corrupting them produces a non-zero relation contribution, so sumcheck catches it. Some of these + * are operation selectors — setting them to non-zero activates subrelations that require consistency + * with non-shiftable columns (e.g., `op = 0` at the empty first row). + * + * - transcript_mul (col 0): OPCODE_WELL_FORMED catches mismatch with non-shiftable `op`. + * - msm_transition (col 8): BoolsRelation catches non-boolean values. + * - transcript_msm_count (col 1): MSM_COUNT_ZERO_WHEN_NOT_MUL forces msm_count = 0 when q_mul = 0. + * - msm_add (col 9): activates ADD_ACC_X/Y, ADD1_DECOMPOSITION — all require consistent data. + * - msm_double (col 10): activates DOUBLE_ACC_X/Y. + * - msm_skew (col 11): activates SKEW_ACC_X/Y, SKEW_IMPLIES_ROUND_32. + * - msm_add1 (col 16): ADD1_DECOMPOSITION (add1 - q_add - q_skew = 0). + * - precompute_pc (col 18): INACTIVE_PC forces pc = 0 when precompute_select = 0. + * - precompute_select (col 21): activates SCALAR_SUM_CHECK, ROUND_CHECK, PC_CHECK. + * - transcript_accumulator_x/y (cols 23-24): INFINITY_ACC_X/Y forces acc = 0 when + * accumulator_not_empty = 0. **WARNING**: this safety depends on transcript_accumulator_not_empty + * being constrained to 0 at the lagrange_first row. If a malicious prover sets + * accumulator_not_empty = 1, INFINITY_ACC_X/Y is disabled and acc_x/y become unconstrained. + * Fixing the transcript_accumulator_not_empty gap (above) also fixes acc_x/y. */ template class ShiftedEntities { public: diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_relation_corruption.test.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_relation_corruption.test.cpp index fe6da0f32f24..f9d808c6ebf2 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_relation_corruption.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_relation_corruption.test.cpp @@ -444,3 +444,312 @@ TEST_F(ECCVMRelationCorruptionTests, SetRelationFailsOnZPermNonZeroAtFirstRow) EXPECT_EQ(failures.at(ECCVMSetRelationImpl::Z_PERM_INIT), first_row) << "Failure should be at lagrange_first row"; } + +/** + * @brief Shiftable columns constrained by lagrange_first-activated initialization relations. + * + * @details Corrupting the *shifted* value (P[k+1]) of these columns at the lagrange_first row + * must be caught by specific initialization subrelations in the WNAF relation. + */ +TEST_F(ECCVMRelationCorruptionTests, ShiftableInitializationConstraints) +{ + const size_t first_row = NUM_DISABLED_ROWS_IN_SUMCHECK; + RelationParameters params{}; + + // Evaluate the WNAF relation at exactly the lagrange_first row, returning per-subrelation values. + auto eval_wnaf_at_first_row = [&](const auto& polys) { + typename ECCVMWnafRelation::SumcheckArrayOfValuesOverSubrelations result{}; + for (auto& e : result) { + e = FF(0); + } + ECCVMWnafRelation::accumulate(result, polys.get_row(first_row), params, FF(1)); + return result; + }; + + // precompute_round_shift must be 0 at lagrange_first (ROUND_SHIFT_ZERO) + { + auto polys = build_valid_eccvm_msm_state(); + ASSERT_EQ(polys.lagrange_first[first_row], FF(1)); + auto clean = eval_wnaf_at_first_row(polys); + ASSERT_EQ(clean[ECCVMWnafRelationImpl::ROUND_SHIFT_ZERO], FF(0)) << "Baseline should be zero"; + + polys.precompute_round.at(first_row + 1) = FF(5); + polys.set_shifted(); + auto dirty = eval_wnaf_at_first_row(polys); + EXPECT_NE(dirty[ECCVMWnafRelationImpl::ROUND_SHIFT_ZERO], FF(0)) + << "ROUND_SHIFT_ZERO must catch non-zero precompute_round_shift at lagrange_first row"; + } + + // precompute_scalar_sum_shift must be 0 at lagrange_first (SCALAR_SUM_SHIFT_ZERO) + { + auto polys = build_valid_eccvm_msm_state(); + auto clean = eval_wnaf_at_first_row(polys); + ASSERT_EQ(clean[ECCVMWnafRelationImpl::SCALAR_SUM_SHIFT_ZERO], FF(0)) << "Baseline should be zero"; + + polys.precompute_scalar_sum.at(first_row + 1) = FF(42); + polys.set_shifted(); + auto dirty = eval_wnaf_at_first_row(polys); + EXPECT_NE(dirty[ECCVMWnafRelationImpl::SCALAR_SUM_SHIFT_ZERO], FF(0)) + << "SCALAR_SUM_SHIFT_ZERO must catch non-zero precompute_scalar_sum_shift at lagrange_first row"; + } + + // precompute_s1hi_shift must be in {2, 3} when precompute_select_shift != 0 (FIRST_SLICE_POSITIVE) + { + auto polys = build_valid_eccvm_msm_state(); + ASSERT_NE(polys.precompute_select[first_row + 1], FF(0)) + << "Test assumes precompute_select is active at first_row + 1"; + auto clean = eval_wnaf_at_first_row(polys); + ASSERT_EQ(clean[ECCVMWnafRelationImpl::FIRST_SLICE_POSITIVE], FF(0)) << "Baseline should be zero"; + + polys.precompute_s1hi.at(first_row + 1) = FF(0); // 0 ∉ {2, 3} + polys.set_shifted(); + auto dirty = eval_wnaf_at_first_row(polys); + EXPECT_NE(dirty[ECCVMWnafRelationImpl::FIRST_SLICE_POSITIVE], FF(0)) + << "FIRST_SLICE_POSITIVE must catch s1hi_shift not in {2,3} at lagrange_first row"; + } +} + +// Helper: evaluate a relation at a single row, returning per-subrelation values. +// Used by all single-row corruption tests below. +namespace { +template +auto eval_relation_at_row(const ProverPolynomials& polys, const RelationParameters& params, size_t row) +{ + typename Relation::SumcheckArrayOfValuesOverSubrelations result{}; + for (auto& e : result) { + e = FF(0); + } + Relation::accumulate(result, polys.get_row(row), params, FF(1)); + return result; +} + +// Check that no subrelation went from zero to non-zero between clean and dirty evaluations. +template +void expect_no_new_nonzero(const ArrayT& clean, const ArrayT& dirty, const char* rel, const char* col_name = nullptr) +{ + for (size_t i = 0; i < clean.size(); i++) { + if (clean[i] == FF(0) && dirty[i] != FF(0)) { + if (col_name) { + ADD_FAILURE() << col_name << " is NOT safe: " << rel << " subrelation " << i + << " became non-zero at lagrange_first row"; + } else { + ADD_FAILURE() << rel << " subrelation " << i << " became non-zero at lagrange_first row"; + } + } + } +} + +template bool has_new_nonzero(const ArrayT& clean, const ArrayT& dirty) +{ + for (size_t i = 0; i < clean.size(); i++) { + if (clean[i] == FF(0) && dirty[i] != FF(0)) { + return true; + } + } + return false; +} + +struct ColumnSpec { + Flavor::Polynomial ProverPolynomials::* poly; + const char* name; +}; +} // namespace + +/** + * @brief Verify that "dead" and "multiset-constrained" shiftable columns do not trigger any per-row + * relation at the lagrange_first row. + * + * @details For each column, corrupt its value at the lagrange_first row and verify that no relation + * subrelation goes from zero to non-zero at that row (single-row evaluation across all 6 relation + * families). LookupRelation is omitted: its per-row subrelation depends on the logderivative inverse + * computed from the full trace, so single-row evaluation is not meaningful. + * + * "Dead" columns (precompute_dx/dy/tx/ty, msm_accumulator_x/y) are not referenced by any relation at + * row k — the corruption is completely invisible. + * + * "Multiset-constrained" columns (msm_count/round/pc, transcript_pc) appear in the set relation's + * numerator/denominator, but corruption is caught by the GLOBAL multiset balance (z_perm_shift = 0 at + * lagrange_last), not per-row. Our single-row test doesn't detect this because z_perm is recomputed + * from the corrupted trace. The global constraint nonetheless prevents exploitation. + * + * This test also serves as a regression guard: if a future change adds a relation term that references + * one of these columns at the lagrange_first row without proper gating, this test will fail. + */ +TEST_F(ECCVMRelationCorruptionTests, HarmlessColumnsUnconstrainedAtLagrangeFirst) +{ + const size_t first_row = NUM_DISABLED_ROWS_IN_SUMCHECK; + + // Dead: not referenced by any relation at row k (including set relation) + // Multiset-constrained: appear in set relation num/den but caught by global balance, not per-row + std::vector columns = { + // Dead + { &ProverPolynomials::precompute_dx, "precompute_dx (col 4) [dead]" }, + { &ProverPolynomials::precompute_dy, "precompute_dy (col 5) [dead]" }, + { &ProverPolynomials::precompute_tx, "precompute_tx (col 6) [dead]" }, + { &ProverPolynomials::precompute_ty, "precompute_ty (col 7) [dead]" }, + { &ProverPolynomials::msm_accumulator_x, "msm_accumulator_x (col 12) [dead]" }, + { &ProverPolynomials::msm_accumulator_y, "msm_accumulator_y (col 13) [dead]" }, + // Multiset-constrained (global balance catches, not per-row) + { &ProverPolynomials::msm_count, "msm_count (col 14) [multiset]" }, + { &ProverPolynomials::msm_round, "msm_round (col 15) [multiset]" }, + { &ProverPolynomials::msm_pc, "msm_pc (col 17) [multiset]" }, + { &ProverPolynomials::transcript_pc, "transcript_pc (col 19) [multiset]" }, + }; + + for (const auto& col : columns) { + auto polynomials = build_valid_eccvm_msm_state(); + ASSERT_EQ(polynomials.lagrange_first[first_row], FF(1)); + + // SetRelation needs grand product params + auto params = compute_full_relation_params(polynomials); + + // Evaluate all six relation families at exactly the lagrange_first row (clean baseline) + auto tx_clean = eval_relation_at_row>(polynomials, params, first_row); + auto msm_clean = eval_relation_at_row>(polynomials, params, first_row); + auto wnaf_clean = eval_relation_at_row>(polynomials, params, first_row); + auto pt_clean = eval_relation_at_row>(polynomials, params, first_row); + auto bools_clean = eval_relation_at_row>(polynomials, params, first_row); + auto set_clean = eval_relation_at_row>(polynomials, params, first_row); + + // Corrupt + (polynomials.*col.poly).at(first_row) = FF::random_element(&engine); + polynomials.set_shifted(); + + // Re-evaluate after corruption + auto tx_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto msm_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto wnaf_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto pt_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto bools_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto set_dirty = eval_relation_at_row>(polynomials, params, first_row); + + expect_no_new_nonzero(tx_clean, tx_dirty, "TranscriptRelation", col.name); + expect_no_new_nonzero(msm_clean, msm_dirty, "MSMRelation", col.name); + expect_no_new_nonzero(wnaf_clean, wnaf_dirty, "WnafRelation", col.name); + expect_no_new_nonzero(pt_clean, pt_dirty, "PointTableRelation", col.name); + expect_no_new_nonzero(bools_clean, bools_dirty, "BoolsRelation", col.name); + expect_no_new_nonzero(set_clean, set_dirty, "SetRelation", col.name); + } +} + +/** + * @brief Verify that "self-consistency" shiftable columns ARE caught by existing relations. + * + * @details These columns participate in relations that are NOT gated off at the lagrange_first row. + * Corrupting them must produce a non-zero relation contribution, proving sumcheck would catch it. + * We check all six relation families (excluding LookupRelation for the same reason as above). + */ +TEST_F(ECCVMRelationCorruptionTests, SelfConsistencyColumnsCaughtAtLagrangeFirst) +{ + const size_t first_row = NUM_DISABLED_ROWS_IN_SUMCHECK; + + std::vector self_consistency_columns = { + { &ProverPolynomials::transcript_mul, "transcript_mul (col 0)" }, + { &ProverPolynomials::transcript_msm_count, "transcript_msm_count (col 1)" }, + { &ProverPolynomials::msm_transition, "msm_transition (col 8)" }, + { &ProverPolynomials::msm_add, "msm_add (col 9)" }, + { &ProverPolynomials::msm_double, "msm_double (col 10)" }, + { &ProverPolynomials::msm_skew, "msm_skew (col 11)" }, + { &ProverPolynomials::msm_add1, "msm_add1 (col 16)" }, + { &ProverPolynomials::precompute_pc, "precompute_pc (col 18)" }, + { &ProverPolynomials::precompute_select, "precompute_select (col 21)" }, + { &ProverPolynomials::transcript_accumulator_x, "transcript_accumulator_x (col 23)" }, + { &ProverPolynomials::transcript_accumulator_y, "transcript_accumulator_y (col 24)" }, + }; + + for (const auto& col : self_consistency_columns) { + auto polynomials = build_valid_eccvm_msm_state(); + ASSERT_EQ(polynomials.lagrange_first[first_row], FF(1)); + RelationParameters params{}; + + // Evaluate all relation families at the lagrange_first row (clean baseline) + auto tx_clean = eval_relation_at_row>(polynomials, params, first_row); + auto msm_clean = eval_relation_at_row>(polynomials, params, first_row); + auto wnaf_clean = eval_relation_at_row>(polynomials, params, first_row); + auto pt_clean = eval_relation_at_row>(polynomials, params, first_row); + auto bools_clean = eval_relation_at_row>(polynomials, params, first_row); + + // Corrupt + (polynomials.*col.poly).at(first_row) = FF::random_element(&engine); + polynomials.set_shifted(); + + // Re-evaluate + auto tx_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto msm_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto wnaf_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto pt_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto bools_dirty = eval_relation_at_row>(polynomials, params, first_row); + + // At least one relation must catch the corruption + bool caught = has_new_nonzero(tx_clean, tx_dirty) || has_new_nonzero(msm_clean, msm_dirty) || + has_new_nonzero(wnaf_clean, wnaf_dirty) || has_new_nonzero(pt_clean, pt_dirty) || + has_new_nonzero(bools_clean, bools_dirty); + EXPECT_TRUE(caught) << col.name << " corruption at lagrange_first row was NOT caught by any relation"; + } +} + +// TODO(@notnotraju): +// Add constraint `lagrange_first * transcript_accumulator_not_empty = 0` to ECCVMTranscriptRelation. +// Without it, a malicious prover can set accumulator_not_empty = 1 at the lagrange_first row, +// disabling INFINITY_ACC_X/Y and injecting arbitrary accumulator coordinates undetected. +// Once the constraint is added, flip this test: EXPECT_FALSE(no_new_nonzero(...)) for TranscriptRelation. +// +// Note: transcript_accumulator_x/y (cols 23-24) are in the self-consistency category above ONLY because +// INFINITY_ACC_X/Y catches them when accumulator_not_empty = 0. If a malicious prover ALSO corrupts +// accumulator_not_empty to 1, then acc_x/y become unconstrained — see the test below. + +/** + * @brief Demonstrate that transcript_accumulator_not_empty is UNCONSTRAINED at the lagrange_first row. + * + * @details A malicious prover can set accumulator_not_empty = 1 at the lagrange_first row, which + * disables INFINITY_ACC_X/Y (the constraints that force accumulator coordinates to zero when the + * accumulator is "empty"). This allows injecting arbitrary accumulator coordinates at row k + * without any relation firing. + * + * This test proves the gap exists: all six ECCVM relation families evaluate to the same values + * at the lagrange_first row before and after the corruption. No subrelation catches it. + * + * Fix: add `lagrange_first * transcript_accumulator_not_empty = 0` to the transcript relation. + */ +TEST_F(ECCVMRelationCorruptionTests, AccumulatorNotEmptyUnconstrainedAtLagrangeFirst) +{ + const size_t first_row = NUM_DISABLED_ROWS_IN_SUMCHECK; + + auto polynomials = build_valid_eccvm_msm_state(); + ASSERT_EQ(polynomials.lagrange_first[first_row], FF(1)); + + // SetRelation needs grand product params + auto params = compute_full_relation_params(polynomials); + + // Baseline at the lagrange_first row — all six relation families + auto tx_clean = eval_relation_at_row>(polynomials, params, first_row); + auto msm_clean = eval_relation_at_row>(polynomials, params, first_row); + auto wnaf_clean = eval_relation_at_row>(polynomials, params, first_row); + auto pt_clean = eval_relation_at_row>(polynomials, params, first_row); + auto bools_clean = eval_relation_at_row>(polynomials, params, first_row); + auto set_clean = eval_relation_at_row>(polynomials, params, first_row); + + // Corrupt: set accumulator_not_empty = 1 and inject arbitrary accumulator coordinates. + // With accumulator_not_empty = 1, is_accumulator_empty = 0, so INFINITY_ACC_X/Y no longer + // forces accumulator_x/y to zero. + polynomials.transcript_accumulator_not_empty.at(first_row) = FF(1); + polynomials.transcript_accumulator_x.at(first_row) = FF::random_element(&engine); + polynomials.transcript_accumulator_y.at(first_row) = FF::random_element(&engine); + polynomials.set_shifted(); + + // Evaluate after corruption — all six relation families + auto tx_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto msm_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto wnaf_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto pt_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto bools_dirty = eval_relation_at_row>(polynomials, params, first_row); + auto set_dirty = eval_relation_at_row>(polynomials, params, first_row); + + // No relation should catch this — that's the gap. + EXPECT_FALSE(has_new_nonzero(tx_clean, tx_dirty)) << "TranscriptRelation should not catch this"; + EXPECT_FALSE(has_new_nonzero(msm_clean, msm_dirty)) << "MSMRelation should not catch this"; + EXPECT_FALSE(has_new_nonzero(wnaf_clean, wnaf_dirty)) << "WnafRelation should not catch this"; + EXPECT_FALSE(has_new_nonzero(pt_clean, pt_dirty)) << "PointTableRelation should not catch this"; + EXPECT_FALSE(has_new_nonzero(bools_clean, bools_dirty)) << "BoolsRelation should not catch this"; + EXPECT_FALSE(has_new_nonzero(set_clean, set_dirty)) << "SetRelation should not catch this"; +}