Skip to content

Commit

Permalink
Introduce a SendError (#1136)
Browse files Browse the repository at this point in the history
* Introduce a SendError

This way the user can directly bubble the send error with the `?`
operator.

Unfortunately, I couldn't simplify `extract_send_result` because
`Response` isn't implemented in this crate and `ActorError` is, so
there's no way to implement `From<Response> for `Result<T, ActorError>`.

* Implement ActorContext for all errors

This way, users can directly call `context` on any result where the
error can be interpreted as an actor error (e.g., `SendError`).
  • Loading branch information
Stebalien authored Jan 31, 2023
1 parent 0bccc7a commit 7fe9b77
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 35 deletions.
11 changes: 8 additions & 3 deletions runtime/src/actor_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,16 @@ pub trait ActorContext<T> {
F: FnOnce() -> C;
}

impl<T> ActorContext<T> for Result<T, ActorError> {
impl<T, E> ActorContext<T> for Result<T, E>
where
E: Into<ActorError>,
{
fn context<C>(self, context: C) -> Result<T, ActorError>
where
C: Display + 'static,
{
self.map_err(|mut err| {
self.map_err(|err| {
let mut err = err.into();
err.msg = format!("{}: {}", context, err.msg);
err
})
Expand All @@ -185,7 +189,8 @@ impl<T> ActorContext<T> for Result<T, ActorError> {
C: Display + 'static,
F: FnOnce() -> C,
{
self.map_err(|mut err| {
self.map_err(|err| {
let mut err = err.into();
err.msg = format!("{}: {}", f(), err.msg);
err
})
Expand Down
47 changes: 30 additions & 17 deletions runtime/src/builtin/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,20 @@ where
Ok(())
}

pub fn extract_send_result(
res: Result<fvm_shared::Response, fvm_shared::error::ErrorNumber>,
) -> Result<Option<IpldBlock>, ActorError> {
match res {
Ok(ret) => {
if ret.exit_code.is_success() {
Ok(ret.return_data)
} else {
Err(ActorError::checked(
ret.exit_code,
format!("send aborted with code {}", ret.exit_code),
ret.return_data,
))
}
}
Err(err) => Err(match err {
/// An error returned on a failed send. Can be automatically converted into an [`ActorError`] with
/// the question-mark operator.
#[derive(Debug, Eq, PartialEq, Copy, Clone)]
pub struct SendError(pub fvm_shared::error::ErrorNumber);

impl From<SendError> for fvm_shared::error::ErrorNumber {
fn from(s: SendError) -> fvm_shared::error::ErrorNumber {
s.0
}
}

impl From<SendError> for ActorError {
fn from(s: SendError) -> ActorError {
match s.0 {
// Some of these errors are from operations in the Runtime or SDK layer
// before or after the underlying VM send syscall.
fvm_shared::error::ErrorNumber::NotFound => {
Expand All @@ -115,6 +113,21 @@ pub fn extract_send_result(
// We don't expect any other syscall exit codes.
actor_error!(assertion_failed; "unexpected error: {}", err)
}
}),
}
}
}

pub fn extract_send_result(
res: Result<fvm_shared::Response, SendError>,
) -> Result<Option<IpldBlock>, ActorError> {
let ret = res?;
if ret.exit_code.is_success() {
Ok(ret.return_data)
} else {
Err(ActorError::checked(
ret.exit_code,
format!("send aborted with code {}", ret.exit_code),
ret.return_data,
))
}
}
8 changes: 4 additions & 4 deletions runtime/src/runtime/fvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use crate::runtime::{
ActorCode, ConsensusFault, DomainSeparationTag, MessageInfo, Policy, Primitives, RuntimePolicy,
Verifier,
};
use crate::{actor_error, ActorError, Runtime};
use crate::{actor_error, ActorError, Runtime, SendError};

/// A runtime that bridges to the FVM environment through the FVM SDK.
pub struct FvmRuntime<B = ActorBlockstore> {
Expand Down Expand Up @@ -277,14 +277,14 @@ where
value: TokenAmount,
gas_limit: Option<u64>,
flags: SendFlags,
) -> Result<Response, ErrorNumber> {
) -> Result<Response, SendError> {
if self.in_transaction {
// Note: It's slightly improper to call this ErrorNumber::IllegalOperation,
// since the error arises before getting to the VM.
return Err(ErrorNumber::IllegalOperation);
return Err(SendError(ErrorNumber::IllegalOperation));
}

fvm::send::send(to, method, params, value, gas_limit, flags)
fvm::send::send(to, method, params, value, gas_limit, flags).map_err(SendError)
}

fn new_actor_address(&mut self) -> Result<Address, ActorError> {
Expand Down
7 changes: 3 additions & 4 deletions runtime/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub use self::actor_code::*;
pub use self::policy::*;
pub use self::randomness::DomainSeparationTag;
use crate::runtime::builtins::Type;
use crate::{actor_error, ActorError};
use crate::{actor_error, ActorError, SendError};

mod actor_code;
pub mod builtins;
Expand All @@ -42,7 +42,6 @@ pub(crate) mod empty;

pub use empty::EMPTY_ARR_CID;
use fvm_ipld_encoding::ipld_block::IpldBlock;
use fvm_shared::error::ErrorNumber;
use fvm_shared::sys::SendFlags;
use multihash::Code;

Expand Down Expand Up @@ -162,7 +161,7 @@ pub trait Runtime: Primitives + Verifier + RuntimePolicy {
value: TokenAmount,
gas_limit: Option<u64>,
flags: SendFlags,
) -> Result<Response, ErrorNumber>;
) -> Result<Response, SendError>;

/// Simplified version of [`Runtime::send`] that does not specify a gas limit, nor any send flags.
fn send_simple(
Expand All @@ -171,7 +170,7 @@ pub trait Runtime: Primitives + Verifier + RuntimePolicy {
method: MethodNum,
params: Option<IpldBlock>,
value: TokenAmount,
) -> Result<Response, ErrorNumber> {
) -> Result<Response, SendError> {
self.send(to, method, params, value, None, SendFlags::empty())
}

Expand Down
8 changes: 4 additions & 4 deletions runtime/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use crate::runtime::{
ActorCode, DomainSeparationTag, MessageInfo, Policy, Primitives, Runtime, RuntimePolicy,
Verifier, EMPTY_ARR_CID,
};
use crate::{actor_error, ActorError};
use crate::{actor_error, ActorError, SendError};

use fvm_ipld_encoding::ipld_block::IpldBlock;
use fvm_shared::sys::SendFlags;
Expand Down Expand Up @@ -953,7 +953,7 @@ impl<BS: Blockstore> Runtime for MockRuntime<BS> {
value: TokenAmount,
gas_limit: Option<u64>,
send_flags: SendFlags,
) -> Result<Response, ErrorNumber> {
) -> Result<Response, SendError> {
self.require_in_call();
if self.in_transaction {
return Ok(Response { exit_code: ExitCode::USR_ASSERTION_FAILED, return_data: None });
Expand All @@ -978,13 +978,13 @@ impl<BS: Blockstore> Runtime for MockRuntime<BS> {
assert_eq!(expected_msg.send_flags, send_flags, "send flags did not match expectation");

if let Some(e) = expected_msg.send_error {
return Err(e);
return Err(SendError(e));
}

{
let mut balance = self.balance.borrow_mut();
if value > *balance {
return Err(ErrorNumber::InsufficientFunds);
return Err(SendError(ErrorNumber::InsufficientFunds));
}
*balance -= value;
}
Expand Down
6 changes: 3 additions & 3 deletions test_vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ use fil_actor_power::{Actor as PowerActor, Method as MethodPower, State as Power
use fil_actor_reward::{Actor as RewardActor, State as RewardState};
use fil_actor_system::{Actor as SystemActor, State as SystemState};
use fil_actor_verifreg::{Actor as VerifregActor, State as VerifRegState};
use fil_actors_runtime::actor_error;
use fil_actors_runtime::cbor::serialize;
use fil_actors_runtime::runtime::builtins::Type;
use fil_actors_runtime::runtime::{
ActorCode, DomainSeparationTag, MessageInfo, Policy, Primitives, Runtime, RuntimePolicy,
Verifier, EMPTY_ARR_CID,
};
use fil_actors_runtime::test_utils::*;
use fil_actors_runtime::{actor_error, SendError};
use fil_actors_runtime::{
ActorError, BURNT_FUNDS_ACTOR_ADDR, CRON_ACTOR_ADDR, FIRST_NON_SINGLETON_ADDR, INIT_ACTOR_ADDR,
REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
Expand All @@ -42,7 +42,7 @@ use fvm_shared::clock::ChainEpoch;
use fvm_shared::consensus::ConsensusFault;
use fvm_shared::crypto::signature::Signature;
use fvm_shared::econ::TokenAmount;
use fvm_shared::error::{ErrorNumber, ExitCode};
use fvm_shared::error::ExitCode;
use fvm_shared::piece::PieceInfo;
use fvm_shared::randomness::Randomness;
use fvm_shared::randomness::RANDOMNESS_LENGTH;
Expand Down Expand Up @@ -893,7 +893,7 @@ impl<'invocation, 'bs> Runtime for InvocationCtx<'invocation, 'bs> {
value: TokenAmount,
_gas_limit: Option<u64>,
mut send_flags: SendFlags,
) -> Result<Response, ErrorNumber> {
) -> Result<Response, SendError> {
// replicate FVM by silently propagating read only flag to subcalls
if self.read_only() {
send_flags.set(SendFlags::READ_ONLY, true)
Expand Down

0 comments on commit 7fe9b77

Please sign in to comment.