From 3f46e039022b7aeca151ad5b8211f4081fde481a Mon Sep 17 00:00:00 2001 From: fmoletta <99273364+fmoletta@users.noreply.github.com> Date: Tue, 30 May 2023 02:23:30 +0300 Subject: [PATCH] Fix hint `EC_DOUBLE_ASSIGN_NEW_X_V2` (#1186) * Fix hint `EC_DOUBLE_ASSIGN_NEW_X_V2` * Add changelog entry --------- Co-authored-by: Pedro Fontana --- CHANGELOG.md | 2 + .../builtin_hint_processor_definition.rs | 29 ++-- .../builtin_hint_processor/secp/ec_utils.rs | 136 ++++++++++++++---- 3 files changed, 125 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6014085a38..e6804e66af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ #### Upcoming Changes +* bugfix: Fix `EC_DOUBLE_ASSIGN_NEW_X_V2` hint not taking `SECP_P` value from the current execution scope [#1186](https://github.com/lambdaclass/cairo-rs/pull/1186) + * Fix possible subtraction overflow in `QUAD_BIT` & `DI_BIT` hints [#1185](https://github.com/lambdaclass/cairo-rs/pull/1185) * These hints now return an error when ids.m equals zero diff --git a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs index ef38cc3218..81d46d8bf1 100644 --- a/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs +++ b/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs @@ -20,7 +20,9 @@ use super::{ pack::*, }, }; -use crate::hint_processor::builtin_hint_processor::secp::ec_utils::ec_double_assign_new_x; +use crate::hint_processor::builtin_hint_processor::secp::ec_utils::{ + ec_double_assign_new_x, ec_double_assign_new_x_v2, +}; use crate::{ hint_processor::{ builtin_hint_processor::{ @@ -552,16 +554,21 @@ impl HintProcessor for BuiltinHintProcessor { "pt1", &SECP_P, ), - hint_code::EC_DOUBLE_ASSIGN_NEW_X_V1 | hint_code::EC_DOUBLE_ASSIGN_NEW_X_V2 => { - ec_double_assign_new_x( - vm, - exec_scopes, - &hint_data.ids_data, - &hint_data.ap_tracking, - &SECP_P, - "point", - ) - } + hint_code::EC_DOUBLE_ASSIGN_NEW_X_V1 => ec_double_assign_new_x( + vm, + exec_scopes, + &hint_data.ids_data, + &hint_data.ap_tracking, + &SECP_P, + "point", + ), + hint_code::EC_DOUBLE_ASSIGN_NEW_X_V2 => ec_double_assign_new_x_v2( + vm, + exec_scopes, + &hint_data.ids_data, + &hint_data.ap_tracking, + "point", + ), hint_code::EC_DOUBLE_ASSIGN_NEW_X_V3 => ec_double_assign_new_x( vm, exec_scopes, diff --git a/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs b/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs index 1c89beeb3d..4be9e37e9a 100644 --- a/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs +++ b/src/hint_processor/builtin_hint_processor/secp/ec_utils.rs @@ -266,6 +266,17 @@ pub fn square_slope_minus_xs( Ok(()) } +pub fn ec_double_assign_new_x_v2( + vm: &mut VirtualMachine, + exec_scopes: &mut ExecutionScopes, + ids_data: &HashMap, + ap_tracking: &ApTracking, + point_alias: &str, +) -> Result<(), HintError> { + let secp_p: BigInt = exec_scopes.get("SECP_P")?; + ec_double_assign_new_x(vm, exec_scopes, ids_data, ap_tracking, &secp_p, point_alias) +} + /* Implements hint: %{ @@ -912,36 +923,100 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn run_ec_double_assign_new_x_ok() { - let hint_codes = vec!["from starkware.cairo.common.cairo_secp.secp_utils import SECP_P, pack\n\nslope = pack(ids.slope, PRIME)\nx = pack(ids.point.x, PRIME)\ny = pack(ids.point.y, PRIME)\n\nvalue = new_x = (pow(slope, 2, SECP_P) - 2 * x) % SECP_P", "from starkware.cairo.common.cairo_secp.secp_utils import pack\n\nslope = pack(ids.slope, PRIME)\nx = pack(ids.point.x, PRIME)\ny = pack(ids.point.y, PRIME)\n\nvalue = new_x = (pow(slope, 2, SECP_P) - 2 * x) % SECP_P"]; - - for hint_code in hint_codes { - let mut vm = vm_with_range_check!(); - - //Insert ids.point and ids.slope into memory - vm.segments = segments![ - ((1, 0), 134), - ((1, 1), 5123), - ((1, 2), 140), - ((1, 3), 1232), - ((1, 4), 4652), - ((1, 5), 720), - ((1, 6), 44186171158942157784255469_i128), - ((1, 7), 54173758974262696047492534_i128), - ((1, 8), 8106299688661572814170174_i128) - ]; - - //Initialize fp - vm.run_context.fp = 10; - let ids_data = HashMap::from([ - ("point".to_string(), HintReference::new_simple(-10)), - ("slope".to_string(), HintReference::new_simple(-4)), - ]); - let mut exec_scopes = ExecutionScopes::new(); - - //Execute the hint - assert_matches!(run_hint!(vm, ids_data, hint_code, &mut exec_scopes), Ok(())); - - check_scope!( + let hint_code = hint_code::EC_DOUBLE_ASSIGN_NEW_X_V1; + + let mut vm = vm_with_range_check!(); + + //Insert ids.point and ids.slope into memory + vm.segments = segments![ + ((1, 0), 134), + ((1, 1), 5123), + ((1, 2), 140), + ((1, 3), 1232), + ((1, 4), 4652), + ((1, 5), 720), + ((1, 6), 44186171158942157784255469_i128), + ((1, 7), 54173758974262696047492534_i128), + ((1, 8), 8106299688661572814170174_i128) + ]; + + //Initialize fp + vm.run_context.fp = 10; + let ids_data = HashMap::from([ + ("point".to_string(), HintReference::new_simple(-10)), + ("slope".to_string(), HintReference::new_simple(-4)), + ]); + let mut exec_scopes = ExecutionScopes::new(); + + //Execute the hint + assert_matches!(run_hint!(vm, ids_data, hint_code, &mut exec_scopes), Ok(())); + + check_scope!( + &exec_scopes, + [ + ( + "slope", + bigint_str!( + "48526828616392201132917323266456307435009781900148206102108934970258721901549" + ) + ), + ( + "x", + bigint_str!("838083498911032969414721426845751663479194726707495046") + ), + ( + "y", + bigint_str!("4310143708685312414132851373791311001152018708061750480") + ), + ( + "value", + bigint_str!( + "59479631769792988345961122678598249997181612138456851058217178025444564264149" + ) + ), + ( + "new_x", + bigint_str!( + "59479631769792988345961122678598249997181612138456851058217178025444564264149" + ) + ) + ] + ); + } + + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn run_ec_double_assign_new_x_v2_ok() { + let hint_code = hint_code::EC_DOUBLE_ASSIGN_NEW_X_V2; + + let mut vm = vm_with_range_check!(); + + //Insert ids.point and ids.slope into memory + vm.segments = segments![ + ((1, 0), 134), + ((1, 1), 5123), + ((1, 2), 140), + ((1, 3), 1232), + ((1, 4), 4652), + ((1, 5), 720), + ((1, 6), 44186171158942157784255469_i128), + ((1, 7), 54173758974262696047492534_i128), + ((1, 8), 8106299688661572814170174_i128) + ]; + + //Initialize fp + vm.run_context.fp = 10; + let ids_data = HashMap::from([ + ("point".to_string(), HintReference::new_simple(-10)), + ("slope".to_string(), HintReference::new_simple(-4)), + ]); + let mut exec_scopes = ExecutionScopes::new(); + exec_scopes.assign_or_update_variable("SECP_P", any_box!(SECP_P.clone())); + + //Execute the hint + assert_matches!(run_hint!(vm, ids_data, hint_code, &mut exec_scopes), Ok(())); + + check_scope!( &exec_scopes, [ ( @@ -972,7 +1047,6 @@ mod tests { ) ] ); - } } #[test]