feat: refactor gas charging to remove "borrowing"

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<RawBytes, Abort> = (|| {
-                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

fvm/src/gas/mod.rs

@@ -15,33 +15,19 @@ 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 {
     pub fn new(gas_limit: i64, gas_used: i64) -> Self {
         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<i64> {
-        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

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<i64> {
-        self.call_manager.gas_tracker_mut().borrow_milligas()
+            .charge_milligas(name, compute)?;
+        Ok(())
     }
 
     fn price_list(&self) -> &PriceList {

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<i64>;
-
-    /// 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;
 }

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);

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<InvocationData<impl Kernel>>) -> 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<InvocationData<impl Kernel>>) -> 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::<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)*| {
-                        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<K>>, 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
                     })