Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Remove equivocating validators from fork choice (#3371)
Browse files Browse the repository at this point in the history
## Issue Addressed

Closes #3241
Closes #3242

## Proposed Changes

* [x] Implement logic to remove equivocating validators from fork choice per ethereum/consensus-specs#2845
* [x] Update tests to v1.2.0-rc.1. The new test which exercises `equivocating_indices` is passing.
* [x] Pull in some SSZ abstractions from the `tree-states` branch that make implementing Vec-compatible encoding for types like `BTreeSet` and `BTreeMap`.
* [x] Implement schema upgrades and downgrades for the database (new schema version is V11).
* [x] Apply attester slashings from blocks to fork choice

## Additional Info

* This PR doesn't need the `BTreeMap` impl, but `tree-states` does, and I don't think there's any harm in keeping it. But I could also be convinced to drop it.

Blocked on #3322.
michaelsproul committed Jul 28, 2022
1 parent cf3bcca commit c598945
Showing 25 changed files with 745 additions and 154 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.

16 changes: 15 additions & 1 deletion beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
@@ -2095,11 +2095,20 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
)?)
}

/// Accept some attester slashing and queue it for inclusion in an appropriate block.
/// Accept a verified attester slashing and:
///
/// 1. Apply it to fork choice.
/// 2. Add it to the op pool.
pub fn import_attester_slashing(
&self,
attester_slashing: SigVerifiedOp<AttesterSlashing<T::EthSpec>>,
) {
// Add to fork choice.
self.canonical_head
.fork_choice_write_lock()
.on_attester_slashing(attester_slashing.as_inner());

// Add to the op pool (if we have the ability to propose blocks).
if self.eth1_chain.is_some() {
self.op_pool.insert_attester_slashing(
attester_slashing,
@@ -2717,6 +2726,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
.process_valid_state(current_slot.epoch(T::EthSpec::slots_per_epoch()), &state);
let validator_monitor = self.validator_monitor.read();

// Register each attester slashing in the block with fork choice.
for attester_slashing in block.body().attester_slashings() {
fork_choice.on_attester_slashing(attester_slashing);
}

// Register each attestation in the block with the fork choice service.
for attestation in block.body().attestations() {
let _fork_choice_attestation_timer =
27 changes: 21 additions & 6 deletions beacon_node/beacon_chain/src/beacon_fork_choice_store.rs
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ use crate::{metrics, BeaconSnapshot};
use derivative::Derivative;
use fork_choice::ForkChoiceStore;
use ssz_derive::{Decode, Encode};
use std::collections::BTreeSet;
use std::marker::PhantomData;
use std::sync::Arc;
use store::{Error as StoreError, HotColdDB, ItemStore};
@@ -158,6 +159,7 @@ pub struct BeaconForkChoiceStore<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<
unrealized_justified_checkpoint: Checkpoint,
unrealized_finalized_checkpoint: Checkpoint,
proposer_boost_root: Hash256,
equivocating_indices: BTreeSet<u64>,
_phantom: PhantomData<E>,
}

@@ -206,6 +208,7 @@ where
unrealized_justified_checkpoint: justified_checkpoint,
unrealized_finalized_checkpoint: finalized_checkpoint,
proposer_boost_root: Hash256::zero(),
equivocating_indices: BTreeSet::new(),
_phantom: PhantomData,
}
}
@@ -223,6 +226,7 @@ where
unrealized_justified_checkpoint: self.unrealized_justified_checkpoint,
unrealized_finalized_checkpoint: self.unrealized_finalized_checkpoint,
proposer_boost_root: self.proposer_boost_root,
equivocating_indices: self.equivocating_indices.clone(),
}
}

@@ -242,6 +246,7 @@ where
unrealized_justified_checkpoint: persisted.unrealized_justified_checkpoint,
unrealized_finalized_checkpoint: persisted.unrealized_finalized_checkpoint,
proposer_boost_root: persisted.proposer_boost_root,
equivocating_indices: persisted.equivocating_indices,
_phantom: PhantomData,
})
}
@@ -350,30 +355,40 @@ where
fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256) {
self.proposer_boost_root = proposer_boost_root;
}

fn equivocating_indices(&self) -> &BTreeSet<u64> {
&self.equivocating_indices
}

fn extend_equivocating_indices(&mut self, indices: impl IntoIterator<Item = u64>) {
self.equivocating_indices.extend(indices);
}
}

/// A container which allows persisting the `BeaconForkChoiceStore` to the on-disk database.
#[superstruct(
variants(V1, V7, V8, V10),
variants(V1, V7, V8, V10, V11),
variant_attributes(derive(Encode, Decode)),
no_enum
)]
pub struct PersistedForkChoiceStore {
#[superstruct(only(V1, V7))]
pub balances_cache: BalancesCacheV1,
#[superstruct(only(V8, V10))]
#[superstruct(only(V8, V10, V11))]
pub balances_cache: BalancesCacheV8,
pub time: Slot,
pub finalized_checkpoint: Checkpoint,
pub justified_checkpoint: Checkpoint,
pub justified_balances: Vec<u64>,
pub best_justified_checkpoint: Checkpoint,
#[superstruct(only(V10))]
#[superstruct(only(V10, V11))]
pub unrealized_justified_checkpoint: Checkpoint,
#[superstruct(only(V10))]
#[superstruct(only(V10, V11))]
pub unrealized_finalized_checkpoint: Checkpoint,
#[superstruct(only(V7, V8, V10))]
#[superstruct(only(V7, V8, V10, V11))]
pub proposer_boost_root: Hash256,
#[superstruct(only(V11))]
pub equivocating_indices: BTreeSet<u64>,
}

pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV10;
pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV11;
11 changes: 7 additions & 4 deletions beacon_node/beacon_chain/src/persisted_fork_choice.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
use crate::beacon_fork_choice_store::{
PersistedForkChoiceStoreV1, PersistedForkChoiceStoreV10, PersistedForkChoiceStoreV7,
PersistedForkChoiceStoreV8,
PersistedForkChoiceStoreV1, PersistedForkChoiceStoreV10, PersistedForkChoiceStoreV11,
PersistedForkChoiceStoreV7, PersistedForkChoiceStoreV8,
};
use ssz::{Decode, Encode};
use ssz_derive::{Decode, Encode};
use store::{DBColumn, Error, StoreItem};
use superstruct::superstruct;

// If adding a new version you should update this type alias and fix the breakages.
pub type PersistedForkChoice = PersistedForkChoiceV10;
pub type PersistedForkChoice = PersistedForkChoiceV11;

#[superstruct(
variants(V1, V7, V8, V10),
variants(V1, V7, V8, V10, V11),
variant_attributes(derive(Encode, Decode)),
no_enum
)]
@@ -25,6 +25,8 @@ pub struct PersistedForkChoice {
pub fork_choice_store: PersistedForkChoiceStoreV8,
#[superstruct(only(V10))]
pub fork_choice_store: PersistedForkChoiceStoreV10,
#[superstruct(only(V11))]
pub fork_choice_store: PersistedForkChoiceStoreV11,
}

macro_rules! impl_store_item {
@@ -49,3 +51,4 @@ impl_store_item!(PersistedForkChoiceV1);
impl_store_item!(PersistedForkChoiceV7);
impl_store_item!(PersistedForkChoiceV8);
impl_store_item!(PersistedForkChoiceV10);
impl_store_item!(PersistedForkChoiceV11);
39 changes: 38 additions & 1 deletion beacon_node/beacon_chain/src/schema_change.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Utilities for managing database schema changes.
mod migration_schema_v10;
mod migration_schema_v11;
mod migration_schema_v6;
mod migration_schema_v7;
mod migration_schema_v8;
@@ -8,7 +9,8 @@ mod types;

use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY};
use crate::persisted_fork_choice::{
PersistedForkChoiceV1, PersistedForkChoiceV10, PersistedForkChoiceV7, PersistedForkChoiceV8,
PersistedForkChoiceV1, PersistedForkChoiceV10, PersistedForkChoiceV11, PersistedForkChoiceV7,
PersistedForkChoiceV8,
};
use crate::types::ChainSpec;
use slog::{warn, Logger};
@@ -36,6 +38,12 @@ pub fn migrate_schema<T: BeaconChainTypes>(
migrate_schema::<T>(db.clone(), datadir, from, next, log.clone(), spec)?;
migrate_schema::<T>(db, datadir, next, to, log, spec)
}
// Downgrade across multiple versions by recursively migrating one step at a time.
(_, _) if to.as_u64() + 1 < from.as_u64() => {
let next = SchemaVersion(from.as_u64() - 1);
migrate_schema::<T>(db.clone(), datadir, from, next, log.clone(), spec)?;
migrate_schema::<T>(db, datadir, next, to, log, spec)
}

//
// Migrations from before SchemaVersion(5) are deprecated.
@@ -159,6 +167,35 @@ pub fn migrate_schema<T: BeaconChainTypes>(

Ok(())
}
// Upgrade from v10 to v11 adding support for equivocating indices to fork choice.
(SchemaVersion(10), SchemaVersion(11)) => {
let mut ops = vec![];
let fork_choice_opt = db.get_item::<PersistedForkChoiceV10>(&FORK_CHOICE_DB_KEY)?;
if let Some(fork_choice) = fork_choice_opt {
let updated_fork_choice = migration_schema_v11::update_fork_choice(fork_choice);

ops.push(updated_fork_choice.as_kv_store_op(FORK_CHOICE_DB_KEY));
}

db.store_schema_version_atomically(to, ops)?;

Ok(())
}
// Downgrade from v11 to v10 removing support for equivocating indices from fork choice.
(SchemaVersion(11), SchemaVersion(10)) => {
let mut ops = vec![];
let fork_choice_opt = db.get_item::<PersistedForkChoiceV11>(&FORK_CHOICE_DB_KEY)?;
if let Some(fork_choice) = fork_choice_opt {
let updated_fork_choice =
migration_schema_v11::downgrade_fork_choice(fork_choice, log);

ops.push(updated_fork_choice.as_kv_store_op(FORK_CHOICE_DB_KEY));
}

db.store_schema_version_atomically(to, ops)?;

Ok(())
}
// Anything else is an error.
(_, _) => Err(HotColdDBError::UnsupportedSchemaVersion {
target_version: to,
77 changes: 77 additions & 0 deletions beacon_node/beacon_chain/src/schema_change/migration_schema_v11.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use crate::beacon_fork_choice_store::{PersistedForkChoiceStoreV10, PersistedForkChoiceStoreV11};
use crate::persisted_fork_choice::{PersistedForkChoiceV10, PersistedForkChoiceV11};
use slog::{warn, Logger};
use std::collections::BTreeSet;

/// Add the equivocating indices field.
pub fn update_fork_choice(fork_choice_v10: PersistedForkChoiceV10) -> PersistedForkChoiceV11 {
let PersistedForkChoiceStoreV10 {
balances_cache,
time,
finalized_checkpoint,
justified_checkpoint,
justified_balances,
best_justified_checkpoint,
unrealized_justified_checkpoint,
unrealized_finalized_checkpoint,
proposer_boost_root,
} = fork_choice_v10.fork_choice_store;

PersistedForkChoiceV11 {
fork_choice: fork_choice_v10.fork_choice,
fork_choice_store: PersistedForkChoiceStoreV11 {
balances_cache,
time,
finalized_checkpoint,
justified_checkpoint,
justified_balances,
best_justified_checkpoint,
unrealized_justified_checkpoint,
unrealized_finalized_checkpoint,
proposer_boost_root,
equivocating_indices: BTreeSet::new(),
},
}
}

pub fn downgrade_fork_choice(
fork_choice_v11: PersistedForkChoiceV11,
log: Logger,
) -> PersistedForkChoiceV10 {
let PersistedForkChoiceStoreV11 {
balances_cache,
time,
finalized_checkpoint,
justified_checkpoint,
justified_balances,
best_justified_checkpoint,
unrealized_justified_checkpoint,
unrealized_finalized_checkpoint,
proposer_boost_root,
equivocating_indices,
} = fork_choice_v11.fork_choice_store;

if !equivocating_indices.is_empty() {
warn!(
log,
"Deleting slashed validators from fork choice store";
"count" => equivocating_indices.len(),
"message" => "this may make your node more susceptible to following the wrong chain",
);
}

PersistedForkChoiceV10 {
fork_choice: fork_choice_v11.fork_choice,
fork_choice_store: PersistedForkChoiceStoreV10 {
balances_cache,
time,
finalized_checkpoint,
justified_checkpoint,
justified_balances,
best_justified_checkpoint,
unrealized_justified_checkpoint,
unrealized_finalized_checkpoint,
proposer_boost_root,
},
}
}
2 changes: 1 addition & 1 deletion beacon_node/store/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ use ssz::{Decode, Encode};
use ssz_derive::{Decode, Encode};
use types::{Checkpoint, Hash256, Slot};

pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(10);
pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(11);

// All the keys that get stored under the `BeaconMeta` column.
//
Loading

0 comments on commit c598945

Please sign in to comment.