Skip to content

Commit

Permalink
fix(batcher): treat deadline as error in the validate flow
Browse files Browse the repository at this point in the history
  • Loading branch information
Yael-Starkware committed Nov 19, 2024
1 parent cbb54e6 commit ba48389
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 16 deletions.
2 changes: 1 addition & 1 deletion crates/starknet_batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ async fn send_finish_to_an_invalid_proposal() {
.return_once(|_, _, _, _| { async move { Ok(()) } }.boxed());

let proposal_error = GetProposalResultError::BlockBuilderError(Arc::new(
BlockBuilderError::FailOnError(TransactionExecutorError::BlockFull),
BlockBuilderError::FailOnError(TransactionExecutorError::BlockFull.to_string()),
));
proposal_manager
.expect_wrap_await_proposal_commitment()
Expand Down
16 changes: 10 additions & 6 deletions crates/starknet_batcher/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use blockifier::context::{BlockContext, ChainInfo};
use blockifier::state::cached_state::CommitmentStateDiff;
use blockifier::state::errors::StateError;
use blockifier::state::global_cache::GlobalContractCache;
use blockifier::transaction::errors::TransactionExecutionError as BlockifierTransactionExecutionError;
use blockifier::transaction::objects::TransactionExecutionInfo;
use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction;
use blockifier::versioned_constants::{VersionedConstants, VersionedConstantsOverrides};
Expand Down Expand Up @@ -48,11 +47,9 @@ pub enum BlockBuilderError {
#[error(transparent)]
GetTransactionError(#[from] TransactionProviderError),
#[error(transparent)]
TransactionExecutionError(#[from] BlockifierTransactionExecutionError),
#[error(transparent)]
StreamTransactionsError(#[from] tokio::sync::mpsc::error::SendError<Transaction>),
#[error("Build block with fail_on_err mode, failed on error {}.", _0)]
FailOnError(BlockifierTransactionExecutorError),
FailOnError(String),
#[error("The block builder was aborted.")]
Aborted,
}
Expand Down Expand Up @@ -120,7 +117,14 @@ impl BlockBuilderTrait for BlockBuilder {
let mut block_is_full = false;
let mut execution_infos = IndexMap::new();
// TODO(yael 6/10/2024): delete the timeout condition once the executor has a timeout
while !block_is_full && tokio::time::Instant::now() < self.execution_params.deadline {
while !block_is_full {
if tokio::time::Instant::now() >= self.execution_params.deadline {
info!("Block builder deadline reached.");
if self.execution_params.fail_on_err {
return Err(BlockBuilderError::FailOnError("deadline reached".to_string()));
}
break;
}
if self.abort_signal_receiver.try_recv().is_ok() {
info!("Received abort signal. Aborting block builder.");
return Err(BlockBuilderError::Aborted);
Expand Down Expand Up @@ -189,7 +193,7 @@ async fn collect_execution_results_and_stream_txs(
Err(err) => {
debug!("Transaction {:?} failed with error: {}.", input_tx, err);
if fail_on_err {
return Err(BlockBuilderError::FailOnError(err));
return Err(BlockBuilderError::FailOnError(err.to_string()));
}
}
}
Expand Down
48 changes: 39 additions & 9 deletions crates/starknet_batcher/src/block_builder_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,21 +152,27 @@ fn block_full_test_expectations(
(mock_transaction_executor, mock_tx_provider, expected_block_artifacts)
}

fn test_expectations_with_delay(
input_txs: &[Transaction],
) -> (MockTransactionExecutorTrait, MockTransactionProvider, BlockExecutionArtifacts) {
let first_chunk = input_txs[0..TX_CHUNK_SIZE].to_vec();
let first_chunk_copy = first_chunk.clone();
fn mock_transaction_executor_with_delay(
input_txs: Vec<Transaction>,
) -> MockTransactionExecutorTrait {
let mut mock_transaction_executor = MockTransactionExecutorTrait::new();

mock_transaction_executor
.expect_add_txs_to_block()
.times(1)
.withf(move |blockifier_input| compare_tx_hashes(&first_chunk, blockifier_input))
.withf(move |blockifier_input| compare_tx_hashes(&input_txs, blockifier_input))
.return_once(move |_| {
std::thread::sleep(std::time::Duration::from_secs(BLOCK_GENERATION_DEADLINE_SECS));
(0..TX_CHUNK_SIZE).map(move |_| Ok(execution_info())).collect()
});
mock_transaction_executor
}

fn test_expectations_with_delay(
input_txs: &[Transaction],
) -> (MockTransactionExecutorTrait, MockTransactionProvider, BlockExecutionArtifacts) {
let first_chunk = input_txs[0..TX_CHUNK_SIZE].to_vec();
let first_chunk_copy = first_chunk.clone();
let mut mock_transaction_executor = mock_transaction_executor_with_delay(first_chunk);

let expected_block_artifacts =
set_close_block_expectations(&mut mock_transaction_executor, TX_CHUNK_SIZE);
Expand Down Expand Up @@ -416,8 +422,32 @@ async fn test_validate_block_with_error() {
.unwrap_err();

assert_matches!(
result,
BlockBuilderError::FailOnError(BlockifierTransactionExecutorError::BlockFull)
result, BlockBuilderError::FailOnError(err)
if err == BlockifierTransactionExecutorError::BlockFull.to_string()
);
}

#[tokio::test]
async fn test_validate_block_deadline_reached() {
let input_txs = test_txs(0..3);
let mock_transaction_executor = mock_transaction_executor_with_delay(input_txs.clone());
let mock_tx_provider = mock_tx_provider_limited_calls(1, vec![input_txs]);

let (_abort_sender, abort_receiver) = tokio::sync::oneshot::channel();
let result = run_build_block(
mock_transaction_executor,
mock_tx_provider,
None,
true,
abort_receiver,
BLOCK_GENERATION_DEADLINE_SECS,
)
.await
.unwrap_err();

assert_matches!(
result, BlockBuilderError::FailOnError(err)
if err == *"deadline reached"
);
}

Expand Down

0 comments on commit ba48389

Please sign in to comment.