Fix_no_trace_padding_flow_in_proof_mode (#1909)

YairVaknin-starkware · web-flow · commit df12864e7a1a · 2025-02-03T10:05:14.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 #### Upcoming Changes
 
+* fix(BREAKING): Fix no trace padding flow in proof mode [#1909](https://github.com/lambdaclass/cairo-vm/pull/1909)
+
 * refactor: Limit ret opcode decodeing to Cairo0's standards. [#1925](https://github.com/lambdaclass/cairo-vm/pull/1925)
 
 #### [2.0.0-rc4] - 2025-01-23
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
@@ -34,6 +34,13 @@ pub struct CairoRunConfig<'a> {
     pub dynamic_layout_params: Option<CairoLayoutParams>,
     pub proof_mode: bool,
     pub secure_run: Option<bool>,
+    /// Disable padding of the trace.
+    /// By default, the trace is padded to accommodate the expected builtins-n_steps relationships
+    /// according to the layout.
+    /// When the padding is disabled:
+    /// - It doesn't modify/pad n_steps.
+    /// - It still pads each builtin segment to the next power of 2 (w.r.t the number of used
+    ///   instances of the builtin) compared to their sizes at the end of the execution.
     pub disable_trace_padding: bool,
     pub allow_missing_builtins: Option<bool>,
 }
@@ -75,6 +82,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 +170,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 +244,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,111 @@ 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() {
+        const PEDERSEN_TEST: &[u8] =
+            include_bytes!("../../../../../cairo_programs/proof_programs/pedersen_test.json");
+        const BIGINT_TEST: &[u8] =
+            include_bytes!("../../../../../cairo_programs/proof_programs/bigint.json");
+        const POSEIDON_HASH_TEST: &[u8] =
+            include_bytes!("../../../../../cairo_programs/proof_programs/poseidon_hash.json");
+
+        let program_files = vec![PEDERSEN_TEST, BIGINT_TEST, POSEIDON_HASH_TEST];
+
+        for program_data in program_files {
+            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 (used_cells_true, 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 different when trace padding is
+                // enabled/disabled. Holds for this specific program, not always (that is, in other
+                // programs, padding may be of size 0, or the same).
+                assert!(
+                    n_allocated_instances_true == 0
+                        || n_allocated_instances_true != n_allocated_instances_false
+                );
+
+                // Since the last instance of the builtin isn't guaranteed to have a full output,
+                // the number of used_cells might not be a multiple of cells_per_instance, so we
+                // make sure that the discrepancy is up to the number of output cells.
+                // This is the same for both cases, so we only check one (true).
+                let n_output_cells = builtin_runner_true.cells_per_instance() as usize
+                    - builtin_runner_true.n_input_cells() as usize;
+                assert!(
+                    used_cells_true + n_output_cells
+                        >= (builtin_runner_true.cells_per_instance() as usize)
+                            * builtin_runner_true
+                                .get_used_instances(&runner_true.vm.segments)
+                                .unwrap()
+                );
+            }
+        }
+    }
+
     #[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
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`