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 28, 2024
1 parent 0326d0b commit ec9a40a
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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");
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
36 changes: 32 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.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.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.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.block_info,
proposal_id,
validate_block_input.retrospective_block_hash,
deadline,
Expand Down Expand Up @@ -413,6 +421,17 @@ pub fn deadline_as_instant(deadline: chrono::DateTime<Utc>) -> BatcherResult<tok
}

fn verify_block_input(
height: BlockNumber,
block_number: BlockNumber,
retrospective_block_hash: Option<BlockHashAndNumber>,
) -> 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<BlockHashAndNumber>,
) -> BatcherResult<()> {
Expand All @@ -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(())
}
30 changes: 17 additions & 13 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 INITIAL_BLOCK_INFO: 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,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 {
Expand Down Expand Up @@ -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();

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

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<BlockHashAndNumber>,
deadline: tokio::time::Instant,
Expand All @@ -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<BlockHashAndNumber>,
deadline: tokio::time::Instant,
Expand Down Expand Up @@ -589,15 +593,15 @@ trait ProposalManagerTraitWrapper: Send + Sync {
impl<T: ProposalManagerTraitWrapper> ProposalManagerTrait for T {
async fn propose_block(
&mut self,
height: BlockNumber,
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,
block_info,
proposal_id,
retrospective_block_hash,
deadline,
Expand All @@ -609,14 +613,14 @@ impl<T: ProposalManagerTraitWrapper> ProposalManagerTrait for T {

async fn validate_block(
&mut self,
height: BlockNumber,
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,
block_info,
proposal_id,
retrospective_block_hash,
deadline,
Expand Down
30 changes: 10 additions & 20 deletions crates/starknet_batcher/src/block_builder.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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<BlockHashAndNumber>,
}

Expand Down Expand Up @@ -308,33 +301,30 @@ pub struct BlockBuilderFactory {
impl BlockBuilderFactory {
fn preprocess_and_create_transaction_executor(
&self,
block_metadata: &BlockMetadata,
block_metadata: BlockMetadata,
) -> BlockBuilderResult<TransactionExecutor<PapyrusReader>> {
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,
);

let state_reader = PapyrusReader::new(
self.storage_reader.clone(),
block_metadata.height,
height,
self.global_class_hash_to_class.clone(),
);

Expand All @@ -358,7 +348,7 @@ impl BlockBuilderFactoryTrait for BlockBuilderFactory {
output_content_sender: Option<tokio::sync::mpsc::UnboundedSender<Transaction>>,
abort_signal_receiver: tokio::sync::oneshot::Receiver<()>,
) -> BlockBuilderResult<Box<dyn BlockBuilderTrait>> {
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,
Expand Down
Loading

0 comments on commit ec9a40a

Please sign in to comment.