cleaner error handling

Author: alexytsu

fil_token/src/runtime/mod.rs

@@ -1 +1,8 @@
-pub trait Runtime {}
+use anyhow::Result;
+use fvm_shared::address::Address;
+
+pub trait Runtime {
+    fn caller(&self) -> u64;
+
+    fn resolve_address(&self, addr: &Address) -> Result<u64>;
+}

fil_token/src/token/errors.rs

@@ -1,14 +1,57 @@
-use super::state::StateError;
+use std::{error::Error, fmt::Display};
+
 use fvm_ipld_hamt::Error as HamtError;
 use fvm_shared::address::Address;
 
+#[derive(Debug)]
+pub enum RuntimeError {
+    AddrNotFound(Address),
+}
+
+impl Display for RuntimeError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            RuntimeError::AddrNotFound(_) => write!(f, "Address not found: {}", self),
+        }
+    }
+}
+
+impl Error for RuntimeError {}
+
+#[derive(Debug)]
+pub enum StateError {}
+
+impl Display for StateError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "State error")
+    }
+}
+
+impl Error for StateError {}
+
+#[derive(Debug)]
 pub enum ActorError {
     AddrNotFound(Address),
+    Arithmetic(String),
     IpldState(StateError),
     IpldHamt(HamtError),
-    Arithmetic(String),
+    RuntimeError(RuntimeError),
 }
 
+impl Display for ActorError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ActorError::AddrNotFound(e) => write!(f, "{}", e),
+            ActorError::Arithmetic(e) => write!(f, "{}", e),
+            ActorError::IpldState(e) => write!(f, "{}", e),
+            ActorError::IpldHamt(e) => write!(f, "{}", e),
+            ActorError::RuntimeError(e) => write!(f, "{}", e),
+        }
+    }
+}
+
+impl Error for ActorError {}
+
 impl From<StateError> for ActorError {
     fn from(e: StateError) -> Self {
         Self::IpldState(e)
@@ -20,3 +63,9 @@ impl From<HamtError> for ActorError {
         Self::IpldHamt(e)
     }
 }
+
+impl From<RuntimeError> for ActorError {
+    fn from(e: RuntimeError) -> Self {
+        ActorError::RuntimeError(e)
+    }
+}

fil_token/src/token/mod.rs

@@ -2,19 +2,20 @@ pub mod errors;
 pub mod receiver;
 pub mod state;
 pub mod types;
+use self::errors::ActorError;
+use self::state::TokenState;
+use self::types::*;
+use crate::runtime::Runtime;
 
+use anyhow::bail;
+use anyhow::Result;
 use fvm_ipld_blockstore::Blockstore as IpldStore;
 use fvm_shared::address::Address;
 use fvm_shared::bigint::bigint_ser::BigIntDe;
 use fvm_shared::bigint::BigInt;
 use fvm_shared::bigint::Zero;
 use fvm_shared::econ::TokenAmount;
 
-use self::errors::ActorError;
-use self::state::TokenState;
-use self::types::*;
-use crate::runtime::Runtime;
-
 /// A macro to abort concisely.
 macro_rules! abort {
     ($code:ident, $msg:literal $(, $ex:expr)*) => {
@@ -25,8 +26,6 @@ macro_rules! abort {
     };
 }
 
-type Result<T> = std::result::Result<T, ActorError>;
-
 /// A standard fungible token interface allowing for on-chain transactions
 pub trait Token {
     /// Constructs the token
@@ -82,7 +81,7 @@ where
     /// Injected blockstore
     bs: BS,
     /// Access to the runtime
-    _fvm: FVM,
+    fvm: FVM,
 }
 
 impl<BS, FVM> StandardToken<BS, FVM>
@@ -132,10 +131,9 @@ where
         let mut state = self.load_state();
         let mut balances = state.get_balance_map(&self.bs);
 
-        // FIXME: replace fvm_sdk with abstraction
-        let holder = match fvm_sdk::actor::resolve_address(&params.initial_holder) {
-            Some(id) => id,
-            None => {
+        let holder = match self.fvm.resolve_address(&params.initial_holder) {
+            Ok(id) => id,
+            Err(_) => {
                 return Ok(MintReturn {
                     newly_minted: TokenAmount::zero(),
                     successful: false,
@@ -176,10 +174,7 @@ where
         let balances = state.get_balance_map(&self.bs);
 
         // Resolve the address
-        let addr_id = match fvm_sdk::actor::resolve_address(&holder) {
-            Some(id) => id,
-            None => return Err(ActorError::AddrNotFound(holder)),
-        };
+        let addr_id = self.fvm.resolve_address(&holder)?;
 
         match balances.get(&addr_id) {
             Ok(Some(bal)) => Ok(bal.clone().0),
@@ -196,23 +191,18 @@ where
         // Load the HAMT holding balances
         let state = self.load_state();
 
-        // FIXME: replace with runtime service call
-        let caller_id = fvm_sdk::message::caller();
+        let caller_id = self.fvm.caller();
         let mut caller_allowances_map = state.get_actor_allowance_map(&self.bs, caller_id);
 
-        let spender = match fvm_sdk::actor::resolve_address(&params.spender) {
-            Some(id) => id,
-            None => return Err(ActorError::AddrNotFound(params.spender)),
-        };
-
+        let spender = self.fvm.resolve_address(&&params.spender)?;
         let new_amount = match caller_allowances_map.get(&spender)? {
             // Allowance exists - attempt to calculate new allowance
             Some(existing_allowance) => match existing_allowance.0.checked_add(&params.value) {
                 Some(new_allowance) => {
                     caller_allowances_map.set(spender, BigIntDe(new_allowance.clone()))?;
                     new_allowance
                 }
-                None => return Err(ActorError::Arithmetic(String::from("Allowance overflowed"))),
+                None => bail!(ActorError::Arithmetic(String::from("Allowance overflowed"))),
             },
             // No allowance recorded previously
             None => {
@@ -234,13 +224,10 @@ where
         // Load the HAMT holding balances
         let state = self.load_state();
 
-        // FIXME: replace with runtime service call
-        let caller_id = fvm_sdk::message::caller();
+        let caller_id = self.fvm.caller();
         let mut caller_allowances_map = state.get_actor_allowance_map(&self.bs, caller_id);
-        let spender = match fvm_sdk::actor::resolve_address(&params.spender) {
-            Some(id) => id,
-            None => return Err(ActorError::AddrNotFound(params.spender)),
-        };
+
+        let spender = self.fvm.resolve_address(&&params.spender)?;
 
         let new_allowance = match caller_allowances_map.get(&spender)? {
             Some(existing_allowance) => {
@@ -275,16 +262,13 @@ where
         // Load the HAMT holding balances
         let state = self.load_state();
 
-        // FIXME: replace with runtime service call
-        let caller_id = fvm_sdk::message::caller();
+        let caller_id = self.fvm.caller();
         let mut caller_allowances_map = state.get_actor_allowance_map(&self.bs, caller_id);
-        let spender = match fvm_sdk::actor::resolve_address(&params.spender) {
-            Some(id) => id,
-            None => return Err(ActorError::AddrNotFound(params.spender)),
-        };
 
+        let spender = self.fvm.resolve_address(&&params.spender)?;
         let new_allowance = TokenAmount::zero();
         caller_allowances_map.set(spender, BigIntDe(new_allowance.clone()))?;
+
         state.save(&self.bs);
 
         Ok(AllowanceReturn {
@@ -298,17 +282,10 @@ where
         // Load the HAMT holding balances
         let state = self.load_state();
 
-        // FIXME: replace with runtime service call
-        let owner = match fvm_sdk::actor::resolve_address(&params.owner) {
-            Some(id) => id,
-            None => return Err(ActorError::AddrNotFound(params.spender)),
-        };
-
+        let owner = self.fvm.resolve_address(&params.owner)?;
         let owner_allowances_map = state.get_actor_allowance_map(&self.bs, owner);
-        let spender = match fvm_sdk::actor::resolve_address(&params.spender) {
-            Some(id) => id,
-            None => return Err(ActorError::AddrNotFound(params.spender)),
-        };
+
+        let spender = self.fvm.resolve_address(&&params.spender)?;
 
         let allowance = match owner_allowances_map.get(&spender)? {
             Some(allowance) => allowance.0.clone(),

fil_token/src/token/receiver.rs

@@ -1,9 +1,6 @@
+use anyhow::Result;
 use fvm_shared::{address::Address, econ::TokenAmount};
 
-pub type Result<T> = std::result::Result<T, ReceiverError>;
-
-pub enum ReceiverError {}
-
 /// Standard interface for a contract that wishes to receive tokens
 pub trait TokenReceiver {
     fn token_received(from: Address, amount: TokenAmount, data: &[u8]) -> Result<u32>;

fil_token/src/token/state.rs

@@ -1,4 +1,5 @@
 use anyhow::anyhow;
+use anyhow::Result;
 use cid::multihash::Code;
 use cid::Cid;
 
@@ -10,7 +11,6 @@ use fvm_ipld_encoding::CborStore;
 use fvm_ipld_encoding::DAG_CBOR;
 use fvm_ipld_hamt::Hamt;
 use fvm_sdk::sself;
-use fvm_shared::address::Address;
 use fvm_shared::bigint::bigint_ser;
 use fvm_shared::bigint::bigint_ser::BigIntDe;
 use fvm_shared::econ::TokenAmount;
@@ -46,7 +46,7 @@ pub struct TokenState {
 
 /// Functions to get and modify token state to and from the IPLD layer
 impl TokenState {
-    pub fn new<BS: IpldStore>(store: &BS, name: &str, symbol: &str) -> StateResult<Self> {
+    pub fn new<BS: IpldStore>(store: &BS, name: &str, symbol: &str) -> Result<Self> {
         let empty_balance_map = Hamt::<_, ()>::new_with_bit_width(store, HAMT_BIT_WIDTH)
             .flush()
             .map_err(|e| anyhow!("Failed to create empty balances map state {}", e))?;
@@ -245,42 +245,3 @@ impl TokenState {
 }
 
 impl Cbor for TokenState {}
-
-pub struct TokenAmountDiff {
-    pub required: TokenAmount,
-    pub actual: TokenAmount,
-}
-
-pub enum TransferError {
-    NoRecrHook,
-    InsufficientAllowance(TokenAmountDiff),
-    InsufficientBalance(TokenAmountDiff),
-}
-
-pub enum StateError {
-    AddrNotFound(Address),
-    Arithmetic,
-    Ipld(fvm_ipld_hamt::Error),
-    Err(anyhow::Error),
-    Transfer(TransferError),
-}
-
-pub type StateResult<T> = std::result::Result<T, StateError>;
-
-impl From<anyhow::Error> for StateError {
-    fn from(e: anyhow::Error) -> Self {
-        Self::Err(e)
-    }
-}
-
-impl From<fvm_ipld_hamt::Error> for StateError {
-    fn from(e: fvm_ipld_hamt::Error) -> Self {
-        Self::Ipld(e)
-    }
-}
-
-impl From<TransferError> for StateError {
-    fn from(e: TransferError) -> Self {
-        Self::Transfer(e)
-    }
-}