Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up single-pass a bit #5282

Merged
merged 1 commit into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions beacon_node/beacon_chain/src/attestation_rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
};

for &validator_index in &validators {
// Return 0s for unknown/inactive validator indices. This is a bit different from stable
// where we error for unknown pubkeys.
// Return 0s for unknown/inactive validator indices.
let Ok(validator) = state.get_validator(validator_index) else {
debug!(
self.log,
Expand Down
10 changes: 4 additions & 6 deletions beacon_node/genesis/src/interop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,13 @@ mod test {
}

for v in state.validators() {
let creds = v.withdrawal_credentials;
let creds = v.withdrawal_credentials.as_bytes();
assert_eq!(
creds.as_bytes()[0],
spec.bls_withdrawal_prefix_byte,
creds[0], spec.bls_withdrawal_prefix_byte,
"first byte of withdrawal creds should be bls prefix"
);
assert_eq!(
&creds.as_bytes()[1..],
&creds[1..],
&hash(&v.pubkey.as_ssz_bytes())[1..],
"rest of withdrawal creds should be pubkey hash"
)
Expand Down Expand Up @@ -241,8 +240,7 @@ mod test {
}

for (index, v) in state.validators().iter().enumerate() {
let withdrawal_credientials = v.withdrawal_credentials;
let creds = withdrawal_credientials.as_bytes();
let creds = v.withdrawal_credentials.as_bytes();
if index % 2 == 0 {
assert_eq!(
creds[0], spec.bls_withdrawal_prefix_byte,
Expand Down
4 changes: 1 addition & 3 deletions book/src/validator-inclusion.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ The following fields are returned:
able to vote) during the current epoch.
- `current_epoch_target_attesting_gwei`: the total staked gwei that attested to
the majority-elected Casper FFG target epoch during the current epoch.
- `previous_epoch_active_gwei`: as per `current_epoch_active_gwei`, but during the previous epoch.
- `previous_epoch_target_attesting_gwei`: see `current_epoch_target_attesting_gwei`.
- `previous_epoch_head_attesting_gwei`: the total staked gwei that attested to a
head beacon block that is in the canonical chain.
Expand All @@ -65,7 +64,7 @@ From this data you can calculate:

#### Justification/Finalization Rate

`previous_epoch_target_attesting_gwei / previous_epoch_active_gwei`
`previous_epoch_target_attesting_gwei / current_epoch_active_gwei`

When this value is greater than or equal to `2/3` it is possible that the
beacon chain may justify and/or finalize the epoch.
Expand All @@ -80,7 +79,6 @@ curl -X GET "http://localhost:5052/lighthouse/validator_inclusion/0/global" -H
{
"data": {
"current_epoch_active_gwei": 642688000000000,
"previous_epoch_active_gwei": 642688000000000,
"current_epoch_target_attesting_gwei": 366208000000000,
"previous_epoch_target_attesting_gwei": 1000000000,
"previous_epoch_head_attesting_gwei": 1000000000
Expand Down
6 changes: 2 additions & 4 deletions consensus/state_processing/src/all_caches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ use types::{BeaconState, ChainSpec, EpochCacheError, EthSpec, Hash256, RelativeE
pub trait AllCaches {
/// Build all caches.
///
/// Note that this excludes milhouse's intrinsic tree-hash cache. That needs to be managed
/// separately.
/// Note that this excludes the tree-hash cache. That needs to be managed separately.
fn build_all_caches(&mut self, spec: &ChainSpec) -> Result<(), EpochCacheError>;

/// Return true if all caches are built.
///
/// Note that this excludes milhouse's intrinsic tree-hash cache. That needs to be managed
/// separately.
/// Note that this excludes the tree-hash cache. That needs to be managed separately.
fn all_caches_built(&self) -> bool;
}

Expand Down
2 changes: 1 addition & 1 deletion consensus/state_processing/src/common/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use safe_arith::{ArithError, SafeArith};
use types::*;

/// This type exists to avoid confusing `total_active_balance` with `sqrt_total_active_balance`,
/// since they are used in close proximity and the same type (`u64`).
/// since they are used in close proximity and have the same type (`u64`).
#[derive(Copy, Clone)]
pub struct SqrtTotalActiveBalance(u64);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ pub mod altair_deneb {
})
}

#[allow(clippy::too_many_arguments)]
pub fn process_attestation<T: EthSpec>(
state: &mut BeaconState<T>,
attestation: &Attestation<T>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use types::beacon_state::BeaconState;
use types::chain_spec::ChainSpec;
use types::{BeaconStateError, EthSpec};

/// This implementation is now only used in phase0. Later hard forks use single-pass.
pub fn process_effective_balance_updates<T: EthSpec>(
state: &mut BeaconState<T>,
spec: &ChainSpec,
Expand Down Expand Up @@ -52,6 +53,7 @@ pub fn process_effective_balance_updates<T: EthSpec>(
Ok(())
}

/// Only used to test the effective balance part of single-pass in isolation.
pub fn process_effective_balance_updates_slow<T: EthSpec>(
state: &mut BeaconState<T>,
spec: &ChainSpec,
Expand Down
26 changes: 9 additions & 17 deletions consensus/types/src/beacon_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1480,24 +1480,10 @@ impl<T: EthSpec> BeaconState<T> {
Ok(cache.get_attestation_duties(validator_index))
}

/// Implementation of `get_total_balance`, matching the spec.
/// Build the total active balance cache from scratch.
///
/// Returns minimum `EFFECTIVE_BALANCE_INCREMENT`, to avoid div by 0.
pub fn get_total_balance<'a, I: IntoIterator<Item = &'a usize>>(
&'a self,
validator_indices: I,
spec: &ChainSpec,
) -> Result<u64, Error> {
let total_balance = validator_indices.into_iter().try_fold(0_u64, |acc, i| {
self.get_effective_balance(*i)
.and_then(|bal| Ok(acc.safe_add(bal)?))
})?;
Ok(std::cmp::max(
total_balance,
spec.effective_balance_increment,
))
}

/// This method should rarely be invoked because single-pass epoch processing keeps the total
/// active balance cache up to date.
pub fn compute_total_active_balance_slow(
&self,
epoch: Epoch,
Expand Down Expand Up @@ -1530,6 +1516,7 @@ impl<T: EthSpec> BeaconState<T> {
self.get_total_active_balance_at_epoch(self.current_epoch())
}

/// Get the cached total active balance while checking that it is for the correct `epoch`.
pub fn get_total_active_balance_at_epoch(&self, epoch: Epoch) -> Result<u64, Error> {
let (initialized_epoch, balance) = self
.total_active_balance()
Expand All @@ -1545,6 +1532,10 @@ impl<T: EthSpec> BeaconState<T> {
}
}

/// Manually set the total active balance.
///
/// This should only be called when the total active balance has been computed as part of
/// single-pass epoch processing (or `process_rewards_and_penalties` for phase0).
pub fn set_total_active_balance(&mut self, epoch: Epoch, balance: u64) {
*self.total_active_balance_mut() = Some((epoch, balance));
}
Expand All @@ -1561,6 +1552,7 @@ impl<T: EthSpec> BeaconState<T> {
Ok(())
}

/// Build the total active balance cache, even if it is already built.
pub fn force_build_total_active_balance_cache_at(
&mut self,
epoch: Epoch,
Expand Down
Loading