From 5a92754b7ac60c5991a6ad51537c8d4c7799def1 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Wed, 6 Apr 2022 13:21:52 +0200 Subject: [PATCH 1/3] prototype error handling helpers --- Cargo.lock | 52 +++----- Cargo.toml | 16 +-- actors/account/src/lib.rs | 16 ++- actors/multisig/src/lib.rs | 184 ++++++++++++---------------- actors/runtime/src/actor_error.rs | 36 ++++++ actors/runtime/src/util/downcast.rs | 10 ++ 6 files changed, 157 insertions(+), 157 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66e532e56..2d02c9e16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -550,7 +550,7 @@ dependencies = [ "fil_actors_runtime", "fvm_ipld_blockstore", "fvm_ipld_encoding", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "num-derive", "num-traits", "serde", @@ -581,7 +581,7 @@ dependencies = [ "fil_actors_runtime", "fvm_ipld_blockstore", "fvm_ipld_encoding", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "log", "num-derive", "num-traits", @@ -598,7 +598,7 @@ dependencies = [ "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "log", "num-derive", "num-traits", @@ -618,7 +618,7 @@ dependencies = [ "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "libipld-core", "log", "num-derive", @@ -643,7 +643,7 @@ dependencies = [ "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "itertools", "lazy_static", "log", @@ -664,7 +664,7 @@ dependencies = [ "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "indexmap", "integer-encoding", "num-derive", @@ -683,7 +683,7 @@ dependencies = [ "fvm_ipld_amt", "fvm_ipld_blockstore", "fvm_ipld_encoding", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "num-derive", "num-traits", "serde", @@ -699,7 +699,7 @@ dependencies = [ "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "indexmap", "integer-encoding", "lazy_static", @@ -716,7 +716,7 @@ dependencies = [ "fil_actors_runtime", "fvm_ipld_blockstore", "fvm_ipld_encoding", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "lazy_static", "log", "num-derive", @@ -732,7 +732,7 @@ dependencies = [ "fil_actors_runtime", "fvm_ipld_blockstore", "fvm_ipld_encoding", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "num-derive", "num-traits", "serde", @@ -748,7 +748,7 @@ dependencies = [ "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "lazy_static", "num-derive", "num-traits", @@ -770,7 +770,7 @@ dependencies = [ "fvm_ipld_encoding", "fvm_ipld_hamt", "fvm_sdk", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "getrandom", "hex", "indexmap", @@ -927,8 +927,6 @@ dependencies = [ [[package]] name = "fvm_ipld_amt" version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b3394e5f9c2adb4d586519bc24bbfd659366e01e7ffa6cda676be94a62bab474" dependencies = [ "ahash", "anyhow", @@ -944,8 +942,6 @@ dependencies = [ [[package]] name = "fvm_ipld_bitfield" version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9011349297962982b8ab2663c220034525ec0f95f408c2b561d3d98867f1a803" dependencies = [ "cs_serde_bytes", "fvm_ipld_encoding", @@ -957,8 +953,6 @@ dependencies = [ [[package]] name = "fvm_ipld_blockstore" version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1985eae58ec2fbf54535ce115c72a2141459fb7fb4ff7379e17bffae0e302578" dependencies = [ "anyhow", "cid", @@ -981,8 +975,6 @@ dependencies = [ [[package]] name = "fvm_ipld_encoding" version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43bd635987aac46a753ec81767713af35cb50f182c7cc49d3a429643ede0e709" dependencies = [ "anyhow", "cid", @@ -998,8 +990,6 @@ dependencies = [ [[package]] name = "fvm_ipld_hamt" version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a03c6ae361a882360bc0c0f47265b294429f096baa8d9467247bbd62c6a6683c" dependencies = [ "anyhow", "byteorder", @@ -1019,12 +1009,10 @@ dependencies = [ [[package]] name = "fvm_sdk" version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ee9d472c4f0ad63f91a23b2ef72d9015935a3be040fa3a0ee3a0d6c4549e394" dependencies = [ "cid", "fvm_ipld_encoding", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "lazy_static", "log", "num-traits", @@ -1063,9 +1051,7 @@ dependencies = [ [[package]] name = "fvm_shared" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d1f6ca4fb268d06287c26bc96d7f9e6ac161557b7d70cf557550a186d0714b5" +version = "0.4.1" dependencies = [ "anyhow", "bimap", @@ -1461,9 +1447,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.36" +version = "1.0.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7342d5883fbccae1cc37a2353b09c87c9b0f3afd73f5fb9bba687a1f733b029" +checksum = "ec757218438d5fda206afc041538b2f6d889286160d649a86a24d37e1235afd1" dependencies = [ "unicode-xid", ] @@ -1650,9 +1636,9 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" [[package]] name = "syn" -version = "1.0.90" +version = "1.0.91" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "704df27628939572cd88d33f171cd6f896f4eaca85252c6e0a72d8d8287ee86f" +checksum = "b683b2b825c8eef438b77c36a06dc262294da3d5a5813fac20da149241dcd44d" dependencies = [ "proc-macro2", "quote", @@ -1690,7 +1676,7 @@ dependencies = [ "fvm_ipld_blockstore", "fvm_ipld_encoding", "fvm_ipld_hamt", - "fvm_shared 0.4.0", + "fvm_shared 0.4.1", "indexmap", "log", "num-derive", diff --git a/Cargo.toml b/Cargo.toml index d8098f2c6..552230c38 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,14 +54,14 @@ members = [ ## Uncomment entries below when working locally on ref-fvm and this repo simultaneously. ## Assumes the ref-fvm checkout is in a sibling directory with the same name. ## (Valid once FVM modules are published to crates.io) -#[patch.crates-io] -#fvm_shared = { path = "../ref-fvm/shared" } -#fvm_sdk = { path = "../ref-fvm/sdk" } -#fvm_ipld_hamt = { path = "../ref-fvm/ipld/hamt" } -#fvm_ipld_amt = { path = "../ref-fvm/ipld/amt" } -#fvm_ipld_bitfield = { path = "../ref-fvm/ipld/bitfield"} -#fvm_ipld_encoding = { path = "../ref-fvm/ipld/encoding"} -#fvm_ipld_blockstore = { path = "../ref-fvm/ipld/blockstore"} +[patch.crates-io] +fvm_shared = { path = "../ref-fvm/shared" } +fvm_sdk = { path = "../ref-fvm/sdk" } +fvm_ipld_hamt = { path = "../ref-fvm/ipld/hamt" } +fvm_ipld_amt = { path = "../ref-fvm/ipld/amt" } +fvm_ipld_bitfield = { path = "../ref-fvm/ipld/bitfield"} +fvm_ipld_encoding = { path = "../ref-fvm/ipld/encoding"} +fvm_ipld_blockstore = { path = "../ref-fvm/ipld/blockstore"} [profile.wasm] inherits = "release" diff --git a/actors/account/src/lib.rs b/actors/account/src/lib.rs index b1e17864e..8df62b3c2 100644 --- a/actors/account/src/lib.rs +++ b/actors/account/src/lib.rs @@ -11,7 +11,7 @@ use num_traits::FromPrimitive; use fil_actors_runtime::builtin::singletons::SYSTEM_ACTOR_ADDR; use fil_actors_runtime::cbor; use fil_actors_runtime::runtime::{ActorCode, Runtime}; -use fil_actors_runtime::{actor_error, ActorError}; +use fil_actors_runtime::{actor_error, ensure_args, ActorError}; pub use self::state::State; @@ -39,14 +39,12 @@ impl Actor { BS: Blockstore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR))?; - match address.protocol() { - Protocol::Secp256k1 | Protocol::BLS => {} - protocol => { - return Err(actor_error!(ErrIllegalArgument; - "address must use BLS or SECP protocol, got {}", protocol)); - } - } + rt.validate_immediate_caller_is([&*SYSTEM_ACTOR_ADDR])?; + ensure_args!( + matches!(address.protocol(), Protocol::Secp256k1 | Protocol::BLS), + "address must use BLS or SECP protocol, got {}", + address.protocol(), + ); rt.create(&State { address })?; Ok(()) } diff --git a/actors/multisig/src/lib.rs b/actors/multisig/src/lib.rs index 1caf352ff..10e5090eb 100644 --- a/actors/multisig/src/lib.rs +++ b/actors/multisig/src/lib.rs @@ -6,8 +6,8 @@ use std::collections::BTreeSet; use fil_actors_runtime::cbor::serialize_vec; use fil_actors_runtime::runtime::{ActorCode, Runtime, Syscalls}; use fil_actors_runtime::{ - actor_error, cbor, make_empty_map, make_map_with_root, resolve_to_id_addr, ActorDowncast, - ActorError, Map, INIT_ACTOR_ADDR, + actor_error, cbor, ensure, ensure_args, make_empty_map, make_map_with_root, resolve_to_id_addr, + ActorCastResult, ActorDowncast, ActorError, Map, INIT_ACTOR_ADDR, }; use fvm_ipld_blockstore::Blockstore; use fvm_ipld_encoding::RawBytes; @@ -55,51 +55,37 @@ impl Actor { BS: Blockstore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(&*INIT_ACTOR_ADDR))?; - - if params.signers.is_empty() { - return Err(actor_error!(ErrIllegalArgument; "Must have at least one signer")); - } - - if params.signers.len() > SIGNERS_MAX { - return Err(actor_error!( - ErrIllegalArgument, - "cannot add more than {} signers", - SIGNERS_MAX - )); - } + rt.validate_immediate_caller_is([&*INIT_ACTOR_ADDR])?; + ensure_args!(!params.signers.is_empty(), "Must have at least one signer"); + ensure_args!( + params.signers.len() <= SIGNERS_MAX, + "cannot add more than {} signers", + SIGNERS_MAX + ); // resolve signer addresses and do not allow duplicate signers let mut resolved_signers = Vec::with_capacity(params.signers.len()); let mut dedup_signers = BTreeSet::new(); for signer in ¶ms.signers { - let resolved = resolve_to_id_addr(rt, signer).map_err(|e| { - e.downcast_default( - ExitCode::ErrIllegalState, - format!("failed to resolve addr {} to ID addr", signer), - ) - })?; - if !dedup_signers.insert(resolved.id().expect("address should be resolved")) { - return Err( - actor_error!(ErrIllegalArgument; "duplicate signer not allowed: {}", signer), - ); - } - resolved_signers.push(resolved); - } + let resolved = resolve_to_id_addr(rt, signer).unwrap_with_default( + ExitCode::ErrIllegalState, + format!("failed to resolve addr {} to ID addr", signer), + )?; - if params.num_approvals_threshold > params.signers.len() as u64 { - return Err( - actor_error!(ErrIllegalArgument; "must not require more approvals than signers"), + ensure_args!( + dedup_signers.insert(resolved.id().expect("address should be resolved")), + "duplicate signer not allowed: {}", + signer, ); - } - - if params.num_approvals_threshold < 1 { - return Err(actor_error!(ErrIllegalArgument; "must require at least one approval")); - } - if params.unlock_duration < 0 { - return Err(actor_error!(ErrIllegalArgument; "negative unlock duration disallowed")); + resolved_signers.push(resolved); } + ensure_args!( + params.num_approvals_threshold <= params.signers.len() as u64, + "must not require more approvals than signers" + ); + ensure_args!(params.num_approvals_threshold > 0, "must require at least one approval"); + ensure_args!(params.unlock_duration >= 0, "negative unlock duration disallowed"); let empty_root = make_empty_map::<_, ()>(rt.store(), HAMT_BIT_WIDTH).flush().map_err(|e| { @@ -134,25 +120,22 @@ impl Actor { BS: Blockstore, RT: Runtime, { - rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; + rt.validate_immediate_caller_type(&*CALLER_TYPES_SIGNABLE)?; let proposer: Address = rt.message().caller(); - if params.value.sign() == Sign::Minus { - return Err(actor_error!( - ErrIllegalArgument, - "proposed value must be non-negative, was {}", - params.value - )); - } + ensure_args!( + params.value.sign() != Sign::Minus, + "proposed value must be non-negative, was {}", + params.value + ); let (txn_id, txn) = rt.transaction(|st: &mut State, rt| { - if !st.is_signer(&proposer) { - return Err(actor_error!(ErrForbidden, "{} is not a signer", proposer)); - } + ensure!(st.is_signer(&proposer), ErrForbidden, "{} is not a signer", proposer); - let mut ptx = make_map_with_root(&st.pending_txs, rt.store()).map_err(|e| { - e.downcast_default(ExitCode::ErrIllegalState, "failed to load pending transactions") - })?; + let mut ptx = make_map_with_root(&st.pending_txs, rt.store()).unwrap_with_default( + ExitCode::ErrIllegalState, + "failed to load pending transactions", + )?; let t_id = st.next_tx_id; st.next_tx_id.0 += 1; @@ -165,19 +148,15 @@ impl Actor { approved: Vec::new(), }; - ptx.set(t_id.key(), txn.clone()).map_err(|e| { - e.downcast_default( - ExitCode::ErrIllegalState, - "failed to put transaction for propose", - ) - })?; + ptx.set(t_id.key(), txn.clone()).unwrap_with_default( + ExitCode::ErrIllegalState, + "failed to put transaction for propose", + )?; - st.pending_txs = ptx.flush().map_err(|e| { - e.downcast_default( - ExitCode::ErrIllegalState, - "failed to flush pending transactions", - ) - })?; + st.pending_txs = ptx.flush().unwrap_with_default( + ExitCode::ErrIllegalState, + "failed to flush pending transactions", + )?; Ok((t_id, txn)) })?; @@ -193,18 +172,17 @@ impl Actor { BS: Blockstore, RT: Runtime, { - rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; + rt.validate_immediate_caller_type(&*CALLER_TYPES_SIGNABLE)?; let approver: Address = rt.message().caller(); let id = params.id; let (st, txn) = rt.transaction(|st: &mut State, rt| { - if !st.is_signer(&approver) { - return Err(actor_error!(ErrForbidden; "{} is not a signer", approver)); - } + ensure!(st.is_signer(&approver), ErrForbidden, "{} is not a signer", approver); - let ptx = make_map_with_root(&st.pending_txs, rt.store()).map_err(|e| { - e.downcast_default(ExitCode::ErrIllegalState, "failed to load pending transactions") - })?; + let ptx = make_map_with_root(&st.pending_txs, rt.store()).unwrap_with_default( + ExitCode::ErrIllegalState, + "failed to load pending transactions", + )?; let txn = get_transaction(rt, &ptx, params.id, params.proposal_hash)?; @@ -230,58 +208,50 @@ impl Actor { BS: Blockstore, RT: Runtime, { - rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; + rt.validate_immediate_caller_type(&*CALLER_TYPES_SIGNABLE)?; let caller_addr: Address = rt.message().caller(); rt.transaction(|st: &mut State, rt| { - if !st.is_signer(&caller_addr) { - return Err(actor_error!(ErrForbidden; "{} is not a signer", caller_addr)); - } + ensure!(st.is_signer(&caller_addr), ErrForbidden, "{} is not a signer", caller_addr); let mut ptx = make_map_with_root::<_, Transaction>(&st.pending_txs, rt.store()) - .map_err(|e| { - e.downcast_default( - ExitCode::ErrIllegalState, - "failed to load pending transactions", - ) - })?; + .unwrap_with_default( + ExitCode::ErrIllegalState, + "failed to load pending transactions", + )?; let (_, tx) = ptx .delete(¶ms.id.key()) - .map_err(|e| { - e.downcast_default( - ExitCode::ErrIllegalState, - format!("failed to pop transaction {:?} for cancel", params.id), - ) - })? + .unwrap_with_default( + ExitCode::ErrIllegalState, + format!("failed to pop transaction {:?} for cancel", params.id), + )? .ok_or_else(|| { actor_error!(ErrNotFound, "no such transaction {:?} to cancel", params.id) })?; // Check to make sure transaction proposer is caller address - if tx.approved.get(0) != Some(&caller_addr) { - return Err( - actor_error!(ErrForbidden; "Cannot cancel another signers transaction"), - ); - } + ensure!( + tx.approved.get(0) == Some(&caller_addr), + ErrForbidden, + "Cannot cancel another signers transaction" + ); - let calculated_hash = compute_proposal_hash(&tx, rt).map_err(|e| { - e.downcast_default( - ExitCode::ErrIllegalState, - format!("failed to compute proposal hash for (tx: {:?})", params.id), - ) - })?; + let calculated_hash = compute_proposal_hash(&tx, rt).unwrap_with_default( + ExitCode::ErrIllegalState, + format!("failed to compute proposal hash for (tx: {:?})", params.id), + )?; - if !params.proposal_hash.is_empty() && params.proposal_hash != calculated_hash { - return Err(actor_error!(ErrIllegalState, "hash does not match proposal params")); - } + ensure!( + params.proposal_hash.is_empty() || params.proposal_hash == calculated_hash, + ErrIllegalState, + "hash does not match proposal params" + ); - st.pending_txs = ptx.flush().map_err(|e| { - e.downcast_default( - ExitCode::ErrIllegalState, - "failed to flush pending transactions", - ) - })?; + st.pending_txs = ptx.flush().unwrap_with_default( + ExitCode::ErrIllegalState, + "failed to flush pending transactions", + )?; Ok(()) }) diff --git a/actors/runtime/src/actor_error.rs b/actors/runtime/src/actor_error.rs index 1a6e41be4..fdf9e071c 100644 --- a/actors/runtime/src/actor_error.rs +++ b/actors/runtime/src/actor_error.rs @@ -104,4 +104,40 @@ macro_rules! actor_error { ( $code:ident, $msg:literal $(, $ex:expr)+ ) => { $crate::actor_error!($code; $msg $(, $ex)*) }; + + ($code:ident, $fmt:expr, $($arg:tt)*) => { + $crate::actor_error!($code, format!($fmt, $($arg)*)) + }; +} + +#[macro_export] +macro_rules! ensure { + ($cond:expr, $code:ident, $msg:literal $(,)?) => { + if !$cond { + return Err($crate::actor_error!($code, $msg)); + } + }; + ($cond:expr, $code:ident, $err:expr $(,)?) => { + if !$cond { + return Err($crate::actor_error!($code, $err)); + } + }; + ($cond:expr, $code:ident, $fmt:expr, $($arg:tt)*) => { + if !$cond { + return Err($crate::actor_error!($code, $fmt, $($arg)*)); + } + }; +} + +#[macro_export] +macro_rules! ensure_args { + ($cond:expr, $msg:literal $(,)?) => { + $crate::ensure!($cond, ErrIllegalArgument, $msg) + }; + ($cond:expr, $err:expr $(,)?) => { + $crate::ensure!($cond, ErrIlegalArgument, $err) + }; + ($cond:expr, $fmt:expr, $($arg:tt)*) => { + $crate::ensure!($cond, ErrIllegalArgument, $fmt, $($arg)*) + }; } diff --git a/actors/runtime/src/util/downcast.rs b/actors/runtime/src/util/downcast.rs index e86bf05a1..2de21f327 100644 --- a/actors/runtime/src/util/downcast.rs +++ b/actors/runtime/src/util/downcast.rs @@ -9,6 +9,16 @@ use fvm_shared::error::ExitCode; use crate::ActorError; +pub trait ActorCastResult { + fn unwrap_with_default(self, code: ExitCode, msg: impl AsRef) -> Result; +} + +impl ActorCastResult for Result { + fn unwrap_with_default(self, code: ExitCode, msg: impl AsRef) -> Result { + self.map_err(|err| err.downcast_default(code, msg)) + } +} + /// 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 From 09c204edd2aa5c3f6a0326669181a93621cc61e4 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Wed, 6 Apr 2022 13:43:11 +0200 Subject: [PATCH 2/3] prototype abort error --- actors/account/src/lib.rs | 12 +++---- actors/runtime/src/actor_error.rs | 44 ++++++++++++++++++++++-- actors/runtime/src/runtime/actor_code.rs | 4 +-- actors/runtime/src/runtime/fvm.rs | 5 +-- actors/runtime/src/test_utils.rs | 3 +- actors/runtime/src/util/chaos/mod.rs | 6 ++-- 6 files changed, 57 insertions(+), 17 deletions(-) diff --git a/actors/account/src/lib.rs b/actors/account/src/lib.rs index 8df62b3c2..203c1d1ce 100644 --- a/actors/account/src/lib.rs +++ b/actors/account/src/lib.rs @@ -9,9 +9,9 @@ use num_derive::FromPrimitive; use num_traits::FromPrimitive; use fil_actors_runtime::builtin::singletons::SYSTEM_ACTOR_ADDR; -use fil_actors_runtime::cbor; use fil_actors_runtime::runtime::{ActorCode, Runtime}; -use fil_actors_runtime::{actor_error, ensure_args, ActorError}; +use fil_actors_runtime::{actor_error, ensure_args}; +use fil_actors_runtime::{cbor, Abort}; pub use self::state::State; @@ -34,7 +34,7 @@ pub enum Method { pub struct Actor; impl Actor { /// Constructor for Account actor - pub fn constructor(rt: &mut RT, address: Address) -> Result<(), ActorError> + pub fn constructor(rt: &mut RT, address: Address) -> Result<(), Abort> where BS: Blockstore, RT: Runtime, @@ -50,7 +50,7 @@ impl Actor { } // Fetches the pubkey-type address from this actor. - pub fn pubkey_address(rt: &mut RT) -> Result + pub fn pubkey_address(rt: &mut RT) -> Result where BS: Blockstore, RT: Runtime, @@ -66,7 +66,7 @@ impl ActorCode for Actor { rt: &mut RT, method: MethodNum, params: &RawBytes, - ) -> Result + ) -> Result where BS: Blockstore, RT: Runtime, @@ -80,7 +80,7 @@ impl ActorCode for Actor { let addr = Self::pubkey_address(rt)?; Ok(RawBytes::serialize(addr)?) } - None => Err(actor_error!(SysErrInvalidMethod; "Invalid method")), + None => Err(actor_error!(SysErrInvalidMethod; "Invalid method").into()), } } } diff --git a/actors/runtime/src/actor_error.rs b/actors/runtime/src/actor_error.rs index fdf9e071c..c827b9a5f 100644 --- a/actors/runtime/src/actor_error.rs +++ b/actors/runtime/src/actor_error.rs @@ -1,6 +1,44 @@ use fvm_shared::error::ExitCode; use thiserror::Error; +mod abort { + use crate::ActorError; + use fvm_shared::error::ExitCode; + + #[derive(thiserror::Error, Debug)] + #[error("abort error")] + pub struct Abort { + /// This ensures that this error can not be crated outside. + _private: (), + } + + #[cfg(feature = "fil-actor")] + fn maybe_abort(exit_code: ExitCode, msg: Option<&str>) -> ! { + fvm_sdk::vm::abort(exit_code as u32, msg); + } + #[cfg(not(feature = "fil-actor"))] + fn maybe_abort(exit_code: ExitCode, msg: Option<&str>) -> ! { + // TODO: maybe not panic, needs discussion what we want here + panic!("Abort: {}: {:?}", exit_code, msg); + } + + impl From for Abort { + fn from(err: ActorError) -> Self { + let ActorError { exit_code, msg } = err; + maybe_abort(exit_code, Some(&msg)); + } + } + + /// Converts a raw encoding error into an ErrSerialization. + impl From for Abort { + fn from(e: fvm_ipld_encoding::Error) -> Self { + maybe_abort(ExitCode::ErrSerialization, Some(&e.to_string())); + } + } +} + +pub use abort::Abort; + /// TODO fix error system; actor errors should be transparent to the VM. /// The error type that gets returned by actor method calls. #[derive(Error, Debug, Clone, PartialEq)] @@ -114,17 +152,17 @@ macro_rules! actor_error { macro_rules! ensure { ($cond:expr, $code:ident, $msg:literal $(,)?) => { if !$cond { - return Err($crate::actor_error!($code, $msg)); + return Err($crate::actor_error!($code, $msg).into()); } }; ($cond:expr, $code:ident, $err:expr $(,)?) => { if !$cond { - return Err($crate::actor_error!($code, $err)); + return Err($crate::actor_error!($code, $err).into()); } }; ($cond:expr, $code:ident, $fmt:expr, $($arg:tt)*) => { if !$cond { - return Err($crate::actor_error!($code, $fmt, $($arg)*)); + return Err($crate::actor_error!($code, $fmt, $($arg)*).into()); } }; } diff --git a/actors/runtime/src/runtime/actor_code.rs b/actors/runtime/src/runtime/actor_code.rs index b6895d560..42bd1db95 100644 --- a/actors/runtime/src/runtime/actor_code.rs +++ b/actors/runtime/src/runtime/actor_code.rs @@ -5,7 +5,7 @@ use fvm_ipld_blockstore::Blockstore; use fvm_ipld_encoding::RawBytes; use fvm_shared::MethodNum; -use crate::{ActorError, Runtime}; +use crate::{Abort, Runtime}; /// Interface for invoking methods on an Actor pub trait ActorCode { @@ -15,7 +15,7 @@ pub trait ActorCode { rt: &mut RT, method: MethodNum, params: &RawBytes, - ) -> Result + ) -> Result where // TODO: remove the clone requirement on the blockstore when we fix "replica update" to not // hold onto state between transactions. diff --git a/actors/runtime/src/runtime/fvm.rs b/actors/runtime/src/runtime/fvm.rs index dee547c26..db7b2d4e0 100644 --- a/actors/runtime/src/runtime/fvm.rs +++ b/actors/runtime/src/runtime/fvm.rs @@ -430,8 +430,9 @@ pub fn trampoline(params: u32) -> u32 { // Construct a new runtime. let mut rt = FvmRuntime::default(); // Invoke the method, aborting if the actor returns an errored exit code. - let ret = C::invoke_method(&mut rt, method, ¶ms) - .unwrap_or_else(|err| fvm::vm::abort(err.exit_code() as u32, Some(err.msg()))); + + // can unwrap because `Abort` will already have called rt::abort on an error. + let ret = C::invoke_method(&mut rt, method, ¶ms).unwrap(); // Abort with "illegal actor" if the actor failed to validate the caller somewhere. // We do this after handling the error, because the actor may have encountered an error before diff --git a/actors/runtime/src/test_utils.rs b/actors/runtime/src/test_utils.rs index 0dc0735e1..c2af16ed0 100644 --- a/actors/runtime/src/test_utils.rs +++ b/actors/runtime/src/test_utils.rs @@ -450,7 +450,8 @@ impl MockRuntime { self.state = prev_state; } self.in_call = false; - res + // res + todo!() } pub fn verify(&mut self) { diff --git a/actors/runtime/src/util/chaos/mod.rs b/actors/runtime/src/util/chaos/mod.rs index 2f274fb66..b433534ec 100644 --- a/actors/runtime/src/util/chaos/mod.rs +++ b/actors/runtime/src/util/chaos/mod.rs @@ -14,7 +14,7 @@ pub use state::*; pub use types::*; use crate::runtime::{ActorCode, Runtime}; -use crate::{actor_error, cbor, ActorError}; +use crate::{actor_error, cbor, Abort, ActorError}; mod state; mod types; @@ -197,7 +197,7 @@ impl ActorCode for Actor { rt: &mut RT, method: MethodNum, params: &RawBytes, - ) -> Result + ) -> Result where BS: Blockstore, RT: Runtime, @@ -246,7 +246,7 @@ impl ActorCode for Actor { Ok(RawBytes::serialize(inspect)?) } - None => Err(actor_error!(SysErrInvalidMethod; "Invalid method")), + None => Err(actor_error!(SysErrInvalidMethod; "Invalid method").into()), } } } From bf4c52313c769627c964d853913f866132da750c Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Wed, 6 Apr 2022 21:06:16 +0200 Subject: [PATCH 3/3] Update actors/runtime/src/actor_error.rs Co-authored-by: Steven Allen --- actors/runtime/src/actor_error.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/actors/runtime/src/actor_error.rs b/actors/runtime/src/actor_error.rs index c827b9a5f..96ee34fb6 100644 --- a/actors/runtime/src/actor_error.rs +++ b/actors/runtime/src/actor_error.rs @@ -7,10 +7,7 @@ mod abort { #[derive(thiserror::Error, Debug)] #[error("abort error")] - pub struct Abort { - /// This ensures that this error can not be crated outside. - _private: (), - } + pub enum Abort {} #[cfg(feature = "fil-actor")] fn maybe_abort(exit_code: ExitCode, msg: Option<&str>) -> ! {