Merge branch 'brent/parameterize-gas-scale' (#3391)

* brent/parameterize-gas-scale:
  Updated example of expected string
  Fixes ibc e2e test
  fix unit test
  change comment on Gas Display
  fixes from comments
  changelog: add #3391
  fix and clean up
  Light error handling
  remove hard-coded gas scale
  add gas scale to protocol params

.changelog/unreleased/improvements/3391-parameterize-gas-scale.md

@@ -0,0 +1,3 @@
+- Include the gas scale as a protocol parameter that is
+  mutable via governance rather than as a hard-coded constant.
+  ([\#3391](https://github.com/anoma/namada/pull/3391))

crates/apps_lib/src/config/genesis.rs

@@ -311,6 +311,8 @@ pub struct Parameters {
     pub masp_epoch_multiplier: u64,
     /// Fee unshielding gas limit
     pub fee_unshielding_gas_limit: u64,
+    /// Gas scale
+    pub gas_scale: u64,
     /// Map of the cost per gas unit for every token allowed for fee payment
     pub minimum_gas_price: BTreeMap<Address, token::Amount>,
 }

crates/apps_lib/src/config/genesis/chain.rs

@@ -303,6 +303,7 @@ impl Finalized {
             epochs_per_year,
             masp_epoch_multiplier,
             fee_unshielding_gas_limit,
+            gas_scale,
             max_block_gas,
             minimum_gas_price,
             max_tx_bytes,
@@ -344,6 +345,7 @@ impl Finalized {
             masp_epoch_multiplier,
             max_proposal_bytes,
             fee_unshielding_gas_limit,
+            gas_scale,
             max_block_gas,
             minimum_gas_price: minimum_gas_price
                 .iter()

crates/apps_lib/src/config/genesis/templates.rs

@@ -296,6 +296,8 @@ pub struct ChainParams<T: TemplateValidation> {
     pub max_block_gas: u64,
     /// Fee unshielding gas limit
     pub fee_unshielding_gas_limit: u64,
+    /// Gas scale
+    pub gas_scale: u64,
     /// Map of the cost per gas unit for every token allowed for fee payment
     pub minimum_gas_price: T::GasMinimums,
 }
@@ -318,6 +320,7 @@ impl ChainParams<Unvalidated> {
             masp_epoch_multiplier,
             max_block_gas,
             fee_unshielding_gas_limit,
+            gas_scale,
             minimum_gas_price,
         } = self;
         let mut min_gas_prices = BTreeMap::default();
@@ -362,6 +365,7 @@ impl ChainParams<Unvalidated> {
             masp_epoch_multiplier,
             max_block_gas,
             fee_unshielding_gas_limit,
+            gas_scale,
             minimum_gas_price: min_gas_prices,
         })
     }

crates/core/src/parameters.rs

@@ -49,6 +49,8 @@ pub struct Parameters {
     pub masp_epoch_multiplier: u64,
     /// Fee unshielding gas limit
     pub fee_unshielding_gas_limit: u64,
+    /// Gas scale
+    pub gas_scale: u64,
     /// Map of the cost per gas unit for every token allowed for fee payment
     pub minimum_gas_price: BTreeMap<Address, token::Amount>,
     /// Enable the native token transfer if it is true

crates/gas/src/lib.rs

@@ -113,9 +113,6 @@ pub const MASP_PARALLEL_GAS_DIVIDER: u64 = PARALLEL_GAS_DIVIDER / 2;
 /// Gas module result for functions that may fail
 pub type Result<T> = std::result::Result<T, Error>;
 
-/// Decimal scale of Gas units
-const SCALE: u64 = 100_000_000;
-
 /// Representation of gas in sub-units. This effectively decouples gas metering
 /// from fee payment, allowing higher resolution when accounting for gas while,
 /// at the same time, providing a contained gas value when paying fees.
@@ -155,9 +152,17 @@ impl Gas {
 
     /// Converts the sub gas units to whole ones. If the sub units are not a
     /// multiple of the `SCALE` than ceil the quotient
-    fn get_whole_gas_units(&self) -> u64 {
-        let quotient = self.sub / SCALE;
-        if self.sub % SCALE == 0 {
+    pub fn get_whole_gas_units(&self, scale: u64) -> u64 {
+        let quotient = self
+            .sub
+            .checked_div(scale)
+            .expect("Gas quotient should not overflow on checked division");
+        if self
+            .sub
+            .checked_rem(scale)
+            .expect("Gas quotient remainder should not overflow")
+            == 0
+        {
             quotient
         } else {
             quotient
@@ -167,8 +172,8 @@ impl Gas {
     }
 
     /// Generates a `Gas` instance from a whole amount
-    pub fn from_whole_units(whole: u64) -> Option<Self> {
-        let sub = whole.checked_mul(SCALE)?;
+    pub fn from_whole_units(whole: u64, scale: u64) -> Option<Self> {
+        let sub = whole.checked_mul(scale)?;
         Some(Self { sub })
     }
 }
@@ -187,8 +192,9 @@ impl From<Gas> for u64 {
 
 impl Display for Gas {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        // Display the gas in whole amounts
-        write!(f, "{}", self.get_whole_gas_units())
+        // Need to do this now that the gas scale is a parameter. Should
+        // manually scale gas first before calling this
+        write!(f, "{}", self.sub)
     }
 }
 
@@ -197,8 +203,7 @@ impl FromStr for Gas {
 
     fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
         let raw: u64 = s.parse().map_err(GasParseError::Parse)?;
-        let gas = Gas::from_whole_units(raw).ok_or(GasParseError::Overflow)?;
-        Ok(gas)
+        Ok(Self { sub: raw })
     }
 }
 

crates/namada/src/ledger/mod.rs

@@ -23,7 +23,6 @@ pub use {
 mod dry_run_tx {
     use std::cell::RefCell;
 
-    use namada_gas::Gas;
     use namada_sdk::queries::{EncodedResponseQuery, RequestCtx, RequestQuery};
     use namada_state::{DBIter, ResultExt, StorageHasher, DB};
     use namada_tx::data::{GasLimit, TxResult};
@@ -54,43 +53,47 @@ mod dry_run_tx {
         let tx = Tx::try_from(&request.data[..]).into_storage_result()?;
         tx.validate_tx().into_storage_result()?;
 
-        // Wrapper dry run to allow estimating the gas cost of a transaction
-        let (wrapper_hash, mut tx_result, tx_gas_meter) = match tx
-            .header()
-            .tx_type
-        {
-            TxType::Wrapper(wrapper) => {
-                let gas_limit =
-                    Gas::try_from(wrapper.gas_limit).into_storage_result()?;
-                let tx_gas_meter = RefCell::new(TxGasMeter::new(gas_limit));
-                let tx_result = protocol::apply_wrapper_tx(
-                    &tx,
-                    &wrapper,
-                    &request.data,
-                    &tx_gas_meter,
-                    &mut temp_state,
-                    None,
-                )
-                .into_storage_result()?;
+        let gas_scale = namada_parameters::get_gas_scale(ctx.state)?;
 
-                temp_state.write_log_mut().commit_tx();
-                let available_gas = tx_gas_meter.borrow().get_available_gas();
-                (
-                    Some(tx.header_hash()),
-                    tx_result,
-                    TxGasMeter::new_from_sub_limit(available_gas),
-                )
-            }
-            _ => {
-                // If dry run only the inner tx, use the max block gas as
-                // the gas limit
-                let max_block_gas =
-                    namada_parameters::get_max_block_gas(ctx.state)?;
-                let gas_limit = Gas::try_from(GasLimit::from(max_block_gas))
+        // Wrapper dry run to allow estimating the gas cost of a transaction
+        let (wrapper_hash, mut tx_result, tx_gas_meter) =
+            match tx.header().tx_type {
+                TxType::Wrapper(wrapper) => {
+                    let gas_limit = wrapper
+                        .gas_limit
+                        .as_scaled_gas(gas_scale)
+                        .into_storage_result()?;
+                    let tx_gas_meter = RefCell::new(TxGasMeter::new(gas_limit));
+                    let tx_result = protocol::apply_wrapper_tx(
+                        &tx,
+                        &wrapper,
+                        &request.data,
+                        &tx_gas_meter,
+                        &mut temp_state,
+                        None,
+                    )
                     .into_storage_result()?;
-                (None, TxResult::default(), TxGasMeter::new(gas_limit))
-            }
-        };
+
+                    temp_state.write_log_mut().commit_tx();
+                    let available_gas =
+                        tx_gas_meter.borrow().get_available_gas();
+                    (
+                        Some(tx.header_hash()),
+                        tx_result,
+                        TxGasMeter::new_from_sub_limit(available_gas),
+                    )
+                }
+                _ => {
+                    // If dry run only the inner tx, use the max block gas as
+                    // the gas limit
+                    let max_block_gas =
+                        namada_parameters::get_max_block_gas(ctx.state)?;
+                    let gas_limit = GasLimit::from(max_block_gas)
+                        .as_scaled_gas(gas_scale)
+                        .into_storage_result()?;
+                    (None, TxResult::default(), TxGasMeter::new(gas_limit))
+                }
+            };
 
         let tx_gas_meter = RefCell::new(tx_gas_meter);
         for cmt in tx.commitments() {
@@ -195,6 +198,12 @@ mod test {
                 .expect(
                     "Max block gas parameter must be initialized in storage",
                 );
+            // Initialize mock gas scale
+            let gas_scale_key = namada_parameters::storage::get_gas_scale_key();
+            state
+                .db_write(&gas_scale_key, 100_000_000_u64.serialize_to_vec())
+                .expect("Gas scale parameter must be initialized in storage");
+
             let event_log = EventLog::default();
             let (vp_wasm_cache, vp_cache_dir) =
                 wasm::compilation_cache::common::testing::cache();

crates/namada/src/ledger/protocol/mod.rs

@@ -266,6 +266,7 @@ where
                         tx_wasm_cache,
                     },
                 )?;
+
                 Ok(TxResult {
                     gas_used: tx_gas_meter.borrow().get_tx_consumed_gas(),
                     batch_results: {

crates/node/src/shell/finalize_block.rs

@@ -29,6 +29,7 @@ use namada::tx::event::{Batch, Code};
 use namada::tx::new_tx_event;
 use namada::vote_ext::ethereum_events::MultiSignedEthEvent;
 use namada::vote_ext::ethereum_tx_data_variants;
+use parameters::get_gas_scale;
 
 use super::*;
 use crate::facade::tendermint::abci::types::VoteInfo;
@@ -385,9 +386,17 @@ where
                         .unwrap_or("<unknown>"),
                     msg,
                 );
+                let gas_scale = get_gas_scale(&self.state)
+                    .expect("Failed to get gas scale from parameters");
+                let scaled_gas = Gas::from(
+                    tx_data
+                        .tx_gas_meter
+                        .get_tx_consumed_gas()
+                        .get_whole_gas_units(gas_scale),
+                );
                 tx_logs
                     .tx_event
-                    .extend(GasUsed(tx_data.tx_gas_meter.get_tx_consumed_gas()))
+                    .extend(GasUsed(scaled_gas))
                     .extend(Info(msg.to_string()))
                     .extend(Code(ResultCode::InvalidTx));
                 // Make sure to clean the write logs for the next transaction
@@ -410,9 +419,18 @@ where
                     msg
                 );
 
+                let gas_scale = get_gas_scale(&self.state)
+                    .expect("Failed to get gas scale from parameters");
+                let scaled_gas = Gas::from(
+                    tx_data
+                        .tx_gas_meter
+                        .get_tx_consumed_gas()
+                        .get_whole_gas_units(gas_scale),
+                );
+
                 tx_logs
                     .tx_event
-                    .extend(GasUsed(tx_data.tx_gas_meter.get_tx_consumed_gas()))
+                    .extend(GasUsed(scaled_gas))
                     .extend(Info(msg.to_string()))
                     .extend(Code(ResultCode::WasmRuntimeError));
 
@@ -487,9 +505,18 @@ where
             self.commit_batch_hash(tx_data.replay_protection_hashes);
         }
 
+        let gas_scale = get_gas_scale(&self.state)
+            .expect("Failed to get gas scale from parameters");
+        let scaled_gas = Gas::from(
+            tx_data
+                .tx_gas_meter
+                .get_tx_consumed_gas()
+                .get_whole_gas_units(gas_scale),
+        );
+
         tx_logs
             .tx_event
-            .extend(GasUsed(extended_tx_result.tx_result.gas_used))
+            .extend(GasUsed(scaled_gas))
             .extend(Info("Check batch for result.".to_string()))
             .extend(Batch(&extended_tx_result.tx_result.to_result_string()));
     }
@@ -667,22 +694,25 @@ where
                 TxType::Wrapper(wrapper) => {
                     stats.increment_wrapper_txs();
 
-                    let gas_limit = match Gas::try_from(wrapper.gas_limit) {
-                        Ok(value) => value,
-                        Err(_) => {
-                            response.events.emit(
-                                new_tx_event(&tx, height.0)
-                                    .with(Code(ResultCode::InvalidTx))
-                                    .with(Info(
-                                        "The wrapper gas limit overflowed gas \
-                                         representation"
-                                            .to_owned(),
-                                    ))
-                                    .with(GasUsed(0.into())),
-                            );
-                            continue;
-                        }
-                    };
+                    let gas_scale = get_gas_scale(&self.state)
+                        .expect("Failed to get gas scale from parameters");
+                    let gas_limit =
+                        match wrapper.gas_limit.as_scaled_gas(gas_scale) {
+                            Ok(value) => value,
+                            Err(_) => {
+                                response.events.emit(
+                                    new_tx_event(&tx, height.0)
+                                        .with(Code(ResultCode::InvalidTx))
+                                        .with(Info(
+                                            "The wrapper gas limit overflowed \
+                                             gas representation"
+                                                .to_owned(),
+                                        ))
+                                        .with(GasUsed(0.into())),
+                                );
+                                continue;
+                            }
+                        };
                     let tx_gas_meter = TxGasMeter::new(gas_limit);
                     for cmt in tx.commitments() {
                         if let Some(code_sec) = tx

crates/node/src/shell/mod.rs

@@ -50,7 +50,7 @@ use namada::ledger::pos::namada_proof_of_stake::types::{
 };
 use namada::ledger::protocol::ShellParams;
 use namada::ledger::{parameters, protocol};
-use namada::parameters::validate_tx_bytes;
+use namada::parameters::{get_gas_scale, validate_tx_bytes};
 use namada::proof_of_stake::storage::read_pos_params;
 use namada::state::tx_queue::ExpiredTx;
 use namada::state::{
@@ -1095,9 +1095,22 @@ where
                 }
             },
             TxType::Wrapper(wrapper) => {
+                // Get the gas scale first
+                let gas_scale = match get_gas_scale(&self.state) {
+                    Ok(scale) => scale,
+                    Err(_) => {
+                        response.code = ResultCode::InvalidTx.into();
+                        response.log = "The gas scale could not be found in \
+                                        the parameters storage"
+                            .to_string();
+                        return response;
+                    }
+                };
+
                 // Validate wrapper first
                 // Tx gas limit
-                let gas_limit = match Gas::try_from(wrapper.gas_limit) {
+                let gas_limit = match wrapper.gas_limit.as_scaled_gas(gas_scale)
+                {
                     Ok(value) => value,
                     Err(_) => {
                         response.code = ResultCode::InvalidTx.into();
@@ -1119,6 +1132,7 @@ where
                 // Max block gas
                 let block_gas_limit: Gas = Gas::from_whole_units(
                     namada::parameters::get_max_block_gas(&self.state).unwrap(),
+                    gas_scale,
                 )
                 .expect("Gas limit from parameter must not overflow");
                 if gas_meter.tx_gas_limit > block_gas_limit {