From b025ffbc32d862ec503c5b577afd9d396e8d4f78 Mon Sep 17 00:00:00 2001
From: therealyingtong <yingtong@z.cash>
Date: Thu, 3 Jun 2021 23:20:09 +0800
Subject: [PATCH] Use l_star_plusone in gate

---
 src/circuit/gadget/orchard_action/merkle.rs | 59 ++++++++++++---------
 1 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/src/circuit/gadget/orchard_action/merkle.rs b/src/circuit/gadget/orchard_action/merkle.rs
index a459eac03..cb0b8e64a 100644
--- a/src/circuit/gadget/orchard_action/merkle.rs
+++ b/src/circuit/gadget/orchard_action/merkle.rs
@@ -1,6 +1,6 @@
 use halo2::{
     circuit::{Chip, Layouter},
-    plonk::{Advice, Column, ConstraintSystem, Error, Fixed, Permutation, Selector},
+    plonk::{Advice, Column, ConstraintSystem, Error, Expression, Fixed, Permutation, Selector},
     poly::Rotation,
 };
 use pasta_curves::arithmetic::{CurveAffine, FieldExt};
@@ -128,10 +128,10 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleChip<C, PATH_LENGTH> {
         perm: Permutation,
     ) -> MerkleConfig<PATH_LENGTH> {
         let cond_swap_config =
-            CondSwapChip::configure(meta, advices.clone()[..5].try_into().unwrap(), perm.clone());
+            CondSwapChip::configure(meta, advices[..5].try_into().unwrap(), perm.clone());
         let sinsemilla_config = SinsemillaChip::<C>::configure(
             meta,
-            advices.clone()[..5].try_into().unwrap(),
+            advices[..5].try_into().unwrap(),
             lookup,
             perm.clone(),
         );
@@ -146,6 +146,11 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleChip<C, PATH_LENGTH> {
         let l_star = meta.fixed_column();
 
         // Check that pieces have been decomposed correctly for Sinsemilla hash.
+        // <https://zips.z.cash/protocol/nu5.pdf#orchardmerklecrh>
+        //
+        // `a = a_0||a_1` = `l_star` || (bits 0..=239 of `left`)
+        // `b = b_0||b_1` = (bits 240..=254 of `left`) || (bits 0..=234 of `right`)
+        // `c = bits 235..=254 of `right`
         meta.create_gate("Merkle path validity check", |meta| {
             let a_whole = meta.query_advice(a, Rotation::cur());
             let b_whole = meta.query_advice(b, Rotation::cur());
@@ -158,30 +163,30 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleChip<C, PATH_LENGTH> {
             let b_0 = meta.query_advice(c, Rotation::next());
             let b_1 = meta.query_advice(left, Rotation::next());
 
-            let l_star = meta.query_fixed(l_star, Rotation::cur());
+            let l_star_plus1 = meta.query_fixed(l_star, Rotation::cur());
 
-            // a = a_0||a_1` = `l` (10 bits) || (bits 0..239 of `left`)
+            // a = a_0||a_1` = `l_star` (10 bits) || (bits 0..=239 of `left`)
             // Check that a = a_0 || a_1
             let a_check = a_0.clone() + a_1.clone() * C::Base::from_u64(1 << 10) - a_whole;
 
             // Check that a_0 = l_star
-            let l_star_check = a_0 - l_star.clone();
+            let l_star_check = a_0 - (l_star_plus1.clone() - Expression::Constant(C::Base::one()));
 
-            // `b = b_0||b_1` = (bits 240..254 of `left`) || (bits 0..234 of `right`)
+            // `b = b_0||b_1` = (bits 240..=254 of `left`) || (bits 0..=234 of `right`)
             // Check that b = b_0 (15 bits) || b_1 (235 bits)
             let b_check = b_0.clone() + b_1.clone() * C::Base::from_u64(1 << 15) - b_whole;
 
             // Check that left = a_1 (240 bits) || b_0 (15 bits)
             let two_pow_240 = C::Base::from_u128(1 << 120).square();
-            let left_check = a_1 + b_0.clone() * two_pow_240 - left_node;
+            let left_check = a_1 + b_0 * two_pow_240 - left_node;
 
-            // Check that right = b_1 || c
+            // Check that right = b_1 (235 bits) || c (20 bits)
             let two_pow_235 = C::Base::from_u64(1 << 47).pow(&[5, 0, 0, 0]);
             let right_check = b_1 + c_whole * two_pow_235 - right_node;
 
             [a_check, l_star_check, b_check, left_check, right_check]
                 .iter()
-                .map(|poly| l_star.clone() * poly.clone())
+                .map(|poly| l_star_plus1.clone() * poly.clone())
                 .collect()
         });
 
@@ -326,17 +331,20 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleInstructions<C, PATH_LENGTH
         left: CellValue<C::Base>,
         right: CellValue<C::Base>,
     ) -> Result<CellValue<C::Base>, Error> {
+        // <https://zips.z.cash/protocol/nu5.pdf#orchardmerklecrh>
         // We need to hash `l_star || left || right`, where `l_star` is a 10-bit value.
-        // `a = a_0||a_1` = `l_star` || (bits 0..239 of `left`)
-        // `b = b_0||b_1` = (bits 240..254 of `left`) || (bits 0..234 of `right`)
-        // `c = bits 235..254 of `right`
+        // We allow `left` and `right` to be non-canonical 255-bit encodings.
+        //
+        // `a = a_0||a_1` = `l_star` || (bits 0..=239 of `left`)
+        // `b = b_0||b_1` = (bits 240..=254 of `left`) || (bits 0..=234 of `right`)
+        // `c = bits 235..=254 of `right`
 
-        // `a = a_0||a_1` = `l` || (bits 0..239 of `left`)
+        // `a = a_0||a_1` = `l` || (bits 0..=239 of `left`)
         let (a_0, a_1, a) = {
             // a_0 = l_star
             let a_0 = (C::Base::from_u64(l_star as u64), 0..10);
 
-            // a_1 = (bits 0..239 of `left`)
+            // a_1 = (bits 0..=239 of `left`)
             let a_1 = left.value().map(|left| (left, 0..240));
 
             let a: Option<Vec<bool>> = a_1
@@ -360,14 +368,14 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleInstructions<C, PATH_LENGTH
             (a_0.0, a_1, a)
         };
 
-        // `b = b_0||b_1` = (bits 240..254 of `left`) || (bits 0..234 of `right`)
+        // `b = b_0||b_1` = (bits 240..=254 of `left`) || (bits 0..=234 of `right`)
         let (b_0, b_1, b) = {
-            // b_0 = (bits 240..254 of `left`)
+            // b_0 = (bits 240..=254 of `left`)
             let b_0 = left
                 .value()
                 .map(|left| (left, 240..(C::Base::NUM_BITS as usize)));
 
-            // b_1 = (bits 0..234 of `right`)
+            // b_1 = (bits 0..=234 of `right`)
             let b_1 = right.value().map(|right| (right, 0..235));
 
             let b: Option<Vec<bool>> = b_0
@@ -385,7 +393,7 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleInstructions<C, PATH_LENGTH
                     .iter()
                     .by_val()
                     .take(C::Base::NUM_BITS as usize)
-                    .collect::<Vec<_>>()[b_0.1.clone()];
+                    .collect::<Vec<_>>()[b_0.1];
                 to_field_elem(b_0)
             });
 
@@ -396,7 +404,7 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleInstructions<C, PATH_LENGTH
                     .iter()
                     .by_val()
                     .take(C::Base::NUM_BITS as usize)
-                    .collect::<Vec<_>>()[b_1.1.clone()];
+                    .collect::<Vec<_>>()[b_1.1];
                 to_field_elem(b_1)
             });
 
@@ -404,7 +412,7 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleInstructions<C, PATH_LENGTH
         };
 
         let c = {
-            // `c = bits 235..254 of `right`
+            // `c = bits 235..=254 of `right`
             let c = right
                 .value()
                 .map(|right| (right, 235..(C::Base::NUM_BITS as usize)));
@@ -420,12 +428,13 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleInstructions<C, PATH_LENGTH
             layouter.assign_region(
                 || "Check piece decomposition",
                 |mut region| {
-                    // Set the fixed column `l_star` to the current l_star.
+                    // Set the fixed column `l_star_plus1` to the current l_star + 1.
+                    let l_star_plus1 = (l_star as u64) + 1;
                     region.assign_fixed(
-                        || format!("l_star {}", l_star),
+                        || format!("l_star_plus1 {}", l_star_plus1),
                         config.l_star,
                         0,
-                        || Ok(C::Base::from_u64(l_star as u64)),
+                        || Ok(C::Base::from_u64(l_star_plus1)),
                     )?;
 
                     // Copy in `a, b, c, left, right` to the correct offsets
@@ -494,7 +503,7 @@ impl<C: CurveAffine, const PATH_LENGTH: usize> MerkleInstructions<C, PATH_LENGTH
             Q,
             vec![a, b, c],
         )?;
-        let result = Self::extract(&point).clone();
+        let result = *Self::extract(&point);
 
         // Check layer hash output against Sinsemilla primitives hash
         #[cfg(test)]