From 2ac609b64e20a98b2d8e1368771ed4fa82f6df2e Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Thu, 5 Jan 2023 13:00:44 -0600 Subject: [PATCH] Fixing Moar Failing Tests --- Makefile | 2 +- beacon_node/beacon_chain/src/test_utils.rs | 18 ++++------ .../execution_layer/src/engine_api/http.rs | 6 ++-- beacon_node/operation_pool/src/lib.rs | 33 +++++-------------- 4 files changed, 19 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index 40db61de7..e1fba5b42 100644 --- a/Makefile +++ b/Makefile @@ -120,7 +120,7 @@ run-ef-tests: test-beacon-chain: $(patsubst %,test-beacon-chain-%,$(FORKS)) test-beacon-chain-%: - env FORK_NAME=$* cargo test --release --features fork_from_env -p beacon_chain + env FORK_NAME=$* cargo test --release --features fork_from_env,withdrawals-processing -p beacon_chain # 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/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 53b333c5f..c4bb9f3d8 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -120,23 +120,19 @@ fn make_rng() -> Mutex { Mutex::new(StdRng::seed_from_u64(0x0DDB1A5E5BAD5EEDu64)) } -pub fn get_fork_from_env() -> ForkName { - let fork_string = std::env::var(FORK_NAME_ENV_VAR).unwrap_or_else(|e| { - panic!( - "{} env var must be defined when using fork_from_env: {:?}", - FORK_NAME_ENV_VAR, e - ) - }); - ForkName::from_str(fork_string.as_str()).unwrap() -} - /// Return a `ChainSpec` suitable for test usage. /// /// If the `fork_from_env` feature is enabled, read the fork to use from the FORK_NAME environment /// variable. Otherwise use the default spec. pub fn test_spec() -> ChainSpec { let mut spec = if cfg!(feature = "fork_from_env") { - let fork = get_fork_from_env(); + let fork_name = std::env::var(FORK_NAME_ENV_VAR).unwrap_or_else(|e| { + panic!( + "{} env var must be defined when using fork_from_env: {:?}", + FORK_NAME_ENV_VAR, e + ) + }); + let fork = ForkName::from_str(fork_name.as_str()).unwrap(); fork.make_genesis_spec(E::default_spec()) } else { E::default_spec() diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index bf1da078e..06abe7274 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -852,11 +852,11 @@ impl HttpJsonRpc { pub async fn supported_apis_v1(&self) -> Result { Ok(SupportedApis { new_payload_v1: true, - new_payload_v2: cfg!(feature = "withdrawals-processing"), + new_payload_v2: cfg!(any(feature = "withdrawals-processing", test)), forkchoice_updated_v1: true, - forkchoice_updated_v2: cfg!(feature = "withdrawals-processing"), + forkchoice_updated_v2: cfg!(any(feature = "withdrawals-processing", test)), get_payload_v1: true, - get_payload_v2: cfg!(feature = "withdrawals-processing"), + get_payload_v2: cfg!(any(feature = "withdrawals-processing", test)), exchange_transition_configuration_v1: true, }) } diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 4a895391f..1f4660fc2 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -8,18 +8,19 @@ mod persistence; mod reward_cache; mod sync_aggregate_id; -use crate::attestation_storage::{AttestationMap, CheckpointKey}; -use crate::sync_aggregate_id::SyncAggregateId; pub use attestation::AttMaxCover; pub use attestation_storage::{AttestationRef, SplitAttestation}; -use attester_slashing::AttesterSlashingMaxCover; -use max_cover::maximum_cover; pub use max_cover::MaxCover; -use parking_lot::{RwLock, RwLockWriteGuard}; pub use persistence::{ PersistedOperationPool, PersistedOperationPoolV12, PersistedOperationPoolV5, }; pub use reward_cache::RewardCache; + +use crate::attestation_storage::{AttestationMap, CheckpointKey}; +use crate::sync_aggregate_id::SyncAggregateId; +use attester_slashing::AttesterSlashingMaxCover; +use max_cover::maximum_cover; +use parking_lot::{RwLock, RwLockWriteGuard}; use state_processing::per_block_processing::errors::AttestationValidationError; use state_processing::per_block_processing::{ get_slashable_indices_modular, verify_exit, VerifySignatures, @@ -766,8 +767,7 @@ mod release_tests { use super::attestation::earliest_attestation_validators; use super::*; use beacon_chain::test_utils::{ - get_fork_from_env, test_spec, BeaconChainHarness, EphemeralHarnessType, - RelativeSyncCommittee, + test_spec, BeaconChainHarness, EphemeralHarnessType, RelativeSyncCommittee, }; use lazy_static::lazy_static; use maplit::hashset; @@ -1787,28 +1787,11 @@ mod release_tests { fn cross_fork_harness() -> (BeaconChainHarness>, ChainSpec) { - let mut spec = test_spec::(); - - if cfg!(feature = "fork_from_env") { - let fork = get_fork_from_env(); - match fork { - ForkName::Altair | ForkName::Merge | ForkName::Capella | ForkName::Eip4844 => {} - _ => panic!( - "Unknown fork {}, add it above AND below so this test doesn't panic", - fork - ), - } - } + let mut spec = E::default_spec(); // Give some room to sign surround slashings. - // It appears we need to set _every_ fork to some non-zero value - // here. Otherwise if we set FORK_NAME_ENV_VAR to some fork that - // isn't listed here, tests that use this function will panic in - // non-trivial ways spec.altair_fork_epoch = Some(Epoch::new(3)); spec.bellatrix_fork_epoch = Some(Epoch::new(6)); - spec.capella_fork_epoch = Some(Epoch::new(9)); - spec.eip4844_fork_epoch = Some(Epoch::new(12)); // To make exits immediately valid. spec.shard_committee_period = 0;