diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs index 935faf8cdd..1b4150bfb4 100644 --- a/crates/blockifier/src/execution/errors.rs +++ b/crates/blockifier/src/execution/errors.rs @@ -102,7 +102,7 @@ pub enum EntryPointExecutionError { #[derive(Debug, Error)] pub enum ConstructorEntryPointExecutionError { #[error( - "Error in the contract class {class_hash:?} constructor (selector: \ + "Error in the contract class {class_hash} constructor (selector: \ {constructor_selector:?}, address: {contract_address:?}): {error}" )] ExecutionError { @@ -175,20 +175,26 @@ pub fn gen_transaction_execution_error_trace(error: &TransactionExecutionError) class_hash, storage_address, selector, - } - | TransactionExecutionError::ContractConstructorExecutionFailed( + } => gen_error_trace_from_entry_point_error( + error, + storage_address, + class_hash, + Some(selector), + false, + ), + TransactionExecutionError::ContractConstructorExecutionFailed( ConstructorEntryPointExecutionError::ExecutionError { error, class_hash, contract_address: storage_address, - // TODO(Dori, 5/5/2024): Also handle the no-selector case. - constructor_selector: Some(selector), + constructor_selector, }, ) => gen_error_trace_from_entry_point_error( error, storage_address, class_hash, - Some(selector), + constructor_selector.as_ref(), + true, ), _ => { vec![error.to_string()] @@ -204,16 +210,22 @@ fn gen_error_trace_from_entry_point_error( storage_address: &ContractAddress, class_hash: &ClassHash, entry_point_selector: Option<&EntryPointSelector>, + is_ctor: bool, ) -> ErrorStack { let mut error_stack: ErrorStack = ErrorStack::new(); let depth = 0; - error_stack.push(frame_preamble( - depth, - "Error in the called contract", - storage_address, - class_hash, - entry_point_selector, - )); + let preamble = if is_ctor { + ctor_preamble(depth, storage_address, class_hash, entry_point_selector) + } else { + frame_preamble( + depth, + "Error in the called contract", + storage_address, + class_hash, + entry_point_selector, + ) + }; + error_stack.push(preamble); extract_entry_point_execution_error_into_stack_trace(&mut error_stack, depth + 1, error); error_stack } @@ -306,6 +318,21 @@ fn frame_preamble( ) } +fn ctor_preamble( + depth: usize, + storage_address: &ContractAddress, + class_hash: &ClassHash, + selector: Option<&EntryPointSelector>, +) -> String { + frame_preamble( + depth, + "Error in the contract class constructor", + storage_address, + class_hash, + selector, + ) +} + fn call_contract_preamble( depth: usize, storage_address: &ContractAddress, @@ -354,6 +381,26 @@ fn extract_syscall_execution_error_into_stack_trace( error_stack.push(library_call_preamble(depth, storage_address, class_hash, selector)); extract_syscall_execution_error_into_stack_trace(error_stack, depth + 1, error); } + SyscallExecutionError::ConstructorEntryPointExecutionError( + ConstructorEntryPointExecutionError::ExecutionError { + error: entry_point_error, + class_hash, + contract_address, + constructor_selector, + }, + ) => { + error_stack.push(ctor_preamble( + depth, + contract_address, + class_hash, + constructor_selector.as_ref(), + )); + extract_entry_point_execution_error_into_stack_trace( + error_stack, + depth, + entry_point_error, + ) + } SyscallExecutionError::EntryPointExecutionError(entry_point_error) => { extract_entry_point_execution_error_into_stack_trace( error_stack, @@ -399,6 +446,26 @@ fn extract_deprecated_syscall_execution_error_into_stack_trace( error, ) } + DeprecatedSyscallExecutionError::ConstructorEntryPointExecutionError( + ConstructorEntryPointExecutionError::ExecutionError { + error: entry_point_error, + class_hash, + contract_address, + constructor_selector, + }, + ) => { + error_stack.push(ctor_preamble( + depth, + contract_address, + class_hash, + constructor_selector.as_ref(), + )); + extract_entry_point_execution_error_into_stack_trace( + error_stack, + depth, + entry_point_error, + ) + } DeprecatedSyscallExecutionError::EntryPointExecutionError(entry_point_error) => { extract_entry_point_execution_error_into_stack_trace( error_stack, diff --git a/crates/blockifier/src/test_utils/contracts.rs b/crates/blockifier/src/test_utils/contracts.rs index 6df2b1eaa1..7a69636d74 100644 --- a/crates/blockifier/src/test_utils/contracts.rs +++ b/crates/blockifier/src/test_utils/contracts.rs @@ -5,6 +5,8 @@ use starknet_api::deprecated_contract_class::{ use starknet_api::hash::StarkHash; use starknet_api::{class_hash, contract_address, patricia_key}; +use crate::abi::abi_utils::selector_from_name; +use crate::abi::constants::CONSTRUCTOR_ENTRY_POINT_NAME; use crate::execution::contract_class::{ContractClass, ContractClassV0, ContractClassV1}; use crate::test_utils::{get_raw_contract_class, CairoVersion}; @@ -182,15 +184,16 @@ impl FeatureContract { /// Fetch PC locations from the compiled contract to compute the expected PC locations in the /// traceback. Computation is not robust, but as long as the cairo function itself is not /// edited, this computation should be stable. - pub fn get_entry_point_offset( + fn get_offset( &self, entry_point_selector: EntryPointSelector, + entry_point_type: EntryPointType, ) -> EntryPointOffset { match self.get_class() { ContractClass::V0(class) => { class .entry_points_by_type - .get(&EntryPointType::External) + .get(&entry_point_type) .unwrap() .iter() .find(|ep| ep.selector == entry_point_selector) @@ -200,7 +203,7 @@ impl FeatureContract { ContractClass::V1(class) => { class .entry_points_by_type - .get(&EntryPointType::External) + .get(&entry_point_type) .unwrap() .iter() .find(|ep| ep.selector == entry_point_selector) @@ -209,4 +212,20 @@ impl FeatureContract { } } } + + pub fn get_entry_point_offset( + &self, + entry_point_selector: EntryPointSelector, + ) -> EntryPointOffset { + self.get_offset(entry_point_selector, EntryPointType::External) + } + + pub fn get_ctor_offset( + &self, + entry_point_selector: Option, + ) -> EntryPointOffset { + let selector = + entry_point_selector.unwrap_or(selector_from_name(CONSTRUCTOR_ENTRY_POINT_NAME)); + self.get_offset(selector, EntryPointType::Constructor) + } } diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index e4200a3b43..bb944cb45c 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -5,18 +5,21 @@ use cairo_felt::Felt252; use cairo_vm::vm::runners::cairo_runner::ResourceTracker; use pretty_assertions::assert_eq; use rstest::rstest; -use starknet_api::core::{calculate_contract_address, ClassHash, ContractAddress, PatriciaKey}; +use starknet_api::core::{ + calculate_contract_address, ClassHash, ContractAddress, Nonce, PatriciaKey, +}; use starknet_api::hash::{StarkFelt, StarkHash}; use starknet_api::state::StorageKey; use starknet_api::transaction::{ Calldata, ContractAddressSalt, DeclareTransactionV2, Fee, ResourceBoundsMapping, - TransactionHash, TransactionVersion, + TransactionHash, TransactionSignature, TransactionVersion, }; use starknet_api::{calldata, class_hash, contract_address, patricia_key, stark_felt}; use crate::abi::abi_utils::{ get_fee_token_var_address, get_storage_var_address, selector_from_name, }; +use crate::abi::constants::CONSTRUCTOR_ENTRY_POINT_NAME; use crate::context::BlockContext; use crate::execution::contract_class::{ContractClass, ContractClassV1}; use crate::execution::entry_point::EntryPointExecutionContext; @@ -37,7 +40,10 @@ use crate::test_utils::{ MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE, }; use crate::transaction::account_transaction::AccountTransaction; -use crate::transaction::constants::TRANSFER_ENTRY_POINT_NAME; +use crate::transaction::constants::{ + DEPLOY_CONTRACT_FUNCTION_ENTRY_POINT_NAME, EXECUTE_ENTRY_POINT_NAME, FELT_TRUE, + TRANSFER_ENTRY_POINT_NAME, +}; use crate::transaction::objects::{FeeType, HasRelatedFeeType, TransactionInfoCreator}; use crate::transaction::test_utils::{ account_invoke_tx, block_context, calculate_class_info_for_testing, @@ -505,12 +511,12 @@ fn test_account_ctor_frame_stack_trace( }; fund_account(chain_info, deploy_address, BALANCE * 2, &mut state.state); - let expected_selector = selector_from_name("constructor").0; + let expected_selector = selector_from_name(CONSTRUCTOR_ENTRY_POINT_NAME).0; let expected_address = deploy_address.0.key(); let expected_error = format!( "Contract constructor execution has failed: -0: Error in the called contract (contract address: {expected_address}, class hash: {class_hash}, \ - selector: {expected_selector}): +0: Error in the contract class constructor (contract address: {expected_address}, class hash: \ + {class_hash}, selector: {expected_selector}): " ) + match cairo_version { CairoVersion::Cairo0 => { @@ -534,6 +540,140 @@ An ASSERT_EQ instruction failed: 1 != 0. assert_eq!(error.to_string(), expected_error); } +#[rstest] +/// Tests that hitting an execution error in an contract constructor during a deploy syscall outputs +/// the correct traceback (including correct class hash, contract address and constructor entry +/// point selector). +fn test_contract_ctor_frame_stack_trace( + block_context: BlockContext, + #[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion, +) { + let chain_info = &block_context.chain_info; + let account = FeatureContract::AccountWithoutValidations(cairo_version); + let faulty_ctor = FeatureContract::FaultyAccount(cairo_version); + // Declare both classes, but only "deploy" the dummy account. + let state = &mut test_state(chain_info, BALANCE, &[(account, 1), (faulty_ctor, 0)]); + let account_address = account.get_instance_address(0); + let account_class_hash = account.get_class_hash(); + let faulty_class_hash = faulty_ctor.get_class_hash(); + + let salt = stark_felt!(7_u8); + // Constructor arg: set to true to fail deployment. + let validate_constructor = stark_felt!(FELT_TRUE); + let signature = TransactionSignature(vec![stark_felt!(INVALID)]); + let expected_deployed_address = calculate_contract_address( + ContractAddressSalt(salt), + faulty_class_hash, + &calldata![validate_constructor], + account_address, + ) + .unwrap(); + + // Invoke the deploy_contract function on the dummy account to deploy the faulty contract. + let invoke_deploy_tx = account_invoke_tx(invoke_tx_args! { + max_fee: Fee(BALANCE), + sender_address: account_address, + signature, + calldata: create_calldata( + account_address, + DEPLOY_CONTRACT_FUNCTION_ENTRY_POINT_NAME, + &[ + faulty_class_hash.0, + salt, + stark_felt!(1_u8), // Calldata: ctor args length. + validate_constructor, + ] + ), + version: TransactionVersion::ONE, + nonce: Nonce(stark_felt!(0_u8)), + }); + + // Construct expected output. + let execute_selector = selector_from_name(EXECUTE_ENTRY_POINT_NAME); + let deploy_contract_selector = selector_from_name(DEPLOY_CONTRACT_FUNCTION_ENTRY_POINT_NAME); + let ctor_selector = selector_from_name(CONSTRUCTOR_ENTRY_POINT_NAME); + let account_address_felt = *account_address.0.key(); + let faulty_class_hash = faulty_ctor.get_class_hash(); + let expected_address = expected_deployed_address.0.key(); + + let (frame_0, frame_1, frame_2) = ( + format!( + "Transaction execution has failed: +0: Error in the called contract (contract address: {account_address_felt}, class hash: \ + {account_class_hash}, selector: {}):", + execute_selector.0 + ), + format!( + "1: Error in the called contract (contract address: {account_address_felt}, class \ + hash: {account_class_hash}, selector: {}):", + deploy_contract_selector.0 + ), + format!( + "2: Error in the contract class constructor (contract address: {expected_address}, \ + class hash: {faulty_class_hash}, selector: {}):", + ctor_selector.0 + ), + ); + let (execute_offset, deploy_offset, ctor_offset) = ( + account.get_entry_point_offset(execute_selector).0, + account.get_entry_point_offset(deploy_contract_selector).0, + faulty_ctor.get_ctor_offset(Some(ctor_selector)).0, + ); + + let expected_error = match cairo_version { + CairoVersion::Cairo0 => { + format!( + "{frame_0} +Error at pc=0:7: +Cairo traceback (most recent call last): +Unknown location (pc=0:{}) +Unknown location (pc=0:{}) + +{frame_1} +Error at pc=0:20: +Cairo traceback (most recent call last): +Unknown location (pc=0:{}) +Unknown location (pc=0:{}) + +{frame_2} +Error at pc=0:223: +Cairo traceback (most recent call last): +Unknown location (pc=0:{}) +Unknown location (pc=0:{}) + +An ASSERT_EQ instruction failed: 1 != 0. +", + execute_offset + 18, + execute_offset - 8, + deploy_offset + 14, + deploy_offset - 12, + ctor_offset + 7, + ctor_offset - 9 + ) + } + CairoVersion::Cairo1 => { + // TODO(Dori, 1/1/2025): Get lowest level PC locations from Cairo1 errors (ctor offset + // does not appear in the trace). + format!( + "{frame_0} +Error at pc=0:{}: +{frame_1} +Error at pc=0:{}: +{frame_2} +Execution failed. Failure reason: 0x496e76616c6964207363656e6172696f ('Invalid scenario'). +", + execute_offset + 205, + deploy_offset + 194 + ) + } + }; + + // Compare expected and actual error. + let error = + invoke_deploy_tx.execute(state, &block_context, true, true).unwrap().revert_error.unwrap(); + assert_eq!(error.to_string(), expected_error); +} + #[rstest] /// Tests that failing account deployment should not change state (no fee charge or nonce bump). fn test_fail_deploy_account( diff --git a/crates/blockifier/src/transaction/constants.rs b/crates/blockifier/src/transaction/constants.rs index c7e185b682..16b5d8cb6c 100644 --- a/crates/blockifier/src/transaction/constants.rs +++ b/crates/blockifier/src/transaction/constants.rs @@ -3,6 +3,7 @@ pub const TRANSFER_ENTRY_POINT_NAME: &str = "transfer"; pub const VALIDATE_ENTRY_POINT_NAME: &str = "__validate__"; pub const VALIDATE_DECLARE_ENTRY_POINT_NAME: &str = "__validate_declare__"; pub const VALIDATE_DEPLOY_ENTRY_POINT_NAME: &str = "__validate_deploy__"; +pub const DEPLOY_CONTRACT_FUNCTION_ENTRY_POINT_NAME: &str = "deploy_contract"; pub const TRANSFER_EVENT_NAME: &str = "Transfer";