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

Add phantom nodes on deploy with no constructor #1965

Merged
merged 12 commits into from
Apr 16, 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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ regex = "1.10.4"
serde = { version = "1.0.197", features = ["derive"] }
serde_json = "1.0.115"
starknet = { git = "https://github.com/xJonathanLEI/starknet-rs", rev = "d980869" }
trace-data = { git = "https://github.com/software-mansion/cairo-profiler/", rev = "3af0782" }
trace-data = { git = "https://github.com/software-mansion/cairo-profiler/", rev = "0b9dafb" }
tempfile = "3.10.1"
thiserror = "1.0.58"
ctor = "0.2.7"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::cell::RefCell;
use super::cairo1_execution::execute_entry_point_call_cairo1;
use crate::runtime_extensions::call_to_blockifier_runtime_extension::execution::deprecated::cairo0_execution::execute_entry_point_call_cairo0;
use crate::runtime_extensions::call_to_blockifier_runtime_extension::CheatnetState;
use crate::state::{CallTrace, CheatStatus};
use crate::state::{CallTrace, CallTraceNode, CheatStatus};
use blockifier::execution::call_info::{CallExecution, Retdata};
use blockifier::{
execution::{
Expand Down Expand Up @@ -220,6 +220,9 @@ pub fn execute_constructor_entry_point(
let contract_class = state.get_compiled_contract_class(ctor_context.class_hash)?;
let Some(constructor_selector) = contract_class.constructor_selector() else {
// Contract has no constructor.
cheatnet_state
.trace_data
.add_deploy_without_constructor_node();
return handle_empty_constructor(ctor_context, calldata, remaining_gas);
};

Expand Down Expand Up @@ -278,18 +281,22 @@ fn mocked_call_info(call: CallEntryPoint, ret_data: Vec<StarkFelt>) -> CallInfo

fn aggregate_nested_syscall_counters(trace: &Rc<RefCell<CallTrace>>) -> SyscallCounter {
let mut result = SyscallCounter::new();
for nested_call in &trace.borrow().nested_calls {
let sub_trace_counter = aggregate_syscall_counters(nested_call);
result = sum_syscall_counters(result, &sub_trace_counter);
for nested_call_node in &trace.borrow().nested_calls {
if let CallTraceNode::EntryPointCall(nested_call) = nested_call_node {
let sub_trace_counter = aggregate_syscall_counters(nested_call);
result = sum_syscall_counters(result, &sub_trace_counter);
}
}
result
}

fn aggregate_syscall_counters(trace: &Rc<RefCell<CallTrace>>) -> SyscallCounter {
let mut result = trace.borrow().used_syscalls.clone();
for nested_call in &trace.borrow().nested_calls {
let sub_trace_counter = aggregate_nested_syscall_counters(nested_call);
result = sum_syscall_counters(result, &sub_trace_counter);
for nested_call_node in &trace.borrow().nested_calls {
if let CallTraceNode::EntryPointCall(nested_call) = nested_call_node {
let sub_trace_counter = aggregate_nested_syscall_counters(nested_call);
result = sum_syscall_counters(result, &sub_trace_counter);
}
}
result
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub enum CallResult {
Failure(CallFailure),
}

/// Enum representing possible call failure and its' type.
/// Enum representing possible call failure and its type.
/// `Panic` - Recoverable, meant to be caught by the user.
/// `Error` - Unrecoverable, equivalent of panic! in rust.
#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use self::contracts_data::ContractsData;
use crate::state::CallTraceNode;
use crate::{
runtime_extensions::{
call_to_blockifier_runtime_extension::{
Expand Down Expand Up @@ -686,10 +687,16 @@ fn handle_deploy_result(
fn serialize_call_trace(call_trace: &CallTrace, output: &mut Vec<Felt252>) {
serialize_call_entry_point(&call_trace.entry_point, output);

output.push(Felt252::from(call_trace.nested_calls.len()));
let visible_calls: Vec<_> = call_trace
.nested_calls
.iter()
.filter_map(CallTraceNode::extract_entry_point_call)
.collect();

output.push(Felt252::from(visible_calls.len()));

for call_trace in &call_trace.nested_calls {
serialize_call_trace(&call_trace.borrow(), output);
for call_trace_node in visible_calls {
serialize_call_trace(&call_trace_node.borrow(), output);
}

serialize_call_result(&call_trace.result, output);
Expand Down Expand Up @@ -737,7 +744,7 @@ fn serialize_call_result(call_result: &CallResult, output: &mut Vec<Felt252>) {
serialize_failure_data(0, panic_data.iter().cloned(), panic_data.len(), output);
}
CallFailure::Error { msg } => {
let data = ByteArray::from(msg.as_str()).serialize_with_magic();
let data = ByteArray::from(msg.as_str()).serialize_no_magic();
piotmag769 marked this conversation as resolved.
Show resolved Hide resolved
let len = data.len();
serialize_failure_data(1, data, len, output);
}
Expand Down Expand Up @@ -820,6 +827,7 @@ pub fn update_top_call_execution_resources(runtime: &mut ForgeRuntime) {
let nested_calls_syscalls = top_call
.nested_calls
.iter()
.filter_map(CallTraceNode::extract_entry_point_call)
.fold(SyscallCounter::new(), |syscalls, trace| {
sum_syscall_counters(syscalls, &trace.borrow().used_syscalls)
});
Expand Down
59 changes: 45 additions & 14 deletions crates/cheatnet/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,43 @@ pub struct CallTrace {
pub used_execution_resources: ExecutionResources,
pub used_l1_resources: L1Resources,
pub used_syscalls: SyscallCounter,
pub nested_calls: Vec<Rc<RefCell<CallTrace>>>,
pub nested_calls: Vec<CallTraceNode>,
pub result: CallResult,
pub vm_trace: Option<Vec<TraceEntry>>,
}

impl CallTrace {
fn default_successful_call() -> Self {
Self {
entry_point: Default::default(),
used_execution_resources: Default::default(),
used_l1_resources: Default::default(),
used_syscalls: Default::default(),
nested_calls: vec![],
result: CallResult::Success { ret_data: vec![] },
vm_trace: None,
}
}
}

/// Enum representing node of a trace of a call.
#[derive(Clone)]
pub enum CallTraceNode {
EntryPointCall(Rc<RefCell<CallTrace>>),
DeployWithoutConstructor,
}

impl CallTraceNode {
#[must_use]
pub fn extract_entry_point_call(&self) -> Option<&Rc<RefCell<CallTrace>>> {
if let CallTraceNode::EntryPointCall(trace) = self {
Some(trace)
} else {
None
}
}
}

#[derive(Clone)]
struct CallStackElement {
// when we exit the call we use it to calculate resources used by the call
Expand Down Expand Up @@ -259,12 +291,7 @@ impl Default for CheatnetState {
test_code_entry_point.class_hash = Some(class_hash!(TEST_CONTRACT_CLASS_HASH));
let test_call = Rc::new(RefCell::new(CallTrace {
entry_point: test_code_entry_point,
used_execution_resources: Default::default(),
used_l1_resources: Default::default(),
used_syscalls: Default::default(),
nested_calls: vec![],
result: CallResult::Success { ret_data: vec![] },
vm_trace: None,
..CallTrace::default_successful_call()
}));
Self {
rolled_contracts: Default::default(),
Expand Down Expand Up @@ -396,19 +423,14 @@ impl TraceData {
) {
let new_call = Rc::new(RefCell::new(CallTrace {
entry_point,
used_execution_resources: Default::default(),
used_l1_resources: Default::default(),
used_syscalls: Default::default(),
nested_calls: vec![],
result: CallResult::Success { ret_data: vec![] },
vm_trace: None,
..CallTrace::default_successful_call()
}));
let current_call = self.current_call_stack.top();

current_call
.borrow_mut()
.nested_calls
.push(new_call.clone());
.push(CallTraceNode::EntryPointCall(new_call.clone()));

self.current_call_stack
.push(new_call, resources_used_before_call, cheated_data);
Expand Down Expand Up @@ -446,6 +468,15 @@ impl TraceData {
last_call.result = result;
last_call.vm_trace = vm_trace;
}

pub fn add_deploy_without_constructor_node(&mut self) {
let current_call = self.current_call_stack.top();

current_call
.borrow_mut()
.nested_calls
.push(CallTraceNode::DeployWithoutConstructor);
}
}

fn get_cheat_for_contract<T: Clone>(
Expand Down
18 changes: 15 additions & 3 deletions crates/forge-runner/src/build_trace_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
use cairo_vm::vm::trace::trace_entry::TraceEntry;
use cheatnet::constants::{TEST_CONTRACT_CLASS_HASH, TEST_ENTRY_POINT_SELECTOR};
use cheatnet::runtime_extensions::forge_runtime_extension::contracts_data::ContractsData;
use cheatnet::state::CallTrace;
use cheatnet::state::{CallTrace, CallTraceNode};
use conversions::IntoConv;
use itertools::Itertools;
use starknet::core::utils::get_selector_from_name;
Expand All @@ -22,7 +22,7 @@ use starknet_api::deprecated_contract_class::EntryPointType;
use starknet_api::hash::StarkHash;
use trace_data::{
CallEntryPoint as ProfilerCallEntryPoint, CallTrace as ProfilerCallTrace,
CallType as ProfilerCallType, ContractAddress,
CallTraceNode as ProfilerCallTraceNode, CallType as ProfilerCallType, ContractAddress,
DeprecatedSyscallSelector as ProfilerDeprecatedSyscallSelector, EntryPointSelector,
EntryPointType as ProfilerEntryPointType, ExecutionResources as ProfilerExecutionResources,
TraceEntry as ProfilerTraceEntry, VmExecutionResources,
Expand Down Expand Up @@ -55,12 +55,24 @@ pub fn build_profiler_call_trace(
nested_calls: value
.nested_calls
.iter()
.map(|c| build_profiler_call_trace(c, contracts_data))
.map(|c| build_profiler_call_trace_node(c, contracts_data))
.collect(),
vm_trace,
}
}

fn build_profiler_call_trace_node(
value: &CallTraceNode,
contracts_data: &ContractsData,
) -> ProfilerCallTraceNode {
match value {
CallTraceNode::EntryPointCall(trace) => {
ProfilerCallTraceNode::EntryPointCall(build_profiler_call_trace(trace, contracts_data))
}
CallTraceNode::DeployWithoutConstructor => ProfilerCallTraceNode::DeployWithoutConstructor,
}
}

#[must_use]
pub fn build_profiler_execution_resources(
execution_resources: ExecutionResources,
Expand Down
2 changes: 1 addition & 1 deletion crates/forge/tests/data/trace/tests/test_trace.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use trace_info::{

#[test]
#[feature("safe_dispatcher")]
fn test_trace_print() {
fn test_trace() {
let sc = declare("SimpleContract").unwrap();

let (contract_address_A, _) = sc.deploy(@array![]).unwrap();
Expand Down
1 change: 1 addition & 0 deletions crates/forge/tests/e2e/build_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::common::runner::{setup_package, test_runner};
use forge_runner::profiler_api::PROFILE_DIR;

#[test]
#[ignore] // TODO(#1991): remove ignore when new profiler is released
piotmag769 marked this conversation as resolved.
Show resolved Hide resolved
fn simple_package_build_profile() {
let temp = setup_package("simple_package");

Expand Down
54 changes: 46 additions & 8 deletions crates/forge/tests/e2e/build_trace_data.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use super::common::runner::{setup_package, test_runner};
use crate::e2e::common::get_trace_from_trace_node;
use forge_runner::build_trace_data::{TEST_CODE_CONTRACT_NAME, TEST_CODE_FUNCTION_NAME, TRACE_DIR};
use std::fs;
use trace_data::CallTrace as ProfilerCallTrace;
use trace_data::{CallTrace as ProfilerCallTrace, CallTraceNode as ProfilerCallTraceNode};

#[test]
fn simple_package_save_trace() {
Expand Down Expand Up @@ -50,7 +51,7 @@ fn trace_has_contract_and_function_names() {

let trace_data = fs::read_to_string(
temp.join(TRACE_DIR)
.join("tests::test_trace::test_trace_print.json"),
.join("tests::test_trace::test_trace.json"),
)
.unwrap();

Expand All @@ -65,7 +66,7 @@ fn trace_has_contract_and_function_names() {
call_trace.entry_point.function_name,
Some(String::from(TEST_CODE_FUNCTION_NAME))
);
assert_contract_and_function_names(&call_trace.nested_calls[0]);
assert_contract_and_function_names(get_trace_from_trace_node(&call_trace.nested_calls[3]));
}

fn assert_contract_and_function_names(trace: &ProfilerCallTrace) {
Expand All @@ -79,8 +80,8 @@ fn assert_contract_and_function_names(trace: &ProfilerCallTrace) {
Some(String::from("execute_calls"))
);

for sub_trace in &trace.nested_calls {
assert_contract_and_function_names(sub_trace);
for sub_trace_node in &trace.nested_calls {
assert_contract_and_function_names(get_trace_from_trace_node(sub_trace_node));
}
}

Expand All @@ -97,7 +98,7 @@ fn trace_has_vm_trace() {

let trace_data = fs::read_to_string(
temp.join(TRACE_DIR)
.join("tests::test_trace::test_trace_print.json"),
.join("tests::test_trace::test_trace.json"),
)
.unwrap();

Expand All @@ -112,7 +113,44 @@ fn assert_vm_trace_exists(trace: &ProfilerCallTrace) {
trace.vm_trace.is_some() || trace.entry_point.function_name == Some(String::from("fail"))
);

for sub_trace in &trace.nested_calls {
assert_vm_trace_exists(sub_trace);
for sub_trace_node in &trace.nested_calls {
if let ProfilerCallTraceNode::EntryPointCall(sub_trace) = sub_trace_node {
assert_vm_trace_exists(sub_trace);
}
}
}

#[test]
fn trace_has_deploy_with_no_constructor_phantom_nodes() {
let temp = setup_package("trace");
let snapbox = test_runner(&temp);

snapbox
.arg("--save-trace-data")
.current_dir(&temp)
.assert()
.success();

let trace_data = fs::read_to_string(
temp.join(TRACE_DIR)
.join("tests::test_trace::test_trace.json"),
)
.unwrap();

let call_trace: ProfilerCallTrace =
serde_json::from_str(&trace_data).expect("Failed to parse call_trace");

// 3 first calls are deploys with empty constructors
matches!(
call_trace.nested_calls[0],
trace_data::CallTraceNode::DeployWithoutConstructor
);
matches!(
call_trace.nested_calls[1],
trace_data::CallTraceNode::DeployWithoutConstructor
);
matches!(
call_trace.nested_calls[2],
trace_data::CallTraceNode::DeployWithoutConstructor
);
}
10 changes: 10 additions & 0 deletions crates/forge/tests/e2e/common/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
use trace_data::{CallTrace as ProfilerCallTrace, CallTraceNode as ProfilerCallTraceNode};

pub mod runner;

pub fn get_trace_from_trace_node(trace_node: &ProfilerCallTraceNode) -> &ProfilerCallTrace {
if let ProfilerCallTraceNode::EntryPointCall(trace) = trace_node {
trace
} else {
panic!("Deploy without constructor node was not expected")
}
}
2 changes: 1 addition & 1 deletion crates/forge/tests/e2e/trace_print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn trace_info_print() {
]
Call Result: Success: []

[PASS] tests::test_trace::test_trace_print (gas: [..]
[PASS] tests::test_trace::test_trace (gas: [..]
Tests: 1 passed, 0 failed, 0 skipped, 0 ignored, 0 filtered out
"},
);
Expand Down
Loading
Loading