From e2f895c707acf0b5c74fe42eb3d076f4cebfe4f2 Mon Sep 17 00:00:00 2001 From: pefontana Date: Fri, 18 Oct 2024 18:53:13 -0300 Subject: [PATCH] Revert "fix aot contract executor not passing builtinstats (#849)" This reverts commit aff451c4821fb593eced61a418987a95d3410524. --- src/error.rs | 3 -- src/executor.rs | 1 + src/executor/contract.rs | 59 ++++++++++++---------------------------- 3 files changed, 19 insertions(+), 44 deletions(-) diff --git a/src/error.rs b/src/error.rs index 2cc503d07..c58b2fa57 100644 --- a/src/error.rs +++ b/src/error.rs @@ -69,9 +69,6 @@ pub enum Error { #[error("integer conversion failed")] IntegerConversion, - #[error("missing BuiltinCosts global symbol, should never happen, this is a bug")] - MissingBuiltinCostsSymbol, - #[error(transparent)] IoError(#[from] std::io::Error), diff --git a/src/executor.rs b/src/executor.rs index f88e87735..839d50300 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -177,6 +177,7 @@ fn invoke_dynamic( .to_bytes(&mut invoke_data)?; } CoreTypeConcrete::BuiltinCosts(_) => { + // This builtin should never be an argument but just in case. if let Some(builtin_costs_ptr) = builtin_costs_ptr { builtin_costs_ptr.to_bytes(&mut invoke_data)?; } else { diff --git a/src/executor/contract.rs b/src/executor/contract.rs index 31f9f0b01..94ffc9cfa 100644 --- a/src/executor/contract.rs +++ b/src/executor/contract.rs @@ -34,7 +34,7 @@ use crate::{ arch::AbiArgument, context::NativeContext, - error::{Error, Result}, + error::Result, execution_result::{BuiltinStats, ContractExecutionResult}, executor::invoke_trampoline, module::NativeModule, @@ -77,27 +77,16 @@ pub struct AotContractExecutor { library: Arc, path: PathBuf, is_temp_path: bool, - contract_info: ContractInfo, + entry_points_info: BTreeMap, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub struct ContractInfo { - pub version: ContractInfoVersion, - pub entry_points: BTreeMap, +struct EntryPointInfo { + builtins: Vec, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub enum ContractInfoVersion { - Version0, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub struct EntryPointInfo { - pub builtins: Vec, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] -pub enum BuiltinType { +enum BuiltinType { Bitwise, EcOp, RangeCheck, @@ -147,9 +136,6 @@ impl AotContractExecutor { CoreTypeConcrete::RangeCheck(_) => builtins.push(BuiltinType::RangeCheck), CoreTypeConcrete::Pedersen(_) => builtins.push(BuiltinType::Pedersen), CoreTypeConcrete::Poseidon(_) => builtins.push(BuiltinType::Poseidon), - CoreTypeConcrete::BuiltinCosts(_) => { - builtins.push(BuiltinType::BuiltinCosts) - } CoreTypeConcrete::SegmentArena(_) => { builtins.push(BuiltinType::SegmentArena) } @@ -188,10 +174,7 @@ impl AotContractExecutor { library: Arc::new(unsafe { Library::new(&library_path)? }), path: library_path, is_temp_path: true, - contract_info: ContractInfo { - version: ContractInfoVersion::Version0, - entry_points: infos, - }, + entry_points_info: infos, }) } @@ -200,9 +183,9 @@ impl AotContractExecutor { let to = to.as_ref(); std::fs::copy(&self.path, to)?; - let contract_info = serde_json::to_string(&self.contract_info)?; + let info = serde_json::to_string(&self.entry_points_info)?; let path = to.with_extension("json"); - std::fs::write(path, contract_info)?; + std::fs::write(path, info)?; self.path = to.to_path_buf(); self.is_temp_path = false; @@ -213,12 +196,12 @@ impl AotContractExecutor { /// Load the executor from an already compiled library with the additional info json file. pub fn load(library_path: &Path) -> Result { let info_str = std::fs::read_to_string(library_path.with_extension("json"))?; - let contract_info: ContractInfo = serde_json::from_str(&info_str)?; + let info: BTreeMap = serde_json::from_str(&info_str)?; Ok(Self { library: Arc::new(unsafe { Library::new(library_path)? }), path: library_path.to_path_buf(), is_temp_path: false, - contract_info, + entry_points_info: info, }) } @@ -235,23 +218,20 @@ impl AotContractExecutor { let mut invoke_data = Vec::::new(); let function_ptr = self.find_function_ptr(function_id, true)?; - let builtin_costs_ptr = self - .find_symbol_ptr("builtin_costs") - .ok_or_else(|| Error::MissingBuiltinCostsSymbol)?; + let builtin_costs_ptr = self.find_symbol_ptr("builtin_costs"); let builtin_costs = builtin_costs.unwrap_or_default(); let builtin_costs: [u64; 7] = builtin_costs.into(); - unsafe { - *builtin_costs_ptr.cast() = builtin_costs.as_ptr(); + if let Some(builtin_costs_ptr) = builtin_costs_ptr { + unsafe { + *builtin_costs_ptr.cast() = builtin_costs.as_ptr(); + } } // it can vary from contract to contract thats why we need to store/ load it. // substract 2, which are the gas and syscall builtin - let num_builtins = self.contract_info.entry_points[&function_id.id] - .builtins - .len() - - 2; + let num_builtins = self.entry_points_info[&function_id.id].builtins.len() - 2; // There is always a return ptr because contracts always return more than 1 thing (builtin counters, syscall, enum) let return_ptr = arena.alloc_layout(unsafe { @@ -264,15 +244,12 @@ impl AotContractExecutor { let mut syscall_handler = StarknetSyscallHandlerCallbacks::new(&mut syscall_handler); - for b in &self.contract_info.entry_points[&function_id.id].builtins { + for b in &self.entry_points_info[&function_id.id].builtins { match b { BuiltinType::Gas => { let gas = gas.unwrap_or(0); gas.to_bytes(&mut invoke_data)?; } - BuiltinType::BuiltinCosts => { - builtin_costs_ptr.to_bytes(&mut invoke_data)?; - } BuiltinType::System => { (&mut syscall_handler as *mut StarknetSyscallHandlerCallbacks<_>) .to_bytes(&mut invoke_data)?; @@ -347,7 +324,7 @@ impl AotContractExecutor { let return_ptr = &mut return_ptr.cast(); - for b in &self.contract_info.entry_points[&function_id.id].builtins { + for b in &self.entry_points_info[&function_id.id].builtins { match b { BuiltinType::Gas => { remaining_gas = unsafe { *read_value::(return_ptr) };