fix: check actor error codes (#1039)

This commit:

1. Extracts the actor code checking to methods on the ActorError itself.
2. Uses them in the EVM.

Co-authored-by: vyzo <[email protected]>

actors/evm/src/interpreter/system.rs

@@ -226,17 +226,21 @@ impl<'r, RT: Runtime> System<'r, RT> {
             .rt
             .send_generalized(to, method, params, value, gas_limit, send_flags)
             .map_err(|err| actor_error!(unspecified; "send syscall failed: {}", err))?;
+
+        // Don't bother reloading on abort, just return the error.
+        if !result.exit_code.is_success() {
+            return Err(ActorError::checked_with_data(
+                result.exit_code,
+                format!("failed to call {to} on method {method}"),
+                result.return_data,
+            ));
+        }
+
         if !send_flags.read_only() {
             self.reload()?;
         }
-        match result.exit_code {
-            ExitCode::OK => Ok(result.return_data),
-            e => Err(ActorError::unchecked_with_data(
-                e,
-                "send failed".to_string(),
-                result.return_data,
-            )),
-        }
+
+        Ok(result.return_data)
     }
 
     /// Flush the actor state (bytecode, nonce, and slots).

runtime/src/actor_error.rs

@@ -29,6 +29,40 @@ impl ActorError {
         Self { exit_code: code, msg, data }
     }
 
+    /// Creates a new ActorError. This method checks if the exit code is within the allowed range,
+    /// and automatically converts it into a user code.
+    pub fn checked(code: ExitCode, msg: String) -> Self {
+        let exit_code = match code {
+            // This means the called actor did something wrong. We can't "make up" a
+            // reasonable exit code.
+            ExitCode::SYS_MISSING_RETURN
+            | ExitCode::SYS_ILLEGAL_INSTRUCTION
+            | ExitCode::SYS_ILLEGAL_EXIT_CODE => ExitCode::USR_UNSPECIFIED,
+            // We don't expect any other system errors.
+            code if code.is_system_error() => ExitCode::USR_ASSERTION_FAILED,
+            // Otherwise, pass it through.
+            code => code,
+        };
+        Self { exit_code, msg, data: None }
+    }
+
+    /// Creates a new ActorError. This method checks if the exit code is within the allowed range,
+    /// and automatically converts it into a user code.
+    pub fn checked_with_data(code: ExitCode, msg: String, data: Option<IpldBlock>) -> Self {
+        let exit_code = match code {
+            // This means the called actor did something wrong. We can't "make up" a
+            // reasonable exit code.
+            ExitCode::SYS_MISSING_RETURN
+            | ExitCode::SYS_ILLEGAL_INSTRUCTION
+            | ExitCode::SYS_ILLEGAL_EXIT_CODE => ExitCode::USR_UNSPECIFIED,
+            // We don't expect any other system errors.
+            code if code.is_system_error() => ExitCode::USR_ASSERTION_FAILED,
+            // Otherwise, pass it through.
+            code => code,
+        };
+        Self { exit_code, msg, data }
+    }
+
     pub fn illegal_argument(msg: String) -> Self {
         Self { exit_code: ExitCode::USR_ILLEGAL_ARGUMENT, msg, data: None }
     }

runtime/src/runtime/mod.rs

@@ -203,22 +203,8 @@ pub trait Runtime: Primitives + Verifier + RuntimePolicy {
                 if ret.exit_code.is_success() {
                     Ok(ret.return_data)
                 } else {
-                    // The returned code can't be simply propagated as it may be a system exit code.
-                    // TODO: improve propagation once we return a RuntimeError.
-                    // Ref https://github.com/filecoin-project/builtin-actors/issues/144
-                    let exit_code = match ret.exit_code {
-                        // This means the called actor did something wrong. We can't "make up" a
-                        // reasonable exit code.
-                        ExitCode::SYS_MISSING_RETURN
-                        | ExitCode::SYS_ILLEGAL_INSTRUCTION
-                        | ExitCode::SYS_ILLEGAL_EXIT_CODE => ExitCode::USR_UNSPECIFIED,
-                        // We don't expect any other system errors.
-                        code if code.is_system_error() => ExitCode::USR_ASSERTION_FAILED,
-                        // Otherwise, pass it through.
-                        code => code,
-                    };
-                    Err(ActorError::unchecked_with_data(
-                        exit_code,
+                    Err(ActorError::checked_with_data(
+                        ret.exit_code,
                         format!(
                             "send to {} method {} aborted with code {}",
                             to, method, ret.exit_code