Merge pull request #3 from adlrocha/rebase

Rebase master and fix actor type IDs for HC

fvm/CHANGELOG.md

@@ -3,10 +3,25 @@
 Changes to the reference FVM implementation.
 
 ## Unreleased
+
+- ...
+
+## 0.7.2 [2022-05-09]
 
-- Added `testing` feature to change module visibility 
-- Changed visibility of `account_actor`, `init_actor` and `system_actor` to public to use them in the integration test
-framework.
+- Add `testing` feature to change module visibility; concretely changed
+  visibility of `account_actor`, `init_actor` and `system_actor` to `pub`
+  to use them in the integration test framework.
+- Propagate gas outputs in ApplyRet.
+- Migrate CBOR serde to [cbor4ii](https://github.com/quininer/cbor4ii).
+- Instrument Wasm bytecode with [filecoin-project/fvm-wasm-instrument](https://github.com/filecoin-project/fvm-wasm-instrument), 
+  a fork of [paritytech/wasm-instrument](https://github.com/paritytech/wasm-instrument)
+  for more accurate stack accounting and execution units metering.
+- Abort when aborting fails.
+- Fix syscall binding docs. 
+- Fix bugs in Wasm execution units gas accounting.
+- Fix system actor state serialization.
+- Remove unused dependencies from build graph.
+- Optimize memory resolution so it only happens once.
 
 ## 0.7.1 [2022-04-18]
 
@@ -37,4 +52,4 @@ BREAKING: Updates the FVM to the latest syscall struct alignment
      - `StateTree::consume` -> `StateTree::into_store`
 - BREAKING: remove unused (by the FVM) `verify_post_discount` from the FVM PriceList.
 
-[FIP0032]: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0032.md
+[FIP0032]: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0032.md

fvm/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "fvm"
 description = "Filecoin Virtual Machine reference implementation"
-version = "0.7.1"
+version = "0.7.2"
 license = "MIT OR Apache-2.0"
 authors = ["Protocol Labs", "Filecoin Core Devs"]
 edition = "2021"
@@ -39,6 +39,8 @@ log = "0.4.14"
 byteorder = "1.4.3"
 anymap = "0.12.1"
 blake2b_simd = "1.0.0"
+fvm-wasm-instrument = { version = "0.2.0", features = ["bulk"] }
+yastl = "0.1.2"
 
 [dependencies.wasmtime]
 version = "0.35.2"

fvm/src/call_manager/backtrace.rs

@@ -7,7 +7,9 @@ use fvm_shared::{ActorID, MethodNum};
 
 use crate::kernel::SyscallError;
 
-/// A call backtrace records _why_ an actor exited with a specific error code.
+/// A call backtrace records the actors an error was propagated through, from
+/// the moment it was emitted. The original error is the _cause_. Backtraces are
+/// useful for identifying the root cause of an error.
 #[derive(Debug, Default, Clone)]
 pub struct Backtrace {
     /// The actors through which this error was propagated from bottom (source) to top.
@@ -34,22 +36,35 @@ impl Backtrace {
         self.frames.is_empty() && self.cause.is_none()
     }
 
-    /// Clear the backtrace. This should be called:
-    ///
-    /// 1. Before all syscalls except "abort"
-    /// 2. After an actor returns with a 0 exit code.
+    /// Clear the backtrace.
     pub fn clear(&mut self) {
         self.cause = None;
         self.frames.clear();
     }
 
-    /// Set the backtrace cause. If there is an existing backtrace, this will clear it.
-    pub fn set_cause(&mut self, cause: Cause) {
+    /// Begins a new backtrace. If there is an existing backtrace, this will clear it.
+    ///
+    /// Note: Backtraces are populated _backwards_. That is, a frame is inserted
+    /// every time an actor returns. That's why `begin()` resets any currently
+    /// accumulated state, as once an error occurs, we want to track its
+    /// propagation all the way up.
+    pub fn begin(&mut self, cause: Cause) {
         self.cause = Some(cause);
         self.frames.clear();
     }
 
+    /// Sets the cause of a backtrace.
+    ///
+    /// This is useful to stamp a backtrace with its cause after the frames
+    /// have been collected, such as when we ultimately handle a fatal error at
+    /// the top of its propagation chain.
+    pub fn set_cause(&mut self, cause: Cause) {
+        self.cause = Some(cause);
+    }
+
     /// Push a "frame" (actor exit) onto the backtrace.
+    ///
+    /// This should be called every time an actor exits.
     pub fn push_frame(&mut self, frame: Frame) {
         self.frames.push(frame)
     }
@@ -85,34 +100,69 @@ impl Display for Frame {
 
 /// The ultimate "cause" of a failed message.
 #[derive(Clone, Debug)]
-pub struct Cause {
-    /// The syscall "module".
-    pub module: &'static str,
-    /// The syscall function name.
-    pub function: &'static str,
-    /// The exact syscall error.
-    pub error: ErrorNumber,
-    /// The informational syscall message.
-    pub message: String,
+pub enum Cause {
+    /// The original cause was a syscall error.
+    Syscall {
+        /// The syscall "module".
+        module: &'static str,
+        /// The syscall function name.
+        function: &'static str,
+        /// The exact syscall error.
+        error: ErrorNumber,
+        /// The informational syscall message.
+        message: String,
+    },
+    /// The original cause was a fatal error.
+    Fatal {
+        /// The alternate-formatted message from the anyhow error.
+        error_msg: String,
+        /// The backtrace, captured if the relevant
+        /// [environment variables](https://doc.rust-lang.org/std/backtrace/index.html#environment-variables) are enabled.
+        backtrace: String,
+    },
 }
 
 impl Cause {
-    pub fn new(module: &'static str, function: &'static str, err: SyscallError) -> Self {
-        Self {
+    /// Records a failing syscall as the cause of a backtrace.
+    pub fn from_syscall(module: &'static str, function: &'static str, err: SyscallError) -> Self {
+        Self::Syscall {
             module,
             function,
             error: err.1,
             message: err.0,
         }
     }
+
+    /// Records a fatal error as the cause of a backtrace.
+    pub fn from_fatal(err: anyhow::Error) -> Self {
+        Self::Fatal {
+            error_msg: format!("{:#}", err),
+            backtrace: err.backtrace().to_string(),
+        }
+    }
 }
 
 impl Display for Cause {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(
-            f,
-            "{}::{} -- {} ({}: {})",
-            self.module, self.function, &self.message, self.error as u32, self.error,
-        )
+        match self {
+            Cause::Syscall {
+                module,
+                function,
+                error,
+                message,
+            } => {
+                write!(
+                    f,
+                    "{}::{} -- {} ({}: {})",
+                    module, function, &message, *error as u32, error,
+                )
+            }
+            Cause::Fatal {
+                error_msg,
+                backtrace,
+            } => {
+                write!(f, "[FATAL] Error: {}, Backtrace:\n{}", error_msg, backtrace)
+            }
+        }
     }
 }

fvm/src/call_manager/default.rs

@@ -1,23 +1,21 @@
-use std::cmp::max;
-
 use anyhow::Context;
 use derive_more::{Deref, DerefMut};
 use fvm_ipld_encoding::{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::version::NetworkVersion;
 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::GasTracker;
-use crate::kernel::{ClassifyResult, ExecutionError, Kernel, Result, SyscallError};
+use crate::gas::{Gas, GasTracker};
+use crate::kernel::{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::{account_actor, syscall_error};
 
@@ -73,7 +71,7 @@ where
     fn new(machine: M, gas_limit: i64, origin: Address, nonce: u64) -> Self {
         DefaultCallManager(Some(Box::new(InnerDefaultCallManager {
             machine,
-            gas_tracker: GasTracker::new(gas_limit, 0),
+            gas_tracker: GasTracker::new(Gas::new(gas_limit), Gas::zero()),
             origin,
             nonce,
             num_actors_created: 0,
@@ -156,7 +154,7 @@ where
     }
 
     fn finish(mut self) -> (FinishRet, Self::Machine) {
-        let gas_used = self.gas_tracker.gas_used().max(0);
+        let gas_used = self.gas_tracker.gas_used().max(Gas::zero()).round_up();
 
         let inner = self.0.take().expect("call manager is poisoned");
         // TODO: Having to check against zero here is fishy, but this is what lotus does.
@@ -316,7 +314,6 @@ where
         // it returns a referenced copy.
         let engine = self.engine().clone();
 
-        let gas_available = self.gas_tracker.gas_available();
         log::trace!("calling {} -> {}::{}", from, to, method);
         self.map_mut(|cm| {
             // Make the kernel.
@@ -334,56 +331,38 @@ where
             };
 
             // Make a store.
-            let gas_used = kernel.gas_used();
-            let exec_units_to_add = match kernel.network_version() {
-                NetworkVersion::V14 | NetworkVersion::V15 => i64::MAX,
-                _ => kernel
-                    .price_list()
-                    .gas_to_exec_units(max(gas_available.saturating_sub(gas_used), 0), false),
-            };
-
             let mut store = engine.new_store(kernel);
-            if let Err(err) = store.add_fuel(u64::try_from(exec_units_to_add).unwrap_or(0)) {
-                return (
-                    Err(ExecutionError::Fatal(err)),
-                    store.into_data().kernel.into_call_manager(),
-                );
-            }
-
-            // Instantiate the module.
-            let instance = match engine
-                .get_instance(&mut store, &state.code)
-                .and_then(|i| i.context("actor code not found"))
-                .or_fatal()
-            {
-                Ok(ret) => ret,
-                Err(err) => return (Err(err), store.into_data().kernel.into_call_manager()),
-            };
 
             // From this point on, there are no more syscall errors, only aborts.
             let result: std::result::Result<RawBytes, Abort> = (|| {
+                // Instantiate the module.
+                let instance = engine
+                    .get_instance(&mut store, &state.code)
+                    .and_then(|i| i.context("actor code not found"))
+                    .map_err(Abort::Fatal)?;
+
+                // Resolve and store a reference to the exported memory.
+                let memory = instance
+                    .get_memory(&mut store, "memory")
+                    .context("actor has no memory export")
+                    .map_err(Abort::Fatal)?;
+                store.data_mut().memory = memory;
+
                 // Lookup the invoke method.
                 let invoke: wasmtime::TypedFunc<(u32,), u32> = instance
                     .get_typed_func(&mut store, "invoke")
                     // All actors will have an invoke method.
                     .map_err(Abort::Fatal)?;
 
+                // Set the available gas.
+                update_gas_available(&mut store)?;
+
                 // Invoke it.
                 let res = invoke.call(&mut store, (param_id,));
 
-                // Charge gas for the "latest" use of execution units (all the exec units used since the most recent syscall)
-                // We do this by first loading the _total_ execution units consumed
-                let exec_units_consumed = store
-                    .fuel_consumed()
-                    .context("expected to find fuel consumed")
-                    .map_err(Abort::Fatal)?;
-                // Then, pass the _total_ exec_units_consumed to the InvocationData,
-                // which knows how many execution units had been consumed at the most recent snapshot
-                // It will charge gas for the delta between the total units (the number we provide) and its snapshot
-                store
-                    .data_mut()
-                    .charge_gas_for_exec_units(exec_units_consumed)
-                    .map_err(|e| Abort::from_error(ExitCode::SYS_ASSERTION_FAILED, e))?;
+                // 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
@@ -414,7 +393,7 @@ where
                 Ok(value) => Ok(InvocationResult::Return(value)),
                 Err(abort) => {
                     if let Some(err) = last_error {
-                        cm.backtrace.set_cause(err);
+                        cm.backtrace.begin(err);
                     }
 
                     let (code, message, res) = match abort {

fvm/src/call_manager/mod.rs

@@ -116,7 +116,7 @@ pub trait CallManager: 'static {
 
     /// Charge gas.
     fn charge_gas(&mut self, charge: GasCharge) -> Result<()> {
-        self.gas_tracker_mut().charge_gas(charge)?;
+        self.gas_tracker_mut().apply_charge(charge)?;
         Ok(())
     }
 }