From c80022ae84114680cae14a5df94ff8593e693b17 Mon Sep 17 00:00:00 2001
From: guorong009 <dgr009@pse.dev>
Date: Mon, 10 Jun 2024 11:17:47 +0800
Subject: [PATCH] patch: add extra check in "lookup_any" api

---
 halo2_frontend/src/dev.rs                     | 105 ++++++++++++++++++
 .../src/plonk/circuit/constraint_system.rs    |   3 +
 2 files changed, 108 insertions(+)

diff --git a/halo2_frontend/src/dev.rs b/halo2_frontend/src/dev.rs
index 2d82f1916..edd8fe1ec 100644
--- a/halo2_frontend/src/dev.rs
+++ b/halo2_frontend/src/dev.rs
@@ -1573,6 +1573,111 @@ mod tests {
         .unwrap();
     }
 
+    #[test]
+    #[should_panic(
+        expected = "table expression containing only fixed column supplied to lookup_any argument, should use `lookup` api instead of `lookup_any`"
+    )]
+    fn bad_lookup_any_use_only_fixed_col() {
+        const K: u32 = 4;
+
+        #[derive(Clone)]
+        #[allow(dead_code)]
+        struct FaultyCircuitConfig {
+            a: Column<Advice>,
+            fixed_table: Column<Fixed>,
+            q: Selector,
+        }
+
+        struct FaultyCircuit {}
+
+        impl Circuit<Fp> for FaultyCircuit {
+            type Config = FaultyCircuitConfig;
+            type FloorPlanner = SimpleFloorPlanner;
+            #[cfg(feature = "circuit-params")]
+            type Params = ();
+
+            fn configure(meta: &mut ConstraintSystem<Fp>) -> Self::Config {
+                let a = meta.advice_column();
+                let q = meta.complex_selector();
+                let fixed_table = meta.fixed_column();
+
+                meta.annotate_lookup_any_column(fixed_table, || "Fixed-Table");
+                meta.enable_equality(fixed_table);
+
+                meta.lookup_any("lookup", |cells| {
+                    let a = cells.query_advice(a, Rotation::cur());
+                    let q = cells.query_selector(q);
+                    let fixed_table = cells.query_fixed(fixed_table, Rotation::cur());
+
+                    // If q is enabled, a must be in the table.
+                    // When q is not enabled, lookup the default value instead.
+                    let not_q = Expression::Constant(Fp::one()) - q.clone();
+                    let default = Expression::Constant(Fp::from(2));
+                    vec![(q * a + not_q * default, fixed_table)]
+                });
+
+                FaultyCircuitConfig { a, q, fixed_table }
+            }
+
+            fn without_witnesses(&self) -> Self {
+                Self {}
+            }
+
+            fn synthesize(
+                &self,
+                config: Self::Config,
+                mut layouter: impl Layouter<Fp>,
+            ) -> Result<(), Error> {
+                // Load Fixed lookup table with the values 1, 2, 3, 4.
+                layouter.assign_region(
+                    || "Assign dynamic table",
+                    |mut region| {
+                        for i in 0..4 {
+                            region.assign_fixed(
+                                || "Fixed from instance tables",
+                                config.fixed_table,
+                                i,
+                                || Value::known(Fp::from(i as u64 + 1)),
+                            )?;
+                        }
+                        Ok(())
+                    },
+                )?;
+
+                layouter.assign_region(
+                    || "Faulty synthesis",
+                    |mut region| {
+                        // Enable the lookup on row 0, with a = 4.
+                        config.q.enable(&mut region, 0)?;
+                        region.assign_advice(
+                            || "a = 4",
+                            config.a,
+                            0,
+                            || Value::known(Fp::from(4)),
+                        )?;
+
+                        // BUG: Assign a = 0, which doesn't exist in the table(1, 2, 3, 4)!
+                        // Enable the lookup on row 1, with a = 0. (deliberately wrong)
+                        config.q.enable(&mut region, 1)?;
+                        region.assign_advice(
+                            || "a = 0",
+                            config.a,
+                            1,
+                            || Value::known(Fp::from(0)),
+                        )?;
+
+                        region.name_column(|| "Witness example", config.a);
+
+                        Ok(())
+                    },
+                )
+            }
+        }
+
+        let prover = MockProver::run(K, &FaultyCircuit {}, vec![]).unwrap();
+        assert_eq!(prover.verify(), Ok(()));
+    }
+
     #[test]
     fn good_lookup_any() {
         const K: u32 = 4;
diff --git a/halo2_frontend/src/plonk/circuit/constraint_system.rs b/halo2_frontend/src/plonk/circuit/constraint_system.rs
index 22aa09b8b..10476829e 100644
--- a/halo2_frontend/src/plonk/circuit/constraint_system.rs
+++ b/halo2_frontend/src/plonk/circuit/constraint_system.rs
@@ -415,6 +415,9 @@ impl<F: Field> ConstraintSystem<F> {
                         "table expression supplied to lookup_any argument must include fixed column or selector"
                     );
                 }
+                if table.degree() == 1 {
+                    panic!("table expression containing only fixed column supplied to lookup_any argument, should use `lookup` api instead of `lookup_any`");
+                }
                 input.query_cells(&mut cells);
                 table.query_cells(&mut cells);
                 (input, table)