Skip to content

Add additional broadcast validation tests for Fulu/PeerDAS #7325

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

Open
wants to merge 11 commits into
base: unstable
Choose a base branch
from
36 changes: 19 additions & 17 deletions beacon_node/http_api/src/publish_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,23 +228,25 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlock<T>>(
.into_iter()
.flatten()
.filter(|data_column| sampling_columns_indices.contains(&data_column.index()))
.collect();

// Importing the columns could trigger block import and network publication in the case
// where the block was already seen on gossip.
if let Err(e) =
Box::pin(chain.process_gossip_data_columns(sampling_columns, publish_fn)).await
{
let msg = format!("Invalid data column: {e}");
return if let BroadcastValidation::Gossip = validation_level {
Err(warp_utils::reject::broadcast_without_import(msg))
} else {
error!(
reason = &msg,
"Invalid data column during block publication"
);
Err(warp_utils::reject::custom_bad_request(msg))
};
.collect::<Vec<_>>();

if !sampling_columns.is_empty() {
// Importing the columns could trigger block import and network publication in the case
// where the block was already seen on gossip.
if let Err(e) =
Box::pin(chain.process_gossip_data_columns(sampling_columns, publish_fn)).await
{
let msg = format!("Invalid data column: {e}");
return if let BroadcastValidation::Gossip = validation_level {
Err(warp_utils::reject::broadcast_without_import(msg))
} else {
error!(
reason = &msg,
"Invalid data column during block publication"
);
Err(warp_utils::reject::custom_bad_request(msg))
};
}
}
}

Expand Down
100 changes: 84 additions & 16 deletions beacon_node/http_api/tests/broadcast_validation_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use beacon_chain::{
};
use eth2::reqwest::StatusCode;
use eth2::types::{BroadcastValidation, PublishBlockRequest};
use futures::future::join_all;
use http_api::test_utils::InteractiveTester;
use http_api::{publish_blinded_block, publish_block, reconstruct_block, Config, ProvenancedBlock};
use std::collections::HashSet;
Expand Down Expand Up @@ -1362,17 +1363,30 @@ pub async fn blinded_equivocation_full_pass() {
.block_is_known_to_fork_choice(&block.canonical_root()));
}

/// This test checks that an HTTP POST request with the block & blobs succeeds with a 200 response
/// even if the block has already been seen on gossip without any blobs.
/// This test checks that an HTTP POST request with the block & blobs/columns succeeds with a 200 response
/// even if the block has already been seen on gossip without any blobs/columns.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
pub async fn block_seen_on_gossip_without_blobs() {
pub async fn test_block_seen_on_gossip_without_blobs_or_columns() {
let deneb_enabled_forks = ForkName::list_all()
.into_iter()
.filter(|f| f.deneb_enabled())
.collect::<Vec<_>>();

let futures = deneb_enabled_forks
.into_iter()
.map(block_seen_on_gossip_without_blobs_or_columns);

join_all(futures).await;
}

pub async fn block_seen_on_gossip_without_blobs_or_columns(fork_name: ForkName) {
let validation_level: Option<BroadcastValidation> = Some(BroadcastValidation::Gossip);

// Validator count needs to be at least 32 or proposer boost gets set to 0 when computing
// `validator_count // 32`.
let validator_count = 64;
let num_initial: u64 = 31;
let spec = ForkName::latest_stable().make_genesis_spec(E::default_spec());
let spec = fork_name.make_genesis_spec(E::default_spec());
let tester = InteractiveTester::<E>::new(Some(spec), validator_count).await;

// Create some chain depth.
Expand Down Expand Up @@ -1424,17 +1438,30 @@ pub async fn block_seen_on_gossip_without_blobs() {
.block_is_known_to_fork_choice(&block.canonical_root()));
}

/// This test checks that an HTTP POST request with the block & blobs succeeds with a 200 response
/// even if the block has already been seen on gossip without all blobs.
/// This test checks that an HTTP POST request with the block & blobs/columns succeeds with a 200 response
/// even if the block has already been seen on gossip without all blobs/columns.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
pub async fn block_seen_on_gossip_with_some_blobs() {
pub async fn test_block_seen_on_gossip_with_some_blobs_or_columns() {
let deneb_enabled_forks = ForkName::list_all()
.into_iter()
.filter(|f| f.deneb_enabled())
.collect::<Vec<_>>();

let futures = deneb_enabled_forks
.into_iter()
.map(block_seen_on_gossip_with_some_blobs_or_columns);

join_all(futures).await;
}

pub async fn block_seen_on_gossip_with_some_blobs_or_columns(fork_name: ForkName) {
let validation_level: Option<BroadcastValidation> = Some(BroadcastValidation::Gossip);

// Validator count needs to be at least 32 or proposer boost gets set to 0 when computing
// `validator_count // 32`.
let validator_count = 64;
let num_initial: u64 = 31;
let spec = ForkName::latest_stable().make_genesis_spec(E::default_spec());
let spec = fork_name.make_genesis_spec(E::default_spec());
let tester = InteractiveTester::<E>::new(Some(spec), validator_count).await;

// Create some chain depth.
Expand Down Expand Up @@ -1504,17 +1531,30 @@ pub async fn block_seen_on_gossip_with_some_blobs() {
.block_is_known_to_fork_choice(&block.canonical_root()));
}

/// This test checks that an HTTP POST request with the block & blobs succeeds with a 200 response
/// even if the blobs have already been seen on gossip.
/// This test checks that an HTTP POST request with the block & blobs/columns succeeds with a 200 response
/// even if the blobs/columns have already been seen on gossip.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
pub async fn blobs_seen_on_gossip_without_block() {
pub async fn test_blobs_or_columns_seen_on_gossip_without_block() {
let deneb_enabled_forks = ForkName::list_all()
.into_iter()
.filter(|f| f.deneb_enabled())
.collect::<Vec<_>>();

let futures = deneb_enabled_forks
.into_iter()
.map(blobs_or_columns_seen_on_gossip_without_block);

join_all(futures).await;
}

pub async fn blobs_or_columns_seen_on_gossip_without_block(fork_name: ForkName) {
let spec = fork_name.make_genesis_spec(E::default_spec());
let validation_level: Option<BroadcastValidation> = Some(BroadcastValidation::Gossip);

// Validator count needs to be at least 32 or proposer boost gets set to 0 when computing
// `validator_count // 32`.
let validator_count = 64;
let num_initial: u64 = 31;
let spec = ForkName::latest_stable().make_genesis_spec(E::default_spec());
let tester = InteractiveTester::<E>::new(Some(spec), validator_count).await;

// Create some chain depth.
Expand Down Expand Up @@ -1573,14 +1613,29 @@ pub async fn blobs_seen_on_gossip_without_block() {
/// This test checks that an HTTP POST request with the block succeeds with a 200 response
/// if just the blobs have already been seen on gossip.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
pub async fn blobs_seen_on_gossip_without_block_and_no_http_blobs() {
pub async fn test_blobs_or_columns_seen_on_gossip_without_block_and_no_http_blobs_or_columns() {
let deneb_enabled_forks = ForkName::list_all()
.into_iter()
.filter(|f| f.deneb_enabled())
.collect::<Vec<_>>();

let futures = deneb_enabled_forks
.into_iter()
.map(blobs_or_columns_seen_on_gossip_without_block_and_no_http_blobs_or_columns);

join_all(futures).await;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this pattern feels a bit repetitive.

We could do what we do in the other tests like beacon_chain, op_pool, network, which is use test_spec to initialise the ChainSpec from the FORK_NAME environment variable. This would require us to add some makefile & CI support for the HTTP tests, and exclude them from make test-release.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep sounds good!

}

async fn blobs_or_columns_seen_on_gossip_without_block_and_no_http_blobs_or_columns(
fork_name: ForkName,
) {
let validation_level: Option<BroadcastValidation> = Some(BroadcastValidation::Gossip);

// Validator count needs to be at least 32 or proposer boost gets set to 0 when computing
// `validator_count // 32`.
let validator_count = 64;
let num_initial: u64 = 31;
let spec = ForkName::latest_stable().make_genesis_spec(E::default_spec());
let spec = fork_name.make_genesis_spec(E::default_spec());
let tester = InteractiveTester::<E>::new(Some(spec), validator_count).await;

// Create some chain depth.
Expand Down Expand Up @@ -1641,15 +1696,28 @@ pub async fn blobs_seen_on_gossip_without_block_and_no_http_blobs() {
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
pub async fn slashable_blobs_seen_on_gossip_cause_failure() {
pub async fn test_slashable_blobs_or_columns_seen_on_gossip_cause_failure() {
let deneb_enabled_forks = ForkName::list_all()
.into_iter()
.filter(|f| f.deneb_enabled())
.collect::<Vec<_>>();

let futures = deneb_enabled_forks
.into_iter()
.map(slashable_blobs_or_columns_seen_on_gossip_cause_failure);

join_all(futures).await;
}

async fn slashable_blobs_or_columns_seen_on_gossip_cause_failure(fork_name: ForkName) {
let validation_level: Option<BroadcastValidation> =
Some(BroadcastValidation::ConsensusAndEquivocation);

// Validator count needs to be at least 32 or proposer boost gets set to 0 when computing
// `validator_count // 32`.
let validator_count = 64;
let num_initial: u64 = 31;
let spec = ForkName::latest_stable().make_genesis_spec(E::default_spec());
let spec = fork_name.make_genesis_spec(E::default_spec());
let tester = InteractiveTester::<E>::new(Some(spec), validator_count).await;

// Create some chain depth.
Expand Down
Loading