diff --git a/.gitmodules b/.gitmodules index 18d6ff26..2607df63 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "cairo-lang"] path = cairo-lang - url = https://github.com/starkware-libs/cairo-lang.git + url = https://github.com/notlesh/cairo-lang ignore = all diff --git a/Cargo.lock b/Cargo.lock index 6979fba4..b7a6da78 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1109,7 +1109,7 @@ dependencies = [ [[package]] name = "cairo-vm" version = "1.0.1" -source = "git+https://github.com/Moonsong-Labs/cairo-vm?branch=notlesh%2Fsnos-2024-11-04#1fa902bafae507424b5ea83a625830ffe6b0eca5" +source = "git+https://github.com/Moonsong-Labs/cairo-vm?branch=herman%2Ffix-pie-serialization#6036ab9533e1754f1612fbea9c9d2a27063ca5f8" dependencies = [ "anyhow", "ark-ff", @@ -2041,7 +2041,7 @@ name = "hint_tool" version = "0.1.0" dependencies = [ "blockifier", - "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=notlesh%2Fsnos-2024-11-04)", + "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=herman%2Ffix-pie-serialization)", "clap", "serde", "serde_json", @@ -2904,7 +2904,7 @@ dependencies = [ "blockifier", "cairo-lang-starknet-classes", "cairo-lang-utils", - "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=notlesh%2Fsnos-2024-11-04)", + "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=herman%2Ffix-pie-serialization)", "clap", "env_logger", "futures-core", @@ -3245,7 +3245,7 @@ dependencies = [ "blockifier", "cairo-lang-starknet-classes", "cairo-lang-utils", - "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=notlesh%2Fsnos-2024-11-04)", + "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=herman%2Ffix-pie-serialization)", "clap", "env_logger", "futures-core", @@ -4436,7 +4436,7 @@ dependencies = [ "cairo-lang-starknet", "cairo-lang-starknet-classes", "cairo-type-derive", - "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=notlesh%2Fsnos-2024-11-04)", + "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=herman%2Ffix-pie-serialization)", "futures", "futures-util", "heck 0.4.1", @@ -4475,7 +4475,7 @@ version = "0.1.0" dependencies = [ "blockifier", "cairo-lang-starknet-classes", - "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=notlesh%2Fsnos-2024-11-04)", + "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=herman%2Ffix-pie-serialization)", "flate2", "indoc", "num-bigint", @@ -4755,7 +4755,7 @@ version = "0.1.0" dependencies = [ "blockifier", "cairo-lang-starknet-classes", - "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=notlesh%2Fsnos-2024-11-04)", + "cairo-vm 1.0.1 (git+https://github.com/Moonsong-Labs/cairo-vm?branch=herman%2Ffix-pie-serialization)", "env_logger", "futures", "log", diff --git a/cairo-lang b/cairo-lang index a86e92bf..7a7c522c 160000 --- a/cairo-lang +++ b/cairo-lang @@ -1 +1 @@ -Subproject commit a86e92bfde9c171c0856d7b46580c66e004922f3 +Subproject commit 7a7c522cdc525c7c97290b7829dca66527545d45 diff --git a/crates/starknet-os/src/execution/syscall_handler.rs b/crates/starknet-os/src/execution/syscall_handler.rs index 3965804d..458675a8 100644 --- a/crates/starknet-os/src/execution/syscall_handler.rs +++ b/crates/starknet-os/src/execution/syscall_handler.rs @@ -247,7 +247,6 @@ pub struct DeployHandler; pub struct DeployResponse { pub contract_address: Felt252, pub constructor_retdata: ReadOnlySegment, - pub need_retdata_hack: bool, } impl SyscallHandler for DeployHandler @@ -292,59 +291,13 @@ where "No more deployed contracts available to replay".to_string().into_boxed_str(), ))?; - let need_retdata_hack = if let Some(os_input) = execution_helper.os_input.as_ref() { - let class_hash = os_input - .contract_address_to_class_hash - .get(&contract_address) - .expect("No class_hash for contract_address"); - let num_constructors = - if let Some(compiled_class_hash) = os_input.class_hash_to_compiled_class_hash.get(class_hash) { - let casm = os_input.compiled_classes.get(compiled_class_hash).expect("No CASM"); - let num_constructors = casm - .get_cairo_lang_contract_class() - .expect("couldn't get cairo lang class") - .entry_points_by_type - .constructor - .len(); - num_constructors - } else { - let deprecated_cc = os_input.deprecated_compiled_classes.get(class_hash).expect("no deprecated CC"); - let num_constructors = deprecated_cc - .get_starknet_api_contract_class() - .expect("couldn't get starknet api class") - .entry_points_by_type - .get(&starknet_api::deprecated_contract_class::EntryPointType::Constructor) - .expect("should have constructor list") - .len(); - num_constructors - }; - - // we need the hack if there are no constructor entry points - num_constructors == 0 - } else { - false - }; - - Ok(DeployResponse { contract_address, constructor_retdata, need_retdata_hack }) + Ok(DeployResponse { contract_address, constructor_retdata }) } fn write_response(response: Self::Response, vm: &mut VirtualMachine, ptr: &mut Relocatable) -> WriteResponseResult { write_felt(vm, ptr, response.contract_address)?; + write_segment(vm, ptr, response.constructor_retdata)?; - // Bugfix: - // When retdata size is 0, the OS will return retdata as a Int(0) instead of a relocatable - // as a result of `cast(0, felt*)`. To work around this, we can write the same here so that - // later the OS will assert this value is the same as retdata; that is, both need to be the - // same value which is Int(0). - // - // retdata is cast here: https://github.com/starkware-libs/cairo-lang/blob/a86e92bfde9c171c0856d7b46580c66e004922f3/src/starkware/starknet/core/os/execution/execute_entry_point.cairo#L170 - if response.need_retdata_hack { - log::warn!("Writing Felt::ZERO instead of pointer since retdata size is 0"); - write_felt(vm, ptr, Felt252::ZERO)?; // casted pointer - write_felt(vm, ptr, Felt252::ZERO)?; // size - } else { - write_segment(vm, ptr, response.constructor_retdata)?; - } Ok(()) } } diff --git a/crates/starknet-os/src/hints/execution.rs b/crates/starknet-os/src/hints/execution.rs index c04d0ec5..9b311c85 100644 --- a/crates/starknet-os/src/hints/execution.rs +++ b/crates/starknet-os/src/hints/execution.rs @@ -6,8 +6,7 @@ use std::vec::IntoIter; use cairo_vm::hint_processor::builtin_hint_processor::dict_manager::Dictionary; use cairo_vm::hint_processor::builtin_hint_processor::hint_utils::{ - get_integer_from_var_name, get_maybe_relocatable_from_var_name, get_ptr_from_var_name, insert_value_from_var_name, - insert_value_into_ap, + get_integer_from_var_name, get_ptr_from_var_name, insert_value_from_var_name, insert_value_into_ap, }; use cairo_vm::hint_processor::hint_processor_definition::HintReference; use cairo_vm::hint_processor::hint_processor_utils::felt_to_usize; @@ -1122,23 +1121,10 @@ pub fn check_new_deploy_response( ) -> Result<(), HintError> { let response_ptr = get_ptr_from_var_name(vars::ids::RESPONSE, vm, ids_data, ap_tracking)?; - // Bugfix: - // If constructor_retdata_start is a Int(0) instead of a Relocatable, we need to do nothing. This - // can happen sometimes when a constructor is called but not defined in the class'es entry - // points. Immediately following this, `add_relocation_rule()` will be called with src=0, dest=0 - // and it will also no-op. - // - // If "retdata" is an Int instead of a Relocatable, then the vm will error when we try to - // request it as such. In this case, there is no retdata anyway, so there is no need to assert - // that the contents are equal. - let retdata_start_key: Relocatable = - (response_ptr + new_syscalls::DeployResponse::constructor_retdata_start_offset())?; - let maybe_retdata_start = vm - .get_maybe(&retdata_start_key) - .ok_or(HintError::VariableNotInScopeError("retdata".to_string().into_boxed_str()))?; - let zero = MaybeRelocatable::Int(Felt252::ZERO); - if maybe_retdata_start == zero { - log::warn!("retdata_start is 0, not a relocatable, ignoring add_relocation_rule()"); + let retdata_size = + felt_to_usize(get_integer_from_var_name(vars::ids::RETDATA_SIZE, vm, ids_data, ap_tracking)?.as_ref())?; + if retdata_size == 0 { + log::warn!("Ignoring empty retdata"); return Ok(()); } @@ -1149,8 +1135,6 @@ pub fn check_new_deploy_response( let response_retdata_size = (constructor_retdata_end - constructor_retdata_start)?; let retdata = get_ptr_from_var_name(vars::ids::RETDATA, vm, ids_data, ap_tracking)?; - let retdata_size = - felt_to_usize(get_integer_from_var_name(vars::ids::RETDATA_SIZE, vm, ids_data, ap_tracking)?.as_ref())?; assert_memory_ranges_equal(vm, constructor_retdata_start, response_retdata_size, retdata, retdata_size)?; @@ -1228,8 +1212,8 @@ pub fn check_response_return_value( ap_tracking: &ApTracking, _constants: &HashMap, ) -> Result<(), HintError> { - let retdata = get_ptr_from_var_name(vars::ids::RETDATA, vm, ids_data, ap_tracking)?; let retdata_size = get_integer_from_var_name(vars::ids::RETDATA_SIZE, vm, ids_data, ap_tracking)?; + let retdata = get_ptr_from_var_name(vars::ids::RETDATA, vm, ids_data, ap_tracking)?; let response = get_ptr_from_var_name(vars::ids::RESPONSE, vm, ids_data, ap_tracking)?; let response_retdata_start = @@ -1289,19 +1273,6 @@ pub fn add_relocation_rule( ap_tracking: &ApTracking, _constants: &HashMap, ) -> Result<(), HintError> { - // Bugfix: - // add_relocation_rule() can be called sometimes with Int(0) instead of Relocatables. This is - // a result of `cast(0, felt*)` being treated as Int(0). We can work around this here by - // ensuring that both src and dest end up being Int(0), and in that case we no-op here. - let src_ptr_maybe = get_maybe_relocatable_from_var_name(vars::ids::SRC_PTR, vm, ids_data, ap_tracking)?; - let dest_ptr_maybe = get_maybe_relocatable_from_var_name(vars::ids::DEST_PTR, vm, ids_data, ap_tracking)?; - - let zero = MaybeRelocatable::Int(Felt252::ZERO); - if src_ptr_maybe == zero && dest_ptr_maybe == zero { - log::warn!("add_relocation_rule with Int(0) => Int(0), doing nothing"); - return Ok(()); - } - let src_ptr = get_ptr_from_var_name(vars::ids::SRC_PTR, vm, ids_data, ap_tracking)?; assert_eq!(src_ptr.offset, 0, "src_ptr offset should be 0");