Skip to content

Commit

Permalink
Cache participating indices for Altair epoch processing (sigp#2416)
Browse files Browse the repository at this point in the history
## Issue Addressed

NA

## Proposed Changes

This PR addresses two things:

1. Allows the `ValidatorMonitor` to work with Altair states.
1. Optimizes `altair::process_epoch` (see [code](https://github.com/paulhauner/lighthouse/blob/participation-cache/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs) for description)

## Breaking Changes

The breaking changes in this PR revolve around one premise:

*After the Altair fork, it's not longer possible (given only a `BeaconState`) to identify if a validator had *any* attestation included during some epoch. The best we can do is see if that validator made the "timely" source/target/head flags.*

Whilst this seems annoying, it's not actually too bad. Finalization is based upon "timely target" attestations, so that's really the most important thing. Although there's *some* value in knowing if a validator had *any* attestation included, it's far more important to know about "timely target" participation, since this is what affects finality and justification.

For simplicity and consistency, I've also removed the ability to determine if *any* attestation was included from metrics and API endpoints. Now, all Altair and non-Altair states will simply report on the head/target attestations.

The following section details where we've removed fields and provides replacement values.

### Breaking Changes: Prometheus Metrics

Some participation metrics have been removed and replaced. Some were removed since they are no longer relevant to Altair (e.g., total attesting balance) and others replaced with gwei values instead of pre-computed values. This provides more flexibility at display-time (e.g., Grafana).

The following metrics were added as replacements:

- `beacon_participation_prev_epoch_head_attesting_gwei_total`
- `beacon_participation_prev_epoch_target_attesting_gwei_total`
- `beacon_participation_prev_epoch_source_attesting_gwei_total`
- `beacon_participation_prev_epoch_active_gwei_total`

The following metrics were removed:

- `beacon_participation_prev_epoch_attester`
   - instead use `beacon_participation_prev_epoch_source_attesting_gwei_total / beacon_participation_prev_epoch_active_gwei_total`.
- `beacon_participation_prev_epoch_target_attester`
   - instead use `beacon_participation_prev_epoch_target_attesting_gwei_total / beacon_participation_prev_epoch_active_gwei_total`.
- `beacon_participation_prev_epoch_head_attester`
   - instead use `beacon_participation_prev_epoch_head_attesting_gwei_total / beacon_participation_prev_epoch_active_gwei_total`.

The `beacon_participation_prev_epoch_attester` endpoint has been removed. Users should instead use the pre-existing `beacon_participation_prev_epoch_target_attester`. 

### Breaking Changes: HTTP API

The `/lighthouse/validator_inclusion/{epoch}/{validator_id}` endpoint loses the following fields:

- `current_epoch_attesting_gwei` (use `current_epoch_target_attesting_gwei` instead)
- `previous_epoch_attesting_gwei` (use `previous_epoch_target_attesting_gwei` instead)

The `/lighthouse/validator_inclusion/{epoch}/{validator_id}` endpoint lose the following fields:

- `is_current_epoch_attester` (use `is_current_epoch_target_attester` instead)
- `is_previous_epoch_attester` (use `is_previous_epoch_target_attester` instead)
- `is_active_in_current_epoch` becomes `is_active_unslashed_in_current_epoch`.
- `is_active_in_previous_epoch` becomes `is_active_unslashed_in_previous_epoch`.

## Additional Info

NA

## TODO

- [x] Deal with total balances
- [x] Update validator_inclusion API
- [ ] Ensure `beacon_participation_prev_epoch_target_attester` and `beacon_participation_prev_epoch_head_attester` work before Altair

Co-authored-by: realbigsean <seananderson33@gmail.com>
  • Loading branch information
paulhauner and realbigsean committed Jul 27, 2021
1 parent f5bdca0 commit 6e3ca48
Show file tree
Hide file tree
Showing 26 changed files with 1,069 additions and 370 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ cargo-fmt:
check-benches:
cargo check --workspace --benches

# Typechecks consensus code *without* allowing deprecated legacy arithmetic
# Typechecks consensus code *without* allowing deprecated legacy arithmetic or metrics.
check-consensus:
cargo check --manifest-path=consensus/state_processing/Cargo.toml --no-default-features

Expand Down
67 changes: 20 additions & 47 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,11 @@ use slot_clock::SlotClock;
use ssz::Encode;
use state_processing::{
block_signature_verifier::{BlockSignatureVerifier, Error as BlockSignatureVerifierError},
per_block_processing,
per_epoch_processing::EpochProcessingSummary,
per_slot_processing,
per_block_processing, per_slot_processing,
state_advance::partial_state_advance,
BlockProcessingError, BlockSignatureStrategy, SlotProcessingError,
};
use std::borrow::Cow;
use std::convert::TryFrom;
use std::fs;
use std::io::Write;
use store::{Error as DBError, HotColdDB, HotStateSummary, KeyValueStore, StoreOp};
Expand Down Expand Up @@ -971,12 +968,19 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
};

if let Some(summary) = per_slot_processing(&mut state, Some(state_root), &chain.spec)? {
summaries.push(summary)
// Expose Prometheus metrics.
if let Err(e) = summary.observe_metrics() {
error!(
chain.log,
"Failed to observe epoch summary metrics";
"src" => "block_verification",
"error" => ?e
);
}
summaries.push(summary);
}
}

expose_participation_metrics(&summaries);

// If the block is sufficiently recent, notify the validator monitor.
if let Some(slot) = chain.slot_clock.now() {
let epoch = slot.epoch(T::EthSpec::slots_per_epoch());
Expand All @@ -990,7 +994,15 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
// performing `per_slot_processing`.
for (i, summary) in summaries.iter().enumerate() {
let epoch = state.current_epoch() - Epoch::from(summaries.len() - i);
validator_monitor.process_validator_statuses(epoch, &summary.statuses);
if let Err(e) =
validator_monitor.process_validator_statuses(epoch, &summary, &chain.spec)
{
error!(
chain.log,
"Failed to process validator statuses";
"error" => ?e
);
}
}
}
}
Expand Down Expand Up @@ -1432,45 +1444,6 @@ fn verify_header_signature<T: BeaconChainTypes>(
}
}

fn expose_participation_metrics(summaries: &[EpochProcessingSummary]) {
if !cfg!(feature = "participation_metrics") {
return;
}

for summary in summaries {
let b = &summary.total_balances;

metrics::maybe_set_float_gauge(
&metrics::PARTICIPATION_PREV_EPOCH_ATTESTER,
participation_ratio(b.previous_epoch_attesters(), b.previous_epoch()),
);

metrics::maybe_set_float_gauge(
&metrics::PARTICIPATION_PREV_EPOCH_TARGET_ATTESTER,
participation_ratio(b.previous_epoch_target_attesters(), b.previous_epoch()),
);

metrics::maybe_set_float_gauge(
&metrics::PARTICIPATION_PREV_EPOCH_HEAD_ATTESTER,
participation_ratio(b.previous_epoch_head_attesters(), b.previous_epoch()),
);
}
}

fn participation_ratio(section: u64, total: u64) -> Option<f64> {
// Reduce the precision to help ensure we fit inside a u32.
const PRECISION: u64 = 100_000_000;

let section: f64 = u32::try_from(section / PRECISION).ok()?.into();
let total: f64 = u32::try_from(total / PRECISION).ok()?.into();

if total > 0_f64 {
Some(section / total)
} else {
None
}
}

fn write_state<T: EthSpec>(prefix: &str, state: &BeaconState<T>, log: &Logger) {
if WRITE_BLOCK_PROCESSING_SSZ {
let root = state.tree_hash_root();
Expand Down
15 changes: 0 additions & 15 deletions beacon_node/beacon_chain/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,21 +330,6 @@ lazy_static! {
pub static ref OP_POOL_NUM_SYNC_CONTRIBUTIONS: Result<IntGauge> =
try_create_int_gauge("beacon_op_pool_sync_contributions_total", "Count of sync contributions in the op pool");

/*
* Participation Metrics
*/
pub static ref PARTICIPATION_PREV_EPOCH_ATTESTER: Result<Gauge> = try_create_float_gauge(
"beacon_participation_prev_epoch_attester",
"Ratio of attesting balances to total balances"
);
pub static ref PARTICIPATION_PREV_EPOCH_TARGET_ATTESTER: Result<Gauge> = try_create_float_gauge(
"beacon_participation_prev_epoch_target_attester",
"Ratio of target-attesting balances to total balances"
);
pub static ref PARTICIPATION_PREV_EPOCH_HEAD_ATTESTER: Result<Gauge> = try_create_float_gauge(
"beacon_participation_prev_epoch_head_attester",
"Ratio of head-attesting balances to total balances"
);

/*
* Attestation Observation Metrics
Expand Down
21 changes: 19 additions & 2 deletions beacon_node/beacon_chain/src/state_advance_timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,32 @@ fn advance_head<T: BeaconChainTypes>(
if let Some(summary) = per_slot_processing(&mut state, state_root, &beacon_chain.spec)
.map_err(BeaconChainError::from)?
{
// Expose Prometheus metrics.
if let Err(e) = summary.observe_metrics() {
error!(
log,
"Failed to observe epoch summary metrics";
"src" => "state_advance_timer",
"error" => ?e
);
}

// Only notify the validator monitor for recent blocks.
if state.current_epoch() + VALIDATOR_MONITOR_HISTORIC_EPOCHS as u64
>= current_slot.epoch(T::EthSpec::slots_per_epoch())
{
// Potentially create logs/metrics for locally monitored validators.
beacon_chain
if let Err(e) = beacon_chain
.validator_monitor
.read()
.process_validator_statuses(state.current_epoch(), &summary.statuses);
.process_validator_statuses(state.current_epoch(), &summary, &beacon_chain.spec)
{
error!(
log,
"Unable to process validator statuses";
"error" => ?e
);
}
}
}

Expand Down
Loading

0 comments on commit 6e3ca48

Please sign in to comment.