Skip to content

Commit

Permalink
Fix hint EC_DOUBLE_ASSIGN_NEW_X_V2 (#1186)
Browse files Browse the repository at this point in the history 
* Fix hint `EC_DOUBLE_ASSIGN_NEW_X_V2`

* Add changelog entry

---------

Co-authored-by: Pedro Fontana <fontana.pedro93@gmail.com>
  • Loading branch information
@fmoletta @pefontana
fmoletta and pefontana authored May 29, 2023
1 parent 0a14a99 commit 16db8ee
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 42 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 18 additions & 11 deletions src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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,
Expand Down
136 changes: 105 additions & 31 deletions src/hint_processor/builtin_hint_processor/secp/ec_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, HintReference>,
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:
%{
Expand Down Expand Up @@ -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,
[
(
Expand Down Expand Up @@ -972,7 +1047,6 @@ mod tests {
)
]
);
}
}

#[test]
Expand Down

1 comment on commit 16db8ee

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.

Benchmark suite Current: 16db8ee Previous: 0a14a99 Ratio
build runner 32655 ns/iter (± 1344) 24328 ns/iter (± 12) 1.34
initialize 77944 ns/iter (± 6544) 58792 ns/iter (± 587) 1.33
parse program 25624944 ns/iter (± 967543) 19045665 ns/iter (± 210507) 1.35

This comment was automatically generated by workflow using github-action-benchmark.

CC: @unbalancedparentheses

Please sign in to comment.