From bafb5f0cc0aaea9a1ccae7337ae2d371fda41449 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Fri, 17 May 2024 09:51:32 -0400 Subject: [PATCH] fix slashing handling --- consensus/types/src/indexed_attestation.rs | 16 +++++ slasher/src/array.rs | 20 +++++- slasher/src/lib.rs | 81 +++++++++++----------- slasher/src/slasher.rs | 28 +++++--- 4 files changed, 95 insertions(+), 50 deletions(-) diff --git a/consensus/types/src/indexed_attestation.rs b/consensus/types/src/indexed_attestation.rs index 4e7e061b45f..f5b55f6d9b2 100644 --- a/consensus/types/src/indexed_attestation.rs +++ b/consensus/types/src/indexed_attestation.rs @@ -115,6 +115,22 @@ impl IndexedAttestation { IndexedAttestation::Electra(att) => att.attesting_indices.first(), } } + + pub fn to_electra(self) -> Result, ssz_types::Error> { + Ok(match self { + Self::Base(att) => { + let extended_attesting_indices: VariableList = + 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> { diff --git a/slasher/src/array.rs b/slasher/src/array.rs index 77ddceb85fe..714ccf4e77b 100644 --- a/slasher/src/array.rs +++ b/slasher/src/array.rs @@ -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; @@ -485,6 +486,7 @@ pub fn update( batch: Vec>>, current_epoch: Epoch, config: &Config, + log: &Logger, ) -> Result>, Error> { // Split the batch up into horizontal segments. // Map chunk indexes in the range `0..self.config.chunk_size` to attestations @@ -504,6 +506,7 @@ pub fn update( &chunk_attestations, current_epoch, config, + log, )?; slashings.extend(update_array::<_, MaxTargetChunk>( db, @@ -512,6 +515,7 @@ pub fn update( &chunk_attestations, current_epoch, config, + log, )?); // Update all current epochs. @@ -570,6 +574,7 @@ pub fn update_array( chunk_attestations: &BTreeMap>>>, current_epoch: Epoch, config: &Config, + log: &Logger, ) -> Result>, Error> { let mut slashings = HashSet::new(); // Map from chunk index to updated chunk at that index. @@ -603,8 +608,19 @@ pub fn update_array( 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) => {} } } } diff --git a/slasher/src/lib.rs b/slasher/src/lib.rs index 2577829577e..339ca1f7d3a 100644 --- a/slasher/src/lib.rs +++ b/slasher/src/lib.rs @@ -53,55 +53,56 @@ impl AttesterSlashingStatus { pub fn into_slashing( self, new_attestation: &IndexedAttestation, - ) -> Option> { + ) -> Result>, 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(), + })) + } + } + } + }) } } diff --git a/slasher/src/slasher.rs b/slasher/src/slasher.rs index fc8e8453c89..ef6dcce9b1b 100644 --- a/slasher/src/slasher.rs +++ b/slasher/src/slasher.rs @@ -247,6 +247,7 @@ impl Slasher { batch, current_epoch, &self.config, + &self.log, ) { Ok(slashings) => { if !slashings.is_empty() { @@ -294,14 +295,25 @@ impl Slasher { 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) => {} } }