Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix aot contract executor not passing builtinstats #849

Merged
merged 9 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ 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: 0 additions & 1 deletion src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ fn invoke_dynamic(
.to_bytes(&mut invoke_data)?;
}
CoreTypeConcrete::BuiltinCosts(_) => {
// This builtin should never be an argument but just in case.
edg-l marked this conversation as resolved.
Show resolved Hide resolved
if let Some(builtin_costs_ptr) = builtin_costs_ptr {
builtin_costs_ptr.to_bytes(&mut invoke_data)?;
} else {
Expand Down
59 changes: 41 additions & 18 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::Result,
error::{Error, Result},
execution_result::{BuiltinStats, ContractExecutionResult},
executor::invoke_trampoline,
module::NativeModule,
Expand Down Expand Up @@ -77,16 +77,27 @@ pub struct AotContractExecutor {
library: Arc<Library>,
path: PathBuf,
is_temp_path: bool,
entry_points_info: BTreeMap<u64, EntryPointInfo>,
contract_info: ContractInfo,
}

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

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
enum BuiltinType {
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 {
Bitwise,
EcOp,
RangeCheck,
Expand Down Expand Up @@ -136,6 +147,9 @@ 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 @@ -174,7 +188,10 @@ impl AotContractExecutor {
library: Arc::new(unsafe { Library::new(&library_path)? }),
path: library_path,
is_temp_path: true,
entry_points_info: infos,
contract_info: ContractInfo {
version: ContractInfoVersion::Version0,
entry_points: infos,
},
})
}

Expand All @@ -183,9 +200,9 @@ impl AotContractExecutor {
let to = to.as_ref();
std::fs::copy(&self.path, to)?;

let info = serde_json::to_string(&self.entry_points_info)?;
let contract_info = serde_json::to_string(&self.contract_info)?;
let path = to.with_extension("json");
std::fs::write(path, info)?;
std::fs::write(path, contract_info)?;

self.path = to.to_path_buf();
self.is_temp_path = false;
Expand All @@ -196,12 +213,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 info: BTreeMap<u64, EntryPointInfo> = serde_json::from_str(&info_str)?;
let contract_info: ContractInfo = 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,
entry_points_info: info,
contract_info,
})
}

Expand All @@ -218,20 +235,23 @@ 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");
let builtin_costs_ptr = self
.find_symbol_ptr("builtin_costs")
.ok_or_else(|| Error::MissingBuiltinCostsSymbol)?;

let builtin_costs = builtin_costs.unwrap_or_default();
let builtin_costs: [u64; 7] = builtin_costs.into();

if let Some(builtin_costs_ptr) = builtin_costs_ptr {
unsafe {
*builtin_costs_ptr.cast() = builtin_costs.as_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.entry_points_info[&function_id.id].builtins.len() - 2;
let num_builtins = self.contract_info.entry_points[&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 @@ -244,12 +264,15 @@ impl AotContractExecutor {

let mut syscall_handler = StarknetSyscallHandlerCallbacks::new(&mut syscall_handler);

for b in &self.entry_points_info[&function_id.id].builtins {
for b in &self.contract_info.entry_points[&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 @@ -324,7 +347,7 @@ impl AotContractExecutor {

let return_ptr = &mut return_ptr.cast();

for b in &self.entry_points_info[&function_id.id].builtins {
for b in &self.contract_info.entry_points[&function_id.id].builtins {
match b {
BuiltinType::Gas => {
remaining_gas = unsafe { *read_value::<u128>(return_ptr) };
Expand Down
Loading