diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 887680dddc..76ac68ade4 100644 --- a/actors/market/src/lib.rs +++ b/actors/market/src/lib.rs @@ -981,8 +981,7 @@ where illegal_argument, "deal id {} present multiple times", deal_id - ) - .into()); + )); } let proposal = proposals .get(*deal_id)? diff --git a/actors/market/src/state.rs b/actors/market/src/state.rs index 96b154cba9..c9c50ba034 100644 --- a/actors/market/src/state.rs +++ b/actors/market/src/state.rs @@ -565,7 +565,7 @@ where lock_reason: Reason, ) -> Result<(), ActorError> { if amount.is_negative() { - return Err(actor_error!(illegal_state, "unlock negative amount: {}", amount).into()); + return Err(actor_error!(illegal_state, "unlock negative amount: {}", amount)); } self.locked_table.as_mut().unwrap().must_subtract(addr, amount)?; @@ -618,7 +618,7 @@ where lock_reason: Reason, ) -> Result<(), ActorError> { if amount.is_negative() { - return Err(actor_error!(illegal_state, "negative amount to slash: {}", amount).into()); + return Err(actor_error!(illegal_state, "negative amount to slash: {}", amount)); } // Subtract from locked and escrow tables diff --git a/actors/miner/src/bitfield_queue.rs b/actors/miner/src/bitfield_queue.rs index d466c8abcf..cc002b5e96 100644 --- a/actors/miner/src/bitfield_queue.rs +++ b/actors/miner/src/bitfield_queue.rs @@ -4,7 +4,7 @@ use std::convert::TryInto; use cid::Cid; -use fil_actors_runtime::{ActorContext, ActorDowncast, ActorError, Array}; +use fil_actors_runtime::{ActorContext, ActorError, Array}; use fvm_ipld_amt::Error as AmtError; use fvm_ipld_bitfield::BitField; use fvm_ipld_blockstore::Blockstore; @@ -39,13 +39,13 @@ impl<'db, BS: Blockstore> BitFieldQueue<'db, BS> { let bitfield = self .amt .get(epoch) - .map_err(|e| e.downcast_wrap(format!("failed to lookup queue epoch {}", epoch)))? + .with_context(|| format!("failed to lookup queue epoch {}", epoch))? .cloned() .unwrap_or_default(); self.amt .set(epoch, &bitfield | values) - .map_err(|e| e.downcast_wrap(format!("failed to set queue epoch {}", epoch)))?; + .with_context(|| format!("failed to set queue epoch {}", epoch))?; Ok(()) } diff --git a/actors/miner/src/deadline_state.rs b/actors/miner/src/deadline_state.rs index 73660335e0..744d8fc536 100644 --- a/actors/miner/src/deadline_state.rs +++ b/actors/miner/src/deadline_state.rs @@ -614,7 +614,7 @@ impl Deadline { if let Some(&max_partition) = to_remove_set.iter().max() { if max_partition > partition_count { return Err( - actor_error!(illegal_argument; "partition index {} out of range [0, {})", max_partition, partition_count).into() + actor_error!(illegal_argument; "partition index {} out of range [0, {})", max_partition, partition_count), ); } } else { @@ -625,7 +625,7 @@ impl Deadline { // Should already be checked earlier, but we might as well check again. if !self.early_terminations.is_empty() { return Err( - actor_error!(illegal_argument; "cannot remove partitions from deadline with early terminations").into(), + actor_error!(illegal_argument; "cannot remove partitions from deadline with early terminations"), ); } @@ -663,8 +663,7 @@ impl Deadline { illegal_argument, "cannot remove partition {}: has unproven sectors", partition_idx - ) - .into()); + )); } // Get the live sectors. diff --git a/actors/miner/src/partition_state.rs b/actors/miner/src/partition_state.rs index b5e4003dc8..9233f5e770 100644 --- a/actors/miner/src/partition_state.rs +++ b/actors/miner/src/partition_state.rs @@ -484,7 +484,7 @@ impl Partition { })?; if !live_sectors.contains_all(sector_numbers) { - return Err(actor_error!(illegal_argument, "can only terminate live sectors").into()); + return Err(actor_error!(illegal_argument, "can only terminate live sectors")); } let sector_infos = sectors.load_sector(sector_numbers)?; @@ -733,8 +733,7 @@ impl Partition { return Err(actor_error!( illegal_argument, "skipped faults contains sectors outside partition" - ) - .into()); + )); } // Find all skipped faults that have been labeled recovered diff --git a/actors/miner/src/sectors.rs b/actors/miner/src/sectors.rs index 40d589dc4d..d7032a2441 100644 --- a/actors/miner/src/sectors.rs +++ b/actors/miner/src/sectors.rs @@ -4,11 +4,10 @@ use std::collections::BTreeSet; use cid::Cid; -use fil_actors_runtime::{actor_error, ActorDowncast, ActorError, Array}; +use fil_actors_runtime::{actor_error, ActorContext, ActorError, Array}; use fvm_ipld_amt::Error as AmtError; use fvm_ipld_bitfield::BitField; use fvm_ipld_blockstore::Blockstore; -use fvm_shared::error::ExitCode; use fvm_shared::sector::{SectorNumber, MAX_SECTOR_NUMBER}; use super::SectorOnChainInfo; @@ -36,12 +35,7 @@ impl<'db, BS: Blockstore> Sectors<'db, BS> { let sector_on_chain = self .amt .get(sector_number) - .map_err(|e| { - e.downcast_default( - ExitCode::USR_ILLEGAL_STATE, - format!("failed to load sector {}", sector_number), - ) - })? + .with_context(|| format!("failed to load sector {}", sector_number))? .cloned() .ok_or_else(|| actor_error!(not_found; "sector not found: {}", sector_number))?; sector_infos.push(sector_on_chain); @@ -56,7 +50,7 @@ impl<'db, BS: Blockstore> Sectors<'db, BS> { Ok(self .amt .get(sector_number) - .map_err(|e| e.downcast_wrap(format!("failed to get sector {}", sector_number)))? + .with_context(|| format!("failed to get sector {}", sector_number))? .cloned()) } @@ -72,9 +66,9 @@ impl<'db, BS: Blockstore> Sectors<'db, BS> { )); } - self.amt.set(sector_number, info).map_err(|e| { - e.downcast_wrap(format!("failed to store sector {}", sector_number)) - })?; + self.amt + .set(sector_number, info) + .with_context(|| format!("failed to store sector {}", sector_number))?; } Ok(()) diff --git a/actors/miner/src/state.rs b/actors/miner/src/state.rs index 534f4e5186..2a59de5d76 100644 --- a/actors/miner/src/state.rs +++ b/actors/miner/src/state.rs @@ -620,8 +620,7 @@ impl State { not_found; "sector {} not a member of partition {}, deadline {}", sector_number, partition_idx, deadline_idx - ) - .into()); + )); } let faulty = partition.faults.get(sector_number); @@ -660,8 +659,7 @@ impl State { not_found; "sector {} not a member of partition {}, deadline {}", sector_number, partition_idx, deadline_idx - ) - .into()); + )); } if partition.faults.get(sector_number) { @@ -669,8 +667,7 @@ impl State { forbidden; "sector {} not a member of partition {}, deadline {}", sector_number, partition_idx, deadline_idx - ) - .into()); + )); } if partition.terminated.get(sector_number) { @@ -678,8 +675,7 @@ impl State { not_found; "sector {} not of partition {}, deadline {} is terminated", sector_number, partition_idx, deadline_idx - ) - .into()); + )); } Ok(()) @@ -691,7 +687,7 @@ impl State { store: &BS, sectors: &BitField, ) -> Result, ActorError> { - Ok(Sectors::load(store, &self.sectors)?.load_sector(sectors)?) + Sectors::load(store, &self.sectors)?.load_sector(sectors) } pub fn load_deadlines(&self, store: &BS) -> Result { @@ -717,12 +713,12 @@ impl State { &self, store: &BS, ) -> Result { - Ok(store + store .get_cbor(&self.vesting_funds) .with_context(|| format!("failed to load vesting funds {}", self.vesting_funds))? .ok_or_else( || actor_error!(not_found; "failed to load vesting funds {:?}", self.vesting_funds), - )?) + ) } /// Saves the vesting table to the store. @@ -868,8 +864,7 @@ impl State { "unlocked balance can not repay fee debt ({} < {})", unlocked_balance, self.fee_debt - ) - .into()); + )); } Ok(std::mem::take(&mut self.fee_debt)) @@ -1175,9 +1170,7 @@ impl State { make_map_with_root_and_bitwidth(&self.pre_committed_sectors, store, HAMT_BIT_WIDTH)?; for sector_no in sector_nos.iter() { if sector_no as u64 > MAX_SECTOR_NUMBER { - return Err( - actor_error!(illegal_argument; "sector number greater than maximum").into() - ); + return Err(actor_error!(illegal_argument; "sector number greater than maximum")); } let info: &SectorPreCommitOnChainInfo = precommitted diff --git a/actors/runtime/src/actor_error.rs b/actors/runtime/src/actor_error.rs index ab7e89b049..8968419af2 100644 --- a/actors/runtime/src/actor_error.rs +++ b/actors/runtime/src/actor_error.rs @@ -3,8 +3,6 @@ use std::{fmt::Display, num::TryFromIntError}; use fvm_shared::error::ExitCode; use thiserror::Error; -use crate::ActorDowncast; - /// The error type returned by actor method calls. #[derive(Error, Debug, Clone, PartialEq)] #[error("ActorError(exit_code: {exit_code:?}, msg: {msg})")] @@ -232,7 +230,12 @@ impl> ActorContext for Result { // TODO: remove once the runtime doesn't use anyhow::Result anymore impl From for ActorError { fn from(e: anyhow::Error) -> Self { - // THIS DEFAULT IS WRONG, it is just a placeholder - e.downcast_default(ExitCode::USR_ILLEGAL_ARGUMENT, "runtime error") + match e.downcast::() { + Ok(actor_err) => actor_err, + Err(other) => ActorError::unchecked( + ExitCode::USR_ILLEGAL_ARGUMENT, + format!("runtime error: {}", other), + ), + } } } diff --git a/actors/runtime/src/runtime/actor_blockstore.rs b/actors/runtime/src/runtime/actor_blockstore.rs index 3f3a0e7546..e711a8798f 100644 --- a/actors/runtime/src/runtime/actor_blockstore.rs +++ b/actors/runtime/src/runtime/actor_blockstore.rs @@ -19,9 +19,9 @@ impl fvm_ipld_blockstore::Blockstore for ActorBlockstore { fn get(&self, cid: &Cid) -> Result>, Self::Error> { // If this fails, the _CID_ is invalid. I.e., we have a bug. - fvm::ipld::get(cid).map(Some).map_err(|c| { - actor_error!(illegal_state; "get failed with {:?} on CID '{}'", c, cid).into() - }) + fvm::ipld::get(cid) + .map(Some) + .map_err(|c| actor_error!(illegal_state; "get failed with {:?} on CID '{}'", c, cid)) } fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<(), Self::Error> { @@ -29,7 +29,7 @@ impl fvm_ipld_blockstore::Blockstore for ActorBlockstore { .map_err(|e| actor_error!(serialization, e.to_string()))?; let k2 = self.put(code, &Block::new(k.codec(), block))?; if k != &k2 { - Err(actor_error!(serialization; "put block with cid {} but has cid {}", k, k2).into()) + Err(actor_error!(serialization; "put block with cid {} but has cid {}", k, k2)) } else { Ok(()) } diff --git a/actors/runtime/src/util/downcast.rs b/actors/runtime/src/util/downcast.rs deleted file mode 100644 index cf299fe2f5..0000000000 --- a/actors/runtime/src/util/downcast.rs +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright 2019-2022 ChainSafe Systems -// SPDX-License-Identifier: Apache-2.0, MIT - -use anyhow::anyhow; -use fvm_ipld_amt::Error as AmtError; -use fvm_ipld_hamt::Error as HamtError; -use fvm_shared::error::ExitCode; - -use crate::ActorError; - -/// Trait to allow multiple error types to be able to be downcasted into an `ActorError`. -pub trait ActorDowncast { - /// Downcast a dynamic std Error into an `ActorError`. If the error cannot be downcasted - /// into an ActorError automatically, use the provided `ExitCode` to generate a new error. - fn downcast_default(self, default_exit_code: ExitCode, msg: impl AsRef) -> ActorError; - - /// Wrap the error with a message, without overwriting an exit code. - fn downcast_wrap(self, msg: impl AsRef) -> anyhow::Error; -} - -impl ActorDowncast for anyhow::Error { - fn downcast_default(self, default_exit_code: ExitCode, msg: impl AsRef) -> ActorError { - match downcast_util(self) { - Ok(actor_error) => actor_error.wrap(msg), - Err(other) => { - ActorError::unchecked(default_exit_code, format!("{}: {}", msg.as_ref(), other)) - } - } - } - fn downcast_wrap(self, msg: impl AsRef) -> anyhow::Error { - match downcast_util(self) { - Ok(actor_error) => anyhow!(actor_error.wrap(msg)), - Err(other) => anyhow!("{}: {}", msg.as_ref(), other), - } - } -} - -impl ActorDowncast for AmtError { - fn downcast_default(self, default_exit_code: ExitCode, msg: impl AsRef) -> ActorError { - match self { - // AmtError::Dynamic(e) => e.downcast_default(default_exit_code, msg), - // todo: proper downcast - other => { - ActorError::unchecked(default_exit_code, format!("{}: {}", msg.as_ref(), other)) - } - } - } - fn downcast_wrap(self, msg: impl AsRef) -> anyhow::Error { - match self { - // AmtError::Dynamic(e) => e.downcast_wrap(msg), - // todo: proper downcast - other => anyhow!("{}: {}", msg.as_ref(), other), - } - } -} - -impl ActorDowncast for HamtError { - fn downcast_default(self, default_exit_code: ExitCode, msg: impl AsRef) -> ActorError { - match self { - // HamtError::Dynamic(e) => e.downcast_default(default_exit_code, msg), - // todo: proper downcast - other => { - ActorError::unchecked(default_exit_code, format!("{}: {}", msg.as_ref(), other)) - } - } - } - fn downcast_wrap(self, msg: impl AsRef) -> anyhow::Error { - match self { - // HamtError::Dynamic(e) => e.downcast_wrap(msg), - // todo: proper downcast - other => anyhow!("{}: {}", msg.as_ref(), other), - } - } -} - -/// Attempts to downcast a `Box` into an actor error. -/// Returns `Ok` with the actor error if it can be downcasted automatically -/// and returns `Err` with the original error if it cannot. -fn downcast_util(error: anyhow::Error) -> anyhow::Result { - // Check if error is ActorError, return as such - let error = match error.downcast::() { - Ok(actor_err) => return Ok(actor_err), - Err(other) => other, - }; - - // Could not be downcasted automatically to actor error, return initial dynamic error. - Err(error) -} diff --git a/actors/runtime/src/util/mod.rs b/actors/runtime/src/util/mod.rs index c382432090..ff16a725b3 100644 --- a/actors/runtime/src/util/mod.rs +++ b/actors/runtime/src/util/mod.rs @@ -1,14 +1,12 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -pub use self::downcast::*; pub use self::multimap::{EitherError as MultiMapEitherError, Error as MultiMapError, Multimap}; pub use self::set::Set; pub use self::set_multimap::SetMultimap; pub mod cbor; pub mod chaos; -mod downcast; mod multimap; mod set; mod set_multimap; diff --git a/actors/runtime/src/util/set.rs b/actors/runtime/src/util/set.rs index ca31b9ffcb..277301c5e8 100644 --- a/actors/runtime/src/util/set.rs +++ b/actors/runtime/src/util/set.rs @@ -74,7 +74,7 @@ where // Calls the for each function on the hamt with ignoring the value self.0.try_for_each(|s, _: &()| f(s)).map_err(|err| match err { fvm_ipld_hamt::EitherError::User(e) => e, - fvm_ipld_hamt::EitherError::Hamt(e) => e.into(), + fvm_ipld_hamt::EitherError::Hamt(e) => e, }) } diff --git a/test_vm/src/lib.rs b/test_vm/src/lib.rs index ca3882a073..1e0fe73eaf 100644 --- a/test_vm/src/lib.rs +++ b/test_vm/src/lib.rs @@ -74,8 +74,8 @@ impl Error for TestVMError { } } -impl From for TestVMError { - fn from(h_err: fvm_ipld_hamt::Error) -> Self { +impl From> for TestVMError { + fn from(h_err: fvm_ipld_hamt::Error) -> Self { vm_err(h_err.to_string().as_str()) } }