diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index a94a19900c1..e7c95e88299 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -159,6 +159,28 @@ jobs: - name: Show cache stats if: env.SELF_HOSTED_RUNNERS == 'true' run: sccache --show-stats + http-api-tests: + name: http-api-tests + needs: [check-labels] + if: needs.check-labels.outputs.skip_ci != 'true' + # Use self-hosted runners only on the sigp repo. + runs-on: ${{ github.repository == 'sigp/lighthouse' && fromJson('["self-hosted", "linux", "CI", "large"]') || 'ubuntu-latest' }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v4 + - name: Get latest version of stable Rust + if: env.SELF_HOSTED_RUNNERS == 'false' + uses: moonrepo/setup-rust@v1 + with: + channel: stable + cache-target: release + bins: cargo-nextest + - name: Run http_api tests for all recent forks + run: make test-http-api + - name: Show cache stats + if: env.SELF_HOSTED_RUNNERS == 'true' + run: sccache --show-stats op-pool-tests: name: op-pool-tests needs: [check-labels] @@ -475,6 +497,7 @@ jobs: 'op-pool-tests', 'network-tests', 'slasher-tests', + 'http-api-tests', 'debug-tests-ubuntu', 'state-transition-vectors-ubuntu', 'ef-tests-ubuntu', diff --git a/Makefile b/Makefile index fe5dfbe551f..744d92f49f3 100644 --- a/Makefile +++ b/Makefile @@ -34,6 +34,9 @@ PROFILE ?= release # they run for different forks. FORKS=phase0 altair bellatrix capella deneb electra fulu +# List of all recent hard forks. This list is used to set env variables for http_api tests +RECENT_FORKS=deneb electra fulu + # Extra flags for Cargo CARGO_INSTALL_EXTRA_FLAGS?= @@ -141,24 +144,26 @@ build-release-tarballs: test-release: cargo test --workspace --release --features "$(TEST_FEATURES)" \ --exclude ef_tests --exclude beacon_chain --exclude slasher --exclude network + --exclude http_api # Runs the full workspace tests in **release**, without downloading any additional # test vectors, using nextest. nextest-release: cargo nextest run --workspace --release --features "$(TEST_FEATURES)" \ - --exclude ef_tests --exclude beacon_chain --exclude slasher --exclude network + --exclude ef_tests --exclude beacon_chain --exclude slasher --exclude network \ + --exclude http_api # Runs the full workspace tests in **debug**, without downloading any additional test # vectors. test-debug: cargo test --workspace --features "$(TEST_FEATURES)" \ - --exclude ef_tests --exclude beacon_chain --exclude network + --exclude ef_tests --exclude beacon_chain --exclude network --exclude http_api # Runs the full workspace tests in **debug**, without downloading any additional test # vectors, using nextest. nextest-debug: cargo nextest run --workspace --features "$(TEST_FEATURES)" \ - --exclude ef_tests --exclude beacon_chain --exclude network + --exclude ef_tests --exclude beacon_chain --exclude network --exclude http_api # Runs cargo-fmt (linter). cargo-fmt: @@ -188,6 +193,13 @@ test-beacon-chain: $(patsubst %,test-beacon-chain-%,$(FORKS)) test-beacon-chain-%: env FORK_NAME=$* cargo nextest run --release --features "fork_from_env,slasher/lmdb,$(TEST_FEATURES)" -p beacon_chain +# Run the tests in the `beacon_chain` crate for all known forks. +test-http-api: $(patsubst %,test-beacon-chain-%,$(RECENT_FORKS)) + +test-http-api-%: + env FORK_NAME=$* cargo nextest run --release --features "fork_from_env,slasher/lmdb,$(TEST_FEATURES)" -p http_api + + # Run the tests in the `operation_pool` crate for all known forks. test-op-pool: $(patsubst %,test-op-pool-%,$(FORKS)) diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 9b1a3f8677c..547caf0cb8d 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -229,23 +229,25 @@ pub async fn publish_block>( .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::>(); + + 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)) + }; + } } } diff --git a/beacon_node/http_api/tests/broadcast_validation_tests.rs b/beacon_node/http_api/tests/broadcast_validation_tests.rs index cd590580be4..32447e5885d 100644 --- a/beacon_node/http_api/tests/broadcast_validation_tests.rs +++ b/beacon_node/http_api/tests/broadcast_validation_tests.rs @@ -1,3 +1,4 @@ +use beacon_chain::test_utils::test_spec; use beacon_chain::{ test_utils::{AttestationStrategy, BlockStrategy}, GossipVerifiedBlock, IntoGossipVerifiedBlock, @@ -52,7 +53,8 @@ pub async fn gossip_invalid() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -102,7 +104,8 @@ pub async fn gossip_partial_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -148,7 +151,8 @@ pub async fn gossip_full_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -238,7 +242,8 @@ pub async fn consensus_invalid() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -287,7 +292,8 @@ pub async fn consensus_gossip() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -333,7 +339,8 @@ pub async fn consensus_partial_pass_only_consensus() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -404,7 +411,8 @@ pub async fn consensus_full_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -450,7 +458,8 @@ pub async fn equivocation_invalid() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -500,7 +509,8 @@ pub async fn equivocation_consensus_early_equivocation() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -574,7 +584,8 @@ pub async fn equivocation_gossip() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -624,7 +635,8 @@ pub async fn equivocation_consensus_late_equivocation() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -700,7 +712,8 @@ pub async fn equivocation_full_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -745,7 +758,8 @@ pub async fn blinded_gossip_invalid() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -794,7 +808,8 @@ pub async fn blinded_gossip_partial_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -840,7 +855,8 @@ pub async fn blinded_gossip_full_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -881,7 +897,8 @@ pub async fn blinded_gossip_full_pass_ssz() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -923,7 +940,8 @@ pub async fn blinded_consensus_invalid() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -972,7 +990,8 @@ pub async fn blinded_consensus_gossip() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -1018,7 +1037,8 @@ pub async fn blinded_consensus_full_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -1061,7 +1081,8 @@ pub async fn blinded_equivocation_invalid() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -1111,7 +1132,8 @@ pub async fn blinded_equivocation_consensus_early_equivocation() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -1181,7 +1203,8 @@ pub async fn blinded_equivocation_gossip() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -1234,7 +1257,8 @@ pub async fn blinded_equivocation_consensus_late_equivocation() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -1330,7 +1354,8 @@ pub async fn blinded_equivocation_full_pass() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); @@ -1362,18 +1387,23 @@ 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 block_seen_on_gossip_without_blobs_or_columns() { + let spec = test_spec::(); let validation_level: Option = 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::::new(Some(spec), validator_count).await; + let tester = InteractiveTester::::new(Some(spec.clone()), validator_count).await; + let state = tester.harness.get_current_state(); + let fork_name = state.fork_name(&spec).unwrap(); + if !fork_name.deneb_enabled() { + return; + } // Create some chain depth. tester.harness.advance_slot(); @@ -1424,18 +1454,23 @@ 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 block_seen_on_gossip_with_some_blobs_or_columns() { + let spec = test_spec::(); let validation_level: Option = 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::::new(Some(spec), validator_count).await; + let tester = InteractiveTester::::new(Some(spec.clone()), validator_count).await; + let state = tester.harness.get_current_state(); + let fork_name = state.fork_name(&spec).unwrap(); + if !fork_name.deneb_enabled() { + return; + } // Create some chain depth. tester.harness.advance_slot(); @@ -1504,18 +1539,23 @@ 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 blobs_or_columns_seen_on_gossip_without_block() { + let spec = test_spec::(); let validation_level: Option = 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::::new(Some(spec), validator_count).await; + let tester = InteractiveTester::::new(Some(spec.clone()), validator_count).await; + let state = tester.harness.get_current_state(); + let fork_name = state.fork_name(&spec).unwrap(); + if !fork_name.deneb_enabled() { + return; + } // Create some chain depth. tester.harness.advance_slot(); @@ -1573,15 +1613,20 @@ 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() { +async fn blobs_or_columns_seen_on_gossip_without_block_and_no_http_blobs_or_columns() { + let spec = test_spec::(); let validation_level: Option = 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::::new(Some(spec), validator_count).await; + let tester = InteractiveTester::::new(Some(spec.clone()), validator_count).await; + let state = tester.harness.get_current_state(); + let fork_name = state.fork_name(&spec).unwrap(); + if !fork_name.deneb_enabled() { + return; + } // Create some chain depth. tester.harness.advance_slot(); @@ -1641,7 +1686,7 @@ 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() { +async fn slashable_blobs_or_columns_seen_on_gossip_cause_failure() { let validation_level: Option = Some(BroadcastValidation::ConsensusAndEquivocation); @@ -1649,8 +1694,13 @@ pub async fn slashable_blobs_seen_on_gossip_cause_failure() { // `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::::new(Some(spec), validator_count).await; + let spec = test_spec::(); + let tester = InteractiveTester::::new(Some(spec.clone()), validator_count).await; + let state = tester.harness.get_current_state(); + let fork_name = state.fork_name(&spec).unwrap(); + if !fork_name.deneb_enabled() { + return; + } // Create some chain depth. tester.harness.advance_slot(); @@ -1717,7 +1767,7 @@ pub async fn duplicate_block_status_code() { // `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 = test_spec::(); let duplicate_block_status_code = StatusCode::IM_A_TEAPOT; let tester = InteractiveTester::::new_with_initializer_and_mutator( Some(spec),