Merge pull request #501 from filecoin-project/asr/gas-fix

fix: gas: charge for execunits based on snapshot

Author: magik6k

fvm/src/call_manager/default.rs

@@ -1,3 +1,5 @@
+use std::cmp::max;
+
 use anyhow::Context;
 use derive_more::{Deref, DerefMut};
 use fvm_ipld_encoding::{RawBytes, DAG_CBOR};
@@ -314,6 +316,7 @@ where
         // it returns a referenced copy.
         let engine = self.engine().clone();
 
+        let gas_available = self.gas_tracker.gas_available();
         log::trace!("calling {} -> {}::{}", from, to, method);
         self.map_mut(|cm| {
             // Make the kernel.
@@ -331,10 +334,12 @@ where
             };
 
             // Make a store.
-            let gas_available = kernel.gas_available();
+            let gas_used = kernel.gas_used();
             let exec_units_to_add = match kernel.network_version() {
                 NetworkVersion::V14 | NetworkVersion::V15 => i64::MAX,
-                _ => kernel.price_list().gas_to_exec_units(gas_available, false),
+                _ => kernel
+                    .price_list()
+                    .gas_to_exec_units(max(gas_available.saturating_sub(gas_used), 0), false),
             };
 
             let mut store = engine.new_store(kernel);

fvm/src/kernel/default.rs

@@ -751,8 +751,8 @@ impl<C> GasOps for DefaultKernel<C>
 where
     C: CallManager,
 {
-    fn gas_available(&self) -> i64 {
-        self.call_manager.gas_tracker().gas_available()
+    fn gas_used(&self) -> i64 {
+        self.call_manager.gas_tracker().gas_used()
     }
 
     fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()> {

fvm/src/kernel/mod.rs

@@ -219,8 +219,8 @@ pub trait CircSupplyOps {
 ///  In the future (Phase 1), this should disappear and be replaced by gas instrumentation
 ///  at the WASM level.
 pub trait GasOps {
-    /// GasAvailable return the gas available for the transaction.
-    fn gas_available(&self) -> i64;
+    /// GasUsed return the gas used by the transaction so far.
+    fn gas_used(&self) -> i64;
 
     /// ChargeGas charges specified amount of `gas` for execution.
     /// `name` provides information about gas charging point

fvm/src/syscalls/bind.rs

@@ -106,13 +106,11 @@ fn charge_exec_units_for_gas(caller: &mut Caller<InvocationData<impl Kernel>>) -
         caller.consume_fuel(u64::try_from(exec_units).unwrap_or(0))?;
     }
 
-    let gas_available = caller.data().kernel.gas_available();
+    let gas_used = caller.data().kernel.gas_used();
     let fuel_consumed = caller
         .fuel_consumed()
         .ok_or_else(|| Trap::new("expected to find exec_units consumed"))?;
-    caller
-        .data_mut()
-        .set_snapshots(gas_available, fuel_consumed);
+    caller.data_mut().set_snapshots(gas_used, fuel_consumed);
     Ok(())
 }
 

fvm/src/syscalls/mod.rs

@@ -31,10 +31,10 @@ pub struct InvocationData<K> {
     /// after receiving this error without calling any other syscalls.
     pub last_error: Option<backtrace::Cause>,
 
-    /// This snapshot is used to track changes in available_gas during syscall invocations.
+    /// This snapshot is used to track changes in gas_used during syscall invocations.
     /// The snapshot gets taken when execution exits WASM _after_ charging gas for any newly incurred fuel costs.
-    /// When execution moves back into WASM, we consume fuel for the delta between the snapshot and the new gas_available value.
-    pub gas_available_snapshot: i64,
+    /// When execution moves back into WASM, we consume fuel for the delta between the snapshot and the new gas_used value.
+    pub gas_used_snapshot: i64,
 
     /// This snapshot is used to track changes in fuel_consumed during WASM execution.
     /// The snapshot gets taken when execution enters WASM _after_ consuming fuel for any syscall gas consumption.
@@ -44,46 +44,48 @@ pub struct InvocationData<K> {
 
 impl<K: Kernel> InvocationData<K> {
     pub(crate) fn new(kernel: K) -> Self {
-        let gas_available = kernel.gas_available();
+        let gas_used = kernel.gas_used();
         Self {
             kernel,
             last_error: None,
-            gas_available_snapshot: gas_available,
+            gas_used_snapshot: gas_used,
             exec_units_consumed_snapshot: 0,
         }
     }
 
     /// This method:
-    /// 1) calculates the available_gas delta from the previous snapshot,
-    /// 2) converts this to the corresponding amount of exec_units
-    /// 3) updates the available_gas and exec_units_consumed snapshots. The exec_units_consumed_snapshot is optimistically updated, assuming the value calculated in 2 will be consumed
-    /// 4) returns the value calculated in 2) for its caller to actually consume that exec_units_consumed
+    /// 1) calculates the gas_used delta from the previous snapshot,
+    /// 2) converts this to the corresponding amount of exec_units.
+    /// 3) returns the value calculated in 2) for its caller to actually consume that exec_units_consumed
+    /// The caller should also update the snapshots after doing so.
     pub(crate) fn calculate_exec_units_for_gas(&self) -> Result<i64> {
-        let gas_available = self.kernel.gas_available();
+        let gas_used = self.kernel.gas_used();
         let exec_units_to_consume = self
             .kernel
             .price_list()
-            .gas_to_exec_units(self.gas_available_snapshot - gas_available, true);
+            .gas_to_exec_units(gas_used - self.gas_used_snapshot, true);
         Ok(exec_units_to_consume)
     }
 
-    pub(crate) fn set_snapshots(&mut self, gas_available: i64, exec_units_consumed: u64) {
-        self.gas_available_snapshot = gas_available;
+    pub(crate) fn set_snapshots(&mut self, gas_used: i64, exec_units_consumed: u64) {
+        self.gas_used_snapshot = gas_used;
         self.exec_units_consumed_snapshot = exec_units_consumed;
     }
 
     /// This method:
     /// 1) charges gas corresponding to the exec_units_consumed delta based on the previous snapshot
-    /// 2) updates the exec_units_consumed and gas_available snapshots
+    /// 2) updates the exec_units_consumed and gas_used snapshots
     pub(crate) fn charge_gas_for_exec_units(&mut self, exec_units_consumed: u64) -> Result<()> {
         self.kernel.charge_gas(
             "exec_units",
             self.kernel
                 .price_list()
-                .on_consume_exec_units(exec_units_consumed)
+                .on_consume_exec_units(
+                    exec_units_consumed.saturating_sub(self.exec_units_consumed_snapshot),
+                )
                 .total(),
         )?;
-        self.set_snapshots(self.kernel.gas_available(), exec_units_consumed);
+        self.set_snapshots(self.kernel.gas_used(), exec_units_consumed);
         Ok(())
     }
 }

testing/conformance/src/vm.rs

@@ -488,8 +488,8 @@ where
     C: CallManager<Machine = TestMachine<M>>,
     K: Kernel<CallManager = TestCallManager<C>>,
 {
-    fn gas_available(&self) -> i64 {
-        self.0.gas_available()
+    fn gas_used(&self) -> i64 {
+        self.0.gas_used()
     }
 
     fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()> {