Skip to content

Commit

Permalink
refactor: remove GasOps from the Kernel
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
Stebalien committed Dec 14, 2023
1 parent a8f9be1 commit a627137
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 113 deletions.
1 change: 1 addition & 0 deletions fvm/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
51 changes: 23 additions & 28 deletions fvm/src/gas/price_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 8 additions & 21 deletions fvm/src/kernel/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 9 additions & 2 deletions fvm/src/kernel/filecoin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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)
}
}

Expand Down
37 changes: 13 additions & 24 deletions fvm/src/kernel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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};
Expand All @@ -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,
};
}

Expand Down
7 changes: 4 additions & 3 deletions fvm/src/syscalls/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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())
}
15 changes: 3 additions & 12 deletions fvm/src/syscalls/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)*) => {
Expand All @@ -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();
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit a627137

Please sign in to comment.