Remove progressive balances mode (#5224)

consensus/fork_choice/src/fork_choice.rs

@@ -12,7 +12,6 @@ use std::cmp::Ordering;
 use std::collections::BTreeSet;
 use std::marker::PhantomData;
 use std::time::Duration;
-use types::ProgressiveBalancesMode;
 use types::{
     consts::merge::INTERVALS_PER_SLOT, AbstractExecPayload, AttestationShufflingId,
     AttesterSlashing, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, Checkpoint, Epoch,
@@ -73,7 +72,6 @@ pub enum Error<T> {
     },
     UnrealizedVoteProcessing(state_processing::EpochProcessingError),
     ValidatorStatuses(BeaconStateError),
-    ProgressiveBalancesCacheCheckFailed(String),
 }
 
 impl<T> From<InvalidAttestation> for Error<T> {
@@ -636,8 +634,6 @@ where
     ///
     /// The supplied block **must** pass the `state_transition` function as it will not be run
     /// here.
-    #[allow(clippy::too_many_arguments)]
-    // FIXME(sproul): remove progressive balances mode
     pub fn on_block<Payload: AbstractExecPayload<E>>(
         &mut self,
         system_time_current_slot: Slot,
@@ -646,9 +642,7 @@ where
         block_delay: Duration,
         state: &BeaconState<E>,
         payload_verification_status: PayloadVerificationStatus,
-        _progressive_balances_mode: ProgressiveBalancesMode,
         spec: &ChainSpec,
-        _log: &Logger,
     ) -> Result<(), Error<T::Error>> {
         // If this block has already been processed we do not need to reprocess it.
         // We check this immediately in case re-processing the block mutates some property of the

consensus/fork_choice/tests/tests.rs

@@ -16,8 +16,8 @@ use std::time::Duration;
 use store::MemoryStore;
 use types::{
     test_utils::generate_deterministic_keypair, BeaconBlockRef, BeaconState, ChainSpec, Checkpoint,
-    Epoch, EthSpec, ForkName, Hash256, IndexedAttestation, MainnetEthSpec, ProgressiveBalancesMode,
-    RelativeEpoch, SignedBeaconBlock, Slot, SubnetId,
+    Epoch, EthSpec, ForkName, Hash256, IndexedAttestation, MainnetEthSpec, RelativeEpoch,
+    SignedBeaconBlock, Slot, SubnetId,
 };
 
 pub type E = MainnetEthSpec;
@@ -47,8 +47,10 @@ impl fmt::Debug for ForkChoiceTest {
 impl ForkChoiceTest {
     /// Creates a new tester.
     pub fn new() -> Self {
+        // Run fork choice tests against the latest fork.
+        let spec = ForkName::latest().make_genesis_spec(ChainSpec::default());
         let harness = BeaconChainHarness::builder(MainnetEthSpec)
-            .default_spec()
+            .spec(spec)
             .deterministic_keypairs(VALIDATOR_COUNT)
             .fresh_ephemeral_store()
             .build();
@@ -58,29 +60,13 @@ impl ForkChoiceTest {
 
     /// Creates a new tester with a custom chain config.
     pub fn new_with_chain_config(chain_config: ChainConfig) -> Self {
-        let harness = BeaconChainHarness::builder(MainnetEthSpec)
-            .default_spec()
-            .chain_config(chain_config)
-            .deterministic_keypairs(VALIDATOR_COUNT)
-            .fresh_ephemeral_store()
-            .build();
-
-        Self { harness }
-    }
-
-    /// Creates a new tester with the specified `ProgressiveBalancesMode` and genesis from latest fork.
-    fn new_with_progressive_balances_mode(mode: ProgressiveBalancesMode) -> ForkChoiceTest {
-        // genesis with latest fork (at least altair required to test the cache)
+        // Run fork choice tests against the latest fork.
         let spec = ForkName::latest().make_genesis_spec(ChainSpec::default());
         let harness = BeaconChainHarness::builder(MainnetEthSpec)
             .spec(spec)
-            .chain_config(ChainConfig {
-                progressive_balances_mode: mode,
-                ..ChainConfig::default()
-            })
+            .chain_config(chain_config)
             .deterministic_keypairs(VALIDATOR_COUNT)
             .fresh_ephemeral_store()
-            .mock_execution_layer()
             .build();
 
         Self { harness }
@@ -338,9 +324,7 @@ impl ForkChoiceTest {
                 Duration::from_secs(0),
                 &state,
                 PayloadVerificationStatus::Verified,
-                self.harness.chain.config.progressive_balances_mode,
                 &self.harness.chain.spec,
-                self.harness.logger(),
             )
             .unwrap();
         self
@@ -383,9 +367,7 @@ impl ForkChoiceTest {
                 Duration::from_secs(0),
                 &state,
                 PayloadVerificationStatus::Verified,
-                self.harness.chain.config.progressive_balances_mode,
                 &self.harness.chain.spec,
-                self.harness.logger(),
             )
             .expect_err("on_block did not return an error");
         comparison_func(err);
@@ -1348,7 +1330,7 @@ async fn weak_subjectivity_check_epoch_boundary_is_skip_slot_failure() {
 /// where the slashed validator is a target attester in previous / current epoch.
 #[tokio::test]
 async fn progressive_balances_cache_attester_slashing() {
-    ForkChoiceTest::new_with_progressive_balances_mode(ProgressiveBalancesMode::Strict)
+    ForkChoiceTest::new()
         // first two epochs
         .apply_blocks_while(|_, state| state.finalized_checkpoint().epoch == 0)
         .await
@@ -1379,7 +1361,7 @@ async fn progressive_balances_cache_attester_slashing() {
 /// where the slashed validator is a target attester in previous / current epoch.
 #[tokio::test]
 async fn progressive_balances_cache_proposer_slashing() {
-    ForkChoiceTest::new_with_progressive_balances_mode(ProgressiveBalancesMode::Strict)
+    ForkChoiceTest::new()
         // first two epochs
         .apply_blocks_while(|_, state| state.finalized_checkpoint().epoch == 0)
         .await

consensus/types/src/beacon_state/progressive_balances_cache.rs

@@ -8,8 +8,6 @@ use crate::{
 };
 use arbitrary::Arbitrary;
 use safe_arith::SafeArith;
-use serde::{Deserialize, Serialize};
-use strum::{Display, EnumString, EnumVariantNames};
 
 /// This cache keeps track of the accumulated target attestation balance for the current & previous
 /// epochs. The cached values can be utilised by fork choice to calculate unrealized justification
@@ -285,33 +283,6 @@ impl ProgressiveBalancesCache {
     }
 }
 
-#[derive(
-    Debug, PartialEq, Eq, Clone, Copy, Deserialize, Serialize, Display, EnumString, EnumVariantNames,
-)]
-#[strum(serialize_all = "lowercase")]
-pub enum ProgressiveBalancesMode {
-    /// Disable the usage of progressive cache, and use the existing `ParticipationCache` calculation.
-    Disabled,
-    /// Enable the usage of progressive cache, with checks against the `ParticipationCache` and falls
-    /// back to the existing calculation if there is a balance mismatch.
-    Checked,
-    /// Enable the usage of progressive cache, with checks against the `ParticipationCache`. BeaconStateErrors
-    /// if there is a balance mismatch. Used in testing only.
-    Strict,
-    /// Enable the usage of progressive cache, with no comparative checks against the
-    /// `ParticipationCache`. This is fast but an experimental mode, use with caution.
-    Fast,
-}
-
-impl ProgressiveBalancesMode {
-    pub fn perform_comparative_checks(&self) -> bool {
-        match self {
-            Self::Disabled | Self::Fast => false,
-            Self::Checked | Self::Strict => true,
-        }
-    }
-}
-
 /// `ProgressiveBalancesCache` is only enabled from `Altair` as it requires `ParticipationCache`.
 pub fn is_progressive_balances_enabled<E: EthSpec>(state: &BeaconState<E>) -> bool {
     match state {