diff --git a/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs b/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs index 752222e63e..ef578a1f5a 100644 --- a/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs +++ b/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs @@ -28,7 +28,15 @@ use papyrus_protobuf::consensus::{ TransactionBatch, Vote, }; -use starknet_api::block::{BlockHash, BlockHashAndNumber, BlockNumber}; +use starknet_api::block::{ + BlockHash, + BlockHashAndNumber, + BlockInfo, + BlockNumber, + GasPriceVector, + GasPrices, + NonzeroGasPrice, +}; use starknet_api::executable_transaction::Transaction; use starknet_batcher_types::batcher_types::{ DecisionReachedInput, @@ -45,6 +53,19 @@ use starknet_batcher_types::batcher_types::{ use starknet_batcher_types::communication::BatcherClient; use tracing::{debug, debug_span, error, info, trace, warn, Instrument}; +const TEMPORARY_GAS_PRICES: GasPrices = GasPrices { + eth_gas_prices: GasPriceVector { + l1_gas_price: NonzeroGasPrice::MIN, + l1_data_gas_price: NonzeroGasPrice::MIN, + l2_gas_price: NonzeroGasPrice::MIN, + }, + strk_gas_prices: GasPriceVector { + l1_gas_price: NonzeroGasPrice::MIN, + l1_data_gas_price: NonzeroGasPrice::MIN, + l2_gas_price: NonzeroGasPrice::MIN, + }, +}; + // {height: {proposal_id: (content, [proposal_ids])}} // Note that multiple proposals IDs can be associated with the same content, but we only need to // store one of them. @@ -115,8 +136,12 @@ impl ConsensusContext for SequencerConsensusContext { number: BlockNumber::default(), hash: BlockHash::default(), }), - // TODO: Fill block info. - block_info: Default::default(), + // TODO(Dan, Matan): Fill block info. + block_info: BlockInfo { + block_number: proposal_init.height, + gas_prices: TEMPORARY_GAS_PRICES, + ..Default::default() + }, }; self.maybe_start_height(proposal_init.height).await; // TODO: Should we be returning an error? @@ -174,8 +199,12 @@ impl ConsensusContext for SequencerConsensusContext { number: BlockNumber::default(), hash: BlockHash::default(), }), - // TODO: Fill block info. - block_info: Default::default(), + // TODO(Dan, Matan): Fill block info. + block_info: BlockInfo { + block_number: height, + gas_prices: TEMPORARY_GAS_PRICES, + ..Default::default() + }, }; self.maybe_start_height(height).await; batcher.validate_block(input).await.expect("Failed to initiate proposal validation"); diff --git a/crates/starknet_api/src/block.rs b/crates/starknet_api/src/block.rs index 74f72a462d..355acb86b8 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, Default, Serialize)] +#[derive(Clone, Debug, Deserialize, Default, 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 8a12399602..7585c46ea8 100644 --- a/crates/starknet_batcher/src/batcher.rs +++ b/crates/starknet_batcher/src/batcher.rs @@ -120,7 +120,11 @@ impl Batcher { propose_block_input: ProposeBlockInput, ) -> BatcherResult<()> { let active_height = self.active_height.ok_or(BatcherError::NoActiveHeight)?; - verify_block_input(active_height, propose_block_input.retrospective_block_hash)?; + verify_block_input( + active_height, + propose_block_input.block_info.block_number, + propose_block_input.retrospective_block_hash, + )?; let proposal_id = propose_block_input.proposal_id; let deadline = deadline_as_instant(propose_block_input.deadline)?; @@ -135,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, @@ -154,7 +158,11 @@ impl Batcher { validate_block_input: ValidateBlockInput, ) -> BatcherResult<()> { let active_height = self.active_height.ok_or(BatcherError::NoActiveHeight)?; - verify_block_input(active_height, validate_block_input.retrospective_block_hash)?; + verify_block_input( + active_height, + validate_block_input.block_info.block_number, + validate_block_input.retrospective_block_hash, + )?; let proposal_id = validate_block_input.proposal_id; let deadline = deadline_as_instant(validate_block_input.deadline)?; @@ -169,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, @@ -413,6 +421,17 @@ pub fn deadline_as_instant(deadline: chrono::DateTime) -> BatcherResult, +) -> BatcherResult<()> { + verify_non_empty_retrospective_block_hash(height, retrospective_block_hash)?; + verify_block_number(height, block_number)?; + + Ok(()) +} + +fn verify_non_empty_retrospective_block_hash( height: BlockNumber, retrospective_block_hash: Option, ) -> BatcherResult<()> { @@ -421,5 +440,14 @@ fn verify_block_input( { return Err(BatcherError::MissingRetrospectiveBlockHash); } + + Ok(()) +} + +fn verify_block_number(height: BlockNumber, block_number: BlockNumber) -> BatcherResult<()> { + 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 d31511a229..06dadf99d8 100644 --- a/crates/starknet_batcher/src/batcher_test.rs +++ b/crates/starknet_batcher/src/batcher_test.rs @@ -1,16 +1,17 @@ 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; use mockall::automock; use mockall::predicate::{always, eq}; use rstest::{fixture, rstest}; -use starknet_api::block::{BlockHashAndNumber, BlockNumber}; +use starknet_api::block::{BlockHashAndNumber, BlockInfo, BlockNumber}; use starknet_api::core::{ContractAddress, Nonce, StateDiffCommitment}; use starknet_api::executable_transaction::Transaction; use starknet_api::hash::PoseidonHash; @@ -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: 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: 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: 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 f78df38418..4557c0a068 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, BlockTimestamp}; use starknet_api::core::ContractAddress; use starknet_api::executable_transaction::Transaction; use starknet_api::transaction::TransactionHash; @@ -224,7 +217,7 @@ async fn collect_execution_results_and_stream_txs( } pub struct BlockMetadata { - pub height: BlockNumber, + pub block_info: BlockInfo, pub retrospective_block_hash: Option, } @@ -308,25 +301,22 @@ 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, + // TODO: Remove the overrides of the block info once once they are initialized. + let block_info = BlockInfo { 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, + ..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, @@ -334,7 +324,7 @@ impl BlockBuilderFactory { let state_reader = PapyrusReader::new( self.storage_reader.clone(), - block_metadata.height, + height, self.global_class_hash_to_class.clone(), ); @@ -358,7 +348,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(), diff --git a/crates/starknet_batcher_types/src/errors.rs b/crates/starknet_batcher_types/src/errors.rs index a3a028a815..b2fc3e5968 100644 --- a/crates/starknet_batcher_types/src/errors.rs +++ b/crates/starknet_batcher_types/src/errors.rs @@ -20,6 +20,8 @@ pub enum BatcherError { HeightInProgress, #[error("Internal server error.")] InternalError, + #[error("Invalid block number. The active height is {active_height}, got {block_number}.")] + InvalidBlockNumber { active_height: BlockNumber, block_number: BlockNumber }, #[error("Missing retrospective block hash.")] MissingRetrospectiveBlockHash, #[error("Attempt to start proposal with no active height.")]