bugfix: Fix BuiltinRunner::final_stack for SegmentArena (lambdacl…

…ass#1747) * Fix segment arena final_stack * Update changelog * Update test --------- Co-authored-by: Pedro Fontana <[email protected]> Co-authored-by: Mario Rugiero <[email protected]>
starkware-libs · May 9, 2024 · dd6c311 · dd6c311
1 parent fd14d0d
commit dd6c311
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 #### Upcoming Changes
 
+* bugfix: Fix BuiltinRunner::final_stack for SegmentArena[#1747](https://github.com/lambdaclass/cairo-vm/pull/1747)
+
 * feat: unify `arbitrary`, `hooks`, `print` and `skip_next_instruction_hint` features as a single `test_utils` feature [#1755](https://github.com/lambdaclass/cairo-vm/pull/1755)
   * BREAKING: removed the above features
 

diff --git a/vm/src/vm/runners/builtin_runner/mod.rs b/vm/src/vm/runners/builtin_runner/mod.rs
@@ -143,7 +143,13 @@ impl BuiltinRunner {
                 ))));
             }
             let stop_ptr = stop_pointer.offset;
-            let num_instances = self.get_used_instances(segments)?;
+            let mut num_instances = self.get_used_instances(segments)?;
+            if matches!(self, BuiltinRunner::SegmentArena(_)) {
+                // SegmentArena builtin starts with one instance pre-loaded
+                // This is reflected in the builtin base's offset, but as we compare `stop_ptr.offset` agains `used`
+                // instead of comparing `stop_ptr` against `base + used` we need to account for the base offset (aka the pre-loaded instance) here
+                num_instances += 1;
+            }
             let used = num_instances * self.cells_per_instance() as usize;
             if stop_ptr != used {
                 return Err(RunnerError::InvalidStopPointer(Box::new((

diff --git a/vm/src/vm/runners/builtin_runner/segment_arena.rs b/vm/src/vm/runners/builtin_runner/segment_arena.rs
@@ -150,11 +150,17 @@ mod tests {
         vm.segments.segment_used_sizes = Some(vec![6]);
 
         let pointer = Relocatable::from((2, 2));
+        // USED_CELLS = SEGMENT_USED_SIZE - INITIAL_SIZE = 6 - 3 = 3
+        // NUM_INSTANCES = DIV_CEIL(USED_CELLS, CELLS_PER_INSTANCE) = DIV_CEIL(3, 3) = 1
+
+        // STOP_PTR == BASE + NUM_INSTANCES *  CELLS_PER_INSTANCE
+        // (0, 0) == (0, 3) + 1 * 3
+        // (0, 0) == (0, 6)
         assert_eq!(
             builtin.final_stack(&vm.segments, pointer),
             Err(RunnerError::InvalidStopPointer(Box::new((
                 BuiltinName::segment_arena,
-                relocatable!(0, 3),
+                relocatable!(0, 6),
                 relocatable!(0, 0)
             ))))
         );