Merge branch 'main' into notlesh/segment-arena-relocation-fix-upstream

Author: whichqua

CHANGELOG.md

@@ -4,6 +4,14 @@
 
 * fix: Always use a normal segment in first SegmentArena segment [#1845](https://github.com/lambdaclass/cairo-vm/pull/1845)
 
+* feat: add get_current_step getter [#2034](https://github.com/lambdaclass/cairo-vm/pull/2034)
+
+* feat: implement VirtualMachine::is_accessed [#2033](https://github.com/lambdaclass/cairo-vm/pull/2033)
+
+* Refactor: Replaced HashMap with BTreeMap to guarantee deterministic ordering of the data [#2023] (https://github.com/lambdaclass/cairo-vm/pull/2023)
+
+* fix: Updated the logic for collecting builtin segment data for prover input info, removing dependency on the existence of stop pointers. [#2022](https://github.com/lambdaclass/cairo-vm/pull/2022)
+
 * fix: Keep None values in memory segments for the prover input info [#2021](https://github.com/lambdaclass/cairo-vm/pull/2021)
 
 * refactor: Clap attribute macros from #[clap(...)] to #[arg(...)] and #[command(...)] in v4.x [#2003] (https://github.com/lambdaclass/cairo-vm/pull/2003)

CONTRIBUTING.md

@@ -110,6 +110,9 @@ Enhancement suggestions are tracked as [GitHub issues](https://github.com/lambda
 - You may want to **include screenshots and animated GIFs** which help you demonstrate the steps or point out the part which the suggestion is related to. You can use [this tool](https://www.cockos.com/licecap/) to record GIFs on macOS and Windows, and [this tool](https://github.com/colinkeenan/silentcast) or [this tool](https://github.com/GNOME/byzanz) on Linux. 
 - **Explain why this enhancement would be useful** to most cairo-vm users. You may also want to point out the other projects that solved it better and which could serve as inspiration.
 
+### Pushing commits to Pull Requests
+
+This repository enforces commit signing on all branches. See [this page](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) for more details on how to sign a commit and how to manage signing keys in Github.
 
 <!-- TODO
 ### Your First Code Contribution

vm/src/vm/runners/cairo_runner.rs

@@ -4,7 +4,7 @@ use crate::{
     math_utils::safe_div_usize,
     stdlib::{
         any::Any,
-        collections::{HashMap, HashSet},
+        collections::{BTreeMap, HashMap, HashSet},
         ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign},
         prelude::*,
     },
@@ -1516,10 +1516,21 @@ impl CairoRunner {
             })
             .collect();
 
-        let builtins_segments = self
-            .get_builtin_segment_info_for_pie()?
-            .into_iter()
-            .map(|(name, info)| (info.index as usize, name))
+        let builtins_segments: BTreeMap<usize, BuiltinName> = self
+            .vm
+            .builtin_runners
+            .iter()
+            .filter(|builtin| {
+                // Those segments are not treated as builtins by the prover.
+                !matches!(
+                    builtin,
+                    BuiltinRunner::SegmentArena(_) | BuiltinRunner::Output(_)
+                )
+            })
+            .map(|builtin| {
+                let (index, _) = builtin.get_memory_segment_addresses();
+                (index, builtin.name())
+            })
             .collect();
 
         Ok(ProverInputInfo {
@@ -1543,9 +1554,9 @@ pub struct ProverInputInfo {
     /// A vector of segments, where each segment is a vector of maybe relocatable values or holes (`None`).
     pub relocatable_memory: Vec<Vec<Option<MaybeRelocatable>>>,
     /// A map from segment index to a vector of offsets within the segment, representing the public memory addresses.
-    pub public_memory_offsets: HashMap<usize, Vec<usize>>,
+    pub public_memory_offsets: BTreeMap<usize, Vec<usize>>,
     /// A map from the builtin segment index into its name.
-    pub builtins_segments: HashMap<usize, BuiltinName>,
+    pub builtins_segments: BTreeMap<usize, BuiltinName>,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -5592,7 +5603,29 @@ mod tests {
         assert!(prover_info.public_memory_offsets.is_empty());
         assert_eq!(
             prover_info.builtins_segments,
-            HashMap::from([(2, BuiltinName::ecdsa)])
+            BTreeMap::from([(2, BuiltinName::ecdsa)])
         );
     }
+
+    #[test]
+    fn test_output_not_builtin_segment() {
+        let program_content =
+            include_bytes!("../../../../cairo_programs/proof_programs/split_felt.json");
+        let runner = crate::cairo_run::cairo_run(
+            program_content,
+            &CairoRunConfig {
+                trace_enabled: true,
+                layout: LayoutName::all_cairo,
+                ..Default::default()
+            },
+            &mut BuiltinHintProcessor::new_empty(),
+        )
+        .unwrap();
+        let prover_info = runner.get_prover_input_info().unwrap();
+
+        assert!(!prover_info
+            .builtins_segments
+            .values()
+            .any(|v| *v == BuiltinName::output));
+    }
 }

vm/src/vm/vm_core.rs

@@ -895,6 +895,10 @@ impl VirtualMachine {
         self.run_context.get_pc()
     }
 
+    pub fn get_current_step(&self) -> usize {
+        self.current_step
+    }
+
     ///Gets the integer value corresponding to the Relocatable address
     pub fn get_integer(&self, key: Relocatable) -> Result<Cow<Felt252>, MemoryError> {
         self.segments.memory.get_integer(key)
@@ -971,6 +975,10 @@ impl VirtualMachine {
         self.segments.memory.mem_eq(lhs, rhs, len)
     }
 
+    pub fn is_accessed(&self, addr: &Relocatable) -> Result<bool, MemoryError> {
+        self.segments.is_accessed(addr)
+    }
+
     ///Gets `n_ret` return values from memory
     pub fn get_return_values(&self, n_ret: usize) -> Result<Vec<MaybeRelocatable>, MemoryError> {
         let addr = (self.run_context.get_ap() - n_ret)
@@ -4729,12 +4737,18 @@ mod tests {
             ((0, 1), 0),
             ((0, 2), 1),
             ((0, 10), 10),
-            ((1, 1), 1)
+            ((1, 1), 1),
+            ((1, 2), 0),
+            ((2, 0), 0),
+            ((-1, 0), 0),
+            ((-1, 1), 0)
         ];
         vm.mark_address_range_as_accessed((0, 0).into(), 3).unwrap();
         vm.mark_address_range_as_accessed((0, 10).into(), 2)
             .unwrap();
         vm.mark_address_range_as_accessed((1, 1).into(), 1).unwrap();
+        vm.mark_address_range_as_accessed((-1, 0).into(), 1)
+            .unwrap();
         //Check that the following addresses have been accessed:
         // Addresses have been copied from python execution:
         let mem = &vm.segments.memory.data;
@@ -4755,6 +4769,14 @@ mod tests {
                 .get_amount_of_accessed_addresses_for_segment(1),
             Some(1)
         );
+        assert!(vm.is_accessed(&Relocatable::from((0, 0))).unwrap());
+        assert!(vm.is_accessed(&Relocatable::from((0, 2))).unwrap());
+        assert!(vm.is_accessed(&Relocatable::from((0, 10))).unwrap());
+        assert!(vm.is_accessed(&Relocatable::from((1, 1))).unwrap());
+        assert!(!vm.is_accessed(&Relocatable::from((1, 2))).unwrap());
+        assert!(!vm.is_accessed(&Relocatable::from((2, 0))).unwrap());
+        assert!(vm.is_accessed(&Relocatable::from((-1, 0))).unwrap());
+        assert!(!vm.is_accessed(&Relocatable::from((-1, 1))).unwrap());
     }
 
     #[test]
@@ -5152,7 +5174,7 @@ mod tests {
         let mut virtual_machine_from_builder = virtual_machine_builder.build();
 
         assert!(virtual_machine_from_builder.run_finished);
-        assert_eq!(virtual_machine_from_builder.current_step, 12);
+        assert_eq!(virtual_machine_from_builder.get_current_step(), 12);
         assert_eq!(
             virtual_machine_from_builder
                 .builtin_runners

vm/src/vm/vm_memory/memory.rs

@@ -648,6 +648,23 @@ impl Memory {
         Ok(values)
     }
 
+    fn get_cell(&self, addr: Relocatable) -> Option<&MemoryCell> {
+        let (i, j) = from_relocatable_to_indexes(addr);
+        let data = if addr.segment_index < 0 {
+            &self.temp_data
+        } else {
+            &self.data
+        };
+        data.get(i)?.get(j)
+    }
+
+    pub fn is_accessed(&self, addr: &Relocatable) -> Result<bool, MemoryError> {
+        Ok(self
+            .get_cell(*addr)
+            .ok_or(MemoryError::UnknownMemoryCell(Box::new(*addr)))?
+            .is_accessed())
+    }
+
     pub fn mark_as_accessed(&mut self, addr: Relocatable) {
         let (i, j) = from_relocatable_to_indexes(addr);
         let data = if addr.segment_index < 0 {

vm/src/vm/vm_memory/memory_segments.rs

@@ -200,6 +200,10 @@ impl MemorySegmentManager {
         }
     }
 
+    pub fn is_accessed(&self, addr: &Relocatable) -> Result<bool, MemoryError> {
+        self.memory.is_accessed(addr)
+    }
+
     /// Counts the memory holes (aka unaccessed memory cells) in memory
     /// # Parameters
     /// - `builtin_segment_indexes`: Set representing the segments indexes of the builtins initialized in the VM, except for the output builtin.