From d44d2779044b66c240d548445ac80e9ff1e1dffb Mon Sep 17 00:00:00 2001
From: Constance Beguier <constance@qed-it.com>
Date: Mon, 4 May 2026 14:46:02 +0200
Subject: [PATCH] Add NonIdentityPoint::new_from_constant (#55)

This PR adds `NonIdentityPoint::new_from_constant` to enable creating non-identity points that are properly pinned to constants, which is required by the Orchard ZSA circuit ([Orchard PR](https://github.com/QED-it/orchard/pull/246)).

More precisely, in the Orchard ZSA circuit, `q_init_zec` / `q_init_zsa` must be constrained to fixed constants, as they define the initial point `Q` of the Sinsemilla hash. If constructed via `NonIdentityPoint::new`, they remain unconstrained witnesses, allowing a prover to inject an arbitrary on-curve point and break commitment soundness.
---
 halo2_gadgets/src/ecc.rs                    | 21 ++++++++++++-
 halo2_gadgets/src/ecc/chip.rs               | 12 ++++++++
 halo2_gadgets/src/ecc/chip/witness_point.rs | 34 ++++++++++++++++++++-
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/halo2_gadgets/src/ecc.rs b/halo2_gadgets/src/ecc.rs
index 326b6e4f5e..1070aad187 100644
--- a/halo2_gadgets/src/ecc.rs
+++ b/halo2_gadgets/src/ecc.rs
@@ -77,6 +77,14 @@ pub trait EccInstructions<C: CurveAffine>:
         value: Value<C>,
     ) -> Result<Self::NonIdentityPoint, Error>;
 
+    /// Witnesses the given constant point with both coordinates pinned via fixed columns.
+    /// Returns an error if the point is the identity.
+    fn witness_point_non_id_from_constant(
+        &self,
+        layouter: &mut impl Layouter<C::Base>,
+        value: C,
+    ) -> Result<Self::NonIdentityPoint, Error>;
+
     /// Witnesses a full-width scalar to be used in variable-base multiplication.
     fn witness_scalar_var(
         &self,
@@ -280,7 +288,8 @@ pub struct NonIdentityPoint<C: CurveAffine, EccChip: EccInstructions<C>> {
 }
 
 impl<C: CurveAffine, EccChip: EccInstructions<C>> NonIdentityPoint<C, EccChip> {
-    /// Constructs a new point with the given value.
+    /// Witnesses the given point with only on-curve / non-identity constraints.
+    /// For known-constant points use [`NonIdentityPoint::new_from_constant`].
     pub fn new(
         chip: EccChip,
         mut layouter: impl Layouter<C::Base>,
@@ -290,6 +299,16 @@ impl<C: CurveAffine, EccChip: EccInstructions<C>> NonIdentityPoint<C, EccChip> {
         point.map(|inner| NonIdentityPoint { chip, inner })
     }
 
+    /// Witnesses the given constant point with both coordinates pinned via fixed columns.
+    pub fn new_from_constant(
+        chip: EccChip,
+        mut layouter: impl Layouter<C::Base>,
+        value: C,
+    ) -> Result<Self, Error> {
+        let point = chip.witness_point_non_id_from_constant(&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,
diff --git a/halo2_gadgets/src/ecc/chip.rs b/halo2_gadgets/src/ecc/chip.rs
index 469c19ebec..12ba4a7d72 100644
--- a/halo2_gadgets/src/ecc/chip.rs
+++ b/halo2_gadgets/src/ecc/chip.rs
@@ -492,6 +492,18 @@ where
         )
     }
 
+    fn witness_point_non_id_from_constant(
+        &self,
+        layouter: &mut impl Layouter<pallas::Base>,
+        value: pallas::Affine,
+    ) -> Result<Self::NonIdentityPoint, Error> {
+        let config = self.config().witness_point;
+        layouter.assign_region(
+            || "witness constant non-identity point",
+            |mut region| config.constant_point_non_id(value, 0, &mut region),
+        )
+    }
+
     fn witness_scalar_var(
         &self,
         _layouter: &mut impl Layouter<pallas::Base>,
diff --git a/halo2_gadgets/src/ecc/chip/witness_point.rs b/halo2_gadgets/src/ecc/chip/witness_point.rs
index 98f865a6dc..d7d279d2e4 100644
--- a/halo2_gadgets/src/ecc/chip/witness_point.rs
+++ b/halo2_gadgets/src/ecc/chip/witness_point.rs
@@ -184,6 +184,30 @@ impl Config {
         self.assign_xy(value, offset, region)
             .map(|(x, y)| NonIdentityEccPoint::from_coordinates_unchecked(x, y))
     }
+
+    /// Assigns a constant non-identity point with both coordinates pinned via fixed columns.
+    pub(super) fn constant_point_non_id(
+        &self,
+        value: 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)?;
+
+        // Return an error if the point is the identity.
+        if value == pallas::Affine::identity() {
+            return Err(Error::Synthesis);
+        }
+
+        let value = {
+            let value = value.coordinates().unwrap();
+            (value.x().into(), value.y().into())
+        };
+
+        self.assign_xy_from_constant(value, offset, region)
+            .map(|(x, y)| NonIdentityEccPoint::from_coordinates_unchecked(x, y))
+    }
 }
 
 #[cfg(test)]
@@ -202,10 +226,18 @@ pub mod tests {
     ) {
         // Witnessing the identity should return an error.
         NonIdentityPoint::new(
-            chip,
+            chip.clone(),
             layouter.namespace(|| "witness identity"),
             Value::known(pallas::Affine::identity()),
         )
         .expect_err("witnessing 𝒪 should return an error");
+
+        // Witnessing the identity from a constant should return an error.
+        NonIdentityPoint::new_from_constant(
+            chip,
+            layouter.namespace(|| "witness identity"),
+            pallas::Affine::identity(),
+        )
+        .expect_err("witnessing 𝒪 should return an error");
     }
 }