Skip to content

Commit

Permalink
feat: Add feature lambdaworks-felt to felt & cairo-vm crates (#…
Browse files Browse the repository at this point in the history
…1281)

* wip

* Manually implement some common derives

Also comment `from` impls for primitive numbers and (De)Serialize derives

* Implement FromPrimitive

* Implement ToPrimitive

* Add BitAnd/Xor/Or implementations

* Implement bit shift operators

* Remove Signed implementation

* Remove Integer impl

* Impl Bounded

* Impl from_str_radix

* Add iter_u64_digits impl

* Add Add impls

* Add Sub impls

* Patch arbitrary

* Fix some warnings

* Implement parse_bytes

* Implement utility methods for tests

* Fix test compilation errors

* Add From impl for signed primitive nums

* Impl From<BigInt>

* Re-add bits fn

* Impl Signed

* Impl (De)Serialize

* Fix compile errors and clippy suggestions

* Pin cairo 1 compiler version

* Reorder impls

* Fix compile error

* Fix various errors (tests pass!)

* Remove to_bytes_be

* Fix panicky from_bytes_be

* Fix is_positive

* Fix str conversions

* Add documentation

* Fix from_i64 accepting negative numbers

* Use BigUint in from_bytes_be

* Remove unneded field macro

* Pin lambdaworks commit

* Fix keccak

* Fix from_i64 condition was reversed

* Change Debug::fmt to return number in decimal

* Appease clippy in felt crate

* Silence clippy warning (for now)

* Fix nostd error

* Update lambdaworks to latest revision

* Update rust version in CI

* Change sqrt for lambdaworks'

* Manually build BigDigits on to_biguint

* Use a bigger number of iterations for square bench

* Update lambdaworks-math revision

* Fix sqrt tests and Shl/Shr impl (+add tests)

* Update Cargo.lock

* Appease clippy and fix Shr<usize>

* Square input instead of using prop_assume

* Revert the change to Shr

* Appease clippy

* Use bits instead of shl in range check

* Remove `FeltBigInt`

* Add lamdaworks-benchmarks.sh

* Add lamdaworks-benchmarks.md

* Update lamdaworks-benchmarks.md

* Update commits

* Update .md commits

* Looooong benchmark

* Remove bigbox clippy allow

* Update lambdaworks to latest, and change AddAssign

* Remove `-P` option in `xargs`

In the measurements we got through `perf`, there were 3 to 4 times more page faults compared to base.
This can be explained by the runner using swap memory because of the increase in memory usage.
We can fix it by reducing the amount of processes ran in parallel by xargs (2 -> 1)

* Change `to_(b|l)e_bytes` to not use lw primitives

* Change comments in `Add<&Felt252> for u64`

* Fix: wasn't indexing properly :P

* Override default `div_mod_floor` impl for Felt252

* Disable swap memory before benchmarking

* Reduce number of warmups and runs

* Optimize `assert_le_felt`

(used in math_integration_benchmark)

* Use constant for zero() function

* Extract division by constant

* Use BigUint in assert_le_felt

This part uses comparisons and integer division (that use `to_biguint`), so it's better to use `BigUints` directly.

* Avoid calling `BigInt::abs`

* Update changelog

* Add tests for felt

* Add TODO

* Add other texts

* Update lambdaworks to latest

* Revert hyperfine arguments to main

* Remove unneeded clones and into_owneds

* Remove unneeded references and clones

* Add BREAKING note to changelog

* Make Felt252::one just copy a constant

* Impl From<bool> for Felt252

* Change some uses of get_ref with get_mut_ref

Using `get_ref` and later updating the variable with an `exec_scopes.insert_value(...)` causes two lookups in a hashmap, along with two creation of `String` from a slice.
This change reduces it to just a single lookup and `String` creation.

* Unify mem*_continue_* functions

* Run benchmarks sequentially to avoid mem issues

* Use div_mod_floor instead of div and mod

* Use BigUint for non-modular calculations

* Add TODO

* Include both lib.rs

* Fix lib_bigint_felt

* Add test-lambdaworks-felt workflow

* Fix failling example

* Move extern crate import to lib.rs

* Update changelog

* Fix changelog

* Fix example

* Remove benchmark docs

* Remove clone

* Move crate-level attribute to lib.rs

* Fix changelog

* Remove blank line in toml

* Use one line cfg directives

* Remove reference

* Restore clone

* Fix doc test

* Add `lambdaworks-felt` feature to vm crate

* Add instructions to (de)activate the new feature

* Use different matrix group for lambdaworks felt in CI

* Move the sections a bit

* Update lambdaworks-math version to 0.1.1

* Invert the part talking about features

---------

Co-authored-by: Pedro Fontana <fontana.pedro93@gmail.com>
Co-authored-by: Tomá <47506558+MegaRedHand@users.noreply.github.com>
Co-authored-by: Mario Rugiero <mrugiero@gmail.com>
  • Loading branch information
4 people committed Jun 26, 2023
1 parent 8e133a2 commit d0804dd
Show file tree
Hide file tree
Showing 30 changed files with 3,647 additions and 1,819 deletions.
6 changes: 2 additions & 4 deletions .github/workflows/bench.yml
Expand Up @@ -14,11 +14,9 @@ jobs:
build:
runs-on: ubuntu-20.04
steps:
- name: Install Rust 1.66.1
uses: actions-rs/toolchain@v1
- name: Install Rust
uses: dtolnay/rust-toolchain@1.69.0
with:
toolchain: 1.66.1
override: true
components: rustfmt, clippy
- uses: actions/checkout@v3
- name: Python3 Build
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/hint_accountant.yml
Expand Up @@ -12,7 +12,7 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Install Rust toolchain
uses: dtolnay/rust-toolchain@1.66.1
uses: dtolnay/rust-toolchain@1.69.0
- name: Set up Cargo cache
uses: Swatinem/rust-cache@v2
- name: Checkout
Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/hyperfine.yml
Expand Up @@ -74,9 +74,7 @@ jobs:

- name: Install Rust
if: ${{ steps.cache.outputs.cache-hit != 'true' }}
uses: dtolnay/rust-toolchain@stable
with:
toolchain: 1.66.1
uses: dtolnay/rust-toolchain@1.69.0

- name: Checkout
if: ${{ steps.cache.outputs.cache-hit != 'true' }}
Expand Down
6 changes: 1 addition & 5 deletions .github/workflows/iai_main.yml
Expand Up @@ -9,11 +9,7 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Install Rust
uses: actions-rs/toolchain@v1
with:
toolchain: 1.66.1
override: true
profile: minimal
uses: dtolnay/rust-toolchain@1.69.0
- name: Python3 Build
uses: actions/setup-python@v4
with:
Expand Down
12 changes: 2 additions & 10 deletions .github/workflows/iai_pr.yml
Expand Up @@ -9,11 +9,7 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Install Rust
uses: actions-rs/toolchain@v1
with:
toolchain: 1.66.1
override: true
profile: minimal
uses: dtolnay/rust-toolchain@1.69.0
- name: Python3 Build
uses: actions/setup-python@v4
with:
Expand Down Expand Up @@ -46,11 +42,7 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: Install Rust
uses: actions-rs/toolchain@v1
with:
toolchain: 1.66.1
override: true
profile: minimal
uses: dtolnay/rust-toolchain@1.69.0
- name: Python3 Build
uses: actions/setup-python@v4
with:
Expand Down
7 changes: 1 addition & 6 deletions .github/workflows/publish.yml
Expand Up @@ -13,11 +13,7 @@ jobs:
- name: Checkout sources
uses: actions/checkout@v2
- name: Install stable toolchain
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true
uses: dtolnay/rust-toolchain@1.69.0
- name: Publish crate cairo-take_until_unbalanced
env:
CRATES_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }}
Expand All @@ -34,4 +30,3 @@ jobs:
env:
CRATES_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }}
run: cargo publish --token ${CRATES_TOKEN} --all-features -p cairo-vm

7 changes: 4 additions & 3 deletions .github/workflows/rust.yml
Expand Up @@ -170,6 +170,7 @@ jobs:
strategy:
fail-fast: false
matrix:
special-features: ["", "lambdaworks-felt"]
target: [ test, test-no_std, test-wasm ]
# TODO: features
name: Run tests
Expand Down Expand Up @@ -221,14 +222,14 @@ jobs:
# FIXME: we need to update the Makefile to do this correctly
case ${{ matrix.target }} in
'test')
cargo llvm-cov nextest --lcov --output-path lcov-${{ matrix.target }}.info --workspace --features "cairo-1-hints, test_utils"
cargo llvm-cov nextest --lcov --output-path lcov-${{ matrix.target }}.info --workspace --features "cairo-1-hints, test_utils, ${{ matrix.special_features }}"
;;
'test-no_std')
cargo llvm-cov nextest --lcov --output-path lcov-${{ matrix.target }}.info --workspace --no-default-features
cargo llvm-cov nextest --lcov --output-path lcov-${{ matrix.target }}.info --workspace --no-default-features --features "${{ matrix.special_features }}"
;;
'test-wasm')
# NOTE: release mode is needed to avoid "too many locals" error
wasm-pack test --release --node vm --no-default-features
wasm-pack test --release --node vm --no-default-features --features "${{ matrix.special_features }}"
;;
esac
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,12 @@

#### Upcoming Changes

* feat: Add feature `lambdaworks-felt` to `felt` & `cairo-vm` crates [#1218](https://github.com/lambdaclass/cairo-rs/pull/1281)

Changes under this feature:
* `Felt252` now uses _lambdaworks_' `FieldElement` internally
* BREAKING: some methods of `Felt252` were removed, namely: `modpow` and `to_bytes_be`

#### [0.7.0] - 2023-6-26

* BREAKING: Integrate `RunResources` logic into `HintProcessor` trait [#1274](https://github.com/lambdaclass/cairo-rs/pull/1274)
Expand Down
77 changes: 76 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 14 additions & 2 deletions README.md
Expand Up @@ -81,14 +81,26 @@ These dependencies are only necessary in order to run the original VM, compile C

## 🚀 Usage

### Running cairo-rs
### Adding cairo-rs as a dependency

You can add the following to your rust project's `Cargo.toml`:

```toml
cairo-vm = { version = '0.7.0', features = ["lambdaworks-felt"] }
```

The `features = ["lambdaworks-felt"]` part adds usage of [`lambdaworks-math`](https://github.com/lambdaclass/lambdaworks) as the backend for `Felt252`. This improves performance by more than 20%, and will be the default in the future.

### Running cairo-rs from CLI

To run programs from the command line, first compile the repository from the cairo-vm-cli folder:

```bash
cd cairo-vm-cli; cargo build --release; cd ..
cd cairo-vm-cli; cargo build --release -F lambdaworks-felt; cd ..
```

The `-F lambdaworks-felt` part adds usage of [`lambdaworks-math`](https://github.com/lambdaclass/lambdaworks) as the backend for `Felt252`. This improves performance by more than 20%, and will be the default in the future.

Once the binary is built, it can be found in `target/release/` under the name `cairo-rvm-cli`.

To compile a program, use `cairo-compile [path_to_the_.cairo_file] --output [desired_path_of_the_compiled_.json_file]`. For example:
Expand Down
2 changes: 1 addition & 1 deletion deps/parse-hyperlinks/src/lib.rs
Expand Up @@ -15,7 +15,7 @@ use nom::IResult;
/// ```
/// use nom::bytes::complete::tag;
/// use nom::sequence::delimited;
/// use parse_hyperlinks::take_until_unbalanced;
/// use cairo_take_until_unbalanced::take_until_unbalanced;
///
/// let mut parser = delimited(tag("<"), take_until_unbalanced('<', '>'), tag(">"));
/// assert_eq!(parser("<<inside>inside>abc"), Ok(("abc", "<inside>inside")));
Expand Down
2 changes: 2 additions & 0 deletions felt/Cargo.toml
Expand Up @@ -9,6 +9,7 @@ description = "Field elements representation for the Cairo VM"
default = ["std"]
std = []
alloc = []
lambdaworks-felt = ["dep:lambdaworks-math"]

[dependencies]
num-integer = { version = "0.1.45", default-features = false }
Expand All @@ -18,6 +19,7 @@ lazy_static = { version = "1.4.0", default-features = false, features = [
"spin_no_std",
] }
serde = { version = "1.0", features = ["derive"], default-features = false }
lambdaworks-math = { version = "0.1.1", default-features = false, optional=true }

[dev-dependencies]
proptest = "1.1.0"
Expand Down
File renamed without changes.
47 changes: 47 additions & 0 deletions felt/src/arbitrary_lambdaworks.rs
@@ -0,0 +1,47 @@
use lambdaworks_math::{field::element::FieldElement, unsigned_integer::element::UnsignedInteger};
use num_traits::Zero;
use proptest::prelude::*;

use crate::{Felt252, FIELD_HIGH, FIELD_LOW};

/// Returns a [`Strategy`] that generates any valid Felt252
fn any_felt252() -> impl Strategy<Value = Felt252> {
(0..=FIELD_HIGH)
// turn range into `impl Strategy`
.prop_map(|x| x)
// choose second 128-bit limb capped by first one
.prop_flat_map(|high| {
let low = if high == FIELD_HIGH {
(0..FIELD_LOW).prop_map(|x| x).sboxed()
} else {
any::<u128>().sboxed()
};
(Just(high), low)
})
// turn (u128, u128) into limbs array and then into Felt252
.prop_map(|(high, low)| {
let limbs = [
(high >> 64) as u64,
(high & ((1 << 64) - 1)) as u64,
(low >> 64) as u64,
(low & ((1 << 64) - 1)) as u64,
];
FieldElement::new(UnsignedInteger::from_limbs(limbs))
})
.prop_map(|value| Felt252 { value })
}

/// Returns a [`Strategy`] that generates any nonzero Felt252
pub fn nonzero_felt252() -> impl Strategy<Value = Felt252> {
any_felt252().prop_filter("is zero", |x| !x.is_zero())
}

impl Arbitrary for Felt252 {
type Parameters = ();

fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
any_felt252().sboxed()
}

type Strategy = SBoxedStrategy<Self>;
}
8 changes: 1 addition & 7 deletions felt/src/bigint_felt.rs
Expand Up @@ -12,7 +12,7 @@ use core::{
},
};

use crate::{FeltOps, ParseFeltError};
use crate::{lib_bigint_felt::FeltOps, ParseFeltError};

pub const FIELD_HIGH: u128 = (1 << 123) + (17 << 64); // this is equal to 10633823966279327296825105735305134080
pub const FIELD_LOW: u128 = 1;
Expand Down Expand Up @@ -840,12 +840,6 @@ impl<const PH: u128, const PL: u128> fmt::Debug for FeltBigInt<PH, PL> {
}
}

impl fmt::Display for ParseFeltError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{ParseFeltError:?}")
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down

1 comment on commit d0804dd

@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: d0804dd Previous: 8e133a2 Ratio
initialize 64373 ns/iter (± 2865) 48382 ns/iter (± 1662) 1.33

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

CC: @unbalancedparentheses

Please sign in to comment.