Merge branch 'arbitrary-felts-sqrt-and-proptests' of github.com:lambdaclass/cairo-rs into arbitrary-felts-sqrt-and-proptests

Author: fmoletta

.github/workflows/test_install.yml

@@ -1,8 +1,11 @@
 name: Test dependencies and cairo-rs install
 
 on:
+  merge_group:
   push:
-    branches-ignore: [ 'main' ]
+    branches: [ main ]
+  pull_request:
+    branches: [ '*' ]
 
 jobs:
   install:

CHANGELOG.md

@@ -43,7 +43,7 @@
   * Implement `TryFrom<CasmContractClass> for Program`
   * Add `Cairo1HintProcessor`
 
-* Add `CairoRunner::get_program method` [#1123](https://github.com/lambdaclass/cairo-rs/pull/1123):
+#### [0.4.0] - 2023-05-12
 
 * perf: insert elements from the tail in `load_data` so reallocation happens only once [#1117](https://github.com/lambdaclass/cairo-rs/pull/1117)
 
@@ -197,6 +197,10 @@
 
     BREAKING: added a new error variant `MemoryError::VecCapacityExceeded`
 
+* perf: specialize addition for `u64` and `Felt252` [#932](https://github.com/lambdaclass/cairo-rs/pull/932)
+    * Avoids the creation of a new `Felt252` instance for additions with a very restricted valid range
+    * This impacts specially the addition of `Relocatable` with `Felt252` values in `update_pc`, which take a significant amount of time in some benchmarks
+
 * fix(starknet-crypto): bump version to `0.5.0` [#1088](https://github.com/lambdaclass/cairo-rs/pull/1088)
     * This includes the fix for a `panic!` in `ecdsa::verify`.
       See: [#365](https://github.com/xJonathanLEI/starknet-rs/issues/365) and [#366](https://github.com/xJonathanLEI/starknet-rs/pulls/366)

Cargo.lock

@@ -3,10 +3,11 @@ members = [".", "cairo-vm-cli", "felt", "hint_accountant", "./deps/parse-hyperli
 
 [package]
 name = "cairo-vm"
-version = "0.3.0-rc1"
+version = "0.4.0"
 edition = "2021"
 license = "Apache-2.0"
 description = "Blazing fast Cairo interpreter"
+repository = "https://github.com/lambdaclass/cairo-rs/"
 
 [features]
 default = ["std", "with_mimalloc"]
@@ -73,10 +74,10 @@ thiserror-no-std = "2.0.2"
 # https://stackoverflow.com/questions/70630556/parse-allowing-nested-parentheses-in-nom
 # There is a proposal for extending nom::delimited to use this function:
 # https://github.com/Geal/nom/issues/1253
-parse-hyperlinks = { package = "cairo-take_until_unbalanced", path = "./deps/parse-hyperlinks", version = "0.24.2-rc1", default-features = false, features = [
+parse-hyperlinks = { package = "cairo-take_until_unbalanced", path = "./deps/parse-hyperlinks", version = "0.25.0", default-features = false, features = [
     "alloc",
 ] }
-felt = { package = "cairo-felt", path = "./felt", version = "0.3.0-rc1", default-features = false, features = [
+felt = { package = "cairo-felt", path = "./felt", version = "0.4.0", default-features = false, features = [
     "alloc",
 ] }
 bitvec = { version = "1", default-features = false, features = ["alloc"] }

Cargo.toml

@@ -1,4 +1,7 @@
 # Release Process
+
+## Typical workflow
+
 - [ ] Pull latest from `main` branch.
       `git checkout main && git pull origin main`
 - [ ] Determine release version string dependending on whether changes included
@@ -35,3 +38,11 @@
   - If it is a release candidate, mark it as a draft release.
 - [ ] Announce release through corresponding channels.
 
+## Hotfix releases
+
+Sometimes there's a critical bug in a released version and we made breaking changes in `main` since. To push a non-breaking hotfix, you need the follow the original steps but basing using the release branch you created previously as base and bumping the patch level.
+
+## Retroactive releases
+
+In the case we want to push a new release from a revision other than `HEAD` (say, we introduced features that we'd rather not ship yet), there would be no base commit to compare to for the normal GitHub PR, as it can only merge branches. This means we lose the review instance before publishing.
+A solution is branching twice, once to diverge from main and one for making the needed changes and creating a PR against the original commit.

RELEASE.md

@@ -1,8 +1,10 @@
 use cairo_vm::{
+    felt::Felt252,
     types::program::Program,
     vm::{runners::cairo_runner::CairoRunner, vm_core::VirtualMachine},
 };
-use criterion::{black_box, criterion_group, criterion_main, BatchSize, Criterion};
+use criterion::{black_box, criterion_group, criterion_main, BatchSize, BenchmarkId, Criterion};
+use num_traits::Zero;
 
 #[cfg(feature = "with_mimalloc")]
 use mimalloc::MiMalloc;
@@ -58,5 +60,39 @@ fn load_program_data(c: &mut Criterion) {
     });
 }
 
+fn add_u64_with_felt252(c: &mut Criterion) {
+    // There are 9 possible cases:
+    // - The felt is `0`
+    // - The felt is `-1` and the result in range
+    // - The felt is `-1` but the result is out of range (the u64 is `0`)
+    // - The felt is positive and the result in range
+    // - The felt is positive and the result is out of range
+    // - The felt is `> u64::MAX` which always causes to be out of range
+    // - The felt is `<= -2` and the result is in range
+    // - The felt is `<= -2` and the result is out of range
+    // - The felt is `< -u64::MAX` which always causes to be out of range
+    // I consider all of these cases because branching is the most likely
+    // bottleneck here.
+    let cases = [
+        (1u64, Felt252::zero()),
+        (1u64, Felt252::from(-1i128)),
+        (0u64, Felt252::from(-1i128)),
+        (0u64, Felt252::from(1i128)),
+        (1u64, Felt252::from(u64::MAX as i128)),
+        (0u64, Felt252::from(u64::MAX as i128 + 1i128)),
+        (2u64, Felt252::from(-2i128)),
+        (1u64, Felt252::from(-2i128)),
+        (0u64, Felt252::from(-(u64::MAX as i128) - 1i128)),
+    ];
+    let mut group = c.benchmark_group("add_u64_with_felt");
+    for (i, case) in cases.iter().enumerate() {
+        group.bench_with_input(BenchmarkId::from_parameter(i), case, |b, (lhs, rhs)| {
+            b.iter(|| *lhs + rhs);
+        });
+    }
+    group.finish();
+}
+
+criterion_group!(felt, add_u64_with_felt252);
 criterion_group!(runner, build_many_runners, load_program_data, parse_program);
-criterion_main!(runner);
+criterion_main!(felt, runner);

bench/criterion_benchmark.rs

@@ -1,11 +1,11 @@
 [package]
 name = "cairo-vm-cli"
-version = "0.3.0-rc1"
+version = "0.4.0"
 edition = "2021"
 
 [dependencies]
 bincode = { version = "2.0.0-rc.2", tag = "v2.0.0-rc.2", git = "https://github.com/bincode-org/bincode.git" }
-cairo-vm = { path = "..", version = "0.3.0-rc1" }
+cairo-vm = { path = "..", version = "0.4.0" }
 clap = { version = "3.2.5", features = ["derive"] }
 mimalloc = { version = "0.1.29", default-features = false, optional = true }
 nom = "7"

cairo-vm-cli/Cargo.toml

@@ -0,0 +1,12 @@
+#[contract]
+mod Felt252Dict {
+
+use dict::{felt252_dict_entry_finalize, Felt252DictTrait};
+    /// An external method that requires the `segment_arena` builtin.
+    #[external]
+    fn squash_empty_dict() -> bool {
+        let x = felt252_dict_new::<felt252>();
+        x.squash();
+        return true;
+    }
+}

cairo_programs/cairo-1-contracts/felt_252_dict.cairo

@@ -0,0 +1,19 @@
+#[contract]
+mod FieldSqrt {
+    use core::traits::Into;
+    use option::OptionTrait;   
+    use ec::ec_point_from_x;
+    use ec::ec_point_non_zero;
+    use ec::ec_point_unwrap;
+
+    #[external]
+    fn field_sqrt() -> felt252 {
+        let p = ec_point_from_x(10).unwrap();
+        let p_nz = ec_point_non_zero(p);
+
+        let (qx, _) = ec_point_unwrap(p_nz);
+
+        assert(qx == 10, 'bad finalize x');
+        qx
+    }
+}

cairo_programs/cairo-1-contracts/field_sqrt.cairo

@@ -0,0 +1,27 @@
+#[contract]
+mod RandomEcPoint{
+    use option::OptionTrait;
+    use ec::ec_state_init;
+    use ec::ec_state_add;
+    use ec::ec_state_try_finalize_nz;
+    use ec::ec_point_from_x;
+    use ec::ec_point_non_zero;
+    use ec::ec_point_unwrap;
+
+    // Test taken from https://github.com/starkware-libs/cairo/blob/a0ead7c0b8e297d281c7213151cd43ac11de5042/corelib/src/test/ec_test.cairo#L17
+    #[external]
+    fn random_ec_point() -> felt252{
+        let p = ec_point_from_x(1).unwrap();
+        let p_nz = ec_point_non_zero(p);
+
+        let mut state = ec_state_init();
+        ec_state_add(ref state, p_nz);
+
+        let q = ec_state_try_finalize_nz(state).expect('zero point');
+        let (qx, qy) = ec_point_unwrap(q);
+
+        assert(qx == 1, 'bad finalize x');
+        qx
+    }
+
+}