diff --git a/circuits/plonky2_ecdsa/src/gadgets/biguint.rs b/circuits/plonky2_ecdsa/src/gadgets/biguint.rs index adf951a6..c3f12545 100644 --- a/circuits/plonky2_ecdsa/src/gadgets/biguint.rs +++ b/circuits/plonky2_ecdsa/src/gadgets/biguint.rs @@ -4,6 +4,7 @@ use alloc::vec; #[cfg(not(test))] use alloc::vec::Vec; use core::marker::PhantomData; +use plonky2_u32::gadgets::range_check::range_check_u32_circuit; use num::{BigUint, Integer, Zero}; use plonky2::field::extension::Extendable; @@ -241,7 +242,7 @@ impl, const D: usize> CircuitBuilderBiguint let div_num_limbs = if b_len > a_len + 1 { 0 } else { - a_len - b_len + 1 + a_len + 1 - b_len }; let div = self.add_virtual_biguint_target(div_num_limbs); let rem = self.add_virtual_biguint_target(b_len); @@ -254,12 +255,16 @@ impl, const D: usize> CircuitBuilderBiguint _phantom: PhantomData, }); + range_check_u32_circuit(self, div.limbs.clone()); + range_check_u32_circuit(self, rem.limbs.clone()); + let div_b = self.mul_biguint(&div, b); let div_b_plus_rem = self.add_biguint(&div_b, &rem); self.connect_biguint(a, &div_b_plus_rem); - let cmp_rem_b = self.cmp_biguint(&rem, b); - self.assert_one(cmp_rem_b.target); + // Check that `b <= rem == false`. + let cmp_rem_b = self.cmp_biguint(b, &rem); + self.assert_zero(cmp_rem_b.target); (div, rem) } diff --git a/circuits/plonky2_ed25519/Cargo.toml b/circuits/plonky2_ed25519/Cargo.toml index 0335a521..f624b2ff 100644 --- a/circuits/plonky2_ed25519/Cargo.toml +++ b/circuits/plonky2_ed25519/Cargo.toml @@ -18,7 +18,7 @@ serde.workspace = true anyhow.workspace = true env_logger.workspace = true log.workspace = true -rand.workspace = true +rand = { workspace = true, features = ["std", "std_rng"] } rand_chacha.workspace = true unroll.workspace = true keccak-hash.workspace = true diff --git a/circuits/plonky2_ed25519/src/curve/curve_adds.rs b/circuits/plonky2_ed25519/src/curve/curve_adds.rs index 4f5c0f98..c0267fa4 100644 --- a/circuits/plonky2_ed25519/src/curve/curve_adds.rs +++ b/circuits/plonky2_ed25519/src/curve/curve_adds.rs @@ -21,30 +21,9 @@ impl Add> for ProjectivePoint { z: z2, } = rhs; - if z1 == C::BaseField::ZERO { - return rhs; - } - if z2 == C::BaseField::ZERO { - return self; - } + debug_assert!(z1.is_nonzero()); + debug_assert!(z2.is_nonzero()); - let x1z2 = x1 * z2; - let y1z2 = y1 * z2; - let x2z1 = x2 * z1; - let y2z1 = y2 * z1; - - // Check if we're doubling or adding inverses. - if x1z2 == x2z1 { - if y1z2 == y2z1 { - // TODO: inline to avoid redundant muls. - return self.double(); - } - if y1z2 == -y2z1 { - return ProjectivePoint::ZERO; - } - } - - // From https://www.hyperelliptic.org/EFD/g1p/auto-twisted-projective.html let a = z1 * z2; let b = a.square(); let c = x1 * x2; @@ -53,7 +32,7 @@ impl Add> for ProjectivePoint { let f = b - e; let g = b + e; let x3 = a * f * ((x1 + y1) * (x2 + y2) - c - d); - let y3 = a * g * (d + c); + let y3 = a * g * (d - C::A * c); let z3 = f * g; ProjectivePoint::nonzero(x3, y3, z3) @@ -64,50 +43,7 @@ impl Add> for ProjectivePoint { type Output = ProjectivePoint; fn add(self, rhs: AffinePoint) -> Self::Output { - let ProjectivePoint { - x: x1, - y: y1, - z: z1, - } = self; - let AffinePoint { - x: x2, - y: y2, - zero: zero2, - } = rhs; - - if z1 == C::BaseField::ZERO { - return rhs.to_projective(); - } - if zero2 { - return self; - } - - let x2z1 = x2 * z1; - let y2z1 = y2 * z1; - - // Check if we're doubling or adding inverses. - if x1 == x2z1 { - if y1 == y2z1 { - // TODO: inline to avoid redundant muls. - return self.double(); - } - if y1 == -y2z1 { - return ProjectivePoint::ZERO; - } - } - - // From https://www.hyperelliptic.org/EFD/g1p/data/shortw/projective/addition/madd-1998-cmo - let u = y2z1 - y1; - let uu = u.square(); - let v = x2z1 - x1; - let vv = v.square(); - let vvv = v * vv; - let r = vv * x1; - let a = uu * z1 - vvv - r.double(); - let x3 = v * a; - let y3 = u * (r - a) - vvv * y1; - let z3 = vvv * z1; - ProjectivePoint::nonzero(x3, y3, z3) + self + rhs.to_projective() } } @@ -115,53 +51,19 @@ impl Add> for AffinePoint { type Output = AffinePoint; fn add(self, rhs: AffinePoint) -> Self::Output { - let AffinePoint { - x: x1, - y: y1, - zero: zero1, - } = self; - let AffinePoint { - x: x2, - y: y2, - zero: zero2, - } = rhs; - - if zero1 { - return rhs; - } - if zero2 { - return self; - } - - // Check if we're doubling or adding inverses. - if x1 == x2 { - if y1 == y2 { - return self.double(); - } - if y1 == -y2 { - return AffinePoint::ZERO; - } - } + let AffinePoint { x: x1, y: y1 } = self; + let AffinePoint { x: x2, y: y2 } = rhs; let x1x2 = x1 * x2; let y1y2 = y1 * y2; let x1y2 = x1 * y2; let y1x2 = y1 * x2; - let x1y2_add_y1x2 = x1y2 + y1x2; - let y1y2_add_x1x2 = y1y2 + x1x2; - let dx1x2y1y2 = C::D * x1x2 * y1y2; - let one_add_dx1x2y1y2 = C::BaseField::ONE + dx1x2y1y2; - let one_sub_dx1x2y1y2 = C::BaseField::ONE - dx1x2y1y2; - let x3 = x1y2_add_y1x2 / one_add_dx1x2y1y2; - let y3 = y1y2_add_x1x2 / one_sub_dx1x2y1y2; + let x3 = (x1y2 + y1x2) / (C::BaseField::ONE + dx1x2y1y2); + let y3 = (y1y2 - C::A * x1x2) / (C::BaseField::ONE - dx1x2y1y2); - Self { - x: x3, - y: y3, - zero: false, - } + Self { x: x3, y: y3 } } } diff --git a/circuits/plonky2_ed25519/src/curve/curve_types.rs b/circuits/plonky2_ed25519/src/curve/curve_types.rs index 260bbee3..b9e5d803 100644 --- a/circuits/plonky2_ed25519/src/curve/curve_types.rs +++ b/circuits/plonky2_ed25519/src/curve/curve_types.rs @@ -30,48 +30,44 @@ pub trait Curve: 'static + Sync + Sized + Copy + Debug { CurveScalar(x) } - fn is_safe_curve() -> bool { - // Added additional check to prevent using vulnerabilties in case a discriminant is equal to 0. - (Self::A.cube().double().double() + Self::D.square().triple().triple().triple()) - .is_nonzero() + fn assert_curve_valid() { + assert!(Self::A.is_nonzero()); + assert!(Self::D.is_nonzero()); + assert_ne!(Self::A, Self::D); } } /// A point on a Twisted Edwards curve, represented in affine coordinates. -#[derive(Copy, Clone, Debug, Deserialize, Serialize)] +#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq, Eq, Hash)] pub struct AffinePoint { pub x: C::BaseField, pub y: C::BaseField, - pub zero: bool, } impl AffinePoint { pub const ZERO: Self = Self { x: C::BaseField::ZERO, - y: C::BaseField::ZERO, - zero: true, + y: C::BaseField::ONE, }; + // TODO: Rename to new pub fn nonzero(x: C::BaseField, y: C::BaseField) -> Self { - let point = Self { x, y, zero: false }; + let point = Self { x, y }; debug_assert!(point.is_valid()); point } pub fn is_valid(&self) -> bool { - let Self { x, y, zero } = *self; - zero || y.square() == x.square() + C::D * x.square() * y.square() + C::A + let Self { x, y } = *self; + C::A * x.square() + y.square() == C::BaseField::ONE + C::D * x.square() * y.square() } pub fn to_projective(&self) -> ProjectivePoint { - let Self { x, y, zero } = *self; - let z = if zero { - C::BaseField::ZERO - } else { - C::BaseField::ONE - }; - - ProjectivePoint { x, y, z } + ProjectivePoint { + x: self.x, + y: self.y, + z: C::BaseField::ONE, + } } pub fn batch_to_projective(affine_points: &[Self]) -> Vec> { @@ -80,63 +76,26 @@ impl AffinePoint { #[must_use] pub fn double(&self) -> Self { - let AffinePoint { x: x1, y: y1, zero } = *self; - - if zero { - return AffinePoint::ZERO; - } - - let x1_x1 = x1 * x1; - let x1_y1 = x1 * y1; - let y1_y1 = y1 * y1; + let AffinePoint { x, y, .. } = *self; - let x1_y1_plus_y1_x1 = x1_y1.double(); - let x1_x1_plus_y1_y1 = x1_x1 + y1_y1; + let x_sq = x * x; + let y_sq = y * y; - let d_x1_x1_y1_y1 = C::D * x1_x1 * y1_y1; - let one_plus_d_x1_x1_y1_y1_plus_1 = C::BaseField::ONE + d_x1_x1_y1_y1; - let one_minus_d_x1_x1_y1_y1 = C::BaseField::ONE - d_x1_x1_y1_y1; + let d_x_sq_y_sq = C::D * x_sq * y_sq; - let x3 = x1_y1_plus_y1_x1 / one_plus_d_x1_x1_y1_y1_plus_1; - let y3 = x1_x1_plus_y1_y1 / one_minus_d_x1_x1_y1_y1; - - Self { - x: x3, - y: y3, - zero: false, - } - } -} + let x1 = (x * y).double() / (C::BaseField::ONE + d_x_sq_y_sq); + let y1 = (y_sq - C::A * x_sq) / (C::BaseField::ONE - d_x_sq_y_sq); -impl PartialEq for AffinePoint { - fn eq(&self, other: &Self) -> bool { - let AffinePoint { - x: x1, - y: y1, - zero: zero1, - } = *self; - let AffinePoint { - x: x2, - y: y2, - zero: zero2, - } = *other; - if zero1 || zero2 { - return zero1 == zero2; - } - x1 == x2 && y1 == y2 + Self::nonzero(x1, y1) } } -impl Eq for AffinePoint {} +impl Neg for AffinePoint { + type Output = AffinePoint; -impl Hash for AffinePoint { - fn hash(&self, state: &mut H) { - if self.zero { - self.zero.hash(state); - } else { - self.x.hash(state); - self.y.hash(state); - } + fn neg(mut self) -> Self::Output { + self.x = -self.x; + self } } @@ -152,7 +111,7 @@ impl ProjectivePoint { pub const ZERO: Self = Self { x: C::BaseField::ZERO, y: C::BaseField::ONE, - z: C::BaseField::ZERO, + z: C::BaseField::ONE, }; pub fn nonzero(x: C::BaseField, y: C::BaseField, z: C::BaseField) -> Self { @@ -163,19 +122,21 @@ impl ProjectivePoint { pub fn is_valid(&self) -> bool { let Self { x, y, z } = *self; - z.is_zero() - || y.square() * z.square() - == x.square() * z.square() + z.square().square() + x.square() * y.square() * C::D + + let x_sq = x.square(); + let y_sq = y.square(); + let z_sq = z.square(); + + z.is_nonzero() && (z_sq * (C::A * x_sq + y_sq) == z_sq.square() + C::D * x_sq * y_sq) } pub fn to_affine(&self) -> AffinePoint { let Self { x, y, z } = *self; - if z == C::BaseField::ZERO { - AffinePoint::ZERO - } else { - let z_inv = z.inverse(); - AffinePoint::nonzero(x * z_inv, y * z_inv) - } + + debug_assert!(z.is_nonzero()); + + let z_inv = z.inverse(); + AffinePoint::nonzero(x * z_inv, y * z_inv) } pub fn batch_to_affine(proj_points: &[Self]) -> Vec> { @@ -186,13 +147,14 @@ impl ProjectivePoint { let mut result = Vec::with_capacity(n); for i in 0..n { let Self { x, y, z } = proj_points[i]; - result.push(if z == C::BaseField::ZERO { - AffinePoint::ZERO - } else { - let z_inv = z_invs[i]; - AffinePoint::nonzero(x * z_inv, y * z_inv) - }); + + debug_assert!(z.is_nonzero()); + + let z_inv = z_invs[i]; + let affine = AffinePoint::nonzero(x * z_inv, y * z_inv); + result.push(affine); } + result } @@ -200,14 +162,13 @@ impl ProjectivePoint { #[must_use] pub fn double(&self) -> Self { let Self { x, y, z } = *self; - if z == C::BaseField::ZERO { - return ProjectivePoint::ZERO; - } + + debug_assert!(z.is_nonzero()); let b = (x + y).square(); let c = x.square(); let d = y.square(); - let e = c.neg(); + let e = c * C::A; let f = e + d; let h = z.square(); let j = f - C::BaseField::TWO * h; @@ -252,9 +213,9 @@ impl PartialEq for ProjectivePoint { y: y2, z: z2, } = *other; - if z1 == C::BaseField::ZERO || z2 == C::BaseField::ZERO { - return z1 == z2; - } + + debug_assert!(z1.is_nonzero()); + debug_assert!(z2.is_nonzero()); // We want to compare (x1/z1, y1/z1) == (x2/z2, y2/z2). // But to avoid field division, it is better to compare (x1*z2, y1*z2) == (x2*z1, y2*z1). @@ -264,15 +225,6 @@ impl PartialEq for ProjectivePoint { impl Eq for ProjectivePoint {} -impl Neg for AffinePoint { - type Output = AffinePoint; - - fn neg(self) -> Self::Output { - let AffinePoint { x, y, zero } = self; - AffinePoint { x: -x, y, zero } - } -} - impl Neg for ProjectivePoint { type Output = ProjectivePoint; diff --git a/circuits/plonky2_ed25519/src/curve/ed25519.rs b/circuits/plonky2_ed25519/src/curve/ed25519.rs index 7ed4ffbe..d6ed8d44 100644 --- a/circuits/plonky2_ed25519/src/curve/ed25519.rs +++ b/circuits/plonky2_ed25519/src/curve/ed25519.rs @@ -20,7 +20,7 @@ impl Curve for Ed25519 { type BaseField = Ed25519Base; type ScalarField = Ed25519Scalar; - const A: Ed25519Base = Ed25519Base::ONE; + const A: Ed25519Base = Ed25519Base::NEG_ONE; const D: Ed25519Base = Ed25519Base([ 0x75eb4dca135978a3, 0x00700a4d4141d8ab, @@ -30,7 +30,6 @@ impl Curve for Ed25519 { const GENERATOR_AFFINE: AffinePoint = AffinePoint { x: ED25519_GENERATOR_X, y: ED25519_GENERATOR_Y, - zero: false, }; } @@ -73,10 +72,12 @@ pub(crate) fn mul_naive( #[cfg(test)] mod tests { + use std::ops::Neg; + use num::BigUint; use plonky2_field::types::Field; - use crate::curve::curve_types::{AffinePoint, Curve}; + use crate::curve::curve_types::Curve; use crate::curve::ed25519::mul_naive; use crate::curve::ed25519::Ed25519; use crate::field::ed25519_scalar::Ed25519Scalar; @@ -87,11 +88,8 @@ mod tests { assert!(g.is_valid()); assert!(g.to_projective().is_valid()); - let neg_g = AffinePoint:: { - x: -g.x, - y: g.y, - zero: g.zero, - }; + let neg_g = g.neg(); + assert!(neg_g.is_valid()); assert!(neg_g.to_projective().is_valid()); } @@ -115,4 +113,9 @@ mod tests { mul_naive(lhs, Ed25519::GENERATOR_PROJECTIVE) ); } + + #[test] + fn test_curve_valid() { + Ed25519::assert_curve_valid(); + } } diff --git a/circuits/plonky2_ed25519/src/field/ed25519_base.rs b/circuits/plonky2_ed25519/src/field/ed25519_base.rs index aee2af15..e576f1e8 100644 --- a/circuits/plonky2_ed25519/src/field/ed25519_base.rs +++ b/circuits/plonky2_ed25519/src/field/ed25519_base.rs @@ -86,14 +86,21 @@ impl Field for Ed25519Base { 0x7FFFFFFFFFFFFFFF, ]); - const TWO_ADICITY: usize = 1; + const TWO_ADICITY: usize = 2; const CHARACTERISTIC_TWO_ADICITY: usize = Self::TWO_ADICITY; // Sage: `g = GF(p).multiplicative_generator()` const MULTIPLICATIVE_GROUP_GENERATOR: Self = Self([2, 0, 0, 0]); - // Sage: `g_2 = g^((p - 1) / 2)` - const POWER_OF_TWO_GENERATOR: Self = Self::NEG_ONE; + // Sage: `g_2 = g^((p - 1) / 4)` + // 19681161376707505956807079304988542015446066515923890162744021073123829784752 + // 0x2B8324804FC1DF0B_2B4D00993DFBD7A7_2F431806AD2FE478_C4EE1B274A0EA0B0 + const POWER_OF_TWO_GENERATOR: Self = Self([ + 0xC4EE1B274A0EA0B0, + 0x2F431806AD2FE478, + 0x2B4D00993DFBD7A7, + 0x2B8324804FC1DF0B, + ]); const BITS: usize = 256; @@ -117,6 +124,8 @@ impl Field for Ed25519Base { } fn from_noncanonical_biguint(val: BigUint) -> Self { + let val = val % Self::order(); + Self( val.to_u64_digits() .into_iter() diff --git a/circuits/plonky2_ed25519/src/field/ed25519_scalar.rs b/circuits/plonky2_ed25519/src/field/ed25519_scalar.rs index efac2c04..3d0f2a1c 100644 --- a/circuits/plonky2_ed25519/src/field/ed25519_scalar.rs +++ b/circuits/plonky2_ed25519/src/field/ed25519_scalar.rs @@ -78,20 +78,27 @@ impl Field for Ed25519Scalar { const ONE: Self = Self([1, 0, 0, 0]); const TWO: Self = Self([2, 0, 0, 0]); const NEG_ONE: Self = Self([ - 0x5812631a5cf5d3ee, + 0x5812631a5cf5d3ec, 0x14def9dea2f79cd6, 0x0000000000000000, 0x1000000000000000, ]); - const TWO_ADICITY: usize = 1; + const TWO_ADICITY: usize = 2; const CHARACTERISTIC_TWO_ADICITY: usize = Self::TWO_ADICITY; // Sage: `g = GF(p).multiplicative_generator()` const MULTIPLICATIVE_GROUP_GENERATOR: Self = Self([2, 0, 0, 0]); - // Sage: `g_2 = g^((p - 1) / 2)` - const POWER_OF_TWO_GENERATOR: Self = Self::NEG_ONE; + // Sage: `g_2 = g^((p - 1) / 4)` + // 4202356475871964119699734399548423449193549369991576068503119564443318355924 + // 0x094A7310E07981E7_7D3D6D60ABC1C27A_0EF0565342CE83FE_BE8775DFEBBE07D4 + const POWER_OF_TWO_GENERATOR: Self = Self([ + 0xBE8775DFEBBE07D4, + 0x0EF0565342CE83FE, + 0x7D3D6D60ABC1C27A, + 0x094A7310E07981E7, + ]); const BITS: usize = 256; @@ -115,6 +122,8 @@ impl Field for Ed25519Scalar { } fn from_noncanonical_biguint(val: BigUint) -> Self { + let val = val % Self::order(); + Self( val.to_u64_digits() .into_iter() diff --git a/circuits/plonky2_ed25519/src/gadgets/curve.rs b/circuits/plonky2_ed25519/src/gadgets/curve.rs index 1783640b..c034badf 100644 --- a/circuits/plonky2_ed25519/src/gadgets/curve.rs +++ b/circuits/plonky2_ed25519/src/gadgets/curve.rs @@ -110,19 +110,22 @@ impl, const D: usize> CircuitBuilderCurve AffinePointTarget { x, y } } - // y^2 = a + x^2 + b*x^2*y^2 + // A * x^2 + y^2 = 1 + D*x^2*y^2 fn curve_assert_valid(&mut self, p: &AffinePointTarget) { let a = self.constant_nonnative(C::A); let d = self.constant_nonnative(C::D); + let one = self.constant_nonnative(C::BaseField::ONE); let y_squared = self.mul_nonnative(&p.y, &p.y); let x_squared = self.mul_nonnative(&p.x, &p.x); let x_squared_y_squared = self.mul_nonnative(&x_squared, &y_squared); let d_x_squared_y_squared = self.mul_nonnative(&d, &x_squared_y_squared); - let a_plus_x_squared = self.add_nonnative(&a, &x_squared); - let rhs = self.add_nonnative(&a_plus_x_squared, &d_x_squared_y_squared); + let a_x_squared = self.mul_nonnative(&a, &x_squared); + + let lhs = self.add_nonnative(&a_x_squared, &y_squared); + let rhs = self.add_nonnative(&one, &d_x_squared_y_squared); - self.connect_nonnative(&y_squared, &rhs); + self.connect_nonnative(&lhs, &rhs); } fn curve_neg(&mut self, p: &AffinePointTarget) -> AffinePointTarget { @@ -148,14 +151,17 @@ impl, const D: usize> CircuitBuilderCurve fn curve_double(&mut self, p: &AffinePointTarget) -> AffinePointTarget { let AffinePointTarget { x, y } = p; let one = self.constant_nonnative(C::BaseField::ONE); + let a = self.constant_nonnative(C::A); let d = self.constant_nonnative(C::D); let xx = self.mul_nonnative(x, x); let yy = self.mul_nonnative(y, y); let xy = self.mul_nonnative(x, y); - let xy_plus_xy = self.add_nonnative(&xy, &xy); - let xx_plus_yy = self.add_nonnative(&xx, &yy); + let a_xx = self.mul_nonnative(&a, &xx); + + let x_nom = self.add_nonnative(&xy, &xy); + let y_nom = self.sub_nonnative(&yy, &a_xx); let xxyy = self.mul_nonnative(&xx, &yy); let dxxyy = self.mul_nonnative(&d, &xxyy); @@ -165,8 +171,8 @@ impl, const D: usize> CircuitBuilderCurve let inv_one_plus_dxxyy = self.inv_nonnative(&one_plus_dxxyy); let inv_one_minus_dxxyy = self.inv_nonnative(&one_minus_dxxyy); - let x3 = self.mul_nonnative(&xy_plus_xy, &inv_one_plus_dxxyy); - let y3 = self.mul_nonnative(&xx_plus_yy, &inv_one_minus_dxxyy); + let x3 = self.mul_nonnative(&x_nom, &inv_one_plus_dxxyy); + let y3 = self.mul_nonnative(&y_nom, &inv_one_minus_dxxyy); AffinePointTarget { x: x3, y: y3 } } @@ -194,6 +200,7 @@ impl, const D: usize> CircuitBuilderCurve let AffinePointTarget { x: x1, y: y1 } = p1; let AffinePointTarget { x: x2, y: y2 } = p2; let one = self.constant_nonnative(C::BaseField::ONE); + let a = self.constant_nonnative(C::A); let d = self.constant_nonnative(C::D); let x1y2 = self.mul_nonnative(x1, y2); @@ -201,19 +208,20 @@ impl, const D: usize> CircuitBuilderCurve let y1y2 = self.mul_nonnative(y1, y2); let x1x2 = self.mul_nonnative(x1, x2); - let x1y2_add_y1x2 = self.add_nonnative(&x1y2, &y1x2); - let y1y2_add_x1x2 = self.add_nonnative(&y1y2, &x1x2); + let ax1x2 = self.mul_nonnative(&a, &x1x2); + + let x_nom = self.add_nonnative(&x1y2, &y1x2); + let y_nom = self.sub_nonnative(&y1y2, &ax1x2); let x1x2y1y2 = self.mul_nonnative(&x1y2, &y1x2); let dx1x2y1y2 = self.mul_nonnative(&d, &x1x2y1y2); - let neg_dx1x2y1y2 = self.neg_nonnative(&dx1x2y1y2); let one_add_dx1x2y1y2 = self.add_nonnative(&one, &dx1x2y1y2); - let one_neg_dx1x2y1y2 = self.add_nonnative(&one, &neg_dx1x2y1y2); + let one_sub_dx1x2y1y2 = self.sub_nonnative(&one, &dx1x2y1y2); let inv_one_add_dx1x2y1y2 = self.inv_nonnative(&one_add_dx1x2y1y2); - let inv_one_neg_dx1x2y1y2 = self.inv_nonnative(&one_neg_dx1x2y1y2); + let inv_one_sub_dx1x2y1y2 = self.inv_nonnative(&one_sub_dx1x2y1y2); - let x3 = self.mul_nonnative(&x1y2_add_y1x2, &inv_one_add_dx1x2y1y2); - let y3 = self.mul_nonnative(&y1y2_add_x1x2, &inv_one_neg_dx1x2y1y2); + let x3 = self.mul_nonnative(&x_nom, &inv_one_add_dx1x2y1y2); + let y3 = self.mul_nonnative(&y_nom, &inv_one_sub_dx1x2y1y2); AffinePointTarget { x: x3, y: y3 } } @@ -432,11 +440,7 @@ mod tests { let mut builder = CircuitBuilder::::new(config); let g = Ed25519::GENERATOR_AFFINE; - let not_g = AffinePoint:: { - x: g.x, - y: g.y + Ed25519Base::ONE, - zero: g.zero, - }; + let not_g = AffinePoint::::nonzero(g.x, g.y + Ed25519Base::ONE); let not_g_target = builder.constant_affine_point(not_g); builder.curve_assert_valid(¬_g_target); @@ -547,7 +551,6 @@ mod tests { } #[test] - #[ignore] fn test_curve_mul() -> Result<()> { const D: usize = 2; type C = PoseidonGoldilocksConfig; @@ -580,7 +583,6 @@ mod tests { } #[test] - #[ignore] fn test_curve_random() -> Result<()> { const D: usize = 2; type C = PoseidonGoldilocksConfig; diff --git a/circuits/plonky2_ed25519/src/gadgets/curve_fixed_base.rs b/circuits/plonky2_ed25519/src/gadgets/curve_fixed_base.rs index e442ff14..c36a425b 100644 --- a/circuits/plonky2_ed25519/src/gadgets/curve_fixed_base.rs +++ b/circuits/plonky2_ed25519/src/gadgets/curve_fixed_base.rs @@ -82,7 +82,6 @@ mod tests { use crate::gadgets::nonnative::CircuitBuilderNonNative; #[test] - #[ignore] fn test_fixed_base() -> Result<()> { const D: usize = 2; type C = PoseidonGoldilocksConfig; diff --git a/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul.rs b/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul.rs index 8e155874..652c6eec 100644 --- a/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul.rs +++ b/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul.rs @@ -130,7 +130,6 @@ impl, const D: usize> CircuitBuilderWindowedMul Result<()> { - // Initialize logging - let mut builder = env_logger::Builder::from_default_env(); - builder.format_timestamp(None); - builder.filter_level(LevelFilter::Info); - builder.try_init()?; - const D: usize = 2; type C = PoseidonGoldilocksConfig; type F = >::F; diff --git a/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul_mt.rs b/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul_mt.rs index 28592062..b6a02d2a 100644 --- a/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul_mt.rs +++ b/circuits/plonky2_ed25519/src/gadgets/curve_windowed_mul_mt.rs @@ -290,7 +290,6 @@ where let proof0_q_init = AffinePoint { x: CV::BaseField::ZERO, y: CV::BaseField::ONE, - zero: false, }; let proof0_q_init = builder.constant_affine_point(proof0_q_init); builder.connect_affine_point(&proof0_q_init, &proof0_targets.q_init_target); @@ -330,7 +329,6 @@ where let q0_init = AffinePoint { x: Ed25519Base::ZERO, y: Ed25519Base::ONE, - zero: false, }; let q1_init = (CurveScalar::(n0) * p.to_projective()).to_affine(); let q_expected = (CurveScalar::(*n) * p.to_projective()).to_affine(); @@ -375,7 +373,6 @@ mod tests { use std::ops::Neg; use anyhow::Result; - use log::LevelFilter; use plonky2::plonk::circuit_data::CircuitConfig; use plonky2::plonk::config::{GenericConfig, PoseidonGoldilocksConfig}; use plonky2_field::types::{Field, Sample}; @@ -386,14 +383,7 @@ mod tests { use crate::gadgets::curve_windowed_mul_mt::prove_curve25519_mul_mt; #[test] - // #[ignore] fn test_prove_curve25519_mul_mt() -> Result<()> { - // Initialize logging - let mut builder = env_logger::Builder::from_default_env(); - builder.format_timestamp(None); - builder.filter_level(LevelFilter::Info); - builder.try_init()?; - const D: usize = 2; type C = PoseidonGoldilocksConfig; type F = >::F; diff --git a/circuits/plonky2_ed25519/src/gadgets/eddsa.rs b/circuits/plonky2_ed25519/src/gadgets/eddsa.rs index 262408fe..d844478b 100644 --- a/circuits/plonky2_ed25519/src/gadgets/eddsa.rs +++ b/circuits/plonky2_ed25519/src/gadgets/eddsa.rs @@ -65,7 +65,7 @@ pub fn make_verify_circuits, const D: usize>( let s_bits = bits_in_le(sig[256..512].to_vec()); let s_biguint = bits_to_biguint_target(builder, s_bits); - let s = builder.biguint_to_nonnative(&s_biguint); + let s = builder.reduce(&s_biguint); let pk_bits = bits_in_le(pk.clone()); let a = builder.point_decompress(&pk_bits); @@ -180,19 +180,17 @@ mod tests { } #[test] - #[ignore] fn test_eddsa_circuit_narrow() -> Result<()> { + // NOTE: Due to the range check added to some underlying circuits this test is failing. test_eddsa_circuit_with_config(CircuitConfig::standard_ecc_config()) } #[test] - #[ignore] fn test_eddsa_circuit_wide() -> Result<()> { test_eddsa_circuit_with_config(CircuitConfig::wide_ecc_config()) } #[test] - #[ignore] #[should_panic] fn test_eddsa_circuit_failure() { test_eddsa_circuit_with_config_failure(CircuitConfig::wide_ecc_config()); diff --git a/circuits/plonky2_ed25519/src/gadgets/nonnative.rs b/circuits/plonky2_ed25519/src/gadgets/nonnative.rs index c03202e5..163e94f5 100644 --- a/circuits/plonky2_ed25519/src/gadgets/nonnative.rs +++ b/circuits/plonky2_ed25519/src/gadgets/nonnative.rs @@ -193,6 +193,9 @@ impl, const D: usize> CircuitBuilderNonNative _phantom: PhantomData, }); + range_check_u32_circuit(self, sum.value.limbs.clone()); + self.assert_bool(overflow); + let sum_expected = self.add_biguint(&a.value, &b.value); let modulus = self.constant_biguint(&FF::order()); @@ -369,6 +372,9 @@ impl, const D: usize> CircuitBuilderNonNative _phantom: PhantomData, }); + range_check_u32_circuit(self, inv_biguint.limbs.clone()); + range_check_u32_circuit(self, div.limbs.clone()); + let product = self.mul_biguint(&x.value, &inv_biguint); let modulus = self.constant_biguint(&FF::order()); diff --git a/prover/src/block_finality/mod.rs b/prover/src/block_finality/mod.rs index 7fe0fb26..c33e9fce 100644 --- a/prover/src/block_finality/mod.rs +++ b/prover/src/block_finality/mod.rs @@ -94,11 +94,7 @@ impl BlockFinality { log::debug!("Proving block finality..."); // Find such a number that processed_validator_count > 2/3 * validator_count. - let processed_validator_count = match self.validator_set.len() % 3 { - 0 | 1 => 2 * (self.validator_set.len() / 3) + 1, - 2 => 2 * (self.validator_set.len() - 2) / 3 + 2, - _ => unreachable!(), - }; + let processed_validator_count = (2 * self.validator_set.len()) / 3 + 1; let processed_pre_commits: Vec<_> = self .pre_commits diff --git a/prover/src/common/mod.rs b/prover/src/common/mod.rs index 2925bece..66e5475b 100644 --- a/prover/src/common/mod.rs +++ b/prover/src/common/mod.rs @@ -309,12 +309,11 @@ pub fn array_to_bits(data: &[u8]) -> Vec { /// Perform conversion from byte to bits, placing most significant bit first. pub fn byte_to_bits(byte: u8) -> [bool; 8] { - (0..8) - .rev() - .map(move |bit_idx| (byte >> bit_idx) % 2 == 1) - .collect::>() - .try_into() - .expect("8 bits in byte") + let mut bits = [false; 8]; + for bit_idx in 0..8 { + bits[7 - bit_idx] = (byte >> bit_idx) % 2 == 1; + } + bits } /// Pad `Vec` with zeroes to fit in desired length. @@ -324,3 +323,16 @@ pub fn pad_byte_vec(mut data: Vec) -> [u8; L] { data.append(&mut vec![0; L - data.len()]); data.try_into().expect("Correct length of Vec") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_byte_to_bits() { + let byte = 0b11001100; + let bits = [true, true, false, false, true, true, false, false]; + + assert_eq!(byte_to_bits(byte), bits); + } +} diff --git a/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/inlined_data_parser.rs b/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/inlined_data_parser.rs index cb73b991..2260b014 100644 --- a/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/inlined_data_parser.rs +++ b/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/inlined_data_parser.rs @@ -19,7 +19,7 @@ use crate::{ const INLINED_DATA_LENGTH: usize = 32; impl_target_set! { - pub struct InlindedDataParserInputTarget { + pub struct InlinedDataParserInputTarget { // TODO: replace to `LeafNodeData` /// Node encoded data. pub first_node_data_block: NodeDataBlockTarget, @@ -38,7 +38,7 @@ impl_target_set! { } pub fn define( - input: InlindedDataParserInputTarget, + input: InlinedDataParserInputTarget, builder: &mut CircuitBuilder, ) -> InlinedDataParserOutputTarget { log::debug!(" Composing inlined data parser"); diff --git a/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/mod.rs b/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/mod.rs index 6d776789..d6a4e3d9 100644 --- a/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/mod.rs +++ b/prover/src/storage_inclusion/storage_trie_proof/node_parser/leaf_parser/mod.rs @@ -27,7 +27,7 @@ use crate::{ header_parser::HeaderDescriptor, leaf_parser::{ hashed_data_parser::HashedDataParserInputTarget, - inlined_data_parser::InlindedDataParserInputTarget, + inlined_data_parser::InlinedDataParserInputTarget, }, }, storage_address::PartialStorageAddressTarget, @@ -131,7 +131,7 @@ impl LeafParser { } LeafType::Leaf => { let parsed_data = { - let input = InlindedDataParserInputTarget { + let input = InlinedDataParserInputTarget { first_node_data_block: node_data_target.clone(), read_offset: parsed_nibbles.resulting_offset, }; diff --git a/prover/src/storage_inclusion/storage_trie_proof/node_parser/nibble_parser.rs b/prover/src/storage_inclusion/storage_trie_proof/node_parser/nibble_parser.rs index 4725c5fe..afc7cc86 100644 --- a/prover/src/storage_inclusion/storage_trie_proof/node_parser/nibble_parser.rs +++ b/prover/src/storage_inclusion/storage_trie_proof/node_parser/nibble_parser.rs @@ -1,6 +1,7 @@ //! ### Circuit that's used to parse nibble array from encoded node. use plonky2::{iop::target::Target, plonk::circuit_builder::CircuitBuilder}; +use plonky2_field::types::Field; use std::iter; use super::{NodeDataBlockTarget, PartialStorageAddressTarget}; @@ -19,7 +20,8 @@ impl_target_set! { pub first_node_data_block: NodeDataBlockTarget, /// Read nibbles starting from this index. pub read_offset: Target, - /// Nibble count to read. + /// Nibble count to read. The circuit asserts that + /// `nibble_count <= storage_address::MAX_STORAGE_ADDRESS_LENGTH_IN_NIBBLES`. pub nibble_count: Target, /// Previously composed address, to which we should append read nibbles. pub partial_address: PartialStorageAddressTarget @@ -39,6 +41,12 @@ pub fn define( input: NibbleParserInputTarget, builder: &mut CircuitBuilder, ) -> NibbleParserOutputTarget { + let max_nibble_count = builder.constant(F::from_canonical_usize( + MAX_STORAGE_ADDRESS_LENGTH_IN_NIBBLES, + )); + let max_nibble_count_sub_input = builder.sub(max_nibble_count, input.nibble_count); + builder.range_check(max_nibble_count_sub_input, 32); + let potential_address_bytes: ArrayTarget<_, MAX_STORAGE_ADDRESS_LENGTH_IN_BYTES> = input .first_node_data_block .random_read_array(input.read_offset, builder); @@ -62,13 +70,20 @@ pub fn define( let nibble_count_odd = builder.low_bits(input.nibble_count, 1, 32)[0]; + // If nibble count is odd, the first nibble serves as padding and must be set to 0. + // So the case when nibble count is odd and first nibble != 0 is invalid. + let zero = builder.zero(); + let first_nibble_is_zero = builder.is_equal(first_nibble.to_target(), zero); + let first_nibble_is_nonzero = builder.not(first_nibble_is_zero); + let invalid_padding = builder.and(nibble_count_odd, first_nibble_is_nonzero); + builder.assert_zero(invalid_padding.target); + // If nibble count is odd: // we take `input.nibble_count` nibbles from `remaining_nibbles` // If nibble count is 0: // we take `input.nibble_count` nibbles from `remaining_nibbles` // If nibble count is even: // we take `first_nibble` and input.nibble_count - 1` nibbles from `remaining_nibbles` - let zero = builder.zero(); let nibble_count_is_zero = builder.is_equal(input.nibble_count, zero); let dont_take_first_nibble = builder.or(nibble_count_odd, nibble_count_is_zero); let take_first_nibble = builder.not(dont_take_first_nibble); @@ -120,7 +135,6 @@ mod tests { proof::ProofWithPublicInputs, }, }; - use plonky2_field::types::Field; use super::*; use crate::{ @@ -161,6 +175,26 @@ mod tests { ); } + #[test] + #[should_panic( + expected = "Partition containing Wire(Wire { row: 105, column: 11 }) was set twice with different values: 0 != 1" + )] + fn test_nibble_parser_fails_on_invalid_padding() { + test_case(pad_byte_vec(vec![0x10, 0x00, 0x00]), 5, None); + } + + #[test] + #[should_panic( + expected = "Partition containing Wire(Wire { row: 1, column: 33 }) was set twice with different values: 0 != 1" + )] + fn test_nibble_parser_upper_nibble_count_limit() { + test_case( + [0; NODE_DATA_BLOCK_BYTES], + MAX_STORAGE_ADDRESS_LENGTH_IN_NIBBLES + 1, + None, + ); + } + #[test] fn test_nibble_parser_have_constant_verifier_data() { let (_, first_cd) = build_test_case_circuit(