From 37291a18027c5cfd530a708a2cb3ad7dcabdb11f Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Tue, 25 Jun 2024 14:42:23 +0400 Subject: [PATCH 1/4] feat(gas_adjuster): Use eth_feeHistory for both base fee and blobs --- core/lib/basic_types/src/web3/mod.rs | 16 +- core/lib/eth_client/src/clients/http/query.rs | 27 +++- core/lib/eth_client/src/clients/mock.rs | 78 +++++----- core/lib/eth_client/src/lib.rs | 9 +- .../api_server/src/web3/namespaces/eth.rs | 2 + core/node/eth_sender/src/tests.rs | 21 ++- .../src/l1_gas_price/gas_adjuster/mod.rs | 126 +++++----------- .../src/l1_gas_price/gas_adjuster/tests.rs | 137 +++++++++--------- core/node/state_keeper/src/io/tests/tester.rs | 15 +- 9 files changed, 219 insertions(+), 212 deletions(-) diff --git a/core/lib/basic_types/src/web3/mod.rs b/core/lib/basic_types/src/web3/mod.rs index af9cd1eea3fc..cfeeaa533b36 100644 --- a/core/lib/basic_types/src/web3/mod.rs +++ b/core/lib/basic_types/src/web3/mod.rs @@ -827,6 +827,7 @@ pub enum TransactionCondition { } // `FeeHistory`: from `web3::types::fee_history` +// Adapted to support blobs. /// The fee history type returned from `eth_feeHistory` call. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] @@ -834,14 +835,25 @@ pub enum TransactionCondition { pub struct FeeHistory { /// Lowest number block of the returned range. pub oldest_block: BlockNumber, - /// A vector of block base fees per gas. This includes the next block after the newest of the returned range, because this value can be derived from the newest block. Zeroes are returned for pre-EIP-1559 blocks. + /// A vector of block base fees per gas. This includes the next block after the newest of the returned range, + /// because this value can be derived from the newest block. Zeroes are returned for pre-EIP-1559 blocks. #[serde(default)] // some node implementations skip empty lists pub base_fee_per_gas: Vec, /// A vector of block gas used ratios. These are calculated as the ratio of gas used and gas limit. #[serde(default)] // some node implementations skip empty lists pub gas_used_ratio: Vec, - /// A vector of effective priority fee per gas data points from a single block. All zeroes are returned if the block is empty. Returned only if requested. + /// A vector of effective priority fee per gas data points from a single block. All zeroes are returned if + /// the block is empty. Returned only if requested. pub reward: Option>>, + /// An array of base fees per blob gas for blocks. This includes the next block following the newest in the + /// returned range, as this value can be derived from the latest block. For blocks before EIP-4844, zeroes + /// are returned. + #[serde(default)] // some node implementations skip empty lists + pub base_fee_per_blob_gas: Vec, + /// An array showing the ratios of blob gas used in blocks. These ratios are calculated by dividing blobGasUsed + /// by the maximum blob gas per block. + #[serde(default)] // some node implementations skip empty lists + pub blob_gas_used_ratio: Vec, } // `SyncInfo`, `SyncState`: from `web3::types::sync_state` diff --git a/core/lib/eth_client/src/clients/http/query.rs b/core/lib/eth_client/src/clients/http/query.rs index 33d9838dc735..b421ce6d197e 100644 --- a/core/lib/eth_client/src/clients/http/query.rs +++ b/core/lib/eth_client/src/clients/http/query.rs @@ -8,7 +8,7 @@ use zksync_web3_decl::error::{ClientRpcContext, EnrichedClientError, EnrichedCli use super::{decl::L1EthNamespaceClient, Method, COUNTERS, LATENCIES}; use crate::{ types::{ExecutedTxStatus, FailureInfo}, - EthInterface, RawTransactionBytes, + BaseFees, EthInterface, RawTransactionBytes, }; #[async_trait] @@ -78,7 +78,7 @@ where &self, upto_block: usize, block_count: usize, - ) -> EnrichedClientResult> { + ) -> EnrichedClientResult> { const MAX_REQUEST_CHUNK: usize = 1024; COUNTERS.call[&(Method::BaseFeeHistory, self.component())].inc(); @@ -86,6 +86,14 @@ where let mut history = Vec::with_capacity(block_count); let from_block = upto_block.saturating_sub(block_count); + // Non-panicking conversion to u64. + fn cast_to_u64(value: U256, tag: &str) -> EnrichedClientResult { + u64::try_from(value).map_err(|_| { + let err = ClientError::Custom(format!("{tag} value does not fit in u64")); + EnrichedClientError::new(err, "cast_to_u64").with_arg("value", &value) + }) + } + // Here we are requesting `fee_history` from blocks // `(from_block; upto_block)` in chunks of size `MAX_REQUEST_CHUNK` // starting from the oldest block. @@ -103,11 +111,22 @@ where .with_arg("chunk_size", &chunk_size) .with_arg("block", &chunk_end) .await?; - history.extend(fee_history.base_fee_per_gas); + + for (base, blob) in fee_history + .base_fee_per_gas + .into_iter() + .zip(fee_history.base_fee_per_blob_gas) + { + let fees = BaseFees { + base_fee_per_gas: cast_to_u64(base, "base_fee_per_gas")?, + base_fee_per_blob_gas: blob, + }; + history.push(fees) + } } latency.observe(); - Ok(history.into_iter().map(|fee| fee.as_u64()).collect()) + Ok(history) } async fn get_pending_block_base_fee_per_gas(&self) -> EnrichedClientResult { diff --git a/core/lib/eth_client/src/clients/mock.rs b/core/lib/eth_client/src/clients/mock.rs index 03162c2cfeb4..bd52b6e65875 100644 --- a/core/lib/eth_client/src/clients/mock.rs +++ b/core/lib/eth_client/src/clients/mock.rs @@ -14,7 +14,7 @@ use zksync_web3_decl::client::{DynClient, MockClient, L1}; use crate::{ types::{ContractCallError, SignedCallResult, SigningError}, - BoundEthInterface, Options, RawTransactionBytes, + BaseFees, BoundEthInterface, Options, RawTransactionBytes, }; #[derive(Debug, Clone)] @@ -212,8 +212,7 @@ type CallHandler = pub struct MockEthereumBuilder { max_fee_per_gas: U256, max_priority_fee_per_gas: U256, - base_fee_history: Vec, - excess_blob_gas_history: Vec, + base_fee_history: Vec, /// If true, the mock will not check the ordering nonces of the transactions. /// This is useful for testing the cases when the transactions are executed out of order. non_ordering_confirmations: bool, @@ -228,7 +227,6 @@ impl fmt::Debug for MockEthereumBuilder { .field("max_fee_per_gas", &self.max_fee_per_gas) .field("max_priority_fee_per_gas", &self.max_priority_fee_per_gas) .field("base_fee_history", &self.base_fee_history) - .field("excess_blob_gas_history", &self.excess_blob_gas_history) .field( "non_ordering_confirmations", &self.non_ordering_confirmations, @@ -244,7 +242,6 @@ impl Default for MockEthereumBuilder { max_fee_per_gas: 100.into(), max_priority_fee_per_gas: 10.into(), base_fee_history: vec![], - excess_blob_gas_history: vec![], non_ordering_confirmations: false, inner: Arc::default(), call_handler: Box::new(|call, block_id| { @@ -256,21 +253,13 @@ impl Default for MockEthereumBuilder { impl MockEthereumBuilder { /// Sets fee history for each block in the mocked Ethereum network, starting from the 0th block. - pub fn with_fee_history(self, history: Vec) -> Self { + pub fn with_fee_history(self, history: Vec) -> Self { Self { base_fee_history: history, ..self } } - /// Sets the excess blob gas history for each block in the mocked Ethereum network, starting from the 0th block. - pub fn with_excess_blob_gas_history(self, history: Vec) -> Self { - Self { - excess_blob_gas_history: history, - ..self - } - } - pub fn with_non_ordering_confirmation(self, non_ordering_confirmations: bool) -> Self { Self { non_ordering_confirmations, @@ -306,19 +295,16 @@ impl MockEthereumBuilder { } fn get_block_by_number( - base_fee_history: &[u64], - excess_blob_gas_history: &[u64], + fee_history: &[BaseFees], block: web3::BlockNumber, ) -> Option> { let web3::BlockNumber::Number(number) = block else { panic!("Non-numeric block requested"); }; - let excess_blob_gas = excess_blob_gas_history - .get(number.as_usize()) - .map(|excess_blob_gas| (*excess_blob_gas).into()); - let base_fee_per_gas = base_fee_history + let excess_blob_gas = Some(0.into()); // Not relevant for tests. + let base_fee_per_gas = fee_history .get(number.as_usize()) - .map(|base_fee| (*base_fee).into()); + .map(|fees| fees.base_fee_per_gas.into()); Some(web3::Block { number: Some(number), @@ -341,18 +327,12 @@ impl MockEthereumBuilder { move || Ok(U64::from(inner.read().unwrap().block_number)) }) .method("eth_getBlockByNumber", { - let base_fee_history = self.base_fee_history; - let excess_blob_gas_history = self.excess_blob_gas_history; move |number, full_transactions: bool| { assert!( !full_transactions, "getting blocks with transactions is not mocked" ); - Ok(Self::get_block_by_number( - &base_fee_history, - &excess_blob_gas_history, - number, - )) + Ok(Self::get_block_by_number(&self.base_fee_history, number)) } }) .method("eth_getTransactionCount", { @@ -374,10 +354,14 @@ impl MockEthereumBuilder { oldest_block: start_block.into(), base_fee_per_gas: base_fee_history[start_block..=from_block] .iter() - .copied() - .map(U256::from) + .map(|fee| U256::from(fee.base_fee_per_gas)) .collect(), - gas_used_ratio: vec![], // not used + base_fee_per_blob_gas: base_fee_history[start_block..=from_block] + .iter() + .map(|fee| fee.base_fee_per_blob_gas) + .collect(), + gas_used_ratio: vec![], // not used + blob_gas_used_ratio: vec![], // not used reward: None, }) }, @@ -591,10 +575,25 @@ mod tests { use super::*; use crate::{CallFunctionArgs, EthInterface}; + macro_rules! base_fees { + ($block: expr, $blob: expr) => { + BaseFees { + base_fee_per_gas: $block, + base_fee_per_blob_gas: U256::from($blob), + } + }; + } + #[tokio::test] async fn managing_block_number() { let mock = MockEthereum::builder() - .with_fee_history(vec![0, 1, 2, 3, 4]) + .with_fee_history(vec![ + base_fees!(0, 4), + base_fees!(1, 3), + base_fees!(2, 2), + base_fees!(3, 1), + base_fees!(4, 0), + ]) .build(); let block_number = mock.client.block_number().await.unwrap(); assert_eq!(block_number, 0.into()); @@ -625,17 +624,24 @@ mod tests { #[tokio::test] async fn managing_fee_history() { + let initial_fee_history = vec![ + base_fees!(1, 4), + base_fees!(2, 3), + base_fees!(3, 2), + base_fees!(4, 1), + base_fees!(5, 0), + ]; let client = MockEthereum::builder() - .with_fee_history(vec![1, 2, 3, 4, 5]) + .with_fee_history(initial_fee_history.clone()) .build(); client.advance_block_number(4); let fee_history = client.as_ref().base_fee_history(4, 4).await.unwrap(); - assert_eq!(fee_history, [2, 3, 4, 5]); + assert_eq!(fee_history, &initial_fee_history[1..=4]); let fee_history = client.as_ref().base_fee_history(2, 2).await.unwrap(); - assert_eq!(fee_history, [2, 3]); + assert_eq!(fee_history, &initial_fee_history[1..=2]); let fee_history = client.as_ref().base_fee_history(3, 2).await.unwrap(); - assert_eq!(fee_history, [3, 4]); + assert_eq!(fee_history, &initial_fee_history[2..=3]); } #[tokio::test] diff --git a/core/lib/eth_client/src/lib.rs b/core/lib/eth_client/src/lib.rs index 6e24047dd48c..b6ac3a89b54f 100644 --- a/core/lib/eth_client/src/lib.rs +++ b/core/lib/eth_client/src/lib.rs @@ -65,6 +65,13 @@ impl Options { } } +/// Information about the base fees provided by the L1 client. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct BaseFees { + pub base_fee_per_gas: u64, + pub base_fee_per_blob_gas: U256, +} + /// Common Web3 interface, as seen by the core applications. /// Encapsulates the raw Web3 interaction, providing a high-level interface. Acts as an extension /// trait implemented for L1 / Ethereum [clients](zksync_web3_decl::client::Client). @@ -96,7 +103,7 @@ pub trait EthInterface: Sync + Send { &self, from_block: usize, block_count: usize, - ) -> EnrichedClientResult>; + ) -> EnrichedClientResult>; /// Returns the `base_fee_per_gas` value for the currently pending L1 block. async fn get_pending_block_base_fee_per_gas(&self) -> EnrichedClientResult; diff --git a/core/node/api_server/src/web3/namespaces/eth.rs b/core/node/api_server/src/web3/namespaces/eth.rs index 397ce77c050f..6096855e308c 100644 --- a/core/node/api_server/src/web3/namespaces/eth.rs +++ b/core/node/api_server/src/web3/namespaces/eth.rs @@ -695,6 +695,8 @@ impl EthNamespace { base_fee_per_gas, gas_used_ratio, reward, + base_fee_per_blob_gas: vec![], + blob_gas_used_ratio: vec![], }) } diff --git a/core/node/eth_sender/src/tests.rs b/core/node/eth_sender/src/tests.rs index a3bb9951f44a..4853c7bb2299 100644 --- a/core/node/eth_sender/src/tests.rs +++ b/core/node/eth_sender/src/tests.rs @@ -9,7 +9,7 @@ use zksync_config::{ }; use zksync_contracts::BaseSystemContractsHashes; use zksync_dal::{Connection, ConnectionPool, Core, CoreDal}; -use zksync_eth_client::clients::MockEthereum; +use zksync_eth_client::{clients::MockEthereum, BaseFees}; use zksync_l1_contract_interface::i_executor::methods::{ExecuteBatches, ProveBatches}; use zksync_node_fee_model::l1_gas_price::GasAdjuster; use zksync_node_test_utils::{create_l1_batch, l1_batch_metadata_to_commitment_artifacts}; @@ -130,12 +130,23 @@ impl EthSenderTester { ..eth_sender_config.clone().sender.unwrap() }; + let history: Vec<_> = history + .into_iter() + .map(|base_fee_per_gas| BaseFees { + base_fee_per_gas, + base_fee_per_blob_gas: 0.into(), + }) + .collect(); + let gateway = MockEthereum::builder() .with_fee_history( - std::iter::repeat(0) - .take(Self::WAIT_CONFIRMATIONS as usize) - .chain(history) - .collect(), + std::iter::repeat_with(|| BaseFees { + base_fee_per_gas: 0, + base_fee_per_blob_gas: 0.into(), + }) + .take(Self::WAIT_CONFIRMATIONS as usize) + .chain(history) + .collect(), ) .with_non_ordering_confirmation(non_ordering_confirmations) .with_call_handler(move |call, _| { diff --git a/core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs b/core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs index 34cbee9b09e5..a3a1ed78e5b7 100644 --- a/core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs +++ b/core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs @@ -2,14 +2,13 @@ use std::{ collections::VecDeque, - ops::RangeInclusive, sync::{Arc, RwLock}, }; use tokio::sync::watch; use zksync_config::{configs::eth_sender::PubdataSendingMode, GasAdjusterConfig}; use zksync_eth_client::EthInterface; -use zksync_types::{commitment::L1BatchCommitmentMode, L1_GAS_PER_PUBDATA_BYTE, U256, U64}; +use zksync_types::{commitment::L1BatchCommitmentMode, L1_GAS_PER_PUBDATA_BYTE, U256}; use zksync_web3_decl::client::{DynClient, L1}; use self::metrics::METRICS; @@ -52,26 +51,25 @@ impl GasAdjuster { .await? .as_usize() .saturating_sub(1); - let base_fee_history = eth_client + let fee_history = eth_client .base_fee_history(current_block, config.max_base_fee_samples) .await?; - // Web3 API doesn't provide a method to fetch blob fees for multiple blocks using single request, - // so we request blob base fee only for the latest block. - let (_, last_block_blob_base_fee) = - Self::get_base_fees_history(eth_client.as_ref(), current_block..=current_block).await?; + let base_fee_statistics = GasStatistics::new( + config.max_base_fee_samples, + current_block, + fee_history.iter().map(|fee| fee.base_fee_per_gas), + ); + + let blob_base_fee_statistics = GasStatistics::new( + config.num_samples_for_blob_base_fee_estimate, + current_block, + fee_history.iter().map(|fee| fee.base_fee_per_blob_gas), + ); Ok(Self { - base_fee_statistics: GasStatistics::new( - config.max_base_fee_samples, - current_block, - &base_fee_history, - ), - blob_base_fee_statistics: GasStatistics::new( - config.num_samples_for_blob_base_fee_estimate, - current_block, - &last_block_blob_base_fee, - ), + base_fee_statistics, + blob_base_fee_statistics, config, pubdata_sending_mode, eth_client, @@ -95,25 +93,29 @@ impl GasAdjuster { let last_processed_block = self.base_fee_statistics.last_processed_block(); if current_block > last_processed_block { - let (base_fee_history, blob_base_fee_history) = Self::get_base_fees_history( - self.eth_client.as_ref(), - (last_processed_block + 1)..=current_block, - ) - .await?; + let n_blocks = current_block - last_processed_block; + let base_fees = self + .eth_client + .base_fee_history(current_block, n_blocks) + .await?; // We shouldn't rely on L1 provider to return consistent results, so we check that we have at least one new sample. - if let Some(current_base_fee_per_gas) = base_fee_history.last() { + if let Some(current_base_fee_per_gas) = base_fees.last().map(|fee| fee.base_fee_per_gas) + { METRICS .current_base_fee_per_gas - .set(*current_base_fee_per_gas); + .set(current_base_fee_per_gas); } - self.base_fee_statistics.add_samples(&base_fee_history); + self.base_fee_statistics + .add_samples(base_fees.iter().map(|fee| fee.base_fee_per_gas)); - if let Some(current_blob_base_fee) = blob_base_fee_history.last() { + if let Some(current_blob_base_fee) = + base_fees.last().map(|fee| fee.base_fee_per_blob_gas) + { // Blob base fee overflows `u64` only in very extreme cases. // It doesn't worth to observe exact value with metric because anyway values that can be used // are capped by `self.config.max_blob_base_fee()` of `u64` type. - if current_blob_base_fee > &U256::from(u64::MAX) { + if current_blob_base_fee > U256::from(u64::MAX) { tracing::error!("Failed to report current_blob_base_fee = {current_blob_base_fee}, it exceeds u64::MAX"); } else { METRICS @@ -122,7 +124,7 @@ impl GasAdjuster { } } self.blob_base_fee_statistics - .add_samples(&blob_base_fee_history); + .add_samples(base_fees.iter().map(|fee| fee.base_fee_per_blob_gas)); } Ok(()) } @@ -223,62 +225,6 @@ impl GasAdjuster { } } } - - /// Returns vector of base fees and blob base fees for given block range. - /// Note, that data for pre-dencun blocks won't be included in the vector returned. - async fn get_base_fees_history( - eth_client: &DynClient, - block_range: RangeInclusive, - ) -> anyhow::Result<(Vec, Vec)> { - let mut base_fee_history = Vec::new(); - let mut blob_base_fee_history = Vec::new(); - for block_number in block_range { - let header = eth_client.block(U64::from(block_number).into()).await?; - if let Some(base_fee_per_gas) = - header.as_ref().and_then(|header| header.base_fee_per_gas) - { - base_fee_history.push(base_fee_per_gas.as_u64()) - } - - if let Some(excess_blob_gas) = header.as_ref().and_then(|header| header.excess_blob_gas) - { - blob_base_fee_history.push(Self::blob_base_fee(excess_blob_gas.as_u64())) - } - } - - Ok((base_fee_history, blob_base_fee_history)) - } - - /// Calculates `blob_base_fee` given `excess_blob_gas`. - fn blob_base_fee(excess_blob_gas: u64) -> U256 { - // Constants and formula are taken from EIP4844 specification. - const MIN_BLOB_BASE_FEE: u32 = 1; - const BLOB_BASE_FEE_UPDATE_FRACTION: u32 = 3338477; - - Self::fake_exponential( - MIN_BLOB_BASE_FEE.into(), - excess_blob_gas.into(), - BLOB_BASE_FEE_UPDATE_FRACTION.into(), - ) - } - - /// approximates `factor * e ** (numerator / denominator)` using Taylor expansion. - fn fake_exponential(factor: U256, numerator: U256, denominator: U256) -> U256 { - let mut i = 1_u32; - let mut output = U256::zero(); - let mut accum = factor * denominator; - while !accum.is_zero() { - output += accum; - - accum *= numerator; - accum /= denominator; - accum /= U256::from(i); - - i += 1; - } - - output / denominator - } } impl L1TxParamsProvider for GasAdjuster { @@ -363,7 +309,7 @@ pub(super) struct GasStatisticsInner { } impl GasStatisticsInner { - fn new(max_samples: usize, block: usize, fee_history: &[T]) -> Self { + fn new(max_samples: usize, block: usize, fee_history: impl IntoIterator) -> Self { let mut statistics = Self { max_samples, samples: VecDeque::with_capacity(max_samples), @@ -387,9 +333,11 @@ impl GasStatisticsInner { self.samples.back().copied().unwrap_or(self.median_cached) } - fn add_samples(&mut self, fees: &[T]) { + fn add_samples(&mut self, fees: impl IntoIterator) { + let old_len = self.samples.len(); self.samples.extend(fees); - self.last_processed_block += fees.len(); + let processed_blocks = self.samples.len() - old_len; + self.last_processed_block += processed_blocks; let extra = self.samples.len().saturating_sub(self.max_samples); self.samples.drain(..extra); @@ -407,7 +355,7 @@ impl GasStatisticsInner { pub(super) struct GasStatistics(RwLock>); impl GasStatistics { - pub fn new(max_samples: usize, block: usize, fee_history: &[T]) -> Self { + pub fn new(max_samples: usize, block: usize, fee_history: impl IntoIterator) -> Self { Self(RwLock::new(GasStatisticsInner::new( max_samples, block, @@ -423,7 +371,7 @@ impl GasStatistics { self.0.read().unwrap().last_added_value() } - pub fn add_samples(&self, fees: &[T]) { + pub fn add_samples(&self, fees: impl IntoIterator) { self.0.write().unwrap().add_samples(fees) } diff --git a/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs b/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs index 594efc6915e2..ce8731230c3f 100644 --- a/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs +++ b/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs @@ -2,7 +2,7 @@ use std::collections::VecDeque; use test_casing::test_casing; use zksync_config::{configs::eth_sender::PubdataSendingMode, GasAdjusterConfig}; -use zksync_eth_client::clients::MockEthereum; +use zksync_eth_client::{clients::MockEthereum, BaseFees}; use zksync_types::commitment::L1BatchCommitmentMode; use super::{GasAdjuster, GasStatisticsInner}; @@ -11,19 +11,19 @@ use super::{GasAdjuster, GasStatisticsInner}; #[test] fn median() { // sorted: 4 4 6 7 8 - assert_eq!(GasStatisticsInner::new(5, 5, &[6, 4, 7, 8, 4]).median(), 6); + assert_eq!(GasStatisticsInner::new(5, 5, [6, 4, 7, 8, 4]).median(), 6); // sorted: 4 4 8 10 - assert_eq!(GasStatisticsInner::new(4, 4, &[8, 4, 4, 10]).median(), 8); + assert_eq!(GasStatisticsInner::new(4, 4, [8, 4, 4, 10]).median(), 8); } /// Check that we properly manage the block base fee queue #[test] fn samples_queue() { - let mut stats = GasStatisticsInner::new(5, 5, &[6, 4, 7, 8, 4, 5]); + let mut stats = GasStatisticsInner::new(5, 5, [6, 4, 7, 8, 4, 5]); assert_eq!(stats.samples, VecDeque::from([4, 7, 8, 4, 5])); - stats.add_samples(&[18, 18, 18]); + stats.add_samples([18, 18, 18]); assert_eq!(stats.samples, VecDeque::from([4, 5, 18, 18, 18])); } @@ -32,38 +32,56 @@ fn samples_queue() { #[test_casing(2, [L1BatchCommitmentMode::Rollup, L1BatchCommitmentMode::Validium])] #[tokio::test] async fn kept_updated(commitment_mode: L1BatchCommitmentMode) { - let eth_client = MockEthereum::builder() - .with_fee_history(vec![0, 4, 6, 8, 7, 5, 5, 8, 10, 9]) - .with_excess_blob_gas_history(vec![ - 393216, - 393216 * 2, - 393216, - 393216 * 2, - 393216, - 393216 * 2, - 393216 * 3, - 393216 * 4, - ]) - .build(); + // Helper macro to read a value from adjuster + macro_rules! read { + ($field:expr) => { + $field.0.read().unwrap() + }; + } + + let block_fees = vec![0, 4, 6, 8, 7, 5, 5, 8, 10, 9]; + let blob_fees = vec![ + 0, + 393216, + 393216, + 393216 * 2, + 393216, + 393216 * 2, + 393216 * 2, + 393216 * 3, + 393216 * 4, + 393216, + ]; + let base_fees = block_fees + .into_iter() + .zip(blob_fees) + .map(|(block, blob)| BaseFees { + base_fee_per_gas: block, + base_fee_per_blob_gas: blob.into(), + }) + .collect(); + + let eth_client = MockEthereum::builder().with_fee_history(base_fees).build(); // 5 sampled blocks + additional block to account for latest block subtraction eth_client.advance_block_number(6); + let config = GasAdjusterConfig { + default_priority_fee_per_gas: 5, + max_base_fee_samples: 5, + pricing_formula_parameter_a: 1.5, + pricing_formula_parameter_b: 1.0005, + internal_l1_pricing_multiplier: 0.8, + internal_enforced_l1_gas_price: None, + internal_enforced_pubdata_price: None, + poll_period: 5, + max_l1_gas_price: None, + num_samples_for_blob_base_fee_estimate: 3, + internal_pubdata_pricing_multiplier: 1.0, + max_blob_base_fee: None, + }; let adjuster = GasAdjuster::new( Box::new(eth_client.clone().into_client()), - GasAdjusterConfig { - default_priority_fee_per_gas: 5, - max_base_fee_samples: 5, - pricing_formula_parameter_a: 1.5, - pricing_formula_parameter_b: 1.0005, - internal_l1_pricing_multiplier: 0.8, - internal_enforced_l1_gas_price: None, - internal_enforced_pubdata_price: None, - poll_period: 5, - max_l1_gas_price: None, - num_samples_for_blob_base_fee_estimate: 3, - internal_pubdata_pricing_multiplier: 1.0, - max_blob_base_fee: None, - }, + config, PubdataSendingMode::Calldata, commitment_mode, ) @@ -71,58 +89,35 @@ async fn kept_updated(commitment_mode: L1BatchCommitmentMode) { .unwrap(); assert_eq!( - adjuster.base_fee_statistics.0.read().unwrap().samples.len(), - 5 + read!(adjuster.base_fee_statistics).samples.len(), + config.max_base_fee_samples ); - assert_eq!(adjuster.base_fee_statistics.0.read().unwrap().median(), 6); + assert_eq!(read!(adjuster.base_fee_statistics).median(), 6); - let expected_median_blob_base_fee = GasAdjuster::blob_base_fee(393216); + eprintln!("{:?}", read!(adjuster.blob_base_fee_statistics).samples); + let expected_median_blob_base_fee = 393216 * 2; assert_eq!( - adjuster - .blob_base_fee_statistics - .0 - .read() - .unwrap() - .samples - .len(), - 1 + read!(adjuster.blob_base_fee_statistics).samples.len(), + config.num_samples_for_blob_base_fee_estimate ); assert_eq!( - adjuster.blob_base_fee_statistics.0.read().unwrap().median(), - expected_median_blob_base_fee + read!(adjuster.blob_base_fee_statistics).median(), + expected_median_blob_base_fee.into() ); eth_client.advance_block_number(3); adjuster.keep_updated().await.unwrap(); assert_eq!( - adjuster.base_fee_statistics.0.read().unwrap().samples.len(), - 5 + read!(adjuster.base_fee_statistics).samples.len(), + config.max_base_fee_samples ); - assert_eq!(adjuster.base_fee_statistics.0.read().unwrap().median(), 7); + assert_eq!(read!(adjuster.base_fee_statistics).median(), 7); - let expected_median_blob_base_fee = GasAdjuster::blob_base_fee(393216 * 3); + let expected_median_blob_base_fee = 393216 * 3; + assert_eq!(read!(adjuster.blob_base_fee_statistics).samples.len(), 3); assert_eq!( - adjuster - .blob_base_fee_statistics - .0 - .read() - .unwrap() - .samples - .len(), - 3 + read!(adjuster.blob_base_fee_statistics).median(), + expected_median_blob_base_fee.into() ); - assert_eq!( - adjuster.blob_base_fee_statistics.0.read().unwrap().median(), - expected_median_blob_base_fee - ); -} - -#[test] -fn blob_base_fee_formula() { - const EXCESS_BLOB_GAS: u64 = 0x4b80000; - const EXPECTED_BLOB_BASE_FEE: u64 = 19893400088; - - let blob_base_fee = GasAdjuster::blob_base_fee(EXCESS_BLOB_GAS); - assert_eq!(blob_base_fee.as_u64(), EXPECTED_BLOB_BASE_FEE); } diff --git a/core/node/state_keeper/src/io/tests/tester.rs b/core/node/state_keeper/src/io/tests/tester.rs index 35758c44bc95..0b360a210060 100644 --- a/core/node/state_keeper/src/io/tests/tester.rs +++ b/core/node/state_keeper/src/io/tests/tester.rs @@ -8,7 +8,7 @@ use zksync_config::{ }; use zksync_contracts::BaseSystemContracts; use zksync_dal::{ConnectionPool, Core, CoreDal}; -use zksync_eth_client::clients::MockEthereum; +use zksync_eth_client::{clients::MockEthereum, BaseFees}; use zksync_multivm::vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT; use zksync_node_fee_model::{l1_gas_price::GasAdjuster, MainNodeFeeInputProvider}; use zksync_node_genesis::create_genesis_l1_batch; @@ -47,9 +47,16 @@ impl Tester { } async fn create_gas_adjuster(&self) -> GasAdjuster { - let eth_client = MockEthereum::builder() - .with_fee_history(vec![0, 4, 6, 8, 7, 5, 5, 8, 10, 9]) - .build(); + let block_fees = vec![0, 4, 6, 8, 7, 5, 5, 8, 10, 9]; + let base_fees = block_fees + .into_iter() + .zip(std::iter::repeat(1)) + .map(|(block, blob)| BaseFees { + base_fee_per_gas: block, + base_fee_per_blob_gas: blob.into(), + }) + .collect(); + let eth_client = MockEthereum::builder().with_fee_history(base_fees).build(); let gas_adjuster_config = GasAdjusterConfig { default_priority_fee_per_gas: 10, From 03874714901e305af0469d649045ef94e1f4561c Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Wed, 26 Jun 2024 15:22:28 +0400 Subject: [PATCH 2/4] Fix nitpicks --- core/lib/eth_client/src/clients/http/query.rs | 14 ++++---- core/lib/eth_client/src/clients/mock.rs | 32 +++++++++---------- .../src/l1_gas_price/gas_adjuster/tests.rs | 30 ++++++++--------- core/node/state_keeper/src/io/tests/tester.rs | 7 ++-- 4 files changed, 39 insertions(+), 44 deletions(-) diff --git a/core/lib/eth_client/src/clients/http/query.rs b/core/lib/eth_client/src/clients/http/query.rs index b421ce6d197e..dbd7e68d7779 100644 --- a/core/lib/eth_client/src/clients/http/query.rs +++ b/core/lib/eth_client/src/clients/http/query.rs @@ -79,13 +79,6 @@ where upto_block: usize, block_count: usize, ) -> EnrichedClientResult> { - const MAX_REQUEST_CHUNK: usize = 1024; - - COUNTERS.call[&(Method::BaseFeeHistory, self.component())].inc(); - let latency = LATENCIES.direct[&Method::BaseFeeHistory].start(); - let mut history = Vec::with_capacity(block_count); - let from_block = upto_block.saturating_sub(block_count); - // Non-panicking conversion to u64. fn cast_to_u64(value: U256, tag: &str) -> EnrichedClientResult { u64::try_from(value).map_err(|_| { @@ -94,6 +87,13 @@ where }) } + const MAX_REQUEST_CHUNK: usize = 1024; + + COUNTERS.call[&(Method::BaseFeeHistory, self.component())].inc(); + let latency = LATENCIES.direct[&Method::BaseFeeHistory].start(); + let mut history = Vec::with_capacity(block_count); + let from_block = upto_block.saturating_sub(block_count); + // Here we are requesting `fee_history` from blocks // `(from_block; upto_block)` in chunks of size `MAX_REQUEST_CHUNK` // starting from the oldest block. diff --git a/core/lib/eth_client/src/clients/mock.rs b/core/lib/eth_client/src/clients/mock.rs index bd52b6e65875..9fbc5ceb4b2e 100644 --- a/core/lib/eth_client/src/clients/mock.rs +++ b/core/lib/eth_client/src/clients/mock.rs @@ -575,24 +575,22 @@ mod tests { use super::*; use crate::{CallFunctionArgs, EthInterface}; - macro_rules! base_fees { - ($block: expr, $blob: expr) => { - BaseFees { - base_fee_per_gas: $block, - base_fee_per_blob_gas: U256::from($blob), - } - }; + fn base_fees(block: u64, blob: u64) -> BaseFees { + BaseFees { + base_fee_per_gas: block, + base_fee_per_blob_gas: U256::from(blob), + } } #[tokio::test] async fn managing_block_number() { let mock = MockEthereum::builder() .with_fee_history(vec![ - base_fees!(0, 4), - base_fees!(1, 3), - base_fees!(2, 2), - base_fees!(3, 1), - base_fees!(4, 0), + base_fees(0, 4), + base_fees(1, 3), + base_fees(2, 2), + base_fees(3, 1), + base_fees(4, 0), ]) .build(); let block_number = mock.client.block_number().await.unwrap(); @@ -625,11 +623,11 @@ mod tests { #[tokio::test] async fn managing_fee_history() { let initial_fee_history = vec![ - base_fees!(1, 4), - base_fees!(2, 3), - base_fees!(3, 2), - base_fees!(4, 1), - base_fees!(5, 0), + base_fees(1, 4), + base_fees(2, 3), + base_fees(3, 2), + base_fees(4, 1), + base_fees(5, 0), ]; let client = MockEthereum::builder() .with_fee_history(initial_fee_history.clone()) diff --git a/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs b/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs index ce8731230c3f..200903b6deda 100644 --- a/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs +++ b/core/node/fee_model/src/l1_gas_price/gas_adjuster/tests.rs @@ -1,11 +1,11 @@ -use std::collections::VecDeque; +use std::{collections::VecDeque, sync::RwLockReadGuard}; use test_casing::test_casing; use zksync_config::{configs::eth_sender::PubdataSendingMode, GasAdjusterConfig}; use zksync_eth_client::{clients::MockEthereum, BaseFees}; use zksync_types::commitment::L1BatchCommitmentMode; -use super::{GasAdjuster, GasStatisticsInner}; +use super::{GasAdjuster, GasStatistics, GasStatisticsInner}; /// Check that we compute the median correctly #[test] @@ -32,11 +32,9 @@ fn samples_queue() { #[test_casing(2, [L1BatchCommitmentMode::Rollup, L1BatchCommitmentMode::Validium])] #[tokio::test] async fn kept_updated(commitment_mode: L1BatchCommitmentMode) { - // Helper macro to read a value from adjuster - macro_rules! read { - ($field:expr) => { - $field.0.read().unwrap() - }; + // Helper function to read a value from adjuster + fn read(statistics: &GasStatistics) -> RwLockReadGuard> { + statistics.0.read().unwrap() } let block_fees = vec![0, 4, 6, 8, 7, 5, 5, 8, 10, 9]; @@ -89,19 +87,19 @@ async fn kept_updated(commitment_mode: L1BatchCommitmentMode) { .unwrap(); assert_eq!( - read!(adjuster.base_fee_statistics).samples.len(), + read(&adjuster.base_fee_statistics).samples.len(), config.max_base_fee_samples ); - assert_eq!(read!(adjuster.base_fee_statistics).median(), 6); + assert_eq!(read(&adjuster.base_fee_statistics).median(), 6); - eprintln!("{:?}", read!(adjuster.blob_base_fee_statistics).samples); + eprintln!("{:?}", read(&adjuster.blob_base_fee_statistics).samples); let expected_median_blob_base_fee = 393216 * 2; assert_eq!( - read!(adjuster.blob_base_fee_statistics).samples.len(), + read(&adjuster.blob_base_fee_statistics).samples.len(), config.num_samples_for_blob_base_fee_estimate ); assert_eq!( - read!(adjuster.blob_base_fee_statistics).median(), + read(&adjuster.blob_base_fee_statistics).median(), expected_median_blob_base_fee.into() ); @@ -109,15 +107,15 @@ async fn kept_updated(commitment_mode: L1BatchCommitmentMode) { adjuster.keep_updated().await.unwrap(); assert_eq!( - read!(adjuster.base_fee_statistics).samples.len(), + read(&adjuster.base_fee_statistics).samples.len(), config.max_base_fee_samples ); - assert_eq!(read!(adjuster.base_fee_statistics).median(), 7); + assert_eq!(read(&adjuster.base_fee_statistics).median(), 7); let expected_median_blob_base_fee = 393216 * 3; - assert_eq!(read!(adjuster.blob_base_fee_statistics).samples.len(), 3); + assert_eq!(read(&adjuster.blob_base_fee_statistics).samples.len(), 3); assert_eq!( - read!(adjuster.blob_base_fee_statistics).median(), + read(&adjuster.blob_base_fee_statistics).median(), expected_median_blob_base_fee.into() ); } diff --git a/core/node/state_keeper/src/io/tests/tester.rs b/core/node/state_keeper/src/io/tests/tester.rs index 0b360a210060..f5a132baea3b 100644 --- a/core/node/state_keeper/src/io/tests/tester.rs +++ b/core/node/state_keeper/src/io/tests/tester.rs @@ -50,10 +50,9 @@ impl Tester { let block_fees = vec![0, 4, 6, 8, 7, 5, 5, 8, 10, 9]; let base_fees = block_fees .into_iter() - .zip(std::iter::repeat(1)) - .map(|(block, blob)| BaseFees { - base_fee_per_gas: block, - base_fee_per_blob_gas: blob.into(), + .map(|base_fee_per_gas| BaseFees { + base_fee_per_gas, + base_fee_per_blob_gas: 1.into(), // Not relevant for the test }) .collect(); let eth_client = MockEthereum::builder().with_fee_history(base_fees).build(); From 8b053db69607e662ca7bff4c76b3f158118ba94d Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Thu, 27 Jun 2024 12:18:43 +0400 Subject: [PATCH 3/4] Make API implementation of eth_feeHistory compliant with the spec --- core/lib/eth_client/src/clients/http/query.rs | 12 ++++++++++++ core/node/api_server/src/web3/namespaces/eth.rs | 8 ++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/core/lib/eth_client/src/clients/http/query.rs b/core/lib/eth_client/src/clients/http/query.rs index dbd7e68d7779..cf140f7ecddf 100644 --- a/core/lib/eth_client/src/clients/http/query.rs +++ b/core/lib/eth_client/src/clients/http/query.rs @@ -112,6 +112,18 @@ where .with_arg("block", &chunk_end) .await?; + // Check that the lengths are the same. + // Per specification, the values should always be provided, and must be 0 for blocks + // prior to EIP-4844. + // https://ethereum.github.io/execution-apis/api-documentation/ + if fee_history.base_fee_per_gas.len() != fee_history.base_fee_per_blob_gas.len() { + tracing::warn!( + "base_fee_per_gas and base_fee_per_blob_gas have different lengths: {} and {}", + fee_history.base_fee_per_gas.len(), + fee_history.base_fee_per_blob_gas.len() + ); + } + for (base, blob) in fee_history .base_fee_per_gas .into_iter() diff --git a/core/node/api_server/src/web3/namespaces/eth.rs b/core/node/api_server/src/web3/namespaces/eth.rs index 6096855e308c..33dfa277dc1c 100644 --- a/core/node/api_server/src/web3/namespaces/eth.rs +++ b/core/node/api_server/src/web3/namespaces/eth.rs @@ -688,6 +688,10 @@ impl EthNamespace { base_fee_per_gas.len() ]); + // We do not support EIP-4844, but per API specification we should return 0 for pre EIP-4844 blocks. + let base_fee_per_blob_gas = vec![U256::zero(); base_fee_per_gas.len()]; + let blob_gas_used_ratio = vec![0.0; base_fee_per_gas.len()]; + // `base_fee_per_gas` for next L2 block cannot be calculated, appending last fee as a placeholder. base_fee_per_gas.push(*base_fee_per_gas.last().unwrap()); Ok(FeeHistory { @@ -695,8 +699,8 @@ impl EthNamespace { base_fee_per_gas, gas_used_ratio, reward, - base_fee_per_blob_gas: vec![], - blob_gas_used_ratio: vec![], + base_fee_per_blob_gas, + blob_gas_used_ratio, }) } From f1baaf61f7574ea1dd65c7a1fc9674bbb2a3562d Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Thu, 27 Jun 2024 12:23:02 +0400 Subject: [PATCH 4/4] Propagate warn to error --- core/lib/eth_client/src/clients/http/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/eth_client/src/clients/http/query.rs b/core/lib/eth_client/src/clients/http/query.rs index cf140f7ecddf..1dee9fb0fda5 100644 --- a/core/lib/eth_client/src/clients/http/query.rs +++ b/core/lib/eth_client/src/clients/http/query.rs @@ -117,7 +117,7 @@ where // prior to EIP-4844. // https://ethereum.github.io/execution-apis/api-documentation/ if fee_history.base_fee_per_gas.len() != fee_history.base_fee_per_blob_gas.len() { - tracing::warn!( + tracing::error!( "base_fee_per_gas and base_fee_per_blob_gas have different lengths: {} and {}", fee_history.base_fee_per_gas.len(), fee_history.base_fee_per_blob_gas.len()