Merge pull request #209 from zcash/circuit-bugfixes

Circuit bugfixes

src/circuit.rs

@@ -38,7 +38,7 @@ use crate::{
 use gadget::{
     ecc::{
         chip::{EccChip, EccConfig},
-        FixedPoint, FixedPointBaseField, FixedPointShort, Point,
+        FixedPoint, FixedPointBaseField, FixedPointShort, NonIdentityPoint, Point,
     },
     poseidon::{
         Hash as PoseidonHash, Pow5T3Chip as PoseidonChip, Pow5T3Config as PoseidonConfig,
@@ -356,7 +356,7 @@ impl plonk::Circuit<pallas::Base> for Circuit {
             )?;
 
             // Witness g_d_old
-            let g_d_old = Point::new(
+            let g_d_old = NonIdentityPoint::new(
                 ecc_chip.clone(),
                 layouter.namespace(|| "gd_old"),
                 self.g_d_old.as_ref().map(|gd| gd.to_affine()),
@@ -364,7 +364,7 @@ impl plonk::Circuit<pallas::Base> for Circuit {
 
             // Witness ak.
             let ak: Option<pallas::Point> = self.ak.as_ref().map(|ak| ak.into());
-            let ak = Point::new(
+            let ak = NonIdentityPoint::new(
                 ecc_chip.clone(),
                 layouter.namespace(|| "ak"),
                 ak.map(|ak| ak.to_affine()),
@@ -621,7 +621,7 @@ impl plonk::Circuit<pallas::Base> for Circuit {
                 g_d_old.mul(layouter.namespace(|| "[ivk] g_d_old"), ivk.inner())?;
 
             // Constrain derived pk_d_old to equal witnessed pk_d_old
-            let pk_d_old = Point::new(
+            let pk_d_old = NonIdentityPoint::new(
                 ecc_chip.clone(),
                 layouter.namespace(|| "witness pk_d_old"),
                 self.pk_d_old.map(|pk_d_old| pk_d_old.inner().to_affine()),
@@ -666,7 +666,7 @@ impl plonk::Circuit<pallas::Base> for Circuit {
                 let g_d_new = self
                     .g_d_new_star
                     .map(|bytes| pallas::Affine::from_bytes(&bytes).unwrap());
-                Point::new(
+                NonIdentityPoint::new(
                     ecc_chip.clone(),
                     layouter.namespace(|| "witness g_d_new_star"),
                     g_d_new,
@@ -678,7 +678,7 @@ impl plonk::Circuit<pallas::Base> for Circuit {
                 let pk_d_new = self
                     .pk_d_new_star
                     .map(|bytes| pallas::Affine::from_bytes(&bytes).unwrap());
-                Point::new(
+                NonIdentityPoint::new(
                     ecc_chip,
                     layouter.namespace(|| "witness pk_d_new"),
                     pk_d_new,

src/circuit/gadget/ecc.rs

@@ -35,7 +35,9 @@ pub trait EccInstructions<C: CurveAffine>: Chip<C::Base> + UtilitiesInstructions
     /// A `ScalarFixedShort` must be in the range [-(2^64 - 1), 2^64 - 1].
     type ScalarFixedShort: Clone + Debug;
     /// Variable representing an elliptic curve point.
-    type Point: Clone + Debug;
+    type Point: From<Self::NonIdentityPoint> + Clone + Debug;
+    /// Variable representing a non-identity elliptic curve point.
+    type NonIdentityPoint: Clone + Debug;
     /// Variable representing the affine short Weierstrass x-coordinate of an
     /// elliptic curve point.
     type X: Clone + Debug;
@@ -55,15 +57,24 @@ pub trait EccInstructions<C: CurveAffine>: Chip<C::Base> + UtilitiesInstructions
     ) -> Result<(), Error>;
 
     /// Witnesses the given point as a private input to the circuit.
-    /// This maps the identity to (0, 0) in affine coordinates.
+    /// This allows the point to be the identity, mapped to (0, 0) in
+    /// affine coordinates.
     fn witness_point(
         &self,
         layouter: &mut impl Layouter<C::Base>,
         value: Option<C>,
     ) -> Result<Self::Point, Error>;
 
+    /// Witnesses the given point as a private input to the circuit.
+    /// This returns an error if the point is the identity.
+    fn witness_point_non_id(
+        &self,
+        layouter: &mut impl Layouter<C::Base>,
+        value: Option<C>,
+    ) -> Result<Self::NonIdentityPoint, Error>;
+
     /// Extracts the x-coordinate of a point.
-    fn extract_p(point: &Self::Point) -> &Self::X;
+    fn extract_p<Point: Into<Self::Point> + Clone>(point: &Point) -> Self::X;
 
     /// Performs incomplete point addition, returning `a + b`.
     ///
@@ -71,25 +82,24 @@ pub trait EccInstructions<C: CurveAffine>: Chip<C::Base> + UtilitiesInstructions
     fn add_incomplete(
         &self,
         layouter: &mut impl Layouter<C::Base>,
-        a: &Self::Point,
+        a: &Self::NonIdentityPoint,
-        b: &Self::Point,
+        b: &Self::NonIdentityPoint,
-    ) -> Result<Self::Point, Error>;
+    ) -> Result<Self::NonIdentityPoint, Error>;
 
     /// Performs complete point addition, returning `a + b`.
-    fn add(
+    fn add<A: Into<Self::Point> + Clone, B: Into<Self::Point> + Clone>(
         &self,
         layouter: &mut impl Layouter<C::Base>,
-        a: &Self::Point,
+        a: &A,
-        b: &Self::Point,
+        b: &B,
     ) -> Result<Self::Point, Error>;
 
     /// Performs variable-base scalar multiplication, returning `[scalar] base`.
-    /// Multiplication of the identity `[scalar] 𝒪 ` returns an error.
     fn mul(
         &self,
         layouter: &mut impl Layouter<C::Base>,
         scalar: &Self::Var,
-        base: &Self::Point,
+        base: &Self::NonIdentityPoint,
     ) -> Result<(Self::Point, Self::ScalarVar), Error>;
 
     /// Performs fixed-base scalar multiplication using a full-width scalar, returning `[scalar] base`.
@@ -157,6 +167,125 @@ where
     inner: EccChip::ScalarFixedShort,
 }
 
+/// A non-identity elliptic curve point over the given curve.
+#[derive(Copy, Clone, Debug)]
+pub struct NonIdentityPoint<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> {
+    chip: EccChip,
+    inner: EccChip::NonIdentityPoint,
+}
+
+impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq>
+    NonIdentityPoint<C, EccChip>
+{
+    /// Constructs a new point with the given value.
+    pub fn new(
+        chip: EccChip,
+        mut layouter: impl Layouter<C::Base>,
+        value: Option<C>,
+    ) -> Result<Self, Error> {
+        let point = chip.witness_point_non_id(&mut layouter, value);
+        point.map(|inner| NonIdentityPoint { chip, inner })
+    }
+
+    /// Constrains this point to be equal in value to another point.
+    pub fn constrain_equal<Other: Into<Point<C, EccChip>> + Clone>(
+        &self,
+        mut layouter: impl Layouter<C::Base>,
+        other: &Other,
+    ) -> Result<(), Error> {
+        let other: Point<C, EccChip> = (other.clone()).into();
+        self.chip.constrain_equal(
+            &mut layouter,
+            &Point::<C, EccChip>::from(self.clone()).inner,
+            &other.inner,
+        )
+    }
+
+    /// Returns the inner point.
+    pub fn inner(&self) -> &EccChip::NonIdentityPoint {
+        &self.inner
+    }
+
+    /// Extracts the x-coordinate of a point.
+    pub fn extract_p(&self) -> X<C, EccChip> {
+        X::from_inner(self.chip.clone(), EccChip::extract_p(&self.inner))
+    }
+
+    /// Wraps the given point (obtained directly from an instruction) in a gadget.
+    pub fn from_inner(chip: EccChip, inner: EccChip::NonIdentityPoint) -> Self {
+        NonIdentityPoint { chip, inner }
+    }
+
+    /// Returns `self + other` using complete addition.
+    pub fn add<Other: Into<Point<C, EccChip>> + Clone>(
+        &self,
+        mut layouter: impl Layouter<C::Base>,
+        other: &Other,
+    ) -> Result<Point<C, EccChip>, Error> {
+        let other: Point<C, EccChip> = (other.clone()).into();
+
+        assert_eq!(self.chip, other.chip);
+        self.chip
+            .add(&mut layouter, &self.inner, &other.inner)
+            .map(|inner| Point {
+                chip: self.chip.clone(),
+                inner,
+            })
+    }
+
+    /// Returns `self + other` using incomplete addition.
+    /// The arguments are type-constrained not to be the identity point,
+    /// and since exceptional cases return an Error, the result also cannot
+    /// be the identity point.
+    pub fn add_incomplete(
+        &self,
+        mut layouter: impl Layouter<C::Base>,
+        other: &Self,
+    ) -> Result<Self, Error> {
+        assert_eq!(self.chip, other.chip);
+        self.chip
+            .add_incomplete(&mut layouter, &self.inner, &other.inner)
+            .map(|inner| NonIdentityPoint {
+                chip: self.chip.clone(),
+                inner,
+            })
+    }
+
+    /// Returns `[by] self`.
+    #[allow(clippy::type_complexity)]
+    pub fn mul(
+        &self,
+        mut layouter: impl Layouter<C::Base>,
+        by: &EccChip::Var,
+    ) -> Result<(Point<C, EccChip>, ScalarVar<C, EccChip>), Error> {
+        self.chip
+            .mul(&mut layouter, by, &self.inner.clone())
+            .map(|(point, scalar)| {
+                (
+                    Point {
+                        chip: self.chip.clone(),
+                        inner: point,
+                    },
+                    ScalarVar {
+                        chip: self.chip.clone(),
+                        inner: scalar,
+                    },
+                )
+            })
+    }
+}
+
+impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq>
+    From<NonIdentityPoint<C, EccChip>> for Point<C, EccChip>
+{
+    fn from(non_id_point: NonIdentityPoint<C, EccChip>) -> Self {
+        Self {
+            chip: non_id_point.chip,
+            inner: non_id_point.inner.into(),
+        }
+    }
+}
+
 /// An elliptic curve point over the given curve.
 #[derive(Copy, Clone, Debug)]
 pub struct Point<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> {
@@ -176,11 +305,12 @@ impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> Point<C,
     }
 
     /// Constrains this point to be equal in value to another point.
-    pub fn constrain_equal(
+    pub fn constrain_equal<Other: Into<Point<C, EccChip>> + Clone>(
         &self,
         mut layouter: impl Layouter<C::Base>,
-        other: &Self,
+        other: &Other,
     ) -> Result<(), Error> {
+        let other: Point<C, EccChip> = (other.clone()).into();
         self.chip
             .constrain_equal(&mut layouter, &self.inner, &other.inner)
     }
@@ -192,7 +322,7 @@ impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> Point<C,
 
     /// Extracts the x-coordinate of a point.
     pub fn extract_p(&self) -> X<C, EccChip> {
-        X::from_inner(self.chip.clone(), EccChip::extract_p(&self.inner).clone())
+        X::from_inner(self.chip.clone(), EccChip::extract_p(&self.inner))
     }
 
     /// Wraps the given point (obtained directly from an instruction) in a gadget.
@@ -201,7 +331,13 @@ impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> Point<C,
     }
 
     /// Returns `self + other` using complete addition.
-    pub fn add(&self, mut layouter: impl Layouter<C::Base>, other: &Self) -> Result<Self, Error> {
+    pub fn add<Other: Into<Point<C, EccChip>> + Clone>(
+        &self,
+        mut layouter: impl Layouter<C::Base>,
+        other: &Other,
+    ) -> Result<Point<C, EccChip>, Error> {
+        let other: Point<C, EccChip> = (other.clone()).into();
+
         assert_eq!(self.chip, other.chip);
         self.chip
             .add(&mut layouter, &self.inner, &other.inner)
@@ -210,43 +346,6 @@ impl<C: CurveAffine, EccChip: EccInstructions<C> + Clone + Debug + Eq> Point<C,
                 inner,
             })
     }
-
-    /// Returns `self + other` using incomplete addition.
-    pub fn add_incomplete(
-        &self,
-        mut layouter: impl Layouter<C::Base>,
-        other: &Self,
-    ) -> Result<Self, Error> {
-        assert_eq!(self.chip, other.chip);
-        self.chip
-            .add_incomplete(&mut layouter, &self.inner, &other.inner)
-            .map(|inner| Point {
-                chip: self.chip.clone(),
-                inner,
-            })
-    }
-
-    /// Returns `[by] self`.
-    pub fn mul(
-        &self,
-        mut layouter: impl Layouter<C::Base>,
-        by: &EccChip::Var,
-    ) -> Result<(Self, ScalarVar<C, EccChip>), Error> {
-        self.chip
-            .mul(&mut layouter, by, &self.inner)
-            .map(|(point, scalar)| {
-                (
-                    Point {
-                        chip: self.chip.clone(),
-                        inner: point,
-                    },
-                    ScalarVar {
-                        chip: self.chip.clone(),
-                        inner: scalar,
-                    },
-                )
-            })
-    }
 }
 
 /// The affine short Weierstrass x-coordinate of an elliptic curve point over the
@@ -463,34 +562,60 @@ mod tests {
             // provided by the Sinsemilla chip.
             config.lookup_config.load(&mut layouter)?;
 
-            // Generate a random point P
+            // Generate a random non-identity point P
             let p_val = pallas::Point::random(rand::rngs::OsRng).to_affine(); // P
-            let p = super::Point::new(chip.clone(), layouter.namespace(|| "P"), Some(p_val))?;
+            let p = super::NonIdentityPoint::new(
+                chip.clone(),
+                layouter.namespace(|| "P"),
+                Some(p_val),
+            )?;
             let p_neg = -p_val;
-            let p_neg = super::Point::new(chip.clone(), layouter.namespace(|| "-P"), Some(p_neg))?;
+            let p_neg = super::NonIdentityPoint::new(
+                chip.clone(),
+                layouter.namespace(|| "-P"),
+                Some(p_neg),
+            )?;
 
-            // Generate a random point Q
+            // Generate a random non-identity point Q
             let q_val = pallas::Point::random(rand::rngs::OsRng).to_affine(); // Q
-            let q = super::Point::new(chip.clone(), layouter.namespace(|| "Q"), Some(q_val))?;
+            let q = super::NonIdentityPoint::new(
+                chip.clone(),
+                layouter.namespace(|| "Q"),
+                Some(q_val),
+            )?;
 
             // Make sure P and Q are not the same point.
             assert_ne!(p_val, q_val);
 
-            // Generate a (0,0) point to be used in other tests.
+            // Test that we can witness the identity as a point, but not as a non-identity point.
-            let zero = {
+            {
-                super::Point::new(
+                let _ = super::Point::new(
                     chip.clone(),
                     layouter.namespace(|| "identity"),
                     Some(pallas::Affine::identity()),
-                )?
+                )?;
-            };
+
+                super::NonIdentityPoint::new(
+                    chip.clone(),
+                    layouter.namespace(|| "identity"),
+                    Some(pallas::Affine::identity()),
+                )
+                .expect_err("Trying to witness the identity should return an error");
+            }
+
+            // Test witness non-identity point
+            {
+                super::chip::witness_point::tests::test_witness_non_id(
+                    chip.clone(),
+                    layouter.namespace(|| "witness non-identity point"),
+                )
+            }
 
             // Test complete addition
             {
                 super::chip::add::tests::test_add(
                     chip.clone(),
                     layouter.namespace(|| "complete addition"),
-                    &zero,
                     p_val,
                     &p,
                     q_val,
@@ -504,7 +629,6 @@ mod tests {
                 super::chip::add_incomplete::tests::test_add_incomplete(
                     chip.clone(),
                     layouter.namespace(|| "incomplete addition"),
-                    &zero,
                     p_val,
                     &p,
                     q_val,
@@ -518,7 +642,6 @@ mod tests {
                 super::chip::mul::tests::test_mul(
                     chip.clone(),
                     layouter.namespace(|| "variable-base scalar mul"),
-                    &zero,
                     &p,
                     p_val,
                 )?;

src/circuit/gadget/ecc/chip.rs

@@ -22,9 +22,10 @@ pub(super) mod mul;
 pub(super) mod mul_fixed;
 pub(super) mod witness_point;
 
-/// A curve point represented in affine (x, y) coordinates. Each coordinate is
+/// A curve point represented in affine (x, y) coordinates, or the
-/// assigned to a cell.
+/// identity represented as (0, 0).
-#[derive(Clone, Debug)]
+/// Each coordinate is assigned to a cell.
+#[derive(Copy, Clone, Debug)]
 pub struct EccPoint {
     /// x-coordinate
     x: CellValue<pallas::Base>,
@@ -67,6 +68,62 @@ impl EccPoint {
     pub fn y(&self) -> CellValue<pallas::Base> {
         self.y
     }
+
+    #[cfg(test)]
+    fn is_identity(&self) -> Option<bool> {
+        self.x.value().map(|x| x == pallas::Base::zero())
+    }
+}
+
+/// A non-identity point represented in affine (x, y) coordinates.
+/// Each coordinate is assigned to a cell.
+#[derive(Copy, Clone, Debug)]
+pub struct NonIdentityEccPoint {
+    /// x-coordinate
+    x: CellValue<pallas::Base>,
+    /// y-coordinate
+    y: CellValue<pallas::Base>,
+}
+
+impl NonIdentityEccPoint {
+    /// Constructs a point from its coordinates, without checking they are on the curve.
+    ///
+    /// This is an internal API that we only use where we know we have a valid non-identity
+    /// curve point (specifically inside Sinsemilla).
+    pub(in crate::circuit::gadget) fn from_coordinates_unchecked(
+        x: CellValue<pallas::Base>,
+        y: CellValue<pallas::Base>,
+    ) -> Self {
+        NonIdentityEccPoint { x, y }
+    }
+
+    /// Returns the value of this curve point, if known.
+    pub fn point(&self) -> Option<pallas::Affine> {
+        match (self.x.value(), self.y.value()) {
+            (Some(x), Some(y)) => {
+                assert!(x != pallas::Base::zero() && y != pallas::Base::zero());
+                Some(pallas::Affine::from_xy(x, y).unwrap())
+            }
+            _ => None,
+        }
+    }
+    /// The cell containing the affine short-Weierstrass x-coordinate.
+    pub fn x(&self) -> CellValue<pallas::Base> {
+        self.x
+    }
+    /// The cell containing the affine short-Weierstrass y-coordinate.
+    pub fn y(&self) -> CellValue<pallas::Base> {
+        self.y
+    }
+}
+
+impl From<NonIdentityEccPoint> for EccPoint {
+    fn from(non_id_point: NonIdentityEccPoint) -> Self {
+        Self {
+            x: non_id_point.x,
+            y: non_id_point.y,
+        }
+    }
 }
 
 /// Configuration for the ECC chip
@@ -108,8 +165,10 @@ pub struct EccConfig {
     /// when the scalar is a signed short exponent or a base-field element.
     pub q_mul_fixed_running_sum: Selector,
 
-    /// Witness point
+    /// Witness point (can be identity)
     pub q_point: Selector,
+    /// Witness non-identity point
+    pub q_point_non_id: Selector,
 
     /// Lookup range check using 10-bit lookup table
     pub lookup_config: LookupRangeCheckConfig<pallas::Base, { sinsemilla::K }>,
@@ -206,6 +265,7 @@ impl EccChip {
             q_mul_fixed_base_field: meta.selector(),
             q_mul_fixed_running_sum,
             q_point: meta.selector(),
+            q_point_non_id: meta.selector(),
             lookup_config: range_check,
             running_sum_config,
         };
@@ -320,6 +380,7 @@ impl EccInstructions<pallas::Affine> for EccChip {
     type ScalarFixedShort = EccScalarFixedShort;
     type ScalarVar = CellValue<pallas::Base>;
     type Point = EccPoint;
+    type NonIdentityPoint = NonIdentityEccPoint;
     type X = CellValue<pallas::Base>;
     type FixedPoints = OrchardFixedBasesFull;
     type FixedPointsBaseField = NullifierK;
@@ -350,20 +411,33 @@ impl EccInstructions<pallas::Affine> for EccChip {
         let config: witness_point::Config = self.config().into();
         layouter.assign_region(
             || "witness point",
-            |mut region| config.assign_region(value, 0, &mut region),
+            |mut region| config.point(value, 0, &mut region),
         )
     }
 
-    fn extract_p(point: &Self::Point) -> &Self::X {
+    fn witness_point_non_id(
-        &point.x
+        &self,
+        layouter: &mut impl Layouter<pallas::Base>,
+        value: Option<pallas::Affine>,
+    ) -> Result<Self::NonIdentityPoint, Error> {
+        let config: witness_point::Config = self.config().into();
+        layouter.assign_region(
+            || "witness non-identity point",
+            |mut region| config.point_non_id(value, 0, &mut region),
+        )
+    }
+
+    fn extract_p<Point: Into<Self::Point> + Clone>(point: &Point) -> Self::X {
+        let point: EccPoint = (point.clone()).into();
+        point.x()
     }
 
     fn add_incomplete(
         &self,
         layouter: &mut impl Layouter<pallas::Base>,
-        a: &Self::Point,
+        a: &Self::NonIdentityPoint,
-        b: &Self::Point,
+        b: &Self::NonIdentityPoint,
-    ) -> Result<Self::Point, Error> {
+    ) -> Result<Self::NonIdentityPoint, Error> {
         let config: add_incomplete::Config = self.config().into();
         layouter.assign_region(
             || "incomplete point addition",
@@ -371,16 +445,18 @@ impl EccInstructions<pallas::Affine> for EccChip {
         )
     }
 
-    fn add(
+    fn add<A: Into<Self::Point> + Clone, B: Into<Self::Point> + Clone>(
         &self,
         layouter: &mut impl Layouter<pallas::Base>,
-        a: &Self::Point,
+        a: &A,
-        b: &Self::Point,
+        b: &B,
     ) -> Result<Self::Point, Error> {
         let config: add::Config = self.config().into();
         layouter.assign_region(
             || "complete point addition",
-            |mut region| config.assign_region(a, b, 0, &mut region),
+            |mut region| {
+                config.assign_region(&(a.clone()).into(), &(b.clone()).into(), 0, &mut region)
+            },
         )
     }
 
@@ -388,7 +464,7 @@ impl EccInstructions<pallas::Affine> for EccChip {
         &self,
         layouter: &mut impl Layouter<pallas::Base>,
         scalar: &Self::Var,
-        base: &Self::Point,
+        base: &Self::NonIdentityPoint,
     ) -> Result<(Self::Point, Self::ScalarVar), Error> {
         let config: mul::Config = self.config().into();
         config.assign(

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

@@ -391,38 +391,40 @@ pub mod tests {
     use halo2::{circuit::Layouter, plonk::Error};
     use pasta_curves::{arithmetic::CurveExt, pallas};
 
-    use crate::circuit::gadget::ecc::{EccInstructions, Point};
+    use crate::circuit::gadget::ecc::{chip::EccPoint, EccInstructions, NonIdentityPoint};
 
     #[allow(clippy::too_many_arguments)]
-    pub fn test_add<EccChip: EccInstructions<pallas::Affine> + Clone + Eq + std::fmt::Debug>(
+    pub fn test_add<
+        EccChip: EccInstructions<pallas::Affine, Point = EccPoint> + Clone + Eq + std::fmt::Debug,
+    >(
         chip: EccChip,
         mut layouter: impl Layouter<pallas::Base>,
-        zero: &Point<pallas::Affine, EccChip>,
         p_val: pallas::Affine,
-        p: &Point<pallas::Affine, EccChip>,
+        p: &NonIdentityPoint<pallas::Affine, EccChip>,
         q_val: pallas::Affine,
-        q: &Point<pallas::Affine, EccChip>,
+        q: &NonIdentityPoint<pallas::Affine, EccChip>,
-        p_neg: &Point<pallas::Affine, EccChip>,
+        p_neg: &NonIdentityPoint<pallas::Affine, EccChip>,
     ) -> Result<(), Error> {
         // Make sure P and Q are not the same point.
         assert_ne!(p_val, q_val);
 
         // Check complete addition P + (-P)
-        {
+        let zero = {
             let result = p.add(layouter.namespace(|| "P + (-P)"), p_neg)?;
-            result.constrain_equal(layouter.namespace(|| "P + (-P) = 𝒪"), zero)?;
+            assert!(result.inner().is_identity().unwrap());
-        }
+            result
+        };
 
         // Check complete addition 𝒪 + 𝒪
         {
-            let result = zero.add(layouter.namespace(|| "𝒪 + 𝒪"), zero)?;
+            let result = zero.add(layouter.namespace(|| "𝒪 + 𝒪"), &zero)?;
-            result.constrain_equal(layouter.namespace(|| "𝒪 + 𝒪 = 𝒪"), zero)?;
+            result.constrain_equal(layouter.namespace(|| "𝒪 + 𝒪 = 𝒪"), &zero)?;
         }
 
         // Check P + Q
         {
             let result = p.add(layouter.namespace(|| "P + Q"), q)?;
-            let witnessed_result = Point::new(
+            let witnessed_result = NonIdentityPoint::new(
                 chip.clone(),
                 layouter.namespace(|| "witnessed P + Q"),
                 Some((p_val + q_val).to_affine()),
@@ -433,7 +435,7 @@ pub mod tests {
         // P + P
         {
             let result = p.add(layouter.namespace(|| "P + P"), p)?;
-            let witnessed_result = Point::new(
+            let witnessed_result = NonIdentityPoint::new(
                 chip.clone(),
                 layouter.namespace(|| "witnessed P + P"),
                 Some((p_val + p_val).to_affine()),
@@ -443,7 +445,7 @@ pub mod tests {
 
         // P + 𝒪
         {
-            let result = p.add(layouter.namespace(|| "P + 𝒪"), zero)?;
+            let result = p.add(layouter.namespace(|| "P + 𝒪"), &zero)?;
             result.constrain_equal(layouter.namespace(|| "P + 𝒪 = P"), p)?;
         }
 
@@ -455,7 +457,7 @@ pub mod tests {
 
         // (x, y) + (ζx, y) should behave like normal P + Q.
         let endo_p = p_val.to_curve().endo();
-        let endo_p = Point::new(
+        let endo_p = NonIdentityPoint::new(
             chip.clone(),
             layouter.namespace(|| "endo(P)"),
             Some(endo_p.to_affine()),
@@ -464,7 +466,7 @@ pub mod tests {
 
         // (x, y) + (ζx, -y) should also behave like normal P + Q.
         let endo_p_neg = (-p_val).to_curve().endo();
-        let endo_p_neg = Point::new(
+        let endo_p_neg = NonIdentityPoint::new(
             chip.clone(),
             layouter.namespace(|| "endo(-P)"),
             Some(endo_p_neg.to_affine()),
@@ -473,7 +475,7 @@ pub mod tests {
 
         // (x, y) + ((ζ^2)x, y)
         let endo_2_p = p_val.to_curve().endo().endo();
-        let endo_2_p = Point::new(
+        let endo_2_p = NonIdentityPoint::new(
             chip.clone(),
             layouter.namespace(|| "endo^2(P)"),
             Some(endo_2_p.to_affine()),
@@ -482,7 +484,7 @@ pub mod tests {
 
         // (x, y) + ((ζ^2)x, -y)
         let endo_2_p_neg = (-p_val).to_curve().endo().endo();
-        let endo_2_p_neg = Point::new(
+        let endo_2_p_neg = NonIdentityPoint::new(
             chip,
             layouter.namespace(|| "endo^2(-P)"),
             Some(endo_2_p_neg.to_affine()),

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

@@ -1,6 +1,6 @@
 use std::{array, collections::HashSet};
 
-use super::{copy, CellValue, EccConfig, EccPoint, Var};
+use super::{copy, CellValue, EccConfig, NonIdentityEccPoint, Var};
 use group::Curve;
 use halo2::{
     circuit::Region,
@@ -67,11 +67,11 @@ impl Config {
 
     pub(super) fn assign_region(
         &self,
-        p: &EccPoint,
+        p: &NonIdentityEccPoint,
-        q: &EccPoint,
+        q: &NonIdentityEccPoint,
         offset: usize,
         region: &mut Region<'_, pallas::Base>,
-    ) -> Result<EccPoint, Error> {
+    ) -> Result<NonIdentityEccPoint, Error> {
         // Enable `q_add_incomplete` selector
         self.q_add_incomplete.enable(region, offset)?;
 
@@ -134,7 +134,7 @@ impl Config {
             || y_r.ok_or(Error::SynthesisError),
         )?;
 
-        let result = EccPoint {
+        let result = NonIdentityEccPoint {
             x: CellValue::<pallas::Base>::new(x_r_var, x_r),
             y: CellValue::<pallas::Base>::new(y_r_var, y_r),
         };
@@ -149,7 +149,7 @@ pub mod tests {
     use halo2::{circuit::Layouter, plonk::Error};
     use pasta_curves::pallas;
 
-    use crate::circuit::gadget::ecc::{EccInstructions, Point};
+    use crate::circuit::gadget::ecc::{EccInstructions, NonIdentityPoint};
 
     #[allow(clippy::too_many_arguments)]
     pub fn test_add_incomplete<
@@ -157,17 +157,16 @@ pub mod tests {
     >(
         chip: EccChip,
         mut layouter: impl Layouter<pallas::Base>,
-        zero: &Point<pallas::Affine, EccChip>,
         p_val: pallas::Affine,
-        p: &Point<pallas::Affine, EccChip>,
+        p: &NonIdentityPoint<pallas::Affine, EccChip>,
         q_val: pallas::Affine,
-        q: &Point<pallas::Affine, EccChip>,
+        q: &NonIdentityPoint<pallas::Affine, EccChip>,
-        p_neg: &Point<pallas::Affine, EccChip>,
+        p_neg: &NonIdentityPoint<pallas::Affine, EccChip>,
     ) -> Result<(), Error> {
         // P + Q
         {
             let result = p.add_incomplete(layouter.namespace(|| "P + Q"), q)?;
-            let witnessed_result = Point::new(
+            let witnessed_result = NonIdentityPoint::new(
                 chip,
                 layouter.namespace(|| "witnessed P + Q"),
                 Some((p_val + q_val).to_affine()),
@@ -183,18 +182,6 @@ pub mod tests {
         p.add_incomplete(layouter.namespace(|| "P + (-P)"), p_neg)
             .expect_err("P + (-P) should return an error");
 
-        // P + 𝒪 should return an error
-        p.add_incomplete(layouter.namespace(|| "P + 𝒪"), zero)
-            .expect_err("P + 0 should return an error");
-
-        // 𝒪 + P should return an error
-        zero.add_incomplete(layouter.namespace(|| "𝒪 + P"), p)
-            .expect_err("0 + P should return an error");
-
-        // 𝒪 + 𝒪 should return an error
-        zero.add_incomplete(layouter.namespace(|| "𝒪 + 𝒪"), zero)
-            .expect_err("𝒪 + 𝒪 should return an error");
-
         Ok(())
     }
 }

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

@@ -1,4 +1,4 @@
-use super::{add, CellValue, EccConfig, EccPoint, Var};
+use super::{add, CellValue, EccConfig, EccPoint, NonIdentityEccPoint, Var};
 use crate::{circuit::gadget::utilities::copy, constants::T_Q};
 use std::ops::{Deref, Range};
 
@@ -136,12 +136,16 @@ impl Config {
         &self,
         mut layouter: impl Layouter<pallas::Base>,
         alpha: CellValue<pallas::Base>,
-        base: &EccPoint,
+        base: &NonIdentityEccPoint,
     ) -> Result<(EccPoint, CellValue<pallas::Base>), Error> {
         let (result, zs): (EccPoint, Vec<Z<pallas::Base>>) = layouter.assign_region(
             || "variable-base scalar mul",
             |mut region| {
                 let offset = 0;
+
+                // Case `base` into an `EccPoint` for later use.
+                let base_point: EccPoint = (*base).into();
+
                 // Decompose `k = alpha + t_q` bitwise (big-endian bit order).
                 let bits = decompose_for_scalar_mul(alpha.value());
 
@@ -151,9 +155,9 @@ impl Config {
                 let lsb = bits[pallas::Scalar::NUM_BITS as usize - 1];
 
                 // Initialize the accumulator `acc = [2]base`
-                let acc = self
+                let acc =
-                    .add_config
+                    self.add_config
-                    .assign_region(base, base, offset, &mut region)?;
+                        .assign_region(&base_point, &base_point, offset, &mut region)?;
 
                 // Increase the offset by 1 after complete addition.
                 let offset = offset + 1;
@@ -207,7 +211,7 @@ impl Config {
                         &mut region,
                         offset,
                         bits_complete,
-                        base,
+                        &base_point,
                         x_a,
                         y_a,
                         *z,
@@ -282,7 +286,7 @@ impl Config {
         &self,
         region: &mut Region<'_, pallas::Base>,
         offset: usize,
-        base: &EccPoint,
+        base: &NonIdentityEccPoint,
         acc: EccPoint,
         z_1: Z<pallas::Base>,
         lsb: Option<bool>,
@@ -449,21 +453,23 @@ pub mod tests {
     use pasta_curves::{arithmetic::FieldExt, pallas};
 
     use crate::circuit::gadget::{
-        ecc::{chip::EccChip, EccInstructions, Point},
+        ecc::{
+            chip::{EccChip, EccPoint},
+            EccInstructions, NonIdentityPoint, Point,
+        },
         utilities::UtilitiesInstructions,
     };
 
     pub fn test_mul(
         chip: EccChip,
         mut layouter: impl Layouter<pallas::Base>,
-        zero: &Point<pallas::Affine, EccChip>,
+        p: &NonIdentityPoint<pallas::Affine, EccChip>,
-        p: &Point<pallas::Affine, EccChip>,
         p_val: pallas::Affine,
     ) -> Result<(), Error> {
         let column = chip.config().advices[0];
 
-        fn constrain_equal<
+        fn constrain_equal_non_id<
-            EccChip: EccInstructions<pallas::Affine> + Clone + Eq + std::fmt::Debug,
+            EccChip: EccInstructions<pallas::Affine, Point = EccPoint> + Clone + Eq + std::fmt::Debug,
         >(
             chip: EccChip,
             mut layouter: impl Layouter<pallas::Base>,
@@ -474,7 +480,7 @@ pub mod tests {
             // Move scalar from base field into scalar field (which always fits
             // for Pallas).
             let scalar = pallas::Scalar::from_bytes(&scalar_val.to_bytes()).unwrap();
-            let expected = Point::new(
+            let expected = NonIdentityPoint::new(
                 chip,
                 layouter.namespace(|| "expected point"),
                 Some((base_val * scalar).to_affine()),
@@ -493,7 +499,7 @@ pub mod tests {
                 )?;
                 p.mul(layouter.namespace(|| "random [a]B"), &scalar)?
             };
-            constrain_equal(
+            constrain_equal_non_id(
                 chip.clone(),
                 layouter.namespace(|| "random [a]B"),
                 p_val,
@@ -502,19 +508,6 @@ pub mod tests {
             )?;
         }
 
-        // [a]𝒪 should return an error since variable-base scalar multiplication
-        // uses incomplete addition at the beginning of its double-and-add.
-        {
-            let scalar_val = pallas::Base::rand();
-            let scalar = chip.load_private(
-                layouter.namespace(|| "random scalar"),
-                column,
-                Some(scalar_val),
-            )?;
-            zero.mul(layouter.namespace(|| "[a]𝒪"), &scalar)
-                .expect_err("[a]𝒪 should return an error");
-        }
-
         // [0]B should return (0,0) since variable-base scalar multiplication
         // uses complete addition for the final bits of the scalar.
         {
@@ -524,13 +517,7 @@ pub mod tests {
                     chip.load_private(layouter.namespace(|| "zero"), column, Some(scalar_val))?;
                 p.mul(layouter.namespace(|| "[0]B"), &scalar)?
             };
-            constrain_equal(
+            assert!(result.inner().is_identity().unwrap());
-                chip.clone(),
-                layouter.namespace(|| "[0]B"),
-                p_val,
-                scalar_val,
-                result,
-            )?;
         }
 
         // [-1]B (the largest possible base field element)
@@ -541,7 +528,7 @@ pub mod tests {
                     chip.load_private(layouter.namespace(|| "-1"), column, Some(scalar_val))?;
                 p.mul(layouter.namespace(|| "[-1]B"), &scalar)?
             };
-            constrain_equal(
+            constrain_equal_non_id(
                 chip,
                 layouter.namespace(|| "[-1]B"),
                 p_val,

src/circuit/gadget/ecc/chip/mul/incomplete.rs

@@ -1,6 +1,6 @@
 use std::ops::Deref;
 
-use super::super::{copy, CellValue, EccConfig, EccPoint, Var};
+use super::super::{copy, CellValue, EccConfig, NonIdentityEccPoint, Var};
 use super::{INCOMPLETE_HI_RANGE, INCOMPLETE_LO_RANGE, X, Y, Z};
 use ff::Field;
 use halo2::{
@@ -198,18 +198,19 @@ impl Config {
         });
     }
 
-    // We perform incomplete addition on all but the last three bits of the
+    /// We perform incomplete addition on all but the last three bits of the
-    // decomposed scalar.
+    /// decomposed scalar.
-    // We split the bits in the incomplete addition range into "hi" and "lo"
+    /// We split the bits in the incomplete addition range into "hi" and "lo"
-    // halves and process them side by side, using the same rows but with
+    /// halves and process them side by side, using the same rows but with
-    // non-overlapping columns.
+    /// non-overlapping columns. The base is never the identity point even at
-    // Returns (x, y, z).
+    /// the boundary between halves.
+    /// Returns (x, y, z).
     #[allow(clippy::type_complexity)]
     pub(super) fn double_and_add(
         &self,
         region: &mut Region<'_, pallas::Base>,
         offset: usize,
-        base: &EccPoint,
+        base: &NonIdentityEccPoint,
         bits: &[Option<bool>],
         acc: (X<pallas::Base>, Y<pallas::Base>, Z<pallas::Base>),
     ) -> Result<(X<pallas::Base>, Y<pallas::Base>, Vec<Z<pallas::Base>>), Error> {

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

@@ -1,6 +1,6 @@
 use super::{
-    add, add_incomplete, CellValue, EccBaseFieldElemFixed, EccConfig, EccPoint, EccScalarFixed,
+    add, add_incomplete, CellValue, EccBaseFieldElemFixed, EccConfig, EccScalarFixed,
-    EccScalarFixedShort, Var,
+    EccScalarFixedShort, NonIdentityEccPoint, Var,
 };
 use crate::constants::{
     self,
@@ -220,7 +220,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         scalar: &ScalarFixed,
         base: OrchardFixedBases,
         coords_check_toggle: Selector,
-    ) -> Result<(EccPoint, EccPoint), Error> {
+    ) -> Result<(NonIdentityEccPoint, NonIdentityEccPoint), Error> {
         // Assign fixed columns for given fixed base
         self.assign_fixed_constants(region, offset, base, coords_check_toggle)?;
 
@@ -320,7 +320,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         k: Option<pallas::Scalar>,
         k_usize: Option<usize>,
         base: OrchardFixedBases,
-    ) -> Result<EccPoint, Error> {
+    ) -> Result<NonIdentityEccPoint, Error> {
         let base_value = base.generator();
         let base_u = base.u();
 
@@ -330,7 +330,11 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
                 k.map(|k| base_value * (k + *TWO_SCALAR) * H_SCALAR.pow(&[w as u64, 0, 0, 0]));
             let mul_b = mul_b.map(|mul_b| mul_b.to_affine().coordinates().unwrap());
 
-            let x = mul_b.map(|mul_b| *mul_b.x());
+            let x = mul_b.map(|mul_b| {
+                let x = *mul_b.x();
+                assert!(x != pallas::Base::zero());
+                x
+            });
             let x_cell = region.assign_advice(
                 || format!("mul_b_x, window {}", w),
                 self.x_p,
@@ -339,7 +343,11 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
             )?;
             let x = CellValue::new(x_cell, x);
 
-            let y = mul_b.map(|mul_b| *mul_b.y());
+            let y = mul_b.map(|mul_b| {
+                let y = *mul_b.y();
+                assert!(y != pallas::Base::zero());
+                y
+            });
             let y_cell = region.assign_advice(
                 || format!("mul_b_y, window {}", w),
                 self.y_p,
@@ -348,7 +356,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
             )?;
             let y = CellValue::new(y_cell, y);
 
-            EccPoint { x, y }
+            NonIdentityEccPoint { x, y }
         };
 
         // Assign u = (y_p + z_w).sqrt()
@@ -369,7 +377,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         offset: usize,
         base: OrchardFixedBases,
         scalar: &ScalarFixed,
-    ) -> Result<EccPoint, Error> {
+    ) -> Result<NonIdentityEccPoint, Error> {
         // Recall that the message at each window `w` is represented as
         // `m_w = [(k_w + 2) ⋅ 8^w]B`.
         // When `w = 0`, we have `m_0 = [(k_0 + 2)]B`.
@@ -383,10 +391,10 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         &self,
         region: &mut Region<'_, pallas::Base>,
         offset: usize,
-        mut acc: EccPoint,
+        mut acc: NonIdentityEccPoint,
         base: OrchardFixedBases,
         scalar: &ScalarFixed,
-    ) -> Result<EccPoint, Error> {
+    ) -> Result<NonIdentityEccPoint, Error> {
         let scalar_windows_field = scalar.windows_field();
         let scalar_windows_usize = scalar.windows_usize();
 
@@ -414,7 +422,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         offset: usize,
         base: OrchardFixedBases,
         scalar: &ScalarFixed,
-    ) -> Result<EccPoint, Error> {
+    ) -> Result<NonIdentityEccPoint, Error> {
         // Assign u = (y_p + z_w).sqrt() for the most significant window
         {
             let u_val =
@@ -445,16 +453,25 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
             let mul_b = scalar.map(|scalar| base.generator() * scalar);
             let mul_b = mul_b.map(|mul_b| mul_b.to_affine().coordinates().unwrap());
 
-            let x = mul_b.map(|mul_b| *mul_b.x());
+            let x = mul_b.map(|mul_b| {
+                let x = *mul_b.x();
+                assert!(x != pallas::Base::zero());
+                x
+            });
             let x_cell = region.assign_advice(
                 || format!("mul_b_x, window {}", NUM_WINDOWS - 1),
                 self.x_p,
                 offset + NUM_WINDOWS - 1,
                 || x.ok_or(Error::SynthesisError),
             )?;
+
             let x = CellValue::new(x_cell, x);
 
-            let y = mul_b.map(|mul_b| *mul_b.y());
+            let y = mul_b.map(|mul_b| {
+                let y = *mul_b.y();
+                assert!(y != pallas::Base::zero());
+                y
+            });
             let y_cell = region.assign_advice(
                 || format!("mul_b_y, window {}", NUM_WINDOWS - 1),
                 self.y_p,
@@ -463,7 +480,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
             )?;
             let y = CellValue::new(y_cell, y);
 
-            EccPoint { x, y }
+            NonIdentityEccPoint { x, y }
         };
 
         Ok(mul_b)

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

@@ -196,9 +196,12 @@ impl Config {
         let result = layouter.assign_region(
             || "Base-field elem fixed-base mul (complete addition)",
             |mut region| {
-                self.super_config
+                self.super_config.add_config.assign_region(
-                    .add_config
+                    &mul_b.into(),
-                    .assign_region(&mul_b, &acc, 0, &mut region)
+                    &acc.into(),
+                    0,
+                    &mut region,
+                )
             },
         )?;
 
@@ -386,7 +389,7 @@ pub mod tests {
     use crate::circuit::gadget::{
         ecc::{
             chip::{EccChip, NullifierK},
-            FixedPointBaseField, Point,
+            FixedPointBaseField, NonIdentityPoint, Point,
         },
         utilities::UtilitiesInstructions,
     };
@@ -415,7 +418,7 @@ pub mod tests {
     ) -> Result<(), Error> {
         let column = chip.config().advices[0];
 
-        fn constrain_equal(
+        fn constrain_equal_non_id(
             chip: EccChip,
             mut layouter: impl Layouter<pallas::Base>,
             base_val: pallas::Affine,
@@ -424,7 +427,7 @@ pub mod tests {
         ) -> Result<(), Error> {
             // Move scalar from base field into scalar field (which always fits for Pallas).
             let scalar = pallas::Scalar::from_bytes(&scalar_val.to_bytes()).unwrap();
-            let expected = Point::new(
+            let expected = NonIdentityPoint::new(
                 chip,
                 layouter.namespace(|| "expected point"),
                 Some((base_val * scalar).to_affine()),
@@ -443,7 +446,7 @@ pub mod tests {
                 )?;
                 base.mul(layouter.namespace(|| "random [a]B"), scalar_fixed)?
             };
-            constrain_equal(
+            constrain_equal_non_id(
                 chip.clone(),
                 layouter.namespace(|| "random [a]B"),
                 base_val,
@@ -471,7 +474,7 @@ pub mod tests {
                 )?;
                 base.mul(layouter.namespace(|| "mul with double"), scalar_fixed)?
             };
-            constrain_equal(
+            constrain_equal_non_id(
                 chip.clone(),
                 layouter.namespace(|| "mul with double"),
                 base_val,
@@ -489,13 +492,7 @@ pub mod tests {
                     chip.load_private(layouter.namespace(|| "zero"), column, Some(scalar_fixed))?;
                 base.mul(layouter.namespace(|| "mul by zero"), scalar_fixed)?
             };
-            constrain_equal(
+            assert!(result.inner().is_identity().unwrap());
-                chip.clone(),
-                layouter.namespace(|| "mul by zero"),
-                base_val,
-                scalar_fixed,
-                result,
-            )?;
         }
 
         // [-1]B is the largest base field element
@@ -506,7 +503,7 @@ pub mod tests {
                     chip.load_private(layouter.namespace(|| "-1"), column, Some(scalar_fixed))?;
                 base.mul(layouter.namespace(|| "mul by -1"), scalar_fixed)?
             };
-            constrain_equal(
+            constrain_equal_non_id(
                 chip,
                 layouter.namespace(|| "mul by -1"),
                 base_val,

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

@@ -140,9 +140,12 @@ impl Config {
         let result = layouter.assign_region(
             || "Full-width fixed-base mul (last window, complete addition)",
             |mut region| {
-                self.super_config
+                self.super_config.add_config.assign_region(
-                    .add_config
+                    &mul_b.into(),
-                    .assign_region(&mul_b, &acc, 0, &mut region)
+                    &acc.into(),
+                    0,
+                    &mut region,
+                )
             },
         )?;
 
@@ -172,7 +175,7 @@ pub mod tests {
 
     use crate::circuit::gadget::ecc::{
         chip::{EccChip, OrchardFixedBasesFull},
-        FixedPoint, Point,
+        FixedPoint, NonIdentityPoint, Point,
     };
     use crate::constants;
 
@@ -226,14 +229,14 @@ pub mod tests {
         base: FixedPoint<pallas::Affine, EccChip>,
         base_val: pallas::Affine,
     ) -> Result<(), Error> {
-        fn constrain_equal(
+        fn constrain_equal_non_id(
             chip: EccChip,
             mut layouter: impl Layouter<pallas::Base>,
             base_val: pallas::Affine,
             scalar_val: pallas::Scalar,
             result: Point<pallas::Affine, EccChip>,
         ) -> Result<(), Error> {
-            let expected = Point::new(
+            let expected = NonIdentityPoint::new(
                 chip,
                 layouter.namespace(|| "expected point"),
                 Some((base_val * scalar_val).to_affine()),
@@ -246,7 +249,7 @@ pub mod tests {
             let scalar_fixed = pallas::Scalar::rand();
 
             let (result, _) = base.mul(layouter.namespace(|| "random [a]B"), Some(scalar_fixed))?;
-            constrain_equal(
+            constrain_equal_non_id(
                 chip.clone(),
                 layouter.namespace(|| "random [a]B"),
                 base_val,
@@ -269,7 +272,7 @@ pub mod tests {
             let (result, _) =
                 base.mul(layouter.namespace(|| "mul with double"), Some(scalar_fixed))?;
 
-            constrain_equal(
+            constrain_equal_non_id(
                 chip.clone(),
                 layouter.namespace(|| "mul with double"),
                 base_val,
@@ -283,20 +286,14 @@ pub mod tests {
         {
             let scalar_fixed = pallas::Scalar::zero();
             let (result, _) = base.mul(layouter.namespace(|| "mul by zero"), Some(scalar_fixed))?;
-            constrain_equal(
+            assert!(result.inner().is_identity().unwrap());
-                chip.clone(),
-                layouter.namespace(|| "mul by zero"),
-                base_val,
-                scalar_fixed,
-                result,
-            )?;
         }
 
         // [-1]B is the largest scalar field element.
         {
             let scalar_fixed = -pallas::Scalar::one();
             let (result, _) = base.mul(layouter.namespace(|| "mul by -1"), Some(scalar_fixed))?;
-            constrain_equal(
+            constrain_equal_non_id(
                 chip,
                 layouter.namespace(|| "mul by -1"),
                 base_val,

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

@@ -127,8 +127,8 @@ impl Config {
                 let offset = 0;
                 // Add to the cumulative sum to get `[magnitude]B`.
                 let magnitude_mul = self.super_config.add_config.assign_region(
-                    &mul_b,
+                    &mul_b.into(),
-                    &acc,
+                    &acc.into(),
                     offset,
                     &mut region,
                 )?;
@@ -244,7 +244,7 @@ pub mod tests {
     use pasta_curves::{arithmetic::FieldExt, pallas};
 
     use crate::circuit::gadget::{
-        ecc::{chip::EccChip, FixedPointShort, Point},
+        ecc::{chip::EccChip, FixedPointShort, NonIdentityPoint, Point},
         utilities::{lookup_range_check::LookupRangeCheckConfig, CellValue, UtilitiesInstructions},
     };
     use crate::constants::load::ValueCommitV;
@@ -273,14 +273,14 @@ pub mod tests {
             Ok((magnitude, sign))
         }
 
-        fn constrain_equal(
+        fn constrain_equal_non_id(
             chip: EccChip,
             mut layouter: impl Layouter<pallas::Base>,
             base_val: pallas::Affine,
             scalar_val: pallas::Scalar,
             result: Point<pallas::Affine, EccChip>,
         ) -> Result<(), Error> {
-            let expected = Point::new(
+            let expected = NonIdentityPoint::new(
                 chip,
                 layouter.namespace(|| "expected point"),
                 Some((base_val * scalar_val).to_affine()),
@@ -289,8 +289,6 @@ pub mod tests {
         }
 
         let magnitude_signs = [
-            ("mul by +zero", pallas::Base::zero(), pallas::Base::one()),
-            ("mul by -zero", pallas::Base::zero(), -pallas::Base::one()),
             (
                 "random [a]B",
                 pallas::Base::from_u64(rand::random::<u64>()),
@@ -347,7 +345,7 @@ pub mod tests {
                 };
                 magnitude * sign
             };
-            constrain_equal(
+            constrain_equal_non_id(
                 chip.clone(),
                 layouter.namespace(|| *name),
                 base_val,
@@ -356,6 +354,24 @@ pub mod tests {
             )?;
         }
 
+        let zero_magnitude_signs = [
+            ("mul by +zero", pallas::Base::zero(), pallas::Base::one()),
+            ("mul by -zero", pallas::Base::zero(), -pallas::Base::one()),
+        ];
+
+        for (name, magnitude, sign) in zero_magnitude_signs.iter() {
+            let (result, _) = {
+                let magnitude_sign = load_magnitude_sign(
+                    chip.clone(),
+                    layouter.namespace(|| *name),
+                    *magnitude,
+                    *sign,
+                )?;
+                value_commit_v.mul(layouter.namespace(|| *name), magnitude_sign)?
+            };
+            assert!(result.inner().is_identity().unwrap());
+        }
+
         Ok(())
     }
 
@@ -489,7 +505,7 @@ pub mod tests {
                     Err(vec![
                         VerifyFailure::ConstraintNotSatisfied {
                             constraint: (
-                                (16, "Short fixed-base mul gate").into(),
+                                (17, "Short fixed-base mul gate").into(),
                                 0,
                                 "last_window_check"
                             )
@@ -521,13 +537,13 @@ pub mod tests {
                 prover.verify(),
                 Err(vec![
                     VerifyFailure::ConstraintNotSatisfied {
-                        constraint: ((16, "Short fixed-base mul gate").into(), 1, "sign_check")
+                        constraint: ((17, "Short fixed-base mul gate").into(), 1, "sign_check")
                             .into(),
                         row: 26
                     },
                     VerifyFailure::ConstraintNotSatisfied {
                         constraint: (
-                            (16, "Short fixed-base mul gate").into(),
+                            (17, "Short fixed-base mul gate").into(),
                             3,
                             "negation_check"
                         )

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

@@ -1,10 +1,10 @@
-use super::{CellValue, EccConfig, EccPoint, Var};
+use super::{CellValue, EccConfig, EccPoint, NonIdentityEccPoint, Var};
 
 use group::prime::PrimeCurveAffine;
 
 use halo2::{
     circuit::Region,
-    plonk::{Advice, Column, ConstraintSystem, Error, Expression, Selector},
+    plonk::{Advice, Column, ConstraintSystem, Error, Expression, Selector, VirtualCells},
     poly::Rotation,
 };
 use pasta_curves::{arithmetic::CurveAffine, pallas};
@@ -12,6 +12,7 @@ use pasta_curves::{arithmetic::CurveAffine, pallas};
 #[derive(Clone, Debug)]
 pub struct Config {
     q_point: Selector,
+    q_point_non_id: Selector,
     // x-coordinate
     pub x: Column<Advice>,
     // y-coordinate
@@ -22,6 +23,7 @@ impl From<&EccConfig> for Config {
     fn from(ecc_config: &EccConfig) -> Self {
         Self {
             q_point: ecc_config.q_point,
+            q_point_non_id: ecc_config.q_point_non_id,
             x: ecc_config.advices[0],
             y: ecc_config.advices[1],
         }
@@ -30,8 +32,16 @@ impl From<&EccConfig> for Config {
 
 impl Config {
     pub(super) fn create_gate(&self, meta: &mut ConstraintSystem<pallas::Base>) {
+        let curve_eqn = |meta: &mut VirtualCells<pallas::Base>| {
+            let x = meta.query_advice(self.x, Rotation::cur());
+            let y = meta.query_advice(self.y, Rotation::cur());
+
+            // y^2 = x^3 + b
+            y.square() - (x.clone().square() * x) - Expression::Constant(pallas::Affine::b())
+        };
+
         meta.create_gate("witness point", |meta| {
-            // Check that either the point being witness is either:
+            // Check that the point being witnessed is either:
             // - the identity, which is mapped to (0, 0) in affine coordinates; or
             // - a valid curve point y^2 = x^3 + b, where b = 5 in the Pallas equation
 
@@ -39,37 +49,28 @@ impl Config {
             let x = meta.query_advice(self.x, Rotation::cur());
             let y = meta.query_advice(self.y, Rotation::cur());
 
-            // y^2 = x^3 + b
-            let curve_eqn = y.clone().square()
-                - (x.clone().square() * x.clone())
-                - Expression::Constant(pallas::Affine::b());
-
             vec![
-                ("x == 0 ∨ on_curve", q_point.clone() * x * curve_eqn.clone()),
+                ("x == 0 v on_curve", q_point.clone() * x * curve_eqn(meta)),
-                ("y == 0 ∨ on_curve", q_point * y * curve_eqn),
+                ("y == 0 v on_curve", q_point * y * curve_eqn(meta)),
             ]
         });
+
+        meta.create_gate("witness non-identity point", |meta| {
+            // Check that the point being witnessed is a valid curve point y^2 = x^3 + b,
+            // where b = 5 in the Pallas equation
+
+            let q_point_non_id = meta.query_selector(self.q_point_non_id);
+
+            vec![("on_curve", q_point_non_id * curve_eqn(meta))]
+        });
     }
 
-    pub(super) fn assign_region(
+    fn assign_xy(
         &self,
-        value: Option<pallas::Affine>,
+        value: Option<(pallas::Base, pallas::Base)>,
         offset: usize,
         region: &mut Region<'_, pallas::Base>,
-    ) -> Result<EccPoint, Error> {
+    ) -> Result<(CellValue<pallas::Base>, CellValue<pallas::Base>), Error> {
-        // Enable `q_point` selector
-        self.q_point.enable(region, offset)?;
-
-        let value = value.map(|value| {
-            // Map the identity to (0, 0).
-            if value == pallas::Affine::identity() {
-                (pallas::Base::zero(), pallas::Base::zero())
-            } else {
-                let value = value.coordinates().unwrap();
-                (*value.x(), *value.y())
-            }
-        });
-
         // Assign `x` value
         let x_val = value.map(|value| value.0);
         let x_var = region.assign_advice(
@@ -88,9 +89,83 @@ impl Config {
             || y_val.ok_or(Error::SynthesisError),
         )?;
 
-        Ok(EccPoint {
+        Ok((
-            x: CellValue::<pallas::Base>::new(x_var, x_val),
+            CellValue::<pallas::Base>::new(x_var, x_val),
-            y: CellValue::<pallas::Base>::new(y_var, y_val),
+            CellValue::<pallas::Base>::new(y_var, y_val),
-        })
+        ))
+    }
+
+    /// Assigns a point that can be the identity.
+    pub(super) fn point(
+        &self,
+        value: Option<pallas::Affine>,
+        offset: usize,
+        region: &mut Region<'_, pallas::Base>,
+    ) -> Result<EccPoint, Error> {
+        // Enable `q_point` selector
+        self.q_point.enable(region, offset)?;
+
+        let value = value.map(|value| {
+            // Map the identity to (0, 0).
+            if value == pallas::Affine::identity() {
+                (pallas::Base::zero(), pallas::Base::zero())
+            } else {
+                let value = value.coordinates().unwrap();
+                (*value.x(), *value.y())
+            }
+        });
+
+        self.assign_xy(value, offset, region)
+            .map(|(x, y)| EccPoint { x, y })
+    }
+
+    /// Assigns a non-identity point.
+    pub(super) fn point_non_id(
+        &self,
+        value: Option<pallas::Affine>,
+        offset: usize,
+        region: &mut Region<'_, pallas::Base>,
+    ) -> Result<NonIdentityEccPoint, Error> {
+        // Enable `q_point_non_id` selector
+        self.q_point_non_id.enable(region, offset)?;
+
+        if let Some(value) = value {
+            // Return an error if the point is the identity.
+            if value == pallas::Affine::identity() {
+                return Err(Error::SynthesisError);
+            }
+        };
+
+        let value = value.map(|value| {
+            let value = value.coordinates().unwrap();
+            (*value.x(), *value.y())
+        });
+
+        self.assign_xy(value, offset, region)
+            .map(|(x, y)| NonIdentityEccPoint { x, y })
+    }
+}
+
+#[cfg(test)]
+pub mod tests {
+    use halo2::circuit::Layouter;
+    use pasta_curves::pallas;
+
+    use super::*;
+    use crate::circuit::gadget::ecc::{EccInstructions, NonIdentityPoint};
+
+    pub fn test_witness_non_id<
+        EccChip: EccInstructions<pallas::Affine> + Clone + Eq + std::fmt::Debug,
+    >(
+        chip: EccChip,
+        mut layouter: impl Layouter<pallas::Base>,
+    ) {
+        // Witnessing the identity should return an error.
+        NonIdentityPoint::new(
+            chip,
+            layouter.namespace(|| "witness identity"),
+            Some(pallas::Affine::identity()),
+        )
+        .expect_err("witnessing 𝒪 should return an error");
     }
 }

src/circuit/gadget/sinsemilla.rs

@@ -47,7 +47,7 @@ pub trait SinsemillaInstructions<C: CurveAffine, const K: usize, const MAX_WORDS
     /// The x-coordinate of a point output of [`Self::hash_to_point`].
     type X;
     /// A point output of [`Self::hash_to_point`].
-    type Point: Clone + Debug;
+    type NonIdentityPoint: Clone + Debug;
     /// A type enumerating the fixed points used in `CommitDomains`.
     type FixedPoints: Clone + Debug;
 
@@ -82,10 +82,10 @@ pub trait SinsemillaInstructions<C: CurveAffine, const K: usize, const MAX_WORDS
         layouter: impl Layouter<C::Base>,
         Q: C,
         message: Self::Message,
-    ) -> Result<(Self::Point, Vec<Self::RunningSum>), Error>;
+    ) -> Result<(Self::NonIdentityPoint, Vec<Self::RunningSum>), Error>;
 
     /// Extracts the x-coordinate of the output of a Sinsemilla hash.
-    fn extract(point: &Self::Point) -> Self::X;
+    fn extract(point: &Self::NonIdentityPoint) -> Self::X;
 }
 
 /// A message to be hashed.
@@ -238,7 +238,7 @@ pub struct HashDomain<
     SinsemillaChip: SinsemillaInstructions<C, K, MAX_WORDS> + Clone + Debug + Eq,
     EccChip: EccInstructions<
             C,
-            Point = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::Point,
+            NonIdentityPoint = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::NonIdentityPoint,
             FixedPoints = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::FixedPoints,
         > + Clone
         + Debug
@@ -255,7 +255,7 @@ where
     SinsemillaChip: SinsemillaInstructions<C, K, MAX_WORDS> + Clone + Debug + Eq,
     EccChip: EccInstructions<
             C,
-            Point = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::Point,
+            NonIdentityPoint = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::NonIdentityPoint,
             FixedPoints = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::FixedPoints,
         > + Clone
         + Debug
@@ -283,11 +283,11 @@ where
         &self,
         layouter: impl Layouter<C::Base>,
         message: Message<C, SinsemillaChip, K, MAX_WORDS>,
-    ) -> Result<(ecc::Point<C, EccChip>, Vec<SinsemillaChip::RunningSum>), Error> {
+    ) -> Result<(ecc::NonIdentityPoint<C, EccChip>, Vec<SinsemillaChip::RunningSum>), Error> {
         assert_eq!(self.sinsemilla_chip, message.chip);
         self.sinsemilla_chip
             .hash_to_point(layouter, self.Q, message.inner)
-            .map(|(point, zs)| (ecc::Point::from_inner(self.ecc_chip.clone(), point), zs))
+            .map(|(point, zs)| (ecc::NonIdentityPoint::from_inner(self.ecc_chip.clone(), point), zs))
     }
 
     /// $\mathsf{SinsemillaHash}$ from [§ 5.4.1.9][concretesinsemillahash].
@@ -334,7 +334,7 @@ pub struct CommitDomain<
     SinsemillaChip: SinsemillaInstructions<C, K, MAX_WORDS> + Clone + Debug + Eq,
     EccChip: EccInstructions<
             C,
-            Point = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::Point,
+            NonIdentityPoint = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::NonIdentityPoint,
             FixedPoints = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::FixedPoints,
         > + Clone
         + Debug
@@ -350,7 +350,7 @@ where
     SinsemillaChip: SinsemillaInstructions<C, K, MAX_WORDS> + Clone + Debug + Eq,
     EccChip: EccInstructions<
             C,
-            Point = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::Point,
+            NonIdentityPoint = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::NonIdentityPoint,
             FixedPoints = <SinsemillaChip as SinsemillaInstructions<C, K, MAX_WORDS>>::FixedPoints,
         > + Clone
         + Debug
@@ -377,11 +377,17 @@ where
         mut layouter: impl Layouter<C::Base>,
         message: Message<C, SinsemillaChip, K, MAX_WORDS>,
         r: Option<C::Scalar>,
-    ) -> Result<(ecc::Point<C, EccChip>, Vec<SinsemillaChip::RunningSum>), Error> {
+    ) -> Result<
+        (
+            ecc::Point<C, EccChip>,
+            Vec<SinsemillaChip::RunningSum>,
+        ),
+        Error,
+    > {
         assert_eq!(self.M.sinsemilla_chip, message.chip);
         let (blind, _) = self.R.mul(layouter.namespace(|| "[r] R"), r)?;
         let (p, zs) = self.M.hash_to_point(layouter.namespace(|| "M"), message)?;
-        let commitment = p.add_incomplete(layouter.namespace(|| "M ⸭ [r] R"), &blind)?;
+        let commitment = p.add(layouter.namespace(|| "M + [r] R"), &blind)?;
         Ok((commitment, zs))
     }
 
@@ -418,7 +424,7 @@ mod tests {
         circuit::gadget::{
             ecc::{
                 chip::{EccChip, EccConfig},
-                Point,
+                NonIdentityPoint,
             },
             utilities::lookup_range_check::LookupRangeCheckConfig,
         },
@@ -572,7 +578,7 @@ mod tests {
                         None
                     };
 
-                    Point::new(
+                    NonIdentityPoint::new(
                         ecc_chip.clone(),
                         layouter.namespace(|| "Witness expected parent"),
                         expected_parent,
@@ -623,7 +629,7 @@ mod tests {
                         None
                     };
 
-                    Point::new(
+                    NonIdentityPoint::new(
                         ecc_chip,
                         layouter.namespace(|| "Witness expected result"),
                         expected_result,

src/circuit/gadget/sinsemilla/chip.rs

@@ -4,7 +4,7 @@ use super::{
 };
 use crate::{
     circuit::gadget::{
-        ecc::chip::EccPoint,
+        ecc::chip::NonIdentityEccPoint,
         utilities::{lookup_range_check::LookupRangeCheckConfig, CellValue, Var},
     },
     constants::OrchardFixedBasesFull,
@@ -247,7 +247,7 @@ impl SinsemillaInstructions<pallas::Affine, { sinsemilla::K }, { sinsemilla::C }
     type RunningSum = Vec<Self::CellValue>;
 
     type X = CellValue<pallas::Base>;
-    type Point = EccPoint;
+    type NonIdentityPoint = NonIdentityEccPoint;
     type FixedPoints = OrchardFixedBasesFull;
 
     type HashDomains = SinsemillaHashDomains;
@@ -282,14 +282,14 @@ impl SinsemillaInstructions<pallas::Affine, { sinsemilla::K }, { sinsemilla::C }
         mut layouter: impl Layouter<pallas::Base>,
         Q: pallas::Affine,
         message: Self::Message,
-    ) -> Result<(Self::Point, Vec<Self::RunningSum>), Error> {
+    ) -> Result<(Self::NonIdentityPoint, Vec<Self::RunningSum>), Error> {
         layouter.assign_region(
             || "hash_to_point",
             |mut region| self.hash_message(&mut region, Q, &message),
         )
     }
 
-    fn extract(point: &Self::Point) -> Self::X {
+    fn extract(point: &Self::NonIdentityPoint) -> Self::X {
         point.x()
     }
 }

src/circuit/gadget/sinsemilla/chip/hash_to_point.rs

@@ -1,5 +1,5 @@
 use super::super::SinsemillaInstructions;
-use super::{CellValue, EccPoint, SinsemillaChip, Var};
+use super::{CellValue, NonIdentityEccPoint, SinsemillaChip, Var};
 use crate::primitives::sinsemilla::{self, lebs2ip_k, INV_TWO_POW_K, SINSEMILLA_S};
 use halo2::{
     circuit::{Chip, Region},
@@ -26,7 +26,7 @@ impl SinsemillaChip {
             { sinsemilla::K },
             { sinsemilla::C },
         >>::Message,
-    ) -> Result<(EccPoint, Vec<Vec<CellValue<pallas::Base>>>), Error> {
+    ) -> Result<(NonIdentityEccPoint, Vec<Vec<CellValue<pallas::Base>>>), Error> {
         let config = self.config().clone();
         let mut offset = 0;
 
@@ -147,7 +147,17 @@ impl SinsemillaChip {
             }
         }
 
-        Ok((EccPoint::from_coordinates_unchecked(x_a.0, y_a), zs_sum))
+        if let Some(x_a) = x_a.value() {
+            if let Some(y_a) = y_a.value() {
+                if x_a == pallas::Base::zero() || y_a == pallas::Base::zero() {
+                    return Err(Error::SynthesisError);
+                }
+            }
+        }
+        Ok((
+            NonIdentityEccPoint::from_coordinates_unchecked(x_a.0, y_a),
+            zs_sum,
+        ))
     }
 
     #[allow(clippy::type_complexity)]

src/circuit/gadget/sinsemilla/merkle/chip.rs

@@ -417,11 +417,11 @@ impl SinsemillaInstructions<pallas::Affine, { sinsemilla::K }, { sinsemilla::C }
         { sinsemilla::K },
         { sinsemilla::C },
     >>::X;
-    type Point = <SinsemillaChip as SinsemillaInstructions<
+    type NonIdentityPoint = <SinsemillaChip as SinsemillaInstructions<
         pallas::Affine,
         { sinsemilla::K },
         { sinsemilla::C },
-    >>::Point;
+    >>::NonIdentityPoint;
     type FixedPoints = <SinsemillaChip as SinsemillaInstructions<
         pallas::Affine,
         { sinsemilla::K },
@@ -457,13 +457,13 @@ impl SinsemillaInstructions<pallas::Affine, { sinsemilla::K }, { sinsemilla::C }
         layouter: impl Layouter<pallas::Base>,
         Q: pallas::Affine,
         message: Self::Message,
-    ) -> Result<(Self::Point, Vec<Vec<Self::CellValue>>), Error> {
+    ) -> Result<(Self::NonIdentityPoint, Vec<Vec<Self::CellValue>>), Error> {
         let config = self.config().sinsemilla_config.clone();
         let chip = SinsemillaChip::construct(config);
         chip.hash_to_point(layouter, Q, message)
     }
 
-    fn extract(point: &Self::Point) -> Self::X {
+    fn extract(point: &Self::NonIdentityPoint) -> Self::X {
         SinsemillaChip::extract(point)
     }
 }

src/circuit/gadget/sinsemilla/note_commit.rs

@@ -8,7 +8,7 @@ use pasta_curves::{arithmetic::FieldExt, pallas};
 use crate::{
     circuit::gadget::{
         ecc::{
-            chip::{EccChip, EccPoint},
+            chip::{EccChip, NonIdentityEccPoint},
             Point,
         },
         utilities::{bitrange_subset, bool_check, copy, CellValue, Var},
@@ -523,8 +523,8 @@ impl NoteCommitConfig {
         mut layouter: impl Layouter<pallas::Base>,
         chip: SinsemillaChip,
         ecc_chip: EccChip,
-        g_d: &EccPoint,
+        g_d: &NonIdentityEccPoint,
-        pk_d: &EccPoint,
+        pk_d: &NonIdentityEccPoint,
         value: CellValue<pallas::Base>,
         rho: CellValue<pallas::Base>,
         psi: CellValue<pallas::Base>,
@@ -1432,7 +1432,7 @@ mod tests {
         circuit::gadget::{
             ecc::{
                 chip::{EccChip, EccConfig},
-                Point,
+                NonIdentityPoint,
             },
             sinsemilla::chip::SinsemillaChip,
             utilities::{
@@ -1566,7 +1566,11 @@ mod tests {
                         pallas::Affine::from_xy(x, y).unwrap()
                     });
 
-                    Point::new(ecc_chip.clone(), layouter.namespace(|| "witness g_d"), g_d)?
+                    NonIdentityPoint::new(
+                        ecc_chip.clone(),
+                        layouter.namespace(|| "witness g_d"),
+                        g_d,
+                    )?
                 };
 
                 // Witness pk_d
@@ -1580,7 +1584,7 @@ mod tests {
                         pallas::Affine::from_xy(x, y).unwrap()
                     });
 
-                    Point::new(
+                    NonIdentityPoint::new(
                         ecc_chip.clone(),
                         layouter.namespace(|| "witness pk_d"),
                         pk_d,
@@ -1674,7 +1678,11 @@ mod tests {
                         )
                         .unwrap()
                         .to_affine();
-                    Point::new(ecc_chip, layouter.namespace(|| "witness g_d"), Some(point))?
+                    NonIdentityPoint::new(
+                        ecc_chip,
+                        layouter.namespace(|| "witness cm"),
+                        Some(point),
+                    )?
                 };
                 cm.constrain_equal(layouter.namespace(|| "cm == expected cm"), &expected_cm)
             }

src/primitives/sinsemilla.rs

@@ -174,7 +174,9 @@ impl CommitDomain {
         msg: impl Iterator<Item = bool>,
         r: &pallas::Scalar,
     ) -> CtOption<pallas::Point> {
-        (self.M.hash_to_point_inner(msg) + Wnaf::new().scalar(r).base(self.R)).into()
+        // We use complete addition for the blinding factor.
+        CtOption::<pallas::Point>::from(self.M.hash_to_point_inner(msg))
+            .map(|p| p + Wnaf::new().scalar(r).base(self.R))
     }
 
     /// $\mathsf{SinsemillaShortCommit}$ from [§ 5.4.8.4][concretesinsemillacommit].

src/primitives/sinsemilla/addition.rs

@@ -46,14 +46,6 @@ impl Add for IncompletePoint {
     }
 }
 
-impl Add<pallas::Point> for IncompletePoint {
-    type Output = IncompletePoint;
-
-    fn add(self, rhs: pallas::Point) -> Self::Output {
-        self + IncompletePoint(CtOption::new(rhs, 1.into()))
-    }
-}
-
 impl Add<pallas::Affine> for IncompletePoint {
     type Output = IncompletePoint;