Skip to content

Commit

Permalink
bugfix: Fix hint BIGINT_PACK_DIV_MOD (#1189)
Browse files Browse the repository at this point in the history
* Use `to_signed_felt` instead of `to_bigint`

* fmt

* Add changelog entry
  • Loading branch information
fmoletta committed May 30, 2023
1 parent a08c151 commit e06c560
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 14 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

#### 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: Fix hint `BIGINT_PACK_DIV_MOD` [#1189](https://github.com/lambdaclass/cairo-rs/pull/1189)

* Fix possible subtraction overflow in `QUAD_BIT` & `DI_BIT` hints [#1185](https://github.com/lambdaclass/cairo-rs/pull/1185)
* fix: 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: 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
18 changes: 6 additions & 12 deletions src/hint_processor/builtin_hint_processor/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::{
};
use felt::Felt252;
use num_bigint::BigInt;
use num_bigint::ToBigInt;
use num_traits::{One, Signed, Zero};

use super::hint_utils::insert_value_from_var_name;
Expand All @@ -37,27 +36,22 @@ pub fn bigint_pack_div_mod_hint(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<(), HintError> {
let p: BigInt = BigInt3::from_var_name("P", vm, ids_data, ap_tracking)?
.pack86()
.to_bigint()
.unwrap_or_default();
let p: BigInt = BigInt3::from_var_name("P", vm, ids_data, ap_tracking)?.pack86();

let x: BigInt = {
let x_bigint5 = BigInt5::from_var_name("x", vm, ids_data, ap_tracking)?;
// pack only takes the first three limbs
let x_lower = BigInt3 {
d0: x_bigint5.d0,
d1: x_bigint5.d1,
d2: x_bigint5.d2,
};
let x_lower = x_lower.pack86().to_bigint().unwrap_or_default();
let d3 = x_bigint5.d3.as_ref().to_bigint();
let d4 = x_bigint5.d4.as_ref().to_bigint();
let x_lower = x_lower.pack86();
let d3 = x_bigint5.d3.as_ref().to_signed_felt();
let d4 = x_bigint5.d4.as_ref().to_signed_felt();
x_lower + d3 * BigInt::from(BASE.pow(3)) + d4 * BigInt::from(BASE.pow(4))
};
let y: BigInt = BigInt3::from_var_name("y", vm, ids_data, ap_tracking)?
.pack86()
.to_bigint()
.unwrap_or_default();
let y: BigInt = BigInt3::from_var_name("y", vm, ids_data, ap_tracking)?.pack86();

let res = div_mod(&x, &y, &p);
exec_scopes.insert_value("res", res.clone());
Expand Down

1 comment on commit e06c560

@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: e06c560 Previous: a08c151 Ratio
add_u64_with_felt/0 1 ns/iter (± 0) 0 ns/iter (± 0) Infinity
add_u64_with_felt/5 2 ns/iter (± 0) 1 ns/iter (± 0) 2
add_u64_with_felt/6 4 ns/iter (± 0) 3 ns/iter (± 0) 1.33
add_u64_with_felt/7 4 ns/iter (± 0) 2 ns/iter (± 0) 2
add_u64_with_felt/8 3 ns/iter (± 0) 2 ns/iter (± 0) 1.50

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

CC: @unbalancedparentheses

Please sign in to comment.