Fix_no_trace_padding_flow_in_proof_mode

YairVaknin-starkware · YairVaknin-starkware · commit a8a42d8f2197 · 2025-01-19T17:48:29.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 #### Upcoming Changes
 
+* fix: Fix no trace padding flow in proof mode [#1909](https://github.com/lambdaclass/cairo-vm/pull/1909)
+
 * feat: implement `kzg` data availability hints [#1887](https://github.com/lambdaclass/cairo-vm/pull/1887)
 
 #### [2.0.0-rc3] - 2024-12-26
diff --git a/bench/criterion_benchmark.rs b/bench/criterion_benchmark.rs
@@ -33,6 +33,7 @@ fn build_many_runners(c: &mut Criterion) {
                     black_box(None),
                     black_box(false),
                     black_box(false),
+                    black_box(false),
                 )
                 .unwrap(),
             );
@@ -53,6 +54,7 @@ fn load_program_data(c: &mut Criterion) {
                     None,
                     false,
                     false,
+                    false,
                 )
                 .unwrap()
             },
diff --git a/bench/iai_benchmark.rs b/bench/iai_benchmark.rs
@@ -37,6 +37,7 @@ fn build_runner() {
         None,
         false,
         false,
+        false,
     )
     .unwrap();
     core::mem::drop(black_box(runner));
@@ -54,6 +55,7 @@ fn build_runner_helper() -> CairoRunner {
         None,
         false,
         false,
+        false,
     )
     .unwrap()
 }
diff --git a/cairo1-run/src/cairo_run.rs b/cairo1-run/src/cairo_run.rs
@@ -258,6 +258,7 @@ pub fn cairo_run_program(
         cairo_run_config.dynamic_layout_params.clone(),
         runner_mode,
         cairo_run_config.trace_enabled,
+        false,
     )?;
     let end = runner.initialize(cairo_run_config.proof_mode)?;
     load_arguments(&mut runner, &cairo_run_config, main_func, initial_gas)?;
diff --git a/hint_accountant/src/main.rs b/hint_accountant/src/main.rs
@@ -49,7 +49,7 @@ fn run() {
             whitelists.push(whitelist_file.allowed_hint_expressions);
         }
     }
-    let mut vm = VirtualMachine::new(false);
+    let mut vm = VirtualMachine::new(false, false);
     let mut hint_executor = BuiltinHintProcessor::new_empty();
     let (ap_tracking_data, reference_ids, references, mut exec_scopes, constants) = (
         ApTracking::default(),
diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs
@@ -75,6 +75,7 @@ pub fn cairo_run_program_with_initial_scope(
         cairo_run_config.dynamic_layout_params.clone(),
         cairo_run_config.proof_mode,
         cairo_run_config.trace_enabled,
+        cairo_run_config.disable_trace_padding,
     )?;
 
     cairo_runner.exec_scopes = exec_scopes;
@@ -162,6 +163,7 @@ pub fn cairo_run_pie(
         cairo_run_config.dynamic_layout_params.clone(),
         false,
         cairo_run_config.trace_enabled,
+        cairo_run_config.disable_trace_padding,
     )?;
 
     let end = cairo_runner.initialize(allow_missing_builtins)?;
@@ -235,6 +237,7 @@ pub fn cairo_run_fuzzed_program(
         cairo_run_config.dynamic_layout_params.clone(),
         cairo_run_config.proof_mode,
         cairo_run_config.trace_enabled,
+        cairo_run_config.disable_trace_padding,
     )?;
 
     let _end = cairo_runner.initialize(allow_missing_builtins)?;
diff --git a/vm/src/hint_processor/builtin_hint_processor/secp/cairo0_hints.rs b/vm/src/hint_processor/builtin_hint_processor/secp/cairo0_hints.rs
@@ -372,7 +372,7 @@ mod tests {
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn test_is_on_curve_2() {
-        let mut vm = VirtualMachine::new(false);
+        let mut vm = VirtualMachine::new(false, false);
         vm.set_fp(1);
         let ids_data = non_continuous_ids_data![("is_on_curve", -1)];
         vm.segments = segments![((1, 0), 1)];
@@ -404,7 +404,7 @@ mod tests {
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn test_compute_q_mod_prime() {
-        let mut vm = VirtualMachine::new(false);
+        let mut vm = VirtualMachine::new(false, false);
 
         let ap_tracking = ApTracking::default();
 
@@ -431,7 +431,7 @@ mod tests {
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn test_compute_ids_high_low() {
-        let mut vm = VirtualMachine::new(false);
+        let mut vm = VirtualMachine::new(false, false);
 
         let value = BigInt::from(25);
         let shift = BigInt::from(12);
@@ -501,7 +501,7 @@ mod tests {
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn test_r1_get_point_from_x() {
-        let mut vm = VirtualMachine::new(false);
+        let mut vm = VirtualMachine::new(false, false);
         vm.set_fp(10);
 
         let ids_data = non_continuous_ids_data![("x", -10), ("v", -7)];
@@ -560,7 +560,7 @@ mod tests {
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn test_reduce_value() {
-        let mut vm = VirtualMachine::new(false);
+        let mut vm = VirtualMachine::new(false, false);
 
         //Initialize fp
         vm.run_context.fp = 10;
@@ -616,7 +616,7 @@ mod tests {
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn test_reduce_x() {
-        let mut vm = VirtualMachine::new(false);
+        let mut vm = VirtualMachine::new(false, false);
 
         //Initialize fp
         vm.run_context.fp = 10;
diff --git a/vm/src/tests/cairo_run_test.rs b/vm/src/tests/cairo_run_test.rs
@@ -1185,6 +1185,7 @@ fn run_program_with_custom_mod_builtin_params(
         cairo_run_config.dynamic_layout_params,
         cairo_run_config.proof_mode,
         cairo_run_config.trace_enabled,
+        cairo_run_config.disable_trace_padding,
     )
     .unwrap();
 
diff --git a/vm/src/tests/mod.rs b/vm/src/tests/mod.rs
@@ -116,6 +116,7 @@ fn run_cairo_1_entrypoint(
         None,
         false,
         false,
+        false,
     )
     .unwrap();
 
@@ -221,6 +222,7 @@ fn run_cairo_1_entrypoint_with_run_resources(
         None,
         false,
         false,
+        false,
     )
     .unwrap();
 
diff --git a/vm/src/utils.rs b/vm/src/utils.rs
@@ -234,7 +234,7 @@ pub mod test_utils {
 
     macro_rules! vm_with_range_check {
         () => {{
-            let mut vm = VirtualMachine::new(false);
+            let mut vm = VirtualMachine::new(false, false);
             vm.builtin_runners = vec![
                 $crate::vm::runners::builtin_runner::RangeCheckBuiltinRunner::<8>::new(
                     Some(8),
@@ -255,12 +255,13 @@ pub mod test_utils {
                 None,
                 false,
                 false,
+                false,
             )
             .unwrap()
         };
         ($program:expr, $layout:expr) => {
             crate::vm::runners::cairo_runner::CairoRunner::new(
-                &$program, $layout, None, false, false,
+                &$program, $layout, None, false, false, false,
             )
             .unwrap()
         };
@@ -271,6 +272,7 @@ pub mod test_utils {
                 None,
                 $proof_mode,
                 false,
+                false,
             )
             .unwrap()
         };
@@ -281,6 +283,7 @@ pub mod test_utils {
                 None,
                 $proof_mode,
                 $trace_enabled,
+                false,
             )
             .unwrap()
         };
@@ -405,11 +408,11 @@ pub mod test_utils {
 
     macro_rules! vm {
         () => {{
-            crate::vm::vm_core::VirtualMachine::new(false)
+            crate::vm::vm_core::VirtualMachine::new(false, false)
         }};
 
         ($use_trace:expr) => {{
-            crate::vm::vm_core::VirtualMachine::new($use_trace)
+            crate::vm::vm_core::VirtualMachine::new($use_trace, false)
         }};
     }
     pub(crate) use vm;
diff --git a/vm/src/vm/runners/builtin_runner/mod.rs b/vm/src/vm/runners/builtin_runner/mod.rs
@@ -532,17 +532,29 @@ impl BuiltinRunner {
                 Ok((used, used))
             }
             _ => {
-                let used = self.get_used_cells(&vm.segments)?;
-                let size = self.get_allocated_memory_units(vm)?;
-                if used > size {
-                    return Err(InsufficientAllocatedCellsError::BuiltinCells(Box::new((
-                        self.name(),
-                        used,
-                        size,
-                    )))
-                    .into());
+                let used_cells = self.get_used_cells(&vm.segments)?;
+                if vm.disable_trace_padding {
+                    // If trace padding is disabled, we pad the used cells to still ensure that the
+                    // number of instances is a power of 2.
+                    let num_instances = self.get_used_instances(&vm.segments)?;
+                    let padded_used_cells = if num_instances > 0 {
+                        num_instances.next_power_of_two() * self.cells_per_instance() as usize
+                    } else {
+                        0
+                    };
+                    Ok((used_cells, padded_used_cells))
+                } else {
+                    let size = self.get_allocated_memory_units(vm)?;
+                    if used_cells > size {
+                        return Err(InsufficientAllocatedCellsError::BuiltinCells(Box::new((
+                            self.name(),
+                            used_cells,
+                            size,
+                        )))
+                        .into());
+                    }
+                    Ok((used_cells, size))
                 }
-                Ok((used, size))
             }
         }
     }
@@ -693,11 +705,13 @@ impl From<ModBuiltinRunner> for BuiltinRunner {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::cairo_run::{cairo_run, CairoRunConfig};
     use crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor;
     use crate::relocatable;
     use crate::types::builtin_name::BuiltinName;
     use crate::types::instance_definitions::mod_instance_def::ModInstanceDef;
     use crate::types::instance_definitions::LowRatio;
+    use crate::types::layout_name::LayoutName;
     use crate::types::program::Program;
     use crate::utils::test_utils::*;
     use crate::vm::errors::memory_errors::InsufficientAllocatedCellsError;
@@ -877,6 +891,87 @@ mod tests {
         assert_eq!(builtin.get_allocated_memory_units(&cairo_runner.vm), Ok(5));
     }
 
+    #[test]
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
+    fn compare_proof_mode_with_and_without_disable_trace_padding() {
+        let program_data =
+            include_bytes!("../../../../../cairo_programs/proof_programs/pedersen_test.json");
+
+        let config_false = CairoRunConfig {
+            disable_trace_padding: false,
+            proof_mode: true,
+            layout: LayoutName::all_cairo,
+            ..Default::default()
+        };
+        let mut hint_processor_false = BuiltinHintProcessor::new_empty();
+        let runner_false =
+            cairo_run(program_data, &config_false, &mut hint_processor_false).unwrap();
+        let last_step_false = runner_false.vm.current_step;
+
+        assert!(last_step_false.is_power_of_two());
+
+        let config_true = CairoRunConfig {
+            disable_trace_padding: true,
+            proof_mode: true,
+            layout: LayoutName::all_cairo,
+            ..Default::default()
+        };
+        let mut hint_processor_true = BuiltinHintProcessor::new_empty();
+        let runner_true = cairo_run(program_data, &config_true, &mut hint_processor_true).unwrap();
+        let last_step_true = runner_true.vm.current_step;
+
+        // Ensure the last step is not a power of two - true for this specific program, not always.
+        assert!(!last_step_true.is_power_of_two());
+
+        assert!(last_step_true < last_step_false);
+
+        let builtin_runners_false = &runner_false.vm.builtin_runners;
+        let builtin_runners_true = &runner_true.vm.builtin_runners;
+        assert_eq!(builtin_runners_false.len(), builtin_runners_true.len());
+        // Compare allocated instances for each pair of builtin runners.
+        for (builtin_runner_false, builtin_runner_true) in builtin_runners_false
+            .iter()
+            .zip(builtin_runners_true.iter())
+        {
+            assert_eq!(builtin_runner_false.name(), builtin_runner_true.name());
+            match builtin_runner_false {
+                BuiltinRunner::Output(_) | BuiltinRunner::SegmentArena(_) => {
+                    continue;
+                }
+                _ => {}
+            }
+            let (_, allocated_size_false) = builtin_runner_false
+                .get_used_cells_and_allocated_size(&runner_false.vm)
+                .unwrap();
+            let (_, allocated_size_true) = builtin_runner_true
+                .get_used_cells_and_allocated_size(&runner_true.vm)
+                .unwrap();
+            let n_allocated_instances_false = safe_div_usize(
+                allocated_size_false,
+                builtin_runner_false.cells_per_instance() as usize,
+            )
+            .unwrap();
+            let n_allocated_instances_true = safe_div_usize(
+                allocated_size_true,
+                builtin_runner_true.cells_per_instance() as usize,
+            )
+            .unwrap();
+            assert!(
+                n_allocated_instances_false.is_power_of_two() || n_allocated_instances_false == 0
+            );
+            assert!(
+                n_allocated_instances_true.is_power_of_two() || n_allocated_instances_true == 0
+            );
+            // Checks that the number of allocated instances is diffrent when trace padding is
+            // disabled. True for this specific program, not always. That is, instance could could
+            // happen to be the same.
+            assert!(
+                n_allocated_instances_true == 0
+                    || n_allocated_instances_true != n_allocated_instances_false
+            );
+        }
+    }
+
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn get_allocated_memory_units_ec_op_with_items() {
diff --git a/vm/src/vm/runners/builtin_runner/modulo.rs b/vm/src/vm/runners/builtin_runner/modulo.rs
@@ -817,7 +817,7 @@ mod tests {
         let mut hint_processor = BuiltinHintProcessor::new_empty();
         let program = Program::from_bytes(program_data, Some("main")).unwrap();
         let mut runner =
-            CairoRunner::new(&program, LayoutName::all_cairo, None, true, false).unwrap();
+            CairoRunner::new(&program, LayoutName::all_cairo, None, true, false, false).unwrap();
 
         let end = runner.initialize(false).unwrap();
         // Modify add_mod & mul_mod params
diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs
diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs

Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ fn build_runner() {`
`37`	`37`	`None,`
`38`	`38`	`false,`
`39`	`39`	`false,`
	`40`	`+ false,`
`40`	`41`	`)`
`41`	`42`	`.unwrap();`
`42`	`43`	`core::mem::drop(black_box(runner));`
`@@ -54,6 +55,7 @@ fn build_runner_helper() -> CairoRunner {`
`54`	`55`	`None,`
`55`	`56`	`false,`
`56`	`57`	`false,`
	`58`	`+ false,`
`57`	`59`	`)`
`58`	`60`	`.unwrap()`
`59`	`61`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ fn run() {`
`49`	`49`	`whitelists.push(whitelist_file.allowed_hint_expressions);`
`50`	`50`	`}`
`51`	`51`	`}`
`52`		`- let mut vm = VirtualMachine::new(false);`
	`52`	`+ let mut vm = VirtualMachine::new(false, false);`
`53`	`53`	`let mut hint_executor = BuiltinHintProcessor::new_empty();`
`54`	`54`	`let (ap_tracking_data, reference_ids, references, mut exec_scopes, constants) = (`
`55`	`55`	`ApTracking::default(),`
Original file line number	Diff line number	Diff line change
`@@ -1185,6 +1185,7 @@ fn run_program_with_custom_mod_builtin_params(`
`1185`	`1185`	`cairo_run_config.dynamic_layout_params,`
`1186`	`1186`	`cairo_run_config.proof_mode,`
`1187`	`1187`	`cairo_run_config.trace_enabled,`
	`1188`	`+ cairo_run_config.disable_trace_padding,`
`1188`	`1189`	`)`
`1189`	`1190`	`.unwrap();`
`1190`	`1191`
Original file line number	Diff line number	Diff line change
`@@ -116,6 +116,7 @@ fn run_cairo_1_entrypoint(`
`116`	`116`	`None,`
`117`	`117`	`false,`
`118`	`118`	`false,`
	`119`	`+ false,`
`119`	`120`	`)`
`120`	`121`	`.unwrap();`
`121`	`122`
`@@ -221,6 +222,7 @@ fn run_cairo_1_entrypoint_with_run_resources(`
`221`	`222`	`None,`
`222`	`223`	`false,`
`223`	`224`	`false,`
	`225`	`+ false,`
`224`	`226`	`)`
`225`	`227`	`.unwrap();`
`226`	`228`