From ed3117098dd33c96056880af6fa67f9b2caebfb4 Mon Sep 17 00:00:00 2001 From: yuvalsw <105596564+yuvalsw@users.noreply.github.com> Date: Wed, 4 Sep 2024 18:15:10 +0300 Subject: [PATCH] Merged few VM fixes (#1820) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 Co-authored-by: Omri Eshhar Co-authored-by: Julián González Calderón Co-authored-by: OmriEshhar1 <45786542+OmriEshhar1@users.noreply.github.com> --- CHANGELOG.md | 6 ++++++ vm/src/air_private_input.rs | 7 +++++++ vm/src/cairo_run.rs | 3 ++- vm/src/vm/runners/builtin_runner/modulo.rs | 20 +++++++++---------- vm/src/vm/runners/builtin_runner/signature.rs | 1 - vm/src/vm/runners/cairo_pie.rs | 9 +++++++-- vm/src/vm/runners/cairo_runner.rs | 5 +++++ vm/src/vm/vm_memory/memory_segments.rs | 10 +++++++--- 8 files changed, 43 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fab13c1fb..c8d91f7617 100644 --- 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 index fa05cc86d5..6275ebd6a1 100644 --- 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>, #[serde(skip_serializing_if = "Option::is_none")] + range_check96: Option>, + #[serde(skip_serializing_if = "Option::is_none")] ecdsa: Option>, #[serde(skip_serializing_if = "Option::is_none")] bitwise: Option>, @@ -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 index b7d5d005e3..de2540fb2e 100644 --- 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 index c5d2dc56b5..b4aa0b7fa6 100644 --- 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 { @@ -677,17 +680,16 @@ fn apply_op(lhs: &BigUint, rhs: &BigUint, op: &Operation) -> Result Result { 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 index 4c42ad463d..ecf47892b0 100644 --- 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 index 04a0a58f18..b6d7379786 100644 --- 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;