From 42a652cac8d65c083edba53d922ee866fb4c8862 Mon Sep 17 00:00:00 2001 From: fmoletta <99273364+fmoletta@users.noreply.github.com> Date: Fri, 5 Jan 2024 20:39:43 +0200 Subject: [PATCH] Use `ProjectivePoint` from types-rs in ec_op builtin impl (#1532) * First draft * Fixes * Fix example * Fixes * Update version * Update version * Add changelog * Update types-rs version * Update lock * Remove FIXME * Remove allows * Apply suggestions * Update vm/src/vm/runners/builtin_runner/ec_op.rs Co-authored-by: Mario Rugiero --------- Co-authored-by: Mario Rugiero --- CHANGELOG.md | 2 + Cargo.lock | 11 ++- fuzzer/Cargo.lock | 31 +++++++- vm/Cargo.toml | 2 +- vm/src/math_utils/mod.rs | 11 +-- vm/src/vm/errors/runner_errors.rs | 3 +- vm/src/vm/runners/builtin_runner/ec_op.rs | 96 ++++++++++------------- 7 files changed, 85 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40f85f5d5f..485aeb8cb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ #### Upcoming Changes +* feat: Use `ProjectivePoint` from types-rs in ec_op builtin impl [#1532](https://github.com/lambdaclass/cairo-vm/pull/1532) + * feat(BREAKING): Replace `cairo-felt` crate with `starknet-types-core` (0.0.5) [#1408](https://github.com/lambdaclass/cairo-vm/pull/1408) * feat(BREAKING): Add Cairo 1 proof mode compilation and execution [#1517] (https://github.com/lambdaclass/cairo-vm/pull/1517) diff --git a/Cargo.lock b/Cargo.lock index 51bfb9f3dc..2b05a8af1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1505,9 +1505,12 @@ dependencies = [ [[package]] name = "lambdaworks-math" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "540c0715d7da472edc421ceca702d3f3a716b3b9168b6211f2e3939cb14c863c" +checksum = "3c6c4d0ddd1fcd235be5196b1bcc404f89ad3e911f4c190fa01459e05dbf40f8" +dependencies = [ + "thiserror", +] [[package]] name = "lazy_static" @@ -2450,9 +2453,9 @@ dependencies = [ [[package]] name = "starknet-types-core" -version = "0.0.5" +version = "0.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2441eb61d91a6bae832fda48e50b6ff09faa6cf63bfadd22dc16798d2cc024d" +checksum = "5b6b868f545d43b474c2c00e9349c489fdeb7ff17eb00cdf339744ac4cae0930" dependencies = [ "arbitrary", "bitvec", diff --git a/fuzzer/Cargo.lock b/fuzzer/Cargo.lock index 321c4ad2e1..5d9996922c 100644 --- a/fuzzer/Cargo.lock +++ b/fuzzer/Cargo.lock @@ -412,9 +412,12 @@ dependencies = [ [[package]] name = "lambdaworks-math" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "540c0715d7da472edc421ceca702d3f3a716b3b9168b6211f2e3939cb14c863c" +checksum = "3c6c4d0ddd1fcd235be5196b1bcc404f89ad3e911f4c190fa01459e05dbf40f8" +dependencies = [ + "thiserror", +] [[package]] name = "lazy_static" @@ -904,9 +907,9 @@ dependencies = [ [[package]] name = "starknet-types-core" -version = "0.0.4" +version = "0.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78b4af540664e3dec02f36a47b79ba4df825a7752bf0941954f69753a98a397e" +checksum = "5b6b868f545d43b474c2c00e9349c489fdeb7ff17eb00cdf339744ac4cae0930" dependencies = [ "arbitrary", "bitvec", @@ -958,6 +961,26 @@ version = "0.12.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c39fd04924ca3a864207c66fc2cd7d22d7c016007f9ce846cbb9326331930a" +[[package]] +name = "thiserror" +version = "1.0.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e3de26b0965292219b4287ff031fcba86837900fe9cd2b34ea8ad893c0953d2" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "268026685b2be38d7103e9e507c938a1fcb3d7e6eb15e87870b617bf37b6d581" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.39", +] + [[package]] name = "thiserror-impl-no-std" version = "2.0.2" diff --git a/vm/Cargo.toml b/vm/Cargo.toml index 4ca5ddb687..e7f9fe029a 100644 --- a/vm/Cargo.toml +++ b/vm/Cargo.toml @@ -61,7 +61,7 @@ keccak = { workspace = true } hashbrown = { workspace = true } anyhow = { workspace = true } thiserror-no-std = { workspace = true } -starknet-types-core = { version = "0.0.5", default-features = false, features = ["serde"] } +starknet-types-core = { version = "0.0.6", default-features = false, features = ["serde", "curve", "num-traits"] } # only for std num-prime = { version = "0.4.3", features = ["big-int"], optional = true } diff --git a/vm/src/math_utils/mod.rs b/vm/src/math_utils/mod.rs index 9539a97e13..8381869321 100644 --- a/vm/src/math_utils/mod.rs +++ b/vm/src/math_utils/mod.rs @@ -48,12 +48,13 @@ pub fn pow2_const_nz(n: u32) -> &'static NonZeroFelt { /// # Examples /// /// ``` -/// # use crate::Felt252; -/// let positive = Felt252::new(5); -/// assert_eq!(signed_felt(positive), Felt252::from(5)); +/// # use cairo_vm::{Felt252, math_utils::signed_felt}; +/// # use num_bigint::BigInt; +/// let positive = Felt252::from(5); +/// assert_eq!(signed_felt(positive), BigInt::from(5)); /// -/// let negative = Felt252::max_value(); -/// assert_eq!(signed_felt(negative), Felt252::from(-1)); +/// let negative = Felt252::MAX; +/// assert_eq!(signed_felt(negative), BigInt::from(-1)); /// ``` pub fn signed_felt(felt: Felt252) -> BigInt { diff --git a/vm/src/vm/errors/runner_errors.rs b/vm/src/vm/errors/runner_errors.rs index 05e371e34a..dc1533f509 100644 --- a/vm/src/vm/errors/runner_errors.rs +++ b/vm/src/vm/errors/runner_errors.rs @@ -2,7 +2,6 @@ #![allow(clippy::explicit_auto_deref)] use crate::stdlib::{collections::HashSet, prelude::*}; - use thiserror_no_std::Error; use super::{memory_errors::MemoryError, trace_errors::TraceError}; @@ -101,6 +100,8 @@ pub enum RunnerError { StrippedProgramNoMain, #[error(transparent)] Trace(#[from] TraceError), + #[error("EcOp builtin: Invalid Point")] + InvalidPoint, } #[cfg(test)] diff --git a/vm/src/vm/runners/builtin_runner/ec_op.rs b/vm/src/vm/runners/builtin_runner/ec_op.rs index c62e8dffd8..938cd390fb 100644 --- a/vm/src/vm/runners/builtin_runner/ec_op.rs +++ b/vm/src/vm/runners/builtin_runner/ec_op.rs @@ -1,19 +1,16 @@ -use crate::math_utils::{ec_add, ec_double}; use crate::stdlib::{borrow::Cow, prelude::*}; use crate::stdlib::{cell::RefCell, collections::HashMap}; use crate::types::instance_definitions::ec_op_instance_def::{ EcOpInstanceDef, CELLS_PER_EC_OP, INPUT_CELLS_PER_EC_OP, }; use crate::types::relocatable::{MaybeRelocatable, Relocatable}; -use crate::utils::{bigint_to_felt, felt_to_bigint, CAIRO_PRIME}; use crate::vm::errors::memory_errors::MemoryError; use crate::vm::errors::runner_errors::RunnerError; use crate::vm::vm_memory::memory::Memory; use crate::vm::vm_memory::memory_segments::MemorySegmentManager; use crate::Felt252; -use num_bigint::{BigInt, ToBigInt}; use num_integer::{div_ceil, Integer}; -use num_traits::{One, Zero}; +use starknet_types_core::curve::ProjectivePoint; use super::EC_OP_BUILTIN_NAME; @@ -51,7 +48,6 @@ impl EcOpBuiltinRunner { y.pow(2_u32) == (x.pow(3_u32) + alpha * x) + beta } - #[allow(deprecated)] ///Returns the result of the EC operation P + m * Q. /// where P = (p_x, p_y), Q = (q_x, q_y) are points on the elliptic curve defined as /// y^2 = x^3 + alpha * x + beta (mod prime). @@ -62,31 +58,29 @@ impl EcOpBuiltinRunner { partial_sum: (Felt252, Felt252), doubled_point: (Felt252, Felt252), m: &Felt252, - alpha: &BigInt, - prime: &BigInt, height: u32, - ) -> Result<(BigInt, BigInt), RunnerError> { - let mut slope = felt_to_bigint(*m); - let mut partial_sum_b = (felt_to_bigint(partial_sum.0), felt_to_bigint(partial_sum.1)); - let mut doubled_point_b = ( - felt_to_bigint(doubled_point.0), - felt_to_bigint(doubled_point.1), - ); - for _ in 0..height { - if (doubled_point_b.0.clone() - partial_sum_b.0.clone()).is_zero() { - #[allow(deprecated)] + ) -> Result<(Felt252, Felt252), RunnerError> { + let slope = m.to_biguint(); + let mut partial_sum_b = ProjectivePoint::from_affine(partial_sum.0, partial_sum.1) + .map_err(|_| RunnerError::PointNotOnCurve(Box::new(partial_sum)))?; + let mut doubled_point_b = ProjectivePoint::from_affine(doubled_point.0, doubled_point.1) + .map_err(|_| RunnerError::PointNotOnCurve(Box::new(doubled_point)))?; + for i in 0..(height as u64).min(slope.bits()) { + if partial_sum_b.x() * doubled_point_b.z() == partial_sum_b.z() * doubled_point_b.x() { return Err(RunnerError::EcOpSameXCoordinate( Self::format_ec_op_error(partial_sum_b, slope, doubled_point_b) .into_boxed_str(), )); }; - if !(slope.clone() & &BigInt::one()).is_zero() { - partial_sum_b = ec_add(partial_sum_b, doubled_point_b.clone(), prime)?; + if slope.bit(i) { + partial_sum_b += &doubled_point_b; } - doubled_point_b = ec_double(doubled_point_b, alpha, prime)?; - slope = slope.clone() >> 1_u32; + doubled_point_b = doubled_point_b.double(); } - Ok(partial_sum_b) + partial_sum_b + .to_affine() + .map(|p| (p.x(), p.y())) + .map_err(|_| RunnerError::InvalidPoint) } pub fn initialize_segments(&mut self, segments: &mut MemorySegmentManager) { @@ -179,17 +173,12 @@ impl EcOpBuiltinRunner { )))); }; } - let prime = CAIRO_PRIME.to_bigint().unwrap(); let result = EcOpBuiltinRunner::ec_op_impl( (input_cells[0].to_owned(), input_cells[1].to_owned()), (input_cells[2].to_owned(), input_cells[3].to_owned()), input_cells[4], - #[allow(deprecated)] - &felt_to_bigint(alpha), - &prime, self.ec_op_builtin.scalar_height, )?; - let result = (bigint_to_felt(&result.0)?, bigint_to_felt(&result.1)?); self.cache.borrow_mut().insert(x_addr, result.0); self.cache.borrow_mut().insert( (x_addr + 1usize) @@ -259,10 +248,12 @@ impl EcOpBuiltinRunner { } pub fn format_ec_op_error( - p: (num_bigint::BigInt, num_bigint::BigInt), - m: num_bigint::BigInt, - q: (num_bigint::BigInt, num_bigint::BigInt), + p: ProjectivePoint, + m: num_bigint::BigUint, + q: ProjectivePoint, ) -> String { + let p = p.to_affine().map(|p| (p.x(), p.y())).unwrap_or_default(); + let q = q.to_affine().map(|q| (q.x(), q.y())).unwrap_or_default(); format!("Cannot apply EC operation: computation reached two points with the same x coordinate. \n Attempting to compute P + m * Q where:\n P = {p:?} \n @@ -278,7 +269,7 @@ mod tests { use crate::serde::deserialize_program::BuiltinName; use crate::stdlib::collections::HashMap; use crate::types::program::Program; - use crate::utils::{test_utils::*, CAIRO_PRIME}; + use crate::utils::test_utils::*; use crate::vm::errors::cairo_run_errors::CairoRunError; use crate::vm::errors::vm_errors::VirtualMachineError; use crate::vm::runners::cairo_runner::CairoRunner; @@ -548,18 +539,15 @@ mod tests { felt_hex!("0x5668060aa49730b7be4801df46ec62de53ecd11abe43a32873000c36e8dc1f"), ); let m = Felt252::from(34); - let alpha = bigint!(1); let height = 256; - let prime = (*CAIRO_PRIME).clone().into(); - let result = - EcOpBuiltinRunner::ec_op_impl(partial_sum, doubled_point, &m, &alpha, &prime, height); + let result = EcOpBuiltinRunner::ec_op_impl(partial_sum, doubled_point, &m, height); assert_eq!( result, Ok(( - bigint_str!( + felt_str!( "1977874238339000383330315148209250828062304908491266318460063803060754089297" ), - bigint_str!( + felt_str!( "2969386888251099938335087541720168257053975603483053253007176033556822156706" ) )) @@ -578,18 +566,15 @@ mod tests { felt_hex!("0x5668060aa49730b7be4801df46ec62de53ecd11abe43a32873000c36e8dc1f"), ); let m = Felt252::from(34); - let alpha = bigint!(1); let height = 256; - let prime = (*CAIRO_PRIME).clone().into(); - let result = - EcOpBuiltinRunner::ec_op_impl(partial_sum, doubled_point, &m, &alpha, &prime, height); + let result = EcOpBuiltinRunner::ec_op_impl(partial_sum, doubled_point, &m, height); assert_eq!( result, Ok(( - bigint_str!( + felt_str!( "2778063437308421278851140253538604815869848682781135193774472480292420096757" ), - bigint_str!( + felt_str!( "3598390311618116577316045819420613574162151407434885460365915347732568210029" ) )) @@ -598,26 +583,25 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] - #[allow(deprecated)] fn compute_ec_op_invalid_same_x_coordinate() { - let partial_sum = (Felt252::ONE, Felt252::from(9)); - let doubled_point = (Felt252::ONE, Felt252::from(12)); + let partial_sum = ( + felt_hex!("0x6f0a1ddaf19c44781c8946db396f494a10ffab183c2d8cf6c4cd321a8d87fd9"), + felt_hex!("0x4afa52a9ef8c023d3385fddb6e1d78d57b0693b9b02d45d0f939b526d474c39"), + ); + let doubled_point = ( + felt_hex!("0x6f0a1ddaf19c44781c8946db396f494a10ffab183c2d8cf6c4cd321a8d87fd9"), + felt_hex!("0x4afa52a9ef8c023d3385fddb6e1d78d57b0693b9b02d45d0f939b526d474c39"), + ); let m = Felt252::from(34); - let alpha = bigint!(1); let height = 256; - let prime = (*CAIRO_PRIME).clone().into(); - let result = - EcOpBuiltinRunner::ec_op_impl(partial_sum, doubled_point, &m, &alpha, &prime, height); + let result = EcOpBuiltinRunner::ec_op_impl(partial_sum, doubled_point, &m, height); assert_eq!( result, Err(RunnerError::EcOpSameXCoordinate( EcOpBuiltinRunner::format_ec_op_error( - (felt_to_bigint(partial_sum.0), felt_to_bigint(partial_sum.1)), - felt_to_bigint(m), - ( - felt_to_bigint(doubled_point.0), - felt_to_bigint(doubled_point.1) - ) + ProjectivePoint::from_affine(partial_sum.0, partial_sum.1).unwrap(), + m.to_biguint(), + ProjectivePoint::from_affine(doubled_point.0, doubled_point.1).unwrap(), ) .into_boxed_str() ))