357 unhandeled error in sc runtime

src/as_execution/error.rs

@@ -44,7 +44,7 @@ impl From<InterfaceError> for ABIError {
 
 impl From<wasmer::RuntimeError> for ABIError {
     fn from(e: wasmer::RuntimeError) -> Self {
-        ABIError::RuntimeError(e.to_string())
+        ABIError::RuntimeError(crate::error::runtime_error_without_trace(&e))
     }
 }
 

src/error.rs

@@ -1,5 +1,20 @@
 use displaydoc::Display;
 use thiserror::Error;
+use tracing::debug;
+
+/// Format a wasmer runtime error without its wasm backtrace.
+///
+/// The frames are not worth showing to a smart contract developer: they carry no symbol
+/// name (AssemblyScript strips the wasm `name` section in release builds, so wasmer prints
+/// `<unnamed>`), their content depends on which compiler built the module, and a deep call
+/// stack fills the whole event size budget, pushing the actual cause out of the message.
+/// The full error, backtrace included, is still logged node side.
+pub(crate) fn runtime_error_without_trace(e: &wasmer::RuntimeError) -> String {
+    if !e.trace().is_empty() {
+        debug!("wasm backtrace discarded from error message: {}", e);
+    }
+    format!("RuntimeError: {}", e.message())
+}
 
 pub type VMResult<T> = Result<T, VMError>;
 
@@ -27,10 +42,17 @@ impl From<ABIError> for VMError {
 
 impl From<wasmer::RuntimeError> for VMError {
     fn from(e: wasmer::RuntimeError) -> Self {
-        if let Some(err) = e.downcast_ref::<ABIError>() {
-            VMError::DepthError(err.to_string())
-        } else {
-            VMError::InstanceError(e.to_string())
+        // Only a depth error must keep its variant: any other ABI error trapping out of a
+        // host function is a regular failure and must not be reported as a depth error.
+        // The other arms mirror `From<ABIError> for VMError` so that an already formatted
+        // message is not prefixed twice.
+        match e.downcast_ref::<ABIError>() {
+            Some(ABIError::DepthError(err)) => VMError::DepthError(err.clone()),
+            Some(
+                ABIError::VMError(err) | ABIError::RuntimeError(err) | ABIError::SerdeError(err),
+            ) => VMError::InstanceError(err.clone()),
+            Some(ABIError::Error(err)) => VMError::InstanceError(err.to_string()),
+            None => VMError::InstanceError(runtime_error_without_trace(&e)),
         }
     }
 }
@@ -72,3 +94,44 @@ pub(crate) use exec_bail;
 pub(crate) use vm_bail;
 
 use crate::as_execution::ABIError;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A depth error trapping out of a host function must keep its variant and its message.
+    #[test]
+    fn test_depth_error_is_preserved() {
+        let err = wasmer::RuntimeError::user(Box::new(ABIError::DepthError(
+            "recursion depth limit reached".to_string(),
+        )));
+        match VMError::from(err) {
+            VMError::DepthError(msg) => assert_eq!(msg, "recursion depth limit reached"),
+            e => panic!("expected a depth error, got: {e}"),
+        }
+    }
+
+    /// Any other ABI error must not be reported as a depth error, and must not be prefixed twice.
+    #[test]
+    fn test_other_abi_errors_are_not_depth_errors() {
+        let err = wasmer::RuntimeError::user(Box::new(ABIError::VMError(
+            "VM instance error: RuntimeError: unreachable".to_string(),
+        )));
+        match VMError::from(err) {
+            VMError::InstanceError(msg) => {
+                assert_eq!(msg, "VM instance error: RuntimeError: unreachable")
+            }
+            e => panic!("expected an instance error, got: {e}"),
+        }
+    }
+
+    /// A trap that carries no ABI error at all is an instance error.
+    #[test]
+    fn test_plain_trap_is_an_instance_error() {
+        let err = wasmer::RuntimeError::new("unreachable");
+        match VMError::from(err) {
+            VMError::InstanceError(msg) => assert_eq!(msg, "RuntimeError: unreachable"),
+            e => panic!("expected an instance error, got: {e}"),
+        }
+    }
+}

src/tests/tests_runtime.rs

@@ -1556,3 +1556,59 @@ fn test_gas_limit_300ms_pure_wasm() {
         duration.as_millis()
     );
 }
+
+#[test]
+#[serial]
+/// Test that a wasm trap is reported without its wasm backtrace.
+///
+/// The frames are `<unnamed>` for released contracts and a deep call stack fills the whole
+/// event size budget, pushing the actual cause out of the message. See massalabs/massa#4923.
+fn test_trap_error_has_no_backtrace() {
+    let wat = r#"
+        (module
+          (memory (export "memory") 1)
+          (func $deep unreachable)
+          (func $mid call $deep)
+          (func (export "main") (param i32) (result i32)
+            call $mid
+            unreachable)
+          (func (export "__new") (param i32 i32) (result i32) i32.const 0)
+          (func (export "__pin") (param i32) (result i32) i32.const 0)
+          (func (export "__unpin") (param i32))
+          (func (export "__collect")))
+    "#;
+    let bytecode = wasmer::wat2wasm(wat.as_bytes()).unwrap().to_vec();
+
+    // Both compilers are used in production: Cranelift for cached modules, Singlepass for
+    // the uncached path.
+    // Note: whether wasmer populates the wasm trace at all is platform dependent, so this
+    // test is a no-op on targets that never collect frames.
+    for compiler in [Compiler::SP, Compiler::CL] {
+        let gas_costs = GasCosts::default();
+        let condom_limits = CondomLimits::default();
+        let runtime_module = RuntimeModule::new(
+            &bytecode,
+            gas_costs.clone(),
+            compiler,
+            condom_limits.clone(),
+        )
+        .unwrap();
+        let error = run_main(
+            &TestInterface,
+            runtime_module,
+            100_000_000,
+            gas_costs,
+            condom_limits,
+        )
+        .unwrap_err()
+        .to_string();
+
+        println!("error: {}", error);
+        assert!(error.contains("unreachable"), "unexpected error: {}", error);
+        assert!(
+            !error.contains("    at "),
+            "the wasm backtrace leaked into the error message: {}",
+            error
+        );
+    }
+}

src/wasmv1_execution/ffi.rs

@@ -111,7 +111,10 @@ impl Ffi {
         // Deallocate the buffer if there is a dealloc guest function
         if let Some(guest_dealloc_func) = &self.guest_dealloc_func {
             guest_dealloc_func.call(store, offset).map_err(|err| {
-                WasmV1Error::RuntimeError(format!("__dealloc function call failed: {}", err))
+                WasmV1Error::RuntimeError(format!(
+                    "__dealloc function call failed: {}",
+                    crate::error::runtime_error_without_trace(&err)
+                ))
             })?;
         }
         Ok(buffer)
@@ -127,7 +130,10 @@ impl Ffi {
             WasmV1Error::RuntimeError(format!("Could not convert buffer length to i32: {}", err))
         })?;
         let offset: i32 = self.guest_alloc_func.call(store, len).map_err(|err| {
-            WasmV1Error::RuntimeError(format!("__alloc function call failed: {}", err))
+            WasmV1Error::RuntimeError(format!(
+                "__alloc function call failed: {}",
+                crate::error::runtime_error_without_trace(&err)
+            ))
         })?;
         let Ok(offset_u64): Result<u64, _> = offset.try_into() else {
             return Err(WasmV1Error::RuntimeError(format!(

src/wasmv1_execution/mod.rs

@@ -309,7 +309,11 @@ pub(crate) fn exec_wasmv1_module(
         wasm_func
             .call(&mut store, param_offset)
             .map_err(|err| VMError::ExecutionError {
-                error: format!("Error while calling guest function {}: {}", function, err),
+                error: format!(
+                    "Error while calling guest function {}: {}",
+                    function,
+                    crate::error::runtime_error_without_trace(&err)
+                ),
                 init_gas_cost,
             })?;