From 616d5614243498ddc75536e48bfd1f968b271263 Mon Sep 17 00:00:00 2001 From: Arni Hod Date: Sun, 24 Nov 2024 09:54:00 +0200 Subject: [PATCH] chore(starknet_batcher): pass block info from consensus to batcher --- config/sequencer/default_config.json | 5 --- crates/starknet_api/src/block.rs | 9 ++-- crates/starknet_batcher/src/batcher.rs | 7 +-- crates/starknet_batcher/src/batcher_test.rs | 11 +++-- crates/starknet_batcher/src/block_builder.rs | 45 +++++-------------- .../starknet_batcher/src/proposal_manager.rs | 2 +- 6 files changed, 27 insertions(+), 52 deletions(-) diff --git a/config/sequencer/default_config.json b/config/sequencer/default_config.json index dfbb72e416..1483b2410b 100644 --- a/config/sequencer/default_config.json +++ b/config/sequencer/default_config.json @@ -114,11 +114,6 @@ "privacy": "Public", "value": 100 }, - "batcher_config.block_builder_config.use_kzg_da": { - "description": "Indicates whether the kzg mechanism is used for data availability.", - "privacy": "Public", - "value": true - }, "batcher_config.block_builder_config.versioned_constants_overrides.invoke_tx_max_n_steps": { "description": "Maximum number of steps the invoke function is allowed to run.", "pointer_target": "versioned_constants_overrides.invoke_tx_max_n_steps", diff --git a/crates/starknet_api/src/block.rs b/crates/starknet_api/src/block.rs index b2e31e8bde..dbd0fe7896 100644 --- a/crates/starknet_api/src/block.rs +++ b/crates/starknet_api/src/block.rs @@ -376,7 +376,7 @@ impl GasPrice { /// Utility struct representing a non-zero gas price. Useful when a gas amount must be computed by /// taking a fee amount and dividing by the gas price. -#[derive(Copy, Clone, Debug, Deserialize, Serialize, derive_more::Display)] +#[derive(Copy, Clone, Debug, Deserialize, Eq, PartialEq, Serialize, derive_more::Display)] pub struct NonzeroGasPrice(GasPrice); impl NonzeroGasPrice { @@ -439,7 +439,8 @@ macro_rules! impl_try_from_uint_for_nonzero_gas_price { impl_try_from_uint_for_nonzero_gas_price!(u8, u16, u32, u64, u128); -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +// TODO(Arni): Remove derive of Default. Gas prices should always be set. +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct GasPriceVector { pub l1_gas_price: NonzeroGasPrice, pub l1_data_gas_price: NonzeroGasPrice, @@ -453,7 +454,7 @@ pub enum FeeType { } // TODO(Arni): Remove derive of Default. Gas prices should always be set. -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct GasPrices { pub eth_gas_prices: GasPriceVector, // In wei. pub strk_gas_prices: GasPriceVector, // In fri. @@ -486,7 +487,7 @@ impl GasPrices { )] pub struct BlockTimestamp(pub u64); -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct BlockInfo { pub block_number: BlockNumber, pub block_timestamp: BlockTimestamp, diff --git a/crates/starknet_batcher/src/batcher.rs b/crates/starknet_batcher/src/batcher.rs index 320681a756..0ffcc0fb62 100644 --- a/crates/starknet_batcher/src/batcher.rs +++ b/crates/starknet_batcher/src/batcher.rs @@ -150,7 +150,7 @@ impl Batcher { .block_builder_factory .create_block_builder( BlockMetadata { - height: active_height, + block_info: propose_block_input.block_info, retrospective_block_hash: propose_block_input.retrospective_block_hash, }, BlockBuilderExecutionParams { @@ -196,7 +196,7 @@ impl Batcher { .block_builder_factory .create_block_builder( BlockMetadata { - height: active_height, + block_info: validate_block_input.block_info, retrospective_block_hash: validate_block_input.retrospective_block_hash, }, BlockBuilderExecutionParams { @@ -465,7 +465,6 @@ fn verify_block_input( ) -> BatcherResult<()> { verify_non_empty_retrospective_block_hash(height, retrospective_block_hash)?; verify_block_number(height, block_number)?; - Ok(()) } @@ -478,7 +477,6 @@ fn verify_non_empty_retrospective_block_hash( { return Err(BatcherError::MissingRetrospectiveBlockHash); } - Ok(()) } @@ -486,6 +484,5 @@ fn verify_block_number(height: BlockNumber, block_number: BlockNumber) -> Batche if block_number != height { return Err(BatcherError::InvalidBlockNumber { active_height: height, block_number }); } - Ok(()) } diff --git a/crates/starknet_batcher/src/batcher_test.rs b/crates/starknet_batcher/src/batcher_test.rs index c81dea80e3..e6ca53d1e1 100644 --- a/crates/starknet_batcher/src/batcher_test.rs +++ b/crates/starknet_batcher/src/batcher_test.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use assert_matches::assert_matches; use async_trait::async_trait; use blockifier::abi::constants; +use blockifier::test_utils::struct_impls::BlockInfoExt; use chrono::Utc; use futures::future::BoxFuture; use futures::FutureExt; @@ -62,6 +63,10 @@ const STREAMING_CHUNK_SIZE: usize = 3; const BLOCK_GENERATION_TIMEOUT: tokio::time::Duration = tokio::time::Duration::from_secs(1); const PROPOSAL_ID: ProposalId = ProposalId(0); +fn initial_block_info() -> BlockInfo { + BlockInfo { block_number: INITIAL_HEIGHT, ..BlockInfo::create_for_testing() } +} + fn proposal_commitment() -> ProposalCommitment { ProposalCommitment { state_diff_commitment: StateDiffCommitment(PoseidonHash(felt!(u128::try_from(7).unwrap()))), @@ -263,7 +268,7 @@ async fn validate_block_full_flow() { proposal_id: PROPOSAL_ID, deadline: deadline(), retrospective_block_hash: None, - block_info: BlockInfo { block_number: INITIAL_HEIGHT, ..Default::default() }, + block_info: initial_block_info(), }; batcher.validate_block(validate_block_input).await.unwrap(); @@ -387,7 +392,7 @@ async fn send_finish_to_an_invalid_proposal() { proposal_id: PROPOSAL_ID, deadline: deadline(), retrospective_block_hash: None, - block_info: BlockInfo { block_number: INITIAL_HEIGHT, ..Default::default() }, + block_info: initial_block_info(), }; batcher.validate_block(validate_block_input).await.unwrap(); @@ -424,7 +429,7 @@ async fn propose_block_full_flow() { proposal_id: PROPOSAL_ID, retrospective_block_hash: None, deadline: chrono::Utc::now() + chrono::Duration::seconds(1), - block_info: BlockInfo { block_number: INITIAL_HEIGHT, ..Default::default() }, + block_info: initial_block_info(), }) .await .unwrap(); diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs index b900f35d9c..26a208277d 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -1,7 +1,6 @@ use std::collections::BTreeMap; use async_trait::async_trait; -use blockifier::blockifier::block::validated_gas_prices; use blockifier::blockifier::config::TransactionExecutorConfig; use blockifier::blockifier::transaction_executor::{ TransactionExecutor, @@ -27,13 +26,7 @@ use papyrus_config::{ParamPath, ParamPrivacyInput, SerializedParam}; use papyrus_state_reader::papyrus_state::PapyrusReader; use papyrus_storage::StorageReader; use serde::{Deserialize, Serialize}; -use starknet_api::block::{ - BlockHashAndNumber, - BlockInfo, - BlockNumber, - BlockTimestamp, - NonzeroGasPrice, -}; +use starknet_api::block::{BlockHashAndNumber, BlockInfo}; use starknet_api::core::ContractAddress; use starknet_api::executable_transaction::Transaction; use starknet_api::transaction::TransactionHash; @@ -45,8 +38,6 @@ use crate::transaction_provider::{NextTxs, TransactionProvider, TransactionProvi #[derive(Debug, Error)] pub enum BlockBuilderError { - #[error(transparent)] - BadTimestamp(#[from] std::num::TryFromIntError), #[error(transparent)] BlockifierStateError(#[from] StateError), #[error(transparent)] @@ -238,7 +229,7 @@ async fn collect_execution_results_and_stream_txs( } pub struct BlockMetadata { - pub height: BlockNumber, + pub block_info: BlockInfo, pub retrospective_block_hash: Option, } @@ -263,7 +254,6 @@ pub struct BlockBuilderConfig { pub execute_config: TransactionExecutorConfig, pub bouncer_config: BouncerConfig, pub sequencer_address: ContractAddress, - pub use_kzg_da: bool, pub tx_chunk_size: usize, pub versioned_constants_overrides: VersionedConstantsOverrides, } @@ -276,7 +266,6 @@ impl Default for BlockBuilderConfig { execute_config: TransactionExecutorConfig::default(), bouncer_config: BouncerConfig::default(), sequencer_address: ContractAddress::default(), - use_kzg_da: true, tx_chunk_size: 100, versioned_constants_overrides: VersionedConstantsOverrides::default(), } @@ -294,12 +283,6 @@ impl SerializeConfig for BlockBuilderConfig { "The address of the sequencer.", ParamPrivacyInput::Public, )])); - dump.append(&mut BTreeMap::from([ser_param( - "use_kzg_da", - &self.use_kzg_da, - "Indicates whether the kzg mechanism is used for data availability.", - ParamPrivacyInput::Public, - )])); dump.append(&mut BTreeMap::from([ser_param( "tx_chunk_size", &self.tx_chunk_size, @@ -323,25 +306,19 @@ pub struct BlockBuilderFactory { impl BlockBuilderFactory { fn preprocess_and_create_transaction_executor( &self, - block_metadata: &BlockMetadata, + block_metadata: BlockMetadata, ) -> BlockBuilderResult> { + let height = block_metadata.block_info.block_number; let block_builder_config = self.block_builder_config.clone(); - let next_block_info = BlockInfo { - block_number: block_metadata.height, - block_timestamp: BlockTimestamp(chrono::Utc::now().timestamp().try_into()?), - sequencer_address: block_builder_config.sequencer_address, - // TODO (yael 7/10/2024): add logic to compute gas prices - gas_prices: { - let tmp_val = NonzeroGasPrice::MIN; - validated_gas_prices(tmp_val, tmp_val, tmp_val, tmp_val, tmp_val, tmp_val) - }, - use_kzg_da: block_builder_config.use_kzg_da, - }; + let mut block_info = block_metadata.block_info; + // TODO(Arni): Remove this override and the corresponding config. + block_info.sequencer_address = block_builder_config.sequencer_address; + let versioned_constants = VersionedConstants::get_versioned_constants( block_builder_config.versioned_constants_overrides, ); let block_context = BlockContext::new( - next_block_info, + block_info, block_builder_config.chain_info, versioned_constants, block_builder_config.bouncer_config, @@ -349,7 +326,7 @@ impl BlockBuilderFactory { let state_reader = PapyrusReader::new( self.storage_reader.clone(), - block_metadata.height, + height, self.global_class_hash_to_class.clone(), ); @@ -372,7 +349,7 @@ impl BlockBuilderFactoryTrait for BlockBuilderFactory { tx_provider: Box, output_content_sender: Option>, ) -> BlockBuilderResult<(Box, AbortSignalSender)> { - let executor = self.preprocess_and_create_transaction_executor(&block_metadata)?; + let executor = self.preprocess_and_create_transaction_executor(block_metadata)?; let (abort_signal_sender, abort_signal_receiver) = tokio::sync::oneshot::channel(); let block_builder = Box::new(BlockBuilder::new( Box::new(executor), diff --git a/crates/starknet_batcher/src/proposal_manager.rs b/crates/starknet_batcher/src/proposal_manager.rs index 56c72ab437..d588861c10 100644 --- a/crates/starknet_batcher/src/proposal_manager.rs +++ b/crates/starknet_batcher/src/proposal_manager.rs @@ -86,7 +86,7 @@ struct ProposalTask { /// Taking care of: /// - Proposing new blocks. /// - Validating incoming proposals. -/// - Commiting accepted proposals to the storage. +/// - Committing accepted proposals to the storage. /// /// Triggered by the consensus. pub(crate) struct ProposalManager {