Remove state dependency from core module in consensus/types (#8653)

#8652


  - This removes instances of `BeaconStateError` from `eth_spec.rs`, and replaces them directly with `ArithError` which can be trivially converted back to `BeaconStateError` at the call site.
- Also moves the state related methods on `ChainSpec` to be methods on `BeaconState` instead. I think this might be a more natural place for them to exist anyway.


Co-Authored-By: Mac L <mjladson@pm.me>

beacon_node/beacon_chain/src/beacon_block_reward.rs

@@ -135,7 +135,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 state
                     .get_validator(proposer_slashing.proposer_index() as usize)?
                     .effective_balance
-                    .safe_div(self.spec.whistleblower_reward_quotient_for_state(state))?,
+                    .safe_div(state.get_whistleblower_reward_quotient(&self.spec))?,
             )?;
         }
 
@@ -157,7 +157,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     state
                         .get_validator(attester_index as usize)?
                         .effective_balance
-                        .safe_div(self.spec.whistleblower_reward_quotient_for_state(state))?,
+                        .safe_div(state.get_whistleblower_reward_quotient(&self.spec))?,
                 )?;
             }
         }

beacon_node/beacon_chain/tests/rewards.rs

@@ -269,16 +269,16 @@ async fn test_rewards_electra_slashings() {
     harness.add_attester_slashing(vec![0]).unwrap();
     let slashed_balance_1 = initial_balances.get_mut(0).unwrap();
     let validator_1_effective_balance = state.get_effective_balance(0).unwrap();
-    let delta_1 = validator_1_effective_balance
+    let delta_1 =
-        / harness.spec.min_slashing_penalty_quotient_for_state(&state);
+        validator_1_effective_balance / state.get_min_slashing_penalty_quotient(&harness.spec);
     *slashed_balance_1 -= delta_1;
 
     // add a proposer slashing and calculating slashing penalties
     harness.add_proposer_slashing(1).unwrap();
     let slashed_balance_2 = initial_balances.get_mut(1).unwrap();
     let validator_2_effective_balance = state.get_effective_balance(1).unwrap();
-    let delta_2 = validator_2_effective_balance
+    let delta_2 =
-        / harness.spec.min_slashing_penalty_quotient_for_state(&state);
+        validator_2_effective_balance / state.get_min_slashing_penalty_quotient(&harness.spec);
     *slashed_balance_2 -= delta_2;
 
     check_all_electra_rewards(&harness, initial_balances).await;

consensus/state_processing/src/common/slash_validator.rs

@@ -42,8 +42,7 @@ pub fn slash_validator<E: EthSpec>(
     decrease_balance(
         state,
         slashed_index,
-        validator_effective_balance
+        validator_effective_balance.safe_div(state.get_min_slashing_penalty_quotient(spec))?,
-            .safe_div(spec.min_slashing_penalty_quotient_for_state(state))?,
     )?;
 
     update_progressive_balances_on_slashing(state, slashed_index, validator_effective_balance)?;
@@ -54,8 +53,8 @@ pub fn slash_validator<E: EthSpec>(
     // Apply proposer and whistleblower rewards
     let proposer_index = ctxt.get_proposer_index(state, spec)? as usize;
     let whistleblower_index = opt_whistleblower_index.unwrap_or(proposer_index);
-    let whistleblower_reward = validator_effective_balance
+    let whistleblower_reward =
-        .safe_div(spec.whistleblower_reward_quotient_for_state(state))?;
+        validator_effective_balance.safe_div(state.get_whistleblower_reward_quotient(spec))?;
     let proposer_reward = if state.fork_name_unchecked().altair_enabled() {
         whistleblower_reward
             .safe_mul(PROPOSER_WEIGHT)?

consensus/state_processing/src/per_epoch_processing/single_pass.rs

@@ -886,7 +886,7 @@ impl SlashingsContext {
     ) -> Result<Self, Error> {
         let sum_slashings = state.get_all_slashings().iter().copied().safe_sum()?;
         let adjusted_total_slashing_balance = min(
-            sum_slashings.safe_mul(spec.proportional_slashing_multiplier_for_state(state))?,
+            sum_slashings.safe_mul(state.get_proportional_slashing_multiplier(spec))?,
             state_ctxt.total_active_balance,
         );
 

consensus/state_processing/src/per_epoch_processing/slashings.rs

@@ -17,7 +17,7 @@ pub fn process_slashings<E: EthSpec>(
     let sum_slashings = state.get_all_slashings().iter().copied().safe_sum()?;
 
     let adjusted_total_slashing_balance = std::cmp::min(
-        sum_slashings.safe_mul(spec.proportional_slashing_multiplier_for_state(state))?,
+        sum_slashings.safe_mul(state.get_proportional_slashing_multiplier(spec))?,
         total_balance,
     );
 

consensus/types/src/core/chain_spec.rs

@@ -19,7 +19,6 @@ use crate::{
     data::{BlobIdentifier, DataColumnSubnetId, DataColumnsByRootIdentifier},
     execution::ExecutionBlockHash,
     fork::{Fork, ForkData, ForkName},
-    state::BeaconState,
 };
 
 /// Each of the BLS signature domains.
@@ -418,51 +417,6 @@ impl ChainSpec {
         }
     }
 
-    /// For a given `BeaconState`, return the proportional slashing multiplier associated with its variant.
-    pub fn proportional_slashing_multiplier_for_state<E: EthSpec>(
-        &self,
-        state: &BeaconState<E>,
-    ) -> u64 {
-        let fork_name = state.fork_name_unchecked();
-        if fork_name >= ForkName::Bellatrix {
-            self.proportional_slashing_multiplier_bellatrix
-        } else if fork_name >= ForkName::Altair {
-            self.proportional_slashing_multiplier_altair
-        } else {
-            self.proportional_slashing_multiplier
-        }
-    }
-
-    /// For a given `BeaconState`, return the minimum slashing penalty quotient associated with its variant.
-    pub fn min_slashing_penalty_quotient_for_state<E: EthSpec>(
-        &self,
-        state: &BeaconState<E>,
-    ) -> u64 {
-        let fork_name = state.fork_name_unchecked();
-        if fork_name.electra_enabled() {
-            self.min_slashing_penalty_quotient_electra
-        } else if fork_name >= ForkName::Bellatrix {
-            self.min_slashing_penalty_quotient_bellatrix
-        } else if fork_name >= ForkName::Altair {
-            self.min_slashing_penalty_quotient_altair
-        } else {
-            self.min_slashing_penalty_quotient
-        }
-    }
-
-    /// For a given `BeaconState`, return the whistleblower reward quotient associated with its variant.
-    pub fn whistleblower_reward_quotient_for_state<E: EthSpec>(
-        &self,
-        state: &BeaconState<E>,
-    ) -> u64 {
-        let fork_name = state.fork_name_unchecked();
-        if fork_name.electra_enabled() {
-            self.whistleblower_reward_quotient_electra
-        } else {
-            self.whistleblower_reward_quotient
-        }
-    }
-
     pub fn max_effective_balance_for_fork(&self, fork_name: ForkName) -> u64 {
         if fork_name.electra_enabled() {
             self.max_effective_balance_electra

consensus/types/src/core/eth_spec.rs

@@ -3,7 +3,7 @@ use std::{
     str::FromStr,
 };
 
-use safe_arith::SafeArith;
+use safe_arith::{ArithError, SafeArith};
 use serde::{Deserialize, Serialize};
 use typenum::{
     U0, U1, U2, U4, U8, U16, U17, U32, U64, U128, U256, U512, U625, U1024, U2048, U4096, U8192,
@@ -11,10 +11,7 @@ use typenum::{
     U1099511627776, UInt, Unsigned, bit::B0,
 };
 
-use crate::{
+use crate::core::{ChainSpec, Epoch};
-    core::{ChainSpec, Epoch},
-    state::BeaconStateError,
-};
 
 type U5000 = UInt<UInt<UInt<U625, B0>, B0>, B0>; // 625 * 8 = 5000
 
@@ -196,7 +193,7 @@ pub trait EthSpec: 'static + Default + Sync + Send + Clone + Debug + PartialEq +
     fn get_committee_count_per_slot(
         active_validator_count: usize,
         spec: &ChainSpec,
-    ) -> Result<usize, BeaconStateError> {
+    ) -> Result<usize, ArithError> {
         Self::get_committee_count_per_slot_with(
             active_validator_count,
             spec.max_committees_per_slot,
@@ -208,7 +205,7 @@ pub trait EthSpec: 'static + Default + Sync + Send + Clone + Debug + PartialEq +
         active_validator_count: usize,
         max_committees_per_slot: usize,
         target_committee_size: usize,
-    ) -> Result<usize, BeaconStateError> {
+    ) -> Result<usize, ArithError> {
         let slots_per_epoch = Self::SlotsPerEpoch::to_usize();
 
         Ok(std::cmp::max(

consensus/types/src/state/beacon_state.rs

@@ -2528,6 +2528,42 @@ impl<E: EthSpec> BeaconState<E> {
         self.epoch_cache().get_base_reward(validator_index)
     }
 
+    /// Get the proportional slashing multiplier for the current fork.
+    pub fn get_proportional_slashing_multiplier(&self, spec: &ChainSpec) -> u64 {
+        let fork_name = self.fork_name_unchecked();
+        if fork_name >= ForkName::Bellatrix {
+            spec.proportional_slashing_multiplier_bellatrix
+        } else if fork_name >= ForkName::Altair {
+            spec.proportional_slashing_multiplier_altair
+        } else {
+            spec.proportional_slashing_multiplier
+        }
+    }
+
+    /// Get the minimum slashing penalty quotient for the current fork.
+    pub fn get_min_slashing_penalty_quotient(&self, spec: &ChainSpec) -> u64 {
+        let fork_name = self.fork_name_unchecked();
+        if fork_name.electra_enabled() {
+            spec.min_slashing_penalty_quotient_electra
+        } else if fork_name >= ForkName::Bellatrix {
+            spec.min_slashing_penalty_quotient_bellatrix
+        } else if fork_name >= ForkName::Altair {
+            spec.min_slashing_penalty_quotient_altair
+        } else {
+            spec.min_slashing_penalty_quotient
+        }
+    }
+
+    /// Get the whistleblower reward quotient for the current fork.
+    pub fn get_whistleblower_reward_quotient(&self, spec: &ChainSpec) -> u64 {
+        let fork_name = self.fork_name_unchecked();
+        if fork_name.electra_enabled() {
+            spec.whistleblower_reward_quotient_electra
+        } else {
+            spec.whistleblower_reward_quotient
+        }
+    }
+
     // ******* Electra accessors *******
 
     /// Return the churn limit for the current epoch.

consensus/types/src/state/committee_cache.rs

@@ -100,7 +100,8 @@ impl CommitteeCache {
         }
 
         let committees_per_slot =
-            E::get_committee_count_per_slot(active_validator_indices.len(), spec)? as u64;
+            E::get_committee_count_per_slot(active_validator_indices.len(), spec)
+                .map_err(BeaconStateError::ArithError)? as u64;
 
         let seed = state.get_seed(epoch, Domain::BeaconAttester, spec)?;