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

Electra slashing handling #5816

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
16 changes: 16 additions & 0 deletions consensus/types/src/indexed_attestation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,22 @@ impl<E: EthSpec> IndexedAttestation<E> {
IndexedAttestation::Electra(att) => att.attesting_indices.first(),
}
}

pub fn to_electra(self) -> Result<IndexedAttestationElectra<E>, ssz_types::Error> {
Ok(match self {
Self::Base(att) => {
let extended_attesting_indices: VariableList<u64, E::MaxValidatorsPerSlot> =
VariableList::new(att.attesting_indices.to_vec())?;

IndexedAttestationElectra {
attesting_indices: extended_attesting_indices,
data: att.data,
signature: att.signature,
}
}
Self::Electra(att) => att,
})
}
}

impl<'a, E: EthSpec> IndexedAttestationRef<'a, E> {
Expand Down
20 changes: 18 additions & 2 deletions slasher/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
};
use flate2::bufread::{ZlibDecoder, ZlibEncoder};
use serde::{Deserialize, Serialize};
use slog::Logger;
use std::borrow::Borrow;
use std::collections::{btree_map::Entry, BTreeMap, HashSet};
use std::io::Read;
Expand Down Expand Up @@ -485,6 +486,7 @@ pub fn update<E: EthSpec>(
batch: Vec<Arc<IndexedAttesterRecord<E>>>,
current_epoch: Epoch,
config: &Config,
log: &Logger,
) -> Result<HashSet<AttesterSlashing<E>>, Error> {
// Split the batch up into horizontal segments.
// Map chunk indexes in the range `0..self.config.chunk_size` to attestations
Expand All @@ -504,6 +506,7 @@ pub fn update<E: EthSpec>(
&chunk_attestations,
current_epoch,
config,
log,
)?;
slashings.extend(update_array::<_, MaxTargetChunk>(
db,
Expand All @@ -512,6 +515,7 @@ pub fn update<E: EthSpec>(
&chunk_attestations,
current_epoch,
config,
log,
)?);

// Update all current epochs.
Expand Down Expand Up @@ -570,6 +574,7 @@ pub fn update_array<E: EthSpec, T: TargetArrayChunk>(
chunk_attestations: &BTreeMap<usize, Vec<Arc<IndexedAttesterRecord<E>>>>,
current_epoch: Epoch,
config: &Config,
log: &Logger,
) -> Result<HashSet<AttesterSlashing<E>>, Error> {
let mut slashings = HashSet::new();
// Map from chunk index to updated chunk at that index.
Expand Down Expand Up @@ -603,8 +608,19 @@ pub fn update_array<E: EthSpec, T: TargetArrayChunk>(
current_epoch,
config,
)?;
if let Some(slashing) = slashing_status.into_slashing(&attestation.indexed) {
slashings.insert(slashing);
match slashing_status.into_slashing(&attestation.indexed) {
Ok(Some(slashing)) => {
slashings.insert(slashing);
}
Err(e) => {
slog::error!(
log,
"Invalid slashing conversion";
"validator_index" => validator_index,
"error" => e
);
}
Ok(None) => {}
}
}
}
Expand Down
81 changes: 41 additions & 40 deletions slasher/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,55 +53,56 @@ impl<E: EthSpec> AttesterSlashingStatus<E> {
pub fn into_slashing(
self,
new_attestation: &IndexedAttestation<E>,
) -> Option<AttesterSlashing<E>> {
) -> Result<Option<AttesterSlashing<E>>, String> {
use AttesterSlashingStatus::*;

// The surrounding attestation must be in `attestation_1` to be valid.
match self {
Ok(match self {
NotSlashable => None,
AlreadyDoubleVoted => None,
DoubleVote(existing) | SurroundedByExisting(existing) => {
match (*existing, new_attestation) {
// TODO(electra) - determine when we would convert a Base attestation to Electra / how to handle mismatched attestations here
(IndexedAttestation::Base(existing_att), IndexedAttestation::Base(new_att)) => {
Some(AttesterSlashing::Base(AttesterSlashingBase {
attestation_1: existing_att,
attestation_2: new_att.clone(),
}))
}
(
IndexedAttestation::Electra(existing_att),
IndexedAttestation::Electra(new_att),
) => Some(AttesterSlashing::Electra(AttesterSlashingElectra {
DoubleVote(existing) | SurroundedByExisting(existing) => match *existing {
IndexedAttestation::Base(existing_att) => {
Some(AttesterSlashing::Base(AttesterSlashingBase {
attestation_1: existing_att,
attestation_2: new_att.clone(),
})),
_ => panic!("attestations must be of the same type"),
attestation_2: new_attestation
.as_base()
.map_err(|e| format!("{e:?}"))?
.clone(),
}))
}
}
// TODO(electra): fix this once we superstruct IndexedAttestation (return the correct type)
SurroundsExisting(existing) => match (*existing, new_attestation) {
(IndexedAttestation::Base(existing_att), IndexedAttestation::Base(new_att)) => {
Some(AttesterSlashing::Base(AttesterSlashingBase {
attestation_1: new_att.clone(),
attestation_2: existing_att,
IndexedAttestation::Electra(existing_att) => {
Some(AttesterSlashing::Electra(AttesterSlashingElectra {
attestation_1: existing_att,
// A double vote should never convert, a surround vote where the surrounding
// vote is electra may convert.
attestation_2: new_attestation
.clone()
.to_electra()
.map_err(|e| format!("{e:?}"))?,
}))
}
(
IndexedAttestation::Electra(existing_att),
IndexedAttestation::Electra(new_att),
) => Some(AttesterSlashing::Electra(AttesterSlashingElectra {
attestation_1: new_att.clone(),
attestation_2: existing_att,
})),
_ => panic!("attestations must be of the same type"),
},
/*
Some(AttesterSlashing::Base(AttesterSlashingBase {
attestation_1: new_attestation.clone(),
attestation_2: *existing,
})),
*/
}
SurroundsExisting(existing) => {
match new_attestation {
IndexedAttestation::Base(new_attestation) => {
Some(AttesterSlashing::Base(AttesterSlashingBase {
attestation_1: existing
.as_base()
.map_err(|e| format!("{e:?}"))?
.clone(),
attestation_2: new_attestation.clone(),
}))
}
IndexedAttestation::Electra(new_attestation) => {
Some(AttesterSlashing::Electra(AttesterSlashingElectra {
attestation_1: existing.to_electra().map_err(|e| format!("{e:?}"))?,
// A double vote should never convert, a surround vote where the surrounding
// vote is electra may convert.
attestation_2: new_attestation.clone(),
}))
}
}
}
})
}
}
28 changes: 20 additions & 8 deletions slasher/src/slasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ impl<E: EthSpec> Slasher<E> {
batch,
current_epoch,
&self.config,
&self.log,
) {
Ok(slashings) => {
if !slashings.is_empty() {
Expand Down Expand Up @@ -294,14 +295,25 @@ impl<E: EthSpec> Slasher<E> {
indexed_attestation_id,
)?;

if let Some(slashing) = slashing_status.into_slashing(attestation) {
debug!(
self.log,
"Found double-vote slashing";
"validator_index" => validator_index,
"epoch" => slashing.attestation_1().data().target.epoch,
);
slashings.insert(slashing);
match slashing_status.into_slashing(attestation) {
Ok(Some(slashing)) => {
debug!(
self.log,
"Found double-vote slashing";
"validator_index" => validator_index,
"epoch" => slashing.attestation_1().data().target.epoch,
);
slashings.insert(slashing);
}
Err(e) => {
error!(
self.log,
"Invalid slashing conversion";
"validator_index" => validator_index,
"error" => e
);
}
Ok(None) => {}
}
}

Expand Down
Loading