merge syscall changes to master

.github/workflows/ci.yml

@@ -29,6 +29,7 @@ jobs:
   cargo:
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
         include:
           - name: check-clippy
@@ -79,7 +80,7 @@ jobs:
         version: v0.2.15
         # change this to invalidate sccache for this job
         shared-key: v1
-    - name: Runing ${{ matrix.command }}
+    - name: Running ${{ matrix.command }}
       uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
       with:
         command: ${{ matrix.command }}

fvm/src/call_manager/backtrace.rs

@@ -1,6 +1,5 @@
 use std::fmt::Display;
 
-use fvm_ipld_encoding::RawBytes;
 use fvm_shared::address::Address;
 use fvm_shared::error::{ErrorNumber, ExitCode};
 use fvm_shared::{ActorID, MethodNum};
@@ -77,8 +76,6 @@ pub struct Frame {
     pub source: ActorID,
     /// The method that was invoked.
     pub method: MethodNum,
-    /// The parameters passed to this method.
-    pub params: RawBytes,
     /// The exit code.
     pub code: ExitCode,
     /// The abort message.

fvm/src/call_manager/default.rs

@@ -1,22 +1,23 @@
 use anyhow::{anyhow, Context};
 use derive_more::{Deref, DerefMut};
-use fvm_ipld_encoding::{RawBytes, DAG_CBOR};
+use fvm_ipld_encoding::{to_vec, RawBytes, DAG_CBOR};
 use fvm_shared::actor::builtin::Type;
 use fvm_shared::address::{Address, Protocol};
 use fvm_shared::econ::TokenAmount;
 use fvm_shared::error::{ErrorNumber, ExitCode};
+use fvm_shared::sys::BlockId;
 use fvm_shared::{ActorID, MethodNum, METHOD_SEND};
 use num_traits::Zero;
 
 use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID};
 use crate::call_manager::backtrace::Frame;
 use crate::call_manager::FinishRet;
 use crate::gas::{Gas, GasTracker};
-use crate::kernel::{ExecutionError, Kernel, Result, SyscallError};
+use crate::kernel::{Block, BlockRegistry, ExecutionError, Kernel, Result, SyscallError};
 use crate::machine::Machine;
 use crate::syscalls::error::Abort;
 use crate::syscalls::{charge_for_exec, update_gas_available};
-use crate::trace::{ExecutionEvent, ExecutionTrace, SendParams};
+use crate::trace::{ExecutionEvent, ExecutionTrace};
 use crate::{account_actor, syscall_error};
 
 /// The default [`CallManager`] implementation.
@@ -86,20 +87,23 @@ where
         from: ActorID,
         to: Address,
         method: MethodNum,
-        params: &RawBytes,
+        params: Option<Block>,
         value: &TokenAmount,
     ) -> Result<InvocationResult>
     where
         K: Kernel<CallManager = Self>,
     {
         if self.machine.context().tracing {
-            self.exec_trace.push(ExecutionEvent::Call(SendParams {
+            self.exec_trace.push(ExecutionEvent::Call {
                 from,
                 to,
                 method,
-                params: params.clone(),
+                params: params
+                    .as_ref()
+                    .map(|blk| blk.data().to_owned().into())
+                    .unwrap_or_default(),
                 value: value.clone(),
-            }));
+            });
         }
 
         // We check _then_ set because we don't count the top call. This effectivly allows a
@@ -125,16 +129,23 @@ where
         self.call_stack_depth -= 1;
 
         if self.machine.context().tracing {
-            self.exec_trace.push(ExecutionEvent::Return(match result {
-                Err(ref e) => Err(match e {
-                    ExecutionError::OutOfGas => {
-                        SyscallError::new(ErrorNumber::Forbidden, "out of gas")
-                    }
-                    ExecutionError::Fatal(_) => SyscallError::new(ErrorNumber::Forbidden, "fatal"),
-                    ExecutionError::Syscall(s) => s.clone(),
-                }),
-                Ok(ref v) => Ok(v.clone()),
-            }));
+            self.exec_trace.push(match &result {
+                Ok(InvocationResult::Return(v)) => ExecutionEvent::CallReturn(
+                    v.as_ref()
+                        .map(|blk| RawBytes::from(blk.data().to_vec()))
+                        .unwrap_or_default(),
+                ),
+                Ok(InvocationResult::Failure(code)) => ExecutionEvent::CallAbort(*code),
+
+                Err(ExecutionError::OutOfGas) => ExecutionEvent::CallError(SyscallError::new(
+                    ErrorNumber::Forbidden,
+                    "out of gas",
+                )),
+                Err(ExecutionError::Fatal(_)) => {
+                    ExecutionEvent::CallError(SyscallError::new(ErrorNumber::Forbidden, "fatal"))
+                }
+                Err(ExecutionError::Syscall(s)) => ExecutionEvent::CallError(s.clone()),
+            });
         }
 
         result
@@ -233,15 +244,15 @@ where
 
         // Now invoke the constructor; first create the parameters, then
         // instantiate a new kernel to invoke the constructor.
-        let params = RawBytes::serialize(&addr)
+        let params = to_vec(&addr)
             // TODO(#198) this should be a Sys actor error, but we're copying lotus here.
             .map_err(|e| syscall_error!(Serialization; "failed to serialize params: {}", e))?;
 
         self.send_resolved::<K>(
             account_actor::SYSTEM_ACTOR_ID,
             id,
             fvm_shared::METHOD_CONSTRUCTOR,
-            &params,
+            Some(Block::new(DAG_CBOR, params)),
             &TokenAmount::from(0u32),
         )?;
 
@@ -254,7 +265,7 @@ where
         from: ActorID,
         to: Address,
         method: MethodNum,
-        params: &RawBytes,
+        params: Option<Block>,
         value: &TokenAmount,
     ) -> Result<InvocationResult>
     where
@@ -284,7 +295,7 @@ where
         from: ActorID,
         to: ActorID,
         method: MethodNum,
-        params: &RawBytes,
+        params: Option<Block>,
         value: &TokenAmount,
     ) -> Result<InvocationResult>
     where
@@ -310,31 +321,29 @@ where
             return Ok(InvocationResult::Return(Default::default()));
         }
 
+        // Store the parametrs, and initialize the block registry for the target actor.
+        // TODO: In M2, the block registry may have some form of shared block limit.
+        let mut block_registry = BlockRegistry::new();
+        let params_id = if let Some(blk) = params {
+            block_registry.put(blk)?
+        } else {
+            NO_DATA_BLOCK_ID
+        };
+
         // This is a cheap operation as it doesn't actually clone the struct,
         // it returns a referenced copy.
         let engine = self.engine().clone();
 
         log::trace!("calling {} -> {}::{}", from, to, method);
         self.map_mut(|cm| {
             // Make the kernel.
-            let mut kernel = K::new(cm, from, to, method, value.clone());
-
-            // Store parameters, if any.
-            let param_id = if params.len() > 0 {
-                match kernel.block_create(DAG_CBOR, params) {
-                    Ok(id) => id,
-                    // This could fail if we pass some global memory limit.
-                    Err(err) => return (Err(err), kernel.into_call_manager()),
-                }
-            } else {
-                super::NO_DATA_BLOCK_ID
-            };
+            let kernel = K::new(cm, block_registry, from, to, method, value.clone());
 
             // Make a store.
             let mut store = engine.new_store(kernel);
 
             // From this point on, there are no more syscall errors, only aborts.
-            let result: std::result::Result<RawBytes, Abort> = (|| {
+            let result: std::result::Result<BlockId, Abort> = (|| {
                 // Instantiate the module.
                 let instance = engine
                     .get_instance(&mut store, &state.code)
@@ -359,41 +368,42 @@ where
 
                 // Invoke it.
                 let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
-                    invoke.call(&mut store, (param_id,))
+                    invoke.call(&mut store, (params_id,))
                 }))
                 .map_err(|panic| Abort::Fatal(anyhow!("panic within actor: {:?}", panic)))?;
 
                 // Charge for any remaining uncharged execution gas, returning an error if we run
                 // out.
                 charge_for_exec(&mut store)?;
 
-                // If the invocation failed due to running out of exec_units, we have already detected it and returned OutOfGas above.
-                // Any other invocation failure is returned here as an Abort
-                let return_block_id = res?;
-
-                // Extract the return value, if there is one.
-                let return_value: RawBytes = if return_block_id > NO_DATA_BLOCK_ID {
-                    let (code, ret) = store
-                        .data_mut()
-                        .kernel
-                        .block_get(return_block_id)
-                        .map_err(|e| Abort::from_error(ExitCode::SYS_MISSING_RETURN, e))?;
-                    debug_assert_eq!(code, DAG_CBOR);
-                    RawBytes::new(ret)
-                } else {
-                    RawBytes::default()
-                };
-
-                Ok(return_value)
+                // If the invocation failed due to running out of exec_units, we have already
+                // detected it and returned OutOfGas above. Any other invocation failure is returned
+                // here as an Abort
+                Ok(res?)
             })();
 
             let invocation_data = store.into_data();
             let last_error = invocation_data.last_error;
-            let mut cm = invocation_data.kernel.into_call_manager();
+            let (mut cm, block_registry) = invocation_data.kernel.into_inner();
+
+            // Resolve the return block's ID into an actual block, converting to an abort if it
+            // doesn't exist.
+            let result = result.and_then(|ret_id| {
+                Ok(if ret_id == NO_DATA_BLOCK_ID {
+                    None
+                } else {
+                    Some(block_registry.get(ret_id).map_err(|_| {
+                        Abort::Exit(
+                            ExitCode::SYS_MISSING_RETURN,
+                            String::from("returned block does not exist"),
+                        )
+                    })?)
+                })
+            });
 
             // Process the result, updating the backtrace if necessary.
             let ret = match result {
-                Ok(value) => Ok(InvocationResult::Return(value)),
+                Ok(ret) => Ok(InvocationResult::Return(ret.cloned())),
                 Err(abort) => {
                     if let Some(err) = last_error {
                         cm.backtrace.begin(err);
@@ -419,7 +429,6 @@ where
                         source: to,
                         method,
                         message,
-                        params: params.clone(),
                         code,
                     });
 

fvm/src/call_manager/mod.rs

@@ -1,11 +1,10 @@
-use fvm_ipld_encoding::RawBytes;
 use fvm_shared::address::Address;
 use fvm_shared::econ::TokenAmount;
 use fvm_shared::error::ExitCode;
 use fvm_shared::{ActorID, MethodNum};
 
 use crate::gas::{GasCharge, GasTracker, PriceList};
-use crate::kernel::Result;
+use crate::kernel::{self, Result};
 use crate::machine::{Machine, MachineContext};
 use crate::state_tree::StateTree;
 use crate::Kernel;
@@ -52,7 +51,7 @@ pub trait CallManager: 'static {
         from: ActorID,
         to: Address,
         method: MethodNum,
-        params: &RawBytes,
+        params: Option<kernel::Block>,
         value: &TokenAmount,
     ) -> Result<InvocationResult>;
 
@@ -125,7 +124,7 @@ pub trait CallManager: 'static {
 #[derive(Clone, Debug)]
 pub enum InvocationResult {
     /// Indicates that the actor successfully returned. The value may be empty.
-    Return(RawBytes),
+    Return(Option<kernel::Block>),
     /// Indicates that the actor aborted with the given exit code.
     Failure(ExitCode),
 }

fvm/src/executor/default.rs

@@ -3,6 +3,7 @@ use std::result::Result as StdResult;
 
 use anyhow::{anyhow, Result};
 use cid::Cid;
+use fvm_ipld_encoding::{RawBytes, DAG_CBOR};
 use fvm_shared::actor::builtin::Type;
 use fvm_shared::address::Address;
 use fvm_shared::bigint::{BigInt, Sign};
@@ -16,7 +17,7 @@ use num_traits::Zero;
 use super::{ApplyFailure, ApplyKind, ApplyRet, Executor};
 use crate::call_manager::{backtrace, CallManager, InvocationResult};
 use crate::gas::{Gas, GasCharge, GasOutputs};
-use crate::kernel::{ClassifyResult, Context as _, ExecutionError, Kernel};
+use crate::kernel::{Block, ClassifyResult, Context as _, ExecutionError, Kernel};
 use crate::machine::{Machine, BURNT_FUNDS_ACTOR_ADDR, REWARD_ACTOR_ADDR};
 
 /// The default [`Executor`].
@@ -73,14 +74,21 @@ where
                 return (Err(e), cm.finish().1);
             }
 
+            let params = if msg.params.is_empty() {
+                None
+            } else {
+                Some(Block::new(DAG_CBOR, msg.params.bytes()))
+            };
+
             let result = cm.with_transaction(|cm| {
                 // Invoke the message.
-                let ret =
-                    cm.send::<K>(sender_id, msg.to, msg.method_num, &msg.params, &msg.value)?;
+                let ret = cm.send::<K>(sender_id, msg.to, msg.method_num, params, &msg.value)?;
 
                 // Charge for including the result (before we end the transaction).
-                if let InvocationResult::Return(data) = &ret {
-                    cm.charge_gas(cm.context().price_list.on_chain_return_value(data.len()))?;
+                if let InvocationResult::Return(value) = &ret {
+                    cm.charge_gas(cm.context().price_list.on_chain_return_value(
+                        value.as_ref().map(|v| v.size() as usize).unwrap_or(0),
+                    ))?;
                 }
 
                 Ok(ret)
@@ -94,7 +102,13 @@ where
 
         // Extract the exit code and build the result of the message application.
         let receipt = match res {
-            Ok(InvocationResult::Return(return_data)) => {
+            Ok(InvocationResult::Return(return_value)) => {
+                // Convert back into a top-level return "value". We throw away the codec here,
+                // unfortunately.
+                let return_data = return_value
+                    .map(|blk| RawBytes::from(blk.data().to_vec()))
+                    .unwrap_or_default();
+
                 backtrace.clear();
                 Receipt {
                     exit_code: ExitCode::OK,
@@ -124,16 +138,7 @@ where
                 let exit_code = match err.1 {
                     ErrorNumber::InsufficientFunds => ExitCode::SYS_INSUFFICIENT_FUNDS,
                     ErrorNumber::NotFound => ExitCode::SYS_INVALID_RECEIVER,
-
-                    ErrorNumber::IllegalArgument => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::IllegalOperation => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::LimitExceeded => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::AssertionFailed => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::InvalidHandle => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::IllegalCid => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::IllegalCodec => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::Serialization => ExitCode::SYS_ASSERTION_FAILED,
-                    ErrorNumber::Forbidden => ExitCode::SYS_ASSERTION_FAILED,
+                    _ => ExitCode::SYS_ASSERTION_FAILED,
                 };
 
                 backtrace.begin(backtrace::Cause::from_syscall("send", "send", err));