feat(en): Improved en commitment generation error handling (#4483)

tomg10 · web-flow · commit 1bb82d6a1cf5 · 2025-09-12T09:14:14.000Z
diff --git a/core/bin/custom_genesis_export/src/main.rs b/core/bin/custom_genesis_export/src/main.rs
@@ -120,7 +120,7 @@ async fn main() -> anyhow::Result<()> {
             .protocol_version
             .ok_or(anyhow::anyhow!("No bootloader_hash specified"))?
             .minor,
-    );
+    )?;
 
     genesis_config.genesis_root_hash = Some(genesis_batch_params.root_hash);
     genesis_config.rollup_last_leaf_index = Some(genesis_batch_params.rollup_last_leaf_index);
diff --git a/core/lib/types/src/commitment/mod.rs b/core/lib/types/src/commitment/mod.rs
@@ -9,6 +9,7 @@
 use std::{collections::HashMap, convert::TryFrom};
 
 use serde::{Deserialize, Serialize};
+use thiserror::Error;
 pub use zksync_basic_types::commitment::{L1BatchCommitmentMode, PubdataParams, PubdataType};
 use zksync_contracts::BaseSystemContractsHashes;
 use zksync_crypto_primitives::hasher::{keccak::KeccakHasher, Hasher};
@@ -38,6 +39,21 @@ use crate::{
 #[cfg(test)]
 mod tests;
 
+#[derive(Debug, Error)]
+pub enum CommitmentValidationError {
+    #[error("State diff hash mismatch: expected {expected}, got {actual}")]
+    StateDiffHashMismatch { expected: H256, actual: H256 },
+    #[error("Blob linear hashes mismatch: expected {expected:?}, got {actual:?}")]
+    BlobLinearHashesMismatch {
+        expected: Vec<H256>,
+        actual: Vec<H256>,
+    },
+    #[error("L2 L1 logs tree root mismatch: expected {expected}, got {actual}")]
+    L2L1LogsTreeRootMismatch { expected: H256, actual: H256 },
+    #[error("Serialized size for BlockPassThroughData is bigger than expected: expected {expected}, got {actual}")]
+    SerializedSizeMismatch { expected: usize, actual: usize },
+}
+
 /// Type that can be serialized for commitment.
 pub trait SerializeCommitment {
     /// Size of the structure in bytes.
@@ -335,7 +351,7 @@ pub enum L1BatchAuxiliaryOutput {
 }
 
 impl L1BatchAuxiliaryOutput {
-    fn new(input: CommitmentInput) -> Self {
+    fn new(input: CommitmentInput) -> Result<Self, CommitmentValidationError> {
         match input {
             CommitmentInput::PreBoojum {
                 common: common_input,
@@ -365,14 +381,14 @@ impl L1BatchAuxiliaryOutput {
                 let repeated_writes_compressed = pre_boojum_serialize_commitments(&repeated_writes);
                 let repeated_writes_hash = H256::from(keccak256(&repeated_writes_compressed));
 
-                Self::PreBoojum {
+                Ok(Self::PreBoojum {
                     common: common_output,
                     l2_l1_logs_linear_hash,
                     initial_writes_compressed,
                     initial_writes_hash,
                     repeated_writes_compressed,
                     repeated_writes_hash,
-                }
+                })
             }
             CommitmentInput::PostBoojum {
                 common: common_input,
@@ -419,10 +435,12 @@ impl L1BatchAuxiliaryOutput {
                                     .then_some(log.0.value)
                             })
                             .expect("Failed to find state diff hash in system logs");
-                        assert_eq!(
-                            state_diffs_hash, state_diff_hash_from_logs,
-                            "State diff hash mismatch"
-                        );
+                        if state_diffs_hash != state_diff_hash_from_logs {
+                            return Err(CommitmentValidationError::StateDiffHashMismatch {
+                                expected: state_diff_hash_from_logs,
+                                actual: state_diffs_hash,
+                            });
+                        }
 
                         let blob_linear_hashes_from_logs =
                             parse_system_logs_for_blob_hashes_pre_gateway(
@@ -431,10 +449,12 @@ impl L1BatchAuxiliaryOutput {
                             );
                         let blob_linear_hashes: Vec<_> =
                             blob_hashes.iter().map(|b| b.linear_hash).collect();
-                        assert_eq!(
-                            blob_linear_hashes, blob_linear_hashes_from_logs,
-                            "Blob linear hashes mismatch"
-                        );
+                        if blob_linear_hashes != blob_linear_hashes_from_logs {
+                            return Err(CommitmentValidationError::BlobLinearHashesMismatch {
+                                expected: blob_linear_hashes_from_logs,
+                                actual: blob_linear_hashes,
+                            });
+                        }
                     }
 
                     let l2_to_l1_logs_tree_root_from_logs = system_logs
@@ -444,13 +464,15 @@ impl L1BatchAuxiliaryOutput {
                                 .then_some(log.0.value)
                         })
                         .expect("Failed to find L2 to L1 logs tree root in system logs");
-                    assert_eq!(
-                        l2_l1_logs_merkle_root, l2_to_l1_logs_tree_root_from_logs,
-                        "L2 L1 logs tree root mismatch"
-                    );
+                    if l2_l1_logs_merkle_root != l2_to_l1_logs_tree_root_from_logs {
+                        return Err(CommitmentValidationError::L2L1LogsTreeRootMismatch {
+                            expected: l2_to_l1_logs_tree_root_from_logs,
+                            actual: l2_l1_logs_merkle_root,
+                        });
+                    }
                 }
 
-                Self::PostBoojum {
+                Ok(Self::PostBoojum {
                     common: common_output,
                     system_logs_linear_hash,
                     state_diffs_compressed,
@@ -459,7 +481,7 @@ impl L1BatchAuxiliaryOutput {
                     blob_hashes,
                     local_root,
                     aggregation_root,
-                }
+                })
             }
         }
     }
@@ -589,24 +611,25 @@ struct L1BatchPassThroughData {
 }
 
 impl L1BatchPassThroughData {
-    pub fn to_bytes(&self) -> Vec<u8> {
+    pub fn to_bytes(&self) -> Result<Vec<u8>, CommitmentValidationError> {
         // We assume that currently we have only two shared state: Rollup and ZkPorter where porter is always zero
         const SERIALIZED_SIZE: usize = 8 + 32 + 8 + 32;
         let mut result = Vec::with_capacity(SERIALIZED_SIZE);
         for state in self.shared_states.iter() {
             result.extend_from_slice(&state.last_leaf_index.to_be_bytes());
             result.extend_from_slice(state.root_hash.as_bytes());
         }
-        assert_eq!(
-            result.len(),
-            SERIALIZED_SIZE,
-            "Serialized size for BlockPassThroughData is bigger than expected"
-        );
-        result
+        if result.len() != SERIALIZED_SIZE {
+            return Err(CommitmentValidationError::SerializedSizeMismatch {
+                expected: SERIALIZED_SIZE,
+                actual: result.len(),
+            });
+        }
+        Ok(result)
     }
 
-    pub fn hash(&self) -> H256 {
-        H256::from_slice(&keccak256(&self.to_bytes()))
+    pub fn hash(&self) -> Result<H256, CommitmentValidationError> {
+        Ok(H256::from_slice(&keccak256(&self.to_bytes()?)))
     }
 }
 
@@ -627,7 +650,7 @@ pub struct L1BatchCommitmentHash {
 }
 
 impl L1BatchCommitment {
-    pub fn new(input: CommitmentInput) -> Self {
+    pub fn new(input: CommitmentInput) -> Result<Self, CommitmentValidationError> {
         let meta_parameters = L1BatchMetaParameters {
             zkporter_is_available: ZKPORTER_IS_AVAILABLE,
             bootloader_code_hash: input.common().bootloader_code_hash,
@@ -636,7 +659,7 @@ impl L1BatchCommitment {
             protocol_version: Some(input.common().protocol_version),
         };
 
-        Self {
+        Ok(Self {
             pass_through_data: L1BatchPassThroughData {
                 shared_states: vec![
                     RootState {
@@ -650,9 +673,9 @@ impl L1BatchCommitment {
                     },
                 ],
             },
-            auxiliary_output: L1BatchAuxiliaryOutput::new(input),
+            auxiliary_output: L1BatchAuxiliaryOutput::new(input)?,
             meta_parameters,
-        }
+        })
     }
 
     pub fn meta_parameters(&self) -> L1BatchMetaParameters {
@@ -672,25 +695,25 @@ impl L1BatchCommitment {
         }
     }
 
-    pub fn hash(&self) -> L1BatchCommitmentHash {
+    pub fn hash(&self) -> Result<L1BatchCommitmentHash, CommitmentValidationError> {
         let mut result = vec![];
-        let pass_through_data_hash = self.pass_through_data.hash();
+        let pass_through_data_hash = self.pass_through_data.hash()?;
         result.extend_from_slice(pass_through_data_hash.as_bytes());
         let metadata_hash = self.meta_parameters.hash();
         result.extend_from_slice(metadata_hash.as_bytes());
         let auxiliary_output_hash = self.auxiliary_output.hash();
         result.extend_from_slice(auxiliary_output_hash.as_bytes());
         let hash = keccak256(&result);
         let commitment = H256::from_slice(&hash);
-        L1BatchCommitmentHash {
+        Ok(L1BatchCommitmentHash {
             pass_through_data: pass_through_data_hash,
             aux_output: auxiliary_output_hash,
             meta_parameters: metadata_hash,
             commitment,
-        }
+        })
     }
 
-    pub fn artifacts(&self) -> L1BatchCommitmentArtifacts {
+    pub fn artifacts(&self) -> Result<L1BatchCommitmentArtifacts, CommitmentValidationError> {
         let (compressed_initial_writes, compressed_repeated_writes, compressed_state_diffs) =
             match &self.auxiliary_output {
                 L1BatchAuxiliaryOutput::PostBoojum {
@@ -708,8 +731,8 @@ impl L1BatchCommitment {
                 ),
             };
 
-        L1BatchCommitmentArtifacts {
-            commitment_hash: self.hash(),
+        Ok(L1BatchCommitmentArtifacts {
+            commitment_hash: self.hash()?,
             l2_l1_merkle_root: self.l2_l1_logs_merkle_root(),
             compressed_state_diffs,
             zkporter_is_available: self.meta_parameters.zkporter_is_available,
@@ -719,7 +742,7 @@ impl L1BatchCommitment {
             local_root: self.auxiliary_output.local_root(),
             aggregation_root: self.auxiliary_output.aggregation_root(),
             state_diff_hash: self.auxiliary_output.state_diff_hash(),
-        }
+        })
     }
 }
 
diff --git a/core/lib/types/src/commitment/tests/mod.rs b/core/lib/types/src/commitment/tests/mod.rs
@@ -17,7 +17,7 @@ fn run_test(test_name: &str) {
     let contents = read_to_string(format!("src/commitment/tests/{test_name}.json")).unwrap();
     let commitment_test: CommitmentTest = serde_json::from_str(&contents).unwrap();
 
-    let commitment = L1BatchCommitment::new(commitment_test.input);
+    let commitment = L1BatchCommitment::new(commitment_test.input).unwrap();
 
     assert_eq!(
         commitment.pass_through_data,
@@ -28,7 +28,7 @@ fn run_test(test_name: &str) {
         commitment.auxiliary_output,
         commitment_test.auxiliary_output
     );
-    assert_eq!(commitment.hash(), commitment_test.hashes);
+    assert_eq!(commitment.hash().unwrap(), commitment_test.hashes);
 }
 
 #[test]
diff --git a/core/node/commitment_generator/src/lib.rs b/core/node/commitment_generator/src/lib.rs
@@ -337,9 +337,9 @@ impl CommitmentGenerator {
 
         let latency =
             METRICS.generate_commitment_latency_stage[&CommitmentStage::Calculate].start();
-        let mut commitment = L1BatchCommitment::new(input);
+        let mut commitment = L1BatchCommitment::new(input)?;
         self.post_process_commitment(&mut commitment, commitment_mode);
-        let artifacts = commitment.artifacts();
+        let artifacts = commitment.artifacts()?;
         let latency = latency.observe();
         tracing::debug!(
             "Generated commitment artifacts for L1 batch #{l1_batch_number} in {latency:?}"
diff --git a/core/node/genesis/src/lib.rs b/core/node/genesis/src/lib.rs
@@ -74,6 +74,8 @@ pub enum GenesisError {
     Other(#[from] anyhow::Error),
     #[error("Field: {0} required for genesis")]
     MalformedConfig(&'static str),
+    #[error("Commitment validation error: {0}")]
+    CommitmentValidation(#[from] zksync_types::commitment::CommitmentValidationError),
 }
 
 #[derive(Debug, Clone)]
@@ -191,7 +193,7 @@ pub fn make_genesis_batch_params(
     deduped_log_queries: Vec<LogQuery>,
     base_system_contract_hashes: BaseSystemContractsHashes,
     protocol_version: ProtocolVersionId,
-) -> (GenesisBatchParams, L1BatchCommitment) {
+) -> Result<(GenesisBatchParams, L1BatchCommitment), GenesisError> {
     let storage_logs = deduped_log_queries
         .into_iter()
         .filter(|log_query| log_query.rw_flag) // only writes
@@ -216,17 +218,17 @@ pub fn make_genesis_batch_params(
         base_system_contract_hashes,
         protocol_version,
     );
-    let block_commitment = L1BatchCommitment::new(commitment_input);
-    let commitment = block_commitment.hash().commitment;
+    let block_commitment = L1BatchCommitment::new(commitment_input)?;
+    let commitment = block_commitment.hash()?.commitment;
 
-    (
+    Ok((
         GenesisBatchParams {
             root_hash,
             commitment,
             rollup_last_leaf_index,
         },
         block_commitment,
-    )
+    ))
 }
 
 pub async fn insert_genesis_batch_with_custom_state(
@@ -297,7 +299,7 @@ pub async fn insert_genesis_batch_with_custom_state(
         deduped_log_queries,
         base_system_contract_hashes,
         genesis_params.minor_protocol_version(),
-    );
+    )?;
 
     save_genesis_l1_batch_metadata(
         &mut transaction,
diff --git a/core/node/genesis/src/utils.rs b/core/node/genesis/src/utils.rs
@@ -154,7 +154,7 @@ pub(super) async fn save_genesis_l1_batch_metadata(
         .save_l1_batch_tree_data(L1BatchNumber(0), &tree_data)
         .await?;
 
-    let mut commitment_artifacts = commitment.artifacts();
+    let mut commitment_artifacts = commitment.artifacts()?;
     // `l2_l1_merkle_root` for genesis batch is set to 0 on L1 contract, same must be here.
     commitment_artifacts.l2_l1_merkle_root = H256::zero();
 
