From b0aff4de015982ad5f79fa099de1d194cf6db792 Mon Sep 17 00:00:00 2001 From: Arni Hod Date: Sun, 24 Nov 2024 09:54:00 +0200 Subject: [PATCH] chore(batcher): use block info to create block info --- 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 | 28 ++++++----- crates/starknet_batcher/src/block_builder.rs | 47 ++++--------------- .../starknet_batcher/src/proposal_manager.rs | 16 +++---- .../src/proposal_manager_test.rs | 22 ++++++--- 7 files changed, 57 insertions(+), 77 deletions(-) diff --git a/config/sequencer/default_config.json b/config/sequencer/default_config.json index b5941df09d..0ed86f030c 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 8852e706c6..9f2f0f6e55 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, Deserialize, Serialize)] +#[derive(Clone, Debug, 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 b2c4f9e2c3..63c6796677 100644 --- a/crates/starknet_batcher/src/batcher.rs +++ b/crates/starknet_batcher/src/batcher.rs @@ -139,7 +139,7 @@ impl Batcher { self.proposal_manager .propose_block( - active_height, + propose_block_input.block_info, proposal_id, propose_block_input.retrospective_block_hash, deadline, @@ -177,7 +177,7 @@ impl Batcher { self.proposal_manager .validate_block( - active_height, + validate_block_input.block_info, proposal_id, validate_block_input.retrospective_block_hash, deadline, @@ -427,7 +427,6 @@ fn verify_block_input( ) -> BatcherResult<()> { verify_non_empty_retrospective_block_hash(height, retrospective_block_hash)?; verify_block_number(height, block_number)?; - Ok(()) } @@ -440,7 +439,6 @@ fn verify_non_empty_retrospective_block_hash( { return Err(BatcherError::MissingRetrospectiveBlockHash); } - Ok(()) } @@ -448,6 +446,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 f90f384f5d..7b26272600 100644 --- a/crates/starknet_batcher/src/batcher_test.rs +++ b/crates/starknet_batcher/src/batcher_test.rs @@ -1,9 +1,10 @@ use std::collections::{HashMap, HashSet}; -use std::sync::Arc; +use std::sync::{Arc, LazyLock}; 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; @@ -55,6 +56,9 @@ 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); +static INITIAL_BLOCK_INFO: LazyLock = + LazyLock::new(|| 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()))), @@ -114,7 +118,7 @@ fn mock_proposal_manager_validate_flow() -> MockProposalManagerTraitWrapper { proposal_manager .expect_wrap_validate_block() .times(1) - .with(eq(INITIAL_HEIGHT), eq(PROPOSAL_ID), eq(None), always(), always()) + .with(eq(INITIAL_BLOCK_INFO.clone()), eq(PROPOSAL_ID), eq(None), always(), always()) .return_once(|_, _, _, _, tx_provider| { { async move { @@ -231,7 +235,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.clone(), }; batcher.validate_block(validate_block_input).await.unwrap(); @@ -331,7 +335,7 @@ async fn send_finish_to_an_invalid_proposal() { proposal_manager .expect_wrap_validate_block() .times(1) - .with(eq(INITIAL_HEIGHT), eq(PROPOSAL_ID), eq(None), always(), always()) + .with(eq(INITIAL_BLOCK_INFO.clone()), eq(PROPOSAL_ID), eq(None), always(), always()) .return_once(|_, _, _, _, _| { async move { Ok(()) } }.boxed()); let proposal_error = GetProposalResultError::BlockBuilderError(Arc::new( @@ -350,7 +354,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.clone(), }; batcher.validate_block(validate_block_input).await.unwrap(); @@ -383,7 +387,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.clone(), }) .await .unwrap(); @@ -548,7 +552,7 @@ async fn simulate_build_block_proposal( trait ProposalManagerTraitWrapper: Send + Sync { fn wrap_propose_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -558,7 +562,7 @@ trait ProposalManagerTraitWrapper: Send + Sync { fn wrap_validate_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -589,7 +593,7 @@ trait ProposalManagerTraitWrapper: Send + Sync { impl ProposalManagerTrait for T { async fn propose_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -597,7 +601,7 @@ impl ProposalManagerTrait for T { tx_provider: ProposeTransactionProvider, ) -> Result<(), GenerateProposalError> { self.wrap_propose_block( - height, + block_info, proposal_id, retrospective_block_hash, deadline, @@ -609,14 +613,14 @@ impl ProposalManagerTrait for T { async fn validate_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, tx_provider: ValidateTransactionProvider, ) -> Result<(), GenerateProposalError> { self.wrap_validate_block( - height, + block_info, proposal_id, retrospective_block_hash, deadline, diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs index 745fe8e68e..b4bb10063e 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, @@ -26,13 +25,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(std::num::TryFromIntError), #[error(transparent)] BlockifierStateError(#[from] StateError), #[error(transparent)] @@ -224,7 +215,7 @@ async fn collect_execution_results_and_stream_txs( } pub struct BlockMetadata { - pub height: BlockNumber, + pub block_info: BlockInfo, pub retrospective_block_hash: Option, } @@ -248,7 +239,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, } @@ -261,7 +251,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(), } @@ -279,12 +268,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, @@ -308,30 +291,20 @@ 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() - .map_err(BlockBuilderError::BadTimestamp)?, - ), + // TODO: Remove the overrides of the block info once once they are initialized. + let block_info = BlockInfo { 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, + ..block_metadata.block_info }; 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, @@ -339,7 +312,7 @@ impl BlockBuilderFactory { let state_reader = PapyrusReader::new( self.storage_reader.clone(), - block_metadata.height, + height, self.global_class_hash_to_class.clone(), ); @@ -363,7 +336,7 @@ impl BlockBuilderFactoryTrait for BlockBuilderFactory { output_content_sender: Option>, abort_signal_receiver: tokio::sync::oneshot::Receiver<()>, ) -> BlockBuilderResult> { - let executor = self.preprocess_and_create_transaction_executor(&block_metadata)?; + let executor = self.preprocess_and_create_transaction_executor(block_metadata)?; Ok(Box::new(BlockBuilder::new( Box::new(executor), tx_provider, diff --git a/crates/starknet_batcher/src/proposal_manager.rs b/crates/starknet_batcher/src/proposal_manager.rs index 5f1fc2407c..e8913e889e 100644 --- a/crates/starknet_batcher/src/proposal_manager.rs +++ b/crates/starknet_batcher/src/proposal_manager.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use async_trait::async_trait; use indexmap::IndexMap; -use starknet_api::block::{BlockHashAndNumber, BlockNumber}; +use starknet_api::block::{BlockHashAndNumber, BlockInfo}; use starknet_api::block_hash::state_diff_hash::calculate_state_diff_hash; use starknet_api::core::{ContractAddress, Nonce}; use starknet_api::executable_transaction::Transaction; @@ -63,7 +63,7 @@ pub(crate) enum InternalProposalStatus { pub trait ProposalManagerTrait: Send + Sync { async fn propose_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -73,7 +73,7 @@ pub trait ProposalManagerTrait: Send + Sync { async fn validate_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -108,7 +108,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 { @@ -141,7 +141,7 @@ impl ProposalManagerTrait for ProposalManager { #[instrument(skip(self, tx_sender, tx_provider), err, fields(self.active_height))] async fn propose_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -156,7 +156,7 @@ impl ProposalManagerTrait for ProposalManager { let (abort_signal_sender, abort_signal_receiver) = tokio::sync::oneshot::channel(); let block_builder = self.block_builder_factory.create_block_builder( - BlockMetadata { height, retrospective_block_hash }, + BlockMetadata { block_info, retrospective_block_hash }, BlockBuilderExecutionParams { deadline, fail_on_err: false }, Box::new(tx_provider), Some(tx_sender.clone()), @@ -174,7 +174,7 @@ impl ProposalManagerTrait for ProposalManager { #[instrument(skip(self, tx_provider), err, fields(self.active_height))] async fn validate_block( &mut self, - height: BlockNumber, + block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -188,7 +188,7 @@ impl ProposalManagerTrait for ProposalManager { let (abort_signal_sender, abort_signal_receiver) = tokio::sync::oneshot::channel(); let block_builder = self.block_builder_factory.create_block_builder( - BlockMetadata { height, retrospective_block_hash }, + BlockMetadata { block_info, retrospective_block_hash }, BlockBuilderExecutionParams { deadline, fail_on_err: true }, Box::new(tx_provider), None, diff --git a/crates/starknet_batcher/src/proposal_manager_test.rs b/crates/starknet_batcher/src/proposal_manager_test.rs index b3e52da2e3..4251c4cf29 100644 --- a/crates/starknet_batcher/src/proposal_manager_test.rs +++ b/crates/starknet_batcher/src/proposal_manager_test.rs @@ -1,8 +1,9 @@ -use std::sync::Arc; +use std::sync::{Arc, LazyLock}; use assert_matches::assert_matches; +use blockifier::test_utils::struct_impls::BlockInfoExt; use rstest::{fixture, rstest}; -use starknet_api::block::BlockNumber; +use starknet_api::block::{BlockInfo, BlockNumber}; use starknet_api::executable_transaction::Transaction; use starknet_batcher_types::batcher_types::ProposalId; use starknet_mempool_types::communication::MockMempoolClient; @@ -32,6 +33,9 @@ const BLOCK_GENERATION_TIMEOUT: tokio::time::Duration = tokio::time::Duration::f const MAX_L1_HANDLER_TXS_PER_BLOCK_PROPOSAL: usize = 3; const INPUT_CHANNEL_SIZE: usize = 30; +static INITIAL_BLOCK_INFO: LazyLock = + LazyLock::new(|| BlockInfo { block_number: INITIAL_HEIGHT, ..BlockInfo::create_for_testing() }); + #[fixture] fn output_streaming() -> ( tokio::sync::mpsc::UnboundedSender, @@ -118,7 +122,7 @@ async fn propose_block_non_blocking( let (output_sender, _receiver) = output_streaming(); proposal_manager .propose_block( - INITIAL_HEIGHT, + INITIAL_BLOCK_INFO.clone(), proposal_id, None, proposal_deadline(), @@ -144,7 +148,13 @@ async fn validate_block( proposal_id: ProposalId, ) { proposal_manager - .validate_block(INITIAL_HEIGHT, proposal_id, None, proposal_deadline(), tx_provider) + .validate_block( + INITIAL_BLOCK_INFO.clone(), + proposal_id, + None, + proposal_deadline(), + tx_provider, + ) .await .unwrap(); @@ -211,7 +221,7 @@ async fn multiple_proposals_generation_fail( let (output_sender_0, _rec_0) = output_streaming(); proposal_manager .propose_block( - INITIAL_HEIGHT, + INITIAL_BLOCK_INFO.clone(), ProposalId(0), None, proposal_deadline(), @@ -225,7 +235,7 @@ async fn multiple_proposals_generation_fail( let (output_sender_1, _rec_1) = output_streaming(); let another_generate_request = proposal_manager .propose_block( - INITIAL_HEIGHT, + INITIAL_BLOCK_INFO.clone(), ProposalId(1), None, proposal_deadline(),