feat: implement and test deploy syscall ctor stack trace (#1849)

Signed-off-by: Dori Medini <[email protected]>

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,

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<EntryPointSelector>,
+    ) -> EntryPointOffset {
+        let selector =
+            entry_point_selector.unwrap_or(selector_from_name(CONSTRUCTOR_ENTRY_POINT_NAME));
+        self.get_offset(selector, EntryPointType::Constructor)
+    }
 }

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(