From 6ca3a32575fbb536d1edfee414e65fb7ea9da8b0 Mon Sep 17 00:00:00 2001 From: perekopskiy Date: Fri, 22 Nov 2024 14:04:39 +0200 Subject: [PATCH] aggregate commit ops for validiums with sl=l1 --- core/lib/types/src/aggregated_operations.rs | 6 +- core/node/eth_sender/src/aggregator.rs | 53 +++++++++++---- core/node/eth_sender/src/eth_tx_aggregator.rs | 13 ++-- core/node/eth_sender/src/publish_criterion.rs | 66 +++++++++++++++---- 4 files changed, 106 insertions(+), 32 deletions(-) diff --git a/core/lib/types/src/aggregated_operations.rs b/core/lib/types/src/aggregated_operations.rs index dc0e3e6153c7..3df073bd4d61 100644 --- a/core/lib/types/src/aggregated_operations.rs +++ b/core/lib/types/src/aggregated_operations.rs @@ -40,8 +40,10 @@ impl FromStr for AggregatedActionType { } } -/// Additional cost of processing `Execute` operation per batch. +/// Additional gas cost of processing `Execute` operation per batch. +/// It's applicable iff SL is Ethereum. pub const L1_BATCH_EXECUTE_BASE_COST: u32 = 30_000; -/// Additional cost of processing `Execute` operation per L1->L2 tx. +/// Additional gas cost of processing `Execute` operation per L1->L2 tx. +/// It's applicable iff SL is Ethereum. pub const L1_OPERATION_EXECUTE_COST: u32 = 12_500; diff --git a/core/node/eth_sender/src/aggregator.rs b/core/node/eth_sender/src/aggregator.rs index 631ff4c111ff..91e0957ca63b 100644 --- a/core/node/eth_sender/src/aggregator.rs +++ b/core/node/eth_sender/src/aggregator.rs @@ -19,7 +19,8 @@ use zksync_types::{ use super::{ aggregated_operations::AggregatedOperation, publish_criterion::{ - ExecuteGasCriterion, L1BatchPublishCriterion, NumberCriterion, TimestampDeadlineCriterion, + GasCriterionKind, L1BatchPublishCriterion, L1GasCriterion, NumberCriterion, + TimestampDeadlineCriterion, }, }; @@ -79,23 +80,49 @@ impl Aggregator { deadline_seconds: config.aggregated_block_execute_deadline, max_allowed_lag: Some(config.timestamp_criteria_max_allowed_lag), }), - Box::from(ExecuteGasCriterion::new(config.max_aggregated_tx_gas)), + Box::from(L1GasCriterion::new( + config.max_aggregated_tx_gas, + GasCriterionKind::Execute, + )), ] }; - if config.max_aggregated_blocks_to_commit > 1 { - tracing::warn!( - "config.max_aggregated_blocks_to_commit is set to {} but \ - aggregator does not support aggregating commit operations anymore", - config.max_aggregated_blocks_to_commit - ); - } - - Self { - commit_criteria: vec![Box::from(NumberCriterion { + // It only makes sense to aggregate commit operation when validium chain settles to L1. + let commit_criteria: Vec> = if settlement_mode + == SettlementMode::SettlesToL1 + && commitment_mode == L1BatchCommitmentMode::Validium + { + vec![ + Box::from(NumberCriterion { + op: AggregatedActionType::Commit, + limit: config.max_aggregated_blocks_to_commit, + }), + Box::from(TimestampDeadlineCriterion { + op: AggregatedActionType::Commit, + deadline_seconds: config.aggregated_block_commit_deadline, + max_allowed_lag: Some(config.timestamp_criteria_max_allowed_lag), + }), + Box::from(L1GasCriterion::new( + config.max_aggregated_tx_gas, + GasCriterionKind::CommitValidium, + )), + ] + } else { + if config.max_aggregated_blocks_to_commit > 1 { + tracing::warn!( + "config.max_aggregated_blocks_to_commit is set to {} but \ + aggregator does not support aggregating commit operations anymore", + config.max_aggregated_blocks_to_commit + ); + } + vec![Box::from(NumberCriterion { op: AggregatedActionType::Commit, limit: 1, - })], + })] + }; + + Self { + commit_criteria, proof_criteria: vec![Box::from(NumberCriterion { op: AggregatedActionType::PublishProofOnchain, limit: 1, diff --git a/core/node/eth_sender/src/eth_tx_aggregator.rs b/core/node/eth_sender/src/eth_tx_aggregator.rs index cbd1e326d66b..d507697df2db 100644 --- a/core/node/eth_sender/src/eth_tx_aggregator.rs +++ b/core/node/eth_sender/src/eth_tx_aggregator.rs @@ -14,7 +14,7 @@ use zksync_l1_contract_interface::{ use zksync_shared_metrics::BlockL1Stage; use zksync_types::{ aggregated_operations::AggregatedActionType, - commitment::{L1BatchWithMetadata, SerializeCommitment}, + commitment::{L1BatchCommitmentMode, L1BatchWithMetadata, SerializeCommitment}, eth_sender::{EthTx, EthTxBlobSidecar, EthTxBlobSidecarV1, SidecarBlobV1}, ethabi::{Function, Token}, l2_to_l1_log::UserL2ToL1Log, @@ -28,7 +28,7 @@ use zksync_types::{ use super::aggregated_operations::AggregatedOperation; use crate::{ metrics::{PubdataKind, METRICS}, - publish_criterion::ExecuteGasCriterion, + publish_criterion::L1GasCriterion, zksync_functions::ZkSyncFunctions, Aggregator, EthSenderError, }; @@ -610,14 +610,17 @@ impl EthTxAggregator { self.encode_aggregated_op(aggregated_op, contracts_are_pre_shared_bridge); let l1_batch_number_range = aggregated_op.l1_batch_range(); - let eth_tx_predicted_gas = match (op_type, is_gateway) { - (AggregatedActionType::Execute, false) => Some( - ExecuteGasCriterion::total_execute_gas_amount( + let eth_tx_predicted_gas = match (op_type, is_gateway, self.aggregator.mode()) { + (AggregatedActionType::Execute, false, _) => Some( + L1GasCriterion::total_execute_gas_amount( &mut transaction, l1_batch_number_range.clone(), ) .await, ), + (AggregatedActionType::Commit, false, L1BatchCommitmentMode::Validium) => Some( + L1GasCriterion::total_validium_commit_gas_amount(l1_batch_number_range.clone()), + ), _ => None, }; diff --git a/core/node/eth_sender/src/publish_criterion.rs b/core/node/eth_sender/src/publish_criterion.rs index 36052fe5976a..28ca44697bb6 100644 --- a/core/node/eth_sender/src/publish_criterion.rs +++ b/core/node/eth_sender/src/publish_criterion.rs @@ -122,17 +122,42 @@ impl L1BatchPublishCriterion for TimestampDeadlineCriterion { } } +#[derive(Debug, Clone, Copy)] +pub enum GasCriterionKind { + CommitValidium, + Execute, +} + +impl From for AggregatedActionType { + fn from(value: GasCriterionKind) -> Self { + match value { + GasCriterionKind::CommitValidium => AggregatedActionType::Commit, + GasCriterionKind::Execute => AggregatedActionType::Execute, + } + } +} + #[derive(Debug)] -pub struct ExecuteGasCriterion { +pub struct L1GasCriterion { pub gas_limit: u32, + pub kind: GasCriterionKind, } -impl ExecuteGasCriterion { - /// Base cost of processing aggregated `Execute` operation. +impl L1GasCriterion { + /// Base gas cost of processing aggregated `Execute` operation. + /// It's applicable iff SL is Ethereum. const AGGR_L1_BATCH_EXECUTE_BASE_COST: u32 = 241_000; - pub fn new(gas_limit: u32) -> ExecuteGasCriterion { - ExecuteGasCriterion { gas_limit } + /// Base gas cost of processing aggregated `Commit` operation. + /// It's applicable iff SL is Ethereum. + const AGGR_L1_BATCH_COMMIT_BASE_COST: u32 = 242_000; + + /// Additional gas cost of processing `Commit` operation per batch. + /// It's applicable iff SL is Ethereum. + pub const L1_BATCH_COMMIT_BASE_COST: u32 = 31_000; + + pub fn new(gas_limit: u32, kind: GasCriterionKind) -> L1GasCriterion { + L1GasCriterion { gas_limit, kind } } pub async fn total_execute_gas_amount( @@ -148,6 +173,14 @@ impl ExecuteGasCriterion { total } + pub fn total_validium_commit_gas_amount( + batch_numbers: ops::RangeInclusive, + ) -> u32 { + Self::AGGR_L1_BATCH_COMMIT_BASE_COST + + (batch_numbers.end().0 - batch_numbers.start().0 + 1) + * Self::L1_BATCH_COMMIT_BASE_COST + } + async fn get_execute_gas_amount( storage: &mut Connection<'_, Core>, batch_number: L1BatchNumber, @@ -164,7 +197,7 @@ impl ExecuteGasCriterion { } #[async_trait] -impl L1BatchPublishCriterion for ExecuteGasCriterion { +impl L1BatchPublishCriterion for L1GasCriterion { fn name(&self) -> &'static str { "gas_limit" } @@ -175,17 +208,25 @@ impl L1BatchPublishCriterion for ExecuteGasCriterion { consecutive_l1_batches: &[L1BatchWithMetadata], _last_sealed_l1_batch: L1BatchNumber, ) -> Option { + let aggr_cost = match self.kind { + GasCriterionKind::Execute => Self::AGGR_L1_BATCH_EXECUTE_BASE_COST, + GasCriterionKind::CommitValidium => Self::AGGR_L1_BATCH_COMMIT_BASE_COST, + }; assert!( - self.gas_limit > Self::AGGR_L1_BATCH_EXECUTE_BASE_COST, + self.gas_limit > aggr_cost, "Config max gas cost for operations is too low" ); // We're not sure our predictions are accurate, so it's safer to lower the gas limit by 10% - let mut gas_left = - (self.gas_limit as f64 * 0.9).round() as u32 - Self::AGGR_L1_BATCH_EXECUTE_BASE_COST; + let mut gas_left = (self.gas_limit as f64 * 0.9).round() as u32 - aggr_cost; let mut last_l1_batch = None; for (index, l1_batch) in consecutive_l1_batches.iter().enumerate() { - let batch_gas = Self::get_execute_gas_amount(storage, l1_batch.header.number).await; + let batch_gas = match self.kind { + GasCriterionKind::Execute => { + Self::get_execute_gas_amount(storage, l1_batch.header.number).await + } + GasCriterionKind::CommitValidium => Self::L1_BATCH_COMMIT_BASE_COST, + }; if batch_gas >= gas_left { if index == 0 { panic!( @@ -201,14 +242,15 @@ impl L1BatchPublishCriterion for ExecuteGasCriterion { } if let Some(last_l1_batch) = last_l1_batch { + let op: AggregatedActionType = self.kind.into(); let first_l1_batch_number = consecutive_l1_batches.first().unwrap().header.number.0; tracing::debug!( "`gas_limit` publish criterion (gas={}) triggered for op {} with L1 batch range {:?}", self.gas_limit - gas_left, - AggregatedActionType::Execute, + op, first_l1_batch_number..=last_l1_batch.0 ); - METRICS.block_aggregation_reason[&(AggregatedActionType::Execute, "gas").into()].inc(); + METRICS.block_aggregation_reason[&(op, "gas").into()].inc(); } last_l1_batch }