From dde0fc4b469474525fd5e4fd1594c3710d6d91f5 Mon Sep 17 00:00:00 2001 From: AnastasiiaVashchuk <72273339+AnastasiiaVashchuk@users.noreply.github.com> Date: Wed, 5 Jun 2024 16:59:05 +0300 Subject: [PATCH] feat: Add metrics for transaction execution result in state keeper (#2021) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ ## Why ❔ ## Checklist - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zk fmt` and `zk lint`. - [ ] Spellcheck has been run via `zk spellcheck`. --- core/node/node_sync/src/external_io.rs | 6 +- core/node/state_keeper/src/io/mempool.rs | 23 ++++-- core/node/state_keeper/src/io/mod.rs | 4 +- core/node/state_keeper/src/keeper.rs | 21 +++-- core/node/state_keeper/src/metrics.rs | 58 ++++++++++++- .../src/seal_criteria/criteria/gas.rs | 8 +- .../criteria/gas_for_batch_tip.rs | 6 +- .../criteria/geometry_seal_criteria.rs | 9 +- .../seal_criteria/criteria/pubdata_bytes.rs | 7 +- .../criteria/tx_encoding_size.rs | 11 ++- .../state_keeper/src/seal_criteria/mod.rs | 82 ++++++++++++++++++- .../src/testonly/test_batch_executor.rs | 18 ++-- core/node/state_keeper/src/tests/mod.rs | 10 ++- .../src/updates/l2_block_updates.rs | 16 +++- 14 files changed, 217 insertions(+), 62 deletions(-) diff --git a/core/node/node_sync/src/external_io.rs b/core/node/node_sync/src/external_io.rs index 559c377e4537..690d38f620a0 100644 --- a/core/node/node_sync/src/external_io.rs +++ b/core/node/node_sync/src/external_io.rs @@ -12,7 +12,7 @@ use zksync_state_keeper::{ L1BatchParams, L2BlockParams, PendingBatchData, StateKeeperIO, }, metrics::KEEPER_METRICS, - seal_criteria::IoSealCriteria, + seal_criteria::{IoSealCriteria, UnexecutableReason}, updates::UpdatesManager, }; use zksync_types::{ @@ -304,10 +304,10 @@ impl StateKeeperIO for ExternalIO { anyhow::bail!("Rollback requested. Transaction hash: {:?}", tx.hash()); } - async fn reject(&mut self, tx: &Transaction, error: &str) -> anyhow::Result<()> { + async fn reject(&mut self, tx: &Transaction, reason: UnexecutableReason) -> anyhow::Result<()> { // We are replaying the already executed transactions so no rejections are expected to occur. anyhow::bail!( - "Requested rejection of transaction {:?} because of the following error: {error}. \ + "Requested rejection of transaction {:?} because of the following error: {reason}. \ This is not supported on external node", tx.hash() ); diff --git a/core/node/state_keeper/src/io/mempool.rs b/core/node/state_keeper/src/io/mempool.rs index 8d44e38cc6ee..fcaf85573ef7 100644 --- a/core/node/state_keeper/src/io/mempool.rs +++ b/core/node/state_keeper/src/io/mempool.rs @@ -29,7 +29,9 @@ use crate::{ }, mempool_actor::l2_tx_filter, metrics::KEEPER_METRICS, - seal_criteria::{IoSealCriteria, L2BlockMaxPayloadSizeSealer, TimeoutSealer}, + seal_criteria::{ + IoSealCriteria, L2BlockMaxPayloadSizeSealer, TimeoutSealer, UnexecutableReason, + }, updates::UpdatesManager, MempoolGuard, }; @@ -245,7 +247,8 @@ impl StateKeeperIO for MempoolIO { tx.hash(), tx.gas_limit() ); - self.reject(&tx, &Halt::TooBigGasLimit.to_string()).await?; + self.reject(&tx, UnexecutableReason::Halt(Halt::TooBigGasLimit)) + .await?; continue; } return Ok(Some(tx)); @@ -265,10 +268,14 @@ impl StateKeeperIO for MempoolIO { Ok(()) } - async fn reject(&mut self, rejected: &Transaction, error: &str) -> anyhow::Result<()> { + async fn reject( + &mut self, + rejected: &Transaction, + reason: UnexecutableReason, + ) -> anyhow::Result<()> { anyhow::ensure!( !rejected.is_l1(), - "L1 transactions should not be rejected: {error}" + "L1 transactions should not be rejected: {reason}" ); // Reset the nonces in the mempool, but don't insert the transaction back. @@ -276,14 +283,16 @@ impl StateKeeperIO for MempoolIO { // Mark tx as rejected in the storage. let mut storage = self.pool.connection_tagged("state_keeper").await?; - KEEPER_METRICS.rejected_transactions.inc(); + + KEEPER_METRICS.inc_rejected_txs(reason.as_metric_label()); + tracing::warn!( - "Transaction {} is rejected with error: {error}", + "Transaction {} is rejected with error: {reason}", rejected.hash() ); storage .transactions_dal() - .mark_tx_as_rejected(rejected.hash(), &format!("rejected: {error}")) + .mark_tx_as_rejected(rejected.hash(), &format!("rejected: {reason}")) .await?; Ok(()) } diff --git a/core/node/state_keeper/src/io/mod.rs b/core/node/state_keeper/src/io/mod.rs index 8cdfbd59121f..80ba8e59e2b7 100644 --- a/core/node/state_keeper/src/io/mod.rs +++ b/core/node/state_keeper/src/io/mod.rs @@ -14,7 +14,7 @@ pub use self::{ output_handler::{OutputHandler, StateKeeperOutputHandler}, persistence::{L2BlockSealerTask, StateKeeperPersistence, TreeWritesPersistence}, }; -use super::seal_criteria::IoSealCriteria; +use super::seal_criteria::{IoSealCriteria, UnexecutableReason}; pub mod common; pub(crate) mod mempool; @@ -136,7 +136,7 @@ pub trait StateKeeperIO: 'static + Send + Sync + fmt::Debug + IoSealCriteria { /// Marks the transaction as "not executed", so it can be retrieved from the IO again. async fn rollback(&mut self, tx: Transaction) -> anyhow::Result<()>; /// Marks the transaction as "rejected", e.g. one that is not correct and can't be executed. - async fn reject(&mut self, tx: &Transaction, error: &str) -> anyhow::Result<()>; + async fn reject(&mut self, tx: &Transaction, reason: UnexecutableReason) -> anyhow::Result<()>; /// Loads base system contracts with the specified version. async fn load_base_system_contracts( diff --git a/core/node/state_keeper/src/keeper.rs b/core/node/state_keeper/src/keeper.rs index 6e315ddd6c09..686e0b148665 100644 --- a/core/node/state_keeper/src/keeper.rs +++ b/core/node/state_keeper/src/keeper.rs @@ -23,6 +23,7 @@ use super::{ updates::UpdatesManager, utils::gas_count_from_writes, }; +use crate::seal_criteria::UnexecutableReason; /// Amount of time to block on waiting for some resource. The exact value is not really important, /// we only need it to not block on waiting indefinitely and be able to process cancellation requests. @@ -581,7 +582,7 @@ impl ZkSyncStateKeeper { format!("failed rolling back transaction {tx_hash:?} in batch executor") })?; self.io - .reject(&tx, reason) + .reject(&tx, reason.clone()) .await .with_context(|| format!("cannot reject transaction {tx_hash:?}"))?; } @@ -690,23 +691,29 @@ impl ZkSyncStateKeeper { | TxExecutionResult::RejectedByVm { reason: Halt::NotEnoughGasProvided, } => { - let error_message = match &exec_result { - TxExecutionResult::BootloaderOutOfGasForTx => "bootloader_tx_out_of_gas", + let (reason, criterion) = match &exec_result { + TxExecutionResult::BootloaderOutOfGasForTx => ( + UnexecutableReason::BootloaderOutOfGas, + "bootloader_tx_out_of_gas", + ), TxExecutionResult::RejectedByVm { reason: Halt::NotEnoughGasProvided, - } => "not_enough_gas_provided_to_start_tx", + } => ( + UnexecutableReason::NotEnoughGasProvided, + "not_enough_gas_provided_to_start_tx", + ), _ => unreachable!(), }; let resolution = if is_first_tx { - SealResolution::Unexecutable(error_message.to_string()) + SealResolution::Unexecutable(reason) } else { SealResolution::ExcludeAndSeal }; - AGGREGATION_METRICS.inc(error_message, &resolution); + AGGREGATION_METRICS.inc(criterion, &resolution); resolution } TxExecutionResult::RejectedByVm { reason } => { - SealResolution::Unexecutable(reason.to_string()) + UnexecutableReason::Halt(reason.clone()).into() } TxExecutionResult::Success { tx_result, diff --git a/core/node/state_keeper/src/metrics.rs b/core/node/state_keeper/src/metrics.rs index d1a7269860ff..5a79425ea4f7 100644 --- a/core/node/state_keeper/src/metrics.rs +++ b/core/node/state_keeper/src/metrics.rs @@ -5,7 +5,7 @@ use std::{ time::Duration, }; -use multivm::interface::VmExecutionResultAndLogs; +use multivm::interface::{VmExecutionResultAndLogs, VmRevertReason}; use vise::{ Buckets, Counter, EncodeLabelSet, EncodeLabelValue, Family, Gauge, Histogram, LatencyObserver, Metrics, @@ -30,6 +30,20 @@ pub enum TxExecutionType { L2, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue)] +#[metrics(rename_all = "snake_case")] +pub enum TxExecutionStatus { + Success, + Rejected, + Reverted, +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash, EncodeLabelSet)] +pub struct TxExecutionResult { + status: TxExecutionStatus, + reason: Option<&'static str>, +} + impl TxExecutionType { pub fn from_is_l1(is_l1: bool) -> TxExecutionType { match is_l1 { @@ -57,8 +71,8 @@ pub struct StateKeeperMetrics { /// Latency of the state keeper getting a transaction from the mempool. #[metrics(buckets = Buckets::LATENCIES)] pub get_tx_from_mempool: Histogram, - /// Number of transactions rejected by the state keeper. - pub rejected_transactions: Counter, + /// Number of transactions completed with a specific result. + pub tx_execution_result: Family, /// Time spent waiting for the hash of a previous L1 batch. #[metrics(buckets = Buckets::LATENCIES)] pub wait_for_prev_hash_time: Histogram, @@ -77,6 +91,44 @@ pub struct StateKeeperMetrics { pub blob_base_fee_too_high: Counter, } +fn vm_revert_reason_as_metric_label(reason: &VmRevertReason) -> &'static str { + match reason { + VmRevertReason::General { .. } => "General", + VmRevertReason::InnerTxError => "InnerTxError", + VmRevertReason::VmError => "VmError", + VmRevertReason::Unknown { .. } => "Unknown", + } +} + +impl StateKeeperMetrics { + pub fn inc_rejected_txs(&self, reason: &'static str) { + let result = TxExecutionResult { + status: TxExecutionStatus::Rejected, + reason: Some(reason), + }; + + self.tx_execution_result[&result].inc(); + } + + pub fn inc_succeeded_txs(&self) { + let result = TxExecutionResult { + status: TxExecutionStatus::Success, + reason: None, + }; + + self.tx_execution_result[&result].inc(); + } + + pub fn inc_reverted_txs(&self, reason: &VmRevertReason) { + let result = TxExecutionResult { + status: TxExecutionStatus::Reverted, + reason: Some(vm_revert_reason_as_metric_label(reason)), + }; + + self.tx_execution_result[&result].inc(); + } +} + #[vise::register] pub static KEEPER_METRICS: vise::Global = vise::Global::new(); diff --git a/core/node/state_keeper/src/seal_criteria/criteria/gas.rs b/core/node/state_keeper/src/seal_criteria/criteria/gas.rs index 6677915e4e1f..a97ac6ede353 100644 --- a/core/node/state_keeper/src/seal_criteria/criteria/gas.rs +++ b/core/node/state_keeper/src/seal_criteria/criteria/gas.rs @@ -1,7 +1,9 @@ use zksync_types::ProtocolVersionId; use crate::{ - seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig}, + seal_criteria::{ + SealCriterion, SealData, SealResolution, StateKeeperConfig, UnexecutableReason, + }, utils::new_block_gas_count, }; @@ -30,7 +32,7 @@ impl SealCriterion for GasCriterion { (config.max_single_tx_gas as f64 * config.close_block_at_gas_percentage).round() as u32; if (tx_data.gas_count + new_block_gas_count()).any_field_greater_than(tx_bound) { - SealResolution::Unexecutable("Transaction requires too much gas".into()) + UnexecutableReason::TooMuchGas.into() } else if block_data .gas_count .any_field_greater_than(config.max_single_tx_gas) @@ -103,7 +105,7 @@ mod tests { ); assert_eq!( huge_transaction_resolution, - SealResolution::Unexecutable("Transaction requires too much gas".into()) + UnexecutableReason::TooMuchGas.into() ); // Check criterion workflow diff --git a/core/node/state_keeper/src/seal_criteria/criteria/gas_for_batch_tip.rs b/core/node/state_keeper/src/seal_criteria/criteria/gas_for_batch_tip.rs index ff655880185b..8c15d04d0833 100644 --- a/core/node/state_keeper/src/seal_criteria/criteria/gas_for_batch_tip.rs +++ b/core/node/state_keeper/src/seal_criteria/criteria/gas_for_batch_tip.rs @@ -1,7 +1,9 @@ use multivm::utils::gas_bootloader_batch_tip_overhead; use zksync_types::ProtocolVersionId; -use crate::seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig}; +use crate::seal_criteria::{ + SealCriterion, SealData, SealResolution, StateKeeperConfig, UnexecutableReason, +}; /// Checks whether we should exclude the transaction because we don't have enough gas for batch tip. #[derive(Debug)] @@ -22,7 +24,7 @@ impl SealCriterion for GasForBatchTipCriterion { if tx_data.gas_remaining < batch_tip_overhead { if is_tx_first { - SealResolution::Unexecutable("not_enough_gas_for_batch_tip".to_string()) + UnexecutableReason::OutOfGasForBatchTip.into() } else { SealResolution::ExcludeAndSeal } diff --git a/core/node/state_keeper/src/seal_criteria/criteria/geometry_seal_criteria.rs b/core/node/state_keeper/src/seal_criteria/criteria/geometry_seal_criteria.rs index 91a7ce148cbb..3e800f18e2d9 100644 --- a/core/node/state_keeper/src/seal_criteria/criteria/geometry_seal_criteria.rs +++ b/core/node/state_keeper/src/seal_criteria/criteria/geometry_seal_criteria.rs @@ -5,7 +5,7 @@ use zksync_config::configs::chain::StateKeeperConfig; use zksync_types::ProtocolVersionId; // Local uses -use crate::seal_criteria::{SealCriterion, SealData, SealResolution}; +use crate::seal_criteria::{SealCriterion, SealData, SealResolution, UnexecutableReason}; // Collected vm execution metrics should fit into geometry limits. // Otherwise witness generation will fail and proof won't be generated. @@ -52,7 +52,7 @@ impl SealCriterion for CircuitsCriterion { let used_circuits_batch = block_data.execution_metrics.circuit_statistic.total(); if used_circuits_tx + batch_tip_circuit_overhead >= reject_bound { - SealResolution::Unexecutable("ZK proof cannot be generated for a transaction".into()) + UnexecutableReason::ProofWillFail.into() } else if used_circuits_batch + batch_tip_circuit_overhead >= config.max_circuits_per_batch { SealResolution::ExcludeAndSeal @@ -162,10 +162,7 @@ mod tests { protocol_version, ); - assert_eq!( - block_resolution, - SealResolution::Unexecutable("ZK proof cannot be generated for a transaction".into()) - ); + assert_eq!(block_resolution, UnexecutableReason::ProofWillFail.into()); } #[test] diff --git a/core/node/state_keeper/src/seal_criteria/criteria/pubdata_bytes.rs b/core/node/state_keeper/src/seal_criteria/criteria/pubdata_bytes.rs index 2e17bbb6d77c..e021cc127be7 100644 --- a/core/node/state_keeper/src/seal_criteria/criteria/pubdata_bytes.rs +++ b/core/node/state_keeper/src/seal_criteria/criteria/pubdata_bytes.rs @@ -1,7 +1,9 @@ use multivm::utils::execution_metrics_bootloader_batch_tip_overhead; use zksync_types::ProtocolVersionId; -use crate::seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig}; +use crate::seal_criteria::{ + SealCriterion, SealData, SealResolution, StateKeeperConfig, UnexecutableReason, +}; #[derive(Debug)] pub struct PubDataBytesCriterion { @@ -41,8 +43,7 @@ impl SealCriterion for PubDataBytesCriterion { if tx_size + execution_metrics_bootloader_batch_tip_overhead(protocol_version.into()) > reject_bound as usize { - let message = "Transaction cannot be sent to L1 due to pubdata limits"; - SealResolution::Unexecutable(message.into()) + UnexecutableReason::PubdataLimit.into() } else if block_size + execution_metrics_bootloader_batch_tip_overhead(protocol_version.into()) > max_pubdata_per_l1_batch diff --git a/core/node/state_keeper/src/seal_criteria/criteria/tx_encoding_size.rs b/core/node/state_keeper/src/seal_criteria/criteria/tx_encoding_size.rs index 03c2c3e14c84..13a7f0b0a757 100644 --- a/core/node/state_keeper/src/seal_criteria/criteria/tx_encoding_size.rs +++ b/core/node/state_keeper/src/seal_criteria/criteria/tx_encoding_size.rs @@ -1,7 +1,9 @@ use multivm::utils::get_bootloader_encoding_space; use zksync_types::ProtocolVersionId; -use crate::seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig}; +use crate::seal_criteria::{ + SealCriterion, SealData, SealResolution, StateKeeperConfig, UnexecutableReason, +}; #[derive(Debug)] pub struct TxEncodingSizeCriterion; @@ -26,8 +28,7 @@ impl SealCriterion for TxEncodingSizeCriterion { .round(); if tx_data.cumulative_size > reject_bound as usize { - let message = "Transaction cannot be included due to large encoding size"; - SealResolution::Unexecutable(message.into()) + UnexecutableReason::LargeEncodingSize.into() } else if block_data.cumulative_size > bootloader_tx_encoding_space as usize { SealResolution::ExcludeAndSeal } else if block_data.cumulative_size > include_and_seal_bound as usize { @@ -83,9 +84,7 @@ mod tests { ); assert_eq!( unexecutable_resolution, - SealResolution::Unexecutable( - "Transaction cannot be included due to large encoding size".into() - ) + UnexecutableReason::LargeEncodingSize.into() ); let exclude_and_seal_resolution = criterion.should_seal( diff --git a/core/node/state_keeper/src/seal_criteria/mod.rs b/core/node/state_keeper/src/seal_criteria/mod.rs index 51ad1c4ad906..c1c9e59e49c4 100644 --- a/core/node/state_keeper/src/seal_criteria/mod.rs +++ b/core/node/state_keeper/src/seal_criteria/mod.rs @@ -12,7 +12,7 @@ use std::fmt; -use multivm::vm_latest::TransactionVmExt; +use multivm::{interface::Halt, vm_latest::TransactionVmExt}; use zksync_config::configs::chain::StateKeeperConfig; use zksync_types::{ block::BlockGasCount, @@ -33,6 +33,84 @@ use super::{ utils::{gas_count_from_tx_and_metrics, gas_count_from_writes}, }; +fn halt_as_metric_label(halt: &Halt) -> &'static str { + match halt { + Halt::ValidationFailed(_) => "ValidationFailed", + Halt::PaymasterValidationFailed(_) => "PaymasterValidationFailed", + Halt::PrePaymasterPreparationFailed(_) => "PrePaymasterPreparationFailed", + Halt::PayForTxFailed(_) => "PayForTxFailed", + Halt::FailedToMarkFactoryDependencies(_) => "FailedToMarkFactoryDependencies", + Halt::FailedToChargeFee(_) => "FailedToChargeFee", + Halt::FromIsNotAnAccount => "FromIsNotAnAccount", + Halt::InnerTxError => "InnerTxError", + Halt::Unknown(_) => "Unknown", + Halt::UnexpectedVMBehavior(_) => "UnexpectedVMBehavior", + Halt::BootloaderOutOfGas => "BootloaderOutOfGas", + Halt::ValidationOutOfGas => "ValidationOutOfGas", + Halt::TooBigGasLimit => "TooBigGasLimit", + Halt::NotEnoughGasProvided => "NotEnoughGasProvided", + Halt::MissingInvocationLimitReached => "MissingInvocationLimitReached", + Halt::FailedToSetL2Block(_) => "FailedToSetL2Block", + Halt::FailedToAppendTransactionToL2Block(_) => "FailedToAppendTransactionToL2Block", + Halt::VMPanic => "VMPanic", + Halt::TracerCustom(_) => "TracerCustom", + Halt::FailedToPublishCompressedBytecodes => "FailedToPublishCompressedBytecodes", + } +} + +#[derive(Debug, Clone, PartialEq)] +pub enum UnexecutableReason { + Halt(Halt), + TxEncodingSize, + LargeEncodingSize, + PubdataLimit, + ProofWillFail, + TooMuchGas, + OutOfGasForBatchTip, + BootloaderOutOfGas, + NotEnoughGasProvided, +} + +impl UnexecutableReason { + pub fn as_metric_label(&self) -> &'static str { + match self { + UnexecutableReason::Halt(halt) => halt_as_metric_label(halt), + UnexecutableReason::TxEncodingSize => "TxEncodingSize", + UnexecutableReason::LargeEncodingSize => "LargeEncodingSize", + UnexecutableReason::PubdataLimit => "PubdataLimit", + UnexecutableReason::ProofWillFail => "ProofWillFail", + UnexecutableReason::TooMuchGas => "TooMuchGas", + UnexecutableReason::OutOfGasForBatchTip => "OutOfGasForBatchTip", + UnexecutableReason::BootloaderOutOfGas => "BootloaderOutOfGas", + UnexecutableReason::NotEnoughGasProvided => "NotEnoughGasProvided", + } + } +} + +impl From for SealResolution { + fn from(reason: UnexecutableReason) -> Self { + SealResolution::Unexecutable(reason) + } +} + +impl fmt::Display for UnexecutableReason { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + UnexecutableReason::Halt(halt) => write!(f, "{}", halt), + UnexecutableReason::TxEncodingSize => write!(f, "Transaction encoding size is too big"), + UnexecutableReason::LargeEncodingSize => { + write!(f, "Transaction encoding size is too big") + } + UnexecutableReason::PubdataLimit => write!(f, "Pubdata limit reached"), + UnexecutableReason::ProofWillFail => write!(f, "Proof will fail"), + UnexecutableReason::TooMuchGas => write!(f, "Too much gas"), + UnexecutableReason::OutOfGasForBatchTip => write!(f, "Out of gas for batch tip"), + UnexecutableReason::BootloaderOutOfGas => write!(f, "Bootloader out of gas"), + UnexecutableReason::NotEnoughGasProvided => write!(f, "Not enough gas provided"), + } + } +} + /// Reported decision regarding block sealing. #[derive(Debug, Clone, PartialEq)] pub enum SealResolution { @@ -52,7 +130,7 @@ pub enum SealResolution { /// if the block will consist of it solely. Such a transaction must be rejected. /// /// Contains a reason for why transaction was considered unexecutable. - Unexecutable(String), + Unexecutable(UnexecutableReason), } impl SealResolution { diff --git a/core/node/state_keeper/src/testonly/test_batch_executor.rs b/core/node/state_keeper/src/testonly/test_batch_executor.rs index 5b1bf3ceeba8..4539633174a8 100644 --- a/core/node/state_keeper/src/testonly/test_batch_executor.rs +++ b/core/node/state_keeper/src/testonly/test_batch_executor.rs @@ -29,7 +29,7 @@ use zksync_types::{ use crate::{ batch_executor::{BatchExecutor, BatchExecutorHandle, Command, TxExecutionResult}, io::{IoCursor, L1BatchParams, L2BlockParams, PendingBatchData, StateKeeperIO}, - seal_criteria::{IoSealCriteria, SequencerSealer}, + seal_criteria::{IoSealCriteria, SequencerSealer, UnexecutableReason}, testonly::{default_vm_batch_result, successful_exec, BASE_SYSTEM_CONTRACTS}, types::ExecutionMetricsForCriteria, updates::UpdatesManager, @@ -129,7 +129,7 @@ impl TestScenario { mut self, description: &'static str, tx: Transaction, - err: Option, + err: UnexecutableReason, ) -> Self { self.actions .push_back(ScenarioItem::Reject(description, tx, err)); @@ -283,7 +283,7 @@ enum ScenarioItem { IncrementProtocolVersion(&'static str), Tx(&'static str, Transaction, TxExecutionResult), Rollback(&'static str, Transaction), - Reject(&'static str, Transaction, Option), + Reject(&'static str, Transaction, UnexecutableReason), L2BlockSeal( &'static str, Option>, @@ -761,20 +761,14 @@ impl StateKeeperIO for TestIO { Ok(()) } - async fn reject(&mut self, tx: &Transaction, error: &str) -> anyhow::Result<()> { + async fn reject(&mut self, tx: &Transaction, reason: UnexecutableReason) -> anyhow::Result<()> { let action = self.pop_next_item("reject"); let ScenarioItem::Reject(_, expected_tx, expected_err) = action else { panic!("Unexpected action: {:?}", action); }; assert_eq!(tx, &expected_tx, "Incorrect transaction has been rejected"); - if let Some(expected_err) = expected_err { - assert!( - error.contains(&expected_err), - "Transaction was rejected with an unexpected error. Expected part was {}, but the actual error was {}", - expected_err, - error - ); - } + assert_eq!(reason, expected_err); + self.skipping_txs = false; Ok(()) } diff --git a/core/node/state_keeper/src/tests/mod.rs b/core/node/state_keeper/src/tests/mod.rs index 18d25faf4a4c..b5560605eedf 100644 --- a/core/node/state_keeper/src/tests/mod.rs +++ b/core/node/state_keeper/src/tests/mod.rs @@ -8,7 +8,7 @@ use std::{ use multivm::{ interface::{ - ExecutionResult, L1BatchEnv, L2BlockEnv, Refunds, SystemEnv, TxExecutionMode, + ExecutionResult, Halt, L1BatchEnv, L2BlockEnv, Refunds, SystemEnv, TxExecutionMode, VmExecutionResultAndLogs, VmExecutionStatistics, }, vm_latest::{constants::BATCH_COMPUTATIONAL_GAS_LIMIT, VmExecutionLogs}, @@ -32,7 +32,7 @@ use crate::{ keeper::POLL_WAIT_DURATION, seal_criteria::{ criteria::{GasCriterion, SlotsCriterion}, - SequencerSealer, + SequencerSealer, UnexecutableReason, }, testonly::{ successful_exec, @@ -328,7 +328,11 @@ async fn rejected_tx() { TestScenario::new() .seal_l2_block_when(|updates| updates.l2_block.executed_transactions.len() == 1) .next_tx("Rejected tx", rejected_tx.clone(), rejected_exec()) - .tx_rejected("Tx got rejected", rejected_tx, None) + .tx_rejected( + "Tx got rejected", + rejected_tx, + UnexecutableReason::Halt(Halt::InnerTxError), + ) .next_tx("Successful tx", random_tx(2), successful_exec()) .l2_block_sealed("L2 block with successful tx") .next_tx("Second successful tx", random_tx(3), successful_exec()) diff --git a/core/node/state_keeper/src/updates/l2_block_updates.rs b/core/node/state_keeper/src/updates/l2_block_updates.rs index a74d94be30ef..efc09472fb0c 100644 --- a/core/node/state_keeper/src/updates/l2_block_updates.rs +++ b/core/node/state_keeper/src/updates/l2_block_updates.rs @@ -14,6 +14,8 @@ use zksync_types::{ }; use zksync_utils::bytecode::{hash_bytecode, CompressedBytecodeInfo}; +use crate::metrics::KEEPER_METRICS; + #[derive(Debug, Clone, PartialEq)] pub struct L2BlockUpdates { pub executed_transactions: Vec, @@ -104,9 +106,17 @@ impl L2BlockUpdates { }; let revert_reason = match &tx_execution_result.result { - ExecutionResult::Success { .. } => None, - ExecutionResult::Revert { output } => Some(output.to_string()), - ExecutionResult::Halt { reason } => Some(reason.to_string()), + ExecutionResult::Success { .. } => { + KEEPER_METRICS.inc_succeeded_txs(); + None + } + ExecutionResult::Revert { output } => { + KEEPER_METRICS.inc_reverted_txs(output); + Some(output.to_string()) + } + ExecutionResult::Halt { .. } => { + unreachable!("Tx that is added to `UpdatesManager` must not have Halted status") + } }; // Get transaction factory deps