diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index 838720cf8..5546d84ed 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -7,7 +7,6 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::{ErrorNumber, ExitCode}; use fvm_shared::{ActorID, MethodNum, METHOD_SEND}; use num_traits::Zero; -use wasmtime::Val; use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID}; use crate::call_manager::backtrace::Frame; @@ -16,6 +15,7 @@ use crate::gas::GasTracker; use crate::kernel::{ClassifyResult, 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}; @@ -345,55 +345,21 @@ where // From this point on, there are no more syscall errors, only aborts. let result: std::result::Result = (|| { - let initial_milligas = - store - .data_mut() - .kernel - .borrow_milligas() - .map_err(|e| match e { - ExecutionError::OutOfGas => Abort::OutOfGas, - ExecutionError::Fatal(m) => Abort::Fatal(m), - _ => Abort::Fatal(anyhow::Error::msg( - "setting available gas on entering wasm", - )), - })?; - - store - .data() - .avail_gas_global - .clone() - .set(&mut store, Val::I64(initial_milligas)) - .map_err(Abort::Fatal)?; - // 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,)); - // Return gas tracking to GasTracker - let available_milligas = - match store.data_mut().avail_gas_global.clone().get(&mut store) { - Val::I64(g) => Ok(g), - _ => Err(Abort::Fatal(anyhow::Error::msg( - "failed to get available gas from wasm after returning", - ))), - }?; - - store - .data_mut() - .kernel - .return_milligas("wasm_exec_last", available_milligas) - .map_err(|e| match e { - ExecutionError::OutOfGas => Abort::OutOfGas, - ExecutionError::Fatal(m) => Abort::Fatal(m), - _ => Abort::Fatal(anyhow::Error::msg( - "setting available gas on existing wasm", - )), - })?; + // 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 diff --git a/fvm/src/gas/mod.rs b/fvm/src/gas/mod.rs index dc462b8cf..ec250ce51 100644 --- a/fvm/src/gas/mod.rs +++ b/fvm/src/gas/mod.rs @@ -15,13 +15,6 @@ pub const MILLIGAS_PRECISION: i64 = 1000; pub struct GasTracker { milligas_limit: i64, milligas_used: i64, - - /// A flag indicating whether this GasTracker is currently responsible for - /// gas accounting. A 'false' value indicates that gas accounting is - /// handled somewhere else (eg. in wasm execution). - /// - /// Creating gas charges is only allowed when own_limit is true. - own_limit: bool, } impl GasTracker { @@ -29,19 +22,12 @@ impl GasTracker { Self { milligas_limit: gas_to_milligas(gas_limit), milligas_used: gas_to_milligas(gas_used), - own_limit: true, } } /// Safely consumes gas and returns an out of gas error if there is not sufficient /// enough gas remaining for charge. - fn charge_milligas(&mut self, name: &str, to_use: i64) -> Result<()> { - if !self.own_limit { - return Err(ExecutionError::Fatal(anyhow::Error::msg( - "charge_gas called when gas_limit owned by execution", - ))); - } - + pub fn charge_milligas(&mut self, name: &str, to_use: i64) -> Result<()> { match self.milligas_used.checked_add(to_use) { None => { log::trace!("gas overflow: {}", name); @@ -69,50 +55,33 @@ impl GasTracker { ) } - /// returns available milligas; makes the gas tracker reject gas charges with - /// a fatal error until return_milligas is called. - pub fn borrow_milligas(&mut self) -> Result { - if !self.own_limit { - return Err(ExecutionError::Fatal(anyhow::Error::msg( - "borrow_milligas called on GasTracker which doesn't own gas limit", - ))); - } - self.own_limit = false; - - Ok(self.milligas_limit - self.milligas_used) - } - - /// sets new available gas, creating a new gas charge if needed - pub fn return_milligas(&mut self, name: &str, new_avail_mgas: i64) -> Result<()> { - if self.own_limit { - return Err(ExecutionError::Fatal(anyhow::Error::msg(format!( - "gastracker already owns gas_limit, charge: {}", - name - )))); - } - self.own_limit = true; - - let old_avail_milligas = self.milligas_limit - self.milligas_used; - let used = old_avail_milligas - new_avail_mgas; - - if used < 0 { - return Err(ExecutionError::Fatal(anyhow::Error::msg( - "negative gas charge in set_available_gas", - ))); - } - - self.charge_milligas(name, used) - } - /// Getter for gas available. pub fn gas_limit(&self) -> i64 { milligas_to_gas(self.milligas_limit, false) } + /// Getter for milligas available. + pub fn milligas_limit(&self) -> i64 { + self.milligas_limit + } + /// Getter for gas used. pub fn gas_used(&self) -> i64 { milligas_to_gas(self.milligas_used, true) } + + /// Getter for milligas used. + pub fn milligas_used(&self) -> i64 { + self.milligas_used + } + + pub fn gas_available(&self) -> i64 { + milligas_to_gas(self.milligas_available(), false) + } + + pub fn milligas_available(&self) -> i64 { + self.milligas_limit.saturating_sub(self.milligas_used) + } } /// Converts the specified gas into equivalent fractional gas units diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index c95761ce5..3a043f00b 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -755,19 +755,27 @@ where self.call_manager.gas_tracker().gas_used() } + fn milligas_used(&self) -> i64 { + self.call_manager.gas_tracker().milligas_used() + } + + fn gas_available(&self) -> i64 { + self.call_manager.gas_tracker().gas_available() + } + fn milligas_available(&self) -> i64 { + self.call_manager.gas_tracker().milligas_available() + } + fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()> { let charge = GasCharge::new(name, compute, 0); - self.call_manager.charge_gas(charge) + self.call_manager.gas_tracker_mut().charge_gas(charge) } - fn return_milligas(&mut self, name: &str, new_gas: i64) -> Result<()> { + fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { self.call_manager .gas_tracker_mut() - .return_milligas(name, new_gas) - } - - fn borrow_milligas(&mut self) -> Result { - self.call_manager.gas_tracker_mut().borrow_milligas() + .charge_milligas(name, compute)?; + Ok(()) } fn price_list(&self) -> &PriceList { diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index a9953a119..5eee6911a 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -216,16 +216,16 @@ pub trait CircSupplyOps { pub trait GasOps { /// GasUsed return the gas used by the transaction so far. fn gas_used(&self) -> i64; + fn milligas_used(&self) -> i64; + + fn gas_available(&self) -> i64; + fn milligas_available(&self) -> i64; /// ChargeGas charges specified amount of `gas` for execution. /// `name` provides information about gas charging point fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()>; - /// Returns available gas. - fn borrow_milligas(&mut self) -> Result; - - /// Sets available gas to a new value, creating a gas charge if needed - fn return_milligas(&mut self, name: &str, newgas: i64) -> Result<()>; + fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()>; fn price_list(&self) -> &PriceList; } diff --git a/fvm/src/machine/engine.rs b/fvm/src/machine/engine.rs index 567da8e0b..2137af1f6 100644 --- a/fvm/src/machine/engine.rs +++ b/fvm/src/machine/engine.rs @@ -308,6 +308,7 @@ impl Engine { kernel, last_error: None, avail_gas_global: self.0.dummy_gas_global, + last_milligas_available: 0, }; let mut store = wasmtime::Store::new(&self.0.engine, id); diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index 32fb264d7..7d7cdb737 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -2,11 +2,11 @@ use std::mem; use fvm_shared::error::ErrorNumber; use fvm_shared::sys::SyscallSafe; -use wasmtime::{Caller, Linker, Trap, Val, WasmTy}; +use wasmtime::{Caller, Linker, Trap, WasmTy}; use super::context::Memory; use super::error::Abort; -use super::{Context, InvocationData}; +use super::{charge_for_exec, update_gas_available, Context, InvocationData}; use crate::call_manager::backtrace; use crate::kernel::{self, ExecutionError, Kernel, SyscallError}; @@ -98,41 +98,6 @@ fn memory_and_data<'a, K: Kernel>( Ok((Memory::new(mem), data)) } -fn gastracker_to_wasmgas(caller: &mut Caller>) -> Result<(), Trap> { - let avail_milligas = caller - .data_mut() - .kernel - .borrow_milligas() - .map_err(|_| Trap::new("borrowing available gas"))?; - - let gas_global = caller.data_mut().avail_gas_global; - gas_global - .set(caller, Val::I64(avail_milligas)) - .map_err(|_| Trap::new("failed to set available gas")) -} - -fn wasmgas_to_gastracker(caller: &mut Caller>) -> Result<(), Trap> { - let global = caller.data_mut().avail_gas_global; - - let milligas = match global.get(&mut *caller) { - Val::I64(g) => Ok(g), - _ => Err(Trap::new("failed to get wasm gas")), - }?; - - // note: this should never error: - // * It can't return out-of-gas, because that would mean that we got - // negative available milligas returned from wasm - and wasm - // instrumentation will trap when it sees available gas go below zero - // * If it errors because gastracker thinks it already owns gas, something - // is really wrong - caller - .data_mut() - .kernel - .return_milligas("wasm_exec", milligas) - .map_err(|e| Trap::new(format!("returning available gas: {}", e)))?; - Ok(()) -} - // Unfortunately, we can't implement this for _all_ functions. So we implement it for functions of up to 6 arguments. macro_rules! impl_bind_syscalls { ($($t:ident)*) => { @@ -153,7 +118,7 @@ macro_rules! impl_bind_syscalls { if mem::size_of::() == 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> $(, $t: $t)*| { - wasmgas_to_gastracker(&mut caller)?; + charge_for_exec(&mut caller)?; let (mut memory, mut data) = memory_and_data(&mut caller)?; let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; @@ -174,14 +139,15 @@ macro_rules! impl_bind_syscalls { Err(e) => Err(e.into()), }; - gastracker_to_wasmgas(&mut caller)?; + 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>, ret: u32 $(, $t: $t)*| { - wasmgas_to_gastracker(&mut caller)?; + charge_for_exec(&mut caller)?; + let (mut memory, mut data) = memory_and_data(&mut caller)?; // We need to check to make sure we can store the return value _before_ we do anything. @@ -209,7 +175,7 @@ macro_rules! impl_bind_syscalls { Err(e) => Err(e.into()), }; - gastracker_to_wasmgas(&mut caller)?; + update_gas_available(&mut caller)?; result }) diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index e5919f680..ec902d59f 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -1,7 +1,11 @@ +use std::mem; + +use anyhow::{anyhow, Context as _}; use cid::Cid; -use wasmtime::{Global, Linker}; +use wasmtime::{AsContextMut, Global, Linker, Val}; use crate::call_manager::backtrace; +use crate::kernel::ExecutionError; use crate::Kernel; pub(crate) mod error; @@ -30,11 +34,62 @@ pub struct InvocationData { /// after receiving this error without calling any other syscalls. pub last_error: Option, - /// The global containing remaining available gas + /// The global containing remaining available gas. pub avail_gas_global: Global, + /// The last-set milligas limit. When `charge_for_exec` is called, we charge for the + /// _difference_ between the current gas available (the wasm global) and the + /// `last_milligas_available`. + pub last_milligas_available: i64, +} + +pub fn update_gas_available( + ctx: &mut impl AsContextMut>, +) -> Result<(), Abort> { + let mut ctx = ctx.as_context_mut(); + let avail_milligas = ctx.data_mut().kernel.milligas_available(); + + let gas_global = ctx.data_mut().avail_gas_global; + gas_global + .set(&mut ctx, Val::I64(avail_milligas)) + .map_err(|e| Abort::Fatal(anyhow!("failed to set available gas global: {}", e)))?; + + ctx.data_mut().last_milligas_available = avail_milligas; + Ok(()) +} + +pub fn charge_for_exec( + ctx: &mut impl AsContextMut>, +) -> Result<(), Abort> { + let mut ctx = ctx.as_context_mut(); + let global = ctx.data_mut().avail_gas_global; + + let milligas_available = global + .get(&mut ctx) + .i64() + .context("failed to get wasm gas") + .map_err(Abort::Fatal)?; + + // Determine milligas used, and update the "o + let milligas_used = { + let data = ctx.data_mut(); + let last_milligas = mem::replace(&mut data.last_milligas_available, milligas_available); + // This should never be negative, but we might as well check. + last_milligas.saturating_sub(milligas_available) + }; + + ctx.data_mut() + .kernel + .charge_milligas("wasm_exec", milligas_used) + .map_err(|e| match e { + ExecutionError::OutOfGas => Abort::OutOfGas, + ExecutionError::Fatal(e) => Abort::Fatal(e), + ExecutionError::Syscall(e) => Abort::Fatal(anyhow!("unexpected syscall error: {}", e)), + })?; + Ok(()) } use self::bind::BindSyscall; +use self::error::Abort; /// The maximum supported CID size. (SPEC_AUDIT) pub const MAX_CID_LEN: usize = 100; diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index 640077a91..9d6471f06 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -494,16 +494,24 @@ where self.0.charge_gas(name, compute) } - fn borrow_milligas(&mut self) -> Result { - self.0.borrow_milligas() + fn price_list(&self) -> &PriceList { + self.0.price_list() } - fn return_milligas(&mut self, name: &str, newgas: i64) -> Result<()> { - self.0.return_milligas(name, newgas) + fn milligas_used(&self) -> i64 { + self.0.milligas_used() } - fn price_list(&self) -> &PriceList { - self.0.price_list() + fn gas_available(&self) -> i64 { + self.0.gas_available() + } + + fn milligas_available(&self) -> i64 { + self.0.milligas_available() + } + + fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { + self.0.charge_milligas(name, compute) } }