Skip to content

Commit

Permalink
refactor(batcher): change BuildProposalError to GenerateProposalError…
Browse files Browse the repository at this point in the history
…, to fit it to the validate flow
  • Loading branch information
Yael-Starkware committed Nov 12, 2024
1 parent 5bd1781 commit dd63481
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 23 deletions.
14 changes: 7 additions & 7 deletions crates/batcher/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use tracing::{debug, error, info, instrument, trace};
use crate::block_builder::BlockBuilderFactory;
use crate::config::BatcherConfig;
use crate::proposal_manager::{
BuildProposalError,
GenerateProposalError,
GetProposalResultError,
ProposalManager,
ProposalManagerTrait,
Expand Down Expand Up @@ -314,19 +314,19 @@ impl From<StartHeightError> for BatcherError {
}
}

impl From<BuildProposalError> for BatcherError {
fn from(err: BuildProposalError) -> Self {
impl From<GenerateProposalError> for BatcherError {
fn from(err: GenerateProposalError) -> Self {
match err {
BuildProposalError::AlreadyGeneratingProposal {
GenerateProposalError::AlreadyGeneratingProposal {
current_generating_proposal_id,
new_proposal_id,
} => BatcherError::ServerBusy {
active_proposal_id: current_generating_proposal_id,
new_proposal_id,
},
BuildProposalError::BlockBuilderError(..) => BatcherError::InternalError,
BuildProposalError::NoActiveHeight => BatcherError::NoActiveHeight,
BuildProposalError::ProposalAlreadyExists { proposal_id } => {
GenerateProposalError::BlockBuilderError(..) => BatcherError::InternalError,
GenerateProposalError::NoActiveHeight => BatcherError::NoActiveHeight,
GenerateProposalError::ProposalAlreadyExists { proposal_id } => {
BatcherError::ProposalAlreadyExists { proposal_id }
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use starknet_mempool_types::mempool_types::CommitBlockArgs;
use crate::batcher::{Batcher, MockBatcherStorageReaderTrait, MockBatcherStorageWriterTrait};
use crate::config::BatcherConfig;
use crate::proposal_manager::{
BuildProposalError,
GenerateProposalError,
GetProposalResultError,
ProposalManagerTrait,
ProposalOutput,
Expand Down Expand Up @@ -226,7 +226,7 @@ async fn decision_reached_no_executed_proposal(
async fn simulate_build_block_proposal(
tx_sender: tokio::sync::mpsc::UnboundedSender<Transaction>,
txs: Vec<Transaction>,
) -> Result<(), BuildProposalError> {
) -> Result<(), GenerateProposalError> {
tokio::spawn(async move {
for tx in txs {
tx_sender.send(tx).unwrap();
Expand All @@ -249,7 +249,7 @@ trait ProposalManagerTraitWrapper: Send + Sync {
retrospective_block_hash: Option<BlockHashAndNumber>,
deadline: tokio::time::Instant,
output_content_sender: tokio::sync::mpsc::UnboundedSender<Transaction>,
) -> BoxFuture<'_, Result<(), BuildProposalError>>;
) -> BoxFuture<'_, Result<(), GenerateProposalError>>;

fn wrap_take_proposal_result(
&mut self,
Expand All @@ -276,7 +276,7 @@ impl<T: ProposalManagerTraitWrapper> ProposalManagerTrait for T {
retrospective_block_hash: Option<BlockHashAndNumber>,
deadline: tokio::time::Instant,
output_content_sender: tokio::sync::mpsc::UnboundedSender<Transaction>,
) -> Result<(), BuildProposalError> {
) -> Result<(), GenerateProposalError> {
self.wrap_build_block_proposal(
proposal_id,
retrospective_block_hash,
Expand Down
23 changes: 14 additions & 9 deletions crates/batcher/src/proposal_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ use tokio::sync::Mutex;
use tracing::{debug, error, info, instrument, Instrument};

use crate::batcher::BatcherStorageReaderTrait;
use crate::block_builder::{BlockBuilderError, BlockBuilderFactoryTrait, BlockBuilderTrait, BlockExecutionArtifacts};
use crate::block_builder::{
BlockBuilderError,
BlockBuilderFactoryTrait,
BlockBuilderTrait,
BlockExecutionArtifacts,
};
use crate::transaction_provider::ProposeTransactionProvider;

#[derive(Debug, Error)]
Expand All @@ -36,7 +41,7 @@ pub enum StartHeightError {
}

#[derive(Debug, Error)]
pub enum BuildProposalError {
pub enum GenerateProposalError {
#[error(
"Received proposal generation request with id {new_proposal_id} while already generating \
proposal with id {current_generating_proposal_id}."
Expand Down Expand Up @@ -78,7 +83,7 @@ pub trait ProposalManagerTrait: Send + Sync {
retrospective_block_hash: Option<BlockHashAndNumber>,
deadline: tokio::time::Instant,
tx_sender: tokio::sync::mpsc::UnboundedSender<Transaction>,
) -> Result<(), BuildProposalError>;
) -> Result<(), GenerateProposalError>;

async fn take_proposal_result(
&mut self,
Expand Down Expand Up @@ -164,7 +169,7 @@ impl ProposalManagerTrait for ProposalManager {
retrospective_block_hash: Option<BlockHashAndNumber>,
deadline: tokio::time::Instant,
tx_sender: tokio::sync::mpsc::UnboundedSender<Transaction>,
) -> Result<(), BuildProposalError> {
) -> Result<(), GenerateProposalError> {
self.set_active_proposal(proposal_id).await?;

info!("Starting generation of a new proposal with id {}.", proposal_id);
Expand Down Expand Up @@ -250,7 +255,7 @@ impl ProposalManager {
&mut self,
proposal_id: ProposalId,
mut block_builder: Box<dyn BlockBuilderTrait>,
) -> Result<(), BuildProposalError> {
) -> Result<(), GenerateProposalError> {
let active_proposal = self.active_proposal.clone();
let executed_proposals = self.executed_proposals.clone();

Expand Down Expand Up @@ -291,16 +296,16 @@ impl ProposalManager {
async fn set_active_proposal(
&mut self,
proposal_id: ProposalId,
) -> Result<(), BuildProposalError> {
self.active_height.ok_or(BuildProposalError::NoActiveHeight)?;
) -> Result<(), GenerateProposalError> {
self.active_height.ok_or(GenerateProposalError::NoActiveHeight)?;

if self.executed_proposals.lock().await.contains_key(&proposal_id) {
return Err(BuildProposalError::ProposalAlreadyExists { proposal_id });
return Err(GenerateProposalError::ProposalAlreadyExists { proposal_id });
}

let mut active_proposal = self.active_proposal.lock().await;
if let Some(current_generating_proposal_id) = *active_proposal {
return Err(BuildProposalError::AlreadyGeneratingProposal {
return Err(GenerateProposalError::AlreadyGeneratingProposal {
current_generating_proposal_id,
new_proposal_id: proposal_id,
});
Expand Down
6 changes: 3 additions & 3 deletions crates/batcher/src/proposal_manager_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::block_builder::{
MockBlockBuilderTrait,
};
use crate::proposal_manager::{
BuildProposalError,
GenerateProposalError,
GetProposalResultError,
ProposalManager,
ProposalManagerTrait,
Expand Down Expand Up @@ -157,7 +157,7 @@ async fn proposal_generation_fails_without_start_height(
let err = proposal_manager
.build_block_proposal(ProposalId(0), None, proposal_deadline(), output_streaming.0)
.await;
assert_matches!(err, Err(BuildProposalError::NoActiveHeight));
assert_matches!(err, Err(GenerateProposalError::NoActiveHeight));
}

#[rstest]
Expand Down Expand Up @@ -213,7 +213,7 @@ async fn multiple_proposals_generation_fail(mut mock_dependencies: MockDependenc
.await;
assert_matches!(
another_generate_request,
Err(BuildProposalError::AlreadyGeneratingProposal {
Err(GenerateProposalError::AlreadyGeneratingProposal {
current_generating_proposal_id,
new_proposal_id
}) if current_generating_proposal_id == ProposalId(0) && new_proposal_id == ProposalId(1)
Expand Down

0 comments on commit dd63481

Please sign in to comment.