From dc2ec0308d972bcb1848ac1e7bff6aab99023566 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@electriccoin.co>
Date: Sat, 7 May 2022 23:17:59 +0000
Subject: [PATCH] halo2_gadgets: Deduplicate some Sinsemilla chip expressions

---
 halo2_gadgets/src/sinsemilla/chip.rs          | 53 ++++++++++++-------
 .../src/sinsemilla/chip/generator_table.rs    | 15 ++----
 2 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/halo2_gadgets/src/sinsemilla/chip.rs b/halo2_gadgets/src/sinsemilla/chip.rs
index 45eca619..8aae03e7 100644
--- a/halo2_gadgets/src/sinsemilla/chip.rs
+++ b/halo2_gadgets/src/sinsemilla/chip.rs
@@ -88,6 +88,38 @@ where
     pub fn lookup_config(&self) -> LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }> {
         self.lookup_config
     }
+
+    /// Derives the expression `x_r = lambda_1^2 - x_a - x_p`.
+    fn x_r(
+        &self,
+        meta: &mut VirtualCells<pallas::Base>,
+        rotation: Rotation,
+    ) -> Expression<pallas::Base> {
+        let x_a = meta.query_advice(self.x_a, rotation);
+        let x_p = meta.query_advice(self.x_p, rotation);
+        let lambda_1 = meta.query_advice(self.lambda_1, rotation);
+        lambda_1.square() - x_a - x_p
+    }
+
+    /// Derives the expression `Y_A = (lambda_1 + lambda_2) * (x_a - x_r)`.
+    #[allow(non_snake_case)]
+    fn Y_A(
+        &self,
+        meta: &mut VirtualCells<pallas::Base>,
+        rotation: Rotation,
+    ) -> Expression<pallas::Base> {
+        let x_a = meta.query_advice(self.x_a, rotation);
+        let lambda_1 = meta.query_advice(self.lambda_1, rotation);
+        let lambda_2 = meta.query_advice(self.lambda_2, rotation);
+        (lambda_1 + lambda_2) * (x_a - self.x_r(meta, rotation))
+    }
+
+    /// Derives the expression `q_s3 = (q_s2) * (q_s2 - 1)`.
+    fn q_s3(&self, meta: &mut VirtualCells<pallas::Base>) -> Expression<pallas::Base> {
+        let one = Expression::Constant(pallas::Base::one());
+        let q_s2 = meta.query_fixed(self.q_sinsemilla2, Rotation::cur());
+        q_s2.clone() * (q_s2 - one)
+    }
 }
 
 /// A chip that implements 10-bit Sinsemilla using a lookup table and 5 advice columns.
@@ -184,20 +216,10 @@ where
 
         // Closures for expressions that are derived multiple times
         // x_r = lambda_1^2 - x_a - x_p
-        let x_r = |meta: &mut VirtualCells<pallas::Base>, rotation| {
-            let x_a = meta.query_advice(config.x_a, rotation);
-            let x_p = meta.query_advice(config.x_p, rotation);
-            let lambda_1 = meta.query_advice(config.lambda_1, rotation);
-            lambda_1.square() - x_a - x_p
-        };
+        let x_r = |meta: &mut VirtualCells<pallas::Base>, rotation| config.x_r(meta, rotation);
 
         // Y_A = (lambda_1 + lambda_2) * (x_a - x_r)
-        let Y_A = |meta: &mut VirtualCells<pallas::Base>, rotation| {
-            let x_a = meta.query_advice(config.x_a, rotation);
-            let lambda_1 = meta.query_advice(config.lambda_1, rotation);
-            let lambda_2 = meta.query_advice(config.lambda_2, rotation);
-            (lambda_1 + lambda_2) * (x_a - x_r(meta, rotation))
-        };
+        let Y_A = |meta: &mut VirtualCells<pallas::Base>, rotation| config.Y_A(meta, rotation);
 
         // Check that the initial x_A, x_P, lambda_1, lambda_2 are consistent with y_Q.
         meta.create_gate("Initial y_Q", |meta| {
@@ -215,12 +237,7 @@ where
 
         meta.create_gate("Sinsemilla gate", |meta| {
             let q_s1 = meta.query_selector(config.q_sinsemilla1);
-            // q_s3 = (q_s2) * (q_s2 - 1)
-            let q_s3 = {
-                let one = Expression::Constant(pallas::Base::one());
-                let q_s2 = meta.query_fixed(config.q_sinsemilla2, Rotation::cur());
-                q_s2.clone() * (q_s2 - one)
-            };
+            let q_s3 = config.q_s3(meta);
 
             let lambda_1_next = meta.query_advice(config.lambda_1, Rotation::next());
             let lambda_2_cur = meta.query_advice(config.lambda_2, Rotation::cur());
diff --git a/halo2_gadgets/src/sinsemilla/chip/generator_table.rs b/halo2_gadgets/src/sinsemilla/chip/generator_table.rs
index 1f8c1d58..127f35f9 100644
--- a/halo2_gadgets/src/sinsemilla/chip/generator_table.rs
+++ b/halo2_gadgets/src/sinsemilla/chip/generator_table.rs
@@ -39,10 +39,7 @@ impl GeneratorTableConfig {
         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_s3 = {
-                let one = Expression::Constant(pallas::Base::one());
-                q_s2.clone() * (q_s2.clone() - one)
-            };
+            let q_s3 = config.q_s3(meta);
 
             // m_{i+1} = z_{i} - 2^K * (q_s2 - q_s3) * z_{i + 1}
             // Note that the message words m_i's are 1-indexed while the
@@ -55,17 +52,11 @@ impl GeneratorTableConfig {
 
             let x_p = meta.query_advice(config.x_p, Rotation::cur());
 
-            // y_{p,i} = (Y_{A,i} / 2) - lambda1 * (x_{A,i} - x_{P,i}),
-            // where Y_{A,i} = (lambda1_i + lambda2_i) * (x_{A,i} - x_{R,i}),
-            //       x_{R,i} = lambda1^2 - x_{A,i} - x_{P,i}
-            //
+            // y_{p,i} = (Y_{A,i} / 2) - lambda1 * (x_{A,i} - x_{P,i})
             let y_p = {
                 let lambda1 = meta.query_advice(config.lambda_1, Rotation::cur());
-                let lambda2 = meta.query_advice(config.lambda_2, Rotation::cur());
                 let x_a = meta.query_advice(config.x_a, Rotation::cur());
-
-                let x_r = lambda1.clone().square() - x_a.clone() - x_p.clone();
-                let Y_A = (lambda1.clone() + lambda2) * (x_a.clone() - x_r);
+                let Y_A = config.Y_A(meta, Rotation::cur());
 
                 (Y_A * pallas::Base::TWO_INV) - (lambda1 * (x_a - x_p.clone()))
             };