refactor: remove GasOps from the Kernel (#1951)

It's not really a proper "ops" trait, it's more "basic functionality".

Additionally, the concept of a global price list is going to go away as
we make the kernel more pluggable.

1. Move `GasOps::gas_available`/`GasOps::charge_gas` to the main
`Kernel` trait as all kernels require them.
2. Remove `GasOps::gas_used`, it's only used for testing.
3. Remove `GasOps::price_list`. We're still exposing it on the
CallManager, but one step at a time).

fvm/src/engine/mod.rs

@@ -572,6 +572,7 @@ impl Engine {
             last_memory_bytes: memory_bytes,
             last_charge_time: GasTimer::start(),
             memory: self.inner.dummy_memory,
+            wasm_prices: self.inner.config.wasm_prices,
         };
 
         let mut store = wasmtime::Store::new(&self.inner.engine, id);

fvm/src/gas/price_list.rs

@@ -259,8 +259,6 @@ lazy_static! {
 
         block_persist_compute: Gas::new(172000),
 
-        syscall_cost: Gas::new(14000),
-
         // TODO(#1347)
         builtin_actor_manifest_lookup: Zero::zero(),
         // TODO(#1347)
@@ -286,6 +284,8 @@ lazy_static! {
             // Charge 0.4gas/byte for copying/fill.
             memory_copy_per_byte_cost: Gas::from_milligas(400),
             memory_fill_per_byte_cost: Gas::from_milligas(400),
+
+            host_call_cost: Gas::new(14000),
         },
 
         event_per_entry: ScalingCost {
@@ -441,9 +441,6 @@ pub struct PriceList {
     /// Gas cost to cover the cost of flushing a block.
     pub(crate) block_persist_compute: Gas,
 
-    /// General gas cost for performing a syscall, accounting for the overhead thereof.
-    pub(crate) syscall_cost: Gas,
-
     /// Rules for execution gas.
     pub(crate) wasm_rules: WasmGasPrices,
 
@@ -504,6 +501,27 @@ pub struct WasmGasPrices {
     pub(crate) memory_access_cost: Gas,
     /// Gas cost for every byte copied in Wasm memory.
     pub(crate) memory_copy_per_byte_cost: Gas,
+
+    /// Gas cost for a call from wasm to the system.
+    pub(crate) host_call_cost: Gas,
+}
+
+impl WasmGasPrices {
+    /// Returns the gas required for initializing memory.
+    pub(crate) fn init_memory_gas(&self, min_memory_bytes: usize) -> Gas {
+        self.memory_fill_base_cost + self.memory_fill_per_byte_cost * min_memory_bytes
+    }
+
+    /// Returns the gas required for growing memory.
+    pub(crate) fn grow_memory_gas(&self, grow_memory_bytes: usize) -> Gas {
+        self.memory_fill_base_cost + self.memory_fill_per_byte_cost * grow_memory_bytes
+    }
+
+    /// Returns the gas required for initializing tables.
+    pub(crate) fn init_table_gas(&self, min_table_elements: u32) -> Gas {
+        self.memory_fill_base_cost
+            + self.memory_fill_per_byte_cost * min_table_elements * TABLE_ELEMENT_SIZE
+    }
 }
 
 impl PriceList {
@@ -555,11 +573,6 @@ impl PriceList {
         }
     }
 
-    /// Returns the gas cost to be applied on a syscall.
-    pub fn on_syscall(&self) -> GasCharge {
-        GasCharge::new("OnSyscall", self.syscall_cost, Zero::zero())
-    }
-
     /// Returns the gas required for creating an actor. Pass `true` to when explicitly assigning a
     /// new address.
     #[inline]
@@ -909,24 +922,6 @@ impl PriceList {
         )
     }
 
-    /// Returns the gas required for initializing memory.
-    pub fn init_memory_gas(&self, min_memory_bytes: usize) -> Gas {
-        self.wasm_rules.memory_fill_base_cost
-            + self.wasm_rules.memory_fill_per_byte_cost * min_memory_bytes
-    }
-
-    /// Returns the gas required for growing memory.
-    pub fn grow_memory_gas(&self, grow_memory_bytes: usize) -> Gas {
-        self.wasm_rules.memory_fill_base_cost
-            + self.wasm_rules.memory_fill_per_byte_cost * grow_memory_bytes
-    }
-
-    /// Returns the gas required for initializing tables.
-    pub fn init_table_gas(&self, min_table_elements: u32) -> Gas {
-        self.wasm_rules.memory_fill_base_cost
-            + self.wasm_rules.memory_fill_per_byte_cost * min_table_elements * TABLE_ELEMENT_SIZE
-    }
-
     #[inline]
     pub fn on_actor_event(&self, entries: usize, keysize: usize, valuesize: usize) -> GasCharge {
         // Here we estimate per-event overhead given the constraints on event values.

fvm/src/kernel/default.rs

@@ -269,6 +269,14 @@ where
     fn limiter_mut(&mut self) -> &mut Self::Limiter {
         self.call_manager.limiter_mut()
     }
+
+    fn gas_available(&self) -> Gas {
+        self.call_manager.gas_tracker().gas_available()
+    }
+
+    fn charge_gas(&self, name: &str, compute: Gas) -> Result<GasTimer> {
+        self.call_manager.gas_tracker().charge_gas(name, compute)
+    }
 }
 
 impl<C> DefaultKernel<C>
@@ -628,27 +636,6 @@ where
     }
 }
 
-impl<C> GasOps for DefaultKernel<C>
-where
-    C: CallManager,
-{
-    fn gas_used(&self) -> Gas {
-        self.call_manager.gas_tracker().gas_used()
-    }
-
-    fn gas_available(&self) -> Gas {
-        self.call_manager.gas_tracker().gas_available()
-    }
-
-    fn charge_gas(&self, name: &str, compute: Gas) -> Result<GasTimer> {
-        self.call_manager.gas_tracker().charge_gas(name, compute)
-    }
-
-    fn price_list(&self) -> &PriceList {
-        self.call_manager.price_list()
-    }
-}
-
 impl<C> NetworkOps for DefaultKernel<C>
 where
     C: CallManager,

fvm/src/kernel/filecoin.rs

@@ -93,7 +93,6 @@ pub trait FilecoinKernel: Kernel {
 #[delegate(CryptoOps, where = "C: CallManager")]
 #[delegate(DebugOps, where = "C: CallManager")]
 #[delegate(EventOps, where = "C: CallManager")]
-#[delegate(GasOps, where = "C: CallManager")]
 #[delegate(MessageOps, where = "C: CallManager")]
 #[delegate(NetworkOps, where = "C: CallManager")]
 #[delegate(RandomnessOps, where = "C: CallManager")]
@@ -303,7 +302,15 @@ where
     }
 
     fn limiter_mut(&mut self) -> &mut Self::Limiter {
-        self.0.call_manager.limiter_mut()
+        self.0.limiter_mut()
+    }
+
+    fn gas_available(&self) -> Gas {
+        self.0.gas_available()
+    }
+
+    fn charge_gas(&self, name: &str, compute: Gas) -> Result<GasTimer> {
+        self.0.charge_gas(name, compute)
     }
 }
 

fvm/src/kernel/mod.rs

@@ -32,7 +32,7 @@ pub struct CallResult {
 ///
 /// Actors may call into the kernel via the syscalls defined in the [`syscalls`][crate::syscalls]
 /// module.
-pub trait Kernel: GasOps + SyscallHandler<Self> + 'static {
+pub trait Kernel: SyscallHandler<Self> + 'static {
     /// The [`Kernel`]'s [`CallManager`] is
     type CallManager: CallManager;
     /// The [`Kernel`]'s memory allocation tracker.
@@ -91,6 +91,13 @@ pub trait Kernel: GasOps + SyscallHandler<Self> + 'static {
 
     /// Give access to the limiter of the underlying call manager.
     fn limiter_mut(&mut self) -> &mut Self::Limiter;
+
+    /// Returns the remaining gas for the transaction.
+    fn gas_available(&self) -> Gas;
+
+    /// ChargeGas charges specified amount of `gas` for execution.
+    /// `name` provides information about gas charging point.
+    fn charge_gas(&self, name: &str, compute: Gas) -> Result<GasTimer>;
 }
 
 pub trait SyscallHandler<K>: Sized {
@@ -208,23 +215,6 @@ pub trait ActorOps {
     fn balance_of(&self, actor_id: ActorID) -> Result<TokenAmount>;
 }
 
-/// Operations for explicit gas charging.
-#[delegatable_trait]
-pub trait GasOps {
-    /// Returns the gas used by the transaction so far.
-    fn gas_used(&self) -> Gas;
-
-    /// Returns the remaining gas for the transaction.
-    fn gas_available(&self) -> Gas;
-
-    /// ChargeGas charges specified amount of `gas` for execution.
-    /// `name` provides information about gas charging point.
-    fn charge_gas(&self, name: &str, compute: Gas) -> Result<GasTimer>;
-
-    /// Returns the currently active gas price list.
-    fn price_list(&self) -> &PriceList;
-}
-
 /// Cryptographic primitives provided by the kernel.
 #[delegatable_trait]
 pub trait CryptoOps {
@@ -300,15 +290,15 @@ pub trait EventOps {
 #[doc(hidden)]
 pub use {
     ambassador_impl_CryptoOps, ambassador_impl_DebugOps, ambassador_impl_EventOps,
-    ambassador_impl_GasOps, ambassador_impl_IpldBlockOps, ambassador_impl_MessageOps,
-    ambassador_impl_NetworkOps, ambassador_impl_RandomnessOps, ambassador_impl_SelfOps,
+    ambassador_impl_IpldBlockOps, ambassador_impl_MessageOps, ambassador_impl_NetworkOps,
+    ambassador_impl_RandomnessOps, ambassador_impl_SelfOps,
 };
 
 /// Import this module (with a glob) if you're implementing a kernel, _especially_ if you want to
 /// use ambassador to delegate the implementation.
 pub mod prelude {
     pub use super::{
-        ActorOps, CryptoOps, DebugOps, EventOps, GasOps, IpldBlockOps, MessageOps, NetworkOps,
+        ActorOps, CryptoOps, DebugOps, EventOps, IpldBlockOps, MessageOps, NetworkOps,
         RandomnessOps, SelfOps,
     };
     pub use super::{Block, BlockId, BlockRegistry, BlockStat, CallResult, Kernel, SyscallHandler};
@@ -331,9 +321,8 @@ pub mod prelude {
     pub use multihash::Multihash;
     pub use {
         ambassador_impl_ActorOps, ambassador_impl_CryptoOps, ambassador_impl_DebugOps,
-        ambassador_impl_EventOps, ambassador_impl_GasOps, ambassador_impl_IpldBlockOps,
-        ambassador_impl_MessageOps, ambassador_impl_NetworkOps, ambassador_impl_RandomnessOps,
-        ambassador_impl_SelfOps,
+        ambassador_impl_EventOps, ambassador_impl_IpldBlockOps, ambassador_impl_MessageOps,
+        ambassador_impl_NetworkOps, ambassador_impl_RandomnessOps, ambassador_impl_SelfOps,
     };
 }
 

fvm/src/syscalls/gas.rs

@@ -4,10 +4,11 @@ use std::str;
 
 use super::Context;
 use crate::gas::Gas;
-use crate::kernel::{ClassifyResult, GasOps, Result};
+use crate::kernel::{ClassifyResult, Result};
+use crate::Kernel;
 
 pub fn charge_gas(
-    context: Context<'_, impl GasOps>,
+    context: Context<'_, impl Kernel>,
     name_off: u32,
     name_len: u32,
     compute: u64,
@@ -21,6 +22,6 @@ pub fn charge_gas(
         .map(|_| ())
 }
 
-pub fn available(context: Context<'_, impl GasOps>) -> Result<u64> {
+pub fn available(context: Context<'_, impl Kernel>) -> Result<u64> {
     Ok(context.kernel.gas_available().round_down())
 }

fvm/src/syscalls/linker.rs

@@ -8,7 +8,7 @@ use wasmtime::{Caller, WasmTy};
 
 use super::context::Memory;
 use super::error::Abort;
-use super::{charge_for_exec, update_gas_available, Context, InvocationData};
+use super::{charge_for_exec, charge_syscall_gas, update_gas_available, Context, InvocationData};
 use crate::call_manager::backtrace;
 use crate::kernel::{self, ExecutionError, Kernel, SyscallError};
 
@@ -141,15 +141,6 @@ fn memory_and_data<'a, K: Kernel>(
     (Memory::new(mem), data)
 }
 
-macro_rules! charge_syscall_gas {
-    ($kernel:expr) => {
-        let charge = $kernel.price_list().on_syscall();
-        $kernel
-            .charge_gas(&charge.name, charge.compute_gas)
-            .map_err(Abort::from_error_as_fatal)?;
-    };
-}
-
 // Unfortunately, we can't implement this for _all_ functions. So we implement it for functions of up to 6 arguments.
 macro_rules! impl_syscall {
     ($($t:ident)*) => {
@@ -171,9 +162,9 @@ macro_rules! impl_syscall {
                     // If we're returning a zero-sized "value", we return no value therefore and expect no out pointer.
                     linker.0.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>> $(, $t: $t)*| {
                         charge_for_exec(&mut caller)?;
+                        charge_syscall_gas(&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 = self(ctx $(, $t)*).into_control_flow();
@@ -201,9 +192,9 @@ macro_rules! impl_syscall {
                     // If we're returning an actual value, we need to write it back into the wasm module's memory.
                     linker.0.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>>, ret: u32 $(, $t: $t)*| {
                         charge_for_exec(&mut caller)?;
+                        charge_syscall_gas(&mut caller)?;
 
                         let (mut memory, data) = memory_and_data(&mut caller);
-                        charge_syscall_gas!(data.kernel);
 
                         // We need to check to make sure we can store the return value _before_ we do anything.
                         if (ret as u64) > (memory.len() as u64)