feat: QBFT value checks for BeaconVote (#487)

- #436

Author: dknopik

Cargo.lock

@@ -19,7 +19,9 @@ openssl = { workspace = true }
 operator_key = { workspace = true }
 rusqlite = { workspace = true }
 sha2 = { workspace = true }
+slashing_protection = { workspace = true }
 thiserror = { workspace = true }
+tracing = { workspace = true }
 tree_hash = { workspace = true }
 tree_hash_derive = { workspace = true }
 types = { workspace = true }

anchor/common/ssv_types/Cargo.toml

@@ -1,18 +1,24 @@
 use std::{
+    collections::HashMap,
     fmt::{Debug, DebugStruct, Display, Formatter},
     hash::Hash,
+    marker::PhantomData,
     ops::Deref,
+    sync::Arc,
 };
 
 use derive_more::{From, Into};
 use sha2::{Digest, Sha256};
+use slashing_protection::{NotSafe, SlashingDatabase};
 use ssz::{Decode, DecodeError, Encode};
 use ssz_derive::{Decode, Encode};
+use thiserror::Error;
+use tracing::warn;
 use tree_hash::{PackedEncoding, TreeHash, TreeHashType};
 use tree_hash_derive::TreeHash;
 use types::{
-    Checkpoint, CommitteeIndex, EthSpec, ForkName, Hash256, PublicKeyBytes, Signature, Slot,
-    SyncCommitteeContribution, VariableList,
+    AttestationData, ChainSpec, Checkpoint, CommitteeIndex, Domain, EthSpec, ForkName, Hash256,
+    PublicKeyBytes, Signature, Slot, SyncCommitteeContribution, VariableList,
     typenum::{U13, U56},
 };
 
@@ -360,3 +366,122 @@ impl QbftData for BeaconVote {
         Hash256::from(hash)
     }
 }
+
+pub struct BeaconVoteValidator<E: EthSpec> {
+    slot: Slot,
+    slashing_database: Arc<SlashingDatabase>,
+    disable_slashing_protection: bool,
+    spec: Arc<ChainSpec>,
+    validator_attestation_committees: HashMap<PublicKeyBytes, u64>,
+    genesis_validators_root: Hash256,
+    _phantom: PhantomData<E>,
+}
+
+impl<E: EthSpec> QbftDataValidator<BeaconVote> for BeaconVoteValidator<E> {
+    fn validate(&self, value: &BeaconVote, our_value: &BeaconVote) -> bool {
+        match self.do_validation(value, our_value) {
+            Ok(_) => true,
+            Err(err) => {
+                warn!(%err, "Operator proposed invalid beacon vote");
+                false
+            }
+        }
+    }
+}
+
+impl<E: EthSpec> BeaconVoteValidator<E> {
+    pub fn new(
+        slot: Slot,
+        slashing_database: Arc<SlashingDatabase>,
+        disable_slashing_protection: bool,
+        spec: Arc<ChainSpec>,
+        validator_attestation_committees: HashMap<PublicKeyBytes, u64>,
+        genesis_validators_root: Hash256,
+    ) -> Self {
+        Self {
+            slot,
+            slashing_database,
+            disable_slashing_protection,
+            spec,
+            validator_attestation_committees,
+            genesis_validators_root,
+            _phantom: PhantomData,
+        }
+    }
+
+    pub fn do_validation(
+        &self,
+        value: &BeaconVote,
+        _our_value: &BeaconVote,
+    ) -> Result<(), BeaconVoteValidationError> {
+        // Check target epoch is not too far in the future
+        let current_epoch = self.slot.epoch(E::slots_per_epoch());
+        if value.target.epoch > current_epoch + 1 {
+            return Err(BeaconVoteValidationError::FarFutureTargetEpoch(format!(
+                "current: {}, target: {}",
+                current_epoch.as_u64(),
+                value.target.epoch.as_u64()
+            )));
+        }
+
+        // Check source epoch < target epoch
+        if value.source.epoch >= value.target.epoch {
+            return Err(BeaconVoteValidationError::TargetNotAfterSource(format!(
+                "source {} >= target {}",
+                value.source.epoch.as_u64(),
+                value.target.epoch.as_u64()
+            )));
+        }
+
+        // Check slashing protection for all validator public keys
+        if !self.disable_slashing_protection {
+            self.check_attestation_slashing(value)?;
+        }
+
+        Ok(())
+    }
+
+    fn check_attestation_slashing(
+        &self,
+        value: &BeaconVote,
+    ) -> Result<(), BeaconVoteValidationError> {
+        // Create attestation data for slashing protection check
+        let mut attestation_data = AttestationData {
+            slot: self.slot,
+            index: 0, // Will be individually set below
+            beacon_block_root: value.block_root,
+            source: value.source,
+            target: value.target,
+        };
+
+        let epoch = self.slot.epoch(E::slots_per_epoch());
+
+        let domain_hash = self.spec.get_domain(
+            epoch,
+            Domain::BeaconAttester,
+            &self.spec.fork_at_epoch(epoch),
+            self.genesis_validators_root,
+        );
+
+        for (validator_pubkey, committee_index) in &self.validator_attestation_committees {
+            attestation_data.index = *committee_index;
+            self.slashing_database
+                .preliminary_check_attestation(validator_pubkey, &attestation_data, domain_hash)
+                .map_err(BeaconVoteValidationError::SlashableAttestation)?;
+        }
+
+        Ok(())
+    }
+}
+
+#[derive(Error, Debug)]
+pub enum BeaconVoteValidationError {
+    #[error("Unable to validate, bad slot clock")]
+    BadSlotClock,
+    #[error("Target epoch is too far in future: {0}")]
+    FarFutureTargetEpoch(String),
+    #[error("Invalid epoch order: {0}")]
+    TargetNotAfterSource(String),
+    #[error("Attestation would be slashable: {0}")]
+    SlashableAttestation(NotSafe),
+}

anchor/common/ssv_types/src/consensus.rs

@@ -2,7 +2,7 @@ pub mod metadata_service;
 mod metrics;
 
 use std::{
-    collections::HashMap,
+    collections::{HashMap, HashSet},
     fmt::Debug,
     future::Future,
     num::NonZeroUsize,
@@ -31,11 +31,11 @@ use signature_collector::{
 use slashing_protection::{NotSafe, Safe, SlashingDatabase};
 use slot_clock::SlotClock;
 use ssv_types::{
-    Cluster, ClusterId, ENCRYPTED_KEY_LENGTH, ValidatorIndex, ValidatorMetadata,
+    Cluster, ClusterId, CommitteeId, ENCRYPTED_KEY_LENGTH, ValidatorIndex, ValidatorMetadata,
     consensus::{
         BEACON_ROLE_AGGREGATOR, BEACON_ROLE_PROPOSER, BEACON_ROLE_SYNC_COMMITTEE_CONTRIBUTION,
-        BeaconVote, Contribution, ContributionWrapper, Contributions, NoDataValidation, QbftData,
-        ValidatorConsensusData, ValidatorDuty,
+        BeaconVote, BeaconVoteValidator, Contribution, ContributionWrapper, Contributions,
+        NoDataValidation, QbftData, ValidatorConsensusData, ValidatorDuty,
     },
     msgid::Role,
     partial_sig::PartialSignatureKind,
@@ -228,7 +228,7 @@ impl<T: SlotClock, E: EthSpec> AnchorValidatorStore<T, E> {
                 .map(|validator| {
                     let mut duties = 0;
                     if let Some(idx) = &validator.index {
-                        if slot_metadata.attesting_validators.contains(idx) {
+                        if slot_metadata.attesting_validator_indices.contains(idx) {
                             duties += 1;
                         }
                         if slot_metadata.sync_validators.contains(idx) {
@@ -525,6 +525,45 @@ impl<T: SlotClock, E: EthSpec> AnchorValidatorStore<T, E> {
 
         Ok(signed_exit)
     }
+
+    fn create_beacon_vote_validator(
+        &self,
+        slot: Slot,
+        validator_attestation_committees: HashMap<PublicKeyBytes, u64>,
+    ) -> Box<BeaconVoteValidator<E>> {
+        Box::new(BeaconVoteValidator::new(
+            slot,
+            Arc::clone(&self.slashing_protection),
+            self.disable_slashing_protection,
+            self.spec.clone(),
+            validator_attestation_committees,
+            self.genesis_validators_root,
+        ))
+    }
+
+    fn get_attesting_validators_in_committee(
+        &self,
+        metadata: &SlotMetadata<E>,
+        committee_id: CommitteeId,
+    ) -> HashMap<PublicKeyBytes, u64> {
+        let committee_validators = self
+            .database
+            .state()
+            .metadata()
+            .get_all_by(&committee_id)
+            .map(|v| v.public_key)
+            .collect::<HashSet<_>>();
+
+        metadata
+            .attesting_validator_committees
+            .iter()
+            .filter_map(|(&pubkey, &index)| {
+                committee_validators
+                    .contains(&pubkey)
+                    .then_some((pubkey, index))
+            })
+            .collect::<HashMap<_, _>>()
+    }
 }
 
 /// # Arguments
@@ -609,8 +648,11 @@ struct SlotMetadata<E: EthSpec> {
     slot: Slot,
     /// The BeaconVote we will use as initial QBFT data.
     beacon_vote: BeaconVote,
-    /// All our validators that are attesting in this slot.
-    attesting_validators: Vec<ValidatorIndex>,
+    /// The indices of all our validators that are attesting in this slot.
+    attesting_validator_indices: Vec<ValidatorIndex>,
+    /// The pubkeys of all our validators that are attesting in this slot, mapped to their
+    /// attestation committee index.
+    attesting_validator_committees: HashMap<PublicKeyBytes, u64>,
     /// All our validators that are in the sync committee for this slot.
     sync_validators: Vec<ValidatorIndex>,
     /// All validators that are aggregator for this slot multiple times, and thus require special
@@ -909,6 +951,10 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
             }
 
             let (validator, cluster) = self.get_validator_and_cluster(validator_pubkey)?;
+            let slot_metadata = self.get_slot_metadata(attestation.data().slot).await?;
+
+            let validator_attestation_committees =
+                self.get_attesting_validators_in_committee(&slot_metadata, cluster.committee_id());
 
             let timer =
                 metrics::start_timer_vec(&metrics::CONSENSUS_TIMES, &[metrics::BEACON_VOTE]);
@@ -928,7 +974,10 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                         source: attestation.data().source,
                         target: attestation.data().target,
                     },
-                    Box::new(NoDataValidation),
+                    self.create_beacon_vote_validator(
+                        attestation.data().slot,
+                        validator_attestation_committees,
+                    ),
                     start_time,
                     &cluster,
                 )
@@ -1246,6 +1295,9 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
             let (validator, cluster) = self.get_validator_and_cluster(*validator_pubkey)?;
             let metadata = self.get_slot_metadata(slot).await?;
 
+            let validator_attestation_committees =
+                self.get_attesting_validators_in_committee(&metadata, cluster.committee_id());
+
             let timer =
                 metrics::start_timer_vec(&metrics::CONSENSUS_TIMES, &[metrics::BEACON_VOTE]);
             let start_time = self
@@ -1258,7 +1310,7 @@ impl<T: SlotClock, E: EthSpec> ValidatorStore for AnchorValidatorStore<T, E> {
                         instance_height: slot.as_usize().into(),
                     },
                     metadata.beacon_vote.clone(),
-                    Box::new(NoDataValidation),
+                    self.create_beacon_vote_validator(slot, validator_attestation_committees),
                     start_time,
                     &cluster,
                 )

anchor/validator_store/src/lib.rs

@@ -100,12 +100,17 @@ impl<E: EthSpec, T: SlotClock + 'static> MetadataService<E, T> {
             target: attestation_data.target,
         };
 
-        let attesting_validators = self
+        let (attesting_validator_indices, attesting_validator_committees) = self
             .duties_service
             .attesters(slot)
             .into_iter()
-            .map(|duty| ValidatorIndex(duty.duty.validator_index as usize))
-            .collect();
+            .map(|duty| {
+                (
+                    ValidatorIndex(duty.duty.validator_index as usize),
+                    (duty.duty.pubkey, duty.duty.committee_index),
+                )
+            })
+            .unzip();
 
         let sync_duties = self
             .duties_service
@@ -142,7 +147,8 @@ impl<E: EthSpec, T: SlotClock + 'static> MetadataService<E, T> {
         let metadata = SlotMetadata {
             slot,
             beacon_vote,
-            attesting_validators,
+            attesting_validator_indices,
+            attesting_validator_committees,
             sync_validators,
             multi_sync_aggregators,
         };