From dd634816231c62edcd7a44fb18d5636374a6a736 Mon Sep 17 00:00:00 2001 From: Yael Doweck Date: Tue, 12 Nov 2024 14:38:38 +0200 Subject: [PATCH] refactor(batcher): change BuildProposalError to GenerateProposalError, to fit it to the validate flow --- crates/batcher/src/batcher.rs | 14 ++++++------- crates/batcher/src/batcher_test.rs | 8 +++---- crates/batcher/src/proposal_manager.rs | 23 +++++++++++++-------- crates/batcher/src/proposal_manager_test.rs | 6 +++--- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/crates/batcher/src/batcher.rs b/crates/batcher/src/batcher.rs index c448283ec9..065ba604a6 100644 --- a/crates/batcher/src/batcher.rs +++ b/crates/batcher/src/batcher.rs @@ -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, @@ -314,19 +314,19 @@ impl From for BatcherError { } } -impl From for BatcherError { - fn from(err: BuildProposalError) -> Self { +impl From 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 } } } diff --git a/crates/batcher/src/batcher_test.rs b/crates/batcher/src/batcher_test.rs index 93552bbe59..1c25ff48c2 100644 --- a/crates/batcher/src/batcher_test.rs +++ b/crates/batcher/src/batcher_test.rs @@ -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, @@ -226,7 +226,7 @@ async fn decision_reached_no_executed_proposal( async fn simulate_build_block_proposal( tx_sender: tokio::sync::mpsc::UnboundedSender, txs: Vec, -) -> Result<(), BuildProposalError> { +) -> Result<(), GenerateProposalError> { tokio::spawn(async move { for tx in txs { tx_sender.send(tx).unwrap(); @@ -249,7 +249,7 @@ trait ProposalManagerTraitWrapper: Send + Sync { retrospective_block_hash: Option, deadline: tokio::time::Instant, output_content_sender: tokio::sync::mpsc::UnboundedSender, - ) -> BoxFuture<'_, Result<(), BuildProposalError>>; + ) -> BoxFuture<'_, Result<(), GenerateProposalError>>; fn wrap_take_proposal_result( &mut self, @@ -276,7 +276,7 @@ impl ProposalManagerTrait for T { retrospective_block_hash: Option, deadline: tokio::time::Instant, output_content_sender: tokio::sync::mpsc::UnboundedSender, - ) -> Result<(), BuildProposalError> { + ) -> Result<(), GenerateProposalError> { self.wrap_build_block_proposal( proposal_id, retrospective_block_hash, diff --git a/crates/batcher/src/proposal_manager.rs b/crates/batcher/src/proposal_manager.rs index 9b5b5369b7..2fc7f841a4 100644 --- a/crates/batcher/src/proposal_manager.rs +++ b/crates/batcher/src/proposal_manager.rs @@ -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)] @@ -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}." @@ -78,7 +83,7 @@ pub trait ProposalManagerTrait: Send + Sync { retrospective_block_hash: Option, deadline: tokio::time::Instant, tx_sender: tokio::sync::mpsc::UnboundedSender, - ) -> Result<(), BuildProposalError>; + ) -> Result<(), GenerateProposalError>; async fn take_proposal_result( &mut self, @@ -164,7 +169,7 @@ impl ProposalManagerTrait for ProposalManager { retrospective_block_hash: Option, deadline: tokio::time::Instant, tx_sender: tokio::sync::mpsc::UnboundedSender, - ) -> Result<(), BuildProposalError> { + ) -> Result<(), GenerateProposalError> { self.set_active_proposal(proposal_id).await?; info!("Starting generation of a new proposal with id {}.", proposal_id); @@ -250,7 +255,7 @@ impl ProposalManager { &mut self, proposal_id: ProposalId, mut block_builder: Box, - ) -> Result<(), BuildProposalError> { + ) -> Result<(), GenerateProposalError> { let active_proposal = self.active_proposal.clone(); let executed_proposals = self.executed_proposals.clone(); @@ -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, }); diff --git a/crates/batcher/src/proposal_manager_test.rs b/crates/batcher/src/proposal_manager_test.rs index a30c27f2f8..9a13b9334f 100644 --- a/crates/batcher/src/proposal_manager_test.rs +++ b/crates/batcher/src/proposal_manager_test.rs @@ -16,7 +16,7 @@ use crate::block_builder::{ MockBlockBuilderTrait, }; use crate::proposal_manager::{ - BuildProposalError, + GenerateProposalError, GetProposalResultError, ProposalManager, ProposalManagerTrait, @@ -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] @@ -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)