chore: modify call error tracer to return result of errors (#564)

* chore: modify call error tracer to return result of errors then scream at the user * chore: fix lint * Update e2e-tests/.gitignore Co-authored-by: Daniyar Itegulov <[email protected]> * chore: add comment --------- Co-authored-by: Daniyar Itegulov <[email protected]>
matter-labs · Jan 31, 2025 · 2ff051e · 2ff051e
1 parent 94da53c
commit 2ff051e
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 36 deletions.
diff --git a/crates/core/src/node/call_error_tracer.rs b/crates/core/src/node/call_error_tracer.rs
@@ -1,3 +1,5 @@
+use once_cell::sync::OnceCell;
+use std::sync::Arc;
 use zksync_multivm::interface::storage::WriteStorage;
 use zksync_multivm::{
     tracers::dynamic::vm_1_5_0::DynTracer,
@@ -8,11 +10,13 @@ use zksync_multivm::{
     },
 };
 
-pub struct CallErrorTracer {}
+pub struct CallErrorTracer {
+    result: Arc<OnceCell<ErrorFlags>>,
+}
 
 impl CallErrorTracer {
-    pub fn new() -> Self {
-        Self {}
+    pub fn new(result: Arc<OnceCell<ErrorFlags>>) -> Self {
+        Self { result }
     }
 }
 
@@ -23,38 +27,10 @@ impl<S, H: HistoryMode> DynTracer<S, SimpleMemory<H>> for CallErrorTracer {
         data: AfterDecodingData,
         _memory: &SimpleMemory<H>,
     ) {
+        // The top frame is processed last, its error flags
+        // overwrite any previously observed ones.
         if !data.error_flags_accumulated.is_empty() {
-            tracing::error!("!! Got error flags: ");
-            if data
-                .error_flags_accumulated
-                .contains(ErrorFlags::INVALID_OPCODE)
-            {
-                tracing::error!("INVALID OPCODE");
-            }
-            if data
-                .error_flags_accumulated
-                .contains(ErrorFlags::NOT_ENOUGH_ERGS)
-            {
-                tracing::error!("NOT ENOUGH ERGS");
-            }
-            if data
-                .error_flags_accumulated
-                .contains(ErrorFlags::PRIVILAGED_ACCESS_NOT_FROM_KERNEL)
-            {
-                tracing::error!("PRIVILEGED ACCESS");
-            }
-            if data
-                .error_flags_accumulated
-                .contains(ErrorFlags::WRITE_IN_STATIC_CONTEXT)
-            {
-                tracing::error!("WRITE IN STATIC");
-            }
-            if data
-                .error_flags_accumulated
-                .contains(ErrorFlags::CALLSTACK_IS_FULL)
-            {
-                tracing::error!("CALLSTACK IS FULL");
-            }
+            let _ = self.result.set(data.error_flags_accumulated);
         }
     }
 }

diff --git a/crates/core/src/node/in_memory.rs b/crates/core/src/node/in_memory.rs
@@ -390,9 +390,10 @@ impl InMemoryNode {
         delegate_vm!(vm, push_transaction(tx.clone()));
 
         let call_tracer_result = Arc::new(OnceCell::default());
+        let error_flags_result = Arc::new(OnceCell::new());
 
         let tracers = vec![
-            CallErrorTracer::new().into_tracer_pointer(),
+            CallErrorTracer::new(error_flags_result.clone()).into_tracer_pointer(),
             CallTracer::new(call_tracer_result.clone()).into_tracer_pointer(),
         ];
         let tx_result = delegate_vm!(
@@ -404,6 +405,10 @@ impl InMemoryNode {
             .unwrap()
             .take()
             .unwrap_or_default();
+        let error_flags = Arc::try_unwrap(error_flags_result)
+            .unwrap()
+            .take()
+            .unwrap_or_default();
 
         if inner.config.show_tx_summary {
             tracing::info!("");
@@ -414,9 +419,17 @@ impl InMemoryNode {
                     tracing::info!("Output: {}", serde_json::to_string(&output_bytes).unwrap());
                 }
                 ExecutionResult::Revert { output } => {
+                    // TODO: Once we integrate error-codegen avoid printing error flags returned from
+                    // vm_state and rather pass them to error-codegen to get properly formed error message.
+                    // e.g. NOT_ENOUGH_ERGS -> Transaction ran out of gas.
+                    tracing::warn!("Execution flag raised: {:?}", error_flags);
                     tracing::info!("Call: {}: {}", "FAILED".red(), output);
                 }
                 ExecutionResult::Halt { reason } => {
+                    // TODO: Once we integrate error-codegen avoid printing error flags returned from
+                    // vm_state and rather pass them to error-codegen to get properly formed error message.
+                    // e.g. NOT_ENOUGH_ERGS -> Transaction ran out of gas.
+                    tracing::warn!("Execution flag raised: {:?}", error_flags);
                     tracing::info!("Call: {} {}", "HALTED".red(), reason)
                 }
             };

diff --git a/crates/core/src/node/inner/in_memory_inner.rs b/crates/core/src/node/inner/in_memory_inner.rs
@@ -337,9 +337,10 @@ impl InMemoryNodeInner {
 
         let call_tracer_result = Arc::new(OnceCell::default());
         let bootloader_debug_result = Arc::new(OnceCell::default());
+        let error_flags_result = Arc::new(OnceCell::new());
 
         let tracers = vec![
-            CallErrorTracer::new().into_tracer_pointer(),
+            CallErrorTracer::new(error_flags_result.clone()).into_tracer_pointer(),
             CallTracer::new(call_tracer_result.clone()).into_tracer_pointer(),
             BootloaderDebugTracer {
                 result: bootloader_debug_result.clone(),

diff --git a/e2e-tests/.gitignore b/e2e-tests/.gitignore
@@ -115,3 +115,5 @@ dist
 
 # TernJS port file
 .tern-port
+
+e2e-tests/deployments-zk/
Original file line number	Diff line number	Diff line change
Expand Up		@@ -115,3 +115,5 @@ dist

		# TernJS port file
		.tern-port

		e2e-tests/deployments-zk/