Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add metrics for transaction execution result in state keeper #2021

Merged
merged 19 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions core/lib/multivm/src/interface/types/errors/halt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,34 @@ pub enum Halt {
FailedToPublishCompressedBytecodes,
}

impl Halt {
pub fn to_metrics_friendly_string(&self) -> String {
AnastasiiaVashchuk marked this conversation as resolved.
Show resolved Hide resolved
let result = match self {
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",
};
result.to_string()
}
}

impl Display for Halt {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Expand Down
11 changes: 11 additions & 0 deletions core/lib/multivm/src/interface/types/errors/vm_revert_reason.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ impl VmRevertReason {
}
}

pub fn to_metrics_friendly_string(&self) -> String {
let result = match self {
VmRevertReason::General { .. } => "General",
VmRevertReason::InnerTxError => "InnerTxError",
VmRevertReason::VmError => "VmError",
VmRevertReason::Unknown { .. } => "Unknown",
};

result.to_string()
}

pub fn encoded_data(&self) -> Vec<u8> {
match self {
VmRevertReason::Unknown { data, .. } => data.clone(),
Expand Down
4 changes: 2 additions & 2 deletions core/node/node_sync/src/external_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use zksync_state_keeper::{
L1BatchParams, L2BlockParams, PendingBatchData, StateKeeperIO,
},
metrics::KEEPER_METRICS,
seal_criteria::IoSealCriteria,
seal_criteria::{IoSealCriteria, Reason},
updates::UpdatesManager,
};
use zksync_types::{
Expand Down Expand Up @@ -303,7 +303,7 @@ 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, error: Reason) -> 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}. \
Expand Down
10 changes: 6 additions & 4 deletions core/node/state_keeper/src/io/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
},
mempool_actor::l2_tx_filter,
metrics::KEEPER_METRICS,
seal_criteria::{IoSealCriteria, L2BlockMaxPayloadSizeSealer, TimeoutSealer},
seal_criteria::{IoSealCriteria, L2BlockMaxPayloadSizeSealer, Reason, TimeoutSealer},
updates::UpdatesManager,
MempoolGuard,
};
Expand Down Expand Up @@ -245,7 +245,7 @@ impl StateKeeperIO for MempoolIO {
tx.hash(),
tx.gas_limit()
);
self.reject(&tx, &Halt::TooBigGasLimit.to_string()).await?;
self.reject(&tx, Reason::Halt(Halt::TooBigGasLimit)).await?;
continue;
}
return Ok(Some(tx));
Expand All @@ -265,7 +265,7 @@ impl StateKeeperIO for MempoolIO {
Ok(())
}

async fn reject(&mut self, rejected: &Transaction, error: &str) -> anyhow::Result<()> {
async fn reject(&mut self, rejected: &Transaction, error: Reason) -> anyhow::Result<()> {
anyhow::ensure!(
!rejected.is_l1(),
"L1 transactions should not be rejected: {error}"
Expand All @@ -276,7 +276,9 @@ 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(error.to_metrics_friendly_string());

tracing::warn!(
"Transaction {} is rejected with error: {error}",
rejected.hash()
Expand Down
4 changes: 2 additions & 2 deletions core/node/state_keeper/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub use self::{
output_handler::{OutputHandler, StateKeeperOutputHandler},
persistence::{L2BlockSealerTask, StateKeeperPersistence},
};
use super::seal_criteria::IoSealCriteria;
use super::seal_criteria::{IoSealCriteria, Reason};

pub mod common;
pub(crate) mod mempool;
Expand Down Expand Up @@ -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, error: Reason) -> anyhow::Result<()>;

/// Loads base system contracts with the specified version.
async fn load_base_system_contracts(
Expand Down
8 changes: 5 additions & 3 deletions core/node/state_keeper/src/keeper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use super::{
updates::UpdatesManager,
utils::gas_count_from_writes,
};
use crate::seal_criteria::Reason;

/// 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.
Expand Down Expand Up @@ -561,7 +562,7 @@ impl ZkSyncStateKeeper {
SealResolution::Unexecutable(reason) => {
batch_executor.rollback_last_tx().await;
self.io
.reject(&tx, reason)
.reject(&tx, reason.clone())
.await
.with_context(|| format!("cannot reject transaction {tx_hash:?}"))?;
}
Expand Down Expand Up @@ -654,6 +655,7 @@ impl ZkSyncStateKeeper {
tx: Transaction,
) -> (SealResolution, TxExecutionResult) {
let exec_result = batch_executor.execute_tx(tx.clone()).await;

// All of `TxExecutionResult::BootloaderOutOfGasForTx`,
// `Halt::NotEnoughGasProvided` correspond to out-of-gas errors but of different nature.
// - `BootloaderOutOfGasForTx`: it is returned when bootloader stack frame run out of gas before tx execution finished.
Expand All @@ -677,15 +679,15 @@ impl ZkSyncStateKeeper {
_ => unreachable!(),
};
let resolution = if is_first_tx {
SealResolution::Unexecutable(error_message.to_string())
SealResolution::Unexecutable(Reason::HardcodedText(error_message.to_string()))
} else {
SealResolution::ExcludeAndSeal
};
AGGREGATION_METRICS.inc(error_message, &resolution);
resolution
}
TxExecutionResult::RejectedByVm { reason } => {
SealResolution::Unexecutable(reason.to_string())
SealResolution::Unexecutable(Reason::Halt(reason.clone()))
}
TxExecutionResult::Success {
tx_result,
Expand Down
44 changes: 42 additions & 2 deletions core/node/state_keeper/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
}

impl TxExecutionType {
pub fn from_is_l1(is_l1: bool) -> TxExecutionType {
match is_l1 {
Expand Down Expand Up @@ -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<Duration>,
/// 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<TxExecutionResult, Counter>,
/// Time spent waiting for the hash of a previous L1 batch.
#[metrics(buckets = Buckets::LATENCIES)]
pub wait_for_prev_hash_time: Histogram<Duration>,
Expand All @@ -77,6 +91,32 @@ pub struct StateKeeperMetrics {
pub blob_base_fee_too_high: Counter,
}

impl StateKeeperMetrics {
pub fn inc_rejected_txs(&self, reason: String) {
self.tx_execution_result[&TxExecutionResult {
status: TxExecutionStatus::Rejected,
reason: Some(reason),
}]
.inc();
AnastasiiaVashchuk marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn inc_succeeded_txs(&self) {
self.tx_execution_result[&TxExecutionResult {
status: TxExecutionStatus::Success,
reason: None,
}]
.inc();
}

pub fn inc_reverted_txs(&self, reason: String) {
self.tx_execution_result[&TxExecutionResult {
status: TxExecutionStatus::Reverted,
reason: Some(reason),
}]
.inc();
}
}

#[vise::register]
pub static KEEPER_METRICS: vise::Global<StateKeeperMetrics> = vise::Global::new();

Expand Down
6 changes: 4 additions & 2 deletions core/node/state_keeper/src/seal_criteria/criteria/gas.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use zksync_types::ProtocolVersionId;

use crate::{
seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig},
seal_criteria::{Reason, SealCriterion, SealData, SealResolution, StateKeeperConfig},
utils::new_block_gas_count,
};

Expand Down Expand Up @@ -30,7 +30,9 @@ 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())
SealResolution::Unexecutable(Reason::HardcodedText(
"Transaction requires too much gas".to_string(),
))
} else if block_data
.gas_count
.any_field_greater_than(config.max_single_tx_gas)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use multivm::utils::gas_bootloader_batch_tip_overhead;
use zksync_types::ProtocolVersionId;

use crate::seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig};
use crate::seal_criteria::{Reason, SealCriterion, SealData, SealResolution, StateKeeperConfig};

/// Checks whether we should exclude the transaction because we don't have enough gas for batch tip.
#[derive(Debug)]
Expand All @@ -22,7 +22,9 @@ 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())
SealResolution::Unexecutable(Reason::HardcodedText(
"not_enough_gas_for_batch_tip".to_string(),
))
} else {
SealResolution::ExcludeAndSeal
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::{Reason, SealCriterion, SealData, SealResolution};

// Collected vm execution metrics should fit into geometry limits.
// Otherwise witness generation will fail and proof won't be generated.
Expand Down Expand Up @@ -52,7 +52,9 @@ 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())
SealResolution::Unexecutable(Reason::HardcodedText(
"ZK proof cannot be generated for a transaction".to_string(),
))
} else if used_circuits_batch + batch_tip_circuit_overhead >= config.max_circuits_per_batch
{
SealResolution::ExcludeAndSeal
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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::{Reason, SealCriterion, SealData, SealResolution, StateKeeperConfig};

#[derive(Debug)]
pub struct PubDataBytesCriterion {
Expand Down Expand Up @@ -41,8 +41,9 @@ 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())
SealResolution::Unexecutable(Reason::HardcodedText(
"Transaction cannot be sent to L1 due to pubdata limits".to_string(),
))
} else if block_size
+ execution_metrics_bootloader_batch_tip_overhead(protocol_version.into())
> max_pubdata_per_l1_batch
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use multivm::utils::get_bootloader_encoding_space;
use zksync_types::ProtocolVersionId;

use crate::seal_criteria::{SealCriterion, SealData, SealResolution, StateKeeperConfig};
use crate::seal_criteria::{Reason, SealCriterion, SealData, SealResolution, StateKeeperConfig};

#[derive(Debug)]
pub struct TxEncodingSizeCriterion;
Expand All @@ -26,8 +26,9 @@ 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())
SealResolution::Unexecutable(Reason::HardcodedText(
"Transaction cannot be included due to large encoding size".to_string(),
))
} 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 {
Expand Down
32 changes: 30 additions & 2 deletions core/node/state_keeper/src/seal_criteria/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -33,6 +33,34 @@ use super::{
utils::{gas_count_from_tx_and_metrics, gas_count_from_writes},
};

/// Represents the reason variants for why a transaction was considered unexecutable.
///
/// This enum can capture either a hardcoded textual message or a more structured
/// reason encapsulated in a `Halt` type.
#[derive(Debug, Clone, PartialEq)]
pub enum Reason {
HardcodedText(String),
AnastasiiaVashchuk marked this conversation as resolved.
Show resolved Hide resolved
Halt(Halt),
}

impl Reason {
pub fn to_metrics_friendly_string(&self) -> String {
match self {
Reason::HardcodedText(text) => text.clone(),
Reason::Halt(halt) => halt.to_metrics_friendly_string(),
}
}
}

impl fmt::Display for Reason {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Reason::HardcodedText(text) => write!(f, "{}", text),
Reason::Halt(halt) => write!(f, "{}", halt),
}
}
}

/// Reported decision regarding block sealing.
#[derive(Debug, Clone, PartialEq)]
pub enum SealResolution {
Expand All @@ -52,7 +80,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(Reason),
}

impl SealResolution {
Expand Down
Loading
Loading