diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index bcdb15153..2edf9cde4 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -231,9 +231,6 @@ where ExecutionEvent::CallError(SyscallError::new(ErrorNumber::Forbidden, "fatal")) } Err(ExecutionError::Syscall(s)) => ExecutionEvent::CallError(s.clone()), - Err(ExecutionError::Abort(_)) => { - ExecutionEvent::CallError(SyscallError::new(ErrorNumber::Forbidden, "aborted")) - } }); } @@ -723,9 +720,10 @@ where // From this point on, there are no more syscall errors, only aborts. let result: std::result::Result = (|| { + let code = &state.code; // Instantiate the module. let instance = engine - .instantiate(&mut store, &state.code)? + .instantiate(&mut store, code)? .context("actor not found") .map_err(Abort::Fatal)?; @@ -740,7 +738,11 @@ where let func = match instance.get_func(&mut store, entrypoint.func_name()) { Some(func) => func, None => { - return Err(Abort::EntrypointNotFound); + return Err(Abort::Exit( + ExitCode::SYS_INVALID_RECEIVER, + format!("cannot upgrade to {code}"), + 0, + )); } }; @@ -812,14 +814,6 @@ where Err(ExecutionError::Fatal(anyhow!(e))), ), }, - Abort::EntrypointNotFound => ( - ExitCode::USR_FORBIDDEN, - "entrypoint not found".to_owned(), - Err(ExecutionError::Syscall(SyscallError::new( - ErrorNumber::Forbidden, - "entrypoint not found", - ))), - ), Abort::OutOfGas => ( ExitCode::SYS_OUT_OF_GAS, "out of gas".to_owned(), diff --git a/fvm/src/call_manager/mod.rs b/fvm/src/call_manager/mod.rs index 810645a8c..84d4387dd 100644 --- a/fvm/src/call_manager/mod.rs +++ b/fvm/src/call_manager/mod.rs @@ -10,10 +10,9 @@ use fvm_shared::{ActorID, MethodNum}; use crate::engine::Engine; use crate::gas::{Gas, GasCharge, GasTimer, GasTracker, PriceList}; -use crate::kernel::{self, BlockRegistry, Result}; +use crate::kernel::{self, BlockRegistry, ClassifyResult, Context, Result}; use crate::machine::{Machine, MachineContext}; use crate::state_tree::ActorState; -use crate::syscalls::error::Abort; use crate::Kernel; pub mod backtrace; @@ -235,9 +234,9 @@ impl Entrypoint { match self { Entrypoint::Invoke(_) => Ok(Vec::new()), Entrypoint::Upgrade(ui) => { - let ui_params = to_vec(&ui).map_err(|e| { - Abort::Fatal(anyhow::anyhow!("failed to serialize upgrade params: {}", e)) - })?; + let ui_params = to_vec(&ui) + .or_fatal() + .context("failed to serialize upgrade params")?; // This is CBOR instead of DAG_CBOR because these params are not reachable let block_id = br.put_reachable(kernel::Block::new(CBOR, ui_params, Vec::new()))?; Ok(vec![wasmtime::Val::I32(block_id as i32)]) diff --git a/fvm/src/executor/default.rs b/fvm/src/executor/default.rs index 12e9a631e..69628221c 100644 --- a/fvm/src/executor/default.rs +++ b/fvm/src/executor/default.rs @@ -201,9 +201,6 @@ where events_root, } } - - Err(ExecutionError::Abort(e)) => return Err(anyhow!("actor aborted: {}", e)), - Err(ExecutionError::OutOfGas) => Receipt { exit_code: ExitCode::SYS_OUT_OF_GAS, return_data: Default::default(), diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 73c282ace..f28b2fd5d 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -5,7 +5,6 @@ use std::convert::{TryFrom, TryInto}; use std::panic::{self, UnwindSafe}; use std::path::PathBuf; -use crate::syscalls::error::Abort; use anyhow::{anyhow, Context as _}; use cid::Cid; use filecoin_proofs_api::{self as proofs, ProverId, PublicReplicaInfo, SectorId}; @@ -874,7 +873,11 @@ where .create_actor(code_id, actor_id, delegated_address) } - fn upgrade_actor(&mut self, new_code_cid: Cid, params_id: BlockId) -> Result { + fn upgrade_actor( + &mut self, + new_code_cid: Cid, + params_id: BlockId, + ) -> Result { if self.read_only { return Err( syscall_error!(ReadOnly, "upgrade_actor cannot be called while read-only").into(), @@ -938,27 +941,18 @@ where }); match result { - Ok(InvocationResult { - exit_code: ExitCode::OK, - value, - }) => { - let block_id = match value { - None => NO_DATA_BLOCK_ID, - Some(block) => self.blocks.put_reachable(block).unwrap_or_else(|_| { - log::error!("failed to write to kernel block registry"); - NO_DATA_BLOCK_ID - }), + Ok(InvocationResult { exit_code, value }) => { + let (block_stat, block_id) = match value { + None => (BlockStat { codec: 0, size: 0 }, NO_DATA_BLOCK_ID), + // TODO: Check error cases. At a minimum, we could run out of gas here! + Some(block) => (block.stat(), self.blocks.put_reachable(block)?), }; - Err(ExecutionError::Abort(Abort::Exit( - ExitCode::OK, - "actor upgraded".to_string(), + Ok(SendResult { block_id, - ))) + block_stat, + exit_code, + }) } - Ok(InvocationResult { - exit_code, - value: _, - }) => Ok(exit_code.value()), Err(err) => Err(err), } } diff --git a/fvm/src/kernel/error.rs b/fvm/src/kernel/error.rs index bf9dc9dd7..4b7010324 100644 --- a/fvm/src/kernel/error.rs +++ b/fvm/src/kernel/error.rs @@ -5,8 +5,6 @@ use std::fmt::Display; use derive_more::Display; use fvm_shared::error::ErrorNumber; -use crate::syscalls::error::Abort; - /// Execution result. pub type Result = std::result::Result; @@ -37,7 +35,6 @@ pub enum ExecutionError { OutOfGas, Syscall(SyscallError), Fatal(anyhow::Error), - Abort(Abort), } impl ExecutionError { @@ -55,7 +52,6 @@ impl ExecutionError { match self { Fatal(_) => true, OutOfGas | Syscall(_) => false, - Abort(_) => true, } } @@ -65,7 +61,7 @@ impl ExecutionError { use ExecutionError::*; match self { Syscall(_) => true, - OutOfGas | Fatal(_) | Abort(_) => false, + OutOfGas | Fatal(_) => false, } } } @@ -78,12 +74,6 @@ impl From for ExecutionError { } } -impl From for ExecutionError { - fn from(e: Abort) -> Self { - ExecutionError::Abort(e) - } -} - pub trait ClassifyResult: Sized { type Value; type Error; @@ -158,7 +148,6 @@ impl Context for ExecutionError { Syscall(e) => Syscall(SyscallError(format!("{}: {}", context, e.0), e.1)), Fatal(e) => Fatal(e.context(context.to_string())), OutOfGas => OutOfGas, // no reason necessary - Abort(_) => self, } } @@ -179,7 +168,6 @@ impl From for anyhow::Error { OutOfGas => anyhow::anyhow!("out of gas"), Syscall(err) => anyhow::anyhow!(err.0), Fatal(err) => err, - Abort(e) => anyhow::anyhow!("aborted: {}", e), } } } diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index ad6626fe6..a47338003 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -209,7 +209,11 @@ pub trait ActorOps { delegated_address: Option
, ) -> Result<()>; - fn upgrade_actor(&mut self, new_code_cid: Cid, params_id: BlockId) -> Result; + fn upgrade_actor( + &mut self, + new_code_cid: Cid, + params_id: BlockId, + ) -> Result; /// Installs actor code pointed by cid #[cfg(feature = "m2-native")] diff --git a/fvm/src/syscalls/actor.rs b/fvm/src/syscalls/actor.rs index 5bbff82cf..9aeba3038 100644 --- a/fvm/src/syscalls/actor.rs +++ b/fvm/src/syscalls/actor.rs @@ -3,8 +3,10 @@ use anyhow::{anyhow, Context as _}; use fvm_shared::{sys, ActorID}; +use super::bind::ControlFlow; +use super::error::Abort; use super::Context; -use crate::kernel::{ClassifyResult, Result}; +use crate::kernel::{ClassifyResult, Result, SendResult}; use crate::{syscall_error, Kernel}; pub fn resolve_address( @@ -113,10 +115,31 @@ pub fn upgrade_actor( context: Context<'_, K>, new_code_cid_off: u32, params_id: u32, -) -> Result { - let cid = context.memory.read_cid(new_code_cid_off)?; - - context.kernel.upgrade_actor::(cid, params_id) +) -> ControlFlow { + let cid = match context.memory.read_cid(new_code_cid_off) { + Ok(cid) => cid, + Err(err) => return err.into(), + }; + + match context.kernel.upgrade_actor::(cid, params_id) { + Ok(SendResult { + block_id, + block_stat, + exit_code, + }) => { + if exit_code.is_success() { + ControlFlow::Abort(Abort::Exit(exit_code, String::new(), block_id)) + } else { + ControlFlow::Return(sys::out::send::Send { + exit_code: exit_code.value(), + return_id: block_id, + return_codec: block_stat.codec, + return_size: block_stat.size, + }) + } + } + Err(err) => err.into(), + } } pub fn get_builtin_actor_type( diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index f3a5967d9..f40317ea2 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -49,39 +49,69 @@ pub(super) trait BindSyscall { ) -> anyhow::Result<&mut Self>; } +pub enum ControlFlow { + Return(T), + Error(SyscallError), + Abort(Abort), +} + +impl From for ControlFlow { + fn from(value: ExecutionError) -> Self { + match value { + ExecutionError::Syscall(err) => ControlFlow::Error(err), + ExecutionError::Fatal(err) => ControlFlow::Abort(Abort::Fatal(err)), + ExecutionError::OutOfGas => ControlFlow::Abort(Abort::OutOfGas), + } + } +} + /// The helper trait used by `BindSyscall` to convert kernel results with execution errors into /// results that can be handled by wasmtime. See the documentation on `BindSyscall` for details. #[doc(hidden)] -pub trait IntoSyscallResult: Sized { +pub trait IntoControlFlow: Sized { type Value: SyscallSafe; - fn into(self) -> Result, Abort>; + fn into_control_flow(self) -> ControlFlow; +} + +/// An uninhabited type. We use this in `abort` to make sure there's no way to return without +/// returning an error. +#[derive(Copy, Clone)] +#[doc(hidden)] +pub enum Never {} +unsafe impl SyscallSafe for Never {} + +// Implementations for syscalls that always abort. +impl IntoControlFlow for Abort { + type Value = Never; + fn into_control_flow(self) -> ControlFlow { + ControlFlow::Abort(self) + } } -// Implementations for syscalls that abort on error. -impl IntoSyscallResult for Result +// Implementations for syscalls that can abort. +impl IntoControlFlow for ControlFlow where T: SyscallSafe, { type Value = T; - fn into(self) -> Result, Abort> { - Ok(Ok(self?)) + fn into_control_flow(self) -> ControlFlow { + self } } // Implementations for normal syscalls. -impl IntoSyscallResult for kernel::Result +impl IntoControlFlow for kernel::Result where T: SyscallSafe, { type Value = T; - fn into(self) -> Result, Abort> { + fn into_control_flow(self) -> ControlFlow { match self { - Ok(value) => Ok(Ok(value)), + Ok(value) => ControlFlow::Return(value), Err(e) => match e { - ExecutionError::Syscall(err) => Ok(Err(err)), - ExecutionError::OutOfGas => Err(Abort::OutOfGas), - ExecutionError::Fatal(err) => Err(Abort::Fatal(err)), - ExecutionError::Abort(e) => Err(e), + ExecutionError::Syscall(err) => ControlFlow::Error(err), + ExecutionError::OutOfGas => ControlFlow::Abort(Abort::OutOfGas), + ExecutionError::Fatal(err) => ControlFlow::Abort(Abort::Fatal(err)), }, } } @@ -112,7 +142,7 @@ macro_rules! impl_bind_syscalls { where K: Kernel, Func: Fn(Context<'_, K> $(, $t)*) -> Ret + Send + Sync + 'static, - Ret: IntoSyscallResult, + Ret: IntoControlFlow, $($t: WasmTy+SyscallSafe,)* { fn bind( @@ -130,21 +160,21 @@ macro_rules! impl_bind_syscalls { charge_syscall_gas!(data.kernel); let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; - let out = syscall(ctx $(, $t)*).into(); + let out = syscall(ctx $(, $t)*).into_control_flow(); let result = match out { - Ok(Ok(_)) => { + ControlFlow::Return(_) => { log::trace!("syscall {}::{}: ok", module, name); data.last_error = None; Ok(0) }, - Ok(Err(err)) => { + ControlFlow::Error(err) => { let code = err.1; log::trace!("syscall {}::{}: fail ({})", module, name, code as u32); data.last_error = Some(backtrace::Cause::from_syscall(module, name, err)); Ok(code as u32) }, - Err(e) => Err(e.into()), + ControlFlow::Abort(abort) => Err(abort.into()), }; update_gas_available(&mut caller)?; @@ -168,8 +198,8 @@ macro_rules! impl_bind_syscalls { } let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; - let result = match syscall(ctx $(, $t)*).into() { - Ok(Ok(value)) => { + let result = match syscall(ctx $(, $t)*).into_control_flow() { + ControlFlow::Return(value) => { log::trace!("syscall {}::{}: ok", module, name); unsafe { // We're writing into a user-specified pointer, so avoid @@ -179,13 +209,13 @@ macro_rules! impl_bind_syscalls { data.last_error = None; Ok(0) }, - Ok(Err(err)) => { + ControlFlow::Error(err) => { let code = err.1; log::trace!("syscall {}::{}: fail ({})", module, name, code as u32); data.last_error = Some(backtrace::Cause::from_syscall(module, name, err)); Ok(code as u32) }, - Err(e) => Err(e.into()), + ControlFlow::Abort(abort) => Err(abort.into()), }; update_gas_available(&mut caller)?; diff --git a/fvm/src/syscalls/context.rs b/fvm/src/syscalls/context.rs index 5a3d7a53a..fac49f9b2 100644 --- a/fvm/src/syscalls/context.rs +++ b/fvm/src/syscalls/context.rs @@ -152,9 +152,6 @@ mod test { $crate::kernel::ExecutionError::OutOfGas => { panic!("got unexpected out of gas") } - $crate::kernel::ExecutionError::Abort(abort) => { - panic!("got unexpected abort {}", abort) - } } }; } diff --git a/fvm/src/syscalls/error.rs b/fvm/src/syscalls/error.rs index f786ccded..1dbd81aaa 100644 --- a/fvm/src/syscalls/error.rs +++ b/fvm/src/syscalls/error.rs @@ -17,9 +17,6 @@ pub enum Abort { /// The actor ran out of gas. #[error("out of gas")] OutOfGas, - /// The actor did not export the endpoint that was called. - #[error("entrypoint not found")] - EntrypointNotFound, /// The system failed with a fatal error. #[error("fatal error: {0}")] Fatal(anyhow::Error), @@ -40,7 +37,6 @@ impl Abort { ), ExecutionError::OutOfGas => Abort::OutOfGas, ExecutionError::Fatal(err) => Abort::Fatal(err), - ExecutionError::Abort(e) => e, } } @@ -50,7 +46,6 @@ impl Abort { ExecutionError::OutOfGas => Abort::OutOfGas, ExecutionError::Fatal(e) => Abort::Fatal(e), ExecutionError::Syscall(e) => Abort::Fatal(anyhow!("unexpected syscall error: {}", e)), - ExecutionError::Abort(e) => e, } } } diff --git a/fvm/src/syscalls/vm.rs b/fvm/src/syscalls/vm.rs index 8b8834997..1e5ad4218 100644 --- a/fvm/src/syscalls/vm.rs +++ b/fvm/src/syscalls/vm.rs @@ -2,19 +2,11 @@ // SPDX-License-Identifier: Apache-2.0, MIT use fvm_shared::error::ExitCode; use fvm_shared::sys::out::vm::MessageContext; -use fvm_shared::sys::SyscallSafe; use super::error::Abort; use super::Context; use crate::kernel::Kernel; -/// An uninhabited type. We use this in `abort` to make sure there's no way to return without -/// returning an error. -#[derive(Copy, Clone)] -pub enum Never {} - -unsafe impl SyscallSafe for Never {} - /// The maximum message length included in the backtrace. Given 1024 levels, this gives us a total /// maximum of around 1MiB for debugging. const MAX_MESSAGE_LEN: usize = 1024; @@ -26,14 +18,14 @@ pub fn exit( blk: u32, message_off: u32, message_len: u32, -) -> Result { +) -> Abort { let code = ExitCode::new(code); if !code.is_success() && code.is_system_error() { - return Err(Abort::Exit( + return Abort::Exit( ExitCode::SYS_ILLEGAL_EXIT_CODE, format!("actor aborted with code {}", code), blk, - )); + ); } let message = if message_len == 0 { @@ -57,7 +49,7 @@ pub fn exit( Err(e) => format!("failed to extract error message: {e}"), } }; - Err(Abort::Exit(code, message, blk)) + Abort::Exit(code, message, blk) } pub fn message_context(context: Context<'_, impl Kernel>) -> crate::kernel::Result { diff --git a/fvm/tests/default_kernel/mod.rs b/fvm/tests/default_kernel/mod.rs index 8ef956176..3827e9f45 100644 --- a/fvm/tests/default_kernel/mod.rs +++ b/fvm/tests/default_kernel/mod.rs @@ -76,9 +76,6 @@ macro_rules! expect_syscall_err { ::fvm::kernel::ExecutionError::OutOfGas => { panic!("got unexpected out of gas") } - ::fvm::kernel::ExecutionError::Abort(abort) => { - panic!("got unexpected abort {}", abort) - } } }; } @@ -94,9 +91,6 @@ macro_rules! expect_out_of_gas { ::fvm::kernel::ExecutionError::Fatal(err) => { panic!("got unexpected fatal error: {}", err) } - ::fvm::kernel::ExecutionError::Abort(abort) => { - panic!("got unexpected abort {}", abort) - } } }; } diff --git a/sdk/src/actor.rs b/sdk/src/actor.rs index b0f8bf511..9e798babb 100644 --- a/sdk/src/actor.rs +++ b/sdk/src/actor.rs @@ -1,14 +1,13 @@ // Copyright 2021-2023 Protocol Labs // SPDX-License-Identifier: Apache-2.0, MIT -use core::option::Option; use std::ptr; // no_std use cid::Cid; use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_shared::address::{Address, Payload, MAX_ADDRESS_LEN}; use fvm_shared::econ::TokenAmount; -use fvm_shared::error::ErrorNumber; -use fvm_shared::{ActorID, MAX_CID_LEN}; +use fvm_shared::error::{ErrorNumber, ExitCode}; +use fvm_shared::{ActorID, Response, MAX_CID_LEN}; use log::error; use crate::{sys, SyscallResult, NO_DATA_BLOCK_ID}; @@ -109,7 +108,7 @@ pub fn create_actor( } /// Upgrades an actor using the given block which includes the old code cid and the upgrade params -pub fn upgrade_actor(new_code_cid: Cid, params: Option) -> SyscallResult { +pub fn upgrade_actor(new_code_cid: Cid, params: Option) -> SyscallResult { unsafe { let cid = new_code_cid.to_bytes(); @@ -118,7 +117,35 @@ pub fn upgrade_actor(new_code_cid: Cid, params: Option) -> SyscallRes None => NO_DATA_BLOCK_ID, }; - sys::actor::upgrade_actor(cid.as_ptr(), params_id) + let fvm_shared::sys::out::send::Send { + exit_code, + return_id, + return_codec, + return_size, + } = sys::actor::upgrade_actor(cid.as_ptr(), params_id)?; + + // Process the result. + // TODO: dedup with the send syscall... + let exit_code = ExitCode::new(exit_code); + let return_data = if return_id == NO_DATA_BLOCK_ID { + None + } else { + // Allocate a buffer to read the return data. + let mut bytes = vec![0; return_size as usize]; + + // Now read the return data. + let unread = sys::ipld::block_read(return_id, 0, bytes.as_mut_ptr(), return_size)?; + assert_eq!(0, unread); + Some(IpldBlock { + codec: return_codec, + data: bytes.to_vec(), + }) + }; + + Ok(Response { + exit_code, + return_data, + }) } } diff --git a/sdk/src/sys/actor.rs b/sdk/src/sys/actor.rs index 25f9a8ebc..ad93ca1c8 100644 --- a/sdk/src/sys/actor.rs +++ b/sdk/src/sys/actor.rs @@ -2,6 +2,9 @@ // SPDX-License-Identifier: Apache-2.0, MIT //! Syscalls for creating and resolving actors. +#[doc(inline)] +pub use fvm_shared::sys::out::send::*; + // for documentation links #[cfg(doc)] use crate::sys::ErrorNumber::*; @@ -139,11 +142,11 @@ super::fvm_syscalls! { /// /// # Returns /// - /// On success, this syscall will not return. Instead, the current invocation will "complete" and - /// the return value will be the block returned by the new code's `upgrade` endpoint. + /// On successful upgrade, this syscall will not return. Instead, the current invocation will + /// "complete" and the return value will be the block returned by the new code's `upgrade` endpoint. /// /// If the new code rejects the upgrade (aborts) or performs an illegal operation, this syscall will - /// return the exit code of the `upgrade` endpoint. + /// return the exit code plus the error returned by the upgrade endpoint. /// /// Finally, the syscall will return an error if it fails to call the upgrade endpoint entirely. /// @@ -159,7 +162,7 @@ super::fvm_syscalls! { pub fn upgrade_actor( new_code_cid_off: *const u8, params: u32, - ) -> Result; + ) -> Result; /// Installs and ensures actor code is valid and loaded. /// **Privileged:** May only be called by the init actor. diff --git a/shared/src/error/mod.rs b/shared/src/error/mod.rs index fb4a06a9a..3f4712304 100644 --- a/shared/src/error/mod.rs +++ b/shared/src/error/mod.rs @@ -64,7 +64,8 @@ impl ExitCode { //pub const SYS_RESERVED_3 ExitCode = ExitCode::new(3); /// The message receiver trapped (panicked). pub const SYS_ILLEGAL_INSTRUCTION: ExitCode = ExitCode::new(4); - /// The message receiver doesn't exist and can't be automatically created + /// The message receiver either doesn't exist and can't be automatically created or it doesn't + /// implement the required entrypoint. pub const SYS_INVALID_RECEIVER: ExitCode = ExitCode::new(5); /// The message sender didn't have the requisite funds. pub const SYS_INSUFFICIENT_FUNDS: ExitCode = ExitCode::new(6);