Improve backtrace error for forked contracts

Closes #3385

commit-id:628a8ce9

Author: ksew1

crates/cheatnet/src/runtime_extensions/forge_runtime_extension/contracts_data.rs

@@ -4,11 +4,14 @@ use bimap::BiMap;
 use camino::Utf8PathBuf;
 use conversions::IntoConv;
 use rayon::iter::{IntoParallelIterator, IntoParallelRefIterator, ParallelIterator};
+use runtime::starknet::constants::TEST_CONTRACT_CLASS_HASH;
 use scarb_api::StarknetContractArtifacts;
 use starknet::core::types::contract::{AbiEntry, SierraClass};
 use starknet::core::utils::get_selector_from_name;
 use starknet_api::core::{ClassHash, EntryPointSelector};
+use starknet_types_core::felt::Felt;
 use std::collections::HashMap;
+use std::ops::Not;
 
 type ContractName = String;
 type FunctionName = String;
@@ -105,6 +108,15 @@ impl ContractsData {
     ) -> Option<&FunctionName> {
         self.selectors.get(entry_point_selector)
     }
+
+    #[must_use]
+    pub fn is_fork_class_hash(&self, class_hash: &ClassHash) -> bool {
+        if class_hash.0 == Felt::from_hex_unchecked(TEST_CONTRACT_CLASS_HASH) {
+            false
+        } else {
+            self.class_hashes.contains_right(class_hash).not()
+        }
+    }
 }
 
 fn build_name_selector_map(abi: Vec<AbiEntry>) -> HashMap<EntryPointSelector, FunctionName> {

crates/forge-runner/src/backtrace/data.rs

@@ -1,4 +1,4 @@
-use crate::backtrace::display::{Backtrace, BacktraceStack};
+use crate::backtrace::display::{Backtrace, render_fork_backtrace, render_local_backtrace};
 use anyhow::Context;
 use anyhow::Result;
 use cairo_annotations::annotations::TryFromDebugInfo;
@@ -19,26 +19,50 @@ use rayon::iter::ParallelIterator;
 use starknet_api::core::ClassHash;
 use std::collections::{HashMap, HashSet};
 
-pub struct ContractBacktraceDataMapping(HashMap<ClassHash, ContractBacktraceData>);
+pub struct ContractBacktraceDataMapping(HashMap<ClassHash, ContractOrigin>);
 
 impl ContractBacktraceDataMapping {
     pub fn new(contracts_data: &ContractsData, class_hashes: HashSet<ClassHash>) -> Result<Self> {
         Ok(Self(
             class_hashes
                 .into_par_iter()
                 .map(|class_hash| {
-                    ContractBacktraceData::new(&class_hash, contracts_data)
+                    ContractOrigin::new(&class_hash, contracts_data)
                         .map(|contract_data| (class_hash, contract_data))
                 })
                 .collect::<Result<_>>()?,
         ))
     }
 
-    pub fn get_backtrace(&self, pc: &[usize], class_hash: &ClassHash) -> Result<BacktraceStack> {
+    pub fn get_backtrace(&self, pcs: &[usize], class_hash: &ClassHash) -> Result<String> {
         self.0
             .get(class_hash)
             .expect("class hash should be present in the data mapping")
-            .backtrace_stack_from(pc)
+            .render_backtrace(pcs)
+    }
+}
+
+enum ContractOrigin {
+    Fork(ClassHash),
+    Local(ContractBacktraceData),
+}
+
+impl ContractOrigin {
+    fn new(class_hash: &ClassHash, contracts_data: &ContractsData) -> Result<Self> {
+        if contracts_data.is_fork_class_hash(class_hash) {
+            Ok(ContractOrigin::Fork(*class_hash))
+        } else {
+            Ok(ContractOrigin::Local(ContractBacktraceData::new(
+                class_hash,
+                contracts_data,
+            )?))
+        }
+    }
+    fn render_backtrace(&self, pcs: &[usize]) -> Result<String> {
+        match self {
+            ContractOrigin::Fork(class_hash) => Ok(render_fork_backtrace(class_hash)),
+            ContractOrigin::Local(data) => data.render_backtrace(pcs),
+        }
     }
 }
 
@@ -156,7 +180,7 @@ impl ContractBacktraceData {
         Ok(stack)
     }
 
-    fn backtrace_stack_from(&self, pcs: &[usize]) -> Result<BacktraceStack> {
+    fn render_backtrace(&self, pcs: &[usize]) -> Result<String> {
         let stack = pcs
             .iter()
             .map(|pc| self.backtrace_from(*pc))
@@ -165,9 +189,6 @@ impl ContractBacktraceData {
 
         let contract_name = &self.contract_name;
 
-        Ok(BacktraceStack {
-            contract_name,
-            stack,
-        })
+        Ok(render_local_backtrace(contract_name, stack))
     }
 }

crates/forge-runner/src/backtrace/display.rs

@@ -1,5 +1,6 @@
 use cairo_annotations::annotations::coverage::{CodeLocation, ColumnNumber, LineNumber};
 use cairo_annotations::annotations::profiler::FunctionName;
+use starknet_api::core::ClassHash;
 use std::fmt;
 use std::fmt::Display;
 
@@ -9,7 +10,7 @@ pub struct Backtrace<'a> {
     pub inlined: bool,
 }
 
-pub struct BacktraceStack<'a> {
+struct BacktraceStack<'a> {
     pub contract_name: &'a str,
     pub stack: Vec<Backtrace<'a>>,
 }
@@ -31,11 +32,26 @@ impl Display for Backtrace<'_> {
 
 impl Display for BacktraceStack<'_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        writeln!(f, "error occurred in contract '{}'", self.contract_name,)?;
+        writeln!(f, "error occurred in contract '{}'", self.contract_name)?;
         writeln!(f, "stack backtrace:")?;
         for (i, backtrace) in self.stack.iter().enumerate() {
             writeln!(f, "   {i}: {backtrace}")?;
         }
         Ok(())
     }
 }
+
+pub fn render_local_backtrace(contract_name: &str, stack: Vec<Backtrace>) -> String {
+    BacktraceStack {
+        contract_name,
+        stack,
+    }
+    .to_string()
+}
+
+pub fn render_fork_backtrace(contract_class_hash: &ClassHash) -> String {
+    format!(
+        "error occurred in contract fork with class hash: {:#x}\n",
+        contract_class_hash.0
+    )
+}

crates/forge-runner/src/backtrace/mod.rs

@@ -18,13 +18,10 @@ pub fn add_backtrace_footer(
         return message;
     }
 
-    if !is_backtrace_enabled() {
-        return format!(
-            "{message}\nnote: run with `{BACKTRACE_ENV}=1` environment variable to display a backtrace",
-        );
-    }
+    let backtrace = is_backtrace_enabled()
+        .then(|| get_backtrace(contracts_data, encountered_errors))
+        .unwrap_or_else(|| format!("{message}\nnote: run with `{BACKTRACE_ENV}=1` environment variable to display a backtrace"));
 
-    let backtrace = get_backtrace(contracts_data, encountered_errors);
     format!("{message}\n{backtrace}")
 }
 
@@ -40,7 +37,6 @@ pub fn get_backtrace(
             encountered_errors
                 .iter()
                 .map(|(class_hash, pcs)| data_mapping.get_backtrace(pcs, class_hash))
-                .map(|backtrace| backtrace.map(|backtrace| backtrace.to_string()))
                 .collect::<Result<Vec<_>>>()
                 .map(|backtrace| backtrace.join("\n"))
         })

crates/forge/tests/data/backtrace_panic/src/lib.cairo

@@ -72,4 +72,21 @@ mod Test {
         let dispatcher = IOuterContractDispatcher { contract_address: contract_address_outer };
         dispatcher.outer(contract_address_inner);
     }
+
+    #[test]
+    #[fork(url: "{{ NODE_RPC_URL }}", block_number: 806134)]
+    fn test_fork_contract_panics() {
+        // NOTE: This is not exactly the same as InnerContract here, but will return the same error
+        // Class hash needs to be different otherwise it would be found locally in the state
+        let contract_address_inner =
+            0x066eda239a01152a912129fe6b5bf309c9b21e3f583df4e5b7ee8ede1fad820a
+            .try_into()
+            .unwrap();
+
+        let contract_outer = declare("OuterContract").unwrap().contract_class();
+        let (contract_address_outer, _) = contract_outer.deploy(@array![]).unwrap();
+
+        let dispatcher = IOuterContractDispatcher { contract_address: contract_address_outer };
+        dispatcher.outer(contract_address_inner);
+    }
 }

crates/forge/tests/data/backtrace_vm_error/src/lib.cairo

@@ -66,4 +66,21 @@ mod Test {
         let dispatcher = IOuterContractDispatcher { contract_address: contract_address_outer };
         dispatcher.outer(contract_address_inner);
     }
+
+    #[test]
+    #[fork(url: "{{ NODE_RPC_URL }}", block_number: 806134)]
+    fn test_fork_unwrapped_call_contract_syscall() {
+        // NOTE: This is not exactly the same as InnerContract here, but will return the same error
+        // Class hash needs to be different otherwise it would be found locally in the state
+        let contract_address_inner =
+            0x01506c04bdb56f2cc9ea1f67fcb086c99df7cec2598ce90e56f1d36fffda1cf4
+            .try_into()
+            .unwrap();
+
+        let contract_outer = declare("OuterContract").unwrap().contract_class();
+        let (contract_address_outer, _) = contract_outer.deploy(@array![]).unwrap();
+
+        let dispatcher = IOuterContractDispatcher { contract_address: contract_address_outer };
+        dispatcher.outer(contract_address_inner);
+    }
 }