Removes VpsGas and gas dividers

crates/gas/src/lib.rs

@@ -51,7 +51,7 @@ pub enum GasParseError {
 }
 
 // RAW GAS COSTS
-// ================================================================================
+// =============================================================================
 // The raw gas costs exctracted from the benchmarks.
 //
 const COMPILE_GAS_PER_BYTE_RAW: u64 = 1_664;
@@ -119,7 +119,7 @@ const MASP_CONVERT_CHECK_GAS_RAW: u64 = 188_590;
 const MASP_OUTPUT_CHECK_GAS_RAW: u64 = 204_430;
 // The cost to run the final masp check in the bundle
 const MASP_FINAL_CHECK_GAS_RAW: u64 = 43;
-// ================================================================================
+// =============================================================================
 
 // A correction factor for non-WASM-opcodes costs. We can see that the
 // gas cost we get for wasm codes (txs and vps) is much greater than what we
@@ -134,10 +134,10 @@ const MASP_FINAL_CHECK_GAS_RAW: u64 = 43;
 const GAS_COST_CORRECTION: u64 = 5;
 
 // ADJUSTED GAS COSTS
-// ================================================================================
+// =============================================================================
 // The gas costs adjusted for the correction factor.
 //
-const PARALLEL_GAS_DIVIDER: u64 = 1;
+
 // The compilation cost is reduced by a factor to compensate for the (most
 // likely) presence of the cache
 const COMPILE_GAS_PER_BYTE: u64 =
@@ -207,10 +207,7 @@ pub const MASP_OUTPUT_CHECK_GAS: u64 =
 /// The cost to run the final masp check in the bundle
 pub const MASP_FINAL_CHECK_GAS: u64 =
     MASP_FINAL_CHECK_GAS_RAW * GAS_COST_CORRECTION;
-/// Gas divider specific for the masp vp. Only allocates half of the cores to
-/// the masp vp since we can expect the other half to be busy with other vps
-pub const MASP_PARALLEL_GAS_DIVIDER: u64 = 1;
-// ================================================================================
+// =============================================================================
 
 /// Gas module result for functions that may fail
 pub type Result<T> = std::result::Result<T, Error>;
@@ -295,8 +292,10 @@ impl From<Gas> for u64 {
 
 /// Gas represented in whole units. Used for fee payment and to display
 /// information to the user.
+// FIXME: remove Default trait if we remove WholeGas from VpsResult
 #[derive(
     Debug,
+    Default,
     Clone,
     Copy,
     PartialEq,
@@ -375,6 +374,21 @@ pub trait GasMetering {
 
     /// Get the gas limit
     fn get_gas_limit(&self) -> Gas;
+
+    /// Check if the vps went out of gas. Starts with the gas consumed by the
+    /// transaction.
+    ///
+    fn check_vps_limit(&self, vps_gas: Gas) -> Result<()> {
+        let total = self
+            .get_tx_consumed_gas()
+            .checked_add(vps_gas)
+            .ok_or(Error::GasOverflow)?;
+        if total > self.get_gas_limit() {
+            return Err(Error::TransactionGasExceededError);
+        }
+
+        Ok(())
+    }
 }
 
 /// Gas metering in a transaction
@@ -389,6 +403,7 @@ pub struct TxGasMeter {
 
 /// Gas metering in a validity predicate
 #[derive(Debug, Clone)]
+// FIXME: can we simplify this type?
 pub struct VpGasMeter {
     /// Track gas overflow
     gas_overflow: bool,
@@ -400,23 +415,6 @@ pub struct VpGasMeter {
     current_gas: Gas,
 }
 
-/// Gas meter for VPs parallel runs
-#[derive(
-    Clone,
-    Debug,
-    Default,
-    BorshSerialize,
-    BorshDeserialize,
-    BorshDeserializer,
-    BorshSchema,
-    Serialize,
-    Deserialize,
-)]
-pub struct VpsGas {
-    max: Gas,
-    rest: Vec<Gas>,
-}
-
 impl GasMetering for TxGasMeter {
     fn consume(&mut self, gas: u64) -> Result<()> {
         if self.gas_overflow {
@@ -484,11 +482,6 @@ impl TxGasMeter {
         )
     }
 
-    /// Add the gas cost used in validity predicates to the current transaction.
-    pub fn add_vps_gas(&mut self, vps_gas: &VpsGas) -> Result<()> {
-        self.consume(vps_gas.get_current_gas()?.into())
-    }
-
     /// Get the amount of gas still available to the transaction
     pub fn get_available_gas(&self) -> Gas {
         self.tx_gas_limit
@@ -563,64 +556,10 @@ impl VpGasMeter {
             current_gas: Gas::default(),
         }
     }
-}
-
-impl VpsGas {
-    /// Set the gas cost from a VP run. It consumes the [`VpGasMeter`]
-    /// instance which shouldn't be accessed passed this point.
-    pub fn set(&mut self, vp_gas_meter: VpGasMeter) -> Result<()> {
-        if vp_gas_meter.current_gas > self.max {
-            self.rest.push(self.max);
-            self.max = vp_gas_meter.current_gas;
-        } else {
-            self.rest.push(vp_gas_meter.current_gas);
-        }
-
-        self.check_limit(&vp_gas_meter)
-    }
-
-    /// Merge validity predicates gas meters from parallelized runs. Consumes
-    /// the other `VpsGas` instance which shouldn't be used passed this point.
-    pub fn merge(
-        &mut self,
-        mut other: VpsGas,
-        tx_gas_meter: &TxGasMeter,
-    ) -> Result<()> {
-        if self.max < other.max {
-            self.rest.push(self.max);
-            self.max = other.max;
-        } else {
-            self.rest.push(other.max);
-        }
-        self.rest.append(&mut other.rest);
-
-        self.check_limit(tx_gas_meter)
-    }
-
-    /// Check if the vp went out of gas. Starts from the gas consumed by the
-    /// transaction.
-    fn check_limit(&self, gas_meter: &impl GasMetering) -> Result<()> {
-        let total = gas_meter
-            .get_tx_consumed_gas()
-            .checked_add(self.get_current_gas()?)
-            .ok_or(Error::GasOverflow)?;
-        if total > gas_meter.get_gas_limit() {
-            return Err(Error::TransactionGasExceededError);
-        }
-        Ok(())
-    }
 
-    /// Get the gas consumed by the parallelized VPs
-    fn get_current_gas(&self) -> Result<Gas> {
-        let parallel_gas = self
-            .rest
-            .iter()
-            .try_fold(Gas::default(), |acc, gas| {
-                acc.checked_add(*gas).ok_or(Error::GasOverflow)
-            })?
-            .checked_div(PARALLEL_GAS_DIVIDER)
-            .expect("Div by non-zero int cannot fail");
-        self.max.checked_add(parallel_gas).ok_or(Error::GasOverflow)
+    /// Get the gas consumed by the VP alone
+    pub fn get_vp_consumed_gas(&self) -> Gas {
+        self.current_gas
     }
 }
 

crates/node/src/protocol.rs

@@ -11,7 +11,7 @@ use namada_sdk::events::extend::{
     ComposeEvent, Height as HeightAttr, TxHash as TxHashAttr, UserAccount,
 };
 use namada_sdk::events::EventLevel;
-use namada_sdk::gas::{Gas, GasMetering, TxGasMeter, VpGasMeter};
+use namada_sdk::gas::{self, Gas, GasMetering, TxGasMeter, VpGasMeter};
 use namada_sdk::hash::Hash;
 use namada_sdk::ibc::{IbcTxDataHash, IbcTxDataRefs};
 use namada_sdk::masp::{MaspTxRefs, TxId};
@@ -1104,7 +1104,7 @@ where
     tracing::debug!("Total VPs gas cost {:?}", vps_result.gas_used);
 
     tx_gas_meter
-        .add_vps_gas(&vps_result.gas_used)
+        .consume(vps_result.gas_used.into())
         .map_err(|err| Error::GasError(err.to_string()))?;
 
     Ok(vps_result)
@@ -1324,9 +1324,13 @@ where
             // all the other errors we keep evaluating the vps. This
             // allows to display a consistent VpsResult across all
             // nodes and find any invalid signatures
-            result
+            result.gas_used = result
                 .gas_used
-                .set(gas_meter.into_inner())
+                .checked_add(gas_meter.borrow().get_vp_consumed_gas())
+                .ok_or(Error::GasError(gas::Error::GasOverflow.to_string()))?;
+            gas_meter
+                .borrow()
+                .check_vps_limit(result.gas_used)
                 .map_err(|err| Error::GasError(err.to_string()))?;
 
             Ok(result)
@@ -1351,10 +1355,13 @@ fn merge_vp_results(
     let mut errors = a.errors;
     errors.append(&mut b.errors);
     let status_flags = a.status_flags | b.status_flags;
-    let mut gas_used = a.gas_used;
 
-    gas_used
-        .merge(b.gas_used, tx_gas_meter)
+    let gas_used = a
+        .gas_used
+        .checked_add(b.gas_used)
+        .ok_or(Error::GasError(gas::Error::GasOverflow.to_string()))?;
+    tx_gas_meter
+        .check_vps_limit(gas_used)
         .map_err(|err| Error::GasError(err.to_string()))?;
 
     Ok(VpsResult {

crates/shielded_token/src/validation.rs

@@ -236,7 +236,6 @@ where
         consume_verify_gas(namada_gas::MASP_FIXED_SPEND_GAS)?;
         consume_verify_gas(checked!(
             namada_gas::MASP_VARIABLE_SPEND_GAS * remaining_notes as u64
-                / namada_gas::MASP_PARALLEL_GAS_DIVIDER
         )?)?;
     }
 
@@ -246,7 +245,6 @@ where
         consume_verify_gas(namada_gas::MASP_FIXED_CONVERT_GAS)?;
         consume_verify_gas(checked!(
             namada_gas::MASP_VARIABLE_CONVERT_GAS * remaining_notes as u64
-                / namada_gas::MASP_PARALLEL_GAS_DIVIDER
         )?)?;
     }
 
@@ -256,7 +254,6 @@ where
         consume_verify_gas(namada_gas::MASP_FIXED_OUTPUT_GAS)?;
         consume_verify_gas(checked!(
             namada_gas::MASP_VARIABLE_OUTPUT_GAS * remaining_notes as u64
-                / namada_gas::MASP_PARALLEL_GAS_DIVIDER
         )?)?;
     }
 

crates/tx/src/data/mod.rs

@@ -26,7 +26,7 @@ use namada_core::ibc::IbcTxDataRefs;
 use namada_core::masp::MaspTxRefs;
 use namada_core::storage;
 use namada_events::Event;
-use namada_gas::{VpsGas, WholeGas};
+use namada_gas::{Gas, WholeGas};
 use namada_macros::BorshDeserializer;
 #[cfg(feature = "migrations")]
 use namada_migrations::*;
@@ -482,7 +482,8 @@ pub struct VpsResult {
     /// The addresses whose VPs rejected the transaction
     pub rejected_vps: BTreeSet<Address>,
     /// The total gas used by all the VPs
-    pub gas_used: VpsGas,
+    // FIXME: can't we just remove it?
+    pub gas_used: Gas,
     /// Errors occurred in any of the VPs, if any
     pub errors: Vec<(Address, String)>,
     /// Validity predicate status flags, containing info