From 153a08c75559583bb5a99ffea757e45f782d3b21 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Thu, 14 Jul 2022 14:39:23 -0400 Subject: [PATCH 1/2] query_any: panic if query_fixed is called with non-cur Rotation. --- halo2_proofs/CHANGELOG.md | 4 +++ halo2_proofs/src/plonk/circuit.rs | 32 ++++++++++++------- .../src/plonk/permutation/verifier.rs | 25 ++++----------- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/halo2_proofs/CHANGELOG.md b/halo2_proofs/CHANGELOG.md index a98c82eb..3992fefb 100644 --- a/halo2_proofs/CHANGELOG.md +++ b/halo2_proofs/CHANGELOG.md @@ -27,6 +27,10 @@ and this project adheres to Rust's notion of - `halo2_proofs::arithmetic`: - `best_fft, recursive_butterfly_arithmetic` now use the `FftGroup` trait instead of the (now-removed) `pasta_curves::arithmetic::Group` trait. +- `halo2::proofs::circuit` + - `VirtualCells` + - `query_any` now panics if a non-`cur` `Rotation` is used with the + `Column` variant. ## [0.2.0] - 2022-06-23 ### Added diff --git a/halo2_proofs/src/plonk/circuit.rs b/halo2_proofs/src/plonk/circuit.rs index d16f3b7e..8f804139 100644 --- a/halo2_proofs/src/plonk/circuit.rs +++ b/halo2_proofs/src/plonk/circuit.rs @@ -1169,17 +1169,18 @@ impl ConstraintSystem { panic!("get_instance_query_index called for non-existent query"); } - pub(crate) fn get_any_query_index(&self, column: Column, at: Rotation) -> usize { + pub(crate) fn get_any_query_index(&self, column: Column) -> usize { match column.column_type() { - Any::Advice => { - self.get_advice_query_index(Column::::try_from(column).unwrap(), at) - } - Any::Fixed => { - self.get_fixed_query_index(Column::::try_from(column).unwrap(), at) - } - Any::Instance => { - self.get_instance_query_index(Column::::try_from(column).unwrap(), at) - } + Any::Advice => self.get_advice_query_index( + Column::::try_from(column).unwrap(), + Rotation::cur(), + ), + Any::Fixed => self + .get_fixed_query_index(Column::::try_from(column).unwrap(), Rotation::cur()), + Any::Instance => self.get_instance_query_index( + Column::::try_from(column).unwrap(), + Rotation::cur(), + ), } } @@ -1526,11 +1527,20 @@ impl<'a, F: Field> VirtualCells<'a, F> { } /// Query an Any column at a relative position + /// + /// # Panics + /// + /// Panics if query_fixed is called with a non-cur Rotation. pub fn query_any>>(&mut self, column: C, at: Rotation) -> Expression { let column = column.into(); match column.column_type() { Any::Advice => self.query_advice(Column::::try_from(column).unwrap(), at), - Any::Fixed => self.query_fixed(Column::::try_from(column).unwrap(), at), + Any::Fixed => { + if at != Rotation::cur() { + panic!("Fixed columns can only be queried at the current rotation"); + } + self.query_fixed(Column::::try_from(column).unwrap(), at) + } Any::Instance => self.query_instance(Column::::try_from(column).unwrap(), at), } } diff --git a/halo2_proofs/src/plonk/permutation/verifier.rs b/halo2_proofs/src/plonk/permutation/verifier.rs index 903efb9e..bf3d8f5c 100644 --- a/halo2_proofs/src/plonk/permutation/verifier.rs +++ b/halo2_proofs/src/plonk/permutation/verifier.rs @@ -162,16 +162,9 @@ impl Evaluated { for (eval, permutation_eval) in columns .iter() .map(|&column| match column.column_type() { - Any::Advice => { - advice_evals[vk.cs.get_any_query_index(column, Rotation::cur())] - } - Any::Fixed => { - fixed_evals[vk.cs.get_any_query_index(column, Rotation::cur())] - } - Any::Instance => { - instance_evals - [vk.cs.get_any_query_index(column, Rotation::cur())] - } + Any::Advice => advice_evals[vk.cs.get_any_query_index(column)], + Any::Fixed => fixed_evals[vk.cs.get_any_query_index(column)], + Any::Instance => instance_evals[vk.cs.get_any_query_index(column)], }) .zip(permutation_evals.iter()) { @@ -182,15 +175,9 @@ impl Evaluated { let mut current_delta = (*beta * &*x) * &(C::Scalar::DELTA.pow_vartime([(chunk_index * chunk_len) as u64])); for eval in columns.iter().map(|&column| match column.column_type() { - Any::Advice => { - advice_evals[vk.cs.get_any_query_index(column, Rotation::cur())] - } - Any::Fixed => { - fixed_evals[vk.cs.get_any_query_index(column, Rotation::cur())] - } - Any::Instance => { - instance_evals[vk.cs.get_any_query_index(column, Rotation::cur())] - } + Any::Advice => advice_evals[vk.cs.get_any_query_index(column)], + Any::Fixed => fixed_evals[vk.cs.get_any_query_index(column)], + Any::Instance => instance_evals[vk.cs.get_any_query_index(column)], }) { right *= &(eval + ¤t_delta + &*gamma); current_delta *= &C::Scalar::DELTA; From 41c87eac0f9766dc36af94291ae8537581b1272b Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Thu, 14 Jul 2022 15:08:34 -0400 Subject: [PATCH 2/2] Restrict query_fixed to current Rotation. query_fixed no longer takes a Rotation argument and can only be used to query the current rotation. --- halo2_gadgets/src/ecc/chip/mul_fixed.rs | 6 ++--- halo2_gadgets/src/poseidon/pow5.rs | 10 ++++----- halo2_gadgets/src/sinsemilla/chip.rs | 4 ++-- .../src/sinsemilla/chip/generator_table.rs | 2 +- halo2_proofs/CHANGELOG.md | 2 ++ halo2_proofs/benches/plonk.rs | 8 +++---- halo2_proofs/examples/circuit-layout.rs | 10 ++++----- halo2_proofs/src/plonk/circuit.rs | 22 +++++++++---------- halo2_proofs/tests/plonk_api.rs | 12 +++++----- 9 files changed, 38 insertions(+), 38 deletions(-) diff --git a/halo2_gadgets/src/ecc/chip/mul_fixed.rs b/halo2_gadgets/src/ecc/chip/mul_fixed.rs index b6a98620..d0781056 100644 --- a/halo2_gadgets/src/ecc/chip/mul_fixed.rs +++ b/halo2_gadgets/src/ecc/chip/mul_fixed.rs @@ -137,7 +137,7 @@ impl> Config { ) -> Vec<(&'static str, Expression)> { let y_p = meta.query_advice(self.add_config.y_p, Rotation::cur()); let x_p = meta.query_advice(self.add_config.x_p, Rotation::cur()); - let z = meta.query_fixed(self.fixed_z, Rotation::cur()); + let z = meta.query_fixed(self.fixed_z); let u = meta.query_advice(self.u, Rotation::cur()); let window_pow: Vec> = (0..H) @@ -150,9 +150,7 @@ impl> Config { let interpolated_x = window_pow.iter().zip(self.lagrange_coeffs.iter()).fold( Expression::Constant(pallas::Base::zero()), - |acc, (window_pow, coeff)| { - acc + (window_pow.clone() * meta.query_fixed(*coeff, Rotation::cur())) - }, + |acc, (window_pow, coeff)| acc + (window_pow.clone() * meta.query_fixed(*coeff)), ); // Check interpolation of x-coordinate diff --git a/halo2_gadgets/src/poseidon/pow5.rs b/halo2_gadgets/src/poseidon/pow5.rs index a8e67112..7e7aa45b 100644 --- a/halo2_gadgets/src/poseidon/pow5.rs +++ b/halo2_gadgets/src/poseidon/pow5.rs @@ -102,7 +102,7 @@ impl Pow5Chip { let expr = (0..WIDTH) .map(|idx| { let state_cur = meta.query_advice(state[idx], Rotation::cur()); - let rc_a = meta.query_fixed(rc_a[idx], Rotation::cur()); + let rc_a = meta.query_fixed(rc_a[idx]); pow_5(state_cur + rc_a) * m_reg[next_idx][idx] }) .reduce(|acc, term| acc + term) @@ -117,8 +117,8 @@ impl Pow5Chip { let cur_0 = meta.query_advice(state[0], Rotation::cur()); let mid_0 = meta.query_advice(partial_sbox, Rotation::cur()); - let rc_a0 = meta.query_fixed(rc_a[0], Rotation::cur()); - let rc_b0 = meta.query_fixed(rc_b[0], Rotation::cur()); + let rc_a0 = meta.query_fixed(rc_a[0]); + let rc_b0 = meta.query_fixed(rc_b[0]); let s_partial = meta.query_selector(s_partial); @@ -127,7 +127,7 @@ impl Pow5Chip { let mid = mid_0.clone() * m_reg[idx][0]; (1..WIDTH).fold(mid, |acc, cur_idx| { let cur = meta.query_advice(state[cur_idx], Rotation::cur()); - let rc_a = meta.query_fixed(rc_a[cur_idx], Rotation::cur()); + let rc_a = meta.query_fixed(rc_a[cur_idx]); acc + (cur + rc_a) * m_reg[idx][cur_idx] }) }; @@ -143,7 +143,7 @@ impl Pow5Chip { }; let partial_round_linear = |idx: usize, meta: &mut VirtualCells| { - let rc_b = meta.query_fixed(rc_b[idx], Rotation::cur()); + let rc_b = meta.query_fixed(rc_b[idx]); mid(idx, meta) + rc_b - next(idx, meta) }; diff --git a/halo2_gadgets/src/sinsemilla/chip.rs b/halo2_gadgets/src/sinsemilla/chip.rs index de4bbb1b..ac4c34f7 100644 --- a/halo2_gadgets/src/sinsemilla/chip.rs +++ b/halo2_gadgets/src/sinsemilla/chip.rs @@ -87,7 +87,7 @@ where /// Derives the expression `q_s3 = (q_s2) * (q_s2 - 1)`. fn q_s3(&self, meta: &mut VirtualCells) -> Expression { let one = Expression::Constant(pallas::Base::one()); - let q_s2 = meta.query_fixed(self.q_sinsemilla2, Rotation::cur()); + let q_s2 = meta.query_fixed(self.q_sinsemilla2); q_s2.clone() * (q_s2 - one) } } @@ -203,7 +203,7 @@ where // https://p.z.cash/halo2-0.1:sinsemilla-constraints?partial meta.create_gate("Initial y_Q", |meta| { let q_s4 = meta.query_selector(config.q_sinsemilla4); - let y_q = meta.query_fixed(config.fixed_y_q, Rotation::cur()); + let y_q = meta.query_fixed(config.fixed_y_q); // Y_A = (lambda_1 + lambda_2) * (x_a - x_r) let Y_A_cur = Y_A(meta, Rotation::cur()); diff --git a/halo2_gadgets/src/sinsemilla/chip/generator_table.rs b/halo2_gadgets/src/sinsemilla/chip/generator_table.rs index a3bfa22c..fd0ff03f 100644 --- a/halo2_gadgets/src/sinsemilla/chip/generator_table.rs +++ b/halo2_gadgets/src/sinsemilla/chip/generator_table.rs @@ -40,7 +40,7 @@ impl GeneratorTableConfig { // https://p.z.cash/halo2-0.1:sinsemilla-constraints?partial meta.lookup(|meta| { let q_s1 = meta.query_selector(config.q_sinsemilla1); - let q_s2 = meta.query_fixed(config.q_sinsemilla2, Rotation::cur()); + let q_s2 = meta.query_fixed(config.q_sinsemilla2); let q_s3 = config.q_s3(meta); let q_run = q_s2 - q_s3; diff --git a/halo2_proofs/CHANGELOG.md b/halo2_proofs/CHANGELOG.md index 3992fefb..680b1dc4 100644 --- a/halo2_proofs/CHANGELOG.md +++ b/halo2_proofs/CHANGELOG.md @@ -31,6 +31,8 @@ and this project adheres to Rust's notion of - `VirtualCells` - `query_any` now panics if a non-`cur` `Rotation` is used with the `Column` variant. + - `query_fixed` now no longer takes a `Rotation` argument, + and can only be used to query the current rotation. ## [0.2.0] - 2022-06-23 ### Added diff --git a/halo2_proofs/benches/plonk.rs b/halo2_proofs/benches/plonk.rs index 7a57be8a..64e8197c 100644 --- a/halo2_proofs/benches/plonk.rs +++ b/halo2_proofs/benches/plonk.rs @@ -200,10 +200,10 @@ fn criterion_benchmark(c: &mut Criterion) { let b = meta.query_advice(b, Rotation::cur()); let c = meta.query_advice(c, Rotation::cur()); - let sa = meta.query_fixed(sa, Rotation::cur()); - let sb = meta.query_fixed(sb, Rotation::cur()); - let sc = meta.query_fixed(sc, Rotation::cur()); - let sm = meta.query_fixed(sm, Rotation::cur()); + let sa = meta.query_fixed(sa); + let sb = meta.query_fixed(sb); + let sc = meta.query_fixed(sc); + let sm = meta.query_fixed(sm); vec![a.clone() * sa + b.clone() * sb + a * b * sm - (c * sc)] }); diff --git a/halo2_proofs/examples/circuit-layout.rs b/halo2_proofs/examples/circuit-layout.rs index 71e22ddc..fad839e8 100644 --- a/halo2_proofs/examples/circuit-layout.rs +++ b/halo2_proofs/examples/circuit-layout.rs @@ -211,15 +211,15 @@ impl Circuit for MyCircuit { meta.create_gate("Combined add-mult", |meta| { let d = meta.query_advice(d, Rotation::next()); let a = meta.query_advice(a, Rotation::cur()); - let sf = meta.query_fixed(sf, Rotation::cur()); + let sf = meta.query_fixed(sf); let e = meta.query_advice(e, Rotation::prev()); let b = meta.query_advice(b, Rotation::cur()); let c = meta.query_advice(c, Rotation::cur()); - let sa = meta.query_fixed(sa, Rotation::cur()); - let sb = meta.query_fixed(sb, Rotation::cur()); - let sc = meta.query_fixed(sc, Rotation::cur()); - let sm = meta.query_fixed(sm, Rotation::cur()); + let sa = meta.query_fixed(sa); + let sb = meta.query_fixed(sb); + let sc = meta.query_fixed(sc); + let sm = meta.query_fixed(sm); vec![a.clone() * sa + b.clone() * sb + a * b * sm - (c * sc) + sf * (d * e)] }); diff --git a/halo2_proofs/src/plonk/circuit.rs b/halo2_proofs/src/plonk/circuit.rs index 8f804139..334755dd 100644 --- a/halo2_proofs/src/plonk/circuit.rs +++ b/halo2_proofs/src/plonk/circuit.rs @@ -1070,7 +1070,7 @@ impl ConstraintSystem { panic!("expression containing simple selector supplied to lookup argument"); } - let table = cells.query_fixed(table.inner(), Rotation::cur()); + let table = cells.query_fixed(table.inner()); (input, table) }) @@ -1083,17 +1083,17 @@ impl ConstraintSystem { index } - fn query_fixed_index(&mut self, column: Column, at: Rotation) -> usize { + fn query_fixed_index(&mut self, column: Column) -> usize { // Return existing query, if it exists for (index, fixed_query) in self.fixed_queries.iter().enumerate() { - if fixed_query == &(column, at) { + if fixed_query == &(column, Rotation::cur()) { return index; } } // Make a new query let index = self.fixed_queries.len(); - self.fixed_queries.push((column, at)); + self.fixed_queries.push((column, Rotation::cur())); index } @@ -1132,7 +1132,7 @@ impl ConstraintSystem { fn query_any_index(&mut self, column: Column, at: Rotation) -> usize { match column.column_type() { Any::Advice => self.query_advice_index(Column::::try_from(column).unwrap(), at), - Any::Fixed => self.query_fixed_index(Column::::try_from(column).unwrap(), at), + Any::Fixed => self.query_fixed_index(Column::::try_from(column).unwrap()), Any::Instance => { self.query_instance_index(Column::::try_from(column).unwrap(), at) } @@ -1273,7 +1273,7 @@ impl ConstraintSystem { let column = self.fixed_column(); new_columns.push(column); Expression::Fixed(FixedQuery { - index: self.query_fixed_index(column, Rotation::cur()), + index: self.query_fixed_index(column), column_index: column.index, rotation: Rotation::cur(), }) @@ -1497,12 +1497,12 @@ impl<'a, F: Field> VirtualCells<'a, F> { } /// Query a fixed column at a relative position - pub fn query_fixed(&mut self, column: Column, at: Rotation) -> Expression { - self.queried_cells.push((column, at).into()); + pub fn query_fixed(&mut self, column: Column) -> Expression { + self.queried_cells.push((column, Rotation::cur()).into()); Expression::Fixed(FixedQuery { - index: self.meta.query_fixed_index(column, at), + index: self.meta.query_fixed_index(column), column_index: column.index, - rotation: at, + rotation: Rotation::cur(), }) } @@ -1539,7 +1539,7 @@ impl<'a, F: Field> VirtualCells<'a, F> { if at != Rotation::cur() { panic!("Fixed columns can only be queried at the current rotation"); } - self.query_fixed(Column::::try_from(column).unwrap(), at) + self.query_fixed(Column::::try_from(column).unwrap()) } Any::Instance => self.query_instance(Column::::try_from(column).unwrap(), at), } diff --git a/halo2_proofs/tests/plonk_api.rs b/halo2_proofs/tests/plonk_api.rs index 72d59bb9..11e87616 100644 --- a/halo2_proofs/tests/plonk_api.rs +++ b/halo2_proofs/tests/plonk_api.rs @@ -316,15 +316,15 @@ fn plonk_api() { meta.create_gate("Combined add-mult", |meta| { let d = meta.query_advice(d, Rotation::next()); let a = meta.query_advice(a, Rotation::cur()); - let sf = meta.query_fixed(sf, Rotation::cur()); + let sf = meta.query_fixed(sf); let e = meta.query_advice(e, Rotation::prev()); let b = meta.query_advice(b, Rotation::cur()); let c = meta.query_advice(c, Rotation::cur()); - let sa = meta.query_fixed(sa, Rotation::cur()); - let sb = meta.query_fixed(sb, Rotation::cur()); - let sc = meta.query_fixed(sc, Rotation::cur()); - let sm = meta.query_fixed(sm, Rotation::cur()); + let sa = meta.query_fixed(sa); + let sb = meta.query_fixed(sb); + let sc = meta.query_fixed(sc); + let sm = meta.query_fixed(sm); vec![a.clone() * sa + b.clone() * sb + a * b * sm - (c * sc) + sf * (d * e)] }); @@ -332,7 +332,7 @@ fn plonk_api() { meta.create_gate("Public input", |meta| { let a = meta.query_advice(a, Rotation::cur()); let p = meta.query_instance(p, Rotation::cur()); - let sp = meta.query_fixed(sp, Rotation::cur()); + let sp = meta.query_fixed(sp); vec![sp * (a - p)] });