Merge pull request #2452 from eqlabs/sistemd/remove-storage-class-commitment

refactor(common): remove `storage_commitment` and `class_commitment` from `BlockHeader`

Author: sistemd

CHANGELOG.md

@@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Removed
+
+- `storage_commitment` and `class_commitment` fields from the `pathfinder_subscribe_newHeads` method response.
+- `class_commitment` from the `pathfinder_getProof` and `pathfinder_getClassProof` method responses.
+
 ### Fixed
 
 - `pathfinder_getProof`, `pathfinder_getClassProof` return `ProofMissing` (10001) when Pathfinder is in `archive` mode and queried block's tries are empty.

crates/common/src/header.rs

@@ -17,10 +17,8 @@ pub struct BlockHeader {
     pub strk_l2_gas_price: GasPrice,
     pub sequencer_address: SequencerAddress,
     pub starknet_version: StarknetVersion,
-    pub class_commitment: ClassCommitment,
     pub event_commitment: EventCommitment,
     pub state_commitment: StateCommitment,
-    pub storage_commitment: StorageCommitment,
     pub transaction_commitment: TransactionCommitment,
     pub transaction_count: usize,
     pub event_count: usize,
@@ -88,11 +86,14 @@ impl BlockHeaderBuilder {
         self
     }
 
-    /// Sets the [StateCommitment] by calculating its value from the current
+    /// Sets the [StateCommitment] by calculating its value from the passed
     /// [StorageCommitment] and [ClassCommitment].
-    pub fn calculated_state_commitment(mut self) -> Self {
-        self.0.state_commitment =
-            StateCommitment::calculate(self.0.storage_commitment, self.0.class_commitment);
+    pub fn calculated_state_commitment(
+        mut self,
+        storage_commitment: StorageCommitment,
+        class_commitment: ClassCommitment,
+    ) -> Self {
+        self.0.state_commitment = StateCommitment::calculate(storage_commitment, class_commitment);
         self
     }
 
@@ -146,16 +147,6 @@ impl BlockHeaderBuilder {
         self
     }
 
-    pub fn storage_commitment(mut self, storage_commitment: StorageCommitment) -> Self {
-        self.0.storage_commitment = storage_commitment;
-        self
-    }
-
-    pub fn class_commitment(mut self, class_commitment: ClassCommitment) -> Self {
-        self.0.class_commitment = class_commitment;
-        self
-    }
-
     pub fn starknet_version(mut self, starknet_version: StarknetVersion) -> Self {
         self.0.starknet_version = starknet_version;
         self

crates/p2p/src/client/peer_agnostic/fixtures.rs

@@ -149,9 +149,6 @@ pub fn hdr(tag: i32) -> SignedBlockHeader {
     Tagged::get(format!("header {tag}"), || SignedBlockHeader {
         header: BlockHeader {
             number: BlockNumber::new_or_panic(tag as u64),
-            // TODO Set storage and class commitment
-            storage_commitment: Default::default(),
-            class_commitment: Default::default(),
             ..Faker.fake()
         },
         ..Faker.fake()

crates/p2p/src/client/types.rs

@@ -11,7 +11,6 @@ use pathfinder_common::{
     BlockHeader,
     BlockNumber,
     BlockTimestamp,
-    ClassCommitment,
     ClassHash,
     EventCommitment,
     Fee,
@@ -22,7 +21,6 @@ use pathfinder_common::{
     SignedBlockHeader,
     StateCommitment,
     StateDiffCommitment,
-    StorageCommitment,
     TransactionCommitment,
     TransactionHash,
     TransactionIndex,
@@ -118,8 +116,6 @@ impl TryFromDto<p2p_proto::header::SignedBlockHeader> for SignedBlockHeader {
                 l1_da_mode: TryFromDto::try_from_dto(dto.l1_data_availability_mode)?,
                 state_diff_commitment: StateDiffCommitment(dto.state_diff_commitment.root.0),
                 state_diff_length: dto.state_diff_commitment.state_diff_length,
-                class_commitment: ClassCommitment::ZERO,
-                storage_commitment: StorageCommitment::ZERO,
             },
             signature,
         })

crates/pathfinder/examples/compute_pre0132_hashes.rs

@@ -8,7 +8,6 @@ use pathfinder_common::{
     ReceiptCommitment,
     StarknetVersion,
     StateCommitment,
-    StorageCommitment,
 };
 use pathfinder_lib::state::block_hash::{
     calculate_event_commitment,
@@ -119,10 +118,6 @@ fn main() -> anyhow::Result<()> {
             header.state_commitment != StateCommitment::ZERO,
             "state_commitment missing"
         );
-        ensure!(
-            header.storage_commitment != StorageCommitment::ZERO,
-            "storage_commitment missing"
-        );
 
         // Compute the block hash in the 0.13.2 style
         let header_data = get_header_data(&header);

crates/pathfinder/src/p2p_network/sync_handlers/tests.rs

@@ -185,9 +185,6 @@ mod prop {
             let expected = overlapping::get(in_db, start_block, limit, step, num_blocks, direction)
                 .into_iter().map(|Block { header, .. }| pathfinder_common::SignedBlockHeader {
                     header: pathfinder_common::BlockHeader {
-                        // TODO Set the storage and class commitment
-                        storage_commitment: Default::default(),
-                        class_commitment: Default::default(),
                         ..header.header
                     },
                     signature: header.signature,

crates/pathfinder/src/state/sync.rs

@@ -912,10 +912,8 @@ async fn l2_update(
                 .sequencer_address
                 .unwrap_or(SequencerAddress(Felt::ZERO)),
             starknet_version: block.starknet_version,
-            class_commitment,
             event_commitment,
             state_commitment,
-            storage_commitment,
             transaction_commitment,
             transaction_count,
             event_count,

crates/pathfinder/src/state/sync/revert.rs

@@ -4,6 +4,7 @@ use pathfinder_common::{
     BlockNumber,
     ClassCommitment,
     ClassCommitmentLeafHash,
+    StateCommitment,
     StorageCommitment,
 };
 use pathfinder_merkle_tree::{ClassCommitmentTree, StorageCommitmentTree};
@@ -37,31 +38,30 @@ pub fn revert_starknet_state(
     target_block: BlockNumber,
     target_header: BlockHeader,
 ) -> Result<(), anyhow::Error> {
-    revert_contract_updates(
-        transaction,
-        head,
-        target_block,
-        target_header.storage_commitment,
-    )?;
-    revert_class_updates(
-        transaction,
-        head,
-        target_block,
-        target_header.class_commitment,
-    )?;
+    let storage_commitment = revert_contract_updates(transaction, head, target_block)?;
+    let class_commitment = revert_class_updates(transaction, head, target_block)?;
+
+    let state_commitment = StateCommitment::calculate(storage_commitment, class_commitment);
+    if state_commitment != target_header.state_commitment {
+        anyhow::bail!(
+            "State commitment mismatch: expected {}, calculated {}",
+            target_header.state_commitment,
+            state_commitment,
+        );
+    }
+
     transaction.coalesce_trie_removals(target_block)
 }
 
 /// Revert all contract/global storage trie updates.
 ///
 /// Fetches reverse updates from the database and updates all tries, returning
-/// the storage commitment and the storage root node index.
+/// the [`StorageCommitment`].
 fn revert_contract_updates(
     transaction: &Transaction<'_>,
     head: BlockNumber,
     target_block: BlockNumber,
-    expected_storage_commitment: StorageCommitment,
-) -> anyhow::Result<()> {
+) -> anyhow::Result<StorageCommitment> {
     let updates = transaction.reverse_contract_updates(head, target_block)?;
 
     let mut global_tree =
@@ -91,14 +91,6 @@ fn revert_contract_updates(
         .commit()
         .context("Committing global state tree")?;
 
-    if expected_storage_commitment != storage_commitment {
-        anyhow::bail!(
-            "Storage commitment mismatch: expected {}, calculated {}",
-            expected_storage_commitment,
-            storage_commitment
-        );
-    }
-
     let root_idx = transaction
         .insert_storage_trie(&trie_update, target_block)
         .context("Persisting storage trie")?;
@@ -108,16 +100,18 @@ fn revert_contract_updates(
         .context("Inserting storage root index")?;
     tracing::debug!(%target_block, %storage_commitment, "Committed global state tree");
 
-    Ok(())
+    Ok(storage_commitment)
 }
 
 /// Revert all class trie updates.
+///
+/// Fetches reverse updates from the database and updates all tries, returning
+/// the [`ClassCommitment`].
 fn revert_class_updates(
     transaction: &Transaction<'_>,
     head: BlockNumber,
     target_block: BlockNumber,
-    expected_class_commitment: ClassCommitment,
-) -> anyhow::Result<()> {
+) -> anyhow::Result<ClassCommitment> {
     let updates = transaction.reverse_sierra_class_updates(head, target_block)?;
 
     let mut class_tree =
@@ -143,14 +137,6 @@ fn revert_class_updates(
 
     let (class_commitment, trie_update) = class_tree.commit().context("Committing class trie")?;
 
-    if expected_class_commitment != class_commitment {
-        anyhow::bail!(
-            "Storage commitment mismatch: expected {}, calculated {}",
-            expected_class_commitment,
-            class_commitment
-        );
-    }
-
     let root_idx = transaction
         .insert_class_trie(&trie_update, target_block)
         .context("Persisting class trie")?;
@@ -161,5 +147,5 @@ fn revert_class_updates(
 
     tracing::debug!(%target_block, %class_commitment, "Committed class trie");
 
-    Ok(())
+    Ok(class_commitment)
 }

crates/pathfinder/src/sync/checkpoint.rs

@@ -785,8 +785,6 @@ mod tests {
                         eth_l2_gas_price: GasPrice(0),
                         strk_l2_gas_price: GasPrice(0),
                         l1_da_mode: L1DataAvailabilityMode::Calldata,
-                        class_commitment: ClassCommitment::ZERO,
-                        storage_commitment: StorageCommitment::ZERO,
                     },
                     signature: BlockCommitmentSignature {
                         r: dto.signature[0],

crates/pathfinder/src/sync/state_updates.rs

@@ -303,8 +303,6 @@ pub async fn batch_update_starknet_state(
             "State root mismatch");
             return Err(SyncError::StateRootMismatch(peer));
         }
-        db.update_storage_and_class_commitments(tail, storage_commitment, class_commitment)
-            .context("Updating storage and class commitments")?;
         db.commit().context("Committing db transaction")?;
 
         Ok(PeerData::new(peer, tail))