diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index e3be20eb71..980f60cbb3 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2141,7 +2141,7 @@ impl BeaconChain { // NOTE: not sure how to handle scenario where head hash is `None` i.e. pre-bellatrix, which // is pre-electra. let Some(parent_hash) = parent_hash else { - debug!("Failed to fetch parent_hash"); + info!("Failed to fetch parent_hash"); return Ok(None); }; @@ -2152,7 +2152,7 @@ impl BeaconChain { // // This prevents the routine from running during sync. if head_slot != current_slot { - debug!(?head_slot, ?current_slot, "Head too old for inclusion list"); + info!(?head_slot, ?current_slot, "Head too old for inclusion list"); return Ok(None); } @@ -2161,16 +2161,16 @@ impl BeaconChain { // // NOTE: does this represent a critical error? should we return an error here or log crit? // is this check redundant? - if request_slot != next_slot { - debug!( - ?request_slot, - ?next_slot, - "Inclusion list request slot not equal to next slot" - ); - return Ok(None); - } + // if request_slot != next_slot { + // info!( + // ?request_slot, + // ?next_slot, + // "Inclusion list request slot not equal to next slot" + // ); + // return Ok(None); + // } - debug!( + info!( %parent_hash, ?current_slot, "Attempt to fetch IL from EL" @@ -2181,7 +2181,7 @@ impl BeaconChain { .await .map_err(|e| Error::ExecutionLayerGetInclusionListFailed(Box::new(e)))?; - debug!( + info!( tx_count = inclusion_list.len(), "Inclusion list fetched from EL" ); @@ -7357,6 +7357,12 @@ impl BeaconChain { .on_inclusion_list(signed_il); } + pub fn inclusion_list_seen(&self, signed_il: &SignedInclusionList) -> bool { + self.inclusion_list_cache + .read() + .inclusion_list_seen(signed_il) + } + pub fn metrics(&self) -> BeaconChainMetrics { BeaconChainMetrics { reqresp_pre_import_cache_len: self.reqresp_pre_import_cache.read().len(), diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index ac8dca3e0e..d2ddefe44e 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -112,7 +112,7 @@ impl PayloadNotifier { .get_inclusion_list_transactions(block.slot()) .unwrap_or(vec![].into()); - debug!( + info!( tx_count = inclusion_list_transactions.len(), slot = ?block.slot(), "Adding inclusion list transactions in the Payload Notifier" diff --git a/beacon_node/beacon_chain/src/inclusion_list_verification.rs b/beacon_node/beacon_chain/src/inclusion_list_verification.rs index bbffe480da..fe23cdbaba 100644 --- a/beacon_node/beacon_chain/src/inclusion_list_verification.rs +++ b/beacon_node/beacon_chain/src/inclusion_list_verification.rs @@ -1,8 +1,14 @@ -use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; +use std::time::Duration; + +use crate::{ + validator_monitor::{get_slot_delay_ms, timestamp_now}, + BeaconChain, BeaconChainError, BeaconChainTypes, +}; use slot_clock::SlotClock; use strum::AsRefStr; -use types::{Domain, EthSpec, SignedInclusionList, SignedRoot, Slot}; +use tree_hash::TreeHash; +use types::{Domain, SignedInclusionList, SignedRoot, Slot}; #[derive(Debug, AsRefStr)] pub enum GossipInclusionListError { @@ -16,6 +22,7 @@ pub enum GossipInclusionListError { InvalidSignature, BeaconChainError(Box), PriorInclusionListKnown, + InclusionListSeen, // TODO: equivocation e.g. PriorInclusionListKnown } @@ -42,6 +49,18 @@ impl GossipVerifiedInclusionList { .now() .ok_or(BeaconChainError::UnableToReadSlot)?; + // TODO(focil) move 8192 to config + if signed_il + .message + .transactions + .iter() + .map(|v| v.len()) + .sum::() + > 8192 + { + return Err(GossipInclusionListError::TooManyTransactions); + } + if message_slot != current_slot && message_slot != current_slot - 1 { return Err(GossipInclusionListError::InvalidSlot { message_slot, @@ -49,23 +68,35 @@ impl GossipVerifiedInclusionList { }); } - // TODO(focil): the slot is equal to the current slot or the previous slot and the current time is - // not past the attestation deadline + let attestation_deadline = Duration::from_secs(chain.spec.seconds_per_slot / 3); - // TODO(focil): the IL committee root is equal to the hash tree root of the expected committee + let inclusion_list_delay_total = + get_slot_delay_ms(timestamp_now(), message_slot, &chain.slot_clock); - // TODO(focil): the validator index is contained in the committee corresponding to the committee - // root + let exceeds_attestation_deadline = attestation_deadline >= inclusion_list_delay_total; - // the transaction length is less than or equal to the specified maximum - // TODO(focil) review this - // if signed_il.message.transactions.len() > T::EthSpec::max_bytes_per_inclusion_list() - // { - // return Err(GossipInclusionListError::TooManyTransactions); - // } + if exceeds_attestation_deadline { + return Err(GossipInclusionListError::InvalidSlot { + message_slot, + current_slot, + }); + } - // TODO: the message is the first or second valid message received from the validator - // corresponding to the validator index + let head_snapshot = chain.head_snapshot(); + + let il_committee = head_snapshot + .beacon_state + .get_inclusion_list_committee(message_slot, &chain.spec) + .map_err(|_| GossipInclusionListError::InvalidCommitteeRoot)?; + + if signed_il.message.inclusion_list_committee_root != il_committee.tree_hash_root() { + tracing::error!("INVALID COMMITTEE ROOT"); + return Err(GossipInclusionListError::InvalidCommitteeRoot); + } + + if !il_committee.contains(&signed_il.message.validator_index) { + return Err(GossipInclusionListError::ValidatorNotInCommittee); + } // the signature is valid w.r.t. the validator index let epoch = chain.epoch()?; @@ -85,11 +116,15 @@ impl GossipVerifiedInclusionList { BeaconChainError::ValidatorIndexUnknown(validator_index), ))); }; - // TODO(focil) fix inclusion list verify + if !signed_il.signature.verify(&pubkey, message) { return Err(GossipInclusionListError::InvalidSignature); } + if chain.inclusion_list_seen(&signed_il) { + return Err(GossipInclusionListError::InclusionListSeen); + } + Ok(Self { signed_il: signed_il.clone(), }) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 585686f090..5fd19c20c5 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -3716,6 +3716,7 @@ pub fn serve( return Err(warp_utils::reject::unhandled_error(e)); } }; + Ok::<_, warp::reject::Rejection>(warp::reply::json(&data).into_response()) }) }, diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 89384bc39e..2e6d3483dd 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -2239,6 +2239,7 @@ impl NetworkBeaconProcessor { | GossipInclusionListError::ValidatorNotInCommittee | GossipInclusionListError::TooManyTransactions | GossipInclusionListError::InvalidSignature + | GossipInclusionListError::InclusionListSeen | GossipInclusionListError::PriorInclusionListKnown => { debug!( error = ?err, diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 1cc804546e..826f4296de 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -874,46 +874,40 @@ impl BeaconState { return Err(Error::SlotOutOfBounds); } - let seed = self.get_inclusion_list_seed(slot, spec)?; - let indices = self.get_active_validator_indices(epoch, spec)?; + let seed = self.get_seed(epoch, Domain::InclusionListCommittee, spec)?; + let active_validator_indices = self.get_active_validator_indices(epoch, spec)?; + let active_validator_count = active_validator_indices.len(); let start = (slot.safe_rem(E::slots_per_epoch())?) .as_usize() .safe_mul(E::InclusionListCommitteeSize::to_usize())?; let end = start.safe_add(E::InclusionListCommitteeSize::to_usize())?; + println!("start {:?}", start); + println!("end {:?}", end); + println!("slot {:?}", slot); + let mut i = start; let mut il_committee_indices = Vec::with_capacity(E::InclusionListCommitteeSize::to_usize()); while i < end { let shuffled_index = compute_shuffled_index( - i.safe_rem(indices.len())?, - indices.len(), - &seed, + i.safe_rem(active_validator_count)?, + active_validator_count, + seed.as_slice(), spec.shuffle_round_count, ) .ok_or(Error::UnableToShuffle)?; - il_committee_indices.push(shuffled_index as u64); + let validator_index = *active_validator_indices + .get(shuffled_index) + .ok_or(Error::ShuffleIndexOutOfBounds(shuffled_index))?; + il_committee_indices.push(validator_index as u64); i.safe_add_assign(1)?; } Ok(InclusionListCommittee::::from(il_committee_indices)) } - /// Compute the seed to use for the beacon inclusion list committee selection at the given - /// `slot`. - /// - /// Spec v0.12.1 - pub fn get_inclusion_list_seed(&self, slot: Slot, spec: &ChainSpec) -> Result, Error> { - let epoch = slot.epoch(E::slots_per_epoch()); - let mut preimage = self - .get_seed(epoch, Domain::InclusionListCommittee, spec)? - .as_slice() - .to_vec(); - preimage.append(&mut int_to_bytes8(slot.as_u64())); - Ok(hash(&preimage)) - } - /// Returns the block root which decided the proposer shuffling for the epoch passed in parameter. This root /// can be used to key this proposer shuffling. /// diff --git a/consensus/types/src/beacon_state/inclusion_list_cache.rs b/consensus/types/src/beacon_state/inclusion_list_cache.rs index a096f532d6..f73f10ef7a 100644 --- a/consensus/types/src/beacon_state/inclusion_list_cache.rs +++ b/consensus/types/src/beacon_state/inclusion_list_cache.rs @@ -2,7 +2,7 @@ use crate::Transactions; use super::{EthSpec, SignedInclusionList, Slot, Transaction}; use std::collections::{HashMap, HashSet}; -use tracing::debug; +use tracing::info; /// Map from slot to inclusion lists #[derive(Debug, Default, Clone, PartialEq)] @@ -25,6 +25,17 @@ impl InclusionListCache { self.inner_map.remove(&slot); } + pub fn inclusion_list_seen(&self, inclusion_list: &SignedInclusionList) -> bool { + let slot = inclusion_list.message.slot; + let Some(inner) = self.inner_map.get(&slot) else { + return false; + }; + + inner + .inclusion_lists_seen + .contains(&inclusion_list.message.validator_index) + } + pub fn on_inclusion_list(&mut self, inclusion_list: SignedInclusionList) { let slot = inclusion_list.message.slot; let inner = self.inner_map.entry(slot).or_default(); @@ -33,7 +44,7 @@ impl InclusionListCache { .inclusion_list_equivocators .contains(&inclusion_list.message.validator_index) { - debug!( + info!( ?slot, inclusion_list.message.validator_index, "This validator was flagged for an equivocating inclusion list", @@ -47,7 +58,7 @@ impl InclusionListCache { .contains(&inclusion_list.message.validator_index) && inner.inclusion_lists.contains(&inclusion_list) { - debug!("Already seen identical inclusion list from this validator"); + info!("Already seen identical inclusion list from this validator"); return; } @@ -56,7 +67,7 @@ impl InclusionListCache { .contains(&inclusion_list.message.validator_index) && !inner.inclusion_lists.contains(&inclusion_list) { - debug!( + info!( ?slot, inclusion_list.message.validator_index, "Equivocating inclusion list", ); @@ -76,7 +87,7 @@ impl InclusionListCache { .insert(inclusion_list.message.validator_index); inner.inclusion_lists.insert(inclusion_list); - debug!( + info!( ?slot, tx_count = inner.inclusion_list_transactions.len(), "Successfully added inclusion list transactions to the cache", diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 2020244d59..c36a9209a0 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -975,7 +975,7 @@ impl ChainSpec { /* * FOCIL params */ - domain_inclusion_list_committee: 13, + domain_inclusion_list_committee: 12, inclusion_list_committee_size: 16, eip7805_fork_epoch: None, eip7805_fork_version: [0x06, 0x00, 0x00, 0x00], @@ -1320,7 +1320,7 @@ impl ChainSpec { /* * FOCIL params */ - domain_inclusion_list_committee: 13, + domain_inclusion_list_committee: 12, inclusion_list_committee_size: 16, eip7805_fork_epoch: None, eip7805_fork_version: [0x06, 0x00, 0x00, 0x00], diff --git a/validator_client/validator_services/src/inclusion_list_service.rs b/validator_client/validator_services/src/inclusion_list_service.rs index 0b6404d565..a22c014657 100644 --- a/validator_client/validator_services/src/inclusion_list_service.rs +++ b/validator_client/validator_services/src/inclusion_list_service.rs @@ -8,7 +8,9 @@ use std::sync::Arc; use task_executor::TaskExecutor; use tokio::time::{sleep, Duration}; use tracing::{debug, error, info, trace, warn}; -use types::{ChainSpec, EthSpec, InclusionList, InclusionListDuty, Slot}; +use types::{ + inclusion_list, ChainSpec, EthSpec, InclusionList, InclusionListDuty, Slot, VariableList, +}; use validator_store::{Error as ValidatorStoreError, ValidatorStore}; /// Builds an `AttestationService`. @@ -184,11 +186,11 @@ impl InclusionListService Result<(), String> { - debug!("Spawning inclusion list task"); + info!("Spawning inclusion list task"); - let next_slot = self.slot_clock.now().ok_or("Failed to read slot clock")? + 1; + let current_slot: Slot = self.slot_clock.now().ok_or("Failed to read slot clock")?; - if !spec.is_focil_enabled_for_epoch(next_slot.epoch(S::E::slots_per_epoch())) { + if !spec.is_focil_enabled_for_epoch(current_slot.epoch(S::E::slots_per_epoch())) { debug!("FOCIL not enabled"); return Ok(()); } @@ -199,7 +201,9 @@ impl InclusionListService InclusionListService InclusionListService