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 ❔
<!-- What are the changes this PR brings about? -->
<!-- Example: This PR adds a PR template to the repo. -->
<!-- (For bigger PRs adding more context is appreciated) -->
## Why ❔
<!-- Why are these changes done? What goal do they contribute to? What
are the principles behind them? -->
<!-- Example: PR templates ensure PR reviewers, observers, and future
iterators are in context about the evolution of repos. -->
## Checklist
<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->
- [ ] 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<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>,
@@ -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<StateKeeperMetrics> = 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<UnexecutableReason> 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<String>,
+ 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<String>),
+ Reject(&'static str, Transaction, UnexecutableReason),
L2BlockSeal(
&'static str,
Option<Box<dyn FnOnce(&UpdatesManager) + Send>>,
@@ -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<TransactionExecutionResult>,
@@ -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