Skip to content

Commit

Permalink
Revert "fix aot contract executor not passing builtinstats (#849)"
Browse files Browse the repository at this point in the history
This reverts commit aff451c.
  • Loading branch information
pefontana committed Oct 18, 2024
1 parent 4ba9e16 commit e2f895c
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 44 deletions.
3 changes: 0 additions & 3 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
1 change: 1 addition & 0 deletions src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
59 changes: 18 additions & 41 deletions src/executor/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
use crate::{
arch::AbiArgument,
context::NativeContext,
error::{Error, Result},
error::Result,
execution_result::{BuiltinStats, ContractExecutionResult},
executor::invoke_trampoline,
module::NativeModule,
Expand Down Expand Up @@ -77,27 +77,16 @@ pub struct AotContractExecutor {
library: Arc<Library>,
path: PathBuf,
is_temp_path: bool,
contract_info: ContractInfo,
entry_points_info: BTreeMap<u64, EntryPointInfo>,
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
pub struct ContractInfo {
pub version: ContractInfoVersion,
pub entry_points: BTreeMap<u64, EntryPointInfo>,
struct EntryPointInfo {
builtins: Vec<BuiltinType>,
}

#[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<BuiltinType>,
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
pub enum BuiltinType {
enum BuiltinType {
Bitwise,
EcOp,
RangeCheck,
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
})
}

Expand All @@ -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;
Expand All @@ -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<Self> {
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<u64, EntryPointInfo> = 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,
})
}

Expand All @@ -235,23 +218,20 @@ impl AotContractExecutor {
let mut invoke_data = Vec::<u8>::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 {
Expand All @@ -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)?;
Expand Down Expand Up @@ -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::<u128>(return_ptr) };
Expand Down

0 comments on commit e2f895c

Please sign in to comment.