From da0383d487ec141570ffb01d40bbd993ccd1022e Mon Sep 17 00:00:00 2001 From: juanbono Date: Wed, 17 Apr 2024 17:54:54 -0300 Subject: [PATCH 1/5] add new RunnerMode --- cairo1-run/src/cairo_run.rs | 2 +- vm/src/vm/runners/cairo_runner.rs | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/cairo1-run/src/cairo_run.rs b/cairo1-run/src/cairo_run.rs index 8477eb7d88..8f671ef6e8 100644 --- a/cairo1-run/src/cairo_run.rs +++ b/cairo1-run/src/cairo_run.rs @@ -188,7 +188,7 @@ pub fn cairo_run_program( let runner_mode = if cairo_run_config.proof_mode { RunnerMode::ProofModeCairo1 } else { - RunnerMode::ExecutionMode + RunnerMode::ExecutionModeCairo1 }; let mut runner = CairoRunner::new_v2(&program, cairo_run_config.layout, runner_mode)?; diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 8ea8600d3e..8c76d7beb0 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -164,11 +164,26 @@ pub struct CairoRunner { #[derive(Clone, Debug, PartialEq)] pub enum RunnerMode { - ExecutionMode, + /// Execution mode for Cairo 0 programs + ExecutionModeCairo0, + /// Execution mode for Cairo 1 programs + ExecutionModeCairo1, + /// Proof mode for Cairo 0 programs ProofModeCanonical, + /// Proof mode for Cairo 1 programs ProofModeCairo1, } +impl RunnerMode { + /// Returns whether the runner mode is proof mode + pub fn is_proof_mode(&self) -> bool { + matches!( + self, + RunnerMode::ProofModeCanonical | RunnerMode::ProofModeCairo1 + ) + } +} + impl CairoRunner { pub fn new_v2( program: &Program, @@ -208,7 +223,7 @@ impl CairoRunner { original_steps: None, relocated_memory: Vec::new(), exec_scopes: ExecutionScopes::new(), - execution_public_memory: if mode != RunnerMode::ExecutionMode { + execution_public_memory: if mode.is_proof_mode() { Some(Vec::new()) } else { None @@ -225,7 +240,7 @@ impl CairoRunner { if proof_mode { Self::new_v2(program, layout, RunnerMode::ProofModeCanonical) } else { - Self::new_v2(program, layout, RunnerMode::ExecutionMode) + Self::new_v2(program, layout, RunnerMode::ExecutionModeCairo0) } } @@ -367,8 +382,7 @@ impl CairoRunner { } fn is_proof_mode(&self) -> bool { - self.runner_mode == RunnerMode::ProofModeCanonical - || self.runner_mode == RunnerMode::ProofModeCairo1 + self.runner_mode.is_proof_mode() } // Initialize all the builtins. Values used are the original one from the CairoFunctionRunner From bae0dd03698e4993fbf6d49ff80d424a129962d1 Mon Sep 17 00:00:00 2001 From: juanbono Date: Wed, 17 Apr 2024 18:18:01 -0300 Subject: [PATCH 2/5] enable segment arena --- cairo1-run/src/cairo_run.rs | 24 ------------------- .../runners/builtin_runner/segment_arena.rs | 4 ++-- vm/src/vm/runners/cairo_runner.rs | 15 ++++++++++++ 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/cairo1-run/src/cairo_run.rs b/cairo1-run/src/cairo_run.rs index 8f671ef6e8..377af78790 100644 --- a/cairo1-run/src/cairo_run.rs +++ b/cairo1-run/src/cairo_run.rs @@ -380,29 +380,9 @@ fn create_entry_code( ap_offset += (1 + values.len()) as i16; } let mut array_args_data_iter = array_args_data.iter(); - let after_arrays_data_offset = ap_offset; let mut arg_iter = config.args.iter().enumerate(); let mut param_index = 0; let mut expected_arguments_size = 0; - if signature.param_types.iter().any(|ty| { - get_info(sierra_program_registry, ty) - .map(|x| x.long_id.generic_id == SegmentArenaType::ID) - .unwrap_or_default() - }) { - casm_extend! {ctx, - // SegmentArena segment. - %{ memory[ap + 0] = segments.add() %} - // Infos segment. - %{ memory[ap + 1] = segments.add() %} - ap += 2; - [ap + 0] = 0, ap++; - // Write Infos segment, n_constructed (0), and n_destructed (0) to the segment. - [ap - 2] = [[ap - 3]]; - [ap - 1] = [[ap - 3] + 1]; - [ap - 1] = [[ap - 3] + 2]; - } - ap_offset += 3; - } for ty in &signature.param_types { let info = get_info(sierra_program_registry, ty) @@ -420,10 +400,6 @@ fn create_entry_code( } else if generic_ty == &GasBuiltinType::ID { casm_extend!(ctx, [ap + 0] = initial_gas, ap++;); ap_offset += 1; - } else if generic_ty == &SegmentArenaType::ID { - let offset = -ap_offset + after_arrays_data_offset; - casm_extend!(ctx, [ap + 0] = [ap + offset] + 3, ap++;); - ap_offset += 1; } else { let ty_size = type_sizes[ty]; let param_ap_offset_end = ap_offset + ty_size; diff --git a/vm/src/vm/runners/builtin_runner/segment_arena.rs b/vm/src/vm/runners/builtin_runner/segment_arena.rs index 5cc61fb706..c673e5118b 100644 --- a/vm/src/vm/runners/builtin_runner/segment_arena.rs +++ b/vm/src/vm/runners/builtin_runner/segment_arena.rs @@ -46,7 +46,7 @@ impl SegmentArenaBuiltinRunner { if used < INITIAL_SEGMENT_SIZE { return Err(MemoryError::InvalidUsedSizeSegmentArena); } - Ok(used - INITIAL_SEGMENT_SIZE) + Ok(used) } pub fn initial_stack(&self) -> Vec { @@ -324,7 +324,7 @@ mod tests { let builtin = BuiltinRunner::SegmentArena(SegmentArenaBuiltinRunner::new(true)); let mut memory_segment_manager = MemorySegmentManager::new(); memory_segment_manager.segment_used_sizes = Some(vec![6]); - // (SIZE(6) - INITIAL_SIZE(3)) / CELLS_PER_INSTANCE(3) + // SIZE(6) / CELLS_PER_INSTANCE(3) assert_eq!(builtin.get_used_instances(&memory_segment_manager), Ok(1)); } diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 8c76d7beb0..b0e187ad5e 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -182,6 +182,14 @@ impl RunnerMode { RunnerMode::ProofModeCanonical | RunnerMode::ProofModeCairo1 ) } + + /// Returns whether the runner mode is for Cairo 1 programs + pub fn is_cairo1(&self) -> bool { + matches!( + self, + RunnerMode::ExecutionModeCairo1 | RunnerMode::ProofModeCairo1 + ) + } } impl CairoRunner { @@ -334,6 +342,13 @@ impl CairoRunner { } } + if self.runner_mode.is_cairo1() { + let included = program_builtins.remove(&BuiltinName::segment_arena); + if included || self.is_proof_mode() { + builtin_runners.push(SegmentArenaBuiltinRunner::new(included).into()); + } + } + if let Some(instance_def) = self.layout.builtins.keccak.as_ref() { let included = program_builtins.remove(&BuiltinName::keccak); if included || self.is_proof_mode() { From da0d9d06028eacdfe4cc6e69e8ed077c936c18cc Mon Sep 17 00:00:00 2001 From: juanbono Date: Wed, 17 Apr 2024 18:21:41 -0300 Subject: [PATCH 3/5] change CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c6bf4581f..1c7be3835f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ #### Upcoming Changes +* fix(BREAKING): Enable segment arena builtin on cairo1-run. + * fix(BREAKING): Remove unsafe impl of `Add for &'a Relocatable`[#1718](https://github.com/lambdaclass/cairo-vm/pull/1718) * fix(BREAKING): Handle triple dereference references[#1708](https://github.com/lambdaclass/cairo-vm/pull/1708) From 809970e0392743c396637a6c79eac1b3355548b4 Mon Sep 17 00:00:00 2001 From: juanbono Date: Wed, 17 Apr 2024 19:14:17 -0300 Subject: [PATCH 4/5] add segment arena to ordered builtins list --- cairo1-run/src/cairo_run.rs | 11 ++++++++++- vm/src/vm/runners/cairo_runner.rs | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/cairo1-run/src/cairo_run.rs b/cairo1-run/src/cairo_run.rs index 377af78790..b91807325c 100644 --- a/cairo1-run/src/cairo_run.rs +++ b/cairo1-run/src/cairo_run.rs @@ -38,7 +38,8 @@ use cairo_vm::{ runners::{ builtin_runner::{ BITWISE_BUILTIN_NAME, EC_OP_BUILTIN_NAME, HASH_BUILTIN_NAME, OUTPUT_BUILTIN_NAME, - POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME, SIGNATURE_BUILTIN_NAME, + POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME, SEGMENT_ARENA_BUILTIN_NAME, + SIGNATURE_BUILTIN_NAME, }, cairo_runner::{CairoRunner, RunResources, RunnerMode}, }, @@ -400,6 +401,8 @@ fn create_entry_code( } else if generic_ty == &GasBuiltinType::ID { casm_extend!(ctx, [ap + 0] = initial_gas, ap++;); ap_offset += 1; + } else if generic_ty == &SegmentArenaType::ID { + continue; } else { let ty_size = type_sizes[ty]; let param_ap_offset_end = ap_offset + ty_size; @@ -596,6 +599,11 @@ fn get_function_builtins( let mut builtin_offset: HashMap = HashMap::new(); let mut current_offset = 3; for (debug_name, builtin_name, sierra_id) in [ + ( + "SegmentArena", + BuiltinName::segment_arena, + SegmentArenaType::ID, + ), ("Poseidon", BuiltinName::poseidon, PoseidonType::ID), ("EcOp", BuiltinName::ec_op, EcOpType::ID), ("Bitwise", BuiltinName::bitwise, BitwiseType::ID), @@ -705,6 +713,7 @@ fn finalize_builtins( "EcOp" => EC_OP_BUILTIN_NAME, "Bitwise" => BITWISE_BUILTIN_NAME, "Pedersen" => HASH_BUILTIN_NAME, + "SegmentArena" => SEGMENT_ARENA_BUILTIN_NAME, "Output" => OUTPUT_BUILTIN_NAME, "Ecdsa" => SIGNATURE_BUILTIN_NAME, _ => { diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index b0e187ad5e..93e36a38dd 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -282,6 +282,7 @@ impl CairoRunner { BuiltinName::ec_op, BuiltinName::keccak, BuiltinName::poseidon, + BuiltinName::segment_arena, BuiltinName::range_check96, BuiltinName::add_mod, BuiltinName::mul_mod, From 6b5290c2fd151105c540a98c643061a233c0a1bc Mon Sep 17 00:00:00 2001 From: juanbono Date: Thu, 18 Apr 2024 12:55:18 -0300 Subject: [PATCH 5/5] fix segment_arena tests to take into account that we no longer subtract INITIAL_SEGMENT_SIZE --- vm/src/vm/runners/builtin_runner/segment_arena.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/vm/src/vm/runners/builtin_runner/segment_arena.rs b/vm/src/vm/runners/builtin_runner/segment_arena.rs index c673e5118b..57cd43253d 100644 --- a/vm/src/vm/runners/builtin_runner/segment_arena.rs +++ b/vm/src/vm/runners/builtin_runner/segment_arena.rs @@ -119,7 +119,7 @@ mod tests { let mut vm = vm!(); vm.segments.segment_used_sizes = Some(vec![3]); - assert_eq!(builtin.get_used_instances(&vm.segments), Ok(0)); + assert_eq!(builtin.get_used_instances(&vm.segments), Ok(1)); } #[test] @@ -130,7 +130,7 @@ mod tests { let mut vm = vm!(); vm.segments.segment_used_sizes = Some(vec![3]); - assert_eq!(builtin.get_used_instances(&vm.segments), Ok(0)); + assert_eq!(builtin.get_used_instances(&vm.segments), Ok(1)); } #[test] @@ -154,7 +154,7 @@ mod tests { builtin.final_stack(&vm.segments, pointer), Err(RunnerError::InvalidStopPointer(Box::new(( SEGMENT_ARENA_BUILTIN_NAME, - relocatable!(0, 3), + relocatable!(0, 6), relocatable!(0, 0) )))) ); @@ -245,7 +245,7 @@ mod tests { assert_eq!( builtin.get_used_cells_and_allocated_size(&vm), - Ok((0_usize, 0)) + Ok((3_usize, 3)) ); } @@ -303,7 +303,7 @@ mod tests { let mut vm = vm!(); vm.segments.segment_used_sizes = Some(vec![4]); - assert_eq!(builtin.get_used_cells(&vm.segments), Ok(1)); + assert_eq!(builtin.get_used_cells(&vm.segments), Ok(1 + 3)); } #[test] @@ -325,7 +325,7 @@ mod tests { let mut memory_segment_manager = MemorySegmentManager::new(); memory_segment_manager.segment_used_sizes = Some(vec![6]); // SIZE(6) / CELLS_PER_INSTANCE(3) - assert_eq!(builtin.get_used_instances(&memory_segment_manager), Ok(1)); + assert_eq!(builtin.get_used_instances(&memory_segment_manager), Ok(2)); } #[test]