Update Actors error handling

blockchain/chain/src/store/errors.rs

@@ -33,8 +33,8 @@ pub enum Error {
     #[error(transparent)]
     Cid(#[from] CidErr),
     /// Amt error
-    #[error(transparent)]
-    Amt(#[from] AmtErr),
+    #[error("State error: {0}")]
+    State(String),
     /// Other chain error
     #[error("{0}")]
     Other(String),
@@ -52,6 +52,12 @@ impl From<SerdeErr> for Error {
     }
 }
 
+impl From<AmtErr> for Error {
+    fn from(e: AmtErr) -> Error {
+        Error::State(e.to_string())
+    }
+}
+
 impl From<String> for Error {
     fn from(e: String) -> Self {
         Error::Other(e)

blockchain/state_manager/src/lib.rs

@@ -845,5 +845,6 @@ where
             .load_actor_state(&*INIT_ACTOR_ADDR, ts.parent_state())
             .map_err(|e| format!("loading power actor state: {}", e))?;
         ps.miner_nominal_power_meets_consensus_minimum(self.blockstore(), addr)
+            .map_err(|e| e.to_string())
     }
 }

encoding/src/errors.rs

@@ -7,59 +7,38 @@ use std::fmt;
 use std::io;
 use thiserror::Error;
 
-/// Error type for encoding and decoding data through any Forest supported protocol
+/// Error type for encoding and decoding data through any Forest supported protocol.
 ///
 /// This error will provide any details about the data which was attempted to be
-/// encoded or decoded. The
-///
-/// Usage:
-/// ```no_run
-/// use forest_encoding::{Error, CodecProtocol};
-///
-/// Error::Marshalling {
-///     description: format!("{:?}", vec![0]),
-///     protocol: CodecProtocol::Cbor,
-/// };
-/// Error::Unmarshalling {
-///     description: format!("{:?}", vec![0]),
-///     protocol: CodecProtocol::Cbor,
-/// };
-/// ```
+/// encoded or decoded.
 #[derive(Debug, PartialEq, Error)]
-pub enum Error {
-    #[error("Could not decode in format {protocol}: {description}")]
-    Unmarshalling {
-        description: String,
-        protocol: CodecProtocol,
-    },
-    #[error("Could not encode in format {protocol}: {description}")]
-    Marshalling {
-        description: String,
-        protocol: CodecProtocol,
-    },
+#[error("Serialization error for {protocol} protocol: {description}")]
+pub struct Error {
+    pub description: String,
+    pub protocol: CodecProtocol,
 }
 
 impl From<CborError> for Error {
     fn from(err: CborError) -> Error {
-        Error::Marshalling {
+        Self {
             description: err.to_string(),
             protocol: CodecProtocol::Cbor,
         }
     }
 }
 
 impl From<CidError> for Error {
-    fn from(err: CidError) -> Error {
-        Error::Marshalling {
+    fn from(err: CidError) -> Self {
+        Self {
             description: err.to_string(),
             protocol: CodecProtocol::Cbor,
         }
     }
 }
 
 impl From<Error> for io::Error {
-    fn from(err: Error) -> io::Error {
-        io::Error::new(io::ErrorKind::Other, err)
+    fn from(err: Error) -> Self {
+        Self::new(io::ErrorKind::Other, err)
     }
 }
 

ipld/amt/src/error.rs

@@ -2,8 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0, MIT
 
 use cid::Error as CidError;
-use db::Error as DBError;
-use encoding::error::Error as EncodingError;
+use encoding::Error as EncodingError;
 use std::error::Error as StdError;
 use thiserror::Error;
 
@@ -16,15 +15,9 @@ pub enum Error {
     /// Height of root node is greater than max.
     #[error("failed to load AMT: height out of bounds: {0} > {1}")]
     MaxHeight(u64, u64),
-    /// Cbor encoding error
-    #[error(transparent)]
-    Encoding(#[from] EncodingError),
     /// Error generating a Cid for data
     #[error(transparent)]
     Cid(#[from] CidError),
-    /// Error interacting with underlying database
-    #[error(transparent)]
-    DB(#[from] DBError),
     /// Error when trying to serialize an AMT without a flushed cache
     #[error("Tried to serialize without saving cache, run flush() on Amt before serializing")]
     Cached,
@@ -37,35 +30,22 @@ pub enum Error {
     /// Cid not found in store error
     #[error("Cid ({0}) did not match any in database")]
     CidNotFound(String),
+    /// Dynamic error for when the error needs to be forwarded as is.
+    #[error("{0}")]
+    Dynamic(Box<dyn StdError>),
     /// Custom AMT error
     #[error("{0}")]
     Other(String),
 }
 
-impl PartialEq for Error {
-    fn eq(&self, other: &Self) -> bool {
-        use Error::*;
-
-        match (self, other) {
-            (&OutOfRange(a), &OutOfRange(b)) => a == b,
-            (&Encoding(_), &Encoding(_)) => true,
-            (&Cid(ref a), &Cid(ref b)) => a == b,
-            (&DB(ref a), &DB(ref b)) => a == b,
-            (&Cached, &Cached) => true,
-            (&Other(ref a), &Other(ref b)) => a == b,
-            _ => false,
-        }
-    }
-}
-
-impl From<Error> for String {
-    fn from(e: Error) -> Self {
-        e.to_string()
+impl From<EncodingError> for Error {
+    fn from(e: EncodingError) -> Self {
+        Self::Dynamic(Box::new(e))
     }
 }
 
 impl From<Box<dyn StdError>> for Error {
     fn from(e: Box<dyn StdError>) -> Self {
-        Self::Other(e.to_string())
+        Self::Dynamic(e)
     }
 }

ipld/amt/tests/amt_tests.rs

@@ -57,7 +57,7 @@ fn out_of_range() {
     assert!(matches!(res, Err(Error::OutOfRange(_))));
 
     let res = a.set(MAX_INDEX, "what is up".to_owned());
-    assert_eq!(res.err(), None);
+    assert!(res.err().is_none());
     // 20 is the max height, custom value to avoid exporting
     assert_eq!(a.height(), 20);
 
@@ -199,8 +199,8 @@ fn delete_fail_check() {
     assert_eq!(a.count(), 2);
     assert_eq!(a.get(1).unwrap(), Some(&"one".to_string()));
     assert_eq!(a.get(9).unwrap(), Some(&"nine".to_string()));
-    assert_eq!(a.delete(10), Ok(false));
-    assert_eq!(a.delete(0), Ok(false));
+    assert_eq!(a.delete(10).unwrap(), false);
+    assert_eq!(a.delete(0).unwrap(), false);
     assert_eq!(a.count(), 2);
     assert_eq!(a.get(1).unwrap(), Some(&"one".to_string()));
     assert_eq!(a.get(9).unwrap(), Some(&"nine".to_string()));

ipld/hamt/src/error.rs

@@ -1,14 +1,12 @@
 // Copyright 2020 ChainSafe Systems
 // SPDX-License-Identifier: Apache-2.0, MIT
 
-use db::Error as DBError;
-use forest_encoding::error::Error as CborError;
-use forest_ipld::Error as IpldError;
+use forest_encoding::Error as EncodingError;
 use std::error::Error as StdError;
 use thiserror::Error;
 
 /// HAMT Error
-#[derive(Debug, PartialEq, Error)]
+#[derive(Debug, Error)]
 pub enum Error {
     /// Maximum depth error
     #[error("Maximum depth reached")]
@@ -19,40 +17,25 @@ pub enum Error {
     /// This should be treated as a fatal error, must have at least one pointer in node
     #[error("Invalid HAMT format, node cannot have 0 pointers")]
     ZeroPointers,
-    /// Error interacting with underlying database
-    #[error(transparent)]
-    Db(#[from] DBError),
-    /// Error encoding/ decoding values in store
-    #[error("{0}")]
-    Encoding(String),
     /// Cid not found in store error
     #[error("Cid ({0}) did not match any in database")]
     CidNotFound(String),
+    /// Dynamic error for when the error needs to be forwarded as is.
+    #[error("{0}")]
+    Dynamic(Box<dyn StdError>),
     /// Custom HAMT error
     #[error("{0}")]
     Other(String),
 }
 
-impl From<Error> for String {
-    fn from(e: Error) -> Self {
-        e.to_string()
-    }
-}
-
-impl From<CborError> for Error {
-    fn from(e: CborError) -> Error {
-        Error::Encoding(e.to_string())
-    }
-}
-
-impl From<IpldError> for Error {
-    fn from(e: IpldError) -> Error {
-        Error::Encoding(e.to_string())
+impl From<EncodingError> for Error {
+    fn from(e: EncodingError) -> Self {
+        Self::Dynamic(Box::new(e))
     }
 }
 
 impl From<Box<dyn StdError>> for Error {
     fn from(e: Box<dyn StdError>) -> Self {
-        Self::Other(e.to_string())
+        Self::Dynamic(e)
     }
 }

ipld/hamt/src/hash_bits.rs

@@ -93,11 +93,11 @@ mod tests {
         assert_eq!(hb.next(5).unwrap(), 0b01010);
         assert_eq!(hb.next(6).unwrap(), 0b111111);
         assert_eq!(hb.next(8).unwrap(), 0b11111111);
-        assert_eq!(hb.next(9), Err(Error::InvalidHashBitLen));
+        assert!(matches!(hb.next(9), Err(Error::InvalidHashBitLen)));
         for _ in 0..28 {
             // Iterate through rest of key to test depth
             hb.next(8).unwrap();
         }
-        assert_eq!(hb.next(1), Err(Error::MaxDepth));
+        assert!(matches!(hb.next(1), Err(Error::MaxDepth)));
     }
 }

utils/json_utils/src/lib.rs

@@ -68,7 +68,6 @@ mod tests {
 
     #[test]
     fn test_json_basic() {
-        // #[derive(Debug, PartialEq)]
         struct BasicJson(Vec<u8>);
         impl<'de> Deserialize<'de> for BasicJson {
             fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>

vm/actor/src/builtin/init/mod.rs

@@ -7,7 +7,7 @@ mod types;
 pub use self::state::State;
 pub use self::types::*;
 use crate::{
-    make_map, MINER_ACTOR_CODE_ID, MULTISIG_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID,
+    make_map, ActorDowncast, MINER_ACTOR_CODE_ID, MULTISIG_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID,
     POWER_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR,
 };
 use address::Address;
@@ -40,9 +40,9 @@ impl Actor {
         let sys_ref: &Address = &SYSTEM_ACTOR_ADDR;
         rt.validate_immediate_caller_is(std::iter::once(sys_ref))?;
         let mut empty_map = make_map::<_, ()>(rt.store());
-        let root = empty_map
-            .flush()
-            .map_err(|err| actor_error!(ErrIllegalState; "failed to construct state: {}", err))?;
+        let root = empty_map.flush().map_err(|err| {
+            err.downcast_default(ExitCode::ErrIllegalState, "failed to construct state")
+        })?;
 
         rt.create(&State::new(root, params.network_name))?;
 
@@ -76,7 +76,9 @@ impl Actor {
         // Store mapping of pubkey or actor address to actor ID
         let id_address: Address = rt.transaction(|s: &mut State, rt| {
             s.map_address_to_new_id(rt.store(), &robust_address)
-                .map_err(|e| actor_error!(ErrIllegalState; "failed to allocate ID address: {}", e))
+                .map_err(|e| {
+                    e.downcast_default(ExitCode::ErrIllegalState, "failed to allocate ID address")
+                })
         })?;
 
         // Create an empty actor

vm/actor/src/builtin/init/state.rs

@@ -8,6 +8,7 @@ use encoding::tuple::*;
 use encoding::Cbor;
 use ipld_blockstore::BlockStore;
 use ipld_hamt::Error as HamtError;
+use std::error::Error as StdError;
 use vm::ActorID;
 
 /// State is reponsible for creating
@@ -58,7 +59,7 @@ impl State {
         &self,
         store: &BS,
         addr: &Address,
-    ) -> Result<Option<Address>, String> {
+    ) -> Result<Option<Address>, Box<dyn StdError>> {
         if addr.protocol() == Protocol::ID {
             return Ok(Some(*addr));
         }