feat: cleanup Linker API

This cleans up and hides the implementation details of our syscall
binding API.

Previously, we passed a wasmtime linker to the `SyscallHandler`.
Unfortunately:

1. This exposed implementation details (wasmtime, the InvocationData
object type, etc.).
2. The user had to import the `BindSyscall` trait to bind syscalls.

I've changed this by:

1. Replacing the `BindSyscall` trait (previously implemented on the
"linker") with a `Syscall` trait implemented on all valid "syscall"
functions.
2. Exposing an opaque `Linker` type with a `bind_syscall` method instead
of exposing the wasmtime linker directly. This lets us change/augment
this type later.

fvm/src/engine/mod.rs

@@ -18,8 +18,8 @@ use fvm_wasm_instrument::gas_metering::GAS_COUNTER_NAME;
 use num_traits::Zero;
 use wasmtime::OptLevel::Speed;
 use wasmtime::{
-    Global, GlobalType, InstanceAllocationStrategy, Linker, Memory, MemoryType, Module, Mutability,
-    Val, ValType,
+    Global, GlobalType, InstanceAllocationStrategy, Memory, MemoryType, Module, Mutability, Val,
+    ValType,
 };
 
 use crate::gas::{Gas, GasTimer, WasmGasPrices};
@@ -28,6 +28,7 @@ use crate::machine::{Machine, NetworkConfig};
 use crate::syscalls::error::Abort;
 use crate::syscalls::{
     charge_for_exec, charge_for_init, record_init_time, update_gas_available, InvocationData,
+    Linker,
 };
 use crate::Kernel;
 
@@ -497,7 +498,7 @@ impl Engine {
     /// linker, syscalls, etc.
     ///
     /// This returns an `Abort` as it may need to execute initialization code, charge gas, etc.
-    pub fn instantiate<K: Kernel>(
+    pub(crate) fn instantiate<K: Kernel>(
         &self,
         store: &mut wasmtime::Store<InvocationData<K>>,
         k: &Cid,
@@ -513,9 +514,9 @@ impl Engine {
                 .expect("invalid instance cache entry"),
             Vacant(e) => &mut *e
                 .insert({
-                    let mut linker = Linker::new(&self.inner.engine);
+                    let mut linker = wasmtime::Linker::new(&self.inner.engine);
                     linker.allow_shadowing(true);
-                    K::bind_syscalls(&mut linker).map_err(Abort::Fatal)?;
+                    K::bind_syscalls(Linker::wrap(&mut linker)).map_err(Abort::Fatal)?;
                     Box::new(Cache { linker })
                 })
                 .downcast_mut()
@@ -595,7 +596,7 @@ impl Engine {
     }
 
     /// Construct a new wasmtime "store" from the given kernel.
-    pub fn new_store<K: Kernel>(&self, mut kernel: K) -> wasmtime::Store<InvocationData<K>> {
+    pub(crate) fn new_store<K: Kernel>(&self, mut kernel: K) -> wasmtime::Store<InvocationData<K>> {
         // Take a new instance and put it into a drop-guard that removes the reservation when
         // we're done.
         #[must_use]

fvm/src/kernel/mod.rs

@@ -2,12 +2,11 @@
 // SPDX-License-Identifier: Apache-2.0, MIT
 use ambassador::delegatable_trait;
 use fvm_shared::event::StampedEvent;
-use wasmtime::Linker;
 
 use crate::call_manager::CallManager;
 use crate::machine::limiter::MemoryLimiter;
 use crate::machine::Machine;
-use crate::syscalls::InvocationData;
+use crate::syscalls::Linker;
 
 mod blocks;
 mod error;
@@ -95,7 +94,7 @@ pub trait Kernel: GasOps + SyscallHandler<Self> + 'static {
 }
 
 pub trait SyscallHandler<K>: Sized {
-    fn bind_syscalls(linker: &mut Linker<InvocationData<K>>) -> anyhow::Result<()>;
+    fn bind_syscalls(linker: &mut Linker<K>) -> anyhow::Result<()>;
 }
 
 /// Network-related operations.

fvm/src/syscalls/bind.rs

@@ -4,31 +4,21 @@ use std::mem;
 
 use fvm_shared::error::ErrorNumber;
 use fvm_shared::sys::SyscallSafe;
-use wasmtime::{Caller, Linker, WasmTy};
+use wasmtime::{Caller, WasmTy};
 
 use super::context::Memory;
 use super::error::Abort;
 use super::{charge_for_exec, update_gas_available, Context, InvocationData};
 use crate::call_manager::backtrace;
 use crate::kernel::{self, ExecutionError, Kernel, SyscallError};
 
-/// Binds syscalls to a linker, converting the returned error according to the syscall convention:
-///
-/// 1. If the error is a syscall error, it's returned as the first return value.
-/// 2. If the error is a fatal error, a Trap is returned.
-pub trait BindSyscall<Args, Ret, Func> {
+/// Bind a [`Syscall`] to a [`Linker`]. Import this trait then call `linker.bind(module, name,
+/// your_syscall)` on your [`Linker`].
+#[repr(transparent)]
+pub struct Linker<K>(wasmtime::Linker<InvocationData<K>>);
+impl<K> Linker<K> {
     /// Bind a syscall to the linker.
     ///
-    /// 1. The return type will be automatically adjusted to return `Result<u32, Trap>` where
-    /// `u32` is the error code.
-    /// 2. If the return type is non-empty (i.e., not `()`), an out-pointer will be prepended to the
-    /// arguments for the return-value.
-    ///
-    /// By example:
-    ///
-    /// - `fn(u32) -> kernel::Result<()>` will become `fn(u32) -> Result<u32, Trap>`.
-    /// - `fn(u32) -> kernel::Result<i64>` will become `fn(u32, u32) -> Result<u32, Trap>`.
-    ///
     /// # Example
     ///
     /// ```ignore
@@ -41,19 +31,52 @@ pub trait BindSyscall<Args, Ret, Func> {
     /// let mut linker = wasmtime::Linker::new(&engine);
     /// linker.bind("my_module", "zero", my_module::zero);
     /// ```
-    fn bind(
+    pub fn bind_syscall<Args, Ret>(
         &mut self,
         module: &'static str,
         name: &'static str,
-        syscall: Func,
-    ) -> anyhow::Result<&mut Self>;
+        syscall: impl Syscall<K, Args, Ret>,
+    ) -> anyhow::Result<&mut Self> {
+        syscall.bind_to(self, module, name)?;
+        Ok(self)
+    }
+
+    /// Wrap a wasmtime Linker in our newtype. We use a newtype to hide the underlying
+    /// implementation.
+    pub(crate) fn wrap(l: &mut wasmtime::Linker<InvocationData<K>>) -> &mut Self {
+        // SAFETY: We're transmuting to a transparent wrapper type.
+        unsafe { &mut *(l as *mut _ as *mut _) }
+    }
+}
+
+/// A [`Syscall`] is a function in the form `fn(Context<'_, K>, I...) -> R` where:
+///
+/// - `K` is the kernel type. Constrain this to the precise kernel operations you need, or even
+///    a specific kernel implementation.
+/// - `I...`, the syscall parameters, are 0-8 types, each one of [`u32`], [`u64`], [`i32`], or
+///    [`i64`].
+/// - `R` is a type implementing [`IntoControlFlow`]. This is usually one of:
+///     - [`kernel::Result<T>`] or [`ControlFlow<T>`] where `T`, the return value type, is
+///       [`SyscallSafe`].
+///     - [`Abort`] for syscalls that only abort (revert) the currently running actor.
+pub trait Syscall<K, Args, Ret>: Send + Sync + 'static {
+    // This is an implementation detail. See [`Linker`] for the user-facing API.
+    #[doc(hidden)]
+    fn bind_to(
+        self,
+        linker: &mut Linker<K>,
+        module: &'static str,
+        name: &'static str,
+    ) -> anyhow::Result<()>;
 }
 
-/// ControlFlow is a general-purpose enum allowing us to pass syscall error up the
-/// stack to the actor and treat error handling there (decide when to abort, etc).
+/// ControlFlow is a general-purpose enum for returning a control-flow decision from a syscall.
 pub enum ControlFlow<T> {
+    /// Return a value to the actor.
     Return(T),
+    /// Fail with the specified syscall error.
     Error(SyscallError),
+    /// Abort the running actor (exit, out of gas, or fatal error).
     Abort(Abort),
 }
 
@@ -67,9 +90,8 @@ impl<T> From<ExecutionError> for ControlFlow<T> {
     }
 }
 
-/// 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)]
+/// The helper trait used by `Syscall` to convert kernel results with execution errors into
+/// results that can be handled by the wasm vm. See the documentation on [`Syscall`] for details.
 pub trait IntoControlFlow: Sized {
     type Value: SyscallSafe;
     fn into_control_flow(self) -> ControlFlow<Self::Value>;
@@ -78,7 +100,6 @@ pub trait IntoControlFlow: Sized {
 /// 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 {}
 
@@ -140,29 +161,29 @@ macro_rules! charge_syscall_gas {
 macro_rules! impl_bind_syscalls {
     ($($t:ident)*) => {
         #[allow(non_snake_case)]
-        impl<$($t,)* Ret, K, Func> BindSyscall<($($t,)*), Ret, Func> for Linker<InvocationData<K>>
+        impl<$($t,)* Ret, K, Func> Syscall<K, ($($t,)*), Ret> for Func
         where
             K: Kernel,
             Func: Fn(Context<'_, K> $(, $t)*) -> Ret + Send + Sync + 'static,
             Ret: IntoControlFlow,
            $($t: WasmTy+SyscallSafe,)*
         {
-            fn bind(
-                &mut self,
+            fn bind_to(
+                self,
+                linker: &mut Linker<K>,
                 module: &'static str,
                 name: &'static str,
-                syscall: Func,
-            ) -> anyhow::Result<&mut Self> {
+            ) -> anyhow::Result<()> {
                 if mem::size_of::<Ret::Value>() == 0 {
                     // If we're returning a zero-sized "value", we return no value therefore and expect no out pointer.
-                    self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>> $(, $t: $t)*| {
+                    linker.0.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>> $(, $t: $t)*| {
                         charge_for_exec(&mut caller)?;
 
                         let (mut memory, data) = memory_and_data(&mut caller);
                         charge_syscall_gas!(data.kernel);
 
                         let ctx = Context{kernel: &mut data.kernel, memory: &mut memory};
-                        let out = syscall(ctx $(, $t)*).into_control_flow();
+                        let out = self(ctx $(, $t)*).into_control_flow();
 
                         let result = match out {
                             ControlFlow::Return(_) => {
@@ -182,10 +203,10 @@ macro_rules! impl_bind_syscalls {
                         update_gas_available(&mut caller)?;
 
                         result
-                    })
+                    })?;
                 } else {
                     // If we're returning an actual value, we need to write it back into the wasm module's memory.
-                    self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>>, ret: u32 $(, $t: $t)*| {
+                    linker.0.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>>, ret: u32 $(, $t: $t)*| {
                         charge_for_exec(&mut caller)?;
 
                         let (mut memory, data) = memory_and_data(&mut caller);
@@ -200,7 +221,7 @@ macro_rules! impl_bind_syscalls {
                         }
 
                         let ctx = Context{kernel: &mut data.kernel, memory: &mut memory};
-                        let result = match syscall(ctx $(, $t)*).into_control_flow() {
+                        let result = match self(ctx $(, $t)*).into_control_flow() {
                             ControlFlow::Return(value) => {
                                 log::trace!("syscall {}::{}: ok", module, name);
                                 unsafe {
@@ -223,8 +244,9 @@ macro_rules! impl_bind_syscalls {
                         update_gas_available(&mut caller)?;
 
                         result
-                    })
+                    })?;
                 }
+                Ok(())
             }
         }
     }

fvm/src/syscalls/error.rs

@@ -8,16 +8,21 @@ use wasmtime::Trap;
 use crate::call_manager::NO_DATA_BLOCK_ID;
 use crate::kernel::{BlockId, ExecutionError};
 
-/// Represents an actor "abort".
+/// Represents an actor "abort". Returning an [`Abort`] from a syscall will cause the currently
+/// running actor to exit and may cause part or all of the actor call stack to unwind.
 #[derive(Debug, thiserror::Error)]
 pub enum Abort {
-    /// The actor explicitly aborted with the given exit code (or panicked).
+    /// The actor explicitly aborted with the given exit code, panicked, or ran out of memory.
     #[error("exit with code {0} ({2})")]
     Exit(ExitCode, String, BlockId),
-    /// The actor ran out of gas.
+    /// The actor ran out of gas. This will unwind the actor call stack until we reach an actor
+    /// invocation with a gas limit _less_ than that of the caller's gas limit, or until we reach
+    /// the top-level call.
     #[error("out of gas")]
     OutOfGas,
-    /// The system failed with a fatal error.
+    /// The system failed with a fatal error indicating a bug in the FVM. This will abort the entire
+    /// top-level message and record a
+    /// [`SYS_ASSERTION_FAILED`][fvm_shared::ExitCode::SYS_ASSERTION_FAILED] exit code on-chain.
     #[error("fatal error: {0}")]
     Fatal(anyhow::Error),
 }