Add quad hint and integration test #715

Juan-M-V · 2023-01-12T19:40:51Z

TITLE

Description

Description of the pull request changes and motivation.

Checklist

Linked to Github Issue
Unit tests added
Integration tests added.
This change requires new documentation.
- Documentation has been added/updated.

Oppen · 2023-01-12T20:38:12Z

felt/src/bigint_felt.rs

+        let mut m = FeltBigInt::zero();
+        for i in 0..48 {
+            let mut adm = &a * &(&d).pow(&m);
+            adm = adm.pow(1_u64 << (47 - i));
+            if &adm == &max_felt {
+                m += 1 << i;
+            }
+        }


I strongly suspect this loop can be changed to perform much fewer calls to pow, which is likely the culprit. I'll need some time to think of a way to do that.

Oppen · 2023-01-12T20:55:56Z

felt/src/bigint_felt.rs

+        let d = (&FeltBigInt::new(3_i32)).pow(&trailing_prime);
+        let mut m = FeltBigInt::zero();
+        for i in 0..48 {
+            let mut adm = &a * &(&d).pow(&m);
+            adm = adm.pow(1_u64 << (47 - i));
+            if &adm == &max_felt {
+                m += 1 << i;
+            }
+        }


I think this might help:

Suggested change

let d = (&FeltBigInt::new(3_i32)).pow(&trailing_prime);

let mut m = FeltBigInt::zero();

for i in 0..48 {

let mut adm = &a * &(&d).pow(&m);

adm = adm.pow(1_u64 << (47 - i));

if &adm == &max_felt {

m += 1 << i;

}

}

let d = (&FeltBigInt::new(3_i32)).pow(&trailing_prime);

let d_pow = d.clone();

let mut adm = a.clone();

for i in 0..48 {

adm = adm.pow(1_u64 << (47 - i));

if &adm == &max_felt {

d_pow *= d_pow * d;

}

}

fmoletta

Great Work!

Oppen · 2023-01-17T22:00:37Z

src/hint_processor/builtin_hint_processor/math_utils.rs

+        fn run_is_quad_residue(ref x in "([1-9][0-9]*)") {
+            let mut vm = vm!();
+            vm.run_context.fp = 2;
+            vm.memory = memory![((1, 1), (&x[..], 10))];
+            let ids_data = ids_data!["y", "x"];
+
+            assert_eq!(run_hint!(vm, ids_data, hint_code::IS_QUAD_RESIDUE), Ok(()));
+
+            let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();
+
+            if x.is_zero() || x.is_one() {
+                assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().as_ref(), x);
+            } else if x.pow(Felt::max_value() >> 1).is_one() {
+                assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().into_owned(), x.sqrt());
+            } else {
+                assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().into_owned(), (x / Felt::new(3)).sqrt());
+            }
+        }
+    }


Suggested change

fn run_is_quad_residue(ref x in "([1-9][0-9]*)") {

let mut vm = vm!();

vm.run_context.fp = 2;

vm.memory = memory![((1, 1), (&x[..], 10))];

let ids_data = ids_data!["y", "x"];

assert_eq!(run_hint!(vm, ids_data, hint_code::IS_QUAD_RESIDUE), Ok(()));

let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();

if x.is_zero() || x.is_one() {

assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().as_ref(), x);

} else if x.pow(Felt::max_value() >> 1).is_one() {

assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().into_owned(), x.sqrt());

} else {

assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().into_owned(), (x / Felt::new(3)).sqrt());

}

}

}

fn run_is_quad_residue_for_quad_residue(ref x in "([1-9][0-9]*)") {

let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();

let quad = x * x;

let mut vm = vm!();

vm.run_context.fp = 2;

vm.memory = memory![((1, 1), &quad)];

let ids_data = ids_data!["y", "x"];

assert_eq!(run_hint!(vm, ids_data, hint_code::IS_QUAD_RESIDUE), Ok(()));

assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().into_owned(), x);

}

}

fn run_is_quad_residue_for_non_quad_residue(ref x in "([1-9][0-9]*)") {

let x = &Felt::parse_bytes(x.as_bytes(), 10).unwrap();

prop_assume!(x.pow(Felt::max_value() >> 1).is_one());

let mut vm = vm!();

vm.run_context.fp = 2;

vm.memory = memory![((1, 1), &x)];

let ids_data = ids_data!["y", "x"];

assert_eq!(run_hint!(vm, ids_data, hint_code::IS_QUAD_RESIDUE), Ok(()));

assert_eq!(vm.get_integer(&Relocatable::from((1, 0))).unwrap().into_owned(), (x / Felt::new(3)).sqrt());

}

}

For 0 and 1 just use unit tests.

codecov · 2023-02-15T15:08:25Z

Codecov Report

Merging #715 (0d91946) into main (2a29461) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #715   +/-   ##
=======================================
  Coverage   97.74%   97.74%           
=======================================
  Files          74       74           
  Lines       30348    30392   +44     
=======================================
+ Hits        29663    29708   +45     
+ Misses        685      684    -1

Impacted Files	Coverage Δ
...int_processor/builtin_hint_processor_definition.rs	`98.66% <100.00%> (+<0.01%)`	⬆️
...int_processor/builtin_hint_processor/math_utils.rs	`97.81% <100.00%> (+0.06%)`	⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Oppen · 2023-03-14T20:07:17Z

felt/src/bigint_felt.rs

+            exponent >>= 1;
+            // if adm ≡ -1 (mod CAIRO_PRIME)
+            if adm == max_felt {
+                m += FeltBigInt::one() << i;


We can make a lookup table with the 192 possible powers of 2 for this.

* Initial progress * Implement random_ec_point hint * Add integration test * Fix alpha & beta values * Remove commented code * fix test program * Pad seed bytes * Pad left * Fix i padding * Fix x & y_coef * Copy sqrt implementation from PR #715 * Move sqrt to math_utils + cleanup * Add proptests * use constants in proptests * Add some tests * Add test * Add test * Add implementation for hint on chained_ec_op * Fix string format * Fix tests * Add test program * Add test program * Add impl for recover_y hint * Add test for hint * Add integration tests * Clippy * Clippy * Add newline at EOf * Remove unused trait impl * Update src/hint_processor/builtin_hint_processor/ec_utils.rs Co-authored-by: Mario Rugiero <[email protected]> * Use constant for Felt::max / 2 * Add missing import * Fix proptest dependency * Try to fix wasm tests --------- Co-authored-by: Mario Rugiero <[email protected]>

…esidue-hint

* Add quad hint and integration test * Fix out of memory bug * Implement hint and fix bugs * Add prop test * Add comment to sqrt * Take smaller root * Fix clippy warnings * Add sqrt prop test * Remove zero from test * Fix qrong sqrt being used * Fix qrong sqrt being used * Reduce integration test lenght * Fix wasm test * Clippy --------- Co-authored-by: Juanma <[email protected]> Co-authored-by: Federica <[email protected]>

Juanma added 2 commits January 12, 2023 16:40

Add quad hint and integration test

75152d8

Fix out of memory bug

c760e9f

Oppen reviewed Jan 12, 2023

View reviewed changes

Juanma added 4 commits January 13, 2023 13:26

Implement hint and fix bugs

de92f72

Add prop test

d9e748c

Add comment to sqrt

d037f2b

Take smaller root

e5e89eb

Juan-M-V marked this pull request as ready for review January 16, 2023 14:23

Juan-M-V requested review from igaray, fmoletta, entropidelic, HermanObst, juanbono, Jrigada and pefontana as code owners January 16, 2023 14:23

Juanma added 3 commits January 16, 2023 12:03

Fix clippy warnings

42d6419

Add sqrt prop test

ba0f03f

Remove zero from test

f198e6e

fmoletta approved these changes Jan 17, 2023

View reviewed changes

Merge branch 'main' into is-quad-residue-hint

91081e6

Oppen reviewed Jan 17, 2023

View reviewed changes

Fix and merge with main

24382ac

fmoletta added a commit that referenced this pull request Mar 10, 2023

Copy sqrt implementation from PR #715

79b7568

fmoletta mentioned this pull request Mar 10, 2023

Implement hints on common/ec.cairo #888

Merged

Oppen reviewed Mar 14, 2023

View reviewed changes

Oppen approved these changes Mar 14, 2023

View reviewed changes

Merge branch 'main' of github.com:lambdaclass/cairo-rs into is-quad-r…

cd9f99d

…esidue-hint

fmoletta added the whitelisted-hint Implementation of hint on whitelist directory label Apr 11, 2023

fmoletta added 5 commits April 11, 2023 14:43

Fix qrong sqrt being used

bd32cae

Fix qrong sqrt being used

fc42539

Reduce integration test lenght

fdeb24b

Fix wasm test

0854e9b

Clippy

0d91946

fmoletta added this pull request to the merge queue Apr 12, 2023

Merged via the queue into main with commit 5fab4cc Apr 12, 2023

fmoletta deleted the is-quad-residue-hint branch April 12, 2023 13:19

fmoletta mentioned this pull request Apr 13, 2023

[New Hint] IS_QUAD_RESIDUE #968

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add quad hint and integration test #715

Add quad hint and integration test #715

Juan-M-V commented Jan 12, 2023

Oppen Jan 12, 2023

Oppen Jan 12, 2023

fmoletta left a comment

Oppen Jan 17, 2023

codecov bot commented Feb 15, 2023 •

edited

Loading

Oppen Mar 14, 2023

Add quad hint and integration test #715

Add quad hint and integration test #715

Conversation

Juan-M-V commented Jan 12, 2023

TITLE

Description

Checklist

Oppen Jan 12, 2023

Choose a reason for hiding this comment

Oppen Jan 12, 2023

Choose a reason for hiding this comment

fmoletta left a comment

Choose a reason for hiding this comment

Oppen Jan 17, 2023

Choose a reason for hiding this comment

codecov bot commented Feb 15, 2023 • edited Loading

Codecov Report

Oppen Mar 14, 2023

Choose a reason for hiding this comment

codecov bot commented Feb 15, 2023 •

edited

Loading