Skip to content

Commit

Permalink
fix(conductor): fix flaky soft_and_firm test (#1472)
Browse files Browse the repository at this point in the history 
## Summary
Fixed flakiness in conductor `soft_and_firm::simple` test.

## Background
Test would previously fail ~6% of the time due to receiving firm block
before soft block, hence never fulfilling the soft update commitment
mock.

## Changes
- Split `simple()` into 2 tests:
1. Tests for receiving soft block first, and then firm block (and
enforcing this order).
2. Tests for receiving firm block first, then ignoring the later soft
block response at the same height. The tests for receiving the soft
block at the next height.
- made changes to `astria-grpc` to enable a delayed response


## Testing
All tests passing.

## Related Issues
closes #1143
  • Loading branch information
@ethanoroshiba
ethanoroshiba committed Sep 26, 2024
1 parent bda4ffc commit 33dae42
Show file tree
Hide file tree
Showing 8 changed files with 375 additions and 113 deletions.
76 changes: 60 additions & 16 deletions crates/astria-conductor/tests/blackbox/helpers/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,36 @@ macro_rules! mount_celestia_blobs {
celestia_height: $celestia_height:expr,
sequencer_heights: [ $($sequencer_height:expr),+ ]
$(,)?
) => {
mount_celestia_blobs!(
$test_env,
celestia_height: $celestia_height,
sequencer_heights: [ $($sequencer_height),+ ],
delay: None,
)
};
(
$test_env:ident,
celestia_height: $celestia_height:expr,
sequencer_heights: [ $($sequencer_height:expr),+ ],
delay: $delay:expr
$(,)?
) => {{
let blobs = $crate::helpers::make_blobs(&[ $( $sequencer_height ),+ ]);
$test_env
.mount_celestia_blob_get_all(
$celestia_height,
$crate::sequencer_namespace(),
vec![blobs.header],
$delay,
)
.await;
$test_env
.mount_celestia_blob_get_all(
$celestia_height,
$crate::rollup_namespace(),
vec![blobs.rollup],
$delay,
)
.await
}};
Expand Down Expand Up @@ -177,13 +193,47 @@ macro_rules! mount_get_commitment_state {

#[macro_export]
macro_rules! mount_update_commitment_state {
(
$test_env:ident,
firm: ( number: $firm_number:expr, hash: $firm_hash:expr, parent: $firm_parent:expr$(,)? ),
soft: ( number: $soft_number:expr, hash: $soft_hash:expr, parent: $soft_parent:expr$(,)? ),
base_celestia_height: $base_celestia_height:expr
$(,)?
) => {
mount_update_commitment_state!(
$test_env,
mock_name: None,
firm: ( number: $firm_number, hash: $firm_hash, parent: $firm_parent, ),
soft: ( number: $soft_number, hash: $soft_hash, parent: $soft_parent, ),
base_celestia_height: $base_celestia_height,
expected_calls: 1,
)
};
(
$test_env:ident,
mock_name: $mock_name:expr,
firm: ( number: $firm_number:expr, hash: $firm_hash:expr, parent: $firm_parent:expr$(,)? ),
soft: ( number: $soft_number:expr, hash: $soft_hash:expr, parent: $soft_parent:expr$(,)? ),
base_celestia_height: $base_celestia_height:expr
$(,)?
) => {
mount_update_commitment_state!(
$test_env,
mock_name: $mock_name,
firm: ( number: $firm_number, hash: $firm_hash, parent: $firm_parent, ),
soft: ( number: $soft_number, hash: $soft_hash, parent: $soft_parent, ),
base_celestia_height: $base_celestia_height,
expected_calls: 1,
)
};
(
$test_env:ident,
mock_name: $mock_name:expr,
firm: ( number: $firm_number:expr, hash: $firm_hash:expr, parent: $firm_parent:expr$(,)? ),
soft: ( number: $soft_number:expr, hash: $soft_hash:expr, parent: $soft_parent:expr$(,)? ),
base_celestia_height: $base_celestia_height:expr,
expected_calls: $expected_calls:expr
$(,)?
) => {
$test_env
.mount_update_commitment_state(
Expand All @@ -201,24 +251,10 @@ macro_rules! mount_update_commitment_state {
),
base_celestia_height: $base_celestia_height,
),
$expected_calls,
)
.await
};
(
$test_env:ident,
firm: ( number: $firm_number:expr, hash: $firm_hash:expr, parent: $firm_parent:expr$(,)? ),
soft: ( number: $soft_number:expr, hash: $soft_hash:expr, parent: $soft_parent:expr$(,)? ),
base_celestia_height: $base_celestia_height:expr
$(,)?
) => {
mount_update_commitment_state!(
$test_env,
mock_name: None,
firm: ( number: $firm_number, hash: $firm_hash, parent: $firm_parent, ),
soft: ( number: $soft_number, hash: $soft_hash, parent: $soft_parent, ),
base_celestia_height: $base_celestia_height,
)
};
}

#[macro_export]
Expand Down Expand Up @@ -270,17 +306,25 @@ macro_rules! mount_executed_block {

#[macro_export]
macro_rules! mount_get_filtered_sequencer_block {
($test_env:ident, sequencer_height: $height:expr $(,)?) => {
($test_env:ident, sequencer_height: $height:expr, delay: $delay:expr $(,)?) => {
$test_env
.mount_get_filtered_sequencer_block(
::astria_core::generated::sequencerblock::v1alpha1::GetFilteredSequencerBlockRequest {
height: $height,
rollup_ids: vec![$crate::ROLLUP_ID.to_raw()],
},
$crate::filtered_sequencer_block!(sequencer_height: $height),
$delay,
)
.await;
};
($test_env:ident, sequencer_height: $height:expr$(,)?) => {
mount_get_filtered_sequencer_block!(
$test_env,
sequencer_height: $height,
delay: Duration::from_secs(0),
)
};
}

#[macro_export]
Expand Down
43 changes: 14 additions & 29 deletions crates/astria-conductor/tests/blackbox/helpers/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,7 @@ use astria_core::{
},
primitive::v1::RollupId,
};
use astria_grpc_mock::{
response::ResponseResult,
AnyMessage,
Respond,
};
use astria_grpc_mock::response::error_response;
use bytes::Bytes;
use celestia_types::{
nmt::Namespace,
Expand Down Expand Up @@ -198,6 +194,7 @@ impl TestConductor {
celestia_height: u64,
namespace: Namespace,
blobs: Vec<Blob>,
delay: Option<Duration>,
) {
use base64::prelude::*;
use wiremock::{
Expand All @@ -209,6 +206,7 @@ impl TestConductor {
Request,
ResponseTemplate,
};
let delay = delay.unwrap_or(Duration::from_millis(0));
let namespace_params = BASE64_STANDARD.encode(namespace.as_bytes());
Mock::given(body_partial_json(json!({
"jsonrpc": "2.0",
Expand All @@ -222,11 +220,13 @@ impl TestConductor {
.respond_with(move |request: &Request| {
let body: serde_json::Value = serde_json::from_slice(&request.body).unwrap();
let id = body.get("id");
ResponseTemplate::new(200).set_body_json(json!({
"jsonrpc": "2.0",
"id": id,
"result": blobs,
}))
ResponseTemplate::new(200)
.set_body_json(json!({
"jsonrpc": "2.0",
"id": id,
"result": blobs,
}))
.set_delay(delay)
})
.expect(1..)
.mount(&self.mock_http)
Expand Down Expand Up @@ -407,6 +407,7 @@ impl TestConductor {
&self,
expected_pbjson: S,
response: FilteredSequencerBlock,
delay: Duration,
) {
use astria_grpc_mock::{
matcher::message_partial_pbjson,
Expand All @@ -417,7 +418,7 @@ impl TestConductor {
"get_filtered_sequencer_block",
message_partial_pbjson(&expected_pbjson),
)
.respond_with(constant_response(response))
.respond_with(constant_response(response).set_delay(delay))
.expect(1..)
.mount(&self.mock_grpc.mock_server)
.await;
Expand All @@ -427,6 +428,7 @@ impl TestConductor {
&self,
mock_name: Option<&str>,
commitment_state: CommitmentState,
expected_calls: u64,
) -> astria_grpc_mock::MockGuard {
use astria_core::generated::execution::v1alpha2::UpdateCommitmentStateRequest;
use astria_grpc_mock::{
Expand All @@ -444,7 +446,7 @@ impl TestConductor {
if let Some(name) = mock_name {
mock = mock.with_name(name);
}
mock.expect(1)
mock.expect(expected_calls)
.mount_as_scoped(&self.mock_grpc.mock_server)
.await
}
Expand Down Expand Up @@ -697,20 +699,3 @@ pub fn rollup_namespace() -> Namespace {
pub fn sequencer_namespace() -> Namespace {
astria_core::celestia::namespace_v0_from_sha256_of_bytes(SEQUENCER_CHAIN_ID.as_bytes())
}

pub struct ErrorResponse {
status: tonic::Status,
}

impl Respond for ErrorResponse {
fn respond(&self, _req: &tonic::Request<AnyMessage>) -> ResponseResult {
Err(self.status.clone())
}
}

#[must_use]
pub fn error_response(code: tonic::Code) -> ErrorResponse {
ErrorResponse {
status: tonic::Status::new(code, "error"),
}
}
Loading

0 comments on commit 33dae42

Please sign in to comment.