Skip to content

Commit

Permalink
refactor(starknet_batcher): move around some types and utils to a com…
Browse files Browse the repository at this point in the history
…mon utils file
  • Loading branch information
dafnamatsry committed Dec 10, 2024
1 parent fe25e29 commit 3bc9a7f
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 149 deletions.
82 changes: 9 additions & 73 deletions crates/starknet_batcher/src/batcher.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
use std::collections::HashMap;
use std::sync::Arc;

use blockifier::abi::constants;
use blockifier::state::global_cache::GlobalContractCache;
use chrono::Utc;
#[cfg(test)]
use mockall::automock;
use papyrus_storage::state::{StateStorageReader, StateStorageWriter};
use starknet_api::block::{BlockHashAndNumber, BlockNumber};
use starknet_api::block::BlockNumber;
use starknet_api::executable_transaction::Transaction;
use starknet_api::state::ThinStateDiff;
use starknet_batcher_types::batcher_types::{
Expand All @@ -33,26 +31,25 @@ use starknet_sequencer_infra::component_definitions::ComponentStarter;
use tracing::{debug, error, info, instrument, trace};

use crate::block_builder::{
BlockBuilderError,
BlockBuilderExecutionParams,
BlockBuilderFactory,
BlockBuilderFactoryTrait,
BlockMetadata,
};
use crate::config::BatcherConfig;
use crate::proposal_manager::{
GenerateProposalError,
ProposalError,
ProposalManager,
ProposalManagerTrait,
ProposalOutput,
ProposalResult,
};
use crate::proposal_manager::{GenerateProposalError, ProposalManager, ProposalManagerTrait};
use crate::transaction_provider::{
DummyL1ProviderClient,
ProposeTransactionProvider,
ValidateTransactionProvider,
};
use crate::utils::{
deadline_as_instant,
proposal_status_from,
verify_block_input,
ProposalOutput,
ProposalResult,
};

type OutputStreamReceiver = tokio::sync::mpsc::UnboundedReceiver<Transaction>;
type InputStreamSender = tokio::sync::mpsc::Sender<Transaction>;
Expand Down Expand Up @@ -458,65 +455,4 @@ impl From<GenerateProposalError> for BatcherError {
}
}

impl From<ProposalError> for BatcherError {
fn from(err: ProposalError) -> Self {
match err {
ProposalError::BlockBuilderError(..) => BatcherError::InternalError,
ProposalError::Aborted => BatcherError::ProposalAborted,
}
}
}

impl ComponentStarter for Batcher {}

pub fn deadline_as_instant(deadline: chrono::DateTime<Utc>) -> BatcherResult<tokio::time::Instant> {
let time_to_deadline = deadline - chrono::Utc::now();
let as_duration =
time_to_deadline.to_std().map_err(|_| BatcherError::TimeToDeadlineError { deadline })?;
Ok((std::time::Instant::now() + as_duration).into())
}

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<()> {
if height >= BlockNumber(constants::STORED_BLOCK_HASH_BUFFER)
&& retrospective_block_hash.is_none()
{
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(())
}

// Return the appropriate ProposalStatus for a given ProposalError.
fn proposal_status_from(proposal_error: ProposalError) -> BatcherResult<ProposalStatus> {
match proposal_error {
ProposalError::BlockBuilderError(err) => {
if let BlockBuilderError::FailOnError(_) = err.as_ref() {
// The proposal either failed due to bad input (e.g. invalid transactions), or
// couldn't finish in time.
Ok(ProposalStatus::InvalidProposal)
} else {
Err(BatcherError::InternalError)
}
}
ProposalError::Aborted => Err(BatcherError::ProposalAborted),
}
}
9 changes: 2 additions & 7 deletions crates/starknet_batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,10 @@ use crate::block_builder::{
MockBlockBuilderTrait,
};
use crate::config::BatcherConfig;
use crate::proposal_manager::{
GenerateProposalError,
ProposalError,
ProposalManagerTrait,
ProposalOutput,
ProposalResult,
};
use crate::proposal_manager::{GenerateProposalError, ProposalManagerTrait};
use crate::test_utils::test_txs;
use crate::transaction_provider::NextTxs;
use crate::utils::{ProposalError, ProposalOutput, ProposalResult};

const INITIAL_HEIGHT: BlockNumber = BlockNumber(3);
const STREAMING_CHUNK_SIZE: usize = 3;
Expand Down
1 change: 1 addition & 0 deletions crates/starknet_batcher/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod transaction_executor;
mod transaction_provider;
#[cfg(test)]
mod transaction_provider_test;
mod utils;

// Re-export so it can be used in the general config of the sequencer node without depending on
// blockifier.
Expand Down
66 changes: 4 additions & 62 deletions crates/starknet_batcher/src/proposal_manager.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
use std::collections::{HashMap, HashSet};
use std::collections::HashMap;
use std::sync::Arc;

use async_trait::async_trait;
use indexmap::IndexMap;
use starknet_api::block_hash::state_diff_hash::calculate_state_diff_hash;
use starknet_api::core::{ContractAddress, Nonce};
use starknet_api::state::ThinStateDiff;
use starknet_api::transaction::TransactionHash;
use starknet_batcher_types::batcher_types::{ProposalCommitment, ProposalId};
use starknet_batcher_types::batcher_types::ProposalId;
use thiserror::Error;
use tokio::sync::Mutex;
use tracing::{debug, error, info, instrument, Instrument};

use crate::block_builder::{BlockBuilderError, BlockBuilderTrait, BlockExecutionArtifacts};
use crate::block_builder::{BlockBuilderError, BlockBuilderTrait};
use crate::utils::{ProposalError, ProposalOutput, ProposalResult, ProposalTask};

#[derive(Debug, Error)]
pub enum GenerateProposalError {
Expand All @@ -32,14 +28,6 @@ pub enum GenerateProposalError {
ProposalAlreadyExists { proposal_id: ProposalId },
}

#[derive(Clone, Debug, Error)]
pub enum ProposalError {
#[error(transparent)]
BlockBuilderError(Arc<BlockBuilderError>),
#[error("Proposal was aborted")]
Aborted,
}

#[async_trait]
pub trait ProposalManagerTrait: Send + Sync {
async fn spawn_proposal(
Expand Down Expand Up @@ -68,12 +56,6 @@ pub trait ProposalManagerTrait: Send + Sync {
async fn reset(&mut self);
}

// Represents a spawned task of building new block proposal.
struct ProposalTask {
abort_signal_sender: tokio::sync::oneshot::Sender<()>,
join_handle: tokio::task::JoinHandle<()>,
}

/// Main struct for handling block proposals.
/// Taking care of:
/// - Proposing new blocks.
Expand All @@ -91,16 +73,6 @@ pub(crate) struct ProposalManager {
executed_proposals: Arc<Mutex<HashMap<ProposalId, ProposalResult<ProposalOutput>>>>,
}

pub type ProposalResult<T> = Result<T, ProposalError>;

#[derive(Debug, Default, PartialEq)]
pub struct ProposalOutput {
pub state_diff: ThinStateDiff,
pub commitment: ProposalCommitment,
pub tx_hashes: HashSet<TransactionHash>,
pub nonces: HashMap<ContractAddress, Nonce>,
}

#[async_trait]
impl ProposalManagerTrait for ProposalManager {
/// Starts a new block proposal generation task for the given proposal_id.
Expand Down Expand Up @@ -227,33 +199,3 @@ impl ProposalManager {
}
}
}

impl From<BlockExecutionArtifacts> for ProposalOutput {
fn from(artifacts: BlockExecutionArtifacts) -> Self {
let commitment_state_diff = artifacts.commitment_state_diff;
let nonces = HashMap::from_iter(
commitment_state_diff
.address_to_nonce
.iter()
.map(|(address, nonce)| (*address, *nonce)),
);

// TODO: Get these from the transactions.
let deployed_contracts = IndexMap::new();
let declared_classes = IndexMap::new();
let state_diff = ThinStateDiff {
deployed_contracts,
storage_diffs: commitment_state_diff.storage_updates,
declared_classes,
nonces: commitment_state_diff.address_to_nonce,
// TODO: Remove this when the structure of storage diffs changes.
deprecated_declared_classes: Vec::new(),
replaced_classes: IndexMap::new(),
};
let commitment =
ProposalCommitment { state_diff_commitment: calculate_state_diff_hash(&state_diff) };
let tx_hashes = HashSet::from_iter(artifacts.execution_infos.keys().copied());

Self { state_diff, commitment, tx_hashes, nonces }
}
}
9 changes: 2 additions & 7 deletions crates/starknet_batcher/src/proposal_manager_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,8 @@ use starknet_api::executable_transaction::Transaction;
use starknet_batcher_types::batcher_types::ProposalId;

use crate::block_builder::{BlockBuilderTrait, BlockExecutionArtifacts, MockBlockBuilderTrait};
use crate::proposal_manager::{
GenerateProposalError,
ProposalError,
ProposalManager,
ProposalManagerTrait,
ProposalOutput,
};
use crate::proposal_manager::{GenerateProposalError, ProposalManager, ProposalManagerTrait};
use crate::utils::{ProposalError, ProposalOutput};

const BLOCK_GENERATION_TIMEOUT: tokio::time::Duration = tokio::time::Duration::from_secs(1);

Expand Down
137 changes: 137 additions & 0 deletions crates/starknet_batcher/src/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
use std::collections::{HashMap, HashSet};
use std::sync::Arc;

use blockifier::abi::constants;
use chrono::Utc;
use indexmap::IndexMap;
use starknet_api::block::{BlockHashAndNumber, BlockNumber};
use starknet_api::block_hash::state_diff_hash::calculate_state_diff_hash;
use starknet_api::core::{ContractAddress, Nonce};
use starknet_api::state::ThinStateDiff;
use starknet_api::transaction::TransactionHash;
use starknet_batcher_types::batcher_types::{BatcherResult, ProposalCommitment, ProposalStatus};
use starknet_batcher_types::errors::BatcherError;
use thiserror::Error;

use crate::block_builder::{BlockBuilderError, BlockExecutionArtifacts};

pub(crate) type ProposalResult<T> = Result<T, ProposalError>;

// Represents an error completing a proposal.
#[derive(Clone, Debug, Error)]
pub(crate) enum ProposalError {
#[error(transparent)]
BlockBuilderError(Arc<BlockBuilderError>),
#[error("Proposal was aborted")]
Aborted,
}

impl From<ProposalError> for BatcherError {
fn from(err: ProposalError) -> Self {
match err {
ProposalError::BlockBuilderError(..) => BatcherError::InternalError,
ProposalError::Aborted => BatcherError::ProposalAborted,
}
}
}

// Represents a spawned task of building new block proposal.
pub(crate) struct ProposalTask {
pub abort_signal_sender: tokio::sync::oneshot::Sender<()>,
pub join_handle: tokio::task::JoinHandle<()>,
}

#[derive(Debug, Default, PartialEq)]
pub(crate) struct ProposalOutput {
pub state_diff: ThinStateDiff,
pub commitment: ProposalCommitment,
pub tx_hashes: HashSet<TransactionHash>,
pub nonces: HashMap<ContractAddress, Nonce>,
}

impl From<BlockExecutionArtifacts> for ProposalOutput {
fn from(artifacts: BlockExecutionArtifacts) -> Self {
let commitment_state_diff = artifacts.commitment_state_diff;
let nonces = HashMap::from_iter(
commitment_state_diff
.address_to_nonce
.iter()
.map(|(address, nonce)| (*address, *nonce)),
);

// TODO: Get these from the transactions.
let deployed_contracts = IndexMap::new();
let declared_classes = IndexMap::new();
let state_diff = ThinStateDiff {
deployed_contracts,
storage_diffs: commitment_state_diff.storage_updates,
declared_classes,
nonces: commitment_state_diff.address_to_nonce,
// TODO: Remove this when the structure of storage diffs changes.
deprecated_declared_classes: Vec::new(),
replaced_classes: IndexMap::new(),
};
let commitment =
ProposalCommitment { state_diff_commitment: calculate_state_diff_hash(&state_diff) };
let tx_hashes = HashSet::from_iter(artifacts.execution_infos.keys().copied());

Self { state_diff, commitment, tx_hashes, nonces }
}
}

pub(crate) fn deadline_as_instant(
deadline: chrono::DateTime<Utc>,
) -> BatcherResult<tokio::time::Instant> {
let time_to_deadline = deadline - chrono::Utc::now();
let as_duration =
time_to_deadline.to_std().map_err(|_| BatcherError::TimeToDeadlineError { deadline })?;
Ok((std::time::Instant::now() + as_duration).into())
}

pub(crate) 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(())
}

pub(crate) fn verify_non_empty_retrospective_block_hash(
height: BlockNumber,
retrospective_block_hash: Option<BlockHashAndNumber>,
) -> BatcherResult<()> {
if height >= BlockNumber(constants::STORED_BLOCK_HASH_BUFFER)
&& retrospective_block_hash.is_none()
{
return Err(BatcherError::MissingRetrospectiveBlockHash);
}
Ok(())
}

pub(crate) fn verify_block_number(
height: BlockNumber,
block_number: BlockNumber,
) -> BatcherResult<()> {
if block_number != height {
return Err(BatcherError::InvalidBlockNumber { active_height: height, block_number });
}
Ok(())
}

// Return the appropriate ProposalStatus for a given ProposalError.
pub(crate) fn proposal_status_from(proposal_error: ProposalError) -> BatcherResult<ProposalStatus> {
match proposal_error {
ProposalError::BlockBuilderError(err) => {
if let BlockBuilderError::FailOnError(_) = err.as_ref() {
// The proposal either failed due to bad input (e.g. invalid transactions), or
// couldn't finish in time.
Ok(ProposalStatus::InvalidProposal)
} else {
Err(BatcherError::InternalError)
}
}
ProposalError::Aborted => Err(BatcherError::ProposalAborted),
}
}

0 comments on commit 3bc9a7f

Please sign in to comment.