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): fix error message on transaction provider error on l1 validation #4187

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
19 changes: 13 additions & 6 deletions crates/starknet_batcher/src/transaction_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,15 @@ type TransactionProviderResult<T> = Result<T, TransactionProviderError>;
pub enum TransactionProviderError {
#[error(transparent)]
MempoolError(#[from] MempoolClientError),
#[error("L1Handler transaction validation failed for tx with hash {0}.")]
L1HandlerTransactionValidationFailed(TransactionHash),
#[error(
"L1Handler transaction validation failed for tx with hash {} status {:?}.",
tx_hash.0.to_hex_string(),
validation_status
)]
L1HandlerTransactionValidationFailed {
tx_hash: TransactionHash,
validation_status: L1ValidationStatus,
},
#[error(transparent)]
L1ProviderError(#[from] L1ProviderClientError),
}
Expand Down Expand Up @@ -150,10 +157,10 @@ impl TransactionProvider for ValidateTransactionProvider {
let l1_validation_status =
self.l1_provider_client.validate(tx.tx_hash, self.height).await?;
if l1_validation_status != L1ValidationStatus::Validated {
// TODO(AlonH): add the validation status into the error.
return Err(TransactionProviderError::L1HandlerTransactionValidationFailed(
tx.tx_hash,
));
return Err(TransactionProviderError::L1HandlerTransactionValidationFailed {
tx_hash: tx.tx_hash,
validation_status: l1_validation_status,
});
}
}
}
Expand Down
16 changes: 12 additions & 4 deletions crates/starknet_batcher/src/transaction_provider_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,17 @@ async fn validate_flow(mut mock_dependencies: MockDependencies) {

#[rstest]
#[tokio::test]
async fn validate_fails(mut mock_dependencies: MockDependencies) {
async fn validate_fails(
mut mock_dependencies: MockDependencies,
#[values(
L1ValidationStatus::AlreadyIncludedInProposedBlock,
L1ValidationStatus::AlreadyIncludedOnL2,
L1ValidationStatus::ConsumedOnL1OrUnknown
)]
expected_validation_status: L1ValidationStatus,
) {
let test_tx = test_l1handler_tx();
mock_dependencies
.expect_validate_l1handler(test_tx.clone(), L1ValidationStatus::AlreadyIncludedOnL2);
mock_dependencies.expect_validate_l1handler(test_tx.clone(), expected_validation_status);
mock_dependencies
.simulate_input_txs(vec![
InternalConsensusTransaction::L1Handler(test_tx),
Expand All @@ -218,6 +225,7 @@ async fn validate_fails(mut mock_dependencies: MockDependencies) {
let result = validate_tx_provider.get_txs(MAX_TXS_PER_FETCH).await;
assert_matches!(
result,
Err(TransactionProviderError::L1HandlerTransactionValidationFailed(_tx_hash))
Err(TransactionProviderError::L1HandlerTransactionValidationFailed { validation_status, .. })
if validation_status == expected_validation_status
);
}
3 changes: 3 additions & 0 deletions crates/starknet_l1_provider_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ pub type L1ProviderResult<T> = Result<T, L1ProviderError>;
pub type L1ProviderClientResult<T> = Result<T, L1ProviderClientError>;
pub type SharedL1ProviderClient = Arc<dyn L1ProviderClient>;

// TODO(Arni): Consider splitting this enum into valid and invalid status where the invalid status
// holds the flavor of the invalidity. Propagate this change to
// [TransactionProviderError::L1HandlerTransactionValidationFailed].
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum ValidationStatus {
AlreadyIncludedInProposedBlock,
Expand Down
Loading