From 89f0306ae3b068e7b9963c35738d186f21cbf6f0 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 3 Apr 2025 23:22:18 +0200 Subject: [PATCH 01/35] duty logic validation --- .githooks/pre-commit | 18 +- Cargo.lock | 1 + anchor/message_validator/Cargo.toml | 1 + .../message_validator/src/beacon_network.rs | 132 ++++++++ .../src/consensus_message.rs | 301 +++++++++++++++++- .../message_validator/src/consensus_state.rs | 13 + anchor/message_validator/src/duty_store.rs | 90 ++++++ anchor/message_validator/src/lib.rs | 50 ++- .../src/partial_signature.rs | 8 + 9 files changed, 575 insertions(+), 39 deletions(-) create mode 100644 anchor/message_validator/src/beacon_network.rs create mode 100644 anchor/message_validator/src/duty_store.rs diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 1abe953b5..3edf4577e 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,9 +1,9 @@ -#!/bin/sh -echo "Running cargo fmt --all..." -cargo +nightly fmt --all || exit 1 - -echo "Running cargo clippy --all..." -cargo clippy --all || exit 1 - -echo "Running cargo sort workspace..." -cargo sort --workspace || exit 1 +##!/bin/sh +#echo "Running cargo fmt --all..." +#cargo +nightly fmt --all || exit 1 +# +#echo "Running cargo clippy --all..." +#cargo clippy --all || exit 1 +# +#echo "Running cargo sort workspace..." +#cargo sort --workspace || exit 1 diff --git a/Cargo.lock b/Cargo.lock index fa8becd5d..323f95634 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5291,6 +5291,7 @@ dependencies = [ "ethereum_ssz", "hex", "libp2p-gossipsub 0.48.1", + "once_cell", "openssl", "parking_lot", "processor", diff --git a/anchor/message_validator/Cargo.toml b/anchor/message_validator/Cargo.toml index f1d94949d..fd5c5f2ec 100644 --- a/anchor/message_validator/Cargo.toml +++ b/anchor/message_validator/Cargo.toml @@ -19,6 +19,7 @@ ssv_types = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } +once_cell = "1.18.0" # For static initialization without lazy_static [dev-dependencies] bls = { workspace = true } diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs new file mode 100644 index 000000000..774fb30b8 --- /dev/null +++ b/anchor/message_validator/src/beacon_network.rs @@ -0,0 +1,132 @@ +use std::time::{Duration, SystemTime}; + +use slot_clock::SlotClock; +use types::{Epoch, Slot}; + +/// Wrapper around SlotClock to provide beacon chain network functionality +#[derive(Clone)] +pub struct BeaconNetwork { + slot_clock: S, + slots_per_epoch: u64, + epochs_per_sync_committee_period: u64, +} + +impl BeaconNetwork { + /// Create a new BeaconNetwork + pub fn new(slot_clock: S, slots_per_epoch: u64, epochs_per_sync_committee_period: u64) -> Self { + Self { + slot_clock, + slots_per_epoch, + epochs_per_sync_committee_period, + } + } + + /// Returns the slot clock + pub fn slot_clock(&self) -> &S { + &self.slot_clock + } + + /// Returns the slot duration + pub fn slot_duration(&self) -> Duration { + self.slot_clock.slot_duration() + } + + /// Returns the number of slots per epoch + pub fn slots_per_epoch(&self) -> u64 { + self.slots_per_epoch + } + + /// Estimates the current slot + pub fn estimated_current_slot(&self) -> Slot { + self.slot_clock.now().unwrap_or_default() + } + + /// Estimates the slot at the given time + pub fn estimated_slot_at_time(&self, time: SystemTime) -> Slot { + let since_unix = time + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default(); + + self.slot_clock.slot_of(since_unix).unwrap_or_default() + } + + /// Estimates the time at the given slot + pub fn estimated_time_at_slot(&self, slot: Slot) -> SystemTime { + let duration = self.slot_clock.start_of(slot).unwrap_or_default(); + SystemTime::UNIX_EPOCH + duration + } + + /// Estimates the current epoch + pub fn estimated_current_epoch(&self) -> Epoch { + self.estimated_epoch_at_slot(self.estimated_current_slot()) + } + + /// Estimates the epoch at the given slot + pub fn estimated_epoch_at_slot(&self, slot: Slot) -> Epoch { + Epoch::new(slot.as_u64() / self.slots_per_epoch) + } + + /// Returns the first slot at the given epoch + pub fn first_slot_at_epoch(&self, epoch: u64) -> Slot { + Slot::new(epoch * self.slots_per_epoch) + } + + /// Returns the start time of the given epoch + pub fn epoch_start_time(&self, epoch: u64) -> SystemTime { + self.estimated_time_at_slot(self.first_slot_at_epoch(epoch)) + } + + /// Returns the start time of the given slot + pub fn get_slot_start_time(&self, slot: Slot) -> SystemTime { + self.estimated_time_at_slot(slot) + } + + /// Returns the end time of the given slot + pub fn get_slot_end_time(&self, slot: Slot) -> SystemTime { + self.estimated_time_at_slot(slot + 1) + } + + /// Checks if the given slot is the first slot of its epoch + pub fn is_first_slot_of_epoch(&self, slot: Slot) -> bool { + slot.as_u64() % self.slots_per_epoch == 0 + } + + /// Returns the first slot of the given epoch + pub fn get_epoch_first_slot(&self, epoch: u64) -> Slot { + self.first_slot_at_epoch(epoch) + } + + /// Returns the number of epochs per sync committee period + pub fn epochs_per_sync_committee_period(&self) -> u64 { + self.epochs_per_sync_committee_period + } + + /// Estimates the sync committee period at the given epoch + pub fn estimated_sync_committee_period_at_epoch(&self, epoch: Epoch) -> Epoch { + epoch / self.epochs_per_sync_committee_period + } + + /// Returns the first epoch of the given sync committee period + pub fn first_epoch_of_sync_period(&self, period: u64) -> u64 { + period * self.epochs_per_sync_committee_period + } + + /// Returns the last slot of the given sync committee period + pub fn last_slot_of_sync_period(&self, period: u64) -> Slot { + let last_epoch = self.first_epoch_of_sync_period(period + 1) - 1; + // If we are in the sync committee that ends at slot x we do not generate a message + // during slot x-1 as it will never be included, hence -2. + self.get_epoch_first_slot(last_epoch + 1) - 2 + } +} + +/// Create a test beacon network with manual slot clock +pub fn create_test_beacon_network( + genesis_slot: Slot, + genesis_time: Duration, + slot_duration: Duration, + slots_per_epoch: u64, +) -> BeaconNetwork { + let clock = slot_clock::ManualSlotClock::new(genesis_slot, genesis_time, slot_duration); + BeaconNetwork::new(clock, slots_per_epoch, 256) +} diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index da8ad67b1..6f8875f16 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -8,20 +8,25 @@ use ssv_types::{ consensus::{QbftMessage, QbftMessageType}, message::SignedSSVMessage, msgid::Role, - CommitteeInfo, IndexSet, OperatorId, Round, Slot, VariableList, + CommitteeInfo, IndexSet, OperatorId, Round, Slot, ValidatorIndex, VariableList, }; use ssz::Decode; +use ValidationFailure::EarlySlotMessage; use crate::{ - compute_quorum_size, consensus_state::ConsensusState, hash_data, verify_message_signatures, - ValidatedSSVMessage, ValidationContext, ValidationFailure, + beacon_network::BeaconNetwork, + compute_quorum_size, + consensus_state::{ConsensusState, OperatorState}, + duty_store::DutyStore, + hash_data, verify_message_signatures, ValidatedSSVMessage, ValidationContext, + ValidationFailure, }; pub(crate) fn validate_consensus_message( validation_context: ValidationContext, consensus_state: &mut ConsensusState, - slots_per_epoch: u64, - slot_clock: impl SlotClock, + beacon_network: &BeaconNetwork, + duty_store: &DutyStore, ) -> Result { // Decode message to QbftMessage let consensus_message = match QbftMessage::from_ssz_bytes( @@ -42,7 +47,15 @@ pub(crate) fn validate_consensus_message( &validation_context, &consensus_message, consensus_state, - slot_clock, + beacon_network.slot_clock().clone(), + )?; + + validate_qbft_message_by_duty_logic( + &validation_context, + &consensus_message, + consensus_state, + &beacon_network, + &duty_store, )?; verify_message_signatures( @@ -53,7 +66,7 @@ pub(crate) fn validate_consensus_message( consensus_state.update( validation_context.signed_ssv_message, &consensus_message, - slots_per_epoch, + validation_context.slots_per_epoch, ); // Return the validated message @@ -290,10 +303,7 @@ fn validate_round_in_allowed_spread( ) -> Result<(), ValidationFailure> { // Get the slot let slot = Slot::new(consensus_message.height); - let slot_start_time = match slot_clock.start_of(slot) { - Some(time) => UNIX_EPOCH + time, - None => return Err(ValidationFailure::SlotStartTimeNotFound), - }; + let slot_start_time = get_slot_start_time(slot, &slot_clock)?; let (since_slot_start, estimated_round) = if received_at > slot_start_time { let duration = received_at @@ -355,6 +365,269 @@ fn current_estimated_round(since_slot_start: Duration) -> Round { (QUICK_TIMEOUT_THRESHOLD + FIRST_ROUND + delta_slow).into() } +// Constants needed for time validation +const CLOCK_ERROR_TOLERANCE: Duration = Duration::from_secs(20); +const LATE_MESSAGE_MARGIN: Duration = Duration::from_secs(1); +const LATE_SLOT_ALLOWANCE: u64 = 2; + +/// Validates QBFT messages based on beacon chain duties +pub(crate) fn validate_qbft_message_by_duty_logic( + validation_context: &ValidationContext, + consensus_message: &QbftMessage, + consensus_state: &mut ConsensusState, + beacon_network: &BeaconNetwork, + duty_store: &DutyStore, +) -> Result<(), ValidationFailure> { + let role = validation_context.role; + let signed_ssv_message = validation_context.signed_ssv_message; + + // Rule: Height must not be "old". I.e., signer must not have already advanced to a later slot. + if role != Role::Committee { + // Rule only for validator runners + for &signer in signed_ssv_message.operator_ids() { + let signer_state = consensus_state.get_or_create_operator(&signer); + let max_slot = signer_state.max_slot(); + if max_slot > consensus_message.height { + return Err(ValidationFailure::SlotAlreadyAdvanced { + got: consensus_message.height, + want: max_slot.as_u64(), + }); + } + } + } + + let msg_slot = Slot::new(consensus_message.height); + let randao_msg = false; // Default to false as in the Go code + + validate_beacon_duty( + validation_context, + msg_slot, + randao_msg, + &beacon_network, + duty_store, + )?; + + // Rule: current slot(height) must be between duty's starting slot and: + // - duty's starting slot + 34 (committee and aggregation) + // - duty's starting slot + 3 (other types) + validate_slot_time(msg_slot, validation_context, &beacon_network)?; + + // Rule: valid number of duties per epoch + for &signer in signed_ssv_message.operator_ids() { + let signer_state = consensus_state.get_or_create_operator(&signer); + validate_duty_count( + validation_context, + msg_slot.into(), + signer_state, + &beacon_network, + duty_store, + )?; + } + + Ok(()) +} + +/// Validates if a validator is assigned to a specific duty +pub(crate) fn validate_beacon_duty( + validation_context: &ValidationContext, + slot: Slot, + randao_msg: bool, + beacon_network: &BeaconNetwork, + duty_store: &DutyStore, +) -> Result<(), ValidationFailure> { + let role = validation_context.role; + let epoch = beacon_network.estimated_epoch_at_slot(slot); + + // Rule: For a proposal duty message, check if the validator is assigned to it + if role == Role::Proposer { + // Tolerate missing duties for RANDAO signatures during the first slot of an epoch, + // while duties are still being fetched from the Beacon node. + if randao_msg + && beacon_network.is_first_slot_of_epoch(slot) + && beacon_network.slot_clock().now().unwrap_or_default() <= slot + { + if !duty_store.is_epoch_set(epoch) { + return Ok(()); + } + } + + // Non-committee roles always have one validator index + let validator_index = validation_context + .committee_info + .validator_indices + .first() + .copied() + .unwrap_or_default(); + if !duty_store.validator_has_duty_at_slot(epoch, slot, validator_index) { + return Err(ValidationFailure::NoDuty); + } + } + + // Rule: For a sync committee duty message, check if the validator is assigned + if role == Role::SyncCommittee { + let period = beacon_network.estimated_sync_committee_period_at_epoch(epoch); + let validator_index = validation_context + .committee_info + .validator_indices + .first() + .copied() + .unwrap_or_default(); + if !duty_store.validator_in_sync_committee(period, validator_index) { + return Err(ValidationFailure::NoDuty); + } + } + + Ok(()) +} + +/// Validates that the message's slot timing is correct +pub(crate) fn validate_slot_time( + msg_slot: Slot, + validation_context: &ValidationContext, + beacon_network: &BeaconNetwork, +) -> Result<(), ValidationFailure> { + // Check if the message is too early + let earliness = message_earliness( + msg_slot, + validation_context.received_at, + beacon_network.slot_clock(), + )?; + if earliness > CLOCK_ERROR_TOLERANCE { + return Err(EarlySlotMessage { + got: format!("early by {:?}", earliness), + }); + } + + // Check if the message is too late + let lateness = message_lateness(msg_slot, validation_context, beacon_network.slot_clock())?; + if lateness > CLOCK_ERROR_TOLERANCE { + return Err(ValidationFailure::LateSlotMessage { + got: format!("late by {:?}", lateness), + }); + } + + Ok(()) +} + +/// Returns how early a message is compared to its slot start time +fn message_earliness( + slot: Slot, + received_at: SystemTime, + slot_clock: &impl SlotClock, +) -> Result { + let slot_start = get_slot_start_time(slot, slot_clock)?; + Ok(slot_start.duration_since(received_at).unwrap_or_default()) +} + +/// Returns how late a message is compared to its deadline based on role +fn message_lateness( + slot: Slot, + validation_context: &ValidationContext, + slot_clock: &impl SlotClock, +) -> Result { + let ttl = match validation_context.role { + Role::Proposer | Role::SyncCommittee => 1 + LATE_SLOT_ALLOWANCE, + Role::Committee | Role::Aggregator => { + validation_context.slots_per_epoch + LATE_SLOT_ALLOWANCE + } + // No lateness check for these roles + Role::ValidatorRegistration | Role::VoluntaryExit => return Ok(Duration::from_secs(0)), + }; + + let deadline = get_slot_start_time(slot + ttl, slot_clock)? + .checked_add(LATE_MESSAGE_MARGIN) + .unwrap_or_else(|| { + SystemTime::now() // Fallback if overflow occurs + }); + + Ok(validation_context + .received_at + .duration_since(deadline) + .unwrap_or_default()) +} + +/// Validates the duty count for a specific message and operator +pub(crate) fn validate_duty_count( + validation_context: &ValidationContext, + slot: Slot, + signer_state: &mut OperatorState, + beacon_network: &BeaconNetwork, + duty_store: &DutyStore, +) -> Result<(), ValidationFailure> { + let (limit, should_check) = duty_limit( + validation_context.role, + slot, + &validation_context.committee_info.validator_indices, + beacon_network, + duty_store, + ); + + if should_check { + // Get current duty count for this signer + let duty_count = signer_state.get_duty_count(beacon_network.estimated_epoch_at_slot(slot)); + + if duty_count >= limit { + return Err(ValidationFailure::ExcessiveDutyCount { + got: duty_count, + limit, + }); + } + } + + Ok(()) +} + +/// Determines duty limit based on role and validator indices +fn duty_limit( + role: Role, + slot: Slot, + validator_indices: &[ValidatorIndex], + beacon_network: &BeaconNetwork, + duty_store: &DutyStore, +) -> (u64, bool) { + match role { + Role::VoluntaryExit => { + // For voluntary exit, check the stored duties + // This would need to be adapted to use the actual duty store + (2, true) // Simplification - assuming 2 as in Go code + } + Role::Aggregator | Role::ValidatorRegistration => (2, true), + Role::Committee => { + let validator_index_count = validator_indices.len() as u64; + let slots_per_epoch_val = beacon_network.slots_per_epoch(); + + // Skip duty search if validators * 2 exceeds slots per epoch + if validator_index_count < slots_per_epoch_val / 2 { + let epoch = beacon_network.estimated_epoch_at_slot(slot); + let period = beacon_network.estimated_sync_committee_period_at_epoch(epoch); + + // Check if at least one validator is in the sync committee + for &index in validator_indices { + if duty_store.validator_in_sync_committee(period, index) { + return (slots_per_epoch_val, true); + } + } + } + + ( + std::cmp::min(slots_per_epoch_val, 2 * validator_index_count), + true, + ) + } + _ => (0, false), + } +} + +fn get_slot_start_time( + slot: Slot, + slot_clock: &impl SlotClock, +) -> Result { + match slot_clock.start_of(slot) { + Some(time) => Ok(UNIX_EPOCH + time), + None => return Err(ValidationFailure::SlotStartTimeNotFound), + } +} + #[cfg(test)] mod tests { use bls::{Hash256, PublicKeyBytes}; @@ -548,17 +821,18 @@ mod tests { role: Role::Committee, received_at: SystemTime::now(), operators_pk: &[public_key], + slots_per_epoch: 32, }; let result = validate_ssv_message( validation_context, &mut ConsensusState::new(2), 32, - ManualSlotClock::new( + BeaconNetwork::new(ManualSlotClock::new( Slot::new(0), SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), Duration::from_secs(1), - ), + )), ); match result { @@ -598,6 +872,7 @@ mod tests { role: Role::Committee, received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), + slots_per_epoch: 32, }; let result = validate_ssv_message( diff --git a/anchor/message_validator/src/consensus_state.rs b/anchor/message_validator/src/consensus_state.rs index 0da29de00..700d0f1fc 100644 --- a/anchor/message_validator/src/consensus_state.rs +++ b/anchor/message_validator/src/consensus_state.rs @@ -99,6 +99,19 @@ impl OperatorState { } } + /// Retrieves the maximum slot number processed for this operator. + pub(crate) fn max_slot(&self) -> Slot { + self.max_slot + } + + pub(crate) fn get_duty_count(&self, epoch: Epoch) -> u64 { + match epoch { + e if e == self.max_epoch => self.last_epoch_duties, + e if e == self.max_epoch - 1 => self.prev_epoch_duties, + _ => 0, // unused because messages from too old epochs must be rejected in advance + } + } + /// Retrieves the SignerState for a given slot, if it exists. /// /// The state is stored in a circular buffer and is accessed using modulo arithmetic. diff --git a/anchor/message_validator/src/duty_store.rs b/anchor/message_validator/src/duty_store.rs new file mode 100644 index 000000000..543d67f4a --- /dev/null +++ b/anchor/message_validator/src/duty_store.rs @@ -0,0 +1,90 @@ +use dashmap::DashMap; +use ssv_types::ValidatorIndex; +use types::{Epoch, Slot}; + +#[derive(Clone)] +struct ProposerDuty { + slot: Slot, + validator_index: ValidatorIndex, +} + +#[derive(Clone)] +struct StoreDuty { + slot: Slot, + validator_index: ValidatorIndex, + duty: ProposerDuty, +} + +#[derive(Clone)] +struct StoreSyncCommitteeDuty { + validator_index: ValidatorIndex, + in_committee: bool, +} + +// A comprehensive thread-safe duty store using DashMap +pub struct DutyStore { + // Proposer duties map: epoch -> slot -> validator_index -> duty + proposer_duties: DashMap>>, + + // Sync committee duties map: period -> validator_index -> duty + sync_committee_duties: DashMap>, +} + +impl DutyStore { + fn new() -> Self { + Self { + proposer_duties: DashMap::new(), + sync_committee_duties: DashMap::new(), + } + } + + // Proposer duties methods + pub(crate) fn is_epoch_set(&self, epoch: Epoch) -> bool { + self.proposer_duties.contains_key(&epoch) + } + + pub(crate) fn validator_has_duty_at_slot( + &self, + epoch: Epoch, + slot: Slot, + validator_index: ValidatorIndex, + ) -> bool { + if let Some(epoch_duties) = self.proposer_duties.get(&epoch) { + if let Some(slot_duties) = epoch_duties.get(&slot) { + return slot_duties.contains_key(&validator_index); + } + } + false + } + + // Sync committee methods + pub(crate) fn validator_in_sync_committee( + &self, + period: Epoch, + validator_index: ValidatorIndex, + ) -> bool { + if let Some(period_duties) = self.sync_committee_duties.get(&period) { + if let Some(duty) = period_duties.get(&validator_index) { + return duty.in_committee; + } + } + false + } + + // Method to set sync committee duties + fn set_sync_committee_duties(&self, period: Epoch, duties: Vec) { + let period_map = self + .sync_committee_duties + .entry(period) + .or_insert_with(DashMap::new); + + for duty in duties { + period_map.insert(duty.validator_index, duty); + } + } + + // Method to reset sync committee duties for a period + fn reset_sync_committee_period(&self, period: Epoch) { + self.sync_committee_duties.remove(&period); + } +} diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index de646887f..7e27fa07e 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -1,5 +1,7 @@ +mod beacon_network; mod consensus_message; mod consensus_state; +mod duty_store; mod message_counts; mod partial_signature; @@ -28,7 +30,8 @@ use tokio::sync::watch::Receiver; use tracing::{error, trace}; use crate::{ - consensus_message::validate_consensus_message, consensus_state::ConsensusState, + beacon_network::BeaconNetwork, consensus_message::validate_consensus_message, + consensus_state::ConsensusState, duty_store::DutyStore, partial_signature::validate_partial_signature_message, }; @@ -40,9 +43,16 @@ pub enum ValidationFailure { UnknownValidator, ValidatorLiquidated, ValidatorNotAttesting, - EarlySlotMessage, - LateSlotMessage, - SlotAlreadyAdvanced, + EarlySlotMessage { + got: String, + }, + LateSlotMessage { + got: String, + }, + SlotAlreadyAdvanced { + got: u64, + want: u64, + }, RoundAlreadyAdvanced { got: u64, want: u64, @@ -127,6 +137,10 @@ pub enum ValidationFailure { SignatureVerificationFailed { reason: String, }, + ExcessiveDutyCount { + got: u64, + limit: u64, + }, } impl From<&ValidationFailure> for MessageAcceptance { @@ -137,9 +151,9 @@ impl From<&ValidationFailure> for MessageAcceptance { | ValidationFailure::UnknownValidator | ValidationFailure::ValidatorLiquidated | ValidationFailure::ValidatorNotAttesting - | ValidationFailure::EarlySlotMessage - | ValidationFailure::LateSlotMessage - | ValidationFailure::SlotAlreadyAdvanced + | ValidationFailure::EarlySlotMessage { .. } + | ValidationFailure::LateSlotMessage { .. } + | ValidationFailure::SlotAlreadyAdvanced { .. } | ValidationFailure::RoundAlreadyAdvanced { .. } | ValidationFailure::DecidedWithSameSigners | ValidationFailure::PubSubDataTooBig(_) @@ -190,26 +204,27 @@ struct ValidationContext<'a> { pub committee_info: &'a CommitteeInfo, pub received_at: SystemTime, // Small value type pub operators_pk: &'a [Rsa], + pub slots_per_epoch: u64, } pub struct Validator { network_state_rx: Receiver, consensus_state_map: DashMap, slots_per_epoch: u64, - slot_clock: S, + beacon_network: BeaconNetwork, } impl Validator { pub fn new( network_state_rx: Receiver, slots_per_epoch: u64, - slot_clock: S, + beacon_network: BeaconNetwork, ) -> Self { Self { network_state_rx, consensus_state_map: DashMap::new(), slots_per_epoch, - slot_clock, + beacon_network, } } @@ -256,17 +271,18 @@ impl Validator { let validation_context = ValidationContext { signed_ssv_message: &signed_ssv_message, - role, + role: role, committee_info: &committee_info, received_at: SystemTime::now(), operators_pk: &operators_pks, + slots_per_epoch: self.slots_per_epoch, }; validate_ssv_message( validation_context, consensus_state.value_mut(), - self.slots_per_epoch, - self.slot_clock.clone(), + &self.beacon_network, + &self.duty_store(), ) .map(|validated| ValidatedMessage::new(signed_ssv_message.clone(), validated)) } @@ -313,8 +329,8 @@ impl Validator { fn validate_ssv_message( validation_context: ValidationContext, consensus_state: &mut ConsensusState, - slots_per_epoch: u64, - slot_clock: impl SlotClock, + beacon_network: &BeaconNetwork, + duty_store: &DutyStore, ) -> Result { let ssv_message = validation_context.signed_ssv_message.ssv_message(); @@ -322,8 +338,8 @@ fn validate_ssv_message( MsgType::SSVConsensusMsgType => validate_consensus_message( validation_context, consensus_state, - slots_per_epoch, - slot_clock, + beacon_network, + duty_store, ), MsgType::SSVPartialSignatureMsgType => { validate_partial_signature_message(validation_context) diff --git a/anchor/message_validator/src/partial_signature.rs b/anchor/message_validator/src/partial_signature.rs index 2a15cc548..983b78ebe 100644 --- a/anchor/message_validator/src/partial_signature.rs +++ b/anchor/message_validator/src/partial_signature.rs @@ -234,6 +234,7 @@ mod tests { role: Role::Committee, received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); @@ -279,6 +280,7 @@ mod tests { role: Role::Proposer, received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); @@ -311,6 +313,7 @@ mod tests { role: Role::Proposer, received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); @@ -343,6 +346,7 @@ mod tests { role: Role::Proposer, received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); @@ -375,6 +379,7 @@ mod tests { role: Role::Proposer, received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); @@ -405,6 +410,7 @@ mod tests { role: Role::Proposer, received_at: SystemTime::now(), operators_pk: &[public_key], + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); @@ -447,6 +453,7 @@ mod tests { role: Role::Proposer, // Not a committee role, so validator index is checked received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); @@ -484,6 +491,7 @@ mod tests { role: Role::Committee, // Committee role, so validator index is not checked received_at: SystemTime::now(), operators_pk: &[public_key], + slots_per_epoch: 32, }; let result = validate_partial_signature_message(validation_context); From 62596d711c51ca61e9f8f7aa513984241bb9b4e8 Mon Sep 17 00:00:00 2001 From: diego Date: Tue, 15 Apr 2025 13:39:39 +0200 Subject: [PATCH 02/35] add duties tracker --- Cargo.lock | 9 +- anchor/database/src/state.rs | 8 + anchor/message_validator/Cargo.toml | 7 +- .../src/consensus_message.rs | 4 +- .../src/duties/duties_tracker.rs | 218 ++++++++++++++++++ anchor/message_validator/src/duties/mod.rs | 138 +++++++++++ anchor/message_validator/src/lib.rs | 1 + 7 files changed, 378 insertions(+), 7 deletions(-) create mode 100644 anchor/message_validator/src/duties/duties_tracker.rs create mode 100644 anchor/message_validator/src/duties/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 323f95634..b777d07cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5126,9 +5126,9 @@ dependencies = [ [[package]] name = "logroller" -version = "0.1.6" +version = "0.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e8dd932139da44917b3cd5812ed9536d985aa67203778e0507347579499f49c" +checksum = "90536db32a1cb3672665cdf3269bf030b0f395fabee863895c27b75b9f7a8a7d" dependencies = [ "chrono", "flate2", @@ -5285,19 +5285,22 @@ dependencies = [ name = "message_validator" version = "0.1.0" dependencies = [ + "beacon_node_fallback", "bls", "dashmap", "database", + "eth2", "ethereum_ssz", "hex", "libp2p-gossipsub 0.48.1", - "once_cell", "openssl", "parking_lot", "processor", + "safe_arith", "sha2 0.10.8", "slot_clock", "ssv_types", + "task_executor", "thiserror 2.0.12", "tokio", "tracing", diff --git a/anchor/database/src/state.rs b/anchor/database/src/state.rs index bd9e19ec9..81f78b882 100644 --- a/anchor/database/src/state.rs +++ b/anchor/database/src/state.rs @@ -365,4 +365,12 @@ impl NetworkState { validator_indices: validator_index.map(|idx| vec![idx]).unwrap_or(vec![]), }) } + + pub fn validator_indices(&self) -> Vec { + self.multi_state + .validator_metadata + .values() + .filter_map(|metadata| metadata.index.map(|idx| idx.0 as u64)) + .collect() + } } diff --git a/anchor/message_validator/Cargo.toml b/anchor/message_validator/Cargo.toml index fd5c5f2ec..4f78ae51d 100644 --- a/anchor/message_validator/Cargo.toml +++ b/anchor/message_validator/Cargo.toml @@ -5,21 +5,26 @@ edition = { workspace = true } authors = ["Sigma Prime "] [dependencies] +beacon_node_fallback = { workspace = true } +bls = { workspace = true } dashmap = { workspace = true } database = { workspace = true } +eth2 = { workspace = true } ethereum_ssz = { workspace = true } gossipsub = { workspace = true } hex = { workspace = true } openssl = { workspace = true } parking_lot = { workspace = true } processor = { workspace = true } +safe_arith = { workspace = true } sha2 = { workspace = true } slot_clock = { workspace = true } ssv_types = { workspace = true } +task_executor = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } -once_cell = "1.18.0" # For static initialization without lazy_static +types = { workspace = true } [dev-dependencies] bls = { workspace = true } diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 6f8875f16..e8dda25b5 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -445,10 +445,8 @@ pub(crate) fn validate_beacon_duty( if randao_msg && beacon_network.is_first_slot_of_epoch(slot) && beacon_network.slot_clock().now().unwrap_or_default() <= slot - { - if !duty_store.is_epoch_set(epoch) { + && !duty_store.is_epoch_set(epoch) { return Ok(()); - } } // Non-committee roles always have one validator index diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs new file mode 100644 index 000000000..8f2d800af --- /dev/null +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -0,0 +1,218 @@ +use std::sync::Arc; + +use beacon_node_fallback::BeaconNodeFallback; +use database::NetworkState; +use safe_arith::ArithError; +use slot_clock::SlotClock; +use task_executor::TaskExecutor; +use tokio::{sync::watch, time::sleep}; +use tracing::{debug, error, info, warn}; +use types::ChainSpec; + +use crate::duties::{Duties, ValidatorDuties}; + +#[derive(Debug)] +pub enum Error { + UnableToReadSlotClock, + FailedToDownloadAttesters(#[allow(dead_code)] String), + InvalidModulo(#[allow(dead_code)] ArithError), + Arith(#[allow(dead_code)] ArithError), + SyncDutiesNotFound(#[allow(dead_code)] u64), +} + +pub struct DutiesTracker { + /// The duties tracker. + pub duties: Duties, + /// + pub beacon_nodes: Arc>, + /// + pub spec: Arc, + /// + slots_per_epoch: u64, + /// The slot clock. + pub slot_clock: Arc, + // + /// The runtime for spawning tasks. + pub executor: TaskExecutor, + /// + network_state_rx: watch::Receiver, +} + +impl DutiesTracker { + pub async fn poll_sync_committee_duties(&self) -> Result<(), Error> { + let sync_duties = &self.duties.sync_duties; + let spec = &self.spec; + let current_slot = self.slot_clock.now().ok_or(Error::UnableToReadSlotClock)?; + let current_epoch = current_slot.epoch(self.slots_per_epoch); + + // If the Altair fork is yet to be activated, do not attempt to poll for duties. + if spec + .altair_fork_epoch + .map_or(true, |altair_epoch| current_epoch < altair_epoch) + { + return Ok(()); + } + + let current_sync_committee_period = current_epoch + .sync_committee_period(spec) + .map_err(Error::Arith)?; + let next_sync_committee_period = current_sync_committee_period + 1; + + // Clone the indices to avoid holding the borrow across .await points + let local_indices = { + let network_state = self.network_state_rx.borrow(); + network_state.validator_indices().clone() + }; + + // If duties aren't known for the current period, poll for them. + if !sync_duties.all_duties_known(current_sync_committee_period, &local_indices) { + self.poll_sync_committee_duties_for_period( + &local_indices.as_slice(), + current_sync_committee_period, + ) + .await?; + + // Prune previous duties (we avoid doing this too often as it locks the whole map). + sync_duties.prune(current_sync_committee_period); + } + + // If we're past the point in the current period where we should determine duties for the + // next period and they are not yet known, then poll. + if current_epoch.as_u64() % spec.epochs_per_sync_committee_period.as_u64() + >= epoch_offset(spec) + && !sync_duties.all_duties_known(next_sync_committee_period, &local_indices) + { + self.poll_sync_committee_duties_for_period(&local_indices, next_sync_committee_period) + .await?; + + // Prune (this is the main code path for updating duties, so we should almost always hit + // this prune). + sync_duties.prune(current_sync_committee_period); + } + + Ok(()) + } + + pub async fn poll_sync_committee_duties_for_period( + &self, + local_indices: &[u64], + sync_committee_period: u64, + ) -> Result<(), Error> { + // no local validators don't need to poll for sync committee + if local_indices.is_empty() { + debug!( + sync_committee_period, + "No validators, not polling for sync committee duties" + ); + return Ok(()); + } + + debug!( + sync_committee_period, + num_validators = local_indices.len(), + "Fetching sync committee duties" + ); + + let period_start_epoch = self.spec.epochs_per_sync_committee_period * sync_committee_period; + + let duties_response = self + .beacon_nodes + .first_success(|beacon_node| async move { + // let _timer = validator_metrics::start_timer_vec( + // &validator_metrics::DUTIES_SERVICE_TIMES, + // &[validator_metrics::VALIDATOR_DUTIES_SYNC_HTTP_POST], + // ); + beacon_node + .post_validator_duties_sync(period_start_epoch, local_indices) + .await + }) + .await; + + let duties = match duties_response { + Ok(res) => res.data, + Err(e) => { + warn!( + sync_committee_period, + error = %e, + "Failed to download sync committee duties" + ); + return Ok(()); + } + }; + + debug!(count = duties.len(), "Fetched sync duties from BN"); + + // Add duties to map. + let committee_duties = self + .duties + .sync_duties + .get_or_create_committee_duties(sync_committee_period, local_indices); + + let mut validator_writer = committee_duties.validators.write(); + for duty in duties { + let validator_duties = validator_writer + .get_mut(&duty.validator_index) + .ok_or(Error::SyncDutiesNotFound(duty.validator_index))?; + + let updated = validator_duties.as_ref().map_or(true, |existing_duties| { + let updated_due_to_reorg = existing_duties.duty.validator_sync_committee_indices + != duty.validator_sync_committee_indices; + if updated_due_to_reorg { + warn!( + message = "this could be due to a really long re-org, or a bug", + "Sync committee duties changed" + ); + } + updated_due_to_reorg + }); + + if updated { + info!( + validator_index = duty.validator_index, + sync_committee_period, "Validator in sync committee" + ); + + *validator_duties = Some(ValidatorDuties::new(duty)); + } + } + + Ok(()) + } + + pub fn start_update_service(self: Arc) { + // Spawn the task which keeps track of local sync committee duties. + let duties_tracker = self.clone(); + self.executor.spawn( + async move { + loop { + if let Err(e) = duties_tracker.poll_sync_committee_duties().await { + error!( + error = ?e, + "Failed to poll sync committee duties" + ); + } + + // Wait until the next slot before polling again. + // This doesn't mean that the beacon node will get polled every slot + // as the sync duties service will return early if it deems it already has + // enough information. + if let Some(duration) = duties_tracker.slot_clock.duration_to_next_slot() { + sleep(duration).await; + } else { + // Just sleep for one slot if we are unable to read the system clock, this + // gives us an opportunity for the clock to + // eventually come good. + sleep(duties_tracker.slot_clock.slot_duration()).await; + continue; + } + } + }, + "duties_service_sync_committee", + ); + } +} + +/// Number of epochs to wait from the start of the period before actually fetching duties. +fn epoch_offset(spec: &ChainSpec) -> u64 { + spec.epochs_per_sync_committee_period.as_u64() / 2 +} diff --git a/anchor/message_validator/src/duties/mod.rs b/anchor/message_validator/src/duties/mod.rs new file mode 100644 index 000000000..0bd5081aa --- /dev/null +++ b/anchor/message_validator/src/duties/mod.rs @@ -0,0 +1,138 @@ +use std::collections::HashMap; + +use bls::PublicKeyBytes; +use eth2::types::ProposerData; +use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard, RwLockWriteGuard}; +use types::{Epoch, Hash256, SyncDuty}; + +mod duties_tracker; + +/// Top-level data-structure containing sync duty information. +/// +/// This data is structured as a series of nested `HashMap`s wrapped in `RwLock`s. Fine-grained +/// locking is used to provide maximum concurrency for the different services reading and writing. +/// +/// Deadlocks are prevented by: +/// +/// 1. Hierarchical locking. It is impossible to lock an inner lock (e.g. `validators`) without +/// first locking its parent. +/// 2. One-at-a-time locking. For the innermost locks on the aggregator duties, all of the functions +/// in this file take care to only lock one validator at a time. We never hold a lock while +/// trying to obtain another one (hence no lock ordering issues). +pub struct SyncDutiesMap { + /// Map from sync committee period to duties for members of that sync committee. + committees: RwLock>, + /// Whether we are in `distributed` mode and using reduced lookahead for aggregate pre-compute. + distributed: bool, +} + +impl SyncDutiesMap { + fn new(distributed: bool) -> Self { + Self { + committees: RwLock::new(HashMap::new()), + distributed, + } + } + + fn get_or_create_committee_duties<'a, 'b>( + &'a self, + committee_period: u64, + validator_indices: impl IntoIterator, + ) -> MappedRwLockReadGuard<'a, CommitteeDuties> { + let mut committees_writer = self.committees.write(); + + committees_writer + .entry(committee_period) + .or_default() + .init(validator_indices); + + // Return shared reference + RwLockReadGuard::map( + RwLockWriteGuard::downgrade(committees_writer), + |committees_reader| &committees_reader[&committee_period], + ) + } + + /// Check if duties are already known for all of the given validators for `committee_period`. + fn all_duties_known(&self, committee_period: u64, validator_indices: &[u64]) -> bool { + self.committees + .read() + .get(&committee_period) + .is_some_and(|committee_duties| { + let validator_duties = committee_duties.validators.read(); + validator_indices + .iter() + .all(|index| validator_duties.contains_key(index)) + }) + } + + /// Prune duties for past sync committee periods from the map. + fn prune(&self, current_sync_committee_period: u64) { + self.committees + .write() + .retain(|period, _| *period >= current_sync_committee_period) + } +} + +/// Duties for a single sync committee period. +#[derive(Default)] +pub struct CommitteeDuties { + /// Map from validator index to validator duties. + /// + /// A `None` value indicates that the validator index is known *not* to be a member of the sync + /// committee, while a `Some` indicates a known member. An absent value indicates that the + /// validator index was not part of the set of local validators when the duties were fetched. + /// This allows us to track changes to the set of local validators. + validators: RwLock>>, +} + +impl CommitteeDuties { + fn init<'b>(&mut self, validator_indices: impl IntoIterator) { + validator_indices.into_iter().for_each(|validator_index| { + self.validators + .get_mut() + .entry(*validator_index) + .or_insert(None); + }) + } +} + +/// Duties for a single validator. +pub struct ValidatorDuties { + /// The sync duty: including validator sync committee indices & pubkey. + duty: SyncDuty, +} + +impl ValidatorDuties { + fn new(duty: SyncDuty) -> Self { + Self { duty } + } +} + +/// Duties for multiple validators, for a single slot. +/// +/// This type is returned to the sync service. +pub struct SlotDuties { + /// List of duties for all sync committee members at this slot. + /// + /// Note: this is intentionally NOT split by subnet so that we only sign + /// one `SyncCommitteeMessage` per validator (recall a validator may be part of multiple + /// subnets). + pub duties: Vec, +} + +/// To assist with readability, the dependent root for attester/proposer duties. +type DependentRoot = Hash256; + +type AttesterMap = HashMap>; +type ProposerMap = HashMap)>; + +pub struct Duties { + /// Maps a validator public key to their duties for each epoch. + pub attesters: RwLock, + /// Maps an epoch to all *local* proposers in this epoch. Notably, this does not contain + /// proposals for any validators which are not registered locally. + pub proposers: RwLock, + /// Map from validator index to sync committee duties. + pub sync_duties: SyncDutiesMap, +} diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index 7e27fa07e..4696e597f 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -1,6 +1,7 @@ mod beacon_network; mod consensus_message; mod consensus_state; +mod duties; mod duty_store; mod message_counts; mod partial_signature; From e2c6186b23e2604f61be950701e4887dbbee336b Mon Sep 17 00:00:00 2001 From: diego Date: Tue, 15 Apr 2025 18:39:19 +0200 Subject: [PATCH 03/35] fix compilation and simplify --- anchor/client/src/lib.rs | 18 +++- anchor/common/ssv_types/src/cluster.rs | 6 ++ anchor/message_receiver/src/manager.rs | 14 +-- anchor/message_sender/src/network.rs | 12 +-- .../message_validator/src/beacon_network.rs | 15 +-- .../src/consensus_message.rs | 98 +++++++++++++------ .../src/duties/duties_tracker.rs | 77 ++++++++++++--- anchor/message_validator/src/duties/mod.rs | 67 +++++++++---- anchor/message_validator/src/duty_store.rs | 90 ----------------- anchor/message_validator/src/lib.rs | 23 +++-- 10 files changed, 227 insertions(+), 193 deletions(-) delete mode 100644 anchor/message_validator/src/duty_store.rs diff --git a/anchor/client/src/lib.rs b/anchor/client/src/lib.rs index 261be321e..396331289 100644 --- a/anchor/client/src/lib.rs +++ b/anchor/client/src/lib.rs @@ -28,7 +28,7 @@ use eth2::{ use keygen::{encryption::decrypt, run_keygen, Keygen}; use message_receiver::NetworkMessageReceiver; use message_sender::NetworkMessageSender; -use message_validator::Validator; +use message_validator::{BeaconNetwork, DutiesTracker, Validator}; use network::Network; use openssl::{pkey::Private, rsa::Rsa}; use parking_lot::RwLock; @@ -372,10 +372,24 @@ impl Client { // Network sender/receiver let (network_tx, network_rx) = mpsc::channel::<(SubnetId, Vec)>(9001); + let duties_tracker = Arc::new(DutiesTracker::new( + beacon_nodes.clone(), + spec.clone(), + E::slots_per_epoch(), + slot_clock.clone(), + executor.clone(), + database.watch(), + )); + let message_validator = Arc::new(Validator::new( database.watch(), E::slots_per_epoch(), - slot_clock.clone(), + BeaconNetwork::new( + slot_clock.clone(), + E::slots_per_epoch(), + spec.epochs_per_sync_committee_period.as_u64(), + ), + duties_tracker.clone(), )); let network_message_sender = NetworkMessageSender::new( diff --git a/anchor/common/ssv_types/src/cluster.rs b/anchor/common/ssv_types/src/cluster.rs index 73e200a0a..22c87f6e6 100644 --- a/anchor/common/ssv_types/src/cluster.rs +++ b/anchor/common/ssv_types/src/cluster.rs @@ -70,6 +70,12 @@ pub struct ClusterMember { #[ssz(struct_behaviour = "transparent")] pub struct ValidatorIndex(pub usize); +impl From for u64 { + fn from(value: ValidatorIndex) -> Self { + value.0 as u64 + } +} + /// General Metadata about a Validator #[derive(Debug, Clone)] pub struct ValidatorMetadata { diff --git a/anchor/message_receiver/src/manager.rs b/anchor/message_receiver/src/manager.rs index 47d38c399..49bd7a72f 100644 --- a/anchor/message_receiver/src/manager.rs +++ b/anchor/message_receiver/src/manager.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use database::{NetworkState, UniqueIndex}; use gossipsub::{Message, MessageAcceptance, MessageId}; use libp2p::PeerId; -use message_validator::{ValidatedMessage, ValidatedSSVMessage, Validator}; +use message_validator::{DutiesProvider, ValidatedMessage, ValidatedSSVMessage, Validator}; use qbft_manager::QbftManager; use signature_collector::SignatureCollectorManager; use slot_clock::SlotClock; @@ -22,23 +22,23 @@ pub struct Outcome { } /// A message receiver that passes messages to responsible managers. -pub struct NetworkMessageReceiver { +pub struct NetworkMessageReceiver { processor: processor::Senders, qbft_manager: Arc, signature_collector: Arc, network_state_rx: watch::Receiver, outcome_tx: mpsc::Sender, - validator: Arc>, + validator: Arc>, } -impl NetworkMessageReceiver { +impl NetworkMessageReceiver { pub fn new( processor: processor::Senders, qbft_manager: Arc, signature_collector: Arc, network_state_rx: watch::Receiver, outcome_tx: mpsc::Sender, - validator: Arc>, + validator: Arc>, ) -> Arc { Arc::new(Self { processor, @@ -51,7 +51,9 @@ impl NetworkMessageReceiver { } } -impl MessageReceiver for Arc> { +impl MessageReceiver + for Arc> +{ fn receive( &self, propagation_source: PeerId, diff --git a/anchor/message_sender/src/network.rs b/anchor/message_sender/src/network.rs index 2b5ed0db9..a5f7e442b 100644 --- a/anchor/message_sender/src/network.rs +++ b/anchor/message_sender/src/network.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use message_validator::Validator; +use message_validator::{DutiesProvider, Validator}; use openssl::{ error::ErrorStack, hash::MessageDigest, @@ -22,16 +22,16 @@ use crate::{Error, MessageCallback, MessageSender}; const SIGNER_NAME: &str = "message_sign_and_send"; const SENDER_NAME: &str = "message_send"; -pub struct NetworkMessageSender { +pub struct NetworkMessageSender { processor: processor::Senders, network_tx: mpsc::Sender<(SubnetId, Vec)>, private_key: PKey, operator_id: OperatorId, - validator: Option>>, + validator: Option>>, subnet_count: usize, } -impl MessageSender for Arc> { +impl MessageSender for Arc> { fn sign_and_send( &self, message: UnsignedSSVMessage, @@ -94,13 +94,13 @@ impl MessageSender for Arc> { } } -impl NetworkMessageSender { +impl NetworkMessageSender { pub fn new( processor: processor::Senders, network_tx: mpsc::Sender<(SubnetId, Vec)>, private_key: Rsa, operator_id: OperatorId, - validator: Option>>, + validator: Option>>, subnet_count: usize, ) -> Result, String> { let private_key = PKey::from_rsa(private_key) diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs index 774fb30b8..659eb23ff 100644 --- a/anchor/message_validator/src/beacon_network.rs +++ b/anchor/message_validator/src/beacon_network.rs @@ -102,8 +102,8 @@ impl BeaconNetwork { } /// Estimates the sync committee period at the given epoch - pub fn estimated_sync_committee_period_at_epoch(&self, epoch: Epoch) -> Epoch { - epoch / self.epochs_per_sync_committee_period + pub fn estimated_sync_committee_period_at_epoch(&self, epoch: Epoch) -> u64 { + epoch.as_u64() / self.epochs_per_sync_committee_period } /// Returns the first epoch of the given sync committee period @@ -119,14 +119,3 @@ impl BeaconNetwork { self.get_epoch_first_slot(last_epoch + 1) - 2 } } - -/// Create a test beacon network with manual slot clock -pub fn create_test_beacon_network( - genesis_slot: Slot, - genesis_time: Duration, - slot_duration: Duration, - slots_per_epoch: u64, -) -> BeaconNetwork { - let clock = slot_clock::ManualSlotClock::new(genesis_slot, genesis_time, slot_duration); - BeaconNetwork::new(clock, slots_per_epoch, 256) -} diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index e8dda25b5..96938d09f 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -1,5 +1,6 @@ use std::{ convert::Into, + sync::Arc, time::{Duration, SystemTime, UNIX_EPOCH}, }; @@ -17,7 +18,7 @@ use crate::{ beacon_network::BeaconNetwork, compute_quorum_size, consensus_state::{ConsensusState, OperatorState}, - duty_store::DutyStore, + duties::DutiesProvider, hash_data, verify_message_signatures, ValidatedSSVMessage, ValidationContext, ValidationFailure, }; @@ -26,7 +27,7 @@ pub(crate) fn validate_consensus_message( validation_context: ValidationContext, consensus_state: &mut ConsensusState, beacon_network: &BeaconNetwork, - duty_store: &DutyStore, + duty_provider: Arc, ) -> Result { // Decode message to QbftMessage let consensus_message = match QbftMessage::from_ssz_bytes( @@ -54,8 +55,8 @@ pub(crate) fn validate_consensus_message( &validation_context, &consensus_message, consensus_state, - &beacon_network, - &duty_store, + beacon_network, + duty_provider, )?; verify_message_signatures( @@ -376,7 +377,7 @@ pub(crate) fn validate_qbft_message_by_duty_logic( consensus_message: &QbftMessage, consensus_state: &mut ConsensusState, beacon_network: &BeaconNetwork, - duty_store: &DutyStore, + duty_provider: Arc, ) -> Result<(), ValidationFailure> { let role = validation_context.role; let signed_ssv_message = validation_context.signed_ssv_message; @@ -403,24 +404,24 @@ pub(crate) fn validate_qbft_message_by_duty_logic( validation_context, msg_slot, randao_msg, - &beacon_network, - duty_store, + beacon_network, + duty_provider.clone(), )?; // Rule: current slot(height) must be between duty's starting slot and: // - duty's starting slot + 34 (committee and aggregation) // - duty's starting slot + 3 (other types) - validate_slot_time(msg_slot, validation_context, &beacon_network)?; + validate_slot_time(msg_slot, validation_context, beacon_network)?; // Rule: valid number of duties per epoch for &signer in signed_ssv_message.operator_ids() { let signer_state = consensus_state.get_or_create_operator(&signer); validate_duty_count( validation_context, - msg_slot.into(), + msg_slot, signer_state, - &beacon_network, - duty_store, + beacon_network, + duty_provider.clone(), )?; } @@ -433,7 +434,7 @@ pub(crate) fn validate_beacon_duty( slot: Slot, randao_msg: bool, beacon_network: &BeaconNetwork, - duty_store: &DutyStore, + duty_provider: Arc, ) -> Result<(), ValidationFailure> { let role = validation_context.role; let epoch = beacon_network.estimated_epoch_at_slot(slot); @@ -445,8 +446,9 @@ pub(crate) fn validate_beacon_duty( if randao_msg && beacon_network.is_first_slot_of_epoch(slot) && beacon_network.slot_clock().now().unwrap_or_default() <= slot - && !duty_store.is_epoch_set(epoch) { - return Ok(()); + && !duty_provider.is_epoch_known_for_proposers(epoch) + { + return Ok(()); } // Non-committee roles always have one validator index @@ -456,7 +458,7 @@ pub(crate) fn validate_beacon_duty( .first() .copied() .unwrap_or_default(); - if !duty_store.validator_has_duty_at_slot(epoch, slot, validator_index) { + if !duty_provider.is_validator_proposer_at_slot(slot, validator_index) { return Err(ValidationFailure::NoDuty); } } @@ -470,7 +472,7 @@ pub(crate) fn validate_beacon_duty( .first() .copied() .unwrap_or_default(); - if !duty_store.validator_in_sync_committee(period, validator_index) { + if !duty_provider.is_validator_in_sync_committee(period, validator_index) { return Err(ValidationFailure::NoDuty); } } @@ -550,14 +552,14 @@ pub(crate) fn validate_duty_count( slot: Slot, signer_state: &mut OperatorState, beacon_network: &BeaconNetwork, - duty_store: &DutyStore, + duty_provider: Arc, ) -> Result<(), ValidationFailure> { let (limit, should_check) = duty_limit( validation_context.role, slot, &validation_context.committee_info.validator_indices, beacon_network, - duty_store, + duty_provider, ); if should_check { @@ -581,7 +583,7 @@ fn duty_limit( slot: Slot, validator_indices: &[ValidatorIndex], beacon_network: &BeaconNetwork, - duty_store: &DutyStore, + duty_provider: Arc, ) -> (u64, bool) { match role { Role::VoluntaryExit => { @@ -601,7 +603,7 @@ fn duty_limit( // Check if at least one validator is in the sync committee for &index in validator_indices { - if duty_store.validator_in_sync_committee(period, index) { + if duty_provider.is_validator_in_sync_committee(period, index) { return (slots_per_epoch_val, true); } } @@ -622,7 +624,7 @@ fn get_slot_start_time( ) -> Result { match slot_clock.start_of(slot) { Some(time) => Ok(UNIX_EPOCH + time), - None => return Err(ValidationFailure::SlotStartTimeNotFound), + None => Err(ValidationFailure::SlotStartTimeNotFound), } } @@ -706,6 +708,29 @@ mod tests { } } + struct MockDutiesProvider {} + impl DutiesProvider for MockDutiesProvider { + fn is_validator_in_sync_committee( + &self, + _committee_period: u64, + _validator_index: ValidatorIndex, + ) -> bool { + true + } + + fn is_epoch_known_for_proposers(&self, _epoch: Epoch) -> bool { + true + } + + fn is_validator_proposer_at_slot( + &self, + _slot: Slot, + _validator_index: ValidatorIndex, + ) -> bool { + true + } + } + // Helper for creating SignedSSVMessage with a QbftMessage fn create_signed_consensus_message( qbft_message: QbftMessage, @@ -825,12 +850,16 @@ mod tests { let result = validate_ssv_message( validation_context, &mut ConsensusState::new(2), - 32, - BeaconNetwork::new(ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - )), + &BeaconNetwork::new( + ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), + 32, + 256, + ), + Arc::new(MockDutiesProvider {}), ); match result { @@ -876,12 +905,16 @@ mod tests { let result = validate_ssv_message( validation_context, &mut ConsensusState::new(2), - 32, - ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), + &BeaconNetwork::new( + ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), + 32, + 256, ), + Arc::new(MockDutiesProvider {}), ); assert_validation_error( @@ -1315,6 +1348,7 @@ mod tests { rsa::Rsa, sign::Signer, }; + use types::Epoch; #[test] fn test_verify_message_signatures_success() { diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 8f2d800af..b3564bfef 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -4,42 +4,56 @@ use beacon_node_fallback::BeaconNodeFallback; use database::NetworkState; use safe_arith::ArithError; use slot_clock::SlotClock; +use ssv_types::ValidatorIndex; use task_executor::TaskExecutor; use tokio::{sync::watch, time::sleep}; use tracing::{debug, error, info, warn}; -use types::ChainSpec; +use types::{ChainSpec, Epoch, Slot}; -use crate::duties::{Duties, ValidatorDuties}; +use crate::duties::{Duties, DutiesProvider, ValidatorDuties}; #[derive(Debug)] pub enum Error { UnableToReadSlotClock, - FailedToDownloadAttesters(#[allow(dead_code)] String), - InvalidModulo(#[allow(dead_code)] ArithError), Arith(#[allow(dead_code)] ArithError), SyncDutiesNotFound(#[allow(dead_code)] u64), } pub struct DutiesTracker { - /// The duties tracker. + /// Duties data structures pub duties: Duties, - /// + /// The beacon node fallback clients pub beacon_nodes: Arc>, - /// pub spec: Arc, - /// slots_per_epoch: u64, /// The slot clock. - pub slot_clock: Arc, - // + pub slot_clock: T, /// The runtime for spawning tasks. pub executor: TaskExecutor, - /// network_state_rx: watch::Receiver, } impl DutiesTracker { - pub async fn poll_sync_committee_duties(&self) -> Result<(), Error> { + pub fn new( + beacon_nodes: Arc>, + spec: Arc, + slots_per_epoch: u64, + slot_clock: T, + executor: TaskExecutor, + network_state_rx: watch::Receiver, + ) -> Self { + Self { + duties: Duties::new(), + beacon_nodes, + spec, + slots_per_epoch, + slot_clock, + executor, + network_state_rx, + } + } + + async fn poll_sync_committee_duties(&self) -> Result<(), Error> { let sync_duties = &self.duties.sync_duties; let spec = &self.spec; let current_slot = self.slot_clock.now().ok_or(Error::UnableToReadSlotClock)?; @@ -48,7 +62,7 @@ impl DutiesTracker { // If the Altair fork is yet to be activated, do not attempt to poll for duties. if spec .altair_fork_epoch - .map_or(true, |altair_epoch| current_epoch < altair_epoch) + .is_none_or(|altair_epoch| current_epoch < altair_epoch) { return Ok(()); } @@ -67,7 +81,7 @@ impl DutiesTracker { // If duties aren't known for the current period, poll for them. if !sync_duties.all_duties_known(current_sync_committee_period, &local_indices) { self.poll_sync_committee_duties_for_period( - &local_indices.as_slice(), + local_indices.as_slice(), current_sync_committee_period, ) .await?; @@ -93,7 +107,7 @@ impl DutiesTracker { Ok(()) } - pub async fn poll_sync_committee_duties_for_period( + async fn poll_sync_committee_duties_for_period( &self, local_indices: &[u64], sync_committee_period: u64, @@ -154,7 +168,7 @@ impl DutiesTracker { .get_mut(&duty.validator_index) .ok_or(Error::SyncDutiesNotFound(duty.validator_index))?; - let updated = validator_duties.as_ref().map_or(true, |existing_duties| { + let updated = validator_duties.as_ref().is_none_or(|existing_duties| { let updated_due_to_reorg = existing_duties.duty.validator_sync_committee_indices != duty.validator_sync_committee_indices; if updated_due_to_reorg { @@ -212,6 +226,37 @@ impl DutiesTracker { } } +impl DutiesProvider for DutiesTracker { + fn is_validator_in_sync_committee( + &self, + committee_period: u64, + validator_index: ValidatorIndex, + ) -> bool { + self.duties + .sync_duties + .is_validator_in_sync_committee(committee_period, validator_index.into()) + } + + fn is_epoch_known_for_proposers(&self, epoch: Epoch) -> bool { + self.duties.proposers.read().contains_key(&epoch) + } + + fn is_validator_proposer_at_slot(&self, slot: Slot, validator_index: ValidatorIndex) -> bool { + let epoch = slot.epoch(self.slots_per_epoch); + let validator_index: u64 = validator_index.into(); + self.duties + .proposers + .read() + .get(&epoch) + .map(|(_, proposers)| { + proposers.iter().any(|proposer_data| { + proposer_data.slot == slot && proposer_data.validator_index == validator_index + }) + }) + .unwrap_or_default() + } +} + /// Number of epochs to wait from the start of the period before actually fetching duties. fn epoch_offset(spec: &ChainSpec) -> u64 { spec.epochs_per_sync_committee_period.as_u64() / 2 diff --git a/anchor/message_validator/src/duties/mod.rs b/anchor/message_validator/src/duties/mod.rs index 0bd5081aa..190bbeb3c 100644 --- a/anchor/message_validator/src/duties/mod.rs +++ b/anchor/message_validator/src/duties/mod.rs @@ -3,9 +3,10 @@ use std::collections::HashMap; use bls::PublicKeyBytes; use eth2::types::ProposerData; use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard, RwLockWriteGuard}; -use types::{Epoch, Hash256, SyncDuty}; +use ssv_types::ValidatorIndex; +use types::{Epoch, Hash256, Slot, SyncDuty}; -mod duties_tracker; +pub mod duties_tracker; /// Top-level data-structure containing sync duty information. /// @@ -22,15 +23,12 @@ mod duties_tracker; pub struct SyncDutiesMap { /// Map from sync committee period to duties for members of that sync committee. committees: RwLock>, - /// Whether we are in `distributed` mode and using reduced lookahead for aggregate pre-compute. - distributed: bool, } impl SyncDutiesMap { - fn new(distributed: bool) -> Self { + fn new() -> Self { Self { committees: RwLock::new(HashMap::new()), - distributed, } } @@ -72,6 +70,23 @@ impl SyncDutiesMap { .write() .retain(|period, _| *period >= current_sync_committee_period) } + + pub fn is_validator_in_sync_committee( + &self, + committee_period: u64, + validator_index: u64, + ) -> bool { + self.committees + .read() + .get(&committee_period) + .is_some_and(|committee_duties| { + committee_duties + .validators + .read() + .get(&validator_index) + .is_some_and(|duty| duty.is_some()) + }) + } } /// Duties for a single sync committee period. @@ -109,18 +124,6 @@ impl ValidatorDuties { } } -/// Duties for multiple validators, for a single slot. -/// -/// This type is returned to the sync service. -pub struct SlotDuties { - /// List of duties for all sync committee members at this slot. - /// - /// Note: this is intentionally NOT split by subnet so that we only sign - /// one `SyncCommitteeMessage` per validator (recall a validator may be part of multiple - /// subnets). - pub duties: Vec, -} - /// To assist with readability, the dependent root for attester/proposer duties. type DependentRoot = Hash256; @@ -136,3 +139,31 @@ pub struct Duties { /// Map from validator index to sync committee duties. pub sync_duties: SyncDutiesMap, } + +impl Duties { + pub fn new() -> Self { + Self { + attesters: RwLock::new(HashMap::new()), + proposers: RwLock::new(HashMap::new()), + sync_duties: SyncDutiesMap::new(), + } + } +} + +impl Default for Duties { + fn default() -> Self { + Self::new() + } +} + +pub trait DutiesProvider: Sync + Send + 'static { + fn is_validator_in_sync_committee( + &self, + committee_period: u64, + validator_index: ValidatorIndex, + ) -> bool; + + fn is_epoch_known_for_proposers(&self, epoch: Epoch) -> bool; + + fn is_validator_proposer_at_slot(&self, slot: Slot, validator_index: ValidatorIndex) -> bool; +} diff --git a/anchor/message_validator/src/duty_store.rs b/anchor/message_validator/src/duty_store.rs deleted file mode 100644 index 543d67f4a..000000000 --- a/anchor/message_validator/src/duty_store.rs +++ /dev/null @@ -1,90 +0,0 @@ -use dashmap::DashMap; -use ssv_types::ValidatorIndex; -use types::{Epoch, Slot}; - -#[derive(Clone)] -struct ProposerDuty { - slot: Slot, - validator_index: ValidatorIndex, -} - -#[derive(Clone)] -struct StoreDuty { - slot: Slot, - validator_index: ValidatorIndex, - duty: ProposerDuty, -} - -#[derive(Clone)] -struct StoreSyncCommitteeDuty { - validator_index: ValidatorIndex, - in_committee: bool, -} - -// A comprehensive thread-safe duty store using DashMap -pub struct DutyStore { - // Proposer duties map: epoch -> slot -> validator_index -> duty - proposer_duties: DashMap>>, - - // Sync committee duties map: period -> validator_index -> duty - sync_committee_duties: DashMap>, -} - -impl DutyStore { - fn new() -> Self { - Self { - proposer_duties: DashMap::new(), - sync_committee_duties: DashMap::new(), - } - } - - // Proposer duties methods - pub(crate) fn is_epoch_set(&self, epoch: Epoch) -> bool { - self.proposer_duties.contains_key(&epoch) - } - - pub(crate) fn validator_has_duty_at_slot( - &self, - epoch: Epoch, - slot: Slot, - validator_index: ValidatorIndex, - ) -> bool { - if let Some(epoch_duties) = self.proposer_duties.get(&epoch) { - if let Some(slot_duties) = epoch_duties.get(&slot) { - return slot_duties.contains_key(&validator_index); - } - } - false - } - - // Sync committee methods - pub(crate) fn validator_in_sync_committee( - &self, - period: Epoch, - validator_index: ValidatorIndex, - ) -> bool { - if let Some(period_duties) = self.sync_committee_duties.get(&period) { - if let Some(duty) = period_duties.get(&validator_index) { - return duty.in_committee; - } - } - false - } - - // Method to set sync committee duties - fn set_sync_committee_duties(&self, period: Epoch, duties: Vec) { - let period_map = self - .sync_committee_duties - .entry(period) - .or_insert_with(DashMap::new); - - for duty in duties { - period_map.insert(duty.validator_index, duty); - } - } - - // Method to reset sync committee duties for a period - fn reset_sync_committee_period(&self, period: Epoch) { - self.sync_committee_duties.remove(&period); - } -} diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index 4696e597f..d336a8545 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -2,12 +2,12 @@ mod beacon_network; mod consensus_message; mod consensus_state; mod duties; -mod duty_store; mod message_counts; mod partial_signature; -use std::time::SystemTime; +use std::{sync::Arc, time::SystemTime}; +pub use beacon_network::BeaconNetwork; use dashmap::{mapref::one::RefMut, DashMap}; use database::NetworkState; use gossipsub::MessageAcceptance; @@ -30,9 +30,9 @@ use ssz::{Decode, Encode}; use tokio::sync::watch::Receiver; use tracing::{error, trace}; +pub use crate::duties::{duties_tracker::DutiesTracker, DutiesProvider}; use crate::{ - beacon_network::BeaconNetwork, consensus_message::validate_consensus_message, - consensus_state::ConsensusState, duty_store::DutyStore, + consensus_message::validate_consensus_message, consensus_state::ConsensusState, partial_signature::validate_partial_signature_message, }; @@ -208,24 +208,27 @@ struct ValidationContext<'a> { pub slots_per_epoch: u64, } -pub struct Validator { +pub struct Validator { network_state_rx: Receiver, consensus_state_map: DashMap, slots_per_epoch: u64, beacon_network: BeaconNetwork, + duties_provider: Arc, } -impl Validator { +impl Validator { pub fn new( network_state_rx: Receiver, slots_per_epoch: u64, beacon_network: BeaconNetwork, + duties_provider: Arc, ) -> Self { Self { network_state_rx, consensus_state_map: DashMap::new(), slots_per_epoch, beacon_network, + duties_provider, } } @@ -272,7 +275,7 @@ impl Validator { let validation_context = ValidationContext { signed_ssv_message: &signed_ssv_message, - role: role, + role, committee_info: &committee_info, received_at: SystemTime::now(), operators_pk: &operators_pks, @@ -283,7 +286,7 @@ impl Validator { validation_context, consensus_state.value_mut(), &self.beacon_network, - &self.duty_store(), + self.duties_provider.clone(), ) .map(|validated| ValidatedMessage::new(signed_ssv_message.clone(), validated)) } @@ -331,7 +334,7 @@ fn validate_ssv_message( validation_context: ValidationContext, consensus_state: &mut ConsensusState, beacon_network: &BeaconNetwork, - duty_store: &DutyStore, + duty_provider: Arc, ) -> Result { let ssv_message = validation_context.signed_ssv_message.ssv_message(); @@ -340,7 +343,7 @@ fn validate_ssv_message( validation_context, consensus_state, beacon_network, - duty_store, + duty_provider, ), MsgType::SSVPartialSignatureMsgType => { validate_partial_signature_message(validation_context) From f48e866fce0e33338f649cf047ba0479315e404d Mon Sep 17 00:00:00 2001 From: diego Date: Tue, 15 Apr 2025 19:00:37 +0200 Subject: [PATCH 04/35] add parking_lot --- Cargo.lock | 1 + anchor/message_validator/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 59cf2efed..7bbcc50b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5250,6 +5250,7 @@ dependencies = [ "hex", "libp2p-gossipsub", "openssl", + "parking_lot", "processor", "safe_arith", "sha2 0.10.8", diff --git a/anchor/message_validator/Cargo.toml b/anchor/message_validator/Cargo.toml index 3d9296a6d..1db0d3f8c 100644 --- a/anchor/message_validator/Cargo.toml +++ b/anchor/message_validator/Cargo.toml @@ -14,6 +14,7 @@ ethereum_ssz = { workspace = true } gossipsub = { workspace = true } hex = { workspace = true } openssl = { workspace = true } +parking_lot = { workspace = true } processor = { workspace = true } safe_arith = { workspace = true } sha2 = { workspace = true } @@ -27,4 +28,3 @@ types = { workspace = true } [dev-dependencies] bls = { workspace = true } -types = { workspace = true } From 43a00db8f76581e7d1b146fd02efabf391527efd Mon Sep 17 00:00:00 2001 From: diego Date: Tue, 15 Apr 2025 23:14:50 +0200 Subject: [PATCH 05/35] add proposers duties tracking --- anchor/client/src/lib.rs | 1 + .../src/consensus_message.rs | 4 +- .../src/duties/duties_tracker.rs | 166 ++++++++++++++++-- 3 files changed, 155 insertions(+), 16 deletions(-) diff --git a/anchor/client/src/lib.rs b/anchor/client/src/lib.rs index 396331289..eb80088dc 100644 --- a/anchor/client/src/lib.rs +++ b/anchor/client/src/lib.rs @@ -380,6 +380,7 @@ impl Client { executor.clone(), database.watch(), )); + duties_tracker.clone().start(); let message_validator = Arc::new(Validator::new( database.watch(), diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 96938d09f..40c714cc9 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -587,9 +587,9 @@ fn duty_limit( ) -> (u64, bool) { match role { Role::VoluntaryExit => { - // For voluntary exit, check the stored duties + // TODO For voluntary exit, check the stored duties // This would need to be adapted to use the actual duty store - (2, true) // Simplification - assuming 2 as in Go code + (2, true) } Role::Aggregator | Role::ValidatorRegistration => (2, true), Role::Committee => { diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index b3564bfef..c2d36d528 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{future::Future, sync::Arc}; use beacon_node_fallback::BeaconNodeFallback; use database::NetworkState; @@ -12,6 +12,9 @@ use types::{ChainSpec, Epoch, Slot}; use crate::duties::{Duties, DutiesProvider, ValidatorDuties}; +/// Only retain `HISTORICAL_DUTIES_EPOCHS` duties prior to the current epoch. +const HISTORICAL_DUTIES_EPOCHS: u64 = 2; + #[derive(Debug)] pub enum Error { UnableToReadSlotClock, @@ -72,16 +75,16 @@ impl DutiesTracker { .map_err(Error::Arith)?; let next_sync_committee_period = current_sync_committee_period + 1; - // Clone the indices to avoid holding the borrow across .await points - let local_indices = { + // avoid holding the borrow across .await points + let validator_indices = { let network_state = self.network_state_rx.borrow(); - network_state.validator_indices().clone() + network_state.validator_indices() }; // If duties aren't known for the current period, poll for them. - if !sync_duties.all_duties_known(current_sync_committee_period, &local_indices) { + if !sync_duties.all_duties_known(current_sync_committee_period, &validator_indices) { self.poll_sync_committee_duties_for_period( - local_indices.as_slice(), + validator_indices.as_slice(), current_sync_committee_period, ) .await?; @@ -94,10 +97,13 @@ impl DutiesTracker { // next period and they are not yet known, then poll. if current_epoch.as_u64() % spec.epochs_per_sync_committee_period.as_u64() >= epoch_offset(spec) - && !sync_duties.all_duties_known(next_sync_committee_period, &local_indices) + && !sync_duties.all_duties_known(next_sync_committee_period, &validator_indices) { - self.poll_sync_committee_duties_for_period(&local_indices, next_sync_committee_period) - .await?; + self.poll_sync_committee_duties_for_period( + &validator_indices, + next_sync_committee_period, + ) + .await?; // Prune (this is the main code path for updating duties, so we should almost always hit // this prune). @@ -193,16 +199,148 @@ impl DutiesTracker { Ok(()) } - pub fn start_update_service(self: Arc) { - // Spawn the task which keeps track of local sync committee duties. + /// Download the proposer duties for the current epoch and store them in + /// `duties_service.proposers`. If there are any proposer for this slot, send out a + /// notification to the block proposers. + /// + /// ## Note + /// + /// This function will potentially send *two* notifications to the `BlockService`; it will send + /// a notification initially, then it will download the latest duties and send a *second* + /// notification if those duties have changed. This behaviour simultaneously achieves the + /// following: + /// + /// 1. Block production can happen immediately and does not have to wait for the proposer duties + /// to download. + /// 2. We won't miss a block if the duties for the current slot happen to change with this poll. + /// + /// This sounds great, but is it safe? Firstly, the additional notification will only contain + /// block producers that were not included in the first notification. This should be safe + /// enough. However, we also have the slashing protection as a second line of defence. These + /// two factors provide an acceptable level of safety. + /// + /// It's important to note that since there is a 0-epoch look-ahead (i.e., no look-ahead) for + /// block proposers then it's very likely that a proposal for the first slot of the epoch + /// will need go through the slow path every time. I.e., the proposal will only happen after + /// we've been able to download and process the duties from the BN. This means it is very + /// important to ensure this function is as fast as possible. + async fn poll_beacon_proposers(&self) -> Result<(), Error> { + // let _timer = validator_metrics::start_timer_vec( + // &validator_metrics::DUTIES_SERVICE_TIMES, + // &[validator_metrics::UPDATE_PROPOSERS], + // ); + + let current_slot = self.slot_clock.now().ok_or(Error::UnableToReadSlotClock)?; + let current_epoch = current_slot.epoch(self.slots_per_epoch); + + let download_result = self + .beacon_nodes + .first_success(|beacon_node| async move { + // let _timer = validator_metrics::start_timer_vec( + // &validator_metrics::DUTIES_SERVICE_TIMES, + // &[validator_metrics::PROPOSER_DUTIES_HTTP_GET], + // ); + beacon_node + .get_validator_duties_proposer(current_epoch) + .await + }) + .await; + + match download_result { + Ok(response) => { + let dependent_root = response.dependent_root; + + // avoid holding the borrow across .await points + let validator_indices = { + let network_state = self.network_state_rx.borrow(); + network_state.validator_indices() + }; + + let relevant_duties = response + .data + .into_iter() + .filter(|proposer_duty| { + validator_indices.contains(&proposer_duty.validator_index) + }) + .collect::>(); + + debug!( + %dependent_root, + num_relevant_duties = relevant_duties.len(), + "Downloaded proposer duties" + ); + + if let Some((prior_dependent_root, _)) = self + .duties + .proposers + .write() + .insert(current_epoch, (dependent_root, relevant_duties)) + { + if dependent_root != prior_dependent_root { + warn!( + %prior_dependent_root, + %dependent_root, + msg = "this may happen from time to time", + "Proposer duties re-org" + ) + } + } + } + // Don't return early here, we still want to try and produce blocks using the cached + // values. + Err(e) => error!( + err = %e, + "Failed to download proposer duties" + ), + } + + // Prune old duties. + self.duties + .proposers + .write() + .retain(|&epoch, _| epoch + HISTORICAL_DUTIES_EPOCHS >= current_epoch); + + Ok(()) + } + + pub fn start(self: Arc) { + let self_clone = self.clone(); + self_clone.spawn_polling_task( + |tracker| { + let tracker = tracker.clone(); + async move { tracker.poll_sync_committee_duties().await } + }, + "Failed to poll sync committee duties", + "sync_committee_tracker", + ); + + self.spawn_polling_task( + |tracker| { + let tracker = tracker.clone(); + async move { tracker.poll_beacon_proposers().await } + }, + "Failed to poll beacon proposers", + "proposers_tracker", + ); + } + + fn spawn_polling_task( + self: Arc, + poll_fn: F, + error_msg: &'static str, + task_name: &'static str, + ) where + F: Fn(Arc) -> Fut + Send + 'static, + Fut: Future> + Send + 'static, + { let duties_tracker = self.clone(); self.executor.spawn( async move { loop { - if let Err(e) = duties_tracker.poll_sync_committee_duties().await { + if let Err(e) = poll_fn(duties_tracker.clone()).await { error!( error = ?e, - "Failed to poll sync committee duties" + error_msg ); } @@ -221,7 +359,7 @@ impl DutiesTracker { } } }, - "duties_service_sync_committee", + task_name, ); } } From 17075d38e3807614bbc6d57111b3b932327996e6 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 10:40:01 +0200 Subject: [PATCH 06/35] uncomment pre-commit --- .githooks/pre-commit | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 3edf4577e..267ec031b 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,9 +1,9 @@ ##!/bin/sh -#echo "Running cargo fmt --all..." -#cargo +nightly fmt --all || exit 1 -# -#echo "Running cargo clippy --all..." -#cargo clippy --all || exit 1 -# -#echo "Running cargo sort workspace..." -#cargo sort --workspace || exit 1 +echo "Running cargo fmt --all..." +cargo +nightly fmt --all || exit 1 + +echo "Running cargo clippy --all..." +cargo clippy --all || exit 1 + +echo "Running cargo sort workspace..." +cargo sort --workspace || exit 1 From 3ccb16a23dc8720bdfc5c737f3a3048296abb300 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 14:42:52 +0200 Subject: [PATCH 07/35] simplify duties tracking --- .../src/duties/duties_tracker.rs | 42 +++------ anchor/message_validator/src/duties/mod.rs | 92 +++---------------- 2 files changed, 26 insertions(+), 108 deletions(-) diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index c2d36d528..3bdc71340 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -10,7 +10,7 @@ use tokio::{sync::watch, time::sleep}; use tracing::{debug, error, info, warn}; use types::{ChainSpec, Epoch, Slot}; -use crate::duties::{Duties, DutiesProvider, ValidatorDuties}; +use crate::duties::{Duties, DutiesProvider}; /// Only retain `HISTORICAL_DUTIES_EPOCHS` duties prior to the current epoch. const HISTORICAL_DUTIES_EPOCHS: u64 = 2; @@ -19,7 +19,6 @@ const HISTORICAL_DUTIES_EPOCHS: u64 = 2; pub enum Error { UnableToReadSlotClock, Arith(#[allow(dead_code)] ArithError), - SyncDutiesNotFound(#[allow(dead_code)] u64), } pub struct DutiesTracker { @@ -163,37 +162,18 @@ impl DutiesTracker { debug!(count = duties.len(), "Fetched sync duties from BN"); // Add duties to map. - let committee_duties = self - .duties - .sync_duties - .get_or_create_committee_duties(sync_committee_period, local_indices); + let mut committees_writer = self.duties.sync_duties.committees.write(); - let mut validator_writer = committee_duties.validators.write(); - for duty in duties { - let validator_duties = validator_writer - .get_mut(&duty.validator_index) - .ok_or(Error::SyncDutiesNotFound(duty.validator_index))?; - - let updated = validator_duties.as_ref().is_none_or(|existing_duties| { - let updated_due_to_reorg = existing_duties.duty.validator_sync_committee_indices - != duty.validator_sync_committee_indices; - if updated_due_to_reorg { - warn!( - message = "this could be due to a really long re-org, or a bug", - "Sync committee duties changed" - ); - } - updated_due_to_reorg - }); + let validators = committees_writer.entry(sync_committee_period).or_default(); - if updated { - info!( - validator_index = duty.validator_index, - sync_committee_period, "Validator in sync committee" - ); + for duty in duties { + info!( + validator_index = duty.validator_index, + sync_committee_period, "Validator in sync committee" + ); - *validator_duties = Some(ValidatorDuties::new(duty)); - } + // Simply insert or update the duty + validators.insert(duty.validator_index); } Ok(()) @@ -344,6 +324,8 @@ impl DutiesTracker { ); } + info!(sync_committee = ?duties_tracker.duties.sync_duties); + // Wait until the next slot before polling again. // This doesn't mean that the beacon node will get polled every slot // as the sync duties service will return early if it deems it already has diff --git a/anchor/message_validator/src/duties/mod.rs b/anchor/message_validator/src/duties/mod.rs index 190bbeb3c..b6f9bdc5c 100644 --- a/anchor/message_validator/src/duties/mod.rs +++ b/anchor/message_validator/src/duties/mod.rs @@ -1,10 +1,9 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; -use bls::PublicKeyBytes; use eth2::types::ProposerData; -use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard, RwLockWriteGuard}; +use parking_lot::RwLock; use ssv_types::ValidatorIndex; -use types::{Epoch, Hash256, Slot, SyncDuty}; +use types::{Epoch, Hash256, Slot}; pub mod duties_tracker; @@ -20,47 +19,28 @@ pub mod duties_tracker; /// 2. One-at-a-time locking. For the innermost locks on the aggregator duties, all of the functions /// in this file take care to only lock one validator at a time. We never hold a lock while /// trying to obtain another one (hence no lock ordering issues). -pub struct SyncDutiesMap { - /// Map from sync committee period to duties for members of that sync committee. - committees: RwLock>, +#[derive(Debug)] +pub struct SyncCommitteePerPeriod { + /// Map from sync committee period to members of that sync committee. + committees: RwLock>>, } -impl SyncDutiesMap { +impl SyncCommitteePerPeriod { fn new() -> Self { Self { committees: RwLock::new(HashMap::new()), } } - fn get_or_create_committee_duties<'a, 'b>( - &'a self, - committee_period: u64, - validator_indices: impl IntoIterator, - ) -> MappedRwLockReadGuard<'a, CommitteeDuties> { - let mut committees_writer = self.committees.write(); - - committees_writer - .entry(committee_period) - .or_default() - .init(validator_indices); - - // Return shared reference - RwLockReadGuard::map( - RwLockWriteGuard::downgrade(committees_writer), - |committees_reader| &committees_reader[&committee_period], - ) - } - /// Check if duties are already known for all of the given validators for `committee_period`. fn all_duties_known(&self, committee_period: u64, validator_indices: &[u64]) -> bool { self.committees .read() .get(&committee_period) - .is_some_and(|committee_duties| { - let validator_duties = committee_duties.validators.read(); + .is_some_and(|validators| { validator_indices .iter() - .all(|index| validator_duties.contains_key(index)) + .all(|index| validators.contains(index)) }) } @@ -79,73 +59,29 @@ impl SyncDutiesMap { self.committees .read() .get(&committee_period) - .is_some_and(|committee_duties| { - committee_duties - .validators - .read() - .get(&validator_index) - .is_some_and(|duty| duty.is_some()) - }) - } -} - -/// Duties for a single sync committee period. -#[derive(Default)] -pub struct CommitteeDuties { - /// Map from validator index to validator duties. - /// - /// A `None` value indicates that the validator index is known *not* to be a member of the sync - /// committee, while a `Some` indicates a known member. An absent value indicates that the - /// validator index was not part of the set of local validators when the duties were fetched. - /// This allows us to track changes to the set of local validators. - validators: RwLock>>, -} - -impl CommitteeDuties { - fn init<'b>(&mut self, validator_indices: impl IntoIterator) { - validator_indices.into_iter().for_each(|validator_index| { - self.validators - .get_mut() - .entry(*validator_index) - .or_insert(None); - }) - } -} - -/// Duties for a single validator. -pub struct ValidatorDuties { - /// The sync duty: including validator sync committee indices & pubkey. - duty: SyncDuty, -} - -impl ValidatorDuties { - fn new(duty: SyncDuty) -> Self { - Self { duty } + .is_some_and(|validator_indices| validator_indices.contains(&validator_index)) } } /// To assist with readability, the dependent root for attester/proposer duties. type DependentRoot = Hash256; -type AttesterMap = HashMap>; type ProposerMap = HashMap)>; +#[derive(Debug)] pub struct Duties { - /// Maps a validator public key to their duties for each epoch. - pub attesters: RwLock, /// Maps an epoch to all *local* proposers in this epoch. Notably, this does not contain /// proposals for any validators which are not registered locally. pub proposers: RwLock, /// Map from validator index to sync committee duties. - pub sync_duties: SyncDutiesMap, + pub sync_duties: SyncCommitteePerPeriod, } impl Duties { pub fn new() -> Self { Self { - attesters: RwLock::new(HashMap::new()), proposers: RwLock::new(HashMap::new()), - sync_duties: SyncDutiesMap::new(), + sync_duties: SyncCommitteePerPeriod::new(), } } } From 763589b48ab71e308cd2a2378253da7430c66076 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 14:50:06 +0200 Subject: [PATCH 08/35] use dashmap --- .../message_validator/src/duties/duties_tracker.rs | 14 +++++++++----- anchor/message_validator/src/duties/mod.rs | 8 +++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 3bdc71340..1c48377e7 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -161,18 +161,22 @@ impl DutiesTracker { debug!(count = duties.len(), "Fetched sync duties from BN"); - // Add duties to map. - let mut committees_writer = self.duties.sync_duties.committees.write(); - - let validators = committees_writer.entry(sync_committee_period).or_default(); + // Get or create the HashSet for this committee period + let mut validators = self + .duties + .sync_duties + .committees + .entry(sync_committee_period) + .or_default(); + // Insert only validators that have duties for duty in duties { info!( validator_index = duty.validator_index, sync_committee_period, "Validator in sync committee" ); - // Simply insert or update the duty + // Insert the validator index validators.insert(duty.validator_index); } diff --git a/anchor/message_validator/src/duties/mod.rs b/anchor/message_validator/src/duties/mod.rs index b6f9bdc5c..f87362e2d 100644 --- a/anchor/message_validator/src/duties/mod.rs +++ b/anchor/message_validator/src/duties/mod.rs @@ -1,5 +1,6 @@ use std::collections::{HashMap, HashSet}; +use dashmap::DashMap; use eth2::types::ProposerData; use parking_lot::RwLock; use ssv_types::ValidatorIndex; @@ -22,20 +23,19 @@ pub mod duties_tracker; #[derive(Debug)] pub struct SyncCommitteePerPeriod { /// Map from sync committee period to members of that sync committee. - committees: RwLock>>, + committees: DashMap>, } impl SyncCommitteePerPeriod { fn new() -> Self { Self { - committees: RwLock::new(HashMap::new()), + committees: DashMap::new(), } } /// Check if duties are already known for all of the given validators for `committee_period`. fn all_duties_known(&self, committee_period: u64, validator_indices: &[u64]) -> bool { self.committees - .read() .get(&committee_period) .is_some_and(|validators| { validator_indices @@ -47,7 +47,6 @@ impl SyncCommitteePerPeriod { /// Prune duties for past sync committee periods from the map. fn prune(&self, current_sync_committee_period: u64) { self.committees - .write() .retain(|period, _| *period >= current_sync_committee_period) } @@ -57,7 +56,6 @@ impl SyncCommitteePerPeriod { validator_index: u64, ) -> bool { self.committees - .read() .get(&committee_period) .is_some_and(|validator_indices| validator_indices.contains(&validator_index)) } From 4dc2620620837862dc188b4e19a2594fd8d2ba47 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 14:53:31 +0200 Subject: [PATCH 09/35] update docs for SyncCommitteePerPeriod --- anchor/message_validator/src/duties/mod.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/anchor/message_validator/src/duties/mod.rs b/anchor/message_validator/src/duties/mod.rs index f87362e2d..12cd288b2 100644 --- a/anchor/message_validator/src/duties/mod.rs +++ b/anchor/message_validator/src/duties/mod.rs @@ -10,19 +10,22 @@ pub mod duties_tracker; /// Top-level data-structure containing sync duty information. /// -/// This data is structured as a series of nested `HashMap`s wrapped in `RwLock`s. Fine-grained -/// locking is used to provide maximum concurrency for the different services reading and writing. +/// This data is structured using a `DashMap` which provides concurrent read/write access +/// with fine-grained locking at the entry level. This allows multiple threads to access +/// different entries without blocking each other. /// -/// Deadlocks are prevented by: +/// Key benefits of using DashMap over RwLock: +/// 1. Fine-grained locking at the individual entry level rather than the entire map +/// 2. Better performance in concurrent scenarios with many readers and occasional writers +/// 3. Simpler code that doesn't require explicit lock acquisition /// -/// 1. Hierarchical locking. It is impossible to lock an inner lock (e.g. `validators`) without -/// first locking its parent. -/// 2. One-at-a-time locking. For the innermost locks on the aggregator duties, all of the functions -/// in this file take care to only lock one validator at a time. We never hold a lock while -/// trying to obtain another one (hence no lock ordering issues). +/// The structure only stores validators that actually have sync committee duties, which +/// helps reduce memory usage compared to storing all validators and marking some as not +/// having duties. #[derive(Debug)] pub struct SyncCommitteePerPeriod { - /// Map from sync committee period to members of that sync committee. + /// Map from sync committee period to validators that are members of that sync committee. + /// Only validators with actual duties are stored in the HashSet for each period. committees: DashMap>, } From 1e7b599b9f7ecf4e05e69c66817251a633a0a650 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 16:47:30 +0200 Subject: [PATCH 10/35] remove metrics --- .../message_validator/src/duties/duties_tracker.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 1c48377e7..3677f967a 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -137,10 +137,6 @@ impl DutiesTracker { let duties_response = self .beacon_nodes .first_success(|beacon_node| async move { - // let _timer = validator_metrics::start_timer_vec( - // &validator_metrics::DUTIES_SERVICE_TIMES, - // &[validator_metrics::VALIDATOR_DUTIES_SYNC_HTTP_POST], - // ); beacon_node .post_validator_duties_sync(period_start_epoch, local_indices) .await @@ -209,10 +205,6 @@ impl DutiesTracker { /// we've been able to download and process the duties from the BN. This means it is very /// important to ensure this function is as fast as possible. async fn poll_beacon_proposers(&self) -> Result<(), Error> { - // let _timer = validator_metrics::start_timer_vec( - // &validator_metrics::DUTIES_SERVICE_TIMES, - // &[validator_metrics::UPDATE_PROPOSERS], - // ); let current_slot = self.slot_clock.now().ok_or(Error::UnableToReadSlotClock)?; let current_epoch = current_slot.epoch(self.slots_per_epoch); @@ -220,10 +212,6 @@ impl DutiesTracker { let download_result = self .beacon_nodes .first_success(|beacon_node| async move { - // let _timer = validator_metrics::start_timer_vec( - // &validator_metrics::DUTIES_SERVICE_TIMES, - // &[validator_metrics::PROPOSER_DUTIES_HTTP_GET], - // ); beacon_node .get_validator_duties_proposer(current_epoch) .await From 80ff366bfd4e99058d05212b853da3081b94dc87 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 16:59:33 +0200 Subject: [PATCH 11/35] improvements --- anchor/client/src/lib.rs | 3 +-- .../src/duties/duties_tracker.rs | 24 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/anchor/client/src/lib.rs b/anchor/client/src/lib.rs index eb80088dc..1e6fe8dce 100644 --- a/anchor/client/src/lib.rs +++ b/anchor/client/src/lib.rs @@ -377,10 +377,9 @@ impl Client { spec.clone(), E::slots_per_epoch(), slot_clock.clone(), - executor.clone(), database.watch(), )); - duties_tracker.clone().start(); + duties_tracker.clone().start(executor.clone()); let message_validator = Arc::new(Validator::new( database.watch(), diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 3677f967a..a6b786769 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -23,15 +23,16 @@ pub enum Error { pub struct DutiesTracker { /// Duties data structures - pub duties: Duties, + duties: Duties, /// The beacon node fallback clients - pub beacon_nodes: Arc>, - pub spec: Arc, + beacon_nodes: Arc>, + /// The chain spec + spec: Arc, + /// The number of slots per epoch slots_per_epoch: u64, /// The slot clock. - pub slot_clock: T, - /// The runtime for spawning tasks. - pub executor: TaskExecutor, + slot_clock: T, + /// The network state receiver. network_state_rx: watch::Receiver, } @@ -41,7 +42,6 @@ impl DutiesTracker { spec: Arc, slots_per_epoch: u64, slot_clock: T, - executor: TaskExecutor, network_state_rx: watch::Receiver, ) -> Self { Self { @@ -50,7 +50,6 @@ impl DutiesTracker { spec, slots_per_epoch, slot_clock, - executor, network_state_rx, } } @@ -88,7 +87,7 @@ impl DutiesTracker { ) .await?; - // Prune previous duties (we avoid doing this too often as it locks the whole map). + // Prune previous duties. sync_duties.prune(current_sync_committee_period); } @@ -275,7 +274,7 @@ impl DutiesTracker { Ok(()) } - pub fn start(self: Arc) { + pub fn start(self: Arc, executor: TaskExecutor) { let self_clone = self.clone(); self_clone.spawn_polling_task( |tracker| { @@ -284,6 +283,7 @@ impl DutiesTracker { }, "Failed to poll sync committee duties", "sync_committee_tracker", + executor.clone(), ); self.spawn_polling_task( @@ -293,6 +293,7 @@ impl DutiesTracker { }, "Failed to poll beacon proposers", "proposers_tracker", + executor, ); } @@ -301,12 +302,13 @@ impl DutiesTracker { poll_fn: F, error_msg: &'static str, task_name: &'static str, + executor: TaskExecutor, ) where F: Fn(Arc) -> Fut + Send + 'static, Fut: Future> + Send + 'static, { let duties_tracker = self.clone(); - self.executor.spawn( + executor.spawn( async move { loop { if let Err(e) = poll_fn(duties_tracker.clone()).await { From ae6a864648e2d35eaa176a46df54ae1dd6c561a1 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 17:05:15 +0200 Subject: [PATCH 12/35] remove functions not used in BeaconNetwork --- .../message_validator/src/beacon_network.rs | 52 ------------------- 1 file changed, 52 deletions(-) diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs index 659eb23ff..98a067084 100644 --- a/anchor/message_validator/src/beacon_network.rs +++ b/anchor/message_validator/src/beacon_network.rs @@ -36,66 +36,27 @@ impl BeaconNetwork { self.slots_per_epoch } - /// Estimates the current slot - pub fn estimated_current_slot(&self) -> Slot { - self.slot_clock.now().unwrap_or_default() - } - - /// Estimates the slot at the given time - pub fn estimated_slot_at_time(&self, time: SystemTime) -> Slot { - let since_unix = time - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap_or_default(); - - self.slot_clock.slot_of(since_unix).unwrap_or_default() - } - /// Estimates the time at the given slot pub fn estimated_time_at_slot(&self, slot: Slot) -> SystemTime { let duration = self.slot_clock.start_of(slot).unwrap_or_default(); SystemTime::UNIX_EPOCH + duration } - /// Estimates the current epoch - pub fn estimated_current_epoch(&self) -> Epoch { - self.estimated_epoch_at_slot(self.estimated_current_slot()) - } - /// Estimates the epoch at the given slot pub fn estimated_epoch_at_slot(&self, slot: Slot) -> Epoch { Epoch::new(slot.as_u64() / self.slots_per_epoch) } - /// Returns the first slot at the given epoch - pub fn first_slot_at_epoch(&self, epoch: u64) -> Slot { - Slot::new(epoch * self.slots_per_epoch) - } - - /// Returns the start time of the given epoch - pub fn epoch_start_time(&self, epoch: u64) -> SystemTime { - self.estimated_time_at_slot(self.first_slot_at_epoch(epoch)) - } - /// Returns the start time of the given slot pub fn get_slot_start_time(&self, slot: Slot) -> SystemTime { self.estimated_time_at_slot(slot) } - /// Returns the end time of the given slot - pub fn get_slot_end_time(&self, slot: Slot) -> SystemTime { - self.estimated_time_at_slot(slot + 1) - } - /// Checks if the given slot is the first slot of its epoch pub fn is_first_slot_of_epoch(&self, slot: Slot) -> bool { slot.as_u64() % self.slots_per_epoch == 0 } - /// Returns the first slot of the given epoch - pub fn get_epoch_first_slot(&self, epoch: u64) -> Slot { - self.first_slot_at_epoch(epoch) - } - /// Returns the number of epochs per sync committee period pub fn epochs_per_sync_committee_period(&self) -> u64 { self.epochs_per_sync_committee_period @@ -105,17 +66,4 @@ impl BeaconNetwork { pub fn estimated_sync_committee_period_at_epoch(&self, epoch: Epoch) -> u64 { epoch.as_u64() / self.epochs_per_sync_committee_period } - - /// Returns the first epoch of the given sync committee period - pub fn first_epoch_of_sync_period(&self, period: u64) -> u64 { - period * self.epochs_per_sync_committee_period - } - - /// Returns the last slot of the given sync committee period - pub fn last_slot_of_sync_period(&self, period: u64) -> Slot { - let last_epoch = self.first_epoch_of_sync_period(period + 1) - 1; - // If we are in the sync committee that ends at slot x we do not generate a message - // during slot x-1 as it will never be included, hence -2. - self.get_epoch_first_slot(last_epoch + 1) - 2 - } } From 2684a675b24e300fef3f6fa158f7ea57342ae558 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 16 Apr 2025 17:06:29 +0200 Subject: [PATCH 13/35] remove slots_per_epoch from Validator --- anchor/client/src/lib.rs | 1 - anchor/message_validator/src/duties/duties_tracker.rs | 1 - anchor/message_validator/src/lib.rs | 11 +++++------ 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/anchor/client/src/lib.rs b/anchor/client/src/lib.rs index 1e6fe8dce..74a3dae4f 100644 --- a/anchor/client/src/lib.rs +++ b/anchor/client/src/lib.rs @@ -383,7 +383,6 @@ impl Client { let message_validator = Arc::new(Validator::new( database.watch(), - E::slots_per_epoch(), BeaconNetwork::new( slot_clock.clone(), E::slots_per_epoch(), diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index a6b786769..08d3055df 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -204,7 +204,6 @@ impl DutiesTracker { /// we've been able to download and process the duties from the BN. This means it is very /// important to ensure this function is as fast as possible. async fn poll_beacon_proposers(&self) -> Result<(), Error> { - let current_slot = self.slot_clock.now().ok_or(Error::UnableToReadSlotClock)?; let current_epoch = current_slot.epoch(self.slots_per_epoch); diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index d336a8545..c61a1e1f2 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -211,7 +211,6 @@ struct ValidationContext<'a> { pub struct Validator { network_state_rx: Receiver, consensus_state_map: DashMap, - slots_per_epoch: u64, beacon_network: BeaconNetwork, duties_provider: Arc, } @@ -219,14 +218,12 @@ pub struct Validator { impl Validator { pub fn new( network_state_rx: Receiver, - slots_per_epoch: u64, beacon_network: BeaconNetwork, duties_provider: Arc, ) -> Self { Self { network_state_rx, consensus_state_map: DashMap::new(), - slots_per_epoch, beacon_network, duties_provider, } @@ -270,8 +267,10 @@ impl Validator { let operators_pks = self.get_operator_pks(signed_ssv_message.operator_ids())?; - let mut consensus_state = - self.get_consensus_state(ssv_message.msg_id(), self.slots_per_epoch); + let mut consensus_state = self.get_consensus_state( + ssv_message.msg_id(), + self.beacon_network.slots_per_epoch(), + ); let validation_context = ValidationContext { signed_ssv_message: &signed_ssv_message, @@ -279,7 +278,7 @@ impl Validator { committee_info: &committee_info, received_at: SystemTime::now(), operators_pk: &operators_pks, - slots_per_epoch: self.slots_per_epoch, + slots_per_epoch: self.beacon_network.slots_per_epoch(), }; validate_ssv_message( From c63075b8ebbf784c8a4ba4a98d6b227010d4123a Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 13:30:18 +0200 Subject: [PATCH 14/35] remove extra # --- .githooks/pre-commit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 267ec031b..1abe953b5 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,4 +1,4 @@ -##!/bin/sh +#!/bin/sh echo "Running cargo fmt --all..." cargo +nightly fmt --all || exit 1 From efb45e23b5e75b91e4398bc228be27158e4bcd0d Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 13:44:59 +0200 Subject: [PATCH 15/35] remove unnecessary get_slot_start_time --- .../src/consensus_message.rs | 41 +++++++------------ 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 40c714cc9..2be93f4d4 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -1,7 +1,7 @@ use std::{ convert::Into, sync::Arc, - time::{Duration, SystemTime, UNIX_EPOCH}, + time::{Duration, SystemTime}, }; use slot_clock::SlotClock; @@ -48,7 +48,7 @@ pub(crate) fn validate_consensus_message( &validation_context, &consensus_message, consensus_state, - beacon_network.slot_clock().clone(), + beacon_network, )?; validate_qbft_message_by_duty_logic( @@ -186,7 +186,7 @@ pub(crate) fn validate_qbft_logic( validation_context: &ValidationContext, consensus_message: &QbftMessage, consensus_state: &mut ConsensusState, - slot_clock: impl SlotClock, + beacon_network: &BeaconNetwork, ) -> Result<(), ValidationFailure> { let signed_ssv_message = validation_context.signed_ssv_message; @@ -265,7 +265,7 @@ pub(crate) fn validate_qbft_logic( consensus_message, validation_context.role, validation_context.received_at, - slot_clock, + beacon_network, )?; } @@ -300,11 +300,11 @@ fn validate_round_in_allowed_spread( consensus_message: &QbftMessage, role: Role, received_at: SystemTime, - slot_clock: impl SlotClock, + beacon_network: &BeaconNetwork, ) -> Result<(), ValidationFailure> { // Get the slot let slot = Slot::new(consensus_message.height); - let slot_start_time = get_slot_start_time(slot, &slot_clock)?; + let slot_start_time = beacon_network.get_slot_start_time(slot); let (since_slot_start, estimated_round) = if received_at > slot_start_time { let duration = received_at @@ -487,11 +487,7 @@ pub(crate) fn validate_slot_time( beacon_network: &BeaconNetwork, ) -> Result<(), ValidationFailure> { // Check if the message is too early - let earliness = message_earliness( - msg_slot, - validation_context.received_at, - beacon_network.slot_clock(), - )?; + let earliness = message_earliness(msg_slot, validation_context.received_at, beacon_network)?; if earliness > CLOCK_ERROR_TOLERANCE { return Err(EarlySlotMessage { got: format!("early by {:?}", earliness), @@ -499,7 +495,7 @@ pub(crate) fn validate_slot_time( } // Check if the message is too late - let lateness = message_lateness(msg_slot, validation_context, beacon_network.slot_clock())?; + let lateness = message_lateness(msg_slot, validation_context, beacon_network)?; if lateness > CLOCK_ERROR_TOLERANCE { return Err(ValidationFailure::LateSlotMessage { got: format!("late by {:?}", lateness), @@ -513,9 +509,9 @@ pub(crate) fn validate_slot_time( fn message_earliness( slot: Slot, received_at: SystemTime, - slot_clock: &impl SlotClock, + beacon_network: &BeaconNetwork, ) -> Result { - let slot_start = get_slot_start_time(slot, slot_clock)?; + let slot_start = beacon_network.get_slot_start_time(slot); Ok(slot_start.duration_since(received_at).unwrap_or_default()) } @@ -523,7 +519,7 @@ fn message_earliness( fn message_lateness( slot: Slot, validation_context: &ValidationContext, - slot_clock: &impl SlotClock, + beacon_network: &BeaconNetwork, ) -> Result { let ttl = match validation_context.role { Role::Proposer | Role::SyncCommittee => 1 + LATE_SLOT_ALLOWANCE, @@ -534,7 +530,8 @@ fn message_lateness( Role::ValidatorRegistration | Role::VoluntaryExit => return Ok(Duration::from_secs(0)), }; - let deadline = get_slot_start_time(slot + ttl, slot_clock)? + let deadline = beacon_network + .get_slot_start_time(slot + ttl) .checked_add(LATE_MESSAGE_MARGIN) .unwrap_or_else(|| { SystemTime::now() // Fallback if overflow occurs @@ -618,18 +615,10 @@ fn duty_limit( } } -fn get_slot_start_time( - slot: Slot, - slot_clock: &impl SlotClock, -) -> Result { - match slot_clock.start_of(slot) { - Some(time) => Ok(UNIX_EPOCH + time), - None => Err(ValidationFailure::SlotStartTimeNotFound), - } -} - #[cfg(test)] mod tests { + use std::time::UNIX_EPOCH; + use bls::{Hash256, PublicKeyBytes}; use openssl::hash::MessageDigest; use slot_clock::ManualSlotClock; From 36b0a68893d33213543ee0137e981157b68f01b6 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 14:17:14 +0200 Subject: [PATCH 16/35] propagate error instead of failing silently --- .../message_validator/src/beacon_network.rs | 19 ++++++++++++++----- .../src/consensus_message.rs | 9 +++++++-- anchor/message_validator/src/lib.rs | 5 ++++- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs index 98a067084..d7b6ddd99 100644 --- a/anchor/message_validator/src/beacon_network.rs +++ b/anchor/message_validator/src/beacon_network.rs @@ -1,8 +1,14 @@ -use std::time::{Duration, SystemTime}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use slot_clock::SlotClock; use types::{Epoch, Slot}; +#[derive(thiserror::Error, Debug)] +pub enum TimeError { + #[error("clock start-of-slot overflow for slot {0}")] + Overflow(Slot), +} + /// Wrapper around SlotClock to provide beacon chain network functionality #[derive(Clone)] pub struct BeaconNetwork { @@ -37,9 +43,12 @@ impl BeaconNetwork { } /// Estimates the time at the given slot - pub fn estimated_time_at_slot(&self, slot: Slot) -> SystemTime { - let duration = self.slot_clock.start_of(slot).unwrap_or_default(); - SystemTime::UNIX_EPOCH + duration + pub fn estimated_time_at_slot(&self, slot: Slot) -> Result { + let dur = self + .slot_clock + .start_of(slot) + .ok_or(TimeError::Overflow(slot))?; + Ok(UNIX_EPOCH + dur) } /// Estimates the epoch at the given slot @@ -48,7 +57,7 @@ impl BeaconNetwork { } /// Returns the start time of the given slot - pub fn get_slot_start_time(&self, slot: Slot) -> SystemTime { + pub fn get_slot_start_time(&self, slot: Slot) -> Result { self.estimated_time_at_slot(slot) } diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 2be93f4d4..2afa15952 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -304,7 +304,9 @@ fn validate_round_in_allowed_spread( ) -> Result<(), ValidationFailure> { // Get the slot let slot = Slot::new(consensus_message.height); - let slot_start_time = beacon_network.get_slot_start_time(slot); + let slot_start_time = beacon_network + .get_slot_start_time(slot) + .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })?; let (since_slot_start, estimated_round) = if received_at > slot_start_time { let duration = received_at @@ -511,7 +513,9 @@ fn message_earliness( received_at: SystemTime, beacon_network: &BeaconNetwork, ) -> Result { - let slot_start = beacon_network.get_slot_start_time(slot); + let slot_start = beacon_network + .get_slot_start_time(slot) + .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })?; Ok(slot_start.duration_since(received_at).unwrap_or_default()) } @@ -532,6 +536,7 @@ fn message_lateness( let deadline = beacon_network .get_slot_start_time(slot + ttl) + .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })? .checked_add(LATE_MESSAGE_MARGIN) .unwrap_or_else(|| { SystemTime::now() // Fallback if overflow occurs diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index c61a1e1f2..547dc1820 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -29,6 +29,7 @@ use ssv_types::{ use ssz::{Decode, Encode}; use tokio::sync::watch::Receiver; use tracing::{error, trace}; +use types::Slot; pub use crate::duties::{duties_tracker::DutiesTracker, DutiesProvider}; use crate::{ @@ -134,7 +135,9 @@ pub enum ValidationFailure { TooManyPartialSignatureMessages, EncodeOperators, FailedToGetMaxRound, - SlotStartTimeNotFound, + SlotStartTimeNotFound { + slot: Slot, + }, SignatureVerificationFailed { reason: String, }, From 070d4346be82ecead83b64759c8c8ef51283ee93 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 15:24:12 +0200 Subject: [PATCH 17/35] improve logging adding gossipsub and ssv msg id --- anchor/message_receiver/src/manager.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/anchor/message_receiver/src/manager.rs b/anchor/message_receiver/src/manager.rs index 6f99a0a73..0afc311bb 100644 --- a/anchor/message_receiver/src/manager.rs +++ b/anchor/message_receiver/src/manager.rs @@ -94,12 +94,12 @@ impl MessageReceiver } = match result { Ok(message) => message, Err(failure) => { - debug!(?failure, "Validation failure"); + debug!(gosspisub_message_id = ?message_id, ?failure, "Validation failure"); return; } }; - let msg_id = signed_ssv_message.ssv_message().msg_id(); + let msg_id = signed_ssv_message.ssv_message().msg_id().clone(); match msg_id.duty_executor() { Some(DutyExecutor::Validator(validator)) => { @@ -111,7 +111,7 @@ impl MessageReceiver .is_none() { // We are not a signer for this validator, return without passing. - trace!(?validator, ?msg_id, "Not interested"); + trace!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?validator, "Not interested"); return; } } @@ -127,12 +127,12 @@ impl MessageReceiver .unwrap_or(false) }) { // We are not a member for this committee, return without passing. - trace!(?committee, ?msg_id, "Not interested"); + trace!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?committee, "Not interested"); return; } } None => { - error!(?msg_id, "Invalid message ID"); + error!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, "Invalid message ID"); return; } } @@ -143,7 +143,7 @@ impl MessageReceiver .qbft_manager .receive_data(signed_ssv_message, qbft_message) { - error!(?err, "Unable to receive QBFT message"); + error!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?err, "Unable to receive QBFT message"); } } ValidatedSSVMessage::PartialSignatureMessages(messages) => { @@ -151,7 +151,7 @@ impl MessageReceiver .signature_collector .receive_partial_signatures(messages) { - error!(?err, "Unable to receive partial signature message"); + error!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?err, "Unable to receive partial signature message"); } } } From 9c9d3f1e55d1d10e877a6f780e99dc475ed84769 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 15:59:14 +0200 Subject: [PATCH 18/35] change log to trace --- anchor/message_validator/src/duties/duties_tracker.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 08d3055df..700f39c57 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -7,7 +7,7 @@ use slot_clock::SlotClock; use ssv_types::ValidatorIndex; use task_executor::TaskExecutor; use tokio::{sync::watch, time::sleep}; -use tracing::{debug, error, info, warn}; +use tracing::{debug, error, info, trace, warn}; use types::{ChainSpec, Epoch, Slot}; use crate::duties::{Duties, DutiesProvider}; @@ -317,7 +317,7 @@ impl DutiesTracker { ); } - info!(sync_committee = ?duties_tracker.duties.sync_duties); + trace!(sync_committee = ?duties_tracker.duties.sync_duties); // Wait until the next slot before polling again. // This doesn't mean that the beacon node will get polled every slot From 2bd726a2197d4f012ab73090c6a77ea3c990b06d Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 16:15:58 +0200 Subject: [PATCH 19/35] remove unnecessary things from proposers tracking --- .../src/duties/duties_tracker.rs | 53 +++---------------- anchor/message_validator/src/duties/mod.rs | 7 +-- 2 files changed, 9 insertions(+), 51 deletions(-) diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 700f39c57..6e56ad2d1 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -178,31 +178,7 @@ impl DutiesTracker { Ok(()) } - /// Download the proposer duties for the current epoch and store them in - /// `duties_service.proposers`. If there are any proposer for this slot, send out a - /// notification to the block proposers. - /// - /// ## Note - /// - /// This function will potentially send *two* notifications to the `BlockService`; it will send - /// a notification initially, then it will download the latest duties and send a *second* - /// notification if those duties have changed. This behaviour simultaneously achieves the - /// following: - /// - /// 1. Block production can happen immediately and does not have to wait for the proposer duties - /// to download. - /// 2. We won't miss a block if the duties for the current slot happen to change with this poll. - /// - /// This sounds great, but is it safe? Firstly, the additional notification will only contain - /// block producers that were not included in the first notification. This should be safe - /// enough. However, we also have the slashing protection as a second line of defence. These - /// two factors provide an acceptable level of safety. - /// - /// It's important to note that since there is a 0-epoch look-ahead (i.e., no look-ahead) for - /// block proposers then it's very likely that a proposal for the first slot of the epoch - /// will need go through the slow path every time. I.e., the proposal will only happen after - /// we've been able to download and process the duties from the BN. This means it is very - /// important to ensure this function is as fast as possible. + /// Download the proposer duties for the current epoch. async fn poll_beacon_proposers(&self) -> Result<(), Error> { let current_slot = self.slot_clock.now().ok_or(Error::UnableToReadSlotClock)?; let current_epoch = current_slot.epoch(self.slots_per_epoch); @@ -218,8 +194,6 @@ impl DutiesTracker { match download_result { Ok(response) => { - let dependent_root = response.dependent_root; - // avoid holding the borrow across .await points let validator_indices = { let network_state = self.network_state_rx.borrow(); @@ -234,31 +208,18 @@ impl DutiesTracker { }) .collect::>(); - debug!( - %dependent_root, + trace!( num_relevant_duties = relevant_duties.len(), "Downloaded proposer duties" ); - if let Some((prior_dependent_root, _)) = self - .duties + self.duties .proposers .write() - .insert(current_epoch, (dependent_root, relevant_duties)) - { - if dependent_root != prior_dependent_root { - warn!( - %prior_dependent_root, - %dependent_root, - msg = "this may happen from time to time", - "Proposer duties re-org" - ) - } - } + .insert(current_epoch, relevant_duties); } - // Don't return early here, we still want to try and produce blocks using the cached - // values. - Err(e) => error!( + // Don't return early here, we"ll try again later + Err(e) => warn!( err = %e, "Failed to download proposer duties" ), @@ -361,7 +322,7 @@ impl DutiesProvider for DutiesTracker { .proposers .read() .get(&epoch) - .map(|(_, proposers)| { + .map(|proposers| { proposers.iter().any(|proposer_data| { proposer_data.slot == slot && proposer_data.validator_index == validator_index }) diff --git a/anchor/message_validator/src/duties/mod.rs b/anchor/message_validator/src/duties/mod.rs index 12cd288b2..46a7692e5 100644 --- a/anchor/message_validator/src/duties/mod.rs +++ b/anchor/message_validator/src/duties/mod.rs @@ -4,7 +4,7 @@ use dashmap::DashMap; use eth2::types::ProposerData; use parking_lot::RwLock; use ssv_types::ValidatorIndex; -use types::{Epoch, Hash256, Slot}; +use types::{Epoch, Slot}; pub mod duties_tracker; @@ -64,10 +64,7 @@ impl SyncCommitteePerPeriod { } } -/// To assist with readability, the dependent root for attester/proposer duties. -type DependentRoot = Hash256; - -type ProposerMap = HashMap)>; +type ProposerMap = HashMap>; #[derive(Debug)] pub struct Duties { From 812e09ace9fcbc93c6c7680f43304c7f4d54a736 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 17:25:40 +0200 Subject: [PATCH 20/35] better error handling in poll_beacon_proposers --- .../src/duties/duties_tracker.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 6e56ad2d1..5ad28fd90 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -6,6 +6,7 @@ use safe_arith::ArithError; use slot_clock::SlotClock; use ssv_types::ValidatorIndex; use task_executor::TaskExecutor; +use thiserror::Error; use tokio::{sync::watch, time::sleep}; use tracing::{debug, error, info, trace, warn}; use types::{ChainSpec, Epoch, Slot}; @@ -15,10 +16,14 @@ use crate::duties::{Duties, DutiesProvider}; /// Only retain `HISTORICAL_DUTIES_EPOCHS` duties prior to the current epoch. const HISTORICAL_DUTIES_EPOCHS: u64 = 2; -#[derive(Debug)] +#[derive(Error, Debug)] pub enum Error { + #[error("Unable to read the slot clock")] UnableToReadSlotClock, + #[error("Arithmetic error")] Arith(#[allow(dead_code)] ArithError), + #[error("Failed to poll proposers: {0}")] + FailedToPollProposers(String), } pub struct DutiesTracker { @@ -192,7 +197,7 @@ impl DutiesTracker { }) .await; - match download_result { + let result = match download_result { Ok(response) => { // avoid holding the borrow across .await points let validator_indices = { @@ -217,13 +222,11 @@ impl DutiesTracker { .proposers .write() .insert(current_epoch, relevant_duties); + Ok(()) } // Don't return early here, we"ll try again later - Err(e) => warn!( - err = %e, - "Failed to download proposer duties" - ), - } + Err(e) => Err(Error::FailedToPollProposers(e.to_string())), + }; // Prune old duties. self.duties @@ -231,7 +234,7 @@ impl DutiesTracker { .write() .retain(|&epoch, _| epoch + HISTORICAL_DUTIES_EPOCHS >= current_epoch); - Ok(()) + result } pub fn start(self: Arc, executor: TaskExecutor) { From 31b1a7e3798b847817595faed0f18ec4dd264ff7 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 17:58:28 +0200 Subject: [PATCH 21/35] remove estimated_epoch_at_slot --- anchor/message_validator/src/beacon_network.rs | 10 ---------- anchor/message_validator/src/consensus_message.rs | 8 ++++---- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs index d7b6ddd99..2b19933dd 100644 --- a/anchor/message_validator/src/beacon_network.rs +++ b/anchor/message_validator/src/beacon_network.rs @@ -51,11 +51,6 @@ impl BeaconNetwork { Ok(UNIX_EPOCH + dur) } - /// Estimates the epoch at the given slot - pub fn estimated_epoch_at_slot(&self, slot: Slot) -> Epoch { - Epoch::new(slot.as_u64() / self.slots_per_epoch) - } - /// Returns the start time of the given slot pub fn get_slot_start_time(&self, slot: Slot) -> Result { self.estimated_time_at_slot(slot) @@ -66,11 +61,6 @@ impl BeaconNetwork { slot.as_u64() % self.slots_per_epoch == 0 } - /// Returns the number of epochs per sync committee period - pub fn epochs_per_sync_committee_period(&self) -> u64 { - self.epochs_per_sync_committee_period - } - /// Estimates the sync committee period at the given epoch pub fn estimated_sync_committee_period_at_epoch(&self, epoch: Epoch) -> u64 { epoch.as_u64() / self.epochs_per_sync_committee_period diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 2afa15952..f8430e1b1 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -439,8 +439,7 @@ pub(crate) fn validate_beacon_duty( duty_provider: Arc, ) -> Result<(), ValidationFailure> { let role = validation_context.role; - let epoch = beacon_network.estimated_epoch_at_slot(slot); - + let epoch = slot.epoch(beacon_network.slots_per_epoch()); // Rule: For a proposal duty message, check if the validator is assigned to it if role == Role::Proposer { // Tolerate missing duties for RANDAO signatures during the first slot of an epoch, @@ -566,7 +565,8 @@ pub(crate) fn validate_duty_count( if should_check { // Get current duty count for this signer - let duty_count = signer_state.get_duty_count(beacon_network.estimated_epoch_at_slot(slot)); + let epoch = slot.epoch(beacon_network.slots_per_epoch()); + let duty_count = signer_state.get_duty_count(epoch); if duty_count >= limit { return Err(ValidationFailure::ExcessiveDutyCount { @@ -600,7 +600,7 @@ fn duty_limit( // Skip duty search if validators * 2 exceeds slots per epoch if validator_index_count < slots_per_epoch_val / 2 { - let epoch = beacon_network.estimated_epoch_at_slot(slot); + let epoch = slot.epoch(beacon_network.slots_per_epoch()); let period = beacon_network.estimated_sync_committee_period_at_epoch(epoch); // Check if at least one validator is in the sync committee From d0e43d27d71adb928ea445f7f3329f133d5b2831 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 18:02:51 +0200 Subject: [PATCH 22/35] remove estimated_time_at_slot --- anchor/message_validator/src/beacon_network.rs | 9 ++------- anchor/message_validator/src/consensus_message.rs | 6 +++--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs index 2b19933dd..aa761cc62 100644 --- a/anchor/message_validator/src/beacon_network.rs +++ b/anchor/message_validator/src/beacon_network.rs @@ -42,8 +42,8 @@ impl BeaconNetwork { self.slots_per_epoch } - /// Estimates the time at the given slot - pub fn estimated_time_at_slot(&self, slot: Slot) -> Result { + /// Returns the start time of the given slot + pub fn slot_start_time(&self, slot: Slot) -> Result { let dur = self .slot_clock .start_of(slot) @@ -51,11 +51,6 @@ impl BeaconNetwork { Ok(UNIX_EPOCH + dur) } - /// Returns the start time of the given slot - pub fn get_slot_start_time(&self, slot: Slot) -> Result { - self.estimated_time_at_slot(slot) - } - /// Checks if the given slot is the first slot of its epoch pub fn is_first_slot_of_epoch(&self, slot: Slot) -> bool { slot.as_u64() % self.slots_per_epoch == 0 diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index f8430e1b1..619b8a71e 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -305,7 +305,7 @@ fn validate_round_in_allowed_spread( // Get the slot let slot = Slot::new(consensus_message.height); let slot_start_time = beacon_network - .get_slot_start_time(slot) + .slot_start_time(slot) .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })?; let (since_slot_start, estimated_round) = if received_at > slot_start_time { @@ -513,7 +513,7 @@ fn message_earliness( beacon_network: &BeaconNetwork, ) -> Result { let slot_start = beacon_network - .get_slot_start_time(slot) + .slot_start_time(slot) .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })?; Ok(slot_start.duration_since(received_at).unwrap_or_default()) } @@ -534,7 +534,7 @@ fn message_lateness( }; let deadline = beacon_network - .get_slot_start_time(slot + ttl) + .slot_start_time(slot + ttl) .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })? .checked_add(LATE_MESSAGE_MARGIN) .unwrap_or_else(|| { From f40dc52cf2542a024149f5b73bcbbe237871004c Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 18:07:31 +0200 Subject: [PATCH 23/35] remove is_first_slot_of_epoch --- anchor/message_validator/src/beacon_network.rs | 5 ----- anchor/message_validator/src/consensus_message.rs | 5 ++++- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs index aa761cc62..b2391b430 100644 --- a/anchor/message_validator/src/beacon_network.rs +++ b/anchor/message_validator/src/beacon_network.rs @@ -51,11 +51,6 @@ impl BeaconNetwork { Ok(UNIX_EPOCH + dur) } - /// Checks if the given slot is the first slot of its epoch - pub fn is_first_slot_of_epoch(&self, slot: Slot) -> bool { - slot.as_u64() % self.slots_per_epoch == 0 - } - /// Estimates the sync committee period at the given epoch pub fn estimated_sync_committee_period_at_epoch(&self, epoch: Epoch) -> u64 { epoch.as_u64() / self.epochs_per_sync_committee_period diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 619b8a71e..6e5b0b536 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -444,8 +444,11 @@ pub(crate) fn validate_beacon_duty( if role == Role::Proposer { // Tolerate missing duties for RANDAO signatures during the first slot of an epoch, // while duties are still being fetched from the Beacon node. + + let is_first_slot_of_epoch = epoch.start_slot(beacon_network.slots_per_epoch()) == slot; + if randao_msg - && beacon_network.is_first_slot_of_epoch(slot) + && is_first_slot_of_epoch && beacon_network.slot_clock().now().unwrap_or_default() <= slot && !duty_provider.is_epoch_known_for_proposers(epoch) { From 439d3b955a8a428da0d47613ee861a267f5ea69d Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 19:35:50 +0200 Subject: [PATCH 24/35] remove BeaconNetwork --- anchor/client/src/lib.rs | 10 +- .../message_validator/src/beacon_network.rs | 58 ------ .../src/consensus_message.rs | 176 +++++++++--------- anchor/message_validator/src/lib.rs | 72 ++++--- .../src/partial_signature.rs | 56 +++++- 5 files changed, 191 insertions(+), 181 deletions(-) delete mode 100644 anchor/message_validator/src/beacon_network.rs diff --git a/anchor/client/src/lib.rs b/anchor/client/src/lib.rs index 74a3dae4f..b2d4b3282 100644 --- a/anchor/client/src/lib.rs +++ b/anchor/client/src/lib.rs @@ -28,7 +28,7 @@ use eth2::{ use keygen::{encryption::decrypt, run_keygen, Keygen}; use message_receiver::NetworkMessageReceiver; use message_sender::NetworkMessageSender; -use message_validator::{BeaconNetwork, DutiesTracker, Validator}; +use message_validator::{DutiesTracker, Validator}; use network::Network; use openssl::{pkey::Private, rsa::Rsa}; use parking_lot::RwLock; @@ -383,12 +383,10 @@ impl Client { let message_validator = Arc::new(Validator::new( database.watch(), - BeaconNetwork::new( - slot_clock.clone(), - E::slots_per_epoch(), - spec.epochs_per_sync_committee_period.as_u64(), - ), + E::slots_per_epoch(), + spec.epochs_per_sync_committee_period.as_u64(), duties_tracker.clone(), + slot_clock.clone(), )); let network_message_sender = NetworkMessageSender::new( diff --git a/anchor/message_validator/src/beacon_network.rs b/anchor/message_validator/src/beacon_network.rs deleted file mode 100644 index b2391b430..000000000 --- a/anchor/message_validator/src/beacon_network.rs +++ /dev/null @@ -1,58 +0,0 @@ -use std::time::{Duration, SystemTime, UNIX_EPOCH}; - -use slot_clock::SlotClock; -use types::{Epoch, Slot}; - -#[derive(thiserror::Error, Debug)] -pub enum TimeError { - #[error("clock start-of-slot overflow for slot {0}")] - Overflow(Slot), -} - -/// Wrapper around SlotClock to provide beacon chain network functionality -#[derive(Clone)] -pub struct BeaconNetwork { - slot_clock: S, - slots_per_epoch: u64, - epochs_per_sync_committee_period: u64, -} - -impl BeaconNetwork { - /// Create a new BeaconNetwork - pub fn new(slot_clock: S, slots_per_epoch: u64, epochs_per_sync_committee_period: u64) -> Self { - Self { - slot_clock, - slots_per_epoch, - epochs_per_sync_committee_period, - } - } - - /// Returns the slot clock - pub fn slot_clock(&self) -> &S { - &self.slot_clock - } - - /// Returns the slot duration - pub fn slot_duration(&self) -> Duration { - self.slot_clock.slot_duration() - } - - /// Returns the number of slots per epoch - pub fn slots_per_epoch(&self) -> u64 { - self.slots_per_epoch - } - - /// Returns the start time of the given slot - pub fn slot_start_time(&self, slot: Slot) -> Result { - let dur = self - .slot_clock - .start_of(slot) - .ok_or(TimeError::Overflow(slot))?; - Ok(UNIX_EPOCH + dur) - } - - /// Estimates the sync committee period at the given epoch - pub fn estimated_sync_committee_period_at_epoch(&self, epoch: Epoch) -> u64 { - epoch.as_u64() / self.epochs_per_sync_committee_period - } -} diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 6e5b0b536..a98d86127 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -15,18 +15,16 @@ use ssz::Decode; use ValidationFailure::EarlySlotMessage; use crate::{ - beacon_network::BeaconNetwork, compute_quorum_size, consensus_state::{ConsensusState, OperatorState}, duties::DutiesProvider, - hash_data, verify_message_signatures, ValidatedSSVMessage, ValidationContext, - ValidationFailure, + hash_data, slot_start_time, sync_committee_period, verify_message_signatures, + ValidatedSSVMessage, ValidationContext, ValidationFailure, }; pub(crate) fn validate_consensus_message( - validation_context: ValidationContext, + validation_context: ValidationContext, consensus_state: &mut ConsensusState, - beacon_network: &BeaconNetwork, duty_provider: Arc, ) -> Result { // Decode message to QbftMessage @@ -44,18 +42,12 @@ pub(crate) fn validate_consensus_message( validation_context.committee_info, )?; - validate_qbft_logic( - &validation_context, - &consensus_message, - consensus_state, - beacon_network, - )?; + validate_qbft_logic(&validation_context, &consensus_message, consensus_state)?; validate_qbft_message_by_duty_logic( &validation_context, &consensus_message, consensus_state, - beacon_network, duty_provider, )?; @@ -183,10 +175,9 @@ pub(crate) fn validate_justifications( #[allow(clippy::comparison_chain)] pub(crate) fn validate_qbft_logic( - validation_context: &ValidationContext, + validation_context: &ValidationContext, consensus_message: &QbftMessage, consensus_state: &mut ConsensusState, - beacon_network: &BeaconNetwork, ) -> Result<(), ValidationFailure> { let signed_ssv_message = validation_context.signed_ssv_message; @@ -261,12 +252,7 @@ pub(crate) fn validate_qbft_logic( // Rule: Round must be within allowed spread from current time if signers.len() == 1 { - validate_round_in_allowed_spread( - consensus_message, - validation_context.role, - validation_context.received_at, - beacon_network, - )?; + validate_round_in_allowed_spread(consensus_message, validation_context)?; } Ok(()) @@ -298,18 +284,16 @@ fn round_robin_proposer( /// Validate that the message round is within the allowed spread fn validate_round_in_allowed_spread( consensus_message: &QbftMessage, - role: Role, - received_at: SystemTime, - beacon_network: &BeaconNetwork, + validation_context: &ValidationContext, ) -> Result<(), ValidationFailure> { // Get the slot let slot = Slot::new(consensus_message.height); - let slot_start_time = beacon_network - .slot_start_time(slot) + let slot_start_time = slot_start_time(slot, validation_context.slot_clock.clone()) .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })?; - let (since_slot_start, estimated_round) = if received_at > slot_start_time { - let duration = received_at + let (since_slot_start, estimated_round) = if validation_context.received_at > slot_start_time { + let duration = validation_context + .received_at .duration_since(slot_start_time) .unwrap_or_default(); (duration, current_estimated_round(duration)) @@ -324,10 +308,13 @@ fn validate_round_in_allowed_spread( if consensus_message.round < lowest_allowed || consensus_message.round > highest_allowed.into() { return Err(ValidationFailure::EstimatedRoundNotInAllowedSpread { - got: format!("{} ({} role)", consensus_message.round, role), + got: format!( + "{} ({} role)", + consensus_message.round, validation_context.role + ), want: format!( "between {} and {} ({} role) / {:?} passed", - lowest_allowed, highest_allowed, role, since_slot_start + lowest_allowed, highest_allowed, validation_context.role, since_slot_start ), }); } @@ -375,10 +362,9 @@ const LATE_SLOT_ALLOWANCE: u64 = 2; /// Validates QBFT messages based on beacon chain duties pub(crate) fn validate_qbft_message_by_duty_logic( - validation_context: &ValidationContext, + validation_context: &ValidationContext, consensus_message: &QbftMessage, consensus_state: &mut ConsensusState, - beacon_network: &BeaconNetwork, duty_provider: Arc, ) -> Result<(), ValidationFailure> { let role = validation_context.role; @@ -406,14 +392,13 @@ pub(crate) fn validate_qbft_message_by_duty_logic( validation_context, msg_slot, randao_msg, - beacon_network, duty_provider.clone(), )?; // Rule: current slot(height) must be between duty's starting slot and: // - duty's starting slot + 34 (committee and aggregation) // - duty's starting slot + 3 (other types) - validate_slot_time(msg_slot, validation_context, beacon_network)?; + validate_slot_time(msg_slot, validation_context)?; // Rule: valid number of duties per epoch for &signer in signed_ssv_message.operator_ids() { @@ -422,7 +407,6 @@ pub(crate) fn validate_qbft_message_by_duty_logic( validation_context, msg_slot, signer_state, - beacon_network, duty_provider.clone(), )?; } @@ -432,24 +416,23 @@ pub(crate) fn validate_qbft_message_by_duty_logic( /// Validates if a validator is assigned to a specific duty pub(crate) fn validate_beacon_duty( - validation_context: &ValidationContext, + validation_context: &ValidationContext, slot: Slot, randao_msg: bool, - beacon_network: &BeaconNetwork, duty_provider: Arc, ) -> Result<(), ValidationFailure> { let role = validation_context.role; - let epoch = slot.epoch(beacon_network.slots_per_epoch()); + let epoch = slot.epoch(validation_context.slots_per_epoch); // Rule: For a proposal duty message, check if the validator is assigned to it if role == Role::Proposer { // Tolerate missing duties for RANDAO signatures during the first slot of an epoch, // while duties are still being fetched from the Beacon node. - let is_first_slot_of_epoch = epoch.start_slot(beacon_network.slots_per_epoch()) == slot; + let is_first_slot_of_epoch = epoch.start_slot(validation_context.slots_per_epoch) == slot; if randao_msg && is_first_slot_of_epoch - && beacon_network.slot_clock().now().unwrap_or_default() <= slot + && validation_context.slot_clock.now().unwrap_or_default() <= slot && !duty_provider.is_epoch_known_for_proposers(epoch) { return Ok(()); @@ -469,7 +452,8 @@ pub(crate) fn validate_beacon_duty( // Rule: For a sync committee duty message, check if the validator is assigned if role == Role::SyncCommittee { - let period = beacon_network.estimated_sync_committee_period_at_epoch(epoch); + let period = + sync_committee_period(epoch, validation_context.epochs_per_sync_committee_period)?; let validator_index = validation_context .committee_info .validator_indices @@ -487,11 +471,10 @@ pub(crate) fn validate_beacon_duty( /// Validates that the message's slot timing is correct pub(crate) fn validate_slot_time( msg_slot: Slot, - validation_context: &ValidationContext, - beacon_network: &BeaconNetwork, + validation_context: &ValidationContext, ) -> Result<(), ValidationFailure> { // Check if the message is too early - let earliness = message_earliness(msg_slot, validation_context.received_at, beacon_network)?; + let earliness = message_earliness(msg_slot, validation_context)?; if earliness > CLOCK_ERROR_TOLERANCE { return Err(EarlySlotMessage { got: format!("early by {:?}", earliness), @@ -499,7 +482,7 @@ pub(crate) fn validate_slot_time( } // Check if the message is too late - let lateness = message_lateness(msg_slot, validation_context, beacon_network)?; + let lateness = message_lateness(msg_slot, validation_context)?; if lateness > CLOCK_ERROR_TOLERANCE { return Err(ValidationFailure::LateSlotMessage { got: format!("late by {:?}", lateness), @@ -512,20 +495,19 @@ pub(crate) fn validate_slot_time( /// Returns how early a message is compared to its slot start time fn message_earliness( slot: Slot, - received_at: SystemTime, - beacon_network: &BeaconNetwork, + validation_context: &ValidationContext, ) -> Result { - let slot_start = beacon_network - .slot_start_time(slot) + let slot_start = slot_start_time(slot, validation_context.slot_clock.clone()) .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })?; - Ok(slot_start.duration_since(received_at).unwrap_or_default()) + Ok(slot_start + .duration_since(validation_context.received_at) + .unwrap_or_default()) } /// Returns how late a message is compared to its deadline based on role fn message_lateness( slot: Slot, - validation_context: &ValidationContext, - beacon_network: &BeaconNetwork, + validation_context: &ValidationContext, ) -> Result { let ttl = match validation_context.role { Role::Proposer | Role::SyncCommittee => 1 + LATE_SLOT_ALLOWANCE, @@ -536,8 +518,7 @@ fn message_lateness( Role::ValidatorRegistration | Role::VoluntaryExit => return Ok(Duration::from_secs(0)), }; - let deadline = beacon_network - .slot_start_time(slot + ttl) + let deadline = slot_start_time(slot + ttl, validation_context.slot_clock.clone()) .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })? .checked_add(LATE_MESSAGE_MARGIN) .unwrap_or_else(|| { @@ -552,23 +533,21 @@ fn message_lateness( /// Validates the duty count for a specific message and operator pub(crate) fn validate_duty_count( - validation_context: &ValidationContext, + validation_context: &ValidationContext, slot: Slot, signer_state: &mut OperatorState, - beacon_network: &BeaconNetwork, duty_provider: Arc, ) -> Result<(), ValidationFailure> { let (limit, should_check) = duty_limit( - validation_context.role, + validation_context, slot, &validation_context.committee_info.validator_indices, - beacon_network, duty_provider, - ); + )?; if should_check { // Get current duty count for this signer - let epoch = slot.epoch(beacon_network.slots_per_epoch()); + let epoch = slot.epoch(validation_context.slots_per_epoch); let duty_count = signer_state.get_duty_count(epoch); if duty_count >= limit { @@ -584,42 +563,43 @@ pub(crate) fn validate_duty_count( /// Determines duty limit based on role and validator indices fn duty_limit( - role: Role, + validation_context: &ValidationContext, slot: Slot, validator_indices: &[ValidatorIndex], - beacon_network: &BeaconNetwork, duty_provider: Arc, -) -> (u64, bool) { - match role { +) -> Result<(u64, bool), ValidationFailure> { + match validation_context.role { Role::VoluntaryExit => { // TODO For voluntary exit, check the stored duties // This would need to be adapted to use the actual duty store - (2, true) + Ok((2, true)) } - Role::Aggregator | Role::ValidatorRegistration => (2, true), + Role::Aggregator | Role::ValidatorRegistration => Ok((2, true)), Role::Committee => { let validator_index_count = validator_indices.len() as u64; - let slots_per_epoch_val = beacon_network.slots_per_epoch(); + let slots_per_epoch_val = validation_context.slots_per_epoch; // Skip duty search if validators * 2 exceeds slots per epoch if validator_index_count < slots_per_epoch_val / 2 { - let epoch = slot.epoch(beacon_network.slots_per_epoch()); - let period = beacon_network.estimated_sync_committee_period_at_epoch(epoch); + let epoch = slot.epoch(validation_context.slots_per_epoch); + let period = sync_committee_period( + epoch, + validation_context.epochs_per_sync_committee_period, + )?; // Check if at least one validator is in the sync committee for &index in validator_indices { if duty_provider.is_validator_in_sync_committee(period, index) { - return (slots_per_epoch_val, true); + return Ok((slots_per_epoch_val, true)); } } } - - ( + Ok(( std::cmp::min(slots_per_epoch_val, 2 * validator_index_count), true, - ) + )) } - _ => (0, false), + _ => Ok((0, false)), } } @@ -629,7 +609,6 @@ mod tests { use bls::{Hash256, PublicKeyBytes}; use openssl::hash::MessageDigest; - use slot_clock::ManualSlotClock; use ssv_types::{ consensus::{QbftMessage, QbftMessageType}, domain_type::DomainType, @@ -842,20 +821,26 @@ mod tests { received_at: SystemTime::now(), operators_pk: &[public_key], slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_ssv_message( validation_context, &mut ConsensusState::new(2), - &BeaconNetwork::new( - ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - 32, - 256, - ), + // &BeaconNetwork::new( + // ManualSlotClock::new( + // Slot::new(0), + // SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + // Duration::from_secs(1), + // ), + // 32, + // 256, + // ), Arc::new(MockDutiesProvider {}), ); @@ -897,20 +882,26 @@ mod tests { received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_ssv_message( validation_context, &mut ConsensusState::new(2), - &BeaconNetwork::new( - ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - 32, - 256, - ), + // &BeaconNetwork::new( + // ManualSlotClock::new( + // Slot::new(0), + // SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + // Duration::from_secs(1), + // ), + // 32, + // 256, + // ), Arc::new(MockDutiesProvider {}), ); @@ -1345,6 +1336,7 @@ mod tests { rsa::Rsa, sign::Signer, }; + use slot_clock::ManualSlotClock; use types::Epoch; #[test] diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index 547dc1820..b4bff1c1d 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -1,13 +1,14 @@ -mod beacon_network; mod consensus_message; mod consensus_state; mod duties; mod message_counts; mod partial_signature; -use std::{sync::Arc, time::SystemTime}; +use std::{ + sync::Arc, + time::{SystemTime, UNIX_EPOCH}, +}; -pub use beacon_network::BeaconNetwork; use dashmap::{mapref::one::RefMut, DashMap}; use database::NetworkState; use gossipsub::MessageAcceptance; @@ -17,6 +18,7 @@ use openssl::{ rsa::Rsa, sign::Verifier, }; +use safe_arith::SafeArith; use sha2::{Digest, Sha256}; use slot_clock::SlotClock; use ssv_types::{ @@ -29,7 +31,7 @@ use ssv_types::{ use ssz::{Decode, Encode}; use tokio::sync::watch::Receiver; use tracing::{error, trace}; -use types::Slot; +use types::{Epoch, Slot}; pub use crate::duties::{duties_tracker::DutiesTracker, DutiesProvider}; use crate::{ @@ -145,6 +147,7 @@ pub enum ValidationFailure { got: u64, limit: u64, }, + SyncCommitteePeriodCalculationFailure, } impl From<&ValidationFailure> for MessageAcceptance { @@ -202,33 +205,41 @@ pub enum Error { Processor(#[from] ::processor::Error), } -struct ValidationContext<'a> { +struct ValidationContext<'a, S> { pub signed_ssv_message: &'a SignedSSVMessage, pub role: Role, // Small value type can remain owned pub committee_info: &'a CommitteeInfo, pub received_at: SystemTime, // Small value type pub operators_pk: &'a [Rsa], pub slots_per_epoch: u64, + pub epochs_per_sync_committee_period: u64, + pub slot_clock: S, } pub struct Validator { network_state_rx: Receiver, consensus_state_map: DashMap, - beacon_network: BeaconNetwork, + slots_per_epoch: u64, + epochs_per_sync_committee_period: u64, duties_provider: Arc, + slot_clock: S, } impl Validator { pub fn new( network_state_rx: Receiver, - beacon_network: BeaconNetwork, + slots_per_epoch: u64, + epochs_per_sync_committee_period: u64, duties_provider: Arc, + slot_clock: S, ) -> Self { Self { network_state_rx, consensus_state_map: DashMap::new(), - beacon_network, + slots_per_epoch, + epochs_per_sync_committee_period, duties_provider, + slot_clock, } } @@ -270,10 +281,8 @@ impl Validator { let operators_pks = self.get_operator_pks(signed_ssv_message.operator_ids())?; - let mut consensus_state = self.get_consensus_state( - ssv_message.msg_id(), - self.beacon_network.slots_per_epoch(), - ); + let mut consensus_state = + self.get_consensus_state(ssv_message.msg_id(), self.slots_per_epoch); let validation_context = ValidationContext { signed_ssv_message: &signed_ssv_message, @@ -281,13 +290,14 @@ impl Validator { committee_info: &committee_info, received_at: SystemTime::now(), operators_pk: &operators_pks, - slots_per_epoch: self.beacon_network.slots_per_epoch(), + slots_per_epoch: self.slots_per_epoch, + epochs_per_sync_committee_period: self.epochs_per_sync_committee_period, + slot_clock: self.slot_clock.clone(), }; validate_ssv_message( validation_context, consensus_state.value_mut(), - &self.beacon_network, self.duties_provider.clone(), ) .map(|validated| ValidatedMessage::new(signed_ssv_message.clone(), validated)) @@ -333,20 +343,16 @@ impl Validator { } fn validate_ssv_message( - validation_context: ValidationContext, + validation_context: ValidationContext, consensus_state: &mut ConsensusState, - beacon_network: &BeaconNetwork, duty_provider: Arc, ) -> Result { let ssv_message = validation_context.signed_ssv_message.ssv_message(); match ssv_message.msg_type() { - MsgType::SSVConsensusMsgType => validate_consensus_message( - validation_context, - consensus_state, - beacon_network, - duty_provider, - ), + MsgType::SSVConsensusMsgType => { + validate_consensus_message(validation_context, consensus_state, duty_provider) + } MsgType::SSVPartialSignatureMsgType => { validate_partial_signature_message(validation_context) } @@ -408,6 +414,28 @@ fn verify_message_signatures( Ok(()) } +#[derive(thiserror::Error, Debug)] +pub enum TimeError { + #[error("clock start-of-slot overflow for slot {0}")] + Overflow(Slot), +} + +pub fn slot_start_time(slot: Slot, slot_clock: impl SlotClock) -> Result { + let dur = slot_clock.start_of(slot).ok_or(TimeError::Overflow(slot))?; + Ok(UNIX_EPOCH + dur) +} + +/// Compute the sync committee period for an epoch. +pub fn sync_committee_period( + epoch: Epoch, + epochs_per_sync_committee_period: u64, +) -> Result { + Ok(epoch + .safe_div(epochs_per_sync_committee_period) + .map_err(|_| ValidationFailure::SyncCommitteePeriodCalculationFailure)? + .as_u64()) +} + pub(crate) fn compute_quorum_size(committee_size: usize) -> usize { let f = get_f(committee_size); f * 2 + 1 diff --git a/anchor/message_validator/src/partial_signature.rs b/anchor/message_validator/src/partial_signature.rs index 983b78ebe..0ef58de06 100644 --- a/anchor/message_validator/src/partial_signature.rs +++ b/anchor/message_validator/src/partial_signature.rs @@ -1,3 +1,4 @@ +use slot_clock::SlotClock; use ssv_types::{ msgid::Role, partial_sig::{PartialSignatureKind, PartialSignatureMessages}, @@ -7,7 +8,7 @@ use ssz::Decode; use crate::{verify_message_signature, ValidatedSSVMessage, ValidationContext, ValidationFailure}; pub(crate) fn validate_partial_signature_message( - validation_context: ValidationContext, + validation_context: ValidationContext, ) -> Result { // Decode message directly to PartialSignatureMessages let messages = match PartialSignatureMessages::from_ssz_bytes( @@ -43,7 +44,7 @@ pub(crate) fn validate_partial_signature_message( } fn validate_partial_signature_message_semantics( - validation_context: &ValidationContext, + validation_context: &ValidationContext, partial_signature_messages: &PartialSignatureMessages, ) -> Result<(), ValidationFailure> { // Rule: Partial Signature message must have 1 signer @@ -120,7 +121,7 @@ fn partial_signature_type_matches_role(kind: PartialSignatureKind, role: Role) - #[cfg(test)] mod tests { - use std::time::SystemTime; + use std::time::{Duration, SystemTime, UNIX_EPOCH}; use bls::{Hash256, Signature}; use openssl::{ @@ -129,6 +130,7 @@ mod tests { rsa::Rsa, sign::Signer, }; + use slot_clock::{ManualSlotClock, SlotClock}; use ssv_types::{ message::{MsgType, SSVMessage, SignedSSVMessage, RSA_SIGNATURE_SIZE}, partial_sig::PartialSignatureMessage, @@ -235,6 +237,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); @@ -281,6 +289,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); @@ -314,6 +328,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); @@ -347,6 +367,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); @@ -380,6 +406,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); @@ -411,6 +443,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &[public_key], slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); @@ -454,6 +492,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); @@ -492,6 +536,12 @@ mod tests { received_at: SystemTime::now(), operators_pk: &[public_key], slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), }; let result = validate_partial_signature_message(validation_context); From 91db909c856585d6db4e8de6498dfd3f05623b92 Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 17 Apr 2025 20:07:07 +0200 Subject: [PATCH 25/35] add issue to todo --- anchor/message_validator/src/consensus_message.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index a98d86127..5d463052a 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -570,7 +570,7 @@ fn duty_limit( ) -> Result<(u64, bool), ValidationFailure> { match validation_context.role { Role::VoluntaryExit => { - // TODO For voluntary exit, check the stored duties + // TODO For voluntary exit, check the stored duties https://github.com/sigp/anchor/issues/277 // This would need to be adapted to use the actual duty store Ok((2, true)) } From de8938df6cca0f7c66a3cd062a4183c7178d85e8 Mon Sep 17 00:00:00 2001 From: diego Date: Fri, 18 Apr 2025 12:05:44 +0200 Subject: [PATCH 26/35] propagate error in message_lateness --- anchor/message_validator/src/consensus_message.rs | 14 +++++--------- anchor/message_validator/src/lib.rs | 3 +++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 5d463052a..bba298bc8 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -1,8 +1,4 @@ -use std::{ - convert::Into, - sync::Arc, - time::{Duration, SystemTime}, -}; +use std::{convert::Into, sync::Arc, time::Duration}; use slot_clock::SlotClock; use ssv_types::{ @@ -521,9 +517,9 @@ fn message_lateness( let deadline = slot_start_time(slot + ttl, validation_context.slot_clock.clone()) .map_err(|_| ValidationFailure::SlotStartTimeNotFound { slot })? .checked_add(LATE_MESSAGE_MARGIN) - .unwrap_or_else(|| { - SystemTime::now() // Fallback if overflow occurs - }); + .ok_or(ValidationFailure::UnexpectedFailure { + msg: "Unexpected overflow calculating message deadline".to_string(), + })?; Ok(validation_context .received_at @@ -605,7 +601,7 @@ fn duty_limit( #[cfg(test)] mod tests { - use std::time::UNIX_EPOCH; + use std::time::{SystemTime, UNIX_EPOCH}; use bls::{Hash256, PublicKeyBytes}; use openssl::hash::MessageDigest; diff --git a/anchor/message_validator/src/lib.rs b/anchor/message_validator/src/lib.rs index 6849ec1a0..21ce30a61 100644 --- a/anchor/message_validator/src/lib.rs +++ b/anchor/message_validator/src/lib.rs @@ -148,6 +148,9 @@ pub enum ValidationFailure { limit: u64, }, SyncCommitteePeriodCalculationFailure, + UnexpectedFailure { + msg: String, + }, } impl From<&ValidationFailure> for MessageAcceptance { From 7f747429cd75101f635b52c0b92f715dfafcb38e Mon Sep 17 00:00:00 2001 From: diego Date: Fri, 18 Apr 2025 12:26:47 +0200 Subject: [PATCH 27/35] use into() for ValidatorIndex --- anchor/database/src/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/anchor/database/src/state.rs b/anchor/database/src/state.rs index 25f0e5ad2..1d03a635b 100644 --- a/anchor/database/src/state.rs +++ b/anchor/database/src/state.rs @@ -373,7 +373,7 @@ impl NetworkState { self.multi_state .validator_metadata .values() - .filter_map(|metadata| metadata.index.map(|idx| idx.0 as u64)) + .filter_map(|metadata| metadata.index.map(|idx| idx.into())) .collect() } } From 5f367fa7099c210fc017f28661d124c678409504 Mon Sep 17 00:00:00 2001 From: diego Date: Fri, 18 Apr 2025 12:29:34 +0200 Subject: [PATCH 28/35] remove comment --- .../message_validator/src/consensus_message.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index bba298bc8..6f7bb443b 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -828,15 +828,6 @@ mod tests { let result = validate_ssv_message( validation_context, &mut ConsensusState::new(2), - // &BeaconNetwork::new( - // ManualSlotClock::new( - // Slot::new(0), - // SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - // Duration::from_secs(1), - // ), - // 32, - // 256, - // ), Arc::new(MockDutiesProvider {}), ); @@ -889,15 +880,6 @@ mod tests { let result = validate_ssv_message( validation_context, &mut ConsensusState::new(2), - // &BeaconNetwork::new( - // ManualSlotClock::new( - // Slot::new(0), - // SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - // Duration::from_secs(1), - // ), - // 32, - // 256, - // ), Arc::new(MockDutiesProvider {}), ); From db85d16cf750afa0aa3f6957245d78120e18b2be Mon Sep 17 00:00:00 2001 From: diego Date: Fri, 18 Apr 2025 13:25:37 +0200 Subject: [PATCH 29/35] decrease duplication in partial_signature tests --- .../src/partial_signature.rs | 167 ++++++------------ 1 file changed, 55 insertions(+), 112 deletions(-) diff --git a/anchor/message_validator/src/partial_signature.rs b/anchor/message_validator/src/partial_signature.rs index 0ef58de06..0cb2bf98a 100644 --- a/anchor/message_validator/src/partial_signature.rs +++ b/anchor/message_validator/src/partial_signature.rs @@ -218,6 +218,29 @@ mod tests { (private_key, public_key) } + // Helper function to create a ValidationContext for testing + fn create_test_validation_context<'a>( + signed_msg: &'a SignedSSVMessage, + committee_info: &'a crate::CommitteeInfo, + role: Role, + operators_pk: &'a [Rsa], + ) -> ValidationContext<'a, ManualSlotClock> { + ValidationContext { + signed_ssv_message: signed_msg, + committee_info, + role, + received_at: SystemTime::now(), + operators_pk, + slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock: ManualSlotClock::new( + Slot::new(0), + SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ), + } + } + #[test] fn test_partial_signature_message_with_invalid_type_for_role() { let committee_info = create_committee_info(FOUR_NODE_COMMITTEE); @@ -230,20 +253,9 @@ mod tests { None, ); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Committee, - received_at: SystemTime::now(), - operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = generate_random_rsa_public_keys(signed_msg.operator_ids().len()); + let validation_context = + create_test_validation_context(&signed_msg, &committee_info, Role::Committee, &binding); let result = validate_partial_signature_message(validation_context); @@ -282,20 +294,9 @@ mod tests { let signed_msg = SignedSSVMessage::new(signatures, signers, ssv_msg, vec![]) .expect("SignedSSVMessage should be created"); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Proposer, - received_at: SystemTime::now(), - operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = generate_random_rsa_public_keys(signed_msg.operator_ids().len()); + let validation_context = + create_test_validation_context(&signed_msg, &committee_info, Role::Proposer, &binding); let result = validate_partial_signature_message(validation_context); @@ -321,20 +322,9 @@ mod tests { None, ); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Proposer, - received_at: SystemTime::now(), - operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = generate_random_rsa_public_keys(signed_msg.operator_ids().len()); + let validation_context = + create_test_validation_context(&signed_msg, &committee_info, Role::Proposer, &binding); let result = validate_partial_signature_message(validation_context); @@ -360,20 +350,9 @@ mod tests { None, ); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Proposer, - received_at: SystemTime::now(), - operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = generate_random_rsa_public_keys(signed_msg.operator_ids().len()); + let validation_context = + create_test_validation_context(&signed_msg, &committee_info, Role::Proposer, &binding); let result = validate_partial_signature_message(validation_context); @@ -399,20 +378,9 @@ mod tests { None, ); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Proposer, - received_at: SystemTime::now(), - operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = generate_random_rsa_public_keys(signed_msg.operator_ids().len()); + let validation_context = + create_test_validation_context(&signed_msg, &committee_info, Role::Proposer, &binding); let result = validate_partial_signature_message(validation_context); @@ -436,20 +404,9 @@ mod tests { Some(private_key), ); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Proposer, - received_at: SystemTime::now(), - operators_pk: &[public_key], - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = [public_key]; + let validation_context = + create_test_validation_context(&signed_msg, &committee_info, Role::Proposer, &binding); let result = validate_partial_signature_message(validation_context); @@ -485,20 +442,13 @@ mod tests { None, ); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Proposer, // Not a committee role, so validator index is checked - received_at: SystemTime::now(), - operators_pk: &generate_random_rsa_public_keys(signed_msg.operator_ids().len()), - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = generate_random_rsa_public_keys(signed_msg.operator_ids().len()); + let validation_context = create_test_validation_context( + &signed_msg, + &committee_info, + Role::Proposer, // Not a committee role, so validator index is checked + &binding, + ); let result = validate_partial_signature_message(validation_context); @@ -529,20 +479,13 @@ mod tests { Some(private_key), ); - let validation_context = ValidationContext { - signed_ssv_message: &signed_msg, - committee_info: &committee_info, - role: Role::Committee, // Committee role, so validator index is not checked - received_at: SystemTime::now(), - operators_pk: &[public_key], - slots_per_epoch: 32, - epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), - }; + let binding = [public_key]; + let validation_context = create_test_validation_context( + &signed_msg, + &committee_info, + Role::Committee, // Committee role, so validator index is not checked + &binding, + ); let result = validate_partial_signature_message(validation_context); From 8e7c892f5953c499b61fcbef01f5c84bf01292b3 Mon Sep 17 00:00:00 2001 From: diego Date: Fri, 18 Apr 2025 19:24:00 +0200 Subject: [PATCH 30/35] fix constants values --- anchor/message_validator/src/consensus_message.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 6f7bb443b..b3719ba78 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -351,9 +351,10 @@ fn current_estimated_round(since_slot_start: Duration) -> Round { (QUICK_TIMEOUT_THRESHOLD + FIRST_ROUND + delta_slow).into() } -// Constants needed for time validation -const CLOCK_ERROR_TOLERANCE: Duration = Duration::from_secs(20); -const LATE_MESSAGE_MARGIN: Duration = Duration::from_secs(1); +/// clockErrorTolerance is the maximum amount of clock error we expect to see between nodes. +const CLOCK_ERROR_TOLERANCE: Duration = Duration::from_millis(50); +/// lateMessageMargin is the duration past a message's TTL in which it is still considered valid. +const LATE_MESSAGE_MARGIN: Duration = Duration::from_secs(3); const LATE_SLOT_ALLOWANCE: u64 = 2; /// Validates QBFT messages based on beacon chain duties @@ -368,7 +369,6 @@ pub(crate) fn validate_qbft_message_by_duty_logic( // Rule: Height must not be "old". I.e., signer must not have already advanced to a later slot. if role != Role::Committee { - // Rule only for validator runners for &signer in signed_ssv_message.operator_ids() { let signer_state = consensus_state.get_or_create_operator(&signer); let max_slot = signer_state.max_slot(); @@ -488,7 +488,8 @@ pub(crate) fn validate_slot_time( Ok(()) } -/// Returns how early a message is compared to its slot start time +/// Returns how early a message is compared to its slot start time. +/// Returns a zero duration if the message is on time or late. fn message_earliness( slot: Slot, validation_context: &ValidationContext, @@ -500,7 +501,9 @@ fn message_earliness( .unwrap_or_default()) } -/// Returns how late a message is compared to its deadline based on role +/// Returns how late a message is compared to its deadline based on role. +/// If the message was received before the deadline, it returns 0. +/// If the message was received after the deadline, it returns the duration by which it was late. fn message_lateness( slot: Slot, validation_context: &ValidationContext, From ee4cc612c743e9f7d0d96c5c4c5e1bb4dc1a372d Mon Sep 17 00:00:00 2001 From: diego Date: Tue, 22 Apr 2025 17:33:24 +0200 Subject: [PATCH 31/35] fix test and refactor duty_limit --- .../src/consensus_message.rs | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index b3719ba78..c6330ab23 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -537,14 +537,12 @@ pub(crate) fn validate_duty_count( signer_state: &mut OperatorState, duty_provider: Arc, ) -> Result<(), ValidationFailure> { - let (limit, should_check) = duty_limit( + if let Some(limit) = duty_limit( validation_context, slot, &validation_context.committee_info.validator_indices, duty_provider, - )?; - - if should_check { + )? { // Get current duty count for this signer let epoch = slot.epoch(validation_context.slots_per_epoch); let duty_count = signer_state.get_duty_count(epoch); @@ -566,14 +564,14 @@ fn duty_limit( slot: Slot, validator_indices: &[ValidatorIndex], duty_provider: Arc, -) -> Result<(u64, bool), ValidationFailure> { +) -> Result, ValidationFailure> { match validation_context.role { Role::VoluntaryExit => { // TODO For voluntary exit, check the stored duties https://github.com/sigp/anchor/issues/277 // This would need to be adapted to use the actual duty store - Ok((2, true)) + Ok(Some(2)) } - Role::Aggregator | Role::ValidatorRegistration => Ok((2, true)), + Role::Aggregator | Role::ValidatorRegistration => Ok(Some(2)), Role::Committee => { let validator_index_count = validator_indices.len() as u64; let slots_per_epoch_val = validation_context.slots_per_epoch; @@ -589,16 +587,16 @@ fn duty_limit( // Check if at least one validator is in the sync committee for &index in validator_indices { if duty_provider.is_validator_in_sync_committee(period, index) { - return Ok((slots_per_epoch_val, true)); + return Ok(Some(slots_per_epoch_val)); } } } - Ok(( - std::cmp::min(slots_per_epoch_val, 2 * validator_index_count), - true, - )) + Ok(Some(std::cmp::min( + slots_per_epoch_val, + 2 * validator_index_count, + ))) } - _ => Ok((0, false)), + _ => Ok(None), } } @@ -813,6 +811,16 @@ mod tests { vec![private_key], ); + let slot_clock = ManualSlotClock::new( + Slot::new(0), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + // we need this cause the msg is in slot 1 and otherwise would be too early + .saturating_sub(Duration::from_secs(1)), + Duration::from_secs(1), + ); + let validation_context = ValidationContext { signed_ssv_message: &signed_msg, committee_info: &committee_info, @@ -821,11 +829,7 @@ mod tests { operators_pk: &[public_key], slots_per_epoch: 32, epochs_per_sync_committee_period: 256, - slot_clock: ManualSlotClock::new( - Slot::new(0), - SystemTime::now().duration_since(UNIX_EPOCH).unwrap(), - Duration::from_secs(1), - ), + slot_clock, }; let result = validate_ssv_message( From 87973785802b1e5fa92796d5b18c45edbb36ed3e Mon Sep 17 00:00:00 2001 From: diego Date: Tue, 22 Apr 2025 19:32:04 +0200 Subject: [PATCH 32/35] rename var and fix typo --- anchor/message_receiver/src/manager.rs | 8 ++++---- anchor/message_validator/src/duties/duties_tracker.rs | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/anchor/message_receiver/src/manager.rs b/anchor/message_receiver/src/manager.rs index 6b427305c..70e478cfe 100644 --- a/anchor/message_receiver/src/manager.rs +++ b/anchor/message_receiver/src/manager.rs @@ -132,12 +132,12 @@ impl MessageReceiver if is_member { // We are not a member for this committee, return without passing. - trace!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?committee, "Not interested"); + trace!(gossipsub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?committee, "Not interested"); return; } } None => { - error!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, "Invalid message ID"); + error!(gossipsub_message_id = ?message_id, ssv_msg_id = ?msg_id, "Invalid message ID"); return; } } @@ -148,7 +148,7 @@ impl MessageReceiver .qbft_manager .receive_data(signed_ssv_message, qbft_message) { - error!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?err, "Unable to receive QBFT message"); + error!(gossipsub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?err, "Unable to receive QBFT message"); } } ValidatedSSVMessage::PartialSignatureMessages(messages) => { @@ -156,7 +156,7 @@ impl MessageReceiver .signature_collector .receive_partial_signatures(messages) { - error!(gosspisub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?err, "Unable to receive partial signature message"); + error!(gossipsub_message_id = ?message_id, ssv_msg_id = ?msg_id, ?err, "Unable to receive partial signature message"); } } } diff --git a/anchor/message_validator/src/duties/duties_tracker.rs b/anchor/message_validator/src/duties/duties_tracker.rs index 5ad28fd90..3ed1385c8 100644 --- a/anchor/message_validator/src/duties/duties_tracker.rs +++ b/anchor/message_validator/src/duties/duties_tracker.rs @@ -118,11 +118,10 @@ impl DutiesTracker { async fn poll_sync_committee_duties_for_period( &self, - local_indices: &[u64], + validator_indices: &[u64], sync_committee_period: u64, ) -> Result<(), Error> { - // no local validators don't need to poll for sync committee - if local_indices.is_empty() { + if validator_indices.is_empty() { debug!( sync_committee_period, "No validators, not polling for sync committee duties" @@ -132,7 +131,7 @@ impl DutiesTracker { debug!( sync_committee_period, - num_validators = local_indices.len(), + num_validators = validator_indices.len(), "Fetching sync committee duties" ); @@ -142,7 +141,7 @@ impl DutiesTracker { .beacon_nodes .first_success(|beacon_node| async move { beacon_node - .post_validator_duties_sync(period_start_epoch, local_indices) + .post_validator_duties_sync(period_start_epoch, validator_indices) .await }) .await; From 37ba5a8be9ee433b445989d879bcc5a1db8af0b2 Mon Sep 17 00:00:00 2001 From: diego Date: Tue, 22 Apr 2025 23:58:14 +0200 Subject: [PATCH 33/35] add test_early_message_fails_validation and test_late_message_fails_validation --- .../src/consensus_message.rs | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index c6330ab23..ed95eb7a3 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -852,6 +852,101 @@ mod tests { } } + #[test] + fn test_early_message_fails_validation() { + // Generate a key pair + let (private_key, _) = generate_test_key_pair(); + let committee_info = create_committee_info(FOUR_NODE_COMMITTEE); + + let qbft_message = + QbftMessageBuilder::new(Role::Committee, QbftMessageType::Proposal).build(); + let signed_msg = create_signed_consensus_message( + qbft_message, + vec![OperatorId(2)], + vec![], + vec![private_key], + ); + + // Set up slot clock where current time is before slot start time (message too early) + let now = SystemTime::now(); + let slot_clock = ManualSlotClock::new( + Slot::new(0), + // Slot 1 starts in 1 second from now + now.duration_since(UNIX_EPOCH).unwrap(), + Duration::from_secs(1), + ); + + let validation_context = ValidationContext { + signed_ssv_message: &signed_msg, + committee_info: &committee_info, + role: Role::Committee, + received_at: now, + operators_pk: &[], + slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock, + }; + + let result = validate_ssv_message( + validation_context, + &mut ConsensusState::new(2), + Arc::new(MockDutiesProvider {}), + ); + + assert_validation_error( + result, + |failure| matches!(failure, EarlySlotMessage { got: _ }), + "EarlySlotMessage", + ); + } + + #[test] + fn test_late_message_fails_validation() { + // Generate a key pair + let (private_key, _) = generate_test_key_pair(); + let committee_info = create_committee_info(FOUR_NODE_COMMITTEE); + + let qbft_message = + QbftMessageBuilder::new(Role::Proposer, QbftMessageType::Proposal).build(); + let signed_msg = create_signed_consensus_message( + qbft_message, + vec![OperatorId(2)], + vec![], + vec![private_key], + ); + + let now = SystemTime::now(); + let slot_duration = Duration::from_secs(1); + let slot_clock = ManualSlotClock::new( + Slot::new(0), + now.duration_since(UNIX_EPOCH).unwrap(), + slot_duration, + ); + + let validation_context = ValidationContext { + signed_ssv_message: &signed_msg, + committee_info: &committee_info, + role: Role::Proposer, // Proposer role has TTL = 1 + LATE_SLOT_ALLOWANCE + LATE_MESSAGE_MARGIN. To be late for slot 1, we need to add more 2 seconds (2 * slot duration). + received_at: now.checked_add(Duration::from_secs(1 + LATE_SLOT_ALLOWANCE + 2)).unwrap().checked_add(LATE_MESSAGE_MARGIN).unwrap(), + operators_pk: &[], + slots_per_epoch: 32, + epochs_per_sync_committee_period: 256, + slot_clock, + }; + + let result = validate_ssv_message( + validation_context, + &mut ConsensusState::new(2), + Arc::new(MockDutiesProvider {}), + ); + + assert_validation_error( + result, + |failure| matches!(failure, LateSlotMessage { got: _ }), + "LateSlotMessage", + ); + } + #[test] fn test_validate_ssv_message_invalid_consensus_data() { let committee_info = create_committee_info(FOUR_NODE_COMMITTEE); @@ -1324,6 +1419,8 @@ mod tests { use slot_clock::ManualSlotClock; use types::Epoch; + use crate::ValidationFailure::LateSlotMessage; + #[test] fn test_verify_message_signatures_success() { // Generate a key pair From ce86f70afe1239e77f2f42993154c8ee41c385a2 Mon Sep 17 00:00:00 2001 From: diego Date: Wed, 23 Apr 2025 14:28:35 +0200 Subject: [PATCH 34/35] make test_validate_ssv_message_consensus_success easier to understand --- anchor/message_validator/src/consensus_message.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index ed95eb7a3..2b641b4f3 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -811,21 +811,21 @@ mod tests { vec![private_key], ); + let now = SystemTime::now(); + let slot_duration = Duration::from_secs(1); let slot_clock = ManualSlotClock::new( Slot::new(0), - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - // we need this cause the msg is in slot 1 and otherwise would be too early - .saturating_sub(Duration::from_secs(1)), + now.duration_since(UNIX_EPOCH).unwrap(), Duration::from_secs(1), ); + slot_clock.advance_slot(); + slot_clock.advance_time(slot_duration); let validation_context = ValidationContext { signed_ssv_message: &signed_msg, committee_info: &committee_info, role: Role::Committee, - received_at: SystemTime::now(), + received_at: now + slot_duration, operators_pk: &[public_key], slots_per_epoch: 32, epochs_per_sync_committee_period: 256, From 415c9c73c25bec1650061db6cc7bf6af9a9c20ea Mon Sep 17 00:00:00 2001 From: diego Date: Thu, 24 Apr 2025 10:33:32 +0200 Subject: [PATCH 35/35] fix duty limit comparison --- anchor/message_validator/src/consensus_message.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/anchor/message_validator/src/consensus_message.rs b/anchor/message_validator/src/consensus_message.rs index 2b641b4f3..f9e0d24d8 100644 --- a/anchor/message_validator/src/consensus_message.rs +++ b/anchor/message_validator/src/consensus_message.rs @@ -547,7 +547,7 @@ pub(crate) fn validate_duty_count( let epoch = slot.epoch(validation_context.slots_per_epoch); let duty_count = signer_state.get_duty_count(epoch); - if duty_count >= limit { + if duty_count > limit { return Err(ValidationFailure::ExcessiveDutyCount { got: duty_count, limit,