Merged few VM fixes (#1820)

* 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 * remove outdated test * Enable ensure-no_std on test * Add correct test * Rename tset * Add comment --------- Co-authored-by: Alon Titelman <[email protected]> Co-authored-by: Omri Eshhar <[email protected]> Co-authored-by: Julián González Calderón <[email protected]> Co-authored-by: OmriEshhar1 <[email protected]>
lambdaclass · Sep 4, 2024 · ed31170 · ed31170
1 parent 294a10b
commit ed31170
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 #### Upcoming Changes
 
+* fix: Added the following VM fixes: [#1820](https://github.com/lambdaclass/cairo-vm/pull/1820)
+  * Fix zero segment location.
+  * Fix has_zero_segment naming.
+  * Fix prover input.
+  * Fix version reading when no version is supplied.
+
 * chore: bump `cairo-lang-` dependencies to 2.7.1 [#1823](https://github.com/lambdaclass/cairo-vm/pull/1823)
 
 #### [1.0.1] - 2024-08-12

diff --git a/vm/src/air_private_input.rs b/vm/src/air_private_input.rs
@@ -19,6 +19,8 @@ pub struct AirPrivateInputSerializable {
     #[serde(skip_serializing_if = "Option::is_none")]
     range_check: Option<Vec<PrivateInput>>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    range_check96: Option<Vec<PrivateInput>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     ecdsa: Option<Vec<PrivateInput>>,
     #[serde(skip_serializing_if = "Option::is_none")]
     bitwise: Option<Vec<PrivateInput>>,
@@ -157,6 +159,7 @@ impl AirPrivateInput {
             memory_path,
             pedersen: self.0.get(&BuiltinName::pedersen).cloned(),
             range_check: self.0.get(&BuiltinName::range_check).cloned(),
+            range_check96: self.0.get(&BuiltinName::range_check96).cloned(),
             ecdsa: self.0.get(&BuiltinName::ecdsa).cloned(),
             bitwise: self.0.get(&BuiltinName::bitwise).cloned(),
             ec_op: self.0.get(&BuiltinName::ec_op).cloned(),
@@ -230,6 +233,10 @@ mod tests {
                 index: 10000,
                 value: Felt252::from(8000),
             })]),
+            range_check96: Some(vec![PrivateInput::Value(PrivateInputValue {
+                index: 10000,
+                value: Felt252::from(8000),
+            })]),
             ecdsa: Some(vec![PrivateInput::Signature(PrivateInputSignature {
                 index: 0,
                 pubkey: Felt252::from(123),

diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs
@@ -173,7 +173,8 @@ pub fn cairo_run_pie(
         }
     }
     // Load previous execution memory
-    let n_extra_segments = pie.metadata.extra_segments.len();
+    let has_zero_segment = cairo_runner.vm.segments.has_zero_segment() as usize;
+    let n_extra_segments = pie.metadata.extra_segments.len() - has_zero_segment;
     cairo_runner
         .vm
         .segments

diff --git a/vm/src/vm/runners/builtin_runner/modulo.rs b/vm/src/vm/runners/builtin_runner/modulo.rs
@@ -118,7 +118,10 @@ impl ModBuiltinRunner {
 
     pub fn initialize_segments(&mut self, segments: &mut MemorySegmentManager) {
         self.base = segments.add().segment_index as usize; // segments.add() always returns a positive index
-        self.zero_segment_index = segments.add_zero_segment(self.zero_segment_size)
+    }
+
+    pub fn initialize_zero_segment(&mut self, segments: &mut MemorySegmentManager) {
+        self.zero_segment_index = segments.add_zero_segment(self.zero_segment_size);
     }
 
     pub fn initial_stack(&self) -> Vec<MaybeRelocatable> {
@@ -677,17 +680,16 @@ fn apply_op(lhs: &BigUint, rhs: &BigUint, op: &Operation) -> Result<BigUint, Mat
 
 #[cfg(test)]
 mod tests {
-
     #[test]
     #[cfg(feature = "mod_builtin")]
-    fn test_air_private_input_small_batch_size() {
+    fn test_air_private_input_all_cairo() {
         use super::*;
         use crate::{
             air_private_input::{ModInput, ModInputInstance, ModInputMemoryVars, PrivateInput},
             hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor,
             types::layout_name::LayoutName,
             utils::test_utils::Program,
-            vm::runners::{builtin_runner::BuiltinRunner, cairo_runner::CairoRunner},
+            vm::runners::cairo_runner::CairoRunner,
             Felt252,
         };
 
@@ -701,18 +703,14 @@ mod tests {
 
         let end = runner.initialize(false).unwrap();
         // Modify add_mod & mul_mod params
-        for runner in runner.vm.get_builtin_runners_as_mut() {
-            if let BuiltinRunner::Mod(runner) = runner {
-                runner.override_layout_params(1, 3)
-            }
-        }
 
         runner.run_until_pc(end, &mut hint_processor).unwrap();
         runner.run_for_steps(1, &mut hint_processor).unwrap();
         runner.end_run(false, false, &mut hint_processor).unwrap();
         runner.read_return_values(false).unwrap();
         runner.finalize_segments().unwrap();
 
+        // We compare against the execution of python cairo-run with the same layout
         let air_private_input = runner.get_air_private_input();
         assert_eq!(
             air_private_input.0.get(&BuiltinName::add_mod).unwrap()[0],
@@ -779,7 +777,7 @@ mod tests {
                         ),])
                     }
                 ],
-                zero_value_address: 22123
+                zero_value_address: 23019
             })
         );
         assert_eq!(
@@ -877,7 +875,7 @@ mod tests {
                         ),])
                     }
                 ],
-                zero_value_address: 22123
+                zero_value_address: 23019
             })
         )
     }

diff --git a/vm/src/vm/runners/builtin_runner/signature.rs b/vm/src/vm/runners/builtin_runner/signature.rs
@@ -220,7 +220,6 @@ impl SignatureBuiltinRunner {
                 private_inputs.push(PrivateInput::Signature(PrivateInputSignature {
                     index: addr
                         .offset
-                        .saturating_sub(self.base)
                         .checked_div(CELLS_PER_SIGNATURE as usize)
                         .unwrap_or_default(),
                     pubkey: *pubkey,

diff --git a/vm/src/vm/runners/cairo_pie.rs b/vm/src/vm/runners/cairo_pie.rs
@@ -322,8 +322,13 @@ impl CairoPie {
     ) -> Result<CairoPie, std::io::Error> {
         use std::io::Read;
 
-        let reader = std::io::BufReader::new(zip_reader.by_name("version.json")?);
-        let version: CairoPieVersion = serde_json::from_reader(reader)?;
+        let version = match zip_reader.by_name("version.json") {
+            Ok(version_buffer) => {
+                let reader = std::io::BufReader::new(version_buffer);
+                serde_json::from_reader(reader)?
+            }
+            Err(_) => CairoPieVersion { cairo_pie: () },
+        };
 
         let reader = std::io::BufReader::new(zip_reader.by_name("metadata.json")?);
         let metadata: CairoPieMetadata = serde_json::from_reader(reader)?;

diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs
@@ -234,6 +234,11 @@ impl CairoRunner {
         self.initialize_builtins(allow_missing_builtins)?;
         self.initialize_segments(None);
         let end = self.initialize_main_entrypoint()?;
+        for builtin_runner in self.vm.builtin_runners.iter_mut() {
+            if let BuiltinRunner::Mod(runner) = builtin_runner {
+                runner.initialize_zero_segment(&mut self.vm.segments);
+            }
+        }
         self.initialize_vm()?;
         Ok(end)
     }

diff --git a/vm/src/vm/vm_memory/memory_segments.rs b/vm/src/vm/vm_memory/memory_segments.rs
@@ -217,7 +217,7 @@ impl MemorySegmentManager {
             }
             let accessed_amount =
                 // Instead of marking the values in the zero segment until zero_segment_size as accessed we use zero_segment_size as accessed_amount
-                if !self.zero_segment_index.is_zero() && i == self.zero_segment_index {
+                if self.has_zero_segment() && i == self.zero_segment_index {
                     self.zero_segment_size
                 } else {
                     match self.memory.get_amount_of_accessed_addresses_for_segment(i) {
@@ -278,11 +278,15 @@ impl MemorySegmentManager {
             .insert(segment_index, public_memory.cloned().unwrap_or_default());
     }
 
+    pub fn has_zero_segment(&self) -> bool {
+        !self.zero_segment_index.is_zero()
+    }
+
     // Creates the zero segment if it wasn't previously created
     // Fills the segment with the value 0 until size is reached
     // Returns the index of the zero segment
     pub(crate) fn add_zero_segment(&mut self, size: usize) -> usize {
-        if self.zero_segment_index.is_zero() {
+        if !self.has_zero_segment() {
             self.zero_segment_index = self.add().segment_index as usize;
         }
 
@@ -298,7 +302,7 @@ impl MemorySegmentManager {
 
     // Finalizes the zero segment and clears it's tracking data from the manager
     pub(crate) fn finalize_zero_segment(&mut self) {
-        if !self.zero_segment_index.is_zero() {
+        if self.has_zero_segment() {
             self.finalize(Some(self.zero_segment_size), self.zero_segment_index, None);
             self.zero_segment_index = 0;
             self.zero_segment_size = 0;