Skip to content

Commit

Permalink
chore(batcher): use block info to create block info
Browse files Browse the repository at this point in the history
  • Loading branch information
ArniStarkware committed Nov 25, 2024
1 parent 36467b0 commit 8c52c40
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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");
Expand Down
9 changes: 5 additions & 4 deletions crates/starknet_api/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
30 changes: 26 additions & 4 deletions crates/starknet_batcher/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand All @@ -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,
Expand All @@ -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)?;
Expand All @@ -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,
Expand Down Expand Up @@ -412,14 +420,28 @@ pub fn deadline_as_instant(deadline: chrono::DateTime<Utc>) -> BatcherResult<tok
Ok((std::time::Instant::now() + as_duration).into())
}

/// Validates block input by ensuring:
/// 1. `retrospective_block_hash` is provided for blocks beyond the
/// [`STORED_BLOCK_HASH_BUFFER`](constants::STORED_BLOCK_HASH_BUFFER).
/// 2. `next_block_number` matches the current `height`.
///
/// Returns an error if either condition is violated.
fn verify_block_input(
height: BlockNumber,
next_block_number: BlockNumber,
retrospective_block_hash: Option<BlockHashAndNumber>,
) -> BatcherResult<()> {
if height >= BlockNumber(constants::STORED_BLOCK_HASH_BUFFER)
&& retrospective_block_hash.is_none()
{
return Err(BatcherError::MissingRetrospectiveBlockHash);
}
if next_block_number != height {
return Err(BatcherError::InvalidNextBlockNumber {
active_height: height,
next_block_number,
});
}

Ok(())
}
48 changes: 32 additions & 16 deletions crates/starknet_batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<BlockInfo> =
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()))),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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();

Expand Down Expand Up @@ -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(
Expand All @@ -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();

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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<BlockHashAndNumber>,
deadline: tokio::time::Instant,
Expand All @@ -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<BlockHashAndNumber>,
deadline: tokio::time::Instant,
Expand Down Expand Up @@ -589,15 +605,15 @@ trait ProposalManagerTraitWrapper: Send + Sync {
impl<T: ProposalManagerTraitWrapper> ProposalManagerTrait for T {
async fn propose_block(
&mut self,
height: BlockNumber,
next_block_info: BlockInfo,
proposal_id: ProposalId,
retrospective_block_hash: Option<BlockHashAndNumber>,
deadline: tokio::time::Instant,
output_content_sender: tokio::sync::mpsc::UnboundedSender<Transaction>,
tx_provider: ProposeTransactionProvider,
) -> Result<(), GenerateProposalError> {
self.wrap_propose_block(
height,
next_block_info,
proposal_id,
retrospective_block_hash,
deadline,
Expand All @@ -609,14 +625,14 @@ impl<T: ProposalManagerTraitWrapper> ProposalManagerTrait for T {

async fn validate_block(
&mut self,
height: BlockNumber,
next_block_info: BlockInfo,
proposal_id: ProposalId,
retrospective_block_hash: Option<BlockHashAndNumber>,
deadline: tokio::time::Instant,
tx_provider: ValidateTransactionProvider,
) -> Result<(), GenerateProposalError> {
self.wrap_validate_block(
height,
next_block_info,
proposal_id,
retrospective_block_hash,
deadline,
Expand Down
20 changes: 9 additions & 11 deletions crates/starknet_batcher/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<BlockHashAndNumber>,
}

Expand Down Expand Up @@ -312,14 +306,18 @@ impl BlockBuilderFactory {
) -> BlockBuilderResult<TransactionExecutor<PapyrusReader>> {
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(
Expand All @@ -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(),
);

Expand Down
Loading

0 comments on commit 8c52c40

Please sign in to comment.