mul_fixed: Remove unused selectors and duplicate gates.

Selectors previously used in the witness_scalar_* APIs, such as
q_scalar_fixed and q_scalar_fixed_short, are now removed. The
remaining selectors have been renamed for clarity.

The coordinates check for scalars decomposed using a running sum
has been moved into the mul_fixed.rs file, instead of being
duplicated in both mul_fixed::base_field_elem and mul_fixed::short.

The decompose_scalar_fixed() method is now only used in
mul_fixed::full_width, and has been moved there.

src/circuit/gadget/ecc/chip.rs

@@ -99,22 +99,18 @@ pub struct EccConfig {
     pub q_mul_overflow: Selector,
 
     /// Fixed-base full-width scalar multiplication
-    pub q_mul_fixed: Selector,
+    pub q_mul_fixed_full: Selector,
     /// Fixed-base signed short scalar multiplication
     pub q_mul_fixed_short: Selector,
-    /// Fixed-base multiplication using a base field element as the scalar
-    pub q_mul_fixed_running_sum: Selector,
     /// Canonicity checks on base field element used as scalar in fixed-base mul
-    pub base_field_fixed_canon: Selector,
+    pub q_mul_fixed_base_field: Selector,
+    /// Running sum decomposition of a scalar used in fixed-base mul. This is used
+    /// when the scalar is a signed short exponent or a base-field element.
+    pub q_mul_fixed_running_sum: Selector,
 
     /// Witness point
     pub q_point: Selector,
 
-    /// Witness full-width scalar for fixed-base scalar mul
-    pub q_scalar_fixed: Selector,
-    /// Witness signed short scalar for full-width fixed-base scalar mul
-    pub q_scalar_fixed_short: Selector,
-
     /// Shared fixed column used for loading constants. This is included in
     /// the permutation so that cells in advice columns can be constrained to
     /// equal cells in this fixed column.
@@ -195,13 +191,11 @@ impl EccChip {
             q_mul_decompose_var: meta.selector(),
             q_mul_overflow: meta.selector(),
             q_mul_lsb: meta.selector(),
-            q_mul_fixed: meta.selector(),
+            q_mul_fixed_full: meta.selector(),
             q_mul_fixed_short: meta.selector(),
+            q_mul_fixed_base_field: meta.selector(),
             q_mul_fixed_running_sum,
-            base_field_fixed_canon: meta.selector(),
             q_point: meta.selector(),
-            q_scalar_fixed: meta.selector(),
-            q_scalar_fixed_short: meta.selector(),
             constants: constants[1],
             perm,
             lookup_config,
@@ -232,6 +226,14 @@ impl EccChip {
             mul_config.create_gate(meta);
         }
 
+        // Create gate that is used both in fixed-base mul using a short signed exponent,
+        // and fixed-base mul using a base field element.
+        {
+            // The const generic does not matter when creating gates.
+            let mul_fixed_config: mul_fixed::Config<{ constants::NUM_WINDOWS }> = (&config).into();
+            mul_fixed_config.running_sum_coords_gate(meta);
+        }
+
         // Create gate that is only used in full-width fixed-base scalar mul.
         {
             let mul_fixed_full_config: mul_fixed::full_width::Config = (&config).into();

src/circuit/gadget/ecc/chip/mul_fixed.rs

@@ -5,14 +5,15 @@ use super::{
 use crate::constants::{
     self,
     load::{NullifierK, OrchardFixedBase, OrchardFixedBasesFull, ValueCommitV, WindowUs},
-    util,
 };
 
-use arrayvec::ArrayVec;
 use group::Curve;
 use halo2::{
     circuit::Region,
-    plonk::{Advice, Column, Error, Expression, Fixed, Permutation, Selector, VirtualCells},
+    plonk::{
+        Advice, Column, ConstraintSystem, Error, Expression, Fixed, Permutation, Selector,
+        VirtualCells,
+    },
     poly::Rotation,
 };
 use lazy_static::lazy_static;
@@ -79,8 +80,7 @@ impl OrchardFixedBases {
 
 #[derive(Clone, Debug)]
 pub struct Config<const NUM_WINDOWS: usize> {
-    q_mul_fixed: Selector,
+    q_mul_fixed_running_sum: Selector,
-    q_scalar_fixed: Selector,
     // The fixed Lagrange interpolation coefficients for `x_p`.
     lagrange_coeffs: [Column<Fixed>; constants::H],
     // The fixed `z` for each window such that `y + z = u^2`.
@@ -105,8 +105,7 @@ pub struct Config<const NUM_WINDOWS: usize> {
 impl<const NUM_WINDOWS: usize> From<&EccConfig> for Config<NUM_WINDOWS> {
     fn from(ecc_config: &EccConfig) -> Self {
         let config = Self {
-            q_mul_fixed: ecc_config.q_mul_fixed,
+            q_mul_fixed_running_sum: ecc_config.q_mul_fixed_running_sum,
-            q_scalar_fixed: ecc_config.q_scalar_fixed,
             lagrange_coeffs: ecc_config.lagrange_coeffs,
             fixed_z: ecc_config.fixed_z,
             x_p: ecc_config.advices[0],
@@ -153,6 +152,30 @@ impl<const NUM_WINDOWS: usize> From<&EccConfig> for Config<NUM_WINDOWS> {
 }
 
 impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
+    /// Check that each window in the running sum decomposition uses the correct y_p
+    /// and interpolated x_p.
+    ///
+    /// This gate is used both in the mul_fixed::base_field_elem and mul_fixed::short
+    /// helpers, which decompose the scalar using a running sum.
+    ///
+    /// This gate is not used in the mul_fixed::full_width helper, since the full-width
+    /// scalar is witnessed directly as three-bit windows instead of being decomposed
+    /// via a running sum.
+    pub(crate) fn running_sum_coords_gate(&self, meta: &mut ConstraintSystem<pallas::Base>) {
+        meta.create_gate("Running sum coordinates check", |meta| {
+            let q_mul_fixed_running_sum = meta.query_selector(self.q_mul_fixed_running_sum);
+
+            let z_cur = meta.query_advice(self.window, Rotation::cur());
+            let z_next = meta.query_advice(self.window, Rotation::next());
+
+            //    z_{i+1} = (z_i - a_i) / 2^3
+            // => a_i = z_i - z_{i+1} * 2^3
+            let word = z_cur - z_next * pallas::Base::from_u64(constants::H as u64);
+
+            self.coords_check(meta, q_mul_fixed_running_sum, word)
+        });
+    }
+
     #[allow(clippy::op_ref)]
     fn coords_check(
         &self,
@@ -295,55 +318,6 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         Ok(())
     }
 
-    /// Witnesses the given scalar as `NUM_WINDOWS` 3-bit windows.
-    ///
-    /// The scalar is allowed to be non-canonical.
-    fn decompose_scalar_fixed<const SCALAR_NUM_BITS: usize>(
-        &self,
-        scalar: Option<pallas::Scalar>,
-        offset: usize,
-        region: &mut Region<'_, pallas::Base>,
-    ) -> Result<ArrayVec<CellValue<pallas::Base>, NUM_WINDOWS>, Error> {
-        // Enable `q_scalar_fixed` selector
-        for idx in 0..NUM_WINDOWS {
-            self.q_scalar_fixed.enable(region, offset + idx)?;
-        }
-
-        // Decompose scalar into `k-bit` windows
-        let scalar_windows: Option<Vec<u8>> = scalar.map(|scalar| {
-            util::decompose_word::<pallas::Scalar>(
-                scalar,
-                SCALAR_NUM_BITS,
-                constants::FIXED_BASE_WINDOW_SIZE,
-            )
-        });
-
-        // Store the scalar decomposition
-        let mut windows: ArrayVec<CellValue<pallas::Base>, NUM_WINDOWS> = ArrayVec::new();
-
-        let scalar_windows: Vec<Option<pallas::Base>> = if let Some(windows) = scalar_windows {
-            assert_eq!(windows.len(), NUM_WINDOWS);
-            windows
-                .into_iter()
-                .map(|window| Some(pallas::Base::from_u64(window as u64)))
-                .collect()
-        } else {
-            vec![None; NUM_WINDOWS]
-        };
-
-        for (idx, window) in scalar_windows.into_iter().enumerate() {
-            let window_cell = region.assign_advice(
-                || format!("k[{:?}]", offset + idx),
-                self.window,
-                offset + idx,
-                || window.ok_or(Error::SynthesisError),
-            )?;
-            windows.push(CellValue::new(window_cell, window));
-        }
-
-        Ok(windows)
-    }
-
     fn process_window(
         &self,
         region: &mut Region<'_, pallas::Base>,

src/circuit/gadget/ecc/chip/mul_fixed/base_field_elem.rs

@@ -20,7 +20,7 @@ use std::convert::TryInto;
 
 pub struct Config {
     q_mul_fixed_running_sum: Selector,
-    base_field_fixed_canon: Selector,
+    q_mul_fixed_base_field: Selector,
     canon_advices: [Column<Advice>; 3],
     lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
     running_sum_config: RunningSumConfig<pallas::Base, { constants::FIXED_BASE_WINDOW_SIZE }>,
@@ -31,7 +31,7 @@ impl From<&EccConfig> for Config {
     fn from(config: &EccConfig) -> Self {
         let config = Self {
             q_mul_fixed_running_sum: config.q_mul_fixed_running_sum,
-            base_field_fixed_canon: config.base_field_fixed_canon,
+            q_mul_fixed_base_field: config.q_mul_fixed_base_field,
             canon_advices: [config.advices[6], config.advices[7], config.advices[8]],
             lookup_config: config.lookup_config.clone(),
             running_sum_config: config.running_sum_config.clone(),
@@ -71,7 +71,7 @@ impl Config {
 
         // Check that the base field element is canonical.
         meta.create_gate("Canonicity checks", |meta| {
-            let base_field_fixed_canon = meta.query_selector(self.base_field_fixed_canon);
+            let q_mul_fixed_base_field = meta.query_selector(self.q_mul_fixed_base_field);
 
             let alpha = meta.query_advice(self.canon_advices[0], Rotation::prev());
             // The last three bits of α.
@@ -164,7 +164,7 @@ impl Config {
             canon_checks
                 .chain(decomposition_checks)
                 .chain(Some(("alpha_0_prime check", alpha_0_prime_check)))
-                .map(move |(name, poly)| (name, base_field_fixed_canon.clone() * poly))
+                .map(move |(name, poly)| (name, q_mul_fixed_base_field.clone() * poly))
         });
     }
 
@@ -291,7 +291,7 @@ impl Config {
                 let perm = &self.super_config.perm;
 
                 // Activate canonicity check gate
-                self.base_field_fixed_canon.enable(&mut region, 1)?;
+                self.q_mul_fixed_base_field.enable(&mut region, 1)?;
 
                 // Offset 0
                 {

src/circuit/gadget/ecc/chip/mul_fixed/full_width.rs

@@ -1,25 +1,26 @@
 use super::super::{EccConfig, EccPoint, EccScalarFixed, OrchardFixedBasesFull};
 
 use crate::{
-    circuit::gadget::utilities::range_check,
+    circuit::gadget::utilities::{range_check, CellValue, Var},
-    constants::{self, L_ORCHARD_SCALAR, NUM_WINDOWS},
+    constants::{self, util, L_ORCHARD_SCALAR, NUM_WINDOWS},
 };
+use arrayvec::ArrayVec;
 use halo2::{
     circuit::{Layouter, Region},
     plonk::{ConstraintSystem, Error, Selector},
     poly::Rotation,
 };
-use pasta_curves::pallas;
+use pasta_curves::{arithmetic::FieldExt, pallas};
 
 pub struct Config {
-    q_scalar_fixed: Selector,
+    q_mul_fixed_full: Selector,
     super_config: super::Config<NUM_WINDOWS>,
 }
 
 impl From<&EccConfig> for Config {
     fn from(config: &EccConfig) -> Self {
         Self {
-            q_scalar_fixed: config.q_scalar_fixed,
+            q_mul_fixed_full: config.q_mul_fixed_full,
             super_config: config.into(),
         }
     }
@@ -29,17 +30,17 @@ impl Config {
     pub fn create_gate(&self, meta: &mut ConstraintSystem<pallas::Base>) {
         // Check that each window `k` is within 3 bits
         meta.create_gate("Full-width fixed-base scalar mul", |meta| {
-            let q_scalar_fixed = meta.query_selector(self.q_scalar_fixed);
+            let q_mul_fixed_full = meta.query_selector(self.q_mul_fixed_full);
             let window = meta.query_advice(self.super_config.window, Rotation::cur());
 
             self.super_config
-                .coords_check(meta, q_scalar_fixed.clone(), window.clone())
+                .coords_check(meta, q_mul_fixed_full.clone(), window.clone())
                 .into_iter()
                 // Constrain each window to a 3-bit value:
                 // 1 * (window - 0) * (window - 1) * ... * (window - 7)
                 .chain(Some((
                     "window range check",
-                    q_scalar_fixed * range_check(window, constants::H),
+                    q_mul_fixed_full * range_check(window, constants::H),
                 )))
         });
     }
@@ -53,9 +54,7 @@ impl Config {
         offset: usize,
         scalar: Option<pallas::Scalar>,
     ) -> Result<EccScalarFixed, Error> {
-        let windows = self
+        let windows = self.decompose_scalar_fixed::<L_ORCHARD_SCALAR>(scalar, offset, region)?;
-            .super_config
-            .decompose_scalar_fixed::<L_ORCHARD_SCALAR>(scalar, offset, region)?;
 
         Ok(EccScalarFixed {
             value: scalar,
@@ -63,6 +62,55 @@ impl Config {
         })
     }
 
+    /// Witnesses the given scalar as `NUM_WINDOWS` 3-bit windows.
+    ///
+    /// The scalar is allowed to be non-canonical.
+    fn decompose_scalar_fixed<const SCALAR_NUM_BITS: usize>(
+        &self,
+        scalar: Option<pallas::Scalar>,
+        offset: usize,
+        region: &mut Region<'_, pallas::Base>,
+    ) -> Result<ArrayVec<CellValue<pallas::Base>, NUM_WINDOWS>, Error> {
+        // Enable `q_mul_fixed_full` selector
+        for idx in 0..NUM_WINDOWS {
+            self.q_mul_fixed_full.enable(region, offset + idx)?;
+        }
+
+        // Decompose scalar into `k-bit` windows
+        let scalar_windows: Option<Vec<u8>> = scalar.map(|scalar| {
+            util::decompose_word::<pallas::Scalar>(
+                scalar,
+                SCALAR_NUM_BITS,
+                constants::FIXED_BASE_WINDOW_SIZE,
+            )
+        });
+
+        // Store the scalar decomposition
+        let mut windows: ArrayVec<CellValue<pallas::Base>, NUM_WINDOWS> = ArrayVec::new();
+
+        let scalar_windows: Vec<Option<pallas::Base>> = if let Some(windows) = scalar_windows {
+            assert_eq!(windows.len(), NUM_WINDOWS);
+            windows
+                .into_iter()
+                .map(|window| Some(pallas::Base::from_u64(window as u64)))
+                .collect()
+        } else {
+            vec![None; NUM_WINDOWS]
+        };
+
+        for (idx, window) in scalar_windows.into_iter().enumerate() {
+            let window_cell = region.assign_advice(
+                || format!("k[{:?}]", offset + idx),
+                self.super_config.window,
+                offset + idx,
+                || window.ok_or(Error::SynthesisError),
+            )?;
+            windows.push(CellValue::new(window_cell, window));
+        }
+
+        Ok(windows)
+    }
+
     pub fn assign(
         &self,
         mut layouter: impl Layouter<pallas::Base>,
@@ -81,7 +129,7 @@ impl Config {
                     offset,
                     &(&scalar).into(),
                     base.into(),
-                    self.super_config.q_mul_fixed,
+                    self.q_mul_fixed_full,
                 )?;
 
                 Ok((scalar, acc, mul_b))

src/circuit/gadget/ecc/chip/mul_fixed/short.rs

@@ -3,7 +3,7 @@ use std::{array, convert::TryInto};
 use super::super::{EccConfig, EccPoint, EccScalarFixedShort};
 use crate::{
     circuit::gadget::utilities::{copy, decompose_running_sum::RunningSumConfig, CellValue, Var},
-    constants::{self, ValueCommitV, FIXED_BASE_WINDOW_SIZE, L_VALUE, NUM_WINDOWS_SHORT},
+    constants::{ValueCommitV, FIXED_BASE_WINDOW_SIZE, L_VALUE, NUM_WINDOWS_SHORT},
 };
 
 use halo2::{
@@ -11,7 +11,7 @@ use halo2::{
     plonk::{ConstraintSystem, Error, Expression, Selector},
     poly::Rotation,
 };
-use pasta_curves::{arithmetic::FieldExt, pallas};
+use pasta_curves::pallas;
 
 #[derive(Clone)]
 pub struct Config {
@@ -35,21 +35,6 @@ impl From<&EccConfig> for Config {
 
 impl Config {
     pub(crate) fn create_gate(&self, meta: &mut ConstraintSystem<pallas::Base>) {
-        // Check that each window uses the correct y_p and interpolated x_p.
-        meta.create_gate("Coordinates check", |meta| {
-            let q_mul_fixed_running_sum = meta.query_selector(self.q_mul_fixed_running_sum);
-
-            let z_cur = meta.query_advice(self.super_config.window, Rotation::cur());
-            let z_next = meta.query_advice(self.super_config.window, Rotation::next());
-
-            //    z_{i+1} = (z_i - a_i) / 2^3
-            // => a_i = z_i - z_{i+1} * 2^3
-            let word = z_cur - z_next * pallas::Base::from_u64(constants::H as u64);
-
-            self.super_config
-                .coords_check(meta, q_mul_fixed_running_sum, word)
-        });
-
         meta.create_gate("Short fixed-base mul gate", |meta| {
             let q_mul_fixed_short = meta.query_selector(self.q_mul_fixed_short);
             let y_p = meta.query_advice(self.super_config.y_p, Rotation::cur());
@@ -130,7 +115,7 @@ impl Config {
                     offset,
                     &(&scalar).into(),
                     base.clone().into(),
-                    self.super_config.q_mul_fixed,
+                    self.q_mul_fixed_running_sum,
                 )?;
 
                 Ok((scalar, acc, mul_b))
@@ -213,6 +198,7 @@ impl Config {
         // tested at the circuit-level.
         {
             use group::Curve;
+            use pasta_curves::arithmetic::FieldExt;
 
             if let (Some(magnitude), Some(sign)) = (scalar.magnitude.value(), scalar.sign.value()) {
                 let magnitude_is_valid =