From 390534210af738eac47da02603594efebc775e36 Mon Sep 17 00:00:00 2001 From: zeapoz Date: Thu, 23 May 2024 14:27:09 +0200 Subject: [PATCH 1/6] feat: procure `l2_fair_gas_price` from fork source --- src/main.rs | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/src/main.rs b/src/main.rs index d61f9177..e565861f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,6 @@ use crate::cache::CacheConfig; use crate::node::{InMemoryNodeConfig, ShowGasDetails, ShowStorageLogs, ShowVMDetails}; use crate::observability::Observability; -use crate::utils::to_human_size; use clap::{Parser, Subcommand, ValueEnum}; use colored::Colorize; use fork::{ForkDetails, ForkSource}; @@ -362,32 +361,21 @@ async fn main() -> anyhow::Result<()> { DevSystemContracts::Local => system_contracts::Options::Local, }; - // If we're forking we set the price to be equal to that contained within - // `ForkDetails`. If not, we use the `DEFAULT_L2_GAS_PRICE` instead. - let mut l2_fair_gas_price = { + // If L2 gas price has been supplied as an argument use that value, + // otherwise procure it from the fork source, or if that fails, use the + // `DEFAULT_L2_GAS_PRICE`. + let l2_fair_gas_price = opt.l2_gas_price.unwrap_or_else(|| { if let Some(f) = &fork_details { f.l2_fair_gas_price } else { DEFAULT_L2_GAS_PRICE } - }; + }); - // If L2 gas price has been supplied as an argument, override the value - // procured previously. - match opt.l2_gas_price { - Some(l2_gas_price) => { - tracing::info!( - "Starting node with L2 gas price set to {} (overridden from {})", - to_human_size(l2_gas_price.into()), - to_human_size(l2_fair_gas_price.into()) - ); - l2_fair_gas_price = l2_gas_price; - } - None => tracing::info!( - "Starting node with L2 gas price set to {}", - to_human_size(l2_fair_gas_price.into()) - ), - } + tracing::info!( + "Starting node with L2 gas price set to {}", + l2_fair_gas_price + ); let node = InMemoryNode::new( fork_details, From f5a936631e056674db321384dc004d758c8df3ea Mon Sep 17 00:00:00 2001 From: zeapoz Date: Fri, 24 May 2024 14:44:12 +0200 Subject: [PATCH 2/6] chore: explicit info when overriding l2 gas price --- src/main.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/main.rs b/src/main.rs index e565861f..d61f9177 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,7 @@ use crate::cache::CacheConfig; use crate::node::{InMemoryNodeConfig, ShowGasDetails, ShowStorageLogs, ShowVMDetails}; use crate::observability::Observability; +use crate::utils::to_human_size; use clap::{Parser, Subcommand, ValueEnum}; use colored::Colorize; use fork::{ForkDetails, ForkSource}; @@ -361,21 +362,32 @@ async fn main() -> anyhow::Result<()> { DevSystemContracts::Local => system_contracts::Options::Local, }; - // If L2 gas price has been supplied as an argument use that value, - // otherwise procure it from the fork source, or if that fails, use the - // `DEFAULT_L2_GAS_PRICE`. - let l2_fair_gas_price = opt.l2_gas_price.unwrap_or_else(|| { + // If we're forking we set the price to be equal to that contained within + // `ForkDetails`. If not, we use the `DEFAULT_L2_GAS_PRICE` instead. + let mut l2_fair_gas_price = { if let Some(f) = &fork_details { f.l2_fair_gas_price } else { DEFAULT_L2_GAS_PRICE } - }); + }; - tracing::info!( - "Starting node with L2 gas price set to {}", - l2_fair_gas_price - ); + // If L2 gas price has been supplied as an argument, override the value + // procured previously. + match opt.l2_gas_price { + Some(l2_gas_price) => { + tracing::info!( + "Starting node with L2 gas price set to {} (overridden from {})", + to_human_size(l2_gas_price.into()), + to_human_size(l2_fair_gas_price.into()) + ); + l2_fair_gas_price = l2_gas_price; + } + None => tracing::info!( + "Starting node with L2 gas price set to {}", + to_human_size(l2_fair_gas_price.into()) + ), + } let node = InMemoryNode::new( fork_details, From a181824d4667e9e5c8e354f44353df0aa0522d44 Mon Sep 17 00:00:00 2001 From: zeapoz Date: Fri, 31 May 2024 18:07:16 +0200 Subject: [PATCH 3/6] feat: different gas estimation constants per network --- src/fork.rs | 37 +++++++++++++++++++++++++++++++++++-- src/node/fee_model.rs | 13 ++++++++++++- src/node/in_memory.rs | 24 +++++++++++++++++------- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/fork.rs b/src/fork.rs index 6dc4ccb4..6d2d397b 100644 --- a/src/fork.rs +++ b/src/fork.rs @@ -33,9 +33,12 @@ use zksync_web3_decl::{ }; use zksync_web3_decl::{namespaces::EthNamespaceClient, types::Index}; -use crate::system_contracts; use crate::{cache::CacheConfig, node::TEST_NODE_NETWORK_ID}; use crate::{deps::InMemoryStorage, http_fork_source::HttpForkSource}; +use crate::{ + node::{DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, DEFAULT_ESTIMATE_GAS_SCALE_FACTOR}, + system_contracts, +}; pub fn block_on(future: F) -> F::Output where @@ -52,6 +55,19 @@ where .unwrap() } +// TODO: Don't match on url, we should have an enum representing each possible value instead. +pub fn gase_scale_factors_from_url(url: &str) -> (f64, f32) { + match url { + "https://mainnet.era.zksync.io:443" => (1.5, 1.2), + "https://sepolia.era.zksync.dev:443" => (2.0, 1.2), + "https://testnet.era.zksync.dev:443" => (1.2, 1.2), + _ => ( + DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, + DEFAULT_ESTIMATE_GAS_SCALE_FACTOR, + ), + } +} + /// In memory storage, that allows 'forking' from other network. /// If forking is enabled, it reads missing data from remote location. /// S - is a struct that is used for source of the fork. @@ -321,6 +337,10 @@ pub struct ForkDetails { pub overwrite_chain_id: Option, pub l1_gas_price: u64, pub l2_fair_gas_price: u64, + /// L1 Gas Price Scale Factor for gas estimation. + pub estimate_gas_price_scale_factor: f64, + /// The factor by which to scale the gasLimit. + pub estimate_gas_scale_factor: f32, } const SUPPORTED_VERSIONS: &[ProtocolVersionId] = &[ @@ -402,6 +422,8 @@ impl ForkDetails { ); } + let (estimate_gas_price_scale_factor, estimate_gas_scale_factor) = + gase_scale_factors_from_url(url); ForkDetails { fork_source: HttpForkSource::new(url.to_owned(), cache_config), l1_block: l1_batch_number, @@ -412,6 +434,8 @@ impl ForkDetails { overwrite_chain_id: chain_id, l1_gas_price: block_details.base.l1_gas_price, l2_fair_gas_price: block_details.base.l2_fair_gas_price, + estimate_gas_price_scale_factor, + estimate_gas_scale_factor, } } /// Create a fork from a given network at a given height. @@ -508,7 +532,14 @@ mod tests { use zksync_state::ReadStorage; use zksync_types::{api::TransactionVariant, StorageKey}; - use crate::{deps::InMemoryStorage, node::DEFAULT_L2_GAS_PRICE, system_contracts, testing}; + use crate::{ + deps::InMemoryStorage, + node::{ + DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, DEFAULT_ESTIMATE_GAS_SCALE_FACTOR, + DEFAULT_L2_GAS_PRICE, + }, + system_contracts, testing, + }; use super::{ForkDetails, ForkStorage}; @@ -538,6 +569,8 @@ mod tests { overwrite_chain_id: None, l1_gas_price: 100, l2_fair_gas_price: DEFAULT_L2_GAS_PRICE, + estimate_gas_price_scale_factor: DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, + estimate_gas_scale_factor: DEFAULT_ESTIMATE_GAS_SCALE_FACTOR, }; let mut fork_storage = ForkStorage::new(Some(fork_details), &options); diff --git a/src/node/fee_model.rs b/src/node/fee_model.rs index 88e7bf3a..75600b8b 100644 --- a/src/node/fee_model.rs +++ b/src/node/fee_model.rs @@ -7,13 +7,24 @@ use zksync_types::L1_GAS_PER_PUBDATA_BYTE; pub struct TestNodeFeeInputProvider { pub l1_gas_price: u64, pub l2_gas_price: u64, + /// L1 Gas Price Scale Factor for gas estimation. + pub estimate_gas_price_scale_factor: f64, + /// The factor by which to scale the gasLimit. + pub estimate_gas_scale_factor: f32, } impl TestNodeFeeInputProvider { - pub fn new(l1_gas_price: u64, l2_gas_price: u64) -> Self { + pub fn new( + l1_gas_price: u64, + l2_gas_price: u64, + estimate_gas_price_scale_factor: f64, + estimate_gas_scale_factor: f32, + ) -> Self { Self { l1_gas_price, l2_gas_price, + estimate_gas_price_scale_factor, + estimate_gas_scale_factor, } } diff --git a/src/node/in_memory.rs b/src/node/in_memory.rs index ee62c1fd..5779f1bb 100644 --- a/src/node/in_memory.rs +++ b/src/node/in_memory.rs @@ -79,11 +79,11 @@ pub const L1_GAS_PRICE: u64 = 50_000_000_000; /// The default L2 Gas Price to be used if not supplied via the CLI argument. pub const DEFAULT_L2_GAS_PRICE: u64 = 25_000_000; /// L1 Gas Price Scale Factor for gas estimation. -pub const ESTIMATE_GAS_PRICE_SCALE_FACTOR: f64 = 1.5; +pub const DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR: f64 = 1.5; /// Acceptable gas overestimation limit. pub const ESTIMATE_GAS_ACCEPTABLE_OVERESTIMATION: u64 = 1_000; /// The factor by which to scale the gasLimit. -pub const ESTIMATE_GAS_SCALE_FACTOR: f32 = 1.3; +pub const DEFAULT_ESTIMATE_GAS_SCALE_FACTOR: f32 = 1.3; /// The maximum number of previous blocks to store the state for. pub const MAX_PREVIOUS_STATES: u16 = 128; /// The zks protocol version. @@ -455,8 +455,8 @@ impl InMemoryNodeInner { let fee_input = block_on(async move { fee_input_provider .get_batch_fee_input_scaled( - ESTIMATE_GAS_PRICE_SCALE_FACTOR, - ESTIMATE_GAS_PRICE_SCALE_FACTOR, + fee_input_provider.estimate_gas_price_scale_factor, + fee_input_provider.estimate_gas_price_scale_factor, ) .await .unwrap() @@ -584,11 +584,15 @@ impl InMemoryNodeInner { tracing::trace!("Gas Estimation Values:"); tracing::trace!(" Final upper_bound: {}", upper_bound); - tracing::trace!(" ESTIMATE_GAS_SCALE_FACTOR: {}", ESTIMATE_GAS_SCALE_FACTOR); + tracing::trace!( + " ESTIMATE_GAS_SCALE_FACTOR: {}", + self.fee_input_provider.estimate_gas_scale_factor + ); tracing::trace!(" MAX_L2_TX_GAS_LIMIT: {}", MAX_L2_TX_GAS_LIMIT); let tx_body_gas_limit = upper_bound; - let suggested_gas_limit = - ((upper_bound + additional_gas_for_pubdata) as f32 * ESTIMATE_GAS_SCALE_FACTOR) as u64; + let suggested_gas_limit = ((upper_bound + additional_gas_for_pubdata) as f32 + * self.fee_input_provider.estimate_gas_scale_factor) + as u64; let estimate_gas_result = InMemoryNodeInner::estimate_gas_step( l2_tx.clone(), @@ -953,6 +957,8 @@ impl InMemoryNode { fee_input_provider: TestNodeFeeInputProvider::new( f.l1_gas_price, config.l2_fair_gas_price, + f.estimate_gas_price_scale_factor, + f.estimate_gas_scale_factor, ), tx_results: Default::default(), blocks, @@ -990,6 +996,8 @@ impl InMemoryNode { fee_input_provider: TestNodeFeeInputProvider::new( L1_GAS_PRICE, config.l2_fair_gas_price, + DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, + DEFAULT_ESTIMATE_GAS_SCALE_FACTOR, ), tx_results: Default::default(), blocks, @@ -1888,6 +1896,8 @@ mod tests { overwrite_chain_id: None, l1_gas_price: 1000, l2_fair_gas_price: DEFAULT_L2_GAS_PRICE, + estimate_gas_price_scale_factor: DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, + estimate_gas_scale_factor: DEFAULT_ESTIMATE_GAS_SCALE_FACTOR, }), None, Default::default(), From d5eb6029c4c244d3e59b87a496a360709c21189b Mon Sep 17 00:00:00 2001 From: zeapoz Date: Fri, 31 May 2024 18:31:32 +0200 Subject: [PATCH 4/6] ref: `ForkNetwork` enum --- src/fork.rs | 81 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 26 deletions(-) diff --git a/src/fork.rs b/src/fork.rs index 6d2d397b..8f33a2f1 100644 --- a/src/fork.rs +++ b/src/fork.rs @@ -55,16 +55,36 @@ where .unwrap() } -// TODO: Don't match on url, we should have an enum representing each possible value instead. -pub fn gase_scale_factors_from_url(url: &str) -> (f64, f32) { - match url { - "https://mainnet.era.zksync.io:443" => (1.5, 1.2), - "https://sepolia.era.zksync.dev:443" => (2.0, 1.2), - "https://testnet.era.zksync.dev:443" => (1.2, 1.2), - _ => ( - DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, - DEFAULT_ESTIMATE_GAS_SCALE_FACTOR, - ), +/// The possible networks to fork from. +pub enum ForkNetwork { + Mainnet, + SepoliaTestnet, + GoerliTestnet, + Other(String), +} + +impl ForkNetwork { + /// Return the URL for the underlying fork source. + pub fn to_url(&self) -> &str { + match self { + ForkNetwork::Mainnet => "https://mainnet.era.zksync.io:443", + ForkNetwork::SepoliaTestnet => "https://sepolia.era.zksync.dev:443", + ForkNetwork::GoerliTestnet => "https://testnet.era.zksync.dev:443", + ForkNetwork::Other(url) => url, + } + } + + /// Returns the local gas scale factors currently in ues by the upstream network. + pub fn local_gas_scale_factors(&self) -> (f64, f32) { + match self { + ForkNetwork::Mainnet => (1.5, 1.2), + ForkNetwork::SepoliaTestnet => (2.0, 1.2), + ForkNetwork::GoerliTestnet => (1.2, 1.2), + ForkNetwork::Other(_) => ( + DEFAULT_ESTIMATE_GAS_PRICE_SCALE_FACTOR, + DEFAULT_ESTIMATE_GAS_SCALE_FACTOR, + ), + } } } @@ -376,13 +396,14 @@ pub fn supported_versions_to_string() -> String { } impl ForkDetails { - pub async fn from_url_and_miniblock_and_chain( - url: &str, + pub async fn from_network_and_miniblock_and_chain( + network: ForkNetwork, client: Client, miniblock: u64, chain_id: Option, cache_config: CacheConfig, ) -> Self { + let url = network.to_url(); let block_details = client .get_block_details(L2BlockNumber(miniblock as u32)) .await @@ -423,7 +444,7 @@ impl ForkDetails { } let (estimate_gas_price_scale_factor, estimate_gas_scale_factor) = - gase_scale_factors_from_url(url); + network.local_gas_scale_factors(); ForkDetails { fork_source: HttpForkSource::new(url.to_owned(), cache_config), l1_block: l1_batch_number, @@ -440,19 +461,26 @@ impl ForkDetails { } /// Create a fork from a given network at a given height. pub async fn from_network(fork: &str, fork_at: Option, cache_config: CacheConfig) -> Self { - let (url, client) = Self::fork_to_url_and_client(fork); + let (network, client) = Self::fork_network_and_client(fork); let l2_miniblock = if let Some(fork_at) = fork_at { fork_at } else { client.get_block_number().await.unwrap().as_u64() }; - Self::from_url_and_miniblock_and_chain(url, client, l2_miniblock, None, cache_config).await + Self::from_network_and_miniblock_and_chain( + network, + client, + l2_miniblock, + None, + cache_config, + ) + .await } /// Create a fork from a given network, at a height BEFORE a transaction. /// This will allow us to apply this transaction locally on top of this fork. pub async fn from_network_tx(fork: &str, tx: H256, cache_config: CacheConfig) -> Self { - let (url, client) = Self::fork_to_url_and_client(fork); + let (network, client) = Self::fork_network_and_client(fork); let tx_details = client.get_transaction_by_hash(tx).await.unwrap().unwrap(); let overwrite_chain_id = Some( L2ChainId::try_from(tx_details.chain_id.as_u64()).unwrap_or_else(|err| { @@ -463,8 +491,8 @@ impl ForkDetails { // We have to sync to the one-miniblock before the one where transaction is. let l2_miniblock = miniblock_number.saturating_sub(1) as u64; - Self::from_url_and_miniblock_and_chain( - url, + Self::from_network_and_miniblock_and_chain( + network, client, l2_miniblock, overwrite_chain_id, @@ -475,22 +503,23 @@ impl ForkDetails { } impl ForkDetails { - /// Return URL and HTTP client for a given fork name. - pub fn fork_to_url_and_client(fork: &str) -> (&str, Client) { - let url = match fork { - "mainnet" => "https://mainnet.era.zksync.io:443", - "sepolia-testnet" => "https://sepolia.era.zksync.dev:443", - "goerli-testnet" => "https://testnet.era.zksync.dev:443", - _ => fork, + /// Return [`ForkNetwork`] and HTTP client for a given fork name. + pub fn fork_network_and_client(fork: &str) -> (ForkNetwork, Client) { + let network = match fork { + "mainnet" => ForkNetwork::Mainnet, + "sepolia-testnet" => ForkNetwork::SepoliaTestnet, + "goerli-testnet" => ForkNetwork::GoerliTestnet, + _ => ForkNetwork::Other(fork.to_string()), }; + let url = network.to_url(); let parsed_url = SensitiveUrl::from_str(url) .unwrap_or_else(|_| panic!("Unable to parse client URL: {}", &url)); let client = Client::http(parsed_url) .unwrap_or_else(|_| panic!("Unable to create a client for fork: {}", &url)) .build(); - (url, client) + (network, client) } /// Returns transactions that are in the same L2 miniblock as replay_tx, but were executed before it. From 810c6b12adfd3889d6ceef6229639859d84ed127 Mon Sep 17 00:00:00 2001 From: zeapoz Date: Mon, 17 Jun 2024 11:21:46 +0200 Subject: [PATCH 5/6] chore: derive on `ForkNetwork` --- src/fork.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fork.rs b/src/fork.rs index 8f33a2f1..6657035d 100644 --- a/src/fork.rs +++ b/src/fork.rs @@ -56,6 +56,7 @@ where } /// The possible networks to fork from. +#[derive(Debug, Clone)] pub enum ForkNetwork { Mainnet, SepoliaTestnet, From 9c89ea46278a257a94fe47e6a356c2626213f1a7 Mon Sep 17 00:00:00 2001 From: zeapoz Date: Mon, 17 Jun 2024 11:39:44 +0200 Subject: [PATCH 6/6] chore: correct misspelling --- src/fork.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fork.rs b/src/fork.rs index 6657035d..b7e6cb0f 100644 --- a/src/fork.rs +++ b/src/fork.rs @@ -75,7 +75,7 @@ impl ForkNetwork { } } - /// Returns the local gas scale factors currently in ues by the upstream network. + /// Returns the local gas scale factors currently in use by the upstream network. pub fn local_gas_scale_factors(&self) -> (f64, f32) { match self { ForkNetwork::Mainnet => (1.5, 1.2),