From cd68f351714906c3cb1abdcd5e4f731db98893fe Mon Sep 17 00:00:00 2001 From: ptrus Date: Wed, 27 Sep 2023 13:45:29 +0200 Subject: [PATCH] runtime-sdk: support dynamic min-gas-price --- runtime-sdk/modules/evm/src/backend.rs | 3 +- .../modules/evm/src/precompile/testing.rs | 1 + runtime-sdk/src/dispatcher.rs | 1 + runtime-sdk/src/modules/core/mod.rs | 186 ++++++++++++++++-- runtime-sdk/src/modules/core/test.rs | 160 ++++++++++++++- tests/runtimes/benchmarking/src/lib.rs | 1 + tests/runtimes/simple-consensus/src/lib.rs | 1 + tests/runtimes/simple-contracts/src/lib.rs | 1 + tests/runtimes/simple-evm/src/lib.rs | 1 + tests/runtimes/simple-keyvalue/src/lib.rs | 1 + tests/runtimes/simple-keyvalue/src/test.rs | 1 + 11 files changed, 334 insertions(+), 23 deletions(-) diff --git a/runtime-sdk/modules/evm/src/backend.rs b/runtime-sdk/modules/evm/src/backend.rs index 0ae4daac80..2450ba1210 100644 --- a/runtime-sdk/modules/evm/src/backend.rs +++ b/runtime-sdk/modules/evm/src/backend.rs @@ -354,9 +354,10 @@ impl<'ctx, 'backend, 'config, C: TxContext, Cfg: Config> Backend fn block_base_fee_per_gas(&self) -> U256 { ::Core::min_gas_price( - &mut self.backend.ctx.borrow_mut(), + &self.backend.ctx.borrow_mut(), &Cfg::TOKEN_DENOMINATION, ) + .unwrap_or_default() .into() } diff --git a/runtime-sdk/modules/evm/src/precompile/testing.rs b/runtime-sdk/modules/evm/src/precompile/testing.rs index eff8050599..b9717f5f67 100644 --- a/runtime-sdk/modules/evm/src/precompile/testing.rs +++ b/runtime-sdk/modules/evm/src/precompile/testing.rs @@ -221,6 +221,7 @@ impl Runtime for TestRuntime { max_multisig_signers: 8, gas_costs: Default::default(), min_gas_price: BTreeMap::from([(token::Denomination::NATIVE, 0)]), + dynamic_min_gas_price: Default::default(), }, }, accounts::Genesis { diff --git a/runtime-sdk/src/dispatcher.rs b/runtime-sdk/src/dispatcher.rs index 2fc3e0b1f9..8f7a0e590b 100644 --- a/runtime-sdk/src/dispatcher.rs +++ b/runtime-sdk/src/dispatcher.rs @@ -1028,6 +1028,7 @@ mod test { callformat_x25519_deoxysii: 0, }, min_gas_price: BTreeMap::from([(token::Denomination::NATIVE, 0)]), + dynamic_min_gas_price: Default::default(), }, }, (), diff --git a/runtime-sdk/src/modules/core/mod.rs b/runtime-sdk/src/modules/core/mod.rs index f66bf7ad81..51b8558c45 100644 --- a/runtime-sdk/src/modules/core/mod.rs +++ b/runtime-sdk/src/modules/core/mod.rs @@ -23,7 +23,7 @@ use crate::{ sender::SenderMeta, storage::{self, current::TransactionResult, CurrentStore}, types::{ - token, + token::{self, Denomination}, transaction::{ self, AddressSpec, AuthProof, Call, CallFormat, CallerAddress, UnverifiedTransaction, }, @@ -230,6 +230,37 @@ pub struct GasCosts { pub callformat_x25519_deoxysii: u64, } +/// Dynamic min gas price parameters. +#[derive(Clone, Debug, Default, cbor::Encode, cbor::Decode)] +pub struct DynamicMinGasPrice { + /// Enables the dynamic min gas price feature which dynamically adjusts the minimum gas price + /// based on block fullness, inspired by EIP-1559. + /// + /// Only takes effect if `min_gas_price`(s) are set. + pub enabled: bool, + + /// Target block gas usage indicates the desired block gas usage as a percentage of the total + /// block gas limit. + /// + /// The min gas price will adjust up or down depending on whether the actual gas usage is above + /// or below this target. + pub target_block_gas_usage_percentage: u8, + /// Represents a constant value used to limit the rate at which the min price can change + /// between blocks. + /// + /// For example, if `min_price_max_change_denominator` is set to 8, the maximum change in + /// min price is 12.5% between blocks. + pub min_price_max_change_denominator: u8, +} + +/// Errors emitted during core parameter validation. +#[derive(Error, Debug)] +pub enum ParameterValidationError { + #[error("invalid dynamic target block gas usage percentage (10-100)")] + InvalidTargetBlockGasUsagePercentage, + #[error("invalid dynamic min price max change denominator (1-50)")] + InvalidMinPriceMaxChangeDenominator, +} /// Parameters for the core module. #[derive(Clone, Debug, Default, cbor::Encode, cbor::Decode)] pub struct Parameters { @@ -239,10 +270,29 @@ pub struct Parameters { pub max_multisig_signers: u32, pub gas_costs: GasCosts, pub min_gas_price: BTreeMap, + pub dynamic_min_gas_price: DynamicMinGasPrice, } impl module::Parameters for Parameters { - type Error = std::convert::Infallible; + type Error = ParameterValidationError; + + fn validate_basic(&self) -> Result<(), Self::Error> { + // Validate dynamic min gas price parameters. + let dmgp = &self.dynamic_min_gas_price; + if dmgp.enabled { + if dmgp.target_block_gas_usage_percentage < 10 + || dmgp.target_block_gas_usage_percentage > 100 + { + return Err(ParameterValidationError::InvalidTargetBlockGasUsagePercentage); + } + if dmgp.min_price_max_change_denominator < 1 + || dmgp.min_price_max_change_denominator > 50 + { + return Err(ParameterValidationError::InvalidMinPriceMaxChangeDenominator); + } + } + Ok(()) + } } pub trait API { @@ -262,6 +312,9 @@ pub trait API { /// Returns the remaining batch-wide gas. fn remaining_batch_gas(ctx: &mut C) -> u64; + /// Returns the total batch-wide gas used. + fn used_batch_gas(ctx: &mut C) -> u64; + /// Return the remaining tx-wide gas. fn remaining_tx_gas(ctx: &mut C) -> u64; @@ -272,7 +325,7 @@ pub trait API { fn max_batch_gas(ctx: &mut C) -> u64; /// Configured minimum gas price. - fn min_gas_price(ctx: &mut C, denom: &token::Denomination) -> u128; + fn min_gas_price(ctx: &C, denom: &token::Denomination) -> Option; /// Sets the transaction priority to the provided amount. fn set_priority(ctx: &mut C, priority: u64); @@ -331,6 +384,8 @@ pub mod state { pub const MESSAGE_HANDLERS: &[u8] = &[0x02]; /// Last processed epoch for detecting epoch changes. pub const LAST_EPOCH: &[u8] = &[0x03]; + /// Dynamic min gas price. + pub const DYNAMIC_MIN_GAS_PRICE: &[u8] = &[0x04]; } /// Module configuration. @@ -383,7 +438,7 @@ impl API for Module { return Ok(()); } let batch_gas_limit = Self::params().max_batch_gas; - let batch_gas_used = ctx.value::(CONTEXT_KEY_GAS_USED).or_default(); + let batch_gas_used = Self::used_batch_gas(ctx); // NOTE: Going over the batch limit should trigger an abort as the scheduler should never // allow scheduling past the batch limit but a malicious proposer might include too // many transactions. Make sure to vote for failure in this case. @@ -420,8 +475,14 @@ impl API for Module { fn remaining_batch_gas(ctx: &mut C) -> u64 { let batch_gas_limit = Self::params().max_batch_gas; - let batch_gas_used = ctx.value::(CONTEXT_KEY_GAS_USED).or_default(); - batch_gas_limit.saturating_sub(*batch_gas_used) + batch_gas_limit.saturating_sub(Self::used_batch_gas(ctx)) + } + + fn used_batch_gas(ctx: &mut C) -> u64 { + ctx.value::(CONTEXT_KEY_GAS_USED) + .get() + .cloned() + .unwrap_or_default() } fn remaining_tx_gas(ctx: &mut C) -> u64 { @@ -441,12 +502,8 @@ impl API for Module { Self::params().max_batch_gas } - fn min_gas_price(_ctx: &mut C, denom: &token::Denomination) -> u128 { - Self::params() - .min_gas_price - .get(denom) - .copied() - .unwrap_or_default() + fn min_gas_price(ctx: &C, denom: &token::Denomination) -> Option { + Self::min_gas_prices(ctx).get(denom).copied() } fn set_priority(ctx: &mut C, priority: u64) { @@ -774,10 +831,9 @@ impl Module { ctx: &mut C, _args: (), ) -> Result, Error> { - let params = Self::params(); + let mut mgp = Self::min_gas_prices(ctx); // Generate a combined view with local overrides. - let mut mgp = params.min_gas_price; for (denom, price) in mgp.iter_mut() { let local_mgp = Self::get_local_min_gas_price(ctx, denom); if local_mgp > *price { @@ -862,6 +918,22 @@ impl Module { } impl Module { + fn min_gas_prices(_ctx: &C) -> BTreeMap { + let params = Self::params(); + if params.dynamic_min_gas_price.enabled { + CurrentStore::with(|store| { + let store = + storage::TypedStore::new(storage::PrefixStore::new(store, &MODULE_NAME)); + store + .get(state::DYNAMIC_MIN_GAS_PRICE) + // Use static min gas price when dynamic price was not yet computed. + .unwrap_or(params.min_gas_price) + }) + } else { + params.min_gas_price + } + } + fn get_local_min_gas_price(ctx: &C, denom: &token::Denomination) -> u128 { #[allow(clippy::borrow_interior_mutable_const)] ctx.local_config(MODULE_NAME) @@ -885,11 +957,10 @@ impl Module { return Ok(()); } - let params = Self::params(); let fee = ctx.tx_auth_info().fee.clone(); let denom = fee.amount.denomination(); - match params.min_gas_price.get(denom) { + match Self::min_gas_price(ctx, denom) { // If the denomination is not among the global set, reject. None => return Err(Error::GasPriceTooLow), @@ -904,7 +975,7 @@ impl Module { } } - if &fee.gas_price() < min_gas_price { + if fee.gas_price() < min_gas_price { return Err(Error::GasPriceTooLow); } } @@ -1022,6 +1093,40 @@ impl module::TransactionHandler for Module { } } +/// Computes the new minimum gas price based on the current gas usage and the target gas usage. +/// +/// The new price is computed as follows (inspired by EIP-1559): +/// - If the actual gas used is greater than the target gas used, increase the minimum gas price. +/// - If the actual gas used is less than the target gas used, decrease the minimum gas price. +/// +/// The price change is controlled by the `min_price_max_change_denominator` parameter. +fn min_gas_price_update( + gas_used: u128, + target_gas_used: u128, + min_price_max_change_denominator: u128, + current_price: u128, +) -> u128 { + // If the target gas used is zero or the denominator is zero, don't change the price. + if target_gas_used == 0 || min_price_max_change_denominator == 0 { + return current_price; + } + + // Calculate the difference (as a percentage) between the actual gas used in the block and the target gas used. + let delta = (gas_used.max(target_gas_used) - gas_used.min(target_gas_used)).saturating_mul(100) + / target_gas_used; + + // Calculate the change in gas price and divide by `min_price_max_change_denominator`. + let price_change = + (current_price.saturating_mul(delta) / 100) / min_price_max_change_denominator; + + // Adjust the current price based on whether the gas used was above or below the target. + if gas_used > target_gas_used { + current_price.saturating_add(price_change) + } else { + current_price.saturating_sub(price_change) + } +} + impl module::BlockHandler for Module { fn begin_block(ctx: &mut C) { CurrentStore::with(|store| { @@ -1040,6 +1145,53 @@ impl module::BlockHandler for Module { .set(epoch != previous_epoch); }); } + + fn end_block(ctx: &mut C) { + let params = Self::params(); + if !params.dynamic_min_gas_price.enabled { + return; + } + + // Update dynamic min gas price for next block, inspired by EIP-1559. + // + // Adjust the min gas price for each block based on the gas used in the previous block and the desired target + // gas usage set by `target_block_gas_usage_percentage`. + let gas_used = Self::used_batch_gas(ctx) as u128; + let max_batch_gas = Self::max_batch_gas(ctx) as u128; + let target_gas_used = max_batch_gas.saturating_mul( + params + .dynamic_min_gas_price + .target_block_gas_usage_percentage as u128, + ) / 100; + + // Compute new prices. + let mut mgp = Self::min_gas_prices(ctx); + mgp.iter_mut().for_each(|(d, price)| { + let mut new_min_price = min_gas_price_update( + gas_used, + target_gas_used, + params + .dynamic_min_gas_price + .min_price_max_change_denominator as u128, + *price, + ); + + // Ensure that the new price is at least the minimum gas price. + if let Some(min_price) = params.min_gas_price.get(d) { + if new_min_price < *min_price { + new_min_price = *min_price; + } + } + *price = new_min_price; + }); + + // Update min prices. + CurrentStore::with(|store| { + let mut store = storage::PrefixStore::new(store, &MODULE_NAME); + let mut tstore = storage::TypedStore::new(&mut store); + tstore.insert(state::DYNAMIC_MIN_GAS_PRICE, mgp); + }); + } } impl module::InvariantHandler for Module {} diff --git a/runtime-sdk/src/modules/core/test.rs b/runtime-sdk/src/modules/core/test.rs index 292e8f0724..f3325eaaf1 100644 --- a/runtime-sdk/src/modules/core/test.rs +++ b/runtime-sdk/src/modules/core/test.rs @@ -9,7 +9,8 @@ use crate::{ error::Error, event::IntoTags, handler, - module::{self, Module as _, TransactionHandler as _}, + module::{self, BlockHandler, Module as _, TransactionHandler as _}, + modules::core::min_gas_price_update, runtime::Runtime, sdk_derive, sender::SenderMeta, @@ -38,6 +39,7 @@ fn test_use_gas() { mgp.insert(token::Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }); assert_eq!(Core::max_batch_gas(&mut ctx), BLOCK_MAX_GAS); @@ -135,15 +137,16 @@ fn test_query_min_gas_price() { mgp.insert("SMALLER".parse().unwrap(), 1000); mgp }, + dynamic_min_gas_price: Default::default(), }); assert_eq!( Core::min_gas_price(&mut ctx, &token::Denomination::NATIVE), - 123 + Some(123) ); assert_eq!( Core::min_gas_price(&mut ctx, &"SMALLER".parse().unwrap()), - 1000 + Some(1000) ); let mgp = Core::query_min_gas_price(&mut ctx, ()).expect("query_min_gas_price should succeed"); @@ -170,11 +173,11 @@ fn test_query_min_gas_price() { assert_eq!( super::Module::::min_gas_price(&mut ctx, &token::Denomination::NATIVE), - 123 + Some(123) ); assert_eq!( super::Module::::min_gas_price(&mut ctx, &"SMALLER".parse().unwrap()), - 1000 + Some(1000) ); let mgp = super::Module::::query_min_gas_price(&mut ctx, ()) @@ -346,6 +349,7 @@ impl Runtime for GasWasterRuntime { mgp.insert(token::Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }, }, (), @@ -782,6 +786,7 @@ fn test_approve_unverified_tx() { mgp.insert(token::Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }); let dummy_bytes = b"you look, you die".to_vec(); Core::approve_unverified_tx( @@ -887,6 +892,7 @@ fn test_min_gas_price() { mgp.insert("SMALLER".parse().unwrap(), 100); mgp }, + dynamic_min_gas_price: Default::default(), }); let mut tx = transaction::Transaction { @@ -1059,6 +1065,7 @@ fn test_gas_used_events() { mgp.insert(token::Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }); let mut tx = mock::transaction(); @@ -1161,3 +1168,146 @@ fn test_module_info() { } ); } + +#[test] +fn test_min_gas_price_update() { + let cases: Vec<(u128, u128, u128, u128, u128)> = vec![ + // (gas_used, target_gas_used, price_max_change_denominator, price, expected_price) + // General cases. + (50, 50, 1, 100, 100), // No change. + (100, 50, 1, 100, 200), // Increase. + (50, 100, 1, 100, 50), // Decrease. + (0, 50, 1, 100, 0), // Decrease. + // Non base price_max_change_denominator. + (100, 50, 2, 100, 150), // Increase by 50. + (100, 50, 8, 100, 112), // Increase by 12. + (50, 100, 2, 100, 75), // Decrease by 25. + (50, 100, 8, 100, 94), // Decrease by 6. + (0, 100, 2, 100, 50), // Decrease by 50. + (0, 100, 8, 100, 88), // Decrease by 12. + (0, u64::MAX as u128, 1, 100, 0), // Decrease by 100%. + // Invalid configurations (should be handled gracefully) + (100, 100, 0, 100, 100), // price_max_change_denominator == 0. + (100, 0, 1, 100, 100), // target_gas_used == 0 + (1000, 100, 1, 0, 0), // price == 0. + (u128::MAX, u128::MAX, 1, u128::MAX, u128::MAX), // Overflow. + (0, u128::MAX, 1, 100, 99), // Overflow (target_gas_used). + (0, u128::MAX / 2, 1, 100, 98), // Overflow. (target_gas_used). + ]; + for (i, (gas_used, target_gas, max_change_denominator, price, expected_price)) in + cases.into_iter().enumerate() + { + let new_price = min_gas_price_update(gas_used, target_gas, max_change_denominator, price); + assert_eq!( + new_price, expected_price, + "dynamic price should match expected price (test case: {:?})", + i + ); + } +} + +#[test] +fn test_dynamic_min_gas_price() { + let mut mock = mock::Mock::default(); + let mut ctx = mock.create_ctx_for_runtime::(Mode::CheckTx, false); + + let denom: token::Denomination = "SMALLER".parse().unwrap(); + Core::set_params(Parameters { + max_batch_gas: 10_000, + max_tx_size: 32 * 1024, + max_tx_signers: 8, + max_multisig_signers: 8, + gas_costs: super::GasCosts { + tx_byte: 0, + auth_signature: GasWasterRuntime::AUTH_SIGNATURE_GAS, + auth_multisig_signer: GasWasterRuntime::AUTH_MULTISIG_GAS, + callformat_x25519_deoxysii: 0, + }, + min_gas_price: { + let mut mgp = BTreeMap::new(); + mgp.insert(token::Denomination::NATIVE, 1000); + mgp.insert(denom.clone(), 100); + mgp + }, + dynamic_min_gas_price: super::DynamicMinGasPrice { + enabled: true, + target_block_gas_usage_percentage: 50, + min_price_max_change_denominator: 8, + }, + }); + + let tx = transaction::Transaction { + version: 1, + call: transaction::Call { + format: transaction::CallFormat::Plain, + method: GasWasterModule::METHOD_WASTE_GAS.to_owned(), + ..Default::default() + }, + auth_info: transaction::AuthInfo { + signer_info: vec![ + transaction::SignerInfo::new_sigspec(keys::alice::sigspec(), 0), + transaction::SignerInfo::new_multisig( + multisig::Config { + signers: vec![multisig::Signer { + public_key: keys::bob::pk(), + weight: 1, + }], + threshold: 1, + }, + 0, + ), + ], + fee: transaction::Fee { + amount: token::BaseUnits::new(1_000_000_000, token::Denomination::NATIVE), + gas: 10_000, + consensus_messages: 0, + }, + ..Default::default() + }, + }; + assert_eq!( + Core::min_gas_price(&mut ctx, &token::Denomination::NATIVE), + Some(1000) + ); + assert_eq!(Core::min_gas_price(&mut ctx, &denom), Some(100)); + + // Simulate some full blocks (with max gas usage). + for round in 0..=10 { + mock.runtime_header.round = round; + + let mut ctx = mock.create_ctx(); + Core::begin_block(&mut ctx); + + for _ in 0..909 { + // Each tx uses 11 gas, this makes it 9999/10_000 block gas used. + ctx.with_tx(tx.clone().into(), |mut tx_ctx, call| { + Core::before_handle_call(&mut tx_ctx, &call).expect("gas price should be ok"); + }); + } + + Core::end_block(&mut ctx); + } + + let mut ctx = mock.create_ctx(); + assert_eq!( + Core::min_gas_price(&mut ctx, &token::Denomination::NATIVE), + Some(3598) // Gas price should increase. + ); + assert_eq!(Core::min_gas_price(&mut ctx, &denom), Some(350)); + + // Simulate some empty blocks. + for round in 10..=100 { + mock.runtime_header.round = round; + + let mut ctx = mock.create_ctx(); + Core::begin_block(&mut ctx); + Core::end_block(&mut ctx); + } + + let mut ctx = mock.create_ctx(); + assert_eq!( + Core::min_gas_price(&mut ctx, &token::Denomination::NATIVE), + Some(1000) // Gas price should decrease to the configured min gas price. + ); + assert_eq!(Core::min_gas_price(&mut ctx, &denom), Some(100)); +} diff --git a/tests/runtimes/benchmarking/src/lib.rs b/tests/runtimes/benchmarking/src/lib.rs index 14d49a02fd..9103717ed9 100644 --- a/tests/runtimes/benchmarking/src/lib.rs +++ b/tests/runtimes/benchmarking/src/lib.rs @@ -59,6 +59,7 @@ impl sdk::Runtime for Runtime { mgp.insert(Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }, }, modules::accounts::Genesis { diff --git a/tests/runtimes/simple-consensus/src/lib.rs b/tests/runtimes/simple-consensus/src/lib.rs index 592d3aed6a..a695619584 100644 --- a/tests/runtimes/simple-consensus/src/lib.rs +++ b/tests/runtimes/simple-consensus/src/lib.rs @@ -76,6 +76,7 @@ impl sdk::Runtime for Runtime { mgp.insert(Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }, }, ) diff --git a/tests/runtimes/simple-contracts/src/lib.rs b/tests/runtimes/simple-contracts/src/lib.rs index 62e9e8bb35..f258db5c8c 100644 --- a/tests/runtimes/simple-contracts/src/lib.rs +++ b/tests/runtimes/simple-contracts/src/lib.rs @@ -85,6 +85,7 @@ impl sdk::Runtime for Runtime { mgp.insert(Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }, }, contracts::Genesis { diff --git a/tests/runtimes/simple-evm/src/lib.rs b/tests/runtimes/simple-evm/src/lib.rs index f8f96934f2..a9d0348c0b 100644 --- a/tests/runtimes/simple-evm/src/lib.rs +++ b/tests/runtimes/simple-evm/src/lib.rs @@ -110,6 +110,7 @@ impl sdk::Runtime for Runtime { mgp.insert(Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }, }, evm::Genesis { diff --git a/tests/runtimes/simple-keyvalue/src/lib.rs b/tests/runtimes/simple-keyvalue/src/lib.rs index 142be5cdb6..ddd2ea7750 100644 --- a/tests/runtimes/simple-keyvalue/src/lib.rs +++ b/tests/runtimes/simple-keyvalue/src/lib.rs @@ -140,6 +140,7 @@ impl sdk::Runtime for Runtime { mgp.insert(Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }, }, ) diff --git a/tests/runtimes/simple-keyvalue/src/test.rs b/tests/runtimes/simple-keyvalue/src/test.rs index 062866102a..51bd2a374a 100644 --- a/tests/runtimes/simple-keyvalue/src/test.rs +++ b/tests/runtimes/simple-keyvalue/src/test.rs @@ -24,6 +24,7 @@ fn test_impl_for_tuple() { mgp.insert(token::Denomination::NATIVE, 0); mgp }, + dynamic_min_gas_price: Default::default(), }); let dummy_bytes = b"you look, you die".to_vec();