Take into account Pablo's review

halo2_gadgets/src/ecc.rs

@@ -732,16 +732,22 @@ pub(crate) mod tests {
         _lookup_marker: PhantomData<Lookup>,
     }
 
+    impl<Lookup: PallasLookupRangeCheck> MyCircuit<Lookup> {
+        fn new(test_errors: bool) -> Self {
+            Self {
+                test_errors,
+                _lookup_marker: PhantomData,
+            }
+        }
+    }
+
     #[allow(non_snake_case)]
     impl<Lookup: PallasLookupRangeCheck> Circuit<pallas::Base> for MyCircuit<Lookup> {
         type Config = EccConfig<TestFixedBases, Lookup>;
         type FloorPlanner = SimpleFloorPlanner;
 
         fn without_witnesses(&self) -> Self {
-            MyCircuit {
-                test_errors: false,
-                _lookup_marker: PhantomData,
-            }
+            MyCircuit::new(false)
         }
 
         fn configure(meta: &mut ConstraintSystem<pallas::Base>) -> Self::Config {
@@ -908,21 +914,15 @@ pub(crate) mod tests {
 
     #[test]
     fn ecc_chip() {
-        let k = 11;
-        let circuit: MyCircuit<PallasLookupRangeCheckConfig> = MyCircuit {
-            test_errors: true,
-            _lookup_marker: PhantomData,
-        };
+        let k = 13;
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new(true);
         let prover = MockProver::run(k, &circuit, vec![]).unwrap();
         assert_eq!(prover.verify(), Ok(()))
     }
 
     #[test]
     fn test_ecc_chip_against_stored_circuit() {
-        let circuit: MyCircuit<PallasLookupRangeCheckConfig> = MyCircuit {
-            test_errors: false,
-            _lookup_marker: PhantomData,
-        };
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new(false);
         test_against_stored_circuit(circuit, "ecc_chip", 3872);
     }
 
@@ -935,10 +935,7 @@ pub(crate) mod tests {
         root.fill(&WHITE).unwrap();
         let root = root.titled("Ecc Chip Layout", ("sans-serif", 60)).unwrap();
 
-        let circuit: MyCircuit<PallasLookupRangeCheckConfig> = MyCircuit {
-            test_errors: false,
-            _lookup_marker: PhantomData,
-        };
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new(false);
         halo2_proofs::dev::CircuitLayout::default()
             .render(13, &circuit, &root)
             .unwrap();

halo2_gadgets/src/sinsemilla.rs

@@ -519,6 +519,14 @@ pub(crate) mod tests {
         _lookup_marker: PhantomData<Lookup>,
     }
 
+    impl<Lookup: PallasLookupRangeCheck> MyCircuit<Lookup> {
+        fn new() -> Self {
+            MyCircuit {
+                _lookup_marker: PhantomData,
+            }
+        }
+    }
+
     type MyConfig<Lookup> = (
         EccConfig<TestFixedBases, Lookup>,
         SinsemillaConfig<TestHashDomain, TestCommitDomain, TestFixedBases, Lookup>,
@@ -531,9 +539,7 @@ pub(crate) mod tests {
         type FloorPlanner = SimpleFloorPlanner;
 
         fn without_witnesses(&self) -> Self {
-            MyCircuit {
-                _lookup_marker: PhantomData,
-            }
+            MyCircuit::new()
         }
 
         #[allow(non_snake_case)]
@@ -742,18 +748,14 @@ pub(crate) mod tests {
     #[test]
     fn sinsemilla_chip() {
         let k = 11;
-        let circuit: MyCircuit<PallasLookupRangeCheckConfig> = MyCircuit {
-            _lookup_marker: PhantomData,
-        };
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new();
         let prover = MockProver::run(k, &circuit, vec![]).unwrap();
         assert_eq!(prover.verify(), Ok(()))
     }
 
     #[test]
     fn test_sinsemilla_chip_against_stored_circuit() {
-        let circuit: MyCircuit<PallasLookupRangeCheckConfig> = MyCircuit {
-            _lookup_marker: PhantomData,
-        };
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new();
         test_against_stored_circuit(circuit, "sinsemilla_chip", 4576);
     }
 
@@ -767,9 +769,7 @@ pub(crate) mod tests {
         root.fill(&WHITE).unwrap();
         let root = root.titled("SinsemillaHash", ("sans-serif", 60)).unwrap();
 
-        let circuit: MyCircuit<PallasLookupRangeCheckConfig> = MyCircuit {
-            _lookup_marker: PhantomData,
-        };
+        let circuit = MyCircuit::<PallasLookupRangeCheckConfig>::new();
         halo2_proofs::dev::CircuitLayout::default()
             .render(11, &circuit, &root)
             .unwrap();

halo2_gadgets/src/sinsemilla/chip/hash_to_point.rs

@@ -386,7 +386,6 @@ where
 
         Ok((x_a, y_a, zs))
     }
-
     #[allow(unused_variables)]
     #[allow(non_snake_case)]
     #[allow(clippy::type_complexity)]

halo2_gadgets/src/sinsemilla/merkle.rs

@@ -213,6 +213,21 @@ pub mod tests {
         _lookup_marker: PhantomData<Lookup>,
     }
 
+    impl<Lookup: PallasLookupRangeCheck> MyCircuit<Lookup> {
+        fn new(
+            leaf: Value<pallas::Base>,
+            leaf_pos: Value<u32>,
+            merkle_path: Value<[pallas::Base; MERKLE_DEPTH]>,
+        ) -> Self {
+            Self {
+                leaf,
+                leaf_pos,
+                merkle_path,
+                _lookup_marker: PhantomData,
+            }
+        }
+    }
+
     type MyConfig<Lookup> = (
         MerkleConfig<TestHashDomain, TestCommitDomain, TestFixedBases, Lookup>,
         MerkleConfig<TestHashDomain, TestCommitDomain, TestFixedBases, Lookup>,
@@ -223,12 +238,7 @@ pub mod tests {
         type FloorPlanner = SimpleFloorPlanner;
 
         fn without_witnesses(&self) -> Self {
-            MyCircuit {
-                leaf: Value::default(),
-                leaf_pos: Value::default(),
-                merkle_path: Value::default(),
-                _lookup_marker: PhantomData,
-            }
+            MyCircuit::new(Value::default(), Value::default(), Value::default())
         }
 
         fn configure(meta: &mut ConstraintSystem<pallas::Base>) -> Self::Config {
@@ -381,12 +391,11 @@ pub mod tests {
             .collect();
 
         // The root is provided as a public input in the Orchard circuit.
-        MyCircuit {
-            leaf: Value::known(leaf),
-            leaf_pos: Value::known(pos),
-            merkle_path: Value::known(path.try_into().unwrap()),
-            _lookup_marker: PhantomData,
-        }
+        MyCircuit::new(
+            Value::known(leaf),
+            Value::known(pos),
+            Value::known(path.try_into().unwrap()),
+        )
     }
 
     #[test]

halo2_gadgets/src/utilities/lookup_range_check.rs

@@ -481,8 +481,16 @@ mod tests {
     #[derive(Clone, Copy)]
     struct MyLookupCircuit<F: PrimeFieldBits, Lookup: LookupRangeCheck<F, K>> {
         num_words: usize,
-        _field_marker: PhantomData<F>,
-        _lookup_marker: PhantomData<Lookup>,
+        _marker: PhantomData<(F, Lookup)>,
+    }
+
+    impl<F: PrimeFieldBits, Lookup: LookupRangeCheck<F, K>> MyLookupCircuit<F, Lookup> {
+        fn new(num_words: usize) -> Self {
+            MyLookupCircuit {
+                num_words,
+                _marker: PhantomData,
+            }
+        }
     }
 
     impl<F: PrimeFieldBits, Lookup: LookupRangeCheck<F, K> + std::clone::Clone> Circuit<F>
@@ -492,11 +500,7 @@ mod tests {
         type FloorPlanner = SimpleFloorPlanner;
 
         fn without_witnesses(&self) -> Self {
-            MyLookupCircuit {
-                num_words: self.num_words,
-                _field_marker: PhantomData,
-                _lookup_marker: PhantomData,
-            }
+            MyLookupCircuit::new(self.num_words)
         }
 
         fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config {
@@ -571,25 +575,14 @@ mod tests {
 
     #[test]
     fn lookup_range_check() {
-        let circuit: MyLookupCircuit<pallas::Base, PallasLookupRangeCheckConfig> =
-            MyLookupCircuit {
-                num_words: 6,
-                _field_marker: PhantomData,
-                _lookup_marker: PhantomData,
-            };
-
+        let circuit = MyLookupCircuit::<pallas::Base, PallasLookupRangeCheckConfig>::new(6);
         let prover = MockProver::<pallas::Base>::run(11, &circuit, vec![]).unwrap();
         assert_eq!(prover.verify(), Ok(()));
     }
 
     #[test]
     fn test_lookup_range_check_against_stored_circuit() {
-        let circuit: MyLookupCircuit<pallas::Base, PallasLookupRangeCheckConfig> =
-            MyLookupCircuit {
-                num_words: 6,
-                _field_marker: PhantomData,
-                _lookup_marker: PhantomData,
-            };
+        let circuit = MyLookupCircuit::<pallas::Base, PallasLookupRangeCheckConfig>::new(6);
         test_against_stored_circuit(circuit, "lookup_range_check", 1888);
     }
 
@@ -600,18 +593,24 @@ mod tests {
         _lookup_marker: PhantomData<Lookup>,
     }
 
+    impl<F: PrimeFieldBits, Lookup: LookupRangeCheck<F, K>> MyShortRangeCheckCircuit<F, Lookup> {
+        fn new(element: Value<F>, num_bits: usize) -> Self {
+            MyShortRangeCheckCircuit {
+                element,
+                num_bits,
+                _lookup_marker: PhantomData,
+            }
+        }
+    }
+
     impl<F: PrimeFieldBits, Lookup: LookupRangeCheck<F, K> + std::clone::Clone> Circuit<F>
         for MyShortRangeCheckCircuit<F, Lookup>
     {
         type Config = Lookup;
         type FloorPlanner = SimpleFloorPlanner;
 
         fn without_witnesses(&self) -> Self {
-            MyShortRangeCheckCircuit {
-                element: Value::unknown(),
-                num_bits: self.num_bits,
-                _lookup_marker: PhantomData,
-            }
+            MyShortRangeCheckCircuit::new(Value::unknown(), self.num_bits)
         }
 
         fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config {
@@ -649,11 +648,8 @@ mod tests {
         circuit_name: &str,
         expected_proof_size: usize,
     ) {
-        let circuit: MyShortRangeCheckCircuit<pallas::Base, Lookup> = MyShortRangeCheckCircuit {
-            element: Value::known(element),
-            num_bits,
-            _lookup_marker: PhantomData,
-        };
+        let circuit =
+            MyShortRangeCheckCircuit::<pallas::Base, Lookup>::new(Value::known(element), num_bits);
         let prover = MockProver::<pallas::Base>::run(11, &circuit, vec![]).unwrap();
         assert_eq!(prover.verify(), *proof_result);