Skip to content

Commit

Permalink
Merge pull request #2452 from eqlabs/sistemd/remove-storage-class-com…
Browse files Browse the repository at this point in the history
…mitment

refactor(common): remove `storage_commitment` and `class_commitment` from `BlockHeader`
  • Loading branch information
sistemd authored Dec 24, 2024
2 parents 2097a13 + b07d337 commit a054a03
Show file tree
Hide file tree
Showing 26 changed files with 77 additions and 210 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 7 additions & 16 deletions crates/common/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions crates/p2p/src/client/peer_agnostic/fixtures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 0 additions & 4 deletions crates/p2p/src/client/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use pathfinder_common::{
BlockHeader,
BlockNumber,
BlockTimestamp,
ClassCommitment,
ClassHash,
EventCommitment,
Fee,
Expand All @@ -22,7 +21,6 @@ use pathfinder_common::{
SignedBlockHeader,
StateCommitment,
StateDiffCommitment,
StorageCommitment,
TransactionCommitment,
TransactionHash,
TransactionIndex,
Expand Down Expand Up @@ -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,
})
Expand Down
5 changes: 0 additions & 5 deletions crates/pathfinder/examples/compute_pre0132_hashes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use pathfinder_common::{
ReceiptCommitment,
StarknetVersion,
StateCommitment,
StorageCommitment,
};
use pathfinder_lib::state::block_hash::{
calculate_event_commitment,
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions crates/pathfinder/src/p2p_network/sync_handlers/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions crates/pathfinder/src/state/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
56 changes: 21 additions & 35 deletions crates/pathfinder/src/state/sync/revert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use pathfinder_common::{
BlockNumber,
ClassCommitment,
ClassCommitmentLeafHash,
StateCommitment,
StorageCommitment,
};
use pathfinder_merkle_tree::{ClassCommitmentTree, StorageCommitmentTree};
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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")?;
Expand All @@ -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 =
Expand All @@ -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")?;
Expand All @@ -161,5 +147,5 @@ fn revert_class_updates(

tracing::debug!(%target_block, %class_commitment, "Committed class trie");

Ok(())
Ok(class_commitment)
}
2 changes: 0 additions & 2 deletions crates/pathfinder/src/sync/checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
2 changes: 0 additions & 2 deletions crates/pathfinder/src/sync/state_updates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
7 changes: 0 additions & 7 deletions crates/pathfinder/src/sync/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -788,12 +788,8 @@ impl ProcessStage for StoreBlock {
strk_l2_gas_price: header.strk_l2_gas_price,
sequencer_address: header.sequencer_address,
starknet_version: header.starknet_version,
// Class commitment is updated after the class tries are updated.
class_commitment: ClassCommitment::ZERO,
event_commitment: header.event_commitment,
state_commitment: header.state_commitment,
// Storage commitment is updated after the storage tries are updated.
storage_commitment: StorageCommitment::ZERO,
transaction_commitment: header.transaction_commitment,
transaction_count: header.transaction_count,
event_count: header.event_count,
Expand Down Expand Up @@ -841,9 +837,6 @@ impl ProcessStage for StoreBlock {
return Err(SyncError::StateRootMismatch(*peer));
}

db.update_storage_and_class_commitments(block_number, storage_commitment, class_commitment)
.context("Updating storage and class commitments")?;

classes.into_iter().try_for_each(
|CompiledClass {
block_number,
Expand Down
Binary file modified crates/rpc/fixtures/mainnet.sqlite
Binary file not shown.
4 changes: 0 additions & 4 deletions crates/rpc/src/jsonrpc/websocket/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,8 @@ impl serde::Serialize for BlockHeader {
strk_l2_gas_price,
sequencer_address,
starknet_version,
class_commitment,
event_commitment,
state_commitment,
storage_commitment,
transaction_commitment,
transaction_count,
event_count,
Expand All @@ -267,10 +265,8 @@ impl serde::Serialize for BlockHeader {
map.serialize_entry("strk_l2_gas_price", &strk_l2_gas_price)?;
map.serialize_entry("sequencer_address", &sequencer_address)?;
map.serialize_entry("starknet_version", &starknet_version.to_string())?;
map.serialize_entry("class_commitment", &class_commitment)?;
map.serialize_entry("event_commitment", &event_commitment)?;
map.serialize_entry("state_commitment", &state_commitment)?;
map.serialize_entry("storage_commitment", &storage_commitment)?;
map.serialize_entry("transaction_commitment", &transaction_commitment)?;
map.serialize_entry("transaction_count", &transaction_count)?;
map.serialize_entry("event_count", &event_count)?;
Expand Down
12 changes: 3 additions & 9 deletions crates/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,7 @@ pub mod test_utils {
.unwrap();
let header0 = BlockHeader::builder()
.number(BlockNumber::GENESIS)
.storage_commitment(storage_commitment0)
.class_commitment(class_commitment0)
.calculated_state_commitment()
.calculated_state_commitment(storage_commitment0, class_commitment0)
.finalize_with_hash(block_hash_bytes!(b"genesis"));
db_txn.insert_block_header(&header0).unwrap();
db_txn
Expand Down Expand Up @@ -428,9 +426,7 @@ pub mod test_utils {
let header1 = header0
.child_builder()
.timestamp(BlockTimestamp::new_or_panic(1))
.storage_commitment(storage_commitment1)
.class_commitment(class_commitment1)
.calculated_state_commitment()
.calculated_state_commitment(storage_commitment1, class_commitment1)
.eth_l1_gas_price(GasPrice::from(1))
.sequencer_address(sequencer_address_bytes!(&[1u8]))
.finalize_with_hash(block_hash_bytes!(b"block 1"));
Expand Down Expand Up @@ -514,9 +510,7 @@ pub mod test_utils {
let header2 = header1
.child_builder()
.timestamp(BlockTimestamp::new_or_panic(2))
.storage_commitment(storage_commitment2)
.class_commitment(class_commitment2)
.calculated_state_commitment()
.calculated_state_commitment(storage_commitment2, class_commitment2)
.eth_l1_gas_price(GasPrice::from(2))
.sequencer_address(sequencer_address_bytes!(&[2u8]))
.finalize_with_hash(block_hash_bytes!(b"latest"));
Expand Down
2 changes: 0 additions & 2 deletions crates/rpc/src/method/trace_block_transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1031,10 +1031,8 @@ pub(crate) mod tests {
.sequencer_address
.unwrap_or(SequencerAddress(Felt::ZERO)),
starknet_version: block.starknet_version,
class_commitment: Default::default(),
event_commitment: Default::default(),
state_commitment: Default::default(),
storage_commitment: Default::default(),
transaction_commitment: Default::default(),
transaction_count,
event_count,
Expand Down
Loading

0 comments on commit a054a03

Please sign in to comment.