Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(starknet_batcher): pass block info from consensus to batcher #2238

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions config/sequencer/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
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, Default, Deserialize, Serialize)]
#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
pub struct BlockInfo {
pub block_number: BlockNumber,
pub block_timestamp: BlockTimestamp,
Expand Down
7 changes: 2 additions & 5 deletions crates/starknet_batcher/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl Batcher {
.block_builder_factory
.create_block_builder(
BlockMetadata {
height: active_height,
block_info: propose_block_input.block_info,
retrospective_block_hash: propose_block_input.retrospective_block_hash,
},
BlockBuilderExecutionParams {
Expand Down Expand Up @@ -196,7 +196,7 @@ impl Batcher {
.block_builder_factory
.create_block_builder(
BlockMetadata {
height: active_height,
block_info: validate_block_input.block_info,
retrospective_block_hash: validate_block_input.retrospective_block_hash,
},
BlockBuilderExecutionParams {
Expand Down Expand Up @@ -465,7 +465,6 @@ fn verify_block_input(
) -> BatcherResult<()> {
verify_non_empty_retrospective_block_hash(height, retrospective_block_hash)?;
verify_block_number(height, block_number)?;

Ok(())
}

Expand All @@ -478,14 +477,12 @@ fn verify_non_empty_retrospective_block_hash(
{
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(())
}
11 changes: 8 additions & 3 deletions crates/starknet_batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::sync::Arc;
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;
Expand Down Expand Up @@ -62,6 +63,10 @@ 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);

fn initial_block_info() -> BlockInfo {
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 @@ -263,7 +268,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(),
};
batcher.validate_block(validate_block_input).await.unwrap();

Expand Down Expand Up @@ -387,7 +392,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(),
};
batcher.validate_block(validate_block_input).await.unwrap();

Expand Down Expand Up @@ -424,7 +429,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(),
})
.await
.unwrap();
Expand Down
45 changes: 11 additions & 34 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 @@ -27,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};
use starknet_api::core::ContractAddress;
use starknet_api::executable_transaction::Transaction;
use starknet_api::transaction::TransactionHash;
Expand All @@ -45,8 +38,6 @@ use crate::transaction_provider::{NextTxs, TransactionProvider, TransactionProvi

#[derive(Debug, Error)]
pub enum BlockBuilderError {
#[error(transparent)]
BadTimestamp(#[from] std::num::TryFromIntError),
#[error(transparent)]
BlockifierStateError(#[from] StateError),
#[error(transparent)]
Expand Down Expand Up @@ -238,7 +229,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 All @@ -263,7 +254,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,
}
Expand All @@ -276,7 +266,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(),
}
Expand All @@ -294,12 +283,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,
Expand All @@ -323,33 +306,27 @@ 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,
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,
};
let mut block_info = block_metadata.block_info;
// TODO(Arni): Remove this override and the corresponding config.
block_info.sequencer_address = block_builder_config.sequencer_address;

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 @@ -372,7 +349,7 @@ impl BlockBuilderFactoryTrait for BlockBuilderFactory {
tx_provider: Box<dyn TransactionProvider>,
output_content_sender: Option<tokio::sync::mpsc::UnboundedSender<Transaction>>,
) -> BlockBuilderResult<(Box<dyn BlockBuilderTrait>, AbortSignalSender)> {
let executor = self.preprocess_and_create_transaction_executor(&block_metadata)?;
let executor = self.preprocess_and_create_transaction_executor(block_metadata)?;
let (abort_signal_sender, abort_signal_receiver) = tokio::sync::oneshot::channel();
let block_builder = Box::new(BlockBuilder::new(
Box::new(executor),
Expand Down
2 changes: 1 addition & 1 deletion crates/starknet_batcher/src/proposal_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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 {
Expand Down
Loading