From 8edefb7e0d6b367418891a51a605b177c0e81cf9 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 2 May 2023 10:00:07 -0400 Subject: [PATCH] make tests work for all forks --- .github/workflows/test-suite.yml | 14 + Makefile | 9 +- beacon_node/beacon_chain/src/test_utils.rs | 2 +- .../src/subnet_service/attestation_subnets.rs | 9 +- .../src/subnet_service/sync_subnets.rs | 2 +- .../network/src/subnet_service/tests/mod.rs | 1 + .../network/src/sync/block_lookups/tests.rs | 1487 +++++++++-------- 7 files changed, 847 insertions(+), 677 deletions(-) diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index aab5dafe4d..620e83c784 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -118,6 +118,20 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} - name: Run operation_pool tests for all known forks run: make test-op-pool + network-minimal-tests: + name: network-minimal-tests + runs-on: ubuntu-latest + needs: cargo-fmt + steps: + - uses: actions/checkout@v3 + - name: Get latest version of stable Rust + run: rustup update stable + - name: Install Protoc + uses: arduino/setup-protoc@e52d9eb8f7b63115df1ac544a1376fdbf5a39612 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Run network tests for all known forks using the minimal spec + run: make test-network-minimal slasher-tests: name: slasher-tests runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index 95df251889..73e896b539 100644 --- a/Makefile +++ b/Makefile @@ -106,7 +106,7 @@ build-release-tarballs: # Runs the full workspace tests in **release**, without downloading any additional # test vectors. test-release: - cargo test --workspace --release --exclude ef_tests --exclude beacon_chain --exclude slasher + cargo test --workspace --release --exclude ef_tests --exclude beacon_chain --exclude slasher # Runs the full workspace tests in **debug**, without downloading any additional test # vectors. @@ -143,6 +143,13 @@ test-op-pool-%: --features 'beacon_chain/fork_from_env'\ -p operation_pool +test-network-minimal: $(patsubst %,test-network-minimal-%,$(FORKS)) + +test-network-minimal-%: + env FORK_NAME=$* cargo test --release \ + --features 'fork_from_env,spec-minimal'\ + -p network + # Run the tests in the `slasher` crate for all supported database backends. test-slasher: cargo test --release -p slasher --features mdbx diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index db502fcda5..d6d993a635 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -61,7 +61,7 @@ use types::{typenum::U4294967296, *}; // 4th September 2019 pub const HARNESS_GENESIS_TIME: u64 = 1_567_552_690; // Environment variable to read if `fork_from_env` feature is enabled. -const FORK_NAME_ENV_VAR: &str = "FORK_NAME"; +pub const FORK_NAME_ENV_VAR: &str = "FORK_NAME"; // Default target aggregators to set during testing, this ensures an aggregator at each slot. // diff --git a/beacon_node/network/src/subnet_service/attestation_subnets.rs b/beacon_node/network/src/subnet_service/attestation_subnets.rs index 70ba1c8170..2ebb20e64e 100644 --- a/beacon_node/network/src/subnet_service/attestation_subnets.rs +++ b/beacon_node/network/src/subnet_service/attestation_subnets.rs @@ -3,7 +3,10 @@ //! determines whether attestations should be aggregated and/or passed to the beacon node. use super::SubnetServiceMessage; -#[cfg(any(test, feature = "deterministic_long_lived_attnets"))] +#[cfg(any( + all(test, feature = "spec-mainnet"), + feature = "deterministic_long_lived_attnets" +))] use std::collections::HashSet; use std::collections::{HashMap, VecDeque}; use std::pin::Pin; @@ -201,7 +204,7 @@ impl AttestationService { } /// Return count of all currently subscribed subnets (long-lived **and** short-lived). - #[cfg(test)] + #[cfg(all(test, feature = "spec-mainnet"))] pub fn subscription_count(&self) -> usize { if self.subscribe_all_subnets { self.beacon_chain.spec.attestation_subnet_count as usize @@ -225,7 +228,7 @@ impl AttestationService { } /// Returns whether we are subscribed to a subnet for testing purposes. - #[cfg(test)] + #[cfg(all(test, feature = "spec-mainnet"))] pub(crate) fn is_subscribed( &self, subnet_id: &SubnetId, diff --git a/beacon_node/network/src/subnet_service/sync_subnets.rs b/beacon_node/network/src/subnet_service/sync_subnets.rs index 0b27ff527f..c3cc6b39e4 100644 --- a/beacon_node/network/src/subnet_service/sync_subnets.rs +++ b/beacon_node/network/src/subnet_service/sync_subnets.rs @@ -87,7 +87,7 @@ impl SyncCommitteeService { } /// Return count of all currently subscribed subnets. - #[cfg(test)] + #[cfg(all(test, feature = "spec-mainnet"))] pub fn subscription_count(&self) -> usize { use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT; if self.subscribe_all_subnets { diff --git a/beacon_node/network/src/subnet_service/tests/mod.rs b/beacon_node/network/src/subnet_service/tests/mod.rs index a407fe1bcf..a189f9c30d 100644 --- a/beacon_node/network/src/subnet_service/tests/mod.rs +++ b/beacon_node/network/src/subnet_service/tests/mod.rs @@ -1,3 +1,4 @@ +#![cfg(feature = "spec-mainnet")] use super::*; use beacon_chain::{ builder::{BeaconChainBuilder, Witness}, diff --git a/beacon_node/network/src/sync/block_lookups/tests.rs b/beacon_node/network/src/sync/block_lookups/tests.rs index 44bdee225c..fc129f2afe 100644 --- a/beacon_node/network/src/sync/block_lookups/tests.rs +++ b/beacon_node/network/src/sync/block_lookups/tests.rs @@ -1,3 +1,4 @@ +#![cfg(feature = "spec-minimal")] use std::sync::Arc; use crate::service::RequestId; @@ -36,6 +37,11 @@ struct TestRig { const D: Duration = Duration::new(0, 0); +enum NumBlobs { + Random, + None, +} + impl TestRig { fn test_setup(enable_log: bool) -> (BlockLookups, SyncNetworkContext, Self) { let log = build_log(slog::Level::Debug, enable_log); @@ -79,12 +85,13 @@ impl TestRig { } fn rand_block(&mut self, fork_name: ForkName) -> SignedBeaconBlock { - self.rand_block_and_blobs(fork_name).0 + self.rand_block_and_blobs(fork_name, NumBlobs::None).0 } fn rand_block_and_blobs( &mut self, fork_name: ForkName, + num_blobs: NumBlobs, ) -> (SignedBeaconBlock, Vec>) { let inner = map_fork_name!(fork_name, BeaconBlock, <_>::random_for_test(&mut self.rng)); let mut block = @@ -93,7 +100,10 @@ impl TestRig { if let Ok(message) = block.message_deneb_mut() { // get random number between 0 and Max Blobs let mut payload: &mut FullPayloadDeneb = &mut message.body.execution_payload; - let num_blobs = rand::random::() % E::max_blobs_per_block(); + let num_blobs = match num_blobs { + NumBlobs::Random => rand::random::() % E::max_blobs_per_block(), + NumBlobs::None => 0, + }; let (bundle, transactions) = execution_layer::test_utils::generate_random_blobs::( num_blobs, &self.harness.chain.kzg.as_ref().unwrap(), @@ -239,296 +249,429 @@ impl TestRig { } } -macro_rules! common_tests { - ($mod_name: ident, $fork_name: ident) => { - #[cfg(test)] - mod $mod_name { - use super::*; - #[test] - fn test_single_block_lookup_happy_path() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_single_block_lookup_happy_path() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); - let block = rig.rand_block(fork_name); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - // Trigger the request - bl.search_block(block_root, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); - let id = rig.expect_block_request(response_type); + let block = rig.rand_block(fork_name); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + // Trigger the request + bl.search_block(block_root, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); + let id = rig.expect_block_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_block_request(ResponseType::Blob); + } - // The peer provides the correct block, should not be penalized. Now the block should be sent - // for processing. - bl.single_block_lookup_response(id, peer_id, Some(block.into()), D, &mut cx); - rig.expect_empty_network(); - rig.expect_block_process(response_type); + // The peer provides the correct block, should not be penalized. Now the block should be sent + // for processing. + bl.single_block_lookup_response(id, peer_id, Some(block.into()), D, &mut cx); + rig.expect_empty_network(); + rig.expect_block_process(response_type); - // The request should still be active. - assert_eq!(bl.single_block_lookups.len(), 1); + // The request should still be active. + assert_eq!(bl.single_block_lookups.len(), 1); - // Send the stream termination. Peer should have not been penalized, and the request removed - // after processing. - bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); - bl.single_block_processed( - id, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), - response_type, - &mut cx, - ); - rig.expect_empty_network(); - assert_eq!(bl.single_block_lookups.len(), 0); - } + // Send the stream termination. Peer should have not been penalized, and the request removed + // after processing. + bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); + bl.single_block_processed( + id, + BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), + response_type, + &mut cx, + ); + rig.expect_empty_network(); + assert_eq!(bl.single_block_lookups.len(), 0); +} - #[test] - fn test_single_block_lookup_empty_response() { - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_single_block_lookup_empty_response() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); - let block_hash = Hash256::random(); - let peer_id = PeerId::random(); + let block_hash = Hash256::random(); + let peer_id = PeerId::random(); - // Trigger the request - bl.search_block(block_hash, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); - let id = rig.expect_block_request(response_type); + // Trigger the request + bl.search_block(block_hash, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); + let id = rig.expect_block_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_block_request(ResponseType::Blob); + } - // The peer does not have the block. It should be penalized. - bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); - rig.expect_penalty(); + // The peer does not have the block. It should be penalized. + bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); + rig.expect_penalty(); - rig.expect_block_request(response_type); // it should be retried - } + rig.expect_block_request(response_type); // it should be retried +} - #[test] - fn test_single_block_lookup_wrong_response() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_single_block_lookup_wrong_response() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let block_hash = Hash256::random(); - let peer_id = PeerId::random(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let block_hash = Hash256::random(); + let peer_id = PeerId::random(); - // Trigger the request - bl.search_block(block_hash, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); - let id = rig.expect_block_request(response_type); + // Trigger the request + bl.search_block(block_hash, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); + let id = rig.expect_block_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_block_request(ResponseType::Blob); + } - // Peer sends something else. It should be penalized. - let bad_block = rig.rand_block(fork_name); - bl.single_block_lookup_response(id, peer_id, Some(bad_block.into()), D, &mut cx); - rig.expect_penalty(); - rig.expect_block_request(response_type); // should be retried + // Peer sends something else. It should be penalized. + let bad_block = rig.rand_block(fork_name); + bl.single_block_lookup_response(id, peer_id, Some(bad_block.into()), D, &mut cx); + rig.expect_penalty(); + rig.expect_block_request(response_type); // should be retried - // Send the stream termination. This should not produce an additional penalty. - bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); - rig.expect_empty_network(); - } + // Send the stream termination. This should not produce an additional penalty. + bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); + rig.expect_empty_network(); +} - #[test] - fn test_single_block_lookup_failure() { - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_single_block_lookup_failure() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let block_hash = Hash256::random(); - let peer_id = PeerId::random(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let block_hash = Hash256::random(); + let peer_id = PeerId::random(); - // Trigger the request - bl.search_block(block_hash, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); - let id = rig.expect_block_request(response_type); + // Trigger the request + bl.search_block(block_hash, peer_id, PeerShouldHave::BlockAndBlobs, &mut cx); + let id = rig.expect_block_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_block_request(ResponseType::Blob); + } - // The request fails. RPC failures are handled elsewhere so we should not penalize the peer. - bl.single_block_lookup_failed(id, &peer_id, &mut cx, RPCError::UnsupportedProtocol); - rig.expect_block_request(response_type); - rig.expect_empty_network(); - } + // The request fails. RPC failures are handled elsewhere so we should not penalize the peer. + bl.single_block_lookup_failed(id, &peer_id, &mut cx, RPCError::UnsupportedProtocol); + rig.expect_block_request(response_type); + rig.expect_empty_network(); +} - #[test] - fn test_single_block_lookup_becomes_parent_request() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_single_block_lookup_becomes_parent_request() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let block = Arc::new(rig.rand_block(fork_name)); - let peer_id = PeerId::random(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let block = Arc::new(rig.rand_block(fork_name)); + let peer_id = PeerId::random(); - // Trigger the request - bl.search_block( - block.canonical_root(), - peer_id, - PeerShouldHave::BlockAndBlobs, - &mut cx, - ); - let id = rig.expect_block_request(response_type); + // Trigger the request + bl.search_block( + block.canonical_root(), + peer_id, + PeerShouldHave::BlockAndBlobs, + &mut cx, + ); + let id = rig.expect_block_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_block_request(ResponseType::Blob); + } - // The peer provides the correct block, should not be penalized. Now the block should be sent - // for processing. - bl.single_block_lookup_response(id, peer_id, Some(block.clone()), D, &mut cx); - rig.expect_empty_network(); - rig.expect_block_process(response_type); + // The peer provides the correct block, should not be penalized. Now the block should be sent + // for processing. + bl.single_block_lookup_response(id, peer_id, Some(block.clone()), D, &mut cx); + rig.expect_empty_network(); + rig.expect_block_process(response_type); - // The request should still be active. - assert_eq!(bl.single_block_lookups.len(), 1); + // The request should still be active. + assert_eq!(bl.single_block_lookups.len(), 1); - // Send the stream termination. Peer should have not been penalized, and the request moved to a - // parent request after processing. - bl.single_block_processed( - id, - BlockError::ParentUnknown(block.into()).into(), - response_type, - &mut cx, - ); - assert_eq!(bl.single_block_lookups.len(), 1); - rig.expect_parent_request(response_type); - rig.expect_empty_network(); - assert_eq!(bl.parent_lookups.len(), 1); - } + // Send the stream termination. Peer should have not been penalized, and the request moved to a + // parent request after processing. + bl.single_block_processed( + id, + BlockError::ParentUnknown(block.into()).into(), + response_type, + &mut cx, + ); + assert_eq!(bl.single_block_lookups.len(), 1); + rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_parent_request(ResponseType::Blob); + } + rig.expect_empty_network(); + assert_eq!(bl.parent_lookups.len(), 1); +} - #[test] - fn test_parent_lookup_happy_path() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_parent_lookup_happy_path() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let parent = rig.rand_block(fork_name); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let chain_hash = block.canonical_root(); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let parent = rig.rand_block(fork_name); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let chain_hash = block.canonical_root(); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - let id = rig.expect_parent_request(response_type); + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + let id = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_parent_request(ResponseType::Blob); + } - // Peer sends the right block, it should be sent for processing. Peer should not be penalized. - bl.parent_lookup_response(id, peer_id, Some(parent.into()), D, &mut cx); - rig.expect_block_process(response_type); - rig.expect_empty_network(); + // Peer sends the right block, it should be sent for processing. Peer should not be penalized. + bl.parent_lookup_response(id, peer_id, Some(parent.into()), D, &mut cx); + rig.expect_block_process(response_type); + rig.expect_empty_network(); - // Processing succeeds, now the rest of the chain should be sent for processing. - bl.parent_block_processed( - chain_hash, - BlockError::BlockIsAlreadyKnown.into(), - response_type, - &mut cx, - ); - rig.expect_parent_chain_process(); - let process_result = BatchProcessResult::Success { - was_non_empty: true, - }; - bl.parent_chain_processed(chain_hash, process_result, &mut cx); - assert_eq!(bl.parent_lookups.len(), 0); - } + // Processing succeeds, now the rest of the chain should be sent for processing. + bl.parent_block_processed( + chain_hash, + BlockError::BlockIsAlreadyKnown.into(), + response_type, + &mut cx, + ); + rig.expect_parent_chain_process(); + let process_result = BatchProcessResult::Success { + was_non_empty: true, + }; + bl.parent_chain_processed(chain_hash, process_result, &mut cx); + assert_eq!(bl.parent_lookups.len(), 0); +} - #[test] - fn test_parent_lookup_wrong_response() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_parent_lookup_wrong_response() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let parent = rig.rand_block(fork_name); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let chain_hash = block.canonical_root(); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let parent = rig.rand_block(fork_name); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let chain_hash = block.canonical_root(); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - let id1 = rig.expect_parent_request(response_type); + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + let id1 = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_parent_request(ResponseType::Blob); + } - // Peer sends the wrong block, peer should be penalized and the block re-requested. - let bad_block = rig.rand_block(fork_name); - bl.parent_lookup_response(id1, peer_id, Some(bad_block.into()), D, &mut cx); - rig.expect_penalty(); - let id2 = rig.expect_parent_request(response_type); + // Peer sends the wrong block, peer should be penalized and the block re-requested. + let bad_block = rig.rand_block(fork_name); + bl.parent_lookup_response(id1, peer_id, Some(bad_block.into()), D, &mut cx); + rig.expect_penalty(); + let id2 = rig.expect_parent_request(response_type); - // Send the stream termination for the first request. This should not produce extra penalties. - bl.parent_lookup_response(id1, peer_id, None, D, &mut cx); - rig.expect_empty_network(); + // Send the stream termination for the first request. This should not produce extra penalties. + bl.parent_lookup_response(id1, peer_id, None, D, &mut cx); + rig.expect_empty_network(); - // Send the right block this time. - bl.parent_lookup_response(id2, peer_id, Some(parent.into()), D, &mut cx); - rig.expect_block_process(response_type); + // Send the right block this time. + bl.parent_lookup_response(id2, peer_id, Some(parent.into()), D, &mut cx); + rig.expect_block_process(response_type); - // Processing succeeds, now the rest of the chain should be sent for processing. - bl.parent_block_processed( - chain_hash, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), - response_type, - &mut cx, - ); - rig.expect_parent_chain_process(); - let process_result = BatchProcessResult::Success { - was_non_empty: true, - }; - bl.parent_chain_processed(chain_hash, process_result, &mut cx); - assert_eq!(bl.parent_lookups.len(), 0); - } + // Processing succeeds, now the rest of the chain should be sent for processing. + bl.parent_block_processed( + chain_hash, + BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), + response_type, + &mut cx, + ); + rig.expect_parent_chain_process(); + let process_result = BatchProcessResult::Success { + was_non_empty: true, + }; + bl.parent_chain_processed(chain_hash, process_result, &mut cx); + assert_eq!(bl.parent_lookups.len(), 0); +} - #[test] - fn test_parent_lookup_empty_response() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_parent_lookup_empty_response() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let parent = rig.rand_block(fork_name); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let chain_hash = block.canonical_root(); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let parent = rig.rand_block(fork_name); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let chain_hash = block.canonical_root(); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - let id1 = rig.expect_parent_request(response_type); + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + let id1 = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_parent_request(ResponseType::Blob); + } - // Peer sends an empty response, peer should be penalized and the block re-requested. - bl.parent_lookup_response(id1, peer_id, None, D, &mut cx); - rig.expect_penalty(); - let id2 = rig.expect_parent_request(response_type); + // Peer sends an empty response, peer should be penalized and the block re-requested. + bl.parent_lookup_response(id1, peer_id, None, D, &mut cx); + rig.expect_penalty(); + let id2 = rig.expect_parent_request(response_type); - // Send the right block this time. - bl.parent_lookup_response(id2, peer_id, Some(parent.into()), D, &mut cx); - rig.expect_block_process(response_type); + // Send the right block this time. + bl.parent_lookup_response(id2, peer_id, Some(parent.into()), D, &mut cx); + rig.expect_block_process(response_type); - // Processing succeeds, now the rest of the chain should be sent for processing. - bl.parent_block_processed( - chain_hash, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), - response_type, - &mut cx, - ); - rig.expect_parent_chain_process(); - let process_result = BatchProcessResult::Success { - was_non_empty: true, - }; - bl.parent_chain_processed(chain_hash, process_result, &mut cx); - assert_eq!(bl.parent_lookups.len(), 0); - } + // Processing succeeds, now the rest of the chain should be sent for processing. + bl.parent_block_processed( + chain_hash, + BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), + response_type, + &mut cx, + ); + rig.expect_parent_chain_process(); + let process_result = BatchProcessResult::Success { + was_non_empty: true, + }; + bl.parent_chain_processed(chain_hash, process_result, &mut cx); + assert_eq!(bl.parent_lookups.len(), 0); +} - #[test] - fn test_parent_lookup_rpc_failure() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); +#[test] +fn test_parent_lookup_rpc_failure() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let parent = rig.rand_block(fork_name); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let chain_hash = block.canonical_root(); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let parent = rig.rand_block(fork_name); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let chain_hash = block.canonical_root(); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - let id1 = rig.expect_parent_request(response_type); + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + let id1 = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_parent_request(ResponseType::Blob); + } + // The request fails. It should be tried again. + bl.parent_lookup_failed( + id1, + peer_id, + &mut cx, + RPCError::ErrorResponse( + RPCResponseErrorCode::ResourceUnavailable, + "older than deneb".into(), + ), + ); + let id2 = rig.expect_parent_request(response_type); + + // Send the right block this time. + bl.parent_lookup_response(id2, peer_id, Some(parent.into()), D, &mut cx); + rig.expect_block_process(response_type); + + // Processing succeeds, now the rest of the chain should be sent for processing. + bl.parent_block_processed( + chain_hash, + BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), + response_type, + &mut cx, + ); + rig.expect_parent_chain_process(); + let process_result = BatchProcessResult::Success { + was_non_empty: true, + }; + bl.parent_chain_processed(chain_hash, process_result, &mut cx); + assert_eq!(bl.parent_lookups.len(), 0); +} + +#[test] +fn test_parent_lookup_too_many_attempts() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let parent = rig.rand_block(fork_name); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); + + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + for i in 1..=parent_lookup::PARENT_FAIL_TOLERANCE { + let id = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) && i == 1 { + let _ = rig.expect_parent_request(ResponseType::Blob); + } + match i % 2 { + // make sure every error is accounted for + 0 => { // The request fails. It should be tried again. bl.parent_lookup_failed( - id1, + id, peer_id, &mut cx, RPCError::ErrorResponse( @@ -536,450 +679,452 @@ macro_rules! common_tests { "older than deneb".into(), ), ); - let id2 = rig.expect_parent_request(response_type); - - // Send the right block this time. - bl.parent_lookup_response(id2, peer_id, Some(parent.into()), D, &mut cx); - rig.expect_block_process(response_type); - - // Processing succeeds, now the rest of the chain should be sent for processing. - bl.parent_block_processed( - chain_hash, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), - response_type, - &mut cx, - ); - rig.expect_parent_chain_process(); - let process_result = BatchProcessResult::Success { - was_non_empty: true, - }; - bl.parent_chain_processed(chain_hash, process_result, &mut cx); - assert_eq!(bl.parent_lookups.len(), 0); } - - #[test] - fn test_parent_lookup_too_many_attempts() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - - let parent = rig.rand_block(fork_name); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); - - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - for i in 1..=parent_lookup::PARENT_FAIL_TOLERANCE { - let id = rig.expect_parent_request(response_type); - match i % 2 { - // make sure every error is accounted for - 0 => { - // The request fails. It should be tried again. - bl.parent_lookup_failed( - id, - peer_id, - &mut cx, - RPCError::ErrorResponse( - RPCResponseErrorCode::ResourceUnavailable, - "older than deneb".into(), - ), - ); - } - _ => { - // Send a bad block this time. It should be tried again. - let bad_block = rig.rand_block(fork_name); - bl.parent_lookup_response( - id, - peer_id, - Some(bad_block.into()), - D, - &mut cx, - ); - // Send the stream termination - bl.parent_lookup_response(id, peer_id, None, D, &mut cx); - rig.expect_penalty(); - } - } - if i < parent_lookup::PARENT_FAIL_TOLERANCE { - assert_eq!(bl.parent_lookups[0].failed_block_attempts(), dbg!(i)); - } - } - - assert_eq!(bl.parent_lookups.len(), 0); - } - - #[test] - fn test_parent_lookup_too_many_download_attempts_no_blacklist() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - - let parent = rig.rand_block(fork_name); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let block_hash = block.canonical_root(); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); - - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - for i in 1..=parent_lookup::PARENT_FAIL_TOLERANCE { - assert!(!bl.failed_chains.contains(&block_hash)); - let id = rig.expect_parent_request(response_type); - if i % 2 != 0 { - // The request fails. It should be tried again. - bl.parent_lookup_failed( - id, - peer_id, - &mut cx, - RPCError::ErrorResponse( - RPCResponseErrorCode::ResourceUnavailable, - "older than deneb".into(), - ), - ); - } else { - // Send a bad block this time. It should be tried again. - let bad_block = rig.rand_block(fork_name); - bl.parent_lookup_response(id, peer_id, Some(bad_block.into()), D, &mut cx); - rig.expect_penalty(); - } - if i < parent_lookup::PARENT_FAIL_TOLERANCE { - assert_eq!(bl.parent_lookups[0].failed_block_attempts(), dbg!(i)); - } - } - - assert_eq!(bl.parent_lookups.len(), 0); - assert!(!bl.failed_chains.contains(&block_hash)); - assert!(!bl.failed_chains.contains(&parent.canonical_root())); - } - - #[test] - fn test_parent_lookup_too_many_processing_attempts_must_blacklist() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - const PROCESSING_FAILURES: u8 = parent_lookup::PARENT_FAIL_TOLERANCE / 2 + 1; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - - let parent = Arc::new(rig.rand_block(fork_name)); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); - - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - - // Fail downloading the block - for _ in 0..(parent_lookup::PARENT_FAIL_TOLERANCE - PROCESSING_FAILURES) { - let id = rig.expect_parent_request(response_type); - // The request fails. It should be tried again. - bl.parent_lookup_failed( - id, - peer_id, - &mut cx, - RPCError::ErrorResponse( - RPCResponseErrorCode::ResourceUnavailable, - "older than deneb".into(), - ), - ); - } - - // Now fail processing a block in the parent request - for _ in 0..PROCESSING_FAILURES { - let id = dbg!(rig.expect_parent_request(response_type)); - assert!(!bl.failed_chains.contains(&block_root)); - // send the right parent but fail processing - bl.parent_lookup_response(id, peer_id, Some(parent.clone()), D, &mut cx); - bl.parent_block_processed( - block_root, - BlockError::InvalidSignature.into(), - response_type, - &mut cx, - ); - bl.parent_lookup_response(id, peer_id, None, D, &mut cx); - rig.expect_penalty(); - } - - assert!(bl.failed_chains.contains(&block_root)); - assert_eq!(bl.parent_lookups.len(), 0); - } - - #[test] - fn test_parent_lookup_too_deep() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let mut blocks = Vec::>>::with_capacity( - parent_lookup::PARENT_DEPTH_TOLERANCE, - ); - while blocks.len() < parent_lookup::PARENT_DEPTH_TOLERANCE { - let parent = blocks - .last() - .map(|b| b.canonical_root()) - .unwrap_or_else(Hash256::random); - let block = Arc::new(rig.block_with_parent(parent, fork_name)); - blocks.push(block); - } - - let peer_id = PeerId::random(); - let trigger_block = blocks.pop().unwrap(); - let chain_hash = trigger_block.canonical_root(); - let trigger_block_root = trigger_block.canonical_root(); - let trigger_parent_root = trigger_block.parent_root(); - let trigger_slot = trigger_block.slot(); - bl.search_parent( - trigger_slot, - trigger_block_root, - trigger_parent_root, - peer_id, - &mut cx, - ); - - for block in blocks.into_iter().rev() { - let id = rig.expect_parent_request(response_type); - // the block - bl.parent_lookup_response(id, peer_id, Some(block.clone()), D, &mut cx); - // the stream termination - bl.parent_lookup_response(id, peer_id, None, D, &mut cx); - // the processing request - rig.expect_block_process(response_type); - // the processing result - bl.parent_block_processed( - chain_hash, - BlockError::ParentUnknown(block.into()).into(), - response_type, - &mut cx, - ) - } - + _ => { + // Send a bad block this time. It should be tried again. + let bad_block = rig.rand_block(fork_name); + bl.parent_lookup_response(id, peer_id, Some(bad_block.into()), D, &mut cx); + // Send the stream termination + bl.parent_lookup_response(id, peer_id, None, D, &mut cx); rig.expect_penalty(); - assert!(bl.failed_chains.contains(&chain_hash)); - } - - #[test] - fn test_parent_lookup_disconnection() { - let fork_name = ForkName::$fork_name; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - let peer_id = PeerId::random(); - let trigger_block = rig.rand_block(fork_name); - let trigger_block_root = trigger_block.canonical_root(); - let trigger_parent_root = trigger_block.parent_root(); - let trigger_slot = trigger_block.slot(); - bl.search_parent( - trigger_slot, - trigger_block_root, - trigger_parent_root, - peer_id, - &mut cx, - ); - - bl.peer_disconnected(&peer_id, &mut cx); - assert!(bl.parent_lookups.is_empty()); - } - - #[test] - fn test_single_block_lookup_ignored_response() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - - let block = rig.rand_block(fork_name); - let peer_id = PeerId::random(); - - // Trigger the request - bl.search_block( - block.canonical_root(), - peer_id, - PeerShouldHave::BlockAndBlobs, - &mut cx, - ); - let id = rig.expect_block_request(response_type); - - // The peer provides the correct block, should not be penalized. Now the block should be sent - // for processing. - bl.single_block_lookup_response(id, peer_id, Some(block.into()), D, &mut cx); - rig.expect_empty_network(); - rig.expect_block_process(response_type); - - // The request should still be active. - assert_eq!(bl.single_block_lookups.len(), 1); - - // Send the stream termination. Peer should have not been penalized, and the request removed - // after processing. - bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); - // Send an Ignored response, the request should be dropped - bl.single_block_processed( - id, - BlockProcessingResult::Ignored, - response_type, - &mut cx, - ); - rig.expect_empty_network(); - assert_eq!(bl.single_block_lookups.len(), 0); - } - - #[test] - fn test_parent_lookup_ignored_response() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); - - let parent = rig.rand_block(fork_name); - let block = rig.block_with_parent(parent.canonical_root(), fork_name); - let chain_hash = block.canonical_root(); - let peer_id = PeerId::random(); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let slot = block.slot(); - - // Trigger the request - bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); - let id = rig.expect_parent_request(response_type); - - // Peer sends the right block, it should be sent for processing. Peer should not be penalized. - bl.parent_lookup_response(id, peer_id, Some(parent.into()), D, &mut cx); - rig.expect_block_process(response_type); - rig.expect_empty_network(); - - // Return an Ignored result. The request should be dropped - bl.parent_block_processed( - chain_hash, - BlockProcessingResult::Ignored, - response_type, - &mut cx, - ); - rig.expect_empty_network(); - assert_eq!(bl.parent_lookups.len(), 0); - } - - /// This is a regression test. - #[test] - fn test_same_chain_race_condition() { - let fork_name = ForkName::$fork_name; - let response_type = ResponseType::Block; - let (mut bl, mut cx, mut rig) = TestRig::test_setup(true); - - #[track_caller] - fn parent_lookups_consistency(bl: &BlockLookups) { - let hashes: Vec<_> = bl - .parent_lookups - .iter() - .map(|req| req.chain_hash()) - .collect(); - let expected = hashes.len(); - assert_eq!( - expected, - hashes - .into_iter() - .collect::>() - .len(), - "duplicated chain hashes in parent queue" - ) - } - // if we use one or two blocks it will match on the hash or the parent hash, so make a longer - // chain. - let depth = 4; - let mut blocks = Vec::>>::with_capacity(depth); - while blocks.len() < depth { - let parent = blocks - .last() - .map(|b| b.canonical_root()) - .unwrap_or_else(Hash256::random); - let block = Arc::new(rig.block_with_parent(parent, fork_name)); - blocks.push(block); - } - - let peer_id = PeerId::random(); - let trigger_block = blocks.pop().unwrap(); - let chain_hash = trigger_block.canonical_root(); - let trigger_block_root = trigger_block.canonical_root(); - let trigger_parent_root = trigger_block.parent_root(); - let trigger_slot = trigger_block.slot(); - bl.search_parent( - trigger_slot, - trigger_block_root, - trigger_parent_root, - peer_id, - &mut cx, - ); - - for (i, block) in blocks.into_iter().rev().enumerate() { - let id = rig.expect_parent_request(response_type); - // the block - bl.parent_lookup_response(id, peer_id, Some(block.clone()), D, &mut cx); - // the stream termination - bl.parent_lookup_response(id, peer_id, None, D, &mut cx); - // the processing request - rig.expect_block_process(response_type); - // the processing result - if i + 2 == depth { - // one block was removed - bl.parent_block_processed( - chain_hash, - BlockError::BlockIsAlreadyKnown.into(), - response_type, - &mut cx, - ) - } else { - bl.parent_block_processed( - chain_hash, - BlockError::ParentUnknown(block.into()).into(), - response_type, - &mut cx, - ) - } - parent_lookups_consistency(&bl) - } - - // Processing succeeds, now the rest of the chain should be sent for processing. - rig.expect_parent_chain_process(); - - // Try to get this block again while the chain is being processed. We should not request it again. - let peer_id = PeerId::random(); - let trigger_block_root = trigger_block.canonical_root(); - let trigger_parent_root = trigger_block.parent_root(); - let trigger_slot = trigger_block.slot(); - bl.search_parent( - trigger_slot, - trigger_block_root, - trigger_parent_root, - peer_id, - &mut cx, - ); - parent_lookups_consistency(&bl); - - let process_result = BatchProcessResult::Success { - was_non_empty: true, - }; - bl.parent_chain_processed(chain_hash, process_result, &mut cx); - assert_eq!(bl.parent_lookups.len(), 0); } } - }; + if i < parent_lookup::PARENT_FAIL_TOLERANCE { + assert_eq!(bl.parent_lookups[0].failed_block_attempts(), dbg!(i)); + } + } + + assert_eq!(bl.parent_lookups.len(), 0); } -common_tests!(base, Base); -common_tests!(capella, Capella); -common_tests!(deneb, Deneb); +#[test] +fn test_parent_lookup_too_many_download_attempts_no_blacklist() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let parent = rig.rand_block(fork_name); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let block_hash = block.canonical_root(); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); + + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + for i in 1..=parent_lookup::PARENT_FAIL_TOLERANCE { + assert!(!bl.failed_chains.contains(&block_hash)); + let id = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) && i == 1 { + let _ = rig.expect_parent_request(ResponseType::Blob); + } + if i % 2 != 0 { + // The request fails. It should be tried again. + bl.parent_lookup_failed( + id, + peer_id, + &mut cx, + RPCError::ErrorResponse( + RPCResponseErrorCode::ResourceUnavailable, + "older than deneb".into(), + ), + ); + } else { + // Send a bad block this time. It should be tried again. + let bad_block = rig.rand_block(fork_name); + bl.parent_lookup_response(id, peer_id, Some(bad_block.into()), D, &mut cx); + rig.expect_penalty(); + } + if i < parent_lookup::PARENT_FAIL_TOLERANCE { + assert_eq!(bl.parent_lookups[0].failed_block_attempts(), dbg!(i)); + } + } + + assert_eq!(bl.parent_lookups.len(), 0); + assert!(!bl.failed_chains.contains(&block_hash)); + assert!(!bl.failed_chains.contains(&parent.canonical_root())); +} + +#[test] +fn test_parent_lookup_too_many_processing_attempts_must_blacklist() { + let response_type = ResponseType::Block; + const PROCESSING_FAILURES: u8 = parent_lookup::PARENT_FAIL_TOLERANCE / 2 + 1; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + + let parent = Arc::new(rig.rand_block(fork_name)); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); + + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + + // Fail downloading the block + for i in 0..(parent_lookup::PARENT_FAIL_TOLERANCE - PROCESSING_FAILURES) { + let id = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) && i == 0 { + let _ = rig.expect_parent_request(ResponseType::Blob); + } + // The request fails. It should be tried again. + bl.parent_lookup_failed( + id, + peer_id, + &mut cx, + RPCError::ErrorResponse( + RPCResponseErrorCode::ResourceUnavailable, + "older than deneb".into(), + ), + ); + } + + // Now fail processing a block in the parent request + for _ in 0..PROCESSING_FAILURES { + let id = dbg!(rig.expect_parent_request(response_type)); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + assert!(!bl.failed_chains.contains(&block_root)); + // send the right parent but fail processing + bl.parent_lookup_response(id, peer_id, Some(parent.clone()), D, &mut cx); + bl.parent_block_processed( + block_root, + BlockError::InvalidSignature.into(), + response_type, + &mut cx, + ); + bl.parent_lookup_response(id, peer_id, None, D, &mut cx); + rig.expect_penalty(); + } + + assert!(bl.failed_chains.contains(&block_root)); + assert_eq!(bl.parent_lookups.len(), 0); +} + +#[test] +fn test_parent_lookup_too_deep() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let mut blocks = + Vec::>>::with_capacity(parent_lookup::PARENT_DEPTH_TOLERANCE); + while blocks.len() < parent_lookup::PARENT_DEPTH_TOLERANCE { + let parent = blocks + .last() + .map(|b| b.canonical_root()) + .unwrap_or_else(Hash256::random); + let block = Arc::new(rig.block_with_parent(parent, fork_name)); + blocks.push(block); + } + + let peer_id = PeerId::random(); + let trigger_block = blocks.pop().unwrap(); + let chain_hash = trigger_block.canonical_root(); + let trigger_block_root = trigger_block.canonical_root(); + let trigger_parent_root = trigger_block.parent_root(); + let trigger_slot = trigger_block.slot(); + bl.search_parent( + trigger_slot, + trigger_block_root, + trigger_parent_root, + peer_id, + &mut cx, + ); + + for block in blocks.into_iter().rev() { + let id = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + dbg!("here"); + let _ = rig.expect_parent_request(ResponseType::Blob); + } + // the block + bl.parent_lookup_response(id, peer_id, Some(block.clone()), D, &mut cx); + // the stream termination + bl.parent_lookup_response(id, peer_id, None, D, &mut cx); + // the processing request + rig.expect_block_process(response_type); + // the processing result + bl.parent_block_processed( + chain_hash, + BlockError::ParentUnknown(block.into()).into(), + response_type, + &mut cx, + ) + } + + rig.expect_penalty(); + assert!(bl.failed_chains.contains(&chain_hash)); +} + +#[test] +fn test_parent_lookup_disconnection() { + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + let peer_id = PeerId::random(); + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let trigger_block = rig.rand_block(fork_name); + let trigger_block_root = trigger_block.canonical_root(); + let trigger_parent_root = trigger_block.parent_root(); + let trigger_slot = trigger_block.slot(); + bl.search_parent( + trigger_slot, + trigger_block_root, + trigger_parent_root, + peer_id, + &mut cx, + ); + + bl.peer_disconnected(&peer_id, &mut cx); + assert!(bl.parent_lookups.is_empty()); +} + +#[test] +fn test_single_block_lookup_ignored_response() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let block = rig.rand_block(fork_name); + let peer_id = PeerId::random(); + + // Trigger the request + bl.search_block( + block.canonical_root(), + peer_id, + PeerShouldHave::BlockAndBlobs, + &mut cx, + ); + let id = rig.expect_block_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_block_request(ResponseType::Blob); + } + + // The peer provides the correct block, should not be penalized. Now the block should be sent + // for processing. + bl.single_block_lookup_response(id, peer_id, Some(block.into()), D, &mut cx); + rig.expect_empty_network(); + rig.expect_block_process(response_type); + + // The request should still be active. + assert_eq!(bl.single_block_lookups.len(), 1); + + // Send the stream termination. Peer should have not been penalized, and the request removed + // after processing. + bl.single_block_lookup_response(id, peer_id, None, D, &mut cx); + // Send an Ignored response, the request should be dropped + bl.single_block_processed(id, BlockProcessingResult::Ignored, response_type, &mut cx); + rig.expect_empty_network(); + assert_eq!(bl.single_block_lookups.len(), 0); +} + +#[test] +fn test_parent_lookup_ignored_response() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); + + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + let parent = rig.rand_block(fork_name); + let block = rig.block_with_parent(parent.canonical_root(), fork_name); + let chain_hash = block.canonical_root(); + let peer_id = PeerId::random(); + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); + let slot = block.slot(); + + // Trigger the request + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); + let id = rig.expect_parent_request(response_type); + + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_parent_request(ResponseType::Blob); + } + + // Peer sends the right block, it should be sent for processing. Peer should not be penalized. + bl.parent_lookup_response(id, peer_id, Some(parent.into()), D, &mut cx); + rig.expect_block_process(response_type); + rig.expect_empty_network(); + + // Return an Ignored result. The request should be dropped + bl.parent_block_processed( + chain_hash, + BlockProcessingResult::Ignored, + response_type, + &mut cx, + ); + rig.expect_empty_network(); + assert_eq!(bl.parent_lookups.len(), 0); +} + +/// This is a regression test. +#[test] +fn test_same_chain_race_condition() { + let response_type = ResponseType::Block; + let (mut bl, mut cx, mut rig) = TestRig::test_setup(true); + + let fork_name = rig + .harness + .spec + .fork_name_at_slot::(rig.harness.chain.slot().unwrap()); + #[track_caller] + fn parent_lookups_consistency(bl: &BlockLookups) { + let hashes: Vec<_> = bl + .parent_lookups + .iter() + .map(|req| req.chain_hash()) + .collect(); + let expected = hashes.len(); + assert_eq!( + expected, + hashes + .into_iter() + .collect::>() + .len(), + "duplicated chain hashes in parent queue" + ) + } + // if we use one or two blocks it will match on the hash or the parent hash, so make a longer + // chain. + let depth = 4; + let mut blocks = Vec::>>::with_capacity(depth); + while blocks.len() < depth { + let parent = blocks + .last() + .map(|b| b.canonical_root()) + .unwrap_or_else(Hash256::random); + let block = Arc::new(rig.block_with_parent(parent, fork_name)); + blocks.push(block); + } + + let peer_id = PeerId::random(); + let trigger_block = blocks.pop().unwrap(); + let chain_hash = trigger_block.canonical_root(); + let trigger_block_root = trigger_block.canonical_root(); + let trigger_parent_root = trigger_block.parent_root(); + let trigger_slot = trigger_block.slot(); + bl.search_parent( + trigger_slot, + trigger_block_root, + trigger_parent_root, + peer_id, + &mut cx, + ); + + for (i, block) in blocks.into_iter().rev().enumerate() { + let id = rig.expect_parent_request(response_type); + // If we're in deneb, a blob request should have been triggered as well, + // we don't require a response because we're generateing 0-blob blocks in this test. + if matches!(fork_name, ForkName::Deneb) { + let _ = rig.expect_parent_request(ResponseType::Blob); + } + // the block + bl.parent_lookup_response(id, peer_id, Some(block.clone()), D, &mut cx); + // the stream termination + bl.parent_lookup_response(id, peer_id, None, D, &mut cx); + // the processing request + rig.expect_block_process(response_type); + // the processing result + if i + 2 == depth { + // one block was removed + bl.parent_block_processed( + chain_hash, + BlockError::BlockIsAlreadyKnown.into(), + response_type, + &mut cx, + ) + } else { + bl.parent_block_processed( + chain_hash, + BlockError::ParentUnknown(block.into()).into(), + response_type, + &mut cx, + ) + } + parent_lookups_consistency(&bl) + } + + // Processing succeeds, now the rest of the chain should be sent for processing. + rig.expect_parent_chain_process(); + + // Try to get this block again while the chain is being processed. We should not request it again. + let peer_id = PeerId::random(); + let trigger_block_root = trigger_block.canonical_root(); + let trigger_parent_root = trigger_block.parent_root(); + let trigger_slot = trigger_block.slot(); + bl.search_parent( + trigger_slot, + trigger_block_root, + trigger_parent_root, + peer_id, + &mut cx, + ); + parent_lookups_consistency(&bl); + + let process_result = BatchProcessResult::Success { + was_non_empty: true, + }; + bl.parent_chain_processed(chain_hash, process_result, &mut cx); + assert_eq!(bl.parent_lookups.len(), 0); +} mod deneb_only { use super::*; + use std::str::FromStr; + + fn get_fork_name() -> ForkName { + ForkName::from_str( + &std::env::var(beacon_chain::test_utils::FORK_NAME_ENV_VAR).unwrap_or_else(|e| { + panic!( + "{} env var must be defined when using fork_from_env: {:?}", + beacon_chain::test_utils::FORK_NAME_ENV_VAR, + e + ) + }), + ) + .unwrap() + } + #[test] fn test_single_block_lookup_happy_path() { - let fork_name = ForkName::Deneb; - + let fork_name = get_fork_name(); + if !matches!(fork_name, ForkName::Deneb) { + return; + } let (mut bl, mut cx, mut rig) = TestRig::test_setup(false); rig.harness .chain .slot_clock .set_slot(E::slots_per_epoch() * rig.harness.spec.deneb_fork_epoch.unwrap().as_u64()); - let (block, blobs) = rig.rand_block_and_blobs(fork_name); + let (block, blobs) = rig.rand_block_and_blobs(fork_name, NumBlobs::Random); let slot = block.slot(); let peer_id = PeerId::random(); let block_root = block.canonical_root();