feat: cleanup & document the context API

We're now exposing this to users so we need to clean this up a bit:

1. Document it.
2. Remove `Memory::read_cbor` from the public API, it's not
   safe (time-wise) to call on untrusted memory.

fvm/src/syscalls/context.rs

@@ -2,23 +2,29 @@
 // SPDX-License-Identifier: Apache-2.0, MIT
 use std::io::Cursor;
 use std::ops::{Deref, DerefMut};
-use std::panic;
 
 use cid::Cid;
-use fvm_ipld_encoding::from_slice;
 use fvm_shared::address::Address;
 use fvm_shared::error::ErrorNumber;
 use fvm_shared::MAX_CID_LEN;
-use serde::de::DeserializeOwned;
 
 use crate::kernel::{ClassifyResult, Context as _, Result};
 use crate::syscall_error;
 
+#[cfg(doc)]
+use crate::Kernel;
+
+/// The syscall context. Allows syscalls to access the [`Kernel`] and the actor's memory.
 pub struct Context<'a, K> {
+    /// The running actor's [`Kernel`].
     pub kernel: &'a mut K,
+    /// The running actor's [`Memory`].
     pub memory: &'a mut Memory,
 }
 
+/// Represents a Wasm memory. All methods are inexpensive and time-bounded, regardless of the
+/// inputs. It's usually not necessary to explicitly account for the gas costs of calling these
+/// methods a small (< 5) constant number of times while handling a syscall.
 #[repr(transparent)]
 pub struct Memory([u8]);
 
@@ -37,13 +43,15 @@ impl DerefMut for Memory {
 }
 
 impl Memory {
+    /// Construct a new "memory" from the given slice.
     #[allow(clippy::needless_lifetimes)]
     pub fn new<'a>(m: &'a mut [u8]) -> &'a mut Memory {
         // We explicitly specify the lifetimes here to ensure that the cast doesn't inadvertently
         // change them.
         unsafe { &mut *(m as *mut [u8] as *mut Memory) }
     }
 
+    /// Check that the given slice, specified by an offset and length, is in-bounds.
     pub fn check_bounds(&self, offset: u32, len: u32) -> Result<()> {
         if (offset as u64) + (len as u64) <= (self.0.len() as u64) {
             Ok(())
@@ -55,19 +63,33 @@ impl Memory {
         }
     }
 
+    /// Return a slice into the actor's memory.
+    ///
+    /// This slice is valid for the lifetime of the syscall, borrowing the actors memory without
+    /// copying.
+    ///
+    /// On failure, this method returns an [`ErrorNumber::IllegalArgument`] error.
     pub fn try_slice(&self, offset: u32, len: u32) -> Result<&[u8]> {
         self.get(offset as usize..)
             .and_then(|data| data.get(..len as usize))
             .ok_or_else(|| format!("buffer {} (length {}) out of bounds", offset, len))
             .or_error(ErrorNumber::IllegalArgument)
     }
+
+    /// Return a mutable slice into the actor's memory.
+    ///
+    /// This slice is valid for the lifetime of the syscall, borrowing the actors memory without
+    /// copying.
     pub fn try_slice_mut(&mut self, offset: u32, len: u32) -> Result<&mut [u8]> {
         self.get_mut(offset as usize..)
             .and_then(|data| data.get_mut(..len as usize))
             .ok_or_else(|| format!("buffer {} (length {}) out of bounds", offset, len))
             .or_error(ErrorNumber::IllegalArgument)
     }
 
+    /// Read a CID from actor memory starting at the given offset.
+    ///
+    /// On failure, this method returns an [`ErrorNumber::IllegalArgument`] error.
     pub fn read_cid(&self, offset: u32) -> Result<Cid> {
         // NOTE: Be very careful when changing this code.
         //
@@ -87,6 +109,11 @@ impl Memory {
         .context("failed to parse cid")
     }
 
+    /// Write a CID to actor memory at the given offset.
+    ///
+    /// If the CID's length exceeds the specified length, this function will with
+    /// [`ErrorNumber::BufferTooSmall`]. For all other failures (e.g., memory out of bounds errors),
+    /// this method returns an [`ErrorNumber::IllegalArgument`] error.
     pub fn write_cid(&mut self, k: &Cid, offset: u32, len: u32) -> Result<u32> {
         let out = self.try_slice_mut(offset, len)?;
 
@@ -102,22 +129,13 @@ impl Memory {
         Ok(len as u32)
     }
 
+    /// Read a Filecoin address from actor memory.
+    ///
+    /// On failure, this method returns an [`ErrorNumber::IllegalArgument`] error.
     pub fn read_address(&self, offset: u32, len: u32) -> Result<Address> {
         let bytes = self.try_slice(offset, len)?;
         Address::from_bytes(bytes).or_error(ErrorNumber::IllegalArgument)
     }
-
-    pub fn read_cbor<T: DeserializeOwned>(&self, offset: u32, len: u32) -> Result<T> {
-        let bytes = self.try_slice(offset, len)?;
-        // Catch panics when decoding cbor from actors, _just_ in case.
-        match panic::catch_unwind(|| from_slice(bytes).or_error(ErrorNumber::IllegalArgument)) {
-            Ok(v) => v,
-            Err(e) => {
-                log::error!("panic when decoding cbor from actor: {:?}", e);
-                Err(syscall_error!(IllegalArgument; "panic when decoding cbor from actor").into())
-            }
-        }
-    }
 }
 
 #[cfg(test)]

fvm/src/syscalls/filecoin.rs

@@ -1,19 +1,51 @@
+use std::panic;
+
+use fvm_ipld_encoding::de::DeserializeOwned;
+use fvm_shared::error::ErrorNumber;
 // Copyright 2021-2023 Protocol Labs
 // SPDX-License-Identifier: Apache-2.0, MIT
 use fvm_shared::sector::WindowPoStVerifyInfo;
 
+use super::context::Memory;
 use super::Context;
 use crate::kernel::ClassifyResult;
 use crate::kernel::{filecoin::FilecoinKernel, Result};
 use crate::syscall_error;
 use anyhow::anyhow;
 use anyhow::Context as _;
+use fvm_ipld_encoding::from_slice;
 use fvm_shared::piece::PieceInfo;
 use fvm_shared::sector::{
     AggregateSealVerifyProofAndInfos, RegisteredSealProof, ReplicaUpdateInfo, SealVerifyInfo,
 };
 use fvm_shared::sys;
 
+/// Private extension trait for reading CBOR. This operation is not safe to call on untrusted
+/// (user-controlled) memory.
+trait ReadCbor {
+    fn read_cbor<T: DeserializeOwned>(&self, offset: u32, len: u32) -> Result<T>;
+}
+
+impl ReadCbor for Memory {
+    /// Read a CBOR object from actor memory.
+    ///
+    /// **WARNING:** CBOR decoding is complex and this function offers no way to perform gas
+    /// accounting. Only call this on data from _trusted_ (built-in) actors.
+    ///
+    /// On failure, this method returns an [`ErrorNumber::IllegalArgument`] error.
+    fn read_cbor<T: DeserializeOwned>(&self, offset: u32, len: u32) -> Result<T> {
+        let bytes = self.try_slice(offset, len)?;
+        // Catch panics when decoding cbor from actors, _just_ in case.
+        match panic::catch_unwind(|| from_slice(bytes).or_error(ErrorNumber::IllegalArgument)) {
+            Ok(v) => v,
+            Err(e) => {
+                log::error!("panic when decoding cbor from actor: {:?}", e);
+                Err(syscall_error!(IllegalArgument; "panic when decoding cbor from actor").into())
+            }
+        }
+    }
+}
+
 /// Computes an unsealed sector CID (CommD) from its constituent piece CIDs
 /// (CommPs) and sizes.
 ///

fvm/src/syscalls/mod.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0, MIT
 use anyhow::{anyhow, Context as _};
 use num_traits::Zero;
-use wasmtime::{AsContextMut, ExternType, Global, Linker, Memory, Module, Val};
+use wasmtime::{AsContextMut, ExternType, Global, Linker, Module, Val};
 
 use crate::call_manager::backtrace;
 use crate::gas::{Gas, GasInstant, GasTimer};
@@ -32,7 +32,7 @@ mod send;
 mod sself;
 mod vm;
 
-pub use context::Context;
+pub use context::{Context, Memory};
 pub use error::Abort;
 
 /// Invocation data attached to a wasm "store" and available to the syscall binding.
@@ -61,7 +61,7 @@ pub struct InvocationData<K> {
     pub last_charge_time: GasInstant,
 
     /// The invocation's imported "memory".
-    pub memory: Memory,
+    pub memory: wasmtime::Memory,
 }
 
 /// Updates the global available gas in the Wasm module after a syscall, to account for any