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

* brent/parameterize-gas-scale:
  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,11 +53,15 @@ mod dry_run_tx {
         let tx = Tx::try_from(&request.data[..]).into_storage_result()?;
         tx.validate_tx().into_storage_result()?;
 
+        let gas_scale = namada_parameters::get_gas_scale(ctx.state)?;
+
         // Wrapper dry run to allow estimating the gas cost of a transaction
         let (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 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,
@@ -79,7 +82,8 @@ mod dry_run_tx {
                 // 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))
+                let gas_limit = GasLimit::from(max_block_gas)
+                    .as_scaled_gas(gas_scale)
                     .into_storage_result()?;
                 (TxResult::default(), TxGasMeter::new(gas_limit))
             }

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

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

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()));
     }
@@ -669,22 +696,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

@@ -48,7 +48,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::{
@@ -1067,9 +1067,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();
@@ -1091,6 +1104,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 {

crates/node/src/shell/prepare_proposal.rs

@@ -4,9 +4,10 @@ use std::cell::RefCell;
 
 use namada::core::address::Address;
 use namada::core::key::tm_raw_hash_to_string;
-use namada::gas::{Gas, TxGasMeter};
+use namada::gas::TxGasMeter;
 use namada::hash::Hash;
 use namada::ledger::protocol::{self, ShellParams};
+use namada::parameters::get_gas_scale;
 use namada::proof_of_stake::storage::find_validator_by_raw_hash;
 use namada::state::{DBIter, StorageHasher, TempWlState, DB};
 use namada::token::{Amount, DenominatedAmount};
@@ -288,7 +289,9 @@ where
     tx.validate_tx().map_err(|_| ())?;
     if let TxType::Wrapper(wrapper) = tx.header().tx_type {
         // Check tx gas limit for tx size
-        let gas_limit = Gas::try_from(wrapper.gas_limit).map_err(|_| ())?;
+        let gas_scale = get_gas_scale(temp_state).map_err(|_| ())?;
+        let gas_limit =
+            wrapper.gas_limit.as_scaled_gas(gas_scale).map_err(|_| ())?;
         let mut tx_gas_meter = TxGasMeter::new(gas_limit);
         tx_gas_meter.add_wrapper_gas(tx_bytes).map_err(|_| ())?;
 

crates/node/src/shell/process_proposal.rs

@@ -410,7 +410,17 @@ where
                 let allocated_gas =
                     metadata.user_gas.try_dump(u64::from(wrapper.gas_limit));
 
-                let gas_limit = match Gas::try_from(wrapper.gas_limit) {
+                let gas_scale = match get_gas_scale(temp_state) {
+                    Ok(scale) => scale,
+                    Err(_) => {
+                        return TxResult {
+                            code: ResultCode::TxGasLimit.into(),
+                            info: "Failed to get gas scale".to_owned(),
+                        };
+                    }
+                };
+                let gas_limit = match wrapper.gas_limit.as_scaled_gas(gas_scale)
+                {
                     Ok(value) => value,
                     Err(_) => {
                         return TxResult {

crates/node/src/storage/mod.rs

@@ -174,6 +174,7 @@ mod tests {
             epochs_per_year: 365,
             masp_epoch_multiplier: 2,
             fee_unshielding_gas_limit: 0,
+            gas_scale: 100_000_000,
             minimum_gas_price: Default::default(),
             is_native_token_transferable: true,
         };