diff --git a/src/circuit/gadget/ecc.rs b/src/circuit/gadget/ecc.rs index c0194520..fe48336c 100644 --- a/src/circuit/gadget/ecc.rs +++ b/src/circuit/gadget/ecc.rs @@ -642,7 +642,6 @@ mod tests { super::chip::add_incomplete::tests::test_add_incomplete( chip.clone(), layouter.namespace(|| "incomplete addition"), - &zero, p_val, &p, q_val, @@ -656,7 +655,6 @@ mod tests { super::chip::mul::tests::test_mul( chip.clone(), layouter.namespace(|| "variable-base scalar mul"), - &zero, &p, p_val, )?; diff --git a/src/circuit/gadget/ecc/chip/add.rs b/src/circuit/gadget/ecc/chip/add.rs index f1d76245..ab36fd61 100644 --- a/src/circuit/gadget/ecc/chip/add.rs +++ b/src/circuit/gadget/ecc/chip/add.rs @@ -391,7 +391,7 @@ 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::{EccInstructions, NonIdentityPoint, Point}; #[allow(clippy::too_many_arguments)] pub fn test_add + Clone + Eq + std::fmt::Debug>( @@ -399,10 +399,10 @@ pub mod tests { mut layouter: impl Layouter, zero: &Point, p_val: pallas::Affine, - p: &Point, + p: &NonIdentityPoint, q_val: pallas::Affine, - q: &Point, - p_neg: &Point, + q: &NonIdentityPoint, + p_neg: &NonIdentityPoint, ) -> Result<(), Error> { // Make sure P and Q are not the same point. assert_ne!(p_val, q_val); diff --git a/src/circuit/gadget/ecc/chip/add_incomplete.rs b/src/circuit/gadget/ecc/chip/add_incomplete.rs index 5016230c..55495e1e 100644 --- a/src/circuit/gadget/ecc/chip/add_incomplete.rs +++ b/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, - q: &EccPoint, + p: &NonIdentityEccPoint, + q: &NonIdentityEccPoint, offset: usize, region: &mut Region<'_, pallas::Base>, - ) -> Result { + ) -> Result { // 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::::new(x_r_var, x_r), y: CellValue::::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, - zero: &Point, p_val: pallas::Affine, - p: &Point, + p: &NonIdentityPoint, q_val: pallas::Affine, - q: &Point, - p_neg: &Point, + q: &NonIdentityPoint, + p_neg: &NonIdentityPoint, ) -> 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(()) } } diff --git a/src/circuit/gadget/ecc/chip/mul.rs b/src/circuit/gadget/ecc/chip/mul.rs index 05c03bd3..a04bfc42 100644 --- a/src/circuit/gadget/ecc/chip/mul.rs +++ b/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,7 +136,7 @@ impl Config { &self, mut layouter: impl Layouter, alpha: CellValue, - base: &EccPoint, + base: &NonIdentityEccPoint, ) -> Result<(EccPoint, CellValue), Error> { let (result, zs): (EccPoint, Vec>) = layouter.assign_region( || "variable-base scalar mul", @@ -151,9 +151,12 @@ impl Config { let lsb = bits[pallas::Scalar::NUM_BITS as usize - 1]; // Initialize the accumulator `acc = [2]base` - let acc = self - .add_config - .assign_region(base, base, offset, &mut region)?; + let acc = self.add_config.assign_region( + &(base.clone()).into(), + &(base.clone()).into(), + offset, + &mut region, + )?; // Increase the offset by 1 after complete addition. let offset = offset + 1; @@ -207,7 +210,7 @@ impl Config { &mut region, offset, bits_complete, - base, + &(*base).into(), x_a, y_a, *z, @@ -282,7 +285,7 @@ impl Config { &self, region: &mut Region<'_, pallas::Base>, offset: usize, - base: &EccPoint, + base: &NonIdentityEccPoint, acc: EccPoint, z_1: Z, lsb: Option, @@ -449,15 +452,14 @@ pub mod tests { use pasta_curves::{arithmetic::FieldExt, pallas}; use crate::circuit::gadget::{ - ecc::{chip::EccChip, EccInstructions, Point}, + ecc::{chip::EccChip, EccInstructions, NonIdentityPoint, Point}, utilities::UtilitiesInstructions, }; pub fn test_mul( chip: EccChip, mut layouter: impl Layouter, - zero: &Point, - p: &Point, + p: &NonIdentityPoint, p_val: pallas::Affine, ) -> Result<(), Error> { let column = chip.config().advices[0]; @@ -502,19 +504,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. { diff --git a/src/circuit/gadget/ecc/chip/mul/incomplete.rs b/src/circuit/gadget/ecc/chip/mul/incomplete.rs index 29452d94..b6f7d2be 100644 --- a/src/circuit/gadget/ecc/chip/mul/incomplete.rs +++ b/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 - // decomposed scalar. - // 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 - // non-overlapping columns. - // Returns (x, y, z). + /// We perform incomplete addition on all but the last three bits of the + /// decomposed scalar. + /// 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 + /// non-overlapping columns. The base is never the identity point even at + /// 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], acc: (X, Y, Z), ) -> Result<(X, Y, Vec>), Error> { diff --git a/src/circuit/gadget/ecc/chip/mul_fixed.rs b/src/circuit/gadget/ecc/chip/mul_fixed.rs index 90c31aae..2f470111 100644 --- a/src/circuit/gadget/ecc/chip/mul_fixed.rs +++ b/src/circuit/gadget/ecc/chip/mul_fixed.rs @@ -1,6 +1,6 @@ use super::{ - add, add_incomplete, CellValue, EccBaseFieldElemFixed, EccConfig, EccPoint, EccScalarFixed, - EccScalarFixedShort, Var, + add, add_incomplete, CellValue, EccBaseFieldElemFixed, EccConfig, EccScalarFixed, + EccScalarFixedShort, NonIdentityEccPoint, Var, }; use crate::constants::{ self, @@ -220,7 +220,7 @@ impl Config { 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 Config { k: Option, k_usize: Option, base: OrchardFixedBases, - ) -> Result { + ) -> Result { let base_value = base.generator(); let base_u = base.u(); @@ -330,7 +330,11 @@ impl Config { 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 Config { )?; 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 Config { )?; 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 Config { offset: usize, base: OrchardFixedBases, scalar: &ScalarFixed, - ) -> Result { + ) -> Result { // 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 Config { &self, region: &mut Region<'_, pallas::Base>, offset: usize, - mut acc: EccPoint, + mut acc: NonIdentityEccPoint, base: OrchardFixedBases, scalar: &ScalarFixed, - ) -> Result { + ) -> Result { let scalar_windows_field = scalar.windows_field(); let scalar_windows_usize = scalar.windows_usize(); @@ -414,7 +422,7 @@ impl Config { offset: usize, base: OrchardFixedBases, scalar: &ScalarFixed, - ) -> Result { + ) -> Result { // Assign u = (y_p + z_w).sqrt() for the most significant window { let u_val = @@ -445,16 +453,25 @@ impl Config { 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 Config { )?; let y = CellValue::new(y_cell, y); - EccPoint { x, y } + NonIdentityEccPoint { x, y } }; Ok(mul_b) diff --git a/src/circuit/gadget/ecc/chip/mul_fixed/base_field_elem.rs b/src/circuit/gadget/ecc/chip/mul_fixed/base_field_elem.rs index 201624da..b322123d 100644 --- a/src/circuit/gadget/ecc/chip/mul_fixed/base_field_elem.rs +++ 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 - .add_config - .assign_region(&mul_b, &acc, 0, &mut region) + self.super_config.add_config.assign_region( + &mul_b.into(), + &acc.into(), + 0, + &mut region, + ) }, )?; diff --git a/src/circuit/gadget/ecc/chip/mul_fixed/full_width.rs b/src/circuit/gadget/ecc/chip/mul_fixed/full_width.rs index 028b4e53..1fe7947e 100644 --- a/src/circuit/gadget/ecc/chip/mul_fixed/full_width.rs +++ b/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 - .add_config - .assign_region(&mul_b, &acc, 0, &mut region) + self.super_config.add_config.assign_region( + &mul_b.into(), + &acc.into(), + 0, + &mut region, + ) }, )?; diff --git a/src/circuit/gadget/ecc/chip/mul_fixed/short.rs b/src/circuit/gadget/ecc/chip/mul_fixed/short.rs index 0c3223d7..dc2d1ae3 100644 --- a/src/circuit/gadget/ecc/chip/mul_fixed/short.rs +++ b/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, - &acc, + &mul_b.into(), + &acc.into(), offset, &mut region, )?; @@ -489,7 +489,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 +521,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" )