From 78dbcae2fc2bb97ec354d91b5a9c6238f84b9c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cperekopskiy=E2=80=9D?= Date: Thu, 2 Nov 2023 16:06:39 +0200 Subject: [PATCH 1/3] Disable some seal criteria for boojum --- core/lib/constants/src/ethereum.rs | 4 +- core/lib/types/src/storage/writes/mod.rs | 9 ++-- .../src/api_server/tx_sender/mod.rs | 8 ++- core/lib/zksync_core/src/gas_tracker/mod.rs | 11 +++- .../zksync_core/src/state_keeper/keeper.rs | 1 + .../seal_criteria/conditional_sealer.rs | 13 ++++- .../seal_criteria/criteria/gas.rs | 8 +++ .../criteria/geometry_seal_criteria.rs | 50 +++++++++++++------ .../seal_criteria/criteria/pubdata_bytes.rs | 6 ++- .../seal_criteria/criteria/slots.rs | 5 ++ .../criteria/tx_encoding_size.rs | 15 +++++- .../src/state_keeper/seal_criteria/mod.rs | 3 +- 12 files changed, 107 insertions(+), 26 deletions(-) diff --git a/core/lib/constants/src/ethereum.rs b/core/lib/constants/src/ethereum.rs index 15187892ae86..321c680bc346 100644 --- a/core/lib/constants/src/ethereum.rs +++ b/core/lib/constants/src/ethereum.rs @@ -10,7 +10,9 @@ pub static ETHEREUM_ADDRESS: Address = Address::zero(); /// to be published inside the body of transaction (i.e. excluding of factory deps). pub const GUARANTEED_PUBDATA_PER_L1_BATCH: u64 = 4000; -/// The maximum number of pubdata per L1 batch. +/// The maximum number of pubdata per L1 batch. This limit is due to the fact that the Ethereum +/// nodes do not accept transactions that have more than 128kb of pubdata. +/// The 8kb margin is left in case of any inpreciseness of the pubdata calculation. pub const MAX_PUBDATA_PER_L1_BATCH: u64 = 120000; // TODO: import from zkevm_opcode_defs once VM1.3 is supported diff --git a/core/lib/types/src/storage/writes/mod.rs b/core/lib/types/src/storage/writes/mod.rs index 0826af82000a..587b608eedbd 100644 --- a/core/lib/types/src/storage/writes/mod.rs +++ b/core/lib/types/src/storage/writes/mod.rs @@ -4,13 +4,14 @@ use crate::H256; use serde::{Deserialize, Serialize}; use zksync_basic_types::{Address, U256}; -use self::compression::{compress_with_best_strategy, COMPRESSION_VERSION_NUMBER}; +pub(crate) use self::compression::{compress_with_best_strategy, COMPRESSION_VERSION_NUMBER}; mod compression; -const BYTES_PER_ENUMERATION_INDEX: u8 = 4; -// Total byte size of all fields in StateDiffRecord struct -// 20 + 32 + 32 +8 + 32 + 32 +/// The number of bytes being used for state diff enumeration indices. Applicable to repeated writes. +pub const BYTES_PER_ENUMERATION_INDEX: u8 = 4; +/// Total byte size of all fields in StateDiffRecord struct +/// 20 + 32 + 32 + 8 + 32 + 32 const STATE_DIFF_RECORD_SIZE: usize = 156; // 2 * 136 - the size that allows for two keccak rounds. diff --git a/core/lib/zksync_core/src/api_server/tx_sender/mod.rs b/core/lib/zksync_core/src/api_server/tx_sender/mod.rs index 7a29112d879b..e37ce5e06b51 100644 --- a/core/lib/zksync_core/src/api_server/tx_sender/mod.rs +++ b/core/lib/zksync_core/src/api_server/tx_sender/mod.rs @@ -907,7 +907,13 @@ impl TxSender { }; let seal_data = SealData::for_transaction(transaction, tx_metrics); - if let Some(reason) = ConditionalSealer::find_unexecutable_reason(sk_config, &seal_data) { + // Using `ProtocolVersionId::latest()` for a short period we might end up in a scenario where the StateKeeper is still pre-boojum + // but the API assumes we are post boojum. In this situation we will determine a tx as being executable but the StateKeeper will + // still reject them as it's not. + let protocol_version = ProtocolVersionId::latest(); + if let Some(reason) = + ConditionalSealer::find_unexecutable_reason(sk_config, &seal_data, protocol_version) + { let message = format!( "Tx is Unexecutable because of {reason}; inputs for decision: {seal_data:?}" ); diff --git a/core/lib/zksync_core/src/gas_tracker/mod.rs b/core/lib/zksync_core/src/gas_tracker/mod.rs index adadfb5c5304..7988a5bc01a8 100644 --- a/core/lib/zksync_core/src/gas_tracker/mod.rs +++ b/core/lib/zksync_core/src/gas_tracker/mod.rs @@ -103,7 +103,16 @@ pub(crate) fn commit_gas_count_for_l1_batch( let total_factory_deps_len: u32 = sorted_factory_deps .map(|factory_dep| factory_dep.len() as u32) .sum(); - let additional_calldata_bytes = metadata.initial_writes_compressed.len() as u32 + + // Boojum upgrade changes how storage writes are communicated/compressed. + let state_diff_size = if header.protocol_version.unwrap().is_pre_boojum() { + metadata.initial_writes_compressed.len() as u32 + + metadata.repeated_writes_compressed.len() as u32 + } else { + metadata.state_diffs_compressed.len() as u32 + }; + + let additional_calldata_bytes = state_diff_size + metadata.repeated_writes_compressed.len() as u32 + metadata.l2_l1_messages_compressed.len() as u32 + total_messages_len diff --git a/core/lib/zksync_core/src/state_keeper/keeper.rs b/core/lib/zksync_core/src/state_keeper/keeper.rs index 22ff96d9be94..73fb2e3ea4df 100644 --- a/core/lib/zksync_core/src/state_keeper/keeper.rs +++ b/core/lib/zksync_core/src/state_keeper/keeper.rs @@ -653,6 +653,7 @@ impl ZkSyncStateKeeper { updates_manager.pending_executed_transactions_len() + 1, &block_data, &tx_data, + updates_manager.protocol_version(), ) } else { SealResolution::NoSeal diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/conditional_sealer.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/conditional_sealer.rs index 21233c051bc7..2b5fb9fba485 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/conditional_sealer.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/conditional_sealer.rs @@ -4,6 +4,7 @@ //! which unconditionally follows the instructions from the main node). use zksync_config::configs::chain::StateKeeperConfig; +use zksync_types::ProtocolVersionId; use super::{criteria, SealCriterion, SealData, SealResolution, AGGREGATION_METRICS}; @@ -22,12 +23,20 @@ impl ConditionalSealer { pub(crate) fn find_unexecutable_reason( config: &StateKeeperConfig, data: &SealData, + protocol_version: ProtocolVersionId, ) -> Option<&'static str> { for sealer in &Self::default_sealers() { const MOCK_BLOCK_TIMESTAMP: u128 = 0; const TX_COUNT: usize = 1; - let resolution = sealer.should_seal(config, MOCK_BLOCK_TIMESTAMP, TX_COUNT, data, data); + let resolution = sealer.should_seal( + config, + MOCK_BLOCK_TIMESTAMP, + TX_COUNT, + data, + data, + protocol_version, + ); if matches!(resolution, SealResolution::Unexecutable(_)) { return Some(sealer.prom_criterion_name()); } @@ -55,6 +64,7 @@ impl ConditionalSealer { tx_count: usize, block_data: &SealData, tx_data: &SealData, + protocol_version: ProtocolVersionId, ) -> SealResolution { tracing::trace!( "Determining seal resolution for L1 batch #{l1_batch_number} with {tx_count} transactions \ @@ -70,6 +80,7 @@ impl ConditionalSealer { tx_count, block_data, tx_data, + protocol_version, ); match &seal_resolution { SealResolution::IncludeAndSeal diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/gas.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/gas.rs index 29c22c3c96bb..4683d9cf8798 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/gas.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/gas.rs @@ -1,3 +1,5 @@ +use zksync_types::ProtocolVersionId; + use crate::{ gas_tracker::new_block_gas_count, state_keeper::seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig}, @@ -20,6 +22,7 @@ impl SealCriterion for GasCriterion { _tx_count: usize, block_data: &SealData, tx_data: &SealData, + _protocol_version_id: ProtocolVersionId, ) -> SealResolution { let tx_bound = (config.max_single_tx_gas as f64 * config.reject_tx_at_gas_percentage).round() as u32; @@ -67,6 +70,7 @@ mod tests { ..SealData::default() }, &SealData::default(), + ProtocolVersionId::latest(), ); assert_eq!(empty_block_resolution, SealResolution::NoSeal); @@ -88,6 +92,7 @@ mod tests { gas_count: tx_gas, ..SealData::default() }, + ProtocolVersionId::latest(), ); assert_eq!( huge_transaction_resolution, @@ -114,6 +119,7 @@ mod tests { gas_count: tx_gas, ..SealData::default() }, + ProtocolVersionId::latest(), ); assert_eq!(resolution_after_first_tx, SealResolution::NoSeal); @@ -129,6 +135,7 @@ mod tests { gas_count: tx_gas, ..SealData::default() }, + ProtocolVersionId::latest(), ); assert_eq!(resolution_after_second_tx, SealResolution::ExcludeAndSeal); @@ -157,6 +164,7 @@ mod tests { gas_count: tx_gas, ..SealData::default() }, + ProtocolVersionId::latest(), ); assert_eq!(resolution_after_first_tx, SealResolution::IncludeAndSeal); } diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs index 5a03107a97f0..0aa92dcf4427 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs @@ -4,6 +4,7 @@ use zksync_config::configs::chain::StateKeeperConfig; use zksync_types::{ circuit::{GEOMETRY_CONFIG, SCHEDULER_UPPER_BOUND}, tx::tx_execution_info::{DeduplicatedWritesMetrics, ExecutionMetrics}, + ProtocolVersionId, }; // Local uses @@ -25,7 +26,7 @@ pub struct L2ToL1LogsCriterion; trait MetricExtractor { const PROM_METRIC_CRITERION_NAME: &'static str; - fn limit_per_block() -> usize; + fn limit_per_block(protocol_version: ProtocolVersionId) -> usize; fn extract(metric: &ExecutionMetrics, writes: &DeduplicatedWritesMetrics) -> usize; } @@ -40,16 +41,19 @@ where _tx_count: usize, block_data: &SealData, tx_data: &SealData, + protocol_version_id: ProtocolVersionId, ) -> SealResolution { - let reject_bound = - (T::limit_per_block() as f64 * config.reject_tx_at_geometry_percentage).round(); - let close_bound = - (T::limit_per_block() as f64 * config.close_block_at_geometry_percentage).round(); + let reject_bound = (T::limit_per_block(protocol_version_id) as f64 + * config.reject_tx_at_geometry_percentage) + .round(); + let close_bound = (T::limit_per_block(protocol_version_id) as f64 + * config.close_block_at_geometry_percentage) + .round(); if T::extract(&tx_data.execution_metrics, &tx_data.writes_metrics) > reject_bound as usize { SealResolution::Unexecutable("ZK proof cannot be generated for a transaction".into()) } else if T::extract(&block_data.execution_metrics, &block_data.writes_metrics) - >= T::limit_per_block() + >= T::limit_per_block(protocol_version_id) { SealResolution::ExcludeAndSeal } else if T::extract(&block_data.execution_metrics, &block_data.writes_metrics) @@ -69,8 +73,13 @@ where impl MetricExtractor for RepeatedWritesCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "repeated_storage_writes"; - fn limit_per_block() -> usize { - GEOMETRY_CONFIG.limit_for_repeated_writes_pubdata_hasher as usize + fn limit_per_block(protocol_version_id: ProtocolVersionId) -> usize { + if !protocol_version_id.is_pre_boojum() { + // In boojum there is no limit for repeated writes. + usize::MAX + } else { + GEOMETRY_CONFIG.limit_for_repeated_writes_pubdata_hasher as usize + } } fn extract(_metrics: &ExecutionMetrics, writes: &DeduplicatedWritesMetrics) -> usize { @@ -81,8 +90,13 @@ impl MetricExtractor for RepeatedWritesCriterion { impl MetricExtractor for InitialWritesCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "initial_storage_writes"; - fn limit_per_block() -> usize { - GEOMETRY_CONFIG.limit_for_initial_writes_pubdata_hasher as usize + fn limit_per_block(protocol_version_id: ProtocolVersionId) -> usize { + if !protocol_version_id.is_pre_boojum() { + // In boojum there is no limit for initial writes. + usize::MAX + } else { + GEOMETRY_CONFIG.limit_for_initial_writes_pubdata_hasher as usize + } } fn extract(_metrics: &ExecutionMetrics, writes: &DeduplicatedWritesMetrics) -> usize { @@ -93,7 +107,7 @@ impl MetricExtractor for InitialWritesCriterion { impl MetricExtractor for MaxCyclesCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "max_cycles"; - fn limit_per_block() -> usize { + fn limit_per_block(_protocol_version_id: ProtocolVersionId) -> usize { MAX_CYCLES_FOR_TX as usize } @@ -105,7 +119,7 @@ impl MetricExtractor for MaxCyclesCriterion { impl MetricExtractor for ComputationalGasCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "computational_gas"; - fn limit_per_block() -> usize { + fn limit_per_block(_protocol_version_id: ProtocolVersionId) -> usize { // We subtract constant to take into account that circuits may be not fully filled. // This constant should be greater than number of circuits types // but we keep it larger to be on the safe side. @@ -124,8 +138,13 @@ impl MetricExtractor for ComputationalGasCriterion { impl MetricExtractor for L2ToL1LogsCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "l2_to_l1_logs"; - fn limit_per_block() -> usize { - GEOMETRY_CONFIG.limit_for_l1_messages_merklizer as usize + fn limit_per_block(protocol_version_id: ProtocolVersionId) -> usize { + if !protocol_version_id.is_pre_boojum() { + // In boojum there is no limit for L2 to L1 logs. + usize::MAX + } else { + GEOMETRY_CONFIG.limit_for_l1_messages_merklizer as usize + } } fn extract(metrics: &ExecutionMetrics, _writes: &DeduplicatedWritesMetrics) -> usize { @@ -161,6 +180,7 @@ mod tests { ..SealData::default() }, &SealData::default(), + ProtocolVersionId::Version17, ); assert_eq!(block_resolution, SealResolution::NoSeal); } @@ -181,6 +201,7 @@ mod tests { ..SealData::default() }, &SealData::default(), + ProtocolVersionId::Version17, ); assert_eq!(block_resolution, SealResolution::IncludeAndSeal); } @@ -201,6 +222,7 @@ mod tests { ..SealData::default() }, &SealData::default(), + ProtocolVersionId::Version17, ); assert_eq!(block_resolution, SealResolution::ExcludeAndSeal); } diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs index 70f35f4278e3..2190e087c433 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/pubdata_bytes.rs @@ -1,4 +1,4 @@ -use zksync_types::MAX_PUBDATA_PER_L1_BATCH; +use zksync_types::{ProtocolVersionId, MAX_PUBDATA_PER_L1_BATCH}; use crate::state_keeper::seal_criteria::{ SealCriterion, SealData, SealResolution, StateKeeperConfig, @@ -15,6 +15,7 @@ impl SealCriterion for PubDataBytesCriterion { _tx_count: usize, block_data: &SealData, tx_data: &SealData, + _protocol_version: ProtocolVersionId, ) -> SealResolution { let max_pubdata_per_l1_batch = MAX_PUBDATA_PER_L1_BATCH as usize; let reject_bound = @@ -69,6 +70,7 @@ mod tests { ..SealData::default() }, &SealData::default(), + ProtocolVersionId::latest(), ); assert_eq!(empty_block_resolution, SealResolution::NoSeal); @@ -89,6 +91,7 @@ mod tests { ..SealData::default() }, &SealData::default(), + ProtocolVersionId::latest(), ); assert_eq!(full_block_resolution, SealResolution::IncludeAndSeal); @@ -105,6 +108,7 @@ mod tests { ..SealData::default() }, &SealData::default(), + ProtocolVersionId::latest(), ); assert_eq!(full_block_resolution, SealResolution::ExcludeAndSeal); } diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/slots.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/slots.rs index 2ab5029f5ae6..b434c3ba42a7 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/slots.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/slots.rs @@ -1,3 +1,5 @@ +use zksync_types::ProtocolVersionId; + use crate::state_keeper::seal_criteria::{ SealCriterion, SealData, SealResolution, StateKeeperConfig, }; @@ -14,6 +16,7 @@ impl SealCriterion for SlotsCriterion { tx_count: usize, _block_data: &SealData, _tx_data: &SealData, + _protocol_version: ProtocolVersionId, ) -> SealResolution { if tx_count >= config.transaction_slots { SealResolution::IncludeAndSeal @@ -42,6 +45,7 @@ mod tests { config.transaction_slots - 1, &SealData::default(), &SealData::default(), + ProtocolVersionId::latest(), ); assert_eq!(almost_full_block_resolution, SealResolution::NoSeal); @@ -51,6 +55,7 @@ mod tests { config.transaction_slots, &SealData::default(), &SealData::default(), + ProtocolVersionId::latest(), ); assert_eq!(full_block_resolution, SealResolution::IncludeAndSeal); } diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/tx_encoding_size.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/tx_encoding_size.rs index 39eb6a68062f..4672ac36e557 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/tx_encoding_size.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/tx_encoding_size.rs @@ -1,4 +1,5 @@ use multivm::vm_latest::constants::BOOTLOADER_TX_ENCODING_SPACE; +use zksync_types::ProtocolVersionId; use crate::state_keeper::seal_criteria::{ SealCriterion, SealData, SealResolution, StateKeeperConfig, @@ -15,6 +16,7 @@ impl SealCriterion for TxEncodingSizeCriterion { _tx_count: usize, block_data: &SealData, tx_data: &SealData, + _protocol_version_id: ProtocolVersionId, ) -> SealResolution { let reject_bound = (BOOTLOADER_TX_ENCODING_SPACE as f64 * config.reject_tx_at_geometry_percentage).round(); @@ -48,8 +50,14 @@ mod tests { let config = StateKeeperConfig::from_env().unwrap(); let criterion = TxEncodingSizeCriterion; - let empty_block_resolution = - criterion.should_seal(&config, 0, 0, &SealData::default(), &SealData::default()); + let empty_block_resolution = criterion.should_seal( + &config, + 0, + 0, + &SealData::default(), + &SealData::default(), + ProtocolVersionId::latest(), + ); assert_eq!(empty_block_resolution, SealResolution::NoSeal); let unexecutable_resolution = criterion.should_seal( @@ -61,6 +69,7 @@ mod tests { cumulative_size: BOOTLOADER_TX_ENCODING_SPACE as usize + 1, ..SealData::default() }, + ProtocolVersionId::latest(), ); assert_eq!( unexecutable_resolution, @@ -81,6 +90,7 @@ mod tests { cumulative_size: 1, ..SealData::default() }, + ProtocolVersionId::latest(), ); assert_eq!(exclude_and_seal_resolution, SealResolution::ExcludeAndSeal); @@ -96,6 +106,7 @@ mod tests { cumulative_size: 1, ..SealData::default() }, + ProtocolVersionId::latest(), ); assert_eq!(include_and_seal_resolution, SealResolution::IncludeAndSeal); } diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/mod.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/mod.rs index da410ebaf5f1..19cd5b87da50 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/mod.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/mod.rs @@ -18,7 +18,7 @@ use zksync_types::{ block::BlockGasCount, fee::TransactionExecutionMetrics, tx::tx_execution_info::{DeduplicatedWritesMetrics, ExecutionMetrics}, - Transaction, + ProtocolVersionId, Transaction, }; use zksync_utils::time::millis_since; @@ -111,6 +111,7 @@ pub(super) trait SealCriterion: fmt::Debug + Send + 'static { tx_count: usize, block_data: &SealData, tx_data: &SealData, + protocol_version: ProtocolVersionId, ) -> SealResolution; // We need self here only for rust restrictions for creating an object from trait From ffc38625f2c701dfad4e8a9710a4a4de7a373cf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cperekopskiy=E2=80=9D?= Date: Thu, 2 Nov 2023 17:22:33 +0200 Subject: [PATCH 2/3] Fix test --- .../criteria/geometry_seal_criteria.rs | 98 ++++++++++++++----- 1 file changed, 76 insertions(+), 22 deletions(-) diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs index 0aa92dcf4427..96454a260f31 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs @@ -168,6 +168,7 @@ mod tests { block_execution_metrics: ExecutionMetrics, block_writes_metrics: DeduplicatedWritesMetrics, criterion: &dyn SealCriterion, + protocol_version: ProtocolVersionId, ) { let config = get_config(); let block_resolution = criterion.should_seal( @@ -180,7 +181,7 @@ mod tests { ..SealData::default() }, &SealData::default(), - ProtocolVersionId::Version17, + protocol_version, ); assert_eq!(block_resolution, SealResolution::NoSeal); } @@ -189,6 +190,7 @@ mod tests { block_execution_metrics: ExecutionMetrics, block_writes_metrics: DeduplicatedWritesMetrics, criterion: &dyn SealCriterion, + protocol_version: ProtocolVersionId, ) { let config = get_config(); let block_resolution = criterion.should_seal( @@ -201,7 +203,7 @@ mod tests { ..SealData::default() }, &SealData::default(), - ProtocolVersionId::Version17, + protocol_version, ); assert_eq!(block_resolution, SealResolution::IncludeAndSeal); } @@ -210,6 +212,7 @@ mod tests { block_execution_metrics: ExecutionMetrics, block_writes_metrics: DeduplicatedWritesMetrics, criterion: &dyn SealCriterion, + protocol_version: ProtocolVersionId, ) { let config = get_config(); let block_resolution = criterion.should_seal( @@ -222,7 +225,7 @@ mod tests { ..SealData::default() }, &SealData::default(), - ProtocolVersionId::Version17, + protocol_version, ); assert_eq!(block_resolution, SealResolution::ExcludeAndSeal); } @@ -231,6 +234,7 @@ mod tests { tx_execution_metrics: ExecutionMetrics, tx_writes_metrics: DeduplicatedWritesMetrics, criterion: &dyn SealCriterion, + protocol_version: ProtocolVersionId, ) { let config = get_config(); let block_resolution = criterion.should_seal( @@ -243,6 +247,7 @@ mod tests { writes_metrics: tx_writes_metrics, ..SealData::default() }, + protocol_version, ); assert_eq!( @@ -252,17 +257,22 @@ mod tests { } macro_rules! test_scenario_execution_metrics { - ($criterion: tt, $metric_name: ident, $metric_type: ty) => { + ($criterion: tt, $metric_name: ident, $metric_type: ty, $protocol_version: expr) => { let config = get_config(); let writes_metrics = DeduplicatedWritesMetrics::default(); let block_execution_metrics = ExecutionMetrics { - $metric_name: ($criterion::limit_per_block() / 2) as $metric_type, + $metric_name: ($criterion::limit_per_block($protocol_version) / 2) as $metric_type, ..ExecutionMetrics::default() }; - test_no_seal_block_resolution(block_execution_metrics, writes_metrics, &$criterion); + test_no_seal_block_resolution( + block_execution_metrics, + writes_metrics, + &$criterion, + $protocol_version, + ); let block_execution_metrics = ExecutionMetrics { - $metric_name: ($criterion::limit_per_block() - 1) as $metric_type, + $metric_name: ($criterion::limit_per_block($protocol_version) - 1) as $metric_type, ..ExecutionMetrics::default() }; @@ -270,10 +280,11 @@ mod tests { block_execution_metrics, writes_metrics, &$criterion, + $protocol_version, ); let block_execution_metrics = ExecutionMetrics { - $metric_name: ($criterion::limit_per_block()) as $metric_type, + $metric_name: ($criterion::limit_per_block($protocol_version)) as $metric_type, ..ExecutionMetrics::default() }; @@ -281,32 +292,43 @@ mod tests { block_execution_metrics, writes_metrics, &$criterion, + $protocol_version, ); let tx_execution_metrics = ExecutionMetrics { - $metric_name: ($criterion::limit_per_block() as f64 + $metric_name: ($criterion::limit_per_block($protocol_version) as f64 * config.reject_tx_at_geometry_percentage + 1f64) .round() as $metric_type, ..ExecutionMetrics::default() }; - test_unexecutable_tx_resolution(tx_execution_metrics, writes_metrics, &$criterion); + test_unexecutable_tx_resolution( + tx_execution_metrics, + writes_metrics, + &$criterion, + $protocol_version, + ); }; } macro_rules! test_scenario_writes_metrics { - ($criterion:tt, $metric_name:ident, $metric_type:ty) => { + ($criterion:tt, $metric_name:ident, $metric_type:ty, $protocol_version:expr) => { let config = get_config(); let execution_metrics = ExecutionMetrics::default(); let block_writes_metrics = DeduplicatedWritesMetrics { - $metric_name: ($criterion::limit_per_block() / 2) as $metric_type, + $metric_name: ($criterion::limit_per_block($protocol_version) / 2) as $metric_type, ..Default::default() }; - test_no_seal_block_resolution(execution_metrics, block_writes_metrics, &$criterion); + test_no_seal_block_resolution( + execution_metrics, + block_writes_metrics, + &$criterion, + $protocol_version, + ); let block_writes_metrics = DeduplicatedWritesMetrics { - $metric_name: ($criterion::limit_per_block() - 1) as $metric_type, + $metric_name: ($criterion::limit_per_block($protocol_version) - 1) as $metric_type, ..Default::default() }; @@ -314,10 +336,11 @@ mod tests { execution_metrics, block_writes_metrics, &$criterion, + $protocol_version, ); let block_writes_metrics = DeduplicatedWritesMetrics { - $metric_name: ($criterion::limit_per_block()) as $metric_type, + $metric_name: ($criterion::limit_per_block($protocol_version)) as $metric_type, ..Default::default() }; @@ -325,42 +348,73 @@ mod tests { execution_metrics, block_writes_metrics, &$criterion, + $protocol_version, ); let tx_writes_metrics = DeduplicatedWritesMetrics { - $metric_name: ($criterion::limit_per_block() as f64 + $metric_name: ($criterion::limit_per_block($protocol_version) as f64 * config.reject_tx_at_geometry_percentage + 1f64) .round() as $metric_type, ..Default::default() }; - test_unexecutable_tx_resolution(execution_metrics, tx_writes_metrics, &$criterion); + test_unexecutable_tx_resolution( + execution_metrics, + tx_writes_metrics, + &$criterion, + $protocol_version, + ); }; } #[test] fn repeated_writes_seal_criterion() { - test_scenario_writes_metrics!(RepeatedWritesCriterion, repeated_storage_writes, usize); + test_scenario_writes_metrics!( + RepeatedWritesCriterion, + repeated_storage_writes, + usize, + ProtocolVersionId::Version17 + ); } #[test] fn initial_writes_seal_criterion() { - test_scenario_writes_metrics!(InitialWritesCriterion, initial_storage_writes, usize); + test_scenario_writes_metrics!( + InitialWritesCriterion, + initial_storage_writes, + usize, + ProtocolVersionId::Version17 + ); } #[test] fn max_cycles_seal_criterion() { - test_scenario_execution_metrics!(MaxCyclesCriterion, cycles_used, u32); + test_scenario_execution_metrics!( + MaxCyclesCriterion, + cycles_used, + u32, + ProtocolVersionId::Version17 + ); } #[test] fn computational_gas_seal_criterion() { - test_scenario_execution_metrics!(ComputationalGasCriterion, computational_gas_used, u32); + test_scenario_execution_metrics!( + ComputationalGasCriterion, + computational_gas_used, + u32, + ProtocolVersionId::Version17 + ); } #[test] fn l2_to_l1_logs_seal_criterion() { - test_scenario_execution_metrics!(L2ToL1LogsCriterion, l2_l1_logs, usize); + test_scenario_execution_metrics!( + L2ToL1LogsCriterion, + l2_l1_logs, + usize, + ProtocolVersionId::Version17 + ); } } From 78944f6d9c42bd039881f6e687f8b45c660b08da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cperekopskiy=E2=80=9D?= Date: Thu, 2 Nov 2023 17:58:34 +0200 Subject: [PATCH 3/3] Make ifs more readable --- .../criteria/geometry_seal_criteria.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs index 96454a260f31..6495b9459039 100644 --- a/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs +++ b/core/lib/zksync_core/src/state_keeper/seal_criteria/criteria/geometry_seal_criteria.rs @@ -74,11 +74,11 @@ impl MetricExtractor for RepeatedWritesCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "repeated_storage_writes"; fn limit_per_block(protocol_version_id: ProtocolVersionId) -> usize { - if !protocol_version_id.is_pre_boojum() { + if protocol_version_id.is_pre_boojum() { + GEOMETRY_CONFIG.limit_for_repeated_writes_pubdata_hasher as usize + } else { // In boojum there is no limit for repeated writes. usize::MAX - } else { - GEOMETRY_CONFIG.limit_for_repeated_writes_pubdata_hasher as usize } } @@ -91,11 +91,11 @@ impl MetricExtractor for InitialWritesCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "initial_storage_writes"; fn limit_per_block(protocol_version_id: ProtocolVersionId) -> usize { - if !protocol_version_id.is_pre_boojum() { + if protocol_version_id.is_pre_boojum() { + GEOMETRY_CONFIG.limit_for_initial_writes_pubdata_hasher as usize + } else { // In boojum there is no limit for initial writes. usize::MAX - } else { - GEOMETRY_CONFIG.limit_for_initial_writes_pubdata_hasher as usize } } @@ -139,11 +139,11 @@ impl MetricExtractor for L2ToL1LogsCriterion { const PROM_METRIC_CRITERION_NAME: &'static str = "l2_to_l1_logs"; fn limit_per_block(protocol_version_id: ProtocolVersionId) -> usize { - if !protocol_version_id.is_pre_boojum() { + if protocol_version_id.is_pre_boojum() { + GEOMETRY_CONFIG.limit_for_l1_messages_merklizer as usize + } else { // In boojum there is no limit for L2 to L1 logs. usize::MAX - } else { - GEOMETRY_CONFIG.limit_for_l1_messages_merklizer as usize } }