Fix modulo builtin constraints (#1841)

JulianGCalderon · Alon-Ti · yuvalsw · web-flow · commit 7d1995678190 · 2024-09-30T21:14:10.000Z
* Fix Zero segment location.

* Fixed has_zero_segment naming

* Fix prover input.

* Fixed version reading when no version is supplied

* Added change to changelog.

* fix test_from_serializable()

* fix panic_impl error

* fix cairo version

* Add dummy changelog

* Pin wasm-bindgen

* Register change in CHANGELOG

* Update Cargo.lock

* Remove changes from CHANGELOG

* Add argument parsing for layout params file

* Add dynamic support (no implement)

* Add cairo_layout_params_file.example.json

* Implement dynamic layout creation

* Update CHANGELOG

* Add cli dynamic support for cairo 1

* Make wasm compatible

* Use public_memory_fraction = 4 vy default

* Deserialize bool from int

* Add comparison with python-vm (failing)

* Rebuild .rs files in makefile

* Use 8 as dynamic public_memory_fraction

The same value is used in python-vm

* Use None ratio for dynamic unused builtins

* Add rangecheck96 to private inputs

* Make dyn py files depend on params_file

* Use cpu_component_step=1 by default

* Fix typo in private inputs

* Add range check value to air private input test

* Fix zero segment location

* Use zero builtin instead of None

* Add debug scripts

* Remove dup makefile recipes

* remove outdated test

* Enable ensure-no_std on test

* Fix tests

* Add correct test

* Rename tset

* Add comment

* Add debugging document

* Update cairo layout params file

* Remove duplicated range check

* Remove dup

* Remove debugging and scrippts (moveed to another branch)

* Add comment

* Add tests

* Add dynamic test to cairo-vm-cli

* Add parse test

* Remove compare all dynamic

* Add script for comparing with dynamic layouts

* Add tests to workflow

* Delete logic changes

They are going to be moved to another branch

* Delete more logic changes

* Update rust.yml

* Rename compare_outputs_dynamic_layout.sh to compare_outputs_dynamic_layouts.sh

* Update test script

* Enforce prover constraints in add, sub, and mul

* Remove debug prints

* Add tests

* Update CHANGELOG.md

* Fix serialization

* Comment failing test

* Uncomment test

* Fix tests

* Remove operation struct and use builtin type instead

* Add unit tests to modulo builtin operations

* Fix security error message

* Test custom serde impl

* Upload mod_builtin coverage

---------

Co-authored-by: Alon Titelman &lt;alon.titelman@starkware.co&gt;
Co-authored-by: Yuval Goldberg &lt;yuval@starkware.co&gt;
Co-authored-by: Omri Eshhar &lt;omri@starkware.co&gt;
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -600,6 +600,36 @@ jobs:
         key: codecov-cache-test-no_std-extensive_hints-${{ github.sha }}
         fail-on-cache-miss: true
 
+    - name: Fetch results for tests with stdlib (w/mod_builtin; part. 1)
+      uses: actions/cache/restore@v3
+      with:
+        path: lcov-test#1-mod_builtin.info
+        key: codecov-cache-test#1-mod_builtin-${{ github.sha }}
+        fail-on-cache-miss: true
+    - name: Fetch results for tests with stdlib (w/mod_builtin; part. 2)
+      uses: actions/cache/restore@v3
+      with:
+        path: lcov-test#2-mod_builtin.info
+        key: codecov-cache-test#2-mod_builtin-${{ github.sha }}
+        fail-on-cache-miss: true
+    - name: Fetch results for tests with stdlib (w/mod_builtin; part. 3)
+      uses: actions/cache/restore@v3
+      with:
+        path: lcov-test#3-mod_builtin.info
+        key: codecov-cache-test#3-mod_builtin-${{ github.sha }}
+        fail-on-cache-miss: true
+    - name: Fetch results for tests with stdlib (w/mod_builtin; part. 4)
+      uses: actions/cache/restore@v3
+      with:
+        path: lcov-test#4-mod_builtin.info
+        key: codecov-cache-test#4-mod_builtin-${{ github.sha }}
+        fail-on-cache-miss: true
+    - name: Fetch results for tests without stdlib (w/mod_builtin)
+      uses: actions/cache/restore@v3
+      with:
+        path: lcov-no_std-mod_builtin.info
+        key: codecov-cache-test-no_std-mod_builtin-${{ github.sha }}
+        fail-on-cache-miss: true
 
     - name: Upload coverage to codecov.io
       uses: codecov/codecov-action@v3
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 #### Upcoming Changes
 
+* fix: [#1841](https://github.com/lambdaclass/cairo-vm/pull/1841):
+  * Fix modulo builtin to comply with prover constraints
+
 * feat(BREAKING): [#1824](https://github.com/lambdaclass/cairo-vm/pull/1824):
     * Add support for dynamic layout
     * CLI change(BREAKING): The flag `cairo_layout_params_file` must be specified when using dynamic layout.
diff --git a/vm/src/air_private_input.rs b/vm/src/air_private_input.rs
@@ -126,6 +126,8 @@ pub struct ModInputInstance {
     pub values_ptr: usize,
     pub offsets_ptr: usize,
     pub n: usize,
+    #[serde(deserialize_with = "mod_input_instance_batch_serde::deserialize")]
+    #[serde(serialize_with = "mod_input_instance_batch_serde::serialize")]
     pub batch: BTreeMap<usize, ModInputMemoryVars>,
 }
 
@@ -205,6 +207,88 @@ impl AirPrivateInputSerializable {
     }
 }
 
+mod mod_input_instance_batch_serde {
+    use super::*;
+
+    use serde::{Deserializer, Serializer};
+
+    pub(crate) fn serialize<S: Serializer>(
+        value: &BTreeMap<usize, ModInputMemoryVars>,
+        s: S,
+    ) -> Result<S::Ok, S::Error> {
+        let value = value.iter().map(|v| v.1).collect::<Vec<_>>();
+
+        value.serialize(s)
+    }
+
+    pub(crate) fn deserialize<'de, D: Deserializer<'de>>(
+        d: D,
+    ) -> Result<BTreeMap<usize, ModInputMemoryVars>, D::Error> {
+        let value = Vec::<ModInputMemoryVars>::deserialize(d)?;
+
+        Ok(value.into_iter().enumerate().collect())
+    }
+
+    #[cfg(feature = "std")]
+    #[test]
+    fn test_serde() {
+        let input_value = vec![
+            (
+                0,
+                ModInputMemoryVars {
+                    a_offset: 5,
+                    b_offset: 5,
+                    c_offset: 5,
+                    a0: Felt252::from(5u32),
+                    a1: Felt252::from(5u32),
+                    a2: Felt252::from(5u32),
+                    a3: Felt252::from(5u32),
+                    b0: Felt252::from(5u32),
+                    b1: Felt252::from(5u32),
+                    b2: Felt252::from(5u32),
+                    b3: Felt252::from(5u32),
+                    c0: Felt252::from(5u32),
+                    c1: Felt252::from(5u32),
+                    c2: Felt252::from(5u32),
+                    c3: Felt252::from(5u32),
+                },
+            ),
+            (
+                1,
+                ModInputMemoryVars {
+                    a_offset: 7,
+                    b_offset: 7,
+                    c_offset: 7,
+                    a0: Felt252::from(7u32),
+                    a1: Felt252::from(7u32),
+                    a2: Felt252::from(7u32),
+                    a3: Felt252::from(7u32),
+                    b0: Felt252::from(7u32),
+                    b1: Felt252::from(7u32),
+                    b2: Felt252::from(7u32),
+                    b3: Felt252::from(7u32),
+                    c0: Felt252::from(7u32),
+                    c1: Felt252::from(7u32),
+                    c2: Felt252::from(7u32),
+                    c3: Felt252::from(7u32),
+                },
+            ),
+        ]
+        .into_iter()
+        .collect::<BTreeMap<usize, _>>();
+
+        let bytes = Vec::new();
+        let mut serializer = serde_json::Serializer::new(bytes);
+        serialize(&input_value, &mut serializer).unwrap();
+        let bytes = serializer.into_inner();
+
+        let mut deserializer = serde_json::Deserializer::from_slice(&bytes);
+        let output_value = deserialize(&mut deserializer).unwrap();
+
+        assert_eq!(input_value, output_value);
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::types::layout_name::LayoutName;
diff --git a/vm/src/tests/compare_outputs_dynamic_layouts.sh b/vm/src/tests/compare_outputs_dynamic_layouts.sh
@@ -82,6 +82,9 @@ CASES=(
     "cairo_programs/proof_programs/sha256.json;double_all_cairo"
     "cairo_programs/proof_programs/keccak.json;all_cairo"
     "cairo_programs/proof_programs/keccak.json;double_all_cairo"
+    "cairo_programs/mod_builtin_feature/proof/mod_builtin.json;all_cairo"
+    "cairo_programs/mod_builtin_feature/proof/mod_builtin_failure.json;all_cairo"
+    "cairo_programs/mod_builtin_feature/proof/apply_poly.json;all_cairo"
 )
 
 passed_tests=0
diff --git a/vm/src/vm/runners/builtin_runner/modulo.rs b/vm/src/vm/runners/builtin_runner/modulo.rs

Original file line number	Diff line number	Diff line change
`@@ -82,6 +82,9 @@ CASES=(`
`82`	`82`	`"cairo_programs/proof_programs/sha256.json;double_all_cairo"`
`83`	`83`	`"cairo_programs/proof_programs/keccak.json;all_cairo"`
`84`	`84`	`"cairo_programs/proof_programs/keccak.json;double_all_cairo"`
	`85`	`+ "cairo_programs/mod_builtin_feature/proof/mod_builtin.json;all_cairo"`
	`86`	`+ "cairo_programs/mod_builtin_feature/proof/mod_builtin_failure.json;all_cairo"`
	`87`	`+ "cairo_programs/mod_builtin_feature/proof/apply_poly.json;all_cairo"`
`85`	`88`	`)`
`86`	`89`
`87`	`90`	`passed_tests=0`