Address review

runtime/src/actor_error.rs

@@ -12,10 +12,10 @@ pub struct ActorError {
     /// The exit code for this invocation.
     /// Codes less than `FIRST_USER_EXIT_CODE` are prohibited and will be overwritten by the VM.
     exit_code: ExitCode,
-    /// Message for debugging purposes,
-    msg: String,
     /// Optional exit data
     data: Option<IpldBlock>,
+    /// Message for debugging purposes,
+    msg: String,
 }
 
 impl ActorError {
@@ -31,24 +31,7 @@ impl ActorError {
 
     /// 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 {
+    pub fn checked(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.
@@ -101,11 +84,6 @@ impl ActorError {
         &self.msg
     }
 
-    /// Returns the optional data that might be associated with the error
-    pub fn data(&self) -> &[u8] {
-        self.data.as_ref().map_or(&[] as &[u8], |d| d.data.as_slice())
-    }
-
     /// Extracts the optional associated data without copying.
     pub fn take_data(&mut self) -> Option<IpldBlock> {
         std::mem::take(&mut self.data)

runtime/src/runtime/fvm.rs

@@ -294,26 +294,13 @@ where
                 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(
-                        exit_code,
+                    Err(ActorError::checked(
+                        ret.exit_code,
                         format!(
                             "send to {} method {} aborted with code {}",
                             to, method, ret.exit_code
                         ),
+                        ret.return_data,
                     ))
                 }
             }