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 2b7cdaff55..45aa8ad3b3 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,7 @@ use papyrus_protobuf::consensus::{ TransactionBatch, Vote, }; -use starknet_api::block::{BlockHash, BlockHashAndNumber, BlockNumber}; +use starknet_api::block::{BlockHash, BlockHashAndNumber, BlockInfo, BlockNumber}; use starknet_api::executable_transaction::Transaction; use starknet_batcher_types::batcher_types::{ DecisionReachedInput, @@ -116,7 +116,7 @@ impl ConsensusContext for SequencerConsensusContext { hash: BlockHash::default(), }), // TODO: Fill block info. - block_info: Default::default(), + next_block_info: BlockInfo { block_number: proposal_init.height, ..Default::default() }, }; self.maybe_start_height(proposal_init.height).await; // TODO: Should we be returning an error? @@ -175,7 +175,7 @@ impl ConsensusContext for SequencerConsensusContext { hash: BlockHash::default(), }), // TODO: Fill block info. - block_info: Default::default(), + next_block_info: BlockInfo { block_number: height, ..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..abb71e0af1 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.next_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.next_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.next_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.next_block_info, proposal_id, validate_block_input.retrospective_block_hash, deadline, @@ -412,8 +420,15 @@ pub fn deadline_as_instant(deadline: chrono::DateTime) -> BatcherResult, ) -> BatcherResult<()> { if height >= BlockNumber(constants::STORED_BLOCK_HASH_BUFFER) @@ -421,5 +436,12 @@ fn verify_block_input( { return Err(BatcherError::MissingRetrospectiveBlockHash); } + if next_block_number != height { + return Err(BatcherError::InvalidNextBlockNumber { + active_height: height, + next_block_number, + }); + } + Ok(()) } diff --git a/crates/starknet_batcher/src/batcher_test.rs b/crates/starknet_batcher/src/batcher_test.rs index d31511a229..e70694b425 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 BLOCK_INFO_AT_INITIAL_HEIGHT: 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,13 @@ 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(BLOCK_INFO_AT_INITIAL_HEIGHT.clone()), + eq(PROPOSAL_ID), + eq(None), + always(), + always(), + ) .return_once(|_, _, _, _, tx_provider| { { async move { @@ -200,7 +210,7 @@ async fn no_active_height() { proposal_id: ProposalId(0), retrospective_block_hash: None, deadline: chrono::Utc::now() + chrono::Duration::seconds(1), - block_info: Default::default(), + next_block_info: Default::default(), }) .await; assert_eq!(result, Err(BatcherError::NoActiveHeight)); @@ -210,7 +220,7 @@ async fn no_active_height() { proposal_id: ProposalId(0), retrospective_block_hash: None, deadline: chrono::Utc::now() + chrono::Duration::seconds(1), - block_info: Default::default(), + next_block_info: Default::default(), }) .await; assert_eq!(result, Err(BatcherError::NoActiveHeight)); @@ -231,7 +241,7 @@ async fn validate_block_full_flow() { proposal_id: PROPOSAL_ID, deadline: deadline(), retrospective_block_hash: None, - block_info: Default::default(), + next_block_info: BLOCK_INFO_AT_INITIAL_HEIGHT.clone(), }; batcher.validate_block(validate_block_input).await.unwrap(); @@ -331,7 +341,13 @@ 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(BLOCK_INFO_AT_INITIAL_HEIGHT.clone()), + eq(PROPOSAL_ID), + eq(None), + always(), + always(), + ) .return_once(|_, _, _, _, _| { async move { Ok(()) } }.boxed()); let proposal_error = GetProposalResultError::BlockBuilderError(Arc::new( @@ -350,7 +366,7 @@ async fn send_finish_to_an_invalid_proposal() { proposal_id: PROPOSAL_ID, deadline: deadline(), retrospective_block_hash: None, - block_info: Default::default(), + next_block_info: BLOCK_INFO_AT_INITIAL_HEIGHT.clone(), }; batcher.validate_block(validate_block_input).await.unwrap(); @@ -383,7 +399,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(), + next_block_info: BLOCK_INFO_AT_INITIAL_HEIGHT.clone(), }) .await .unwrap(); @@ -443,7 +459,7 @@ async fn propose_block_without_retrospective_block_hash() { proposal_id: PROPOSAL_ID, retrospective_block_hash: None, deadline: deadline(), - block_info: Default::default(), + next_block_info: Default::default(), }) .await; @@ -548,7 +564,7 @@ async fn simulate_build_block_proposal( trait ProposalManagerTraitWrapper: Send + Sync { fn wrap_propose_block( &mut self, - height: BlockNumber, + next_block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -558,7 +574,7 @@ trait ProposalManagerTraitWrapper: Send + Sync { fn wrap_validate_block( &mut self, - height: BlockNumber, + next_block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -589,7 +605,7 @@ trait ProposalManagerTraitWrapper: Send + Sync { impl ProposalManagerTrait for T { async fn propose_block( &mut self, - height: BlockNumber, + next_block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, @@ -597,7 +613,7 @@ impl ProposalManagerTrait for T { tx_provider: ProposeTransactionProvider, ) -> Result<(), GenerateProposalError> { self.wrap_propose_block( - height, + next_block_info, proposal_id, retrospective_block_hash, deadline, @@ -609,14 +625,14 @@ impl ProposalManagerTrait for T { async fn validate_block( &mut self, - height: BlockNumber, + next_block_info: BlockInfo, proposal_id: ProposalId, retrospective_block_hash: Option, deadline: tokio::time::Instant, tx_provider: ValidateTransactionProvider, ) -> Result<(), GenerateProposalError> { self.wrap_validate_block( - height, + next_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..998fa9ae1a 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -26,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, BlockTimestamp, NonzeroGasPrice}; use starknet_api::core::ContractAddress; use starknet_api::executable_transaction::Transaction; use starknet_api::transaction::TransactionHash; @@ -224,7 +218,7 @@ async fn collect_execution_results_and_stream_txs( } pub struct BlockMetadata { - pub height: BlockNumber, + pub next_block_info: BlockInfo, pub retrospective_block_hash: Option, } @@ -312,14 +306,18 @@ impl BlockBuilderFactory { ) -> BlockBuilderResult> { let block_builder_config = self.block_builder_config.clone(); let next_block_info = BlockInfo { - block_number: block_metadata.height, + block_number: block_metadata.next_block_info.block_number, + // TODO: Use block_metadata.next_block_info.block_timestamp once it is initialized. block_timestamp: BlockTimestamp(chrono::Utc::now().timestamp().try_into()?), + // TODO: Use block_metadata.next_block_info.block_hash once it is initialized as the + // sequencer address. sequencer_address: block_builder_config.sequencer_address, - // TODO (yael 7/10/2024): add logic to compute gas prices + // TODO: Use block_metadata.next_block_info.gas_prices once it is initialized. gas_prices: { let tmp_val = NonzeroGasPrice::MIN; validated_gas_prices(tmp_val, tmp_val, tmp_val, tmp_val, tmp_val, tmp_val) }, + // TODO: Use block_metadata.next_block_info.use_kzg_da once it is initialized. use_kzg_da: block_builder_config.use_kzg_da, }; let versioned_constants = VersionedConstants::get_versioned_constants( @@ -334,7 +332,7 @@ impl BlockBuilderFactory { let state_reader = PapyrusReader::new( self.storage_reader.clone(), - block_metadata.height, + block_metadata.next_block_info.block_number, self.global_class_hash_to_class.clone(), ); diff --git a/crates/starknet_batcher/src/proposal_manager.rs b/crates/starknet_batcher/src/proposal_manager.rs index 5f1fc2407c..fb8a36c02d 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, + next_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, + next_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, + next_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 { next_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, + next_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 { next_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..bce44b9ee6 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 BLOCK_INFO_AT_INITIAL_HEIGHT: 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, + BLOCK_INFO_AT_INITIAL_HEIGHT.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( + BLOCK_INFO_AT_INITIAL_HEIGHT.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, + BLOCK_INFO_AT_INITIAL_HEIGHT.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, + BLOCK_INFO_AT_INITIAL_HEIGHT.clone(), ProposalId(1), None, proposal_deadline(), diff --git a/crates/starknet_batcher_types/src/batcher_types.rs b/crates/starknet_batcher_types/src/batcher_types.rs index 53d11e3ade..94f645dfae 100644 --- a/crates/starknet_batcher_types/src/batcher_types.rs +++ b/crates/starknet_batcher_types/src/batcher_types.rs @@ -35,7 +35,7 @@ pub struct ProposeBlockInput { pub proposal_id: ProposalId, pub deadline: chrono::DateTime, pub retrospective_block_hash: Option, - pub block_info: BlockInfo, + pub next_block_info: BlockInfo, } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -61,7 +61,7 @@ pub struct ValidateBlockInput { pub proposal_id: ProposalId, pub deadline: chrono::DateTime, pub retrospective_block_hash: Option, - pub block_info: BlockInfo, + pub next_block_info: BlockInfo, } #[derive(Clone, Debug, Serialize, Deserialize)] diff --git a/crates/starknet_batcher_types/src/errors.rs b/crates/starknet_batcher_types/src/errors.rs index a3a028a815..1192cdfc99 100644 --- a/crates/starknet_batcher_types/src/errors.rs +++ b/crates/starknet_batcher_types/src/errors.rs @@ -20,6 +20,10 @@ pub enum BatcherError { HeightInProgress, #[error("Internal server error.")] InternalError, + #[error( + "Invalid next block number. The active height is {active_height}, got {next_block_number}." + )] + InvalidNextBlockNumber { active_height: BlockNumber, next_block_number: BlockNumber }, #[error("Missing retrospective block hash.")] MissingRetrospectiveBlockHash, #[error("Attempt to start proposal with no active height.")]