From 29b5614bcba7ed347974d6927c052de1c6a65de9 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 21 Feb 2025 06:53:54 -0800 Subject: [PATCH 1/7] add logic to gossip light client updates post computation --- beacon_node/client/src/builder.rs | 5 +- .../src/compute_light_client_updates.rs | 58 ++++++++++++++++--- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index e3bfd60a48b..9d056d53139 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -1,5 +1,5 @@ use crate::compute_light_client_updates::{ - compute_light_client_updates, LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY, + compute_and_gossip_light_client_updates, LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY, }; use crate::config::{ClientGenesis, Config as ClientConfig}; use crate::notifier::spawn_notifier; @@ -957,10 +957,11 @@ where let log = light_client_update_context.log().clone(); light_client_update_context.executor.spawn( async move { - compute_light_client_updates( + compute_and_gossip_light_client_updates( &inner_chain, light_client_server_rv, beacon_processor_channels.work_reprocessing_tx, + self.network_senders.clone(), &log, ) .await diff --git a/beacon_node/client/src/compute_light_client_updates.rs b/beacon_node/client/src/compute_light_client_updates.rs index 1eb977d4213..9a87f33b30d 100644 --- a/beacon_node/client/src/compute_light_client_updates.rs +++ b/beacon_node/client/src/compute_light_client_updates.rs @@ -2,6 +2,8 @@ use beacon_chain::{BeaconChain, BeaconChainTypes, LightClientProducerEvent}; use beacon_processor::work_reprocessing_queue::ReprocessQueueMessage; use futures::channel::mpsc::Receiver; use futures::StreamExt; +use lighthouse_network::PubsubMessage; +use network::{NetworkMessage, NetworkSenders}; use slog::{error, Logger}; use tokio::sync::mpsc::Sender; @@ -11,10 +13,11 @@ use tokio::sync::mpsc::Sender; // take a few milliseconds. 32 is a small enough arbitrary number. pub(crate) const LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY: usize = 32; -pub async fn compute_light_client_updates( +pub async fn compute_and_gossip_light_client_updates( chain: &BeaconChain, mut light_client_server_rv: Receiver>, reprocess_tx: Sender, + network_senders: Option>, log: &Logger, ) { // Should only receive events for recent blocks, import_block filters by blocks close to clock. @@ -25,12 +28,53 @@ pub async fn compute_light_client_updates( while let Some(event) = light_client_server_rv.next().await { let parent_root = event.0; - chain - .recompute_and_cache_light_client_updates(event) - .unwrap_or_else(|e| { - error!(log, "error computing light_client updates {:?}", e); - }); - + match chain.recompute_and_cache_light_client_updates(event) { + Err(e) => error!(log, "error computing light_client updates {:?}", e), + Ok(()) => { + // gossip the latest computed light client updates + if let ( + Some(network_tx), + Some(latest_finality_update), + Some(latest_optimistic_update), + ) = ( + network_senders.as_ref().map(|n| n.network_send()), + chain.light_client_server_cache.get_latest_finality_update(), + chain + .light_client_server_cache + .get_latest_optimistic_update(), + ) { + let _ = network_tx + .send(NetworkMessage::Publish { + messages: vec![PubsubMessage::LightClientFinalityUpdate(Box::new( + latest_finality_update, + ))], + }) + .inspect_err(|e| { + error!(log, "error publishing light_client finality update {:?}", e) + }); + let _ = network_tx + .send(NetworkMessage::Publish { + messages: vec![PubsubMessage::LightClientOptimisticUpdate(Box::new( + latest_optimistic_update, + ))], + }) + .inspect_err(|e| { + error!( + log, + "error publishing light_client optimistic update {:?}", e + ) + }); + } else { + error!( + log, + "couldn't publish light_client updates"; + "network_senders_avail" => network_senders.is_some(), + "finality_update_avail" => chain.light_client_server_cache.get_latest_finality_update().is_some(), + "optimistic_update_avail" => chain.light_client_server_cache.get_latest_optimistic_update().is_some(), + ) + } + } + }; let msg = ReprocessQueueMessage::NewLightClientOptimisticUpdate { parent_root }; if reprocess_tx.try_send(msg).is_err() { error!(log, "Failed to inform light client update"; "parent_root" => %parent_root) From f99fef7ff0b0a7af466776e3519944f9450b7651 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 23 Feb 2025 16:39:08 -0800 Subject: [PATCH 2/7] gossip light client data durnig sync committee contributions --- beacon_node/beacon_chain/src/beacon_chain.rs | 9 +++ .../src/light_client_server_cache.rs | 62 +++++++++++++++++++ beacon_node/client/src/builder.rs | 5 +- .../src/compute_light_client_updates.rs | 58 +++-------------- beacon_node/http_api/src/sync_committees.rs | 34 ++++++++++ 5 files changed, 114 insertions(+), 54 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index ca21b519f15..cbb54731f0c 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2187,6 +2187,15 @@ impl BeaconChain { }) } + pub fn get_latest_finality_update(&self) -> Option> { + self.light_client_server_cache.get_latest_finality_update() + } + + pub fn get_latest_optimistic_update(&self) -> Option> { + self.light_client_server_cache + .get_latest_optimistic_update() + } + /// Accepts some 'LightClientFinalityUpdate' from the network and attempts to verify it pub fn verify_finality_update_for_gossip( self: &Arc, diff --git a/beacon_node/beacon_chain/src/light_client_server_cache.rs b/beacon_node/beacon_chain/src/light_client_server_cache.rs index 78442d8df08..246576e2e2b 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -40,6 +40,10 @@ pub struct LightClientServerCache { latest_written_current_sync_committee: RwLock>>>, /// Caches state proofs by block root prev_block_cache: Mutex>>, + /// Tracks the latest broadcasted finality update + latest_broadcasted_finality_update: RwLock>>, + /// Tracks the latest broadcasted optimistic update + latest_broadcasted_optimistic_update: RwLock>>, } impl LightClientServerCache { @@ -49,6 +53,8 @@ impl LightClientServerCache { latest_optimistic_update: None.into(), latest_light_client_update: None.into(), latest_written_current_sync_committee: None.into(), + latest_broadcasted_finality_update: None.into(), + latest_broadcasted_optimistic_update: None.into(), prev_block_cache: lru::LruCache::new(PREV_BLOCK_CACHE_SIZE).into(), } } @@ -333,10 +339,66 @@ impl LightClientServerCache { Ok(new_value) } + pub fn should_broadcast_latest_finality_update( + &self, + ) -> Option> { + if let Some(latest_finality_update) = self.get_latest_finality_update() { + let latest_broadcasted_finality_update = self + .latest_broadcasted_finality_update + .read() + .clone() + .take(); + match latest_broadcasted_finality_update { + Some(latest_broadcasted_finality_update) => { + if latest_broadcasted_finality_update != latest_finality_update { + *self.latest_broadcasted_finality_update.write() = + Some(latest_finality_update.clone()); + return Some(latest_finality_update); + } + } + None => { + *self.latest_broadcasted_finality_update.write() = + Some(latest_finality_update.clone()); + return Some(latest_finality_update); + } + } + } + + return None; + } + pub fn get_latest_finality_update(&self) -> Option> { self.latest_finality_update.read().clone() } + /// Checks if we've already broadcasted the latest optimistic update. + /// If we haven't, update the `latest_broadcasted_optimistic_update` cache + /// and return the latest optimistic update for broadcasting + pub fn should_broadcast_latest_optimistic_update( + &self, + ) -> Option> { + if let Some(latest_optimistic_update) = self.get_latest_optimistic_update() { + let latest_broadcasted_optimistic_update = + self.latest_optimistic_update.read().clone().take(); + match latest_broadcasted_optimistic_update { + Some(latest_broadcasted_optimistic_update) => { + if latest_broadcasted_optimistic_update != latest_optimistic_update { + *self.latest_broadcasted_optimistic_update.write() = + Some(latest_optimistic_update.clone()); + return Some(latest_optimistic_update); + } + } + None => { + *self.latest_broadcasted_optimistic_update.write() = + Some(latest_optimistic_update.clone()); + return Some(latest_optimistic_update); + } + } + } + + return None; + } + pub fn get_latest_optimistic_update(&self) -> Option> { self.latest_optimistic_update.read().clone() } diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 9d056d53139..e3bfd60a48b 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -1,5 +1,5 @@ use crate::compute_light_client_updates::{ - compute_and_gossip_light_client_updates, LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY, + compute_light_client_updates, LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY, }; use crate::config::{ClientGenesis, Config as ClientConfig}; use crate::notifier::spawn_notifier; @@ -957,11 +957,10 @@ where let log = light_client_update_context.log().clone(); light_client_update_context.executor.spawn( async move { - compute_and_gossip_light_client_updates( + compute_light_client_updates( &inner_chain, light_client_server_rv, beacon_processor_channels.work_reprocessing_tx, - self.network_senders.clone(), &log, ) .await diff --git a/beacon_node/client/src/compute_light_client_updates.rs b/beacon_node/client/src/compute_light_client_updates.rs index 9a87f33b30d..1eb977d4213 100644 --- a/beacon_node/client/src/compute_light_client_updates.rs +++ b/beacon_node/client/src/compute_light_client_updates.rs @@ -2,8 +2,6 @@ use beacon_chain::{BeaconChain, BeaconChainTypes, LightClientProducerEvent}; use beacon_processor::work_reprocessing_queue::ReprocessQueueMessage; use futures::channel::mpsc::Receiver; use futures::StreamExt; -use lighthouse_network::PubsubMessage; -use network::{NetworkMessage, NetworkSenders}; use slog::{error, Logger}; use tokio::sync::mpsc::Sender; @@ -13,11 +11,10 @@ use tokio::sync::mpsc::Sender; // take a few milliseconds. 32 is a small enough arbitrary number. pub(crate) const LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY: usize = 32; -pub async fn compute_and_gossip_light_client_updates( +pub async fn compute_light_client_updates( chain: &BeaconChain, mut light_client_server_rv: Receiver>, reprocess_tx: Sender, - network_senders: Option>, log: &Logger, ) { // Should only receive events for recent blocks, import_block filters by blocks close to clock. @@ -28,53 +25,12 @@ pub async fn compute_and_gossip_light_client_updates( while let Some(event) = light_client_server_rv.next().await { let parent_root = event.0; - match chain.recompute_and_cache_light_client_updates(event) { - Err(e) => error!(log, "error computing light_client updates {:?}", e), - Ok(()) => { - // gossip the latest computed light client updates - if let ( - Some(network_tx), - Some(latest_finality_update), - Some(latest_optimistic_update), - ) = ( - network_senders.as_ref().map(|n| n.network_send()), - chain.light_client_server_cache.get_latest_finality_update(), - chain - .light_client_server_cache - .get_latest_optimistic_update(), - ) { - let _ = network_tx - .send(NetworkMessage::Publish { - messages: vec![PubsubMessage::LightClientFinalityUpdate(Box::new( - latest_finality_update, - ))], - }) - .inspect_err(|e| { - error!(log, "error publishing light_client finality update {:?}", e) - }); - let _ = network_tx - .send(NetworkMessage::Publish { - messages: vec![PubsubMessage::LightClientOptimisticUpdate(Box::new( - latest_optimistic_update, - ))], - }) - .inspect_err(|e| { - error!( - log, - "error publishing light_client optimistic update {:?}", e - ) - }); - } else { - error!( - log, - "couldn't publish light_client updates"; - "network_senders_avail" => network_senders.is_some(), - "finality_update_avail" => chain.light_client_server_cache.get_latest_finality_update().is_some(), - "optimistic_update_avail" => chain.light_client_server_cache.get_latest_optimistic_update().is_some(), - ) - } - } - }; + chain + .recompute_and_cache_light_client_updates(event) + .unwrap_or_else(|e| { + error!(log, "error computing light_client updates {:?}", e); + }); + let msg = ReprocessQueueMessage::NewLightClientOptimisticUpdate { parent_root }; if reprocess_tx.try_send(msg).is_err() { error!(log, "Failed to inform light client update"; "parent_root" => %parent_root) diff --git a/beacon_node/http_api/src/sync_committees.rs b/beacon_node/http_api/src/sync_committees.rs index da9f9b7a063..1e006b806ef 100644 --- a/beacon_node/http_api/src/sync_committees.rs +++ b/beacon_node/http_api/src/sync_committees.rs @@ -319,6 +319,40 @@ pub fn process_signed_contribution_and_proofs( let seen_timestamp = timestamp_now(); + if let Some(latest_optimistic_update) = chain + .light_client_server_cache + .should_broadcast_latest_optimistic_update() + { + let _ = publish_pubsub_message( + &network_tx, + PubsubMessage::LightClientOptimisticUpdate(Box::new(latest_optimistic_update)), + ) + .inspect_err(|e| { + error!( + log, + "Unable to broadcast latest light client optimistic update"; + "error" => ?e, + ); + }); + }; + + if let Some(latest_finality_update) = chain + .light_client_server_cache + .should_broadcast_latest_finality_update() + { + let _ = publish_pubsub_message( + &network_tx, + PubsubMessage::LightClientFinalityUpdate(Box::new(latest_finality_update)), + ) + .inspect_err(|e| { + error!( + log, + "Unable to broadcast latest light client finality update"; + "error" => ?e, + ); + }); + }; + // Verify contributions & broadcast to the network. for (index, contribution) in signed_contribution_and_proofs.into_iter().enumerate() { let aggregator_index = contribution.message.aggregator_index; From b51196e4614f9769a68d7bfeb17e9fd90de5c5cd Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 23 Feb 2025 16:41:27 -0800 Subject: [PATCH 3/7] fix comments --- beacon_node/beacon_chain/src/beacon_chain.rs | 9 --------- .../beacon_chain/src/light_client_server_cache.rs | 5 ++++- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index cbb54731f0c..ca21b519f15 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2187,15 +2187,6 @@ impl BeaconChain { }) } - pub fn get_latest_finality_update(&self) -> Option> { - self.light_client_server_cache.get_latest_finality_update() - } - - pub fn get_latest_optimistic_update(&self) -> Option> { - self.light_client_server_cache - .get_latest_optimistic_update() - } - /// Accepts some 'LightClientFinalityUpdate' from the network and attempts to verify it pub fn verify_finality_update_for_gossip( self: &Arc, diff --git a/beacon_node/beacon_chain/src/light_client_server_cache.rs b/beacon_node/beacon_chain/src/light_client_server_cache.rs index 246576e2e2b..8b5b896542f 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -339,6 +339,9 @@ impl LightClientServerCache { Ok(new_value) } + /// Checks if we've already broadcasted the latest finality update. + /// If we haven't, update the `latest_broadcasted_finality_update` cache + /// and return the latest finality update for broadcasting, else return `None`. pub fn should_broadcast_latest_finality_update( &self, ) -> Option> { @@ -373,7 +376,7 @@ impl LightClientServerCache { /// Checks if we've already broadcasted the latest optimistic update. /// If we haven't, update the `latest_broadcasted_optimistic_update` cache - /// and return the latest optimistic update for broadcasting + /// and return the latest optimistic update for broadcasting, else return `None`. pub fn should_broadcast_latest_optimistic_update( &self, ) -> Option> { From 75f00af66d4fd73f4c8294b5284a0ef68fd9cbe6 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 23 Feb 2025 21:04:07 -0800 Subject: [PATCH 4/7] fix --- .../beacon_chain/src/light_client_server_cache.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/beacon_node/beacon_chain/src/light_client_server_cache.rs b/beacon_node/beacon_chain/src/light_client_server_cache.rs index 8b5b896542f..98995312c0a 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -346,11 +346,8 @@ impl LightClientServerCache { &self, ) -> Option> { if let Some(latest_finality_update) = self.get_latest_finality_update() { - let latest_broadcasted_finality_update = self - .latest_broadcasted_finality_update - .read() - .clone() - .take(); + let latest_broadcasted_finality_update = + self.latest_broadcasted_finality_update.read().clone(); match latest_broadcasted_finality_update { Some(latest_broadcasted_finality_update) => { if latest_broadcasted_finality_update != latest_finality_update { @@ -367,7 +364,7 @@ impl LightClientServerCache { } } - return None; + None } pub fn get_latest_finality_update(&self) -> Option> { @@ -381,8 +378,7 @@ impl LightClientServerCache { &self, ) -> Option> { if let Some(latest_optimistic_update) = self.get_latest_optimistic_update() { - let latest_broadcasted_optimistic_update = - self.latest_optimistic_update.read().clone().take(); + let latest_broadcasted_optimistic_update = self.latest_optimistic_update.read().clone(); match latest_broadcasted_optimistic_update { Some(latest_broadcasted_optimistic_update) => { if latest_broadcasted_optimistic_update != latest_optimistic_update { @@ -399,7 +395,7 @@ impl LightClientServerCache { } } - return None; + None } pub fn get_latest_optimistic_update(&self) -> Option> { From 7b861ac3864d49ac82b749b34c3d0c62ca82d68f Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 22 Jun 2025 17:14:32 +0300 Subject: [PATCH 5/7] / --- beacon_node/http_api/src/sync_committees.rs | 10 ++++------ beacon_node/store/src/hot_cold_store.rs | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/beacon_node/http_api/src/sync_committees.rs b/beacon_node/http_api/src/sync_committees.rs index 755fc3a6e79..57c74f8d019 100644 --- a/beacon_node/http_api/src/sync_committees.rs +++ b/beacon_node/http_api/src/sync_committees.rs @@ -330,9 +330,8 @@ pub fn process_signed_contribution_and_proofs( ) .inspect_err(|e| { error!( - log, - "Unable to broadcast latest light client optimistic update"; - "error" => ?e, + error = ?e, + "Unable to broadcast latest light client optimistic update" ); }); }; @@ -347,9 +346,8 @@ pub fn process_signed_contribution_and_proofs( ) .inspect_err(|e| { error!( - log, - "Unable to broadcast latest light client finality update"; - "error" => ?e, + error = ?e, + "Unable to broadcast latest light client finality update" ); }); }; diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 4d94042b5b0..24a0b39368c 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -3538,7 +3538,7 @@ pub fn get_ancestor_state_root<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStor .get_cold_state_root(target_slot) .map_err(Box::new) .map_err(StateSummaryIteratorError::LoadStateRootError)? - .ok_or_else(|| StateSummaryIteratorError::MissingStateRoot { + .ok_or(StateSummaryIteratorError::MissingStateRoot { target_slot, state_upper_limit, }); From 82ca8fbc9905404253d6eb26c80568a20960db8f Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 2 Jul 2025 19:33:12 +0300 Subject: [PATCH 6/7] ignore rebroadcasting lc data in certain circumstances --- ...ght_client_finality_update_verification.rs | 32 +++++++++++++ ...t_client_optimistic_update_verification.rs | 28 +++++++++++ .../src/light_client_server_cache.rs | 46 ++++++++++++++----- .../gossip_methods.rs | 2 + 4 files changed, 97 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs b/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs index 879fa02f7d9..220ecb4e1de 100644 --- a/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs +++ b/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs @@ -27,6 +27,8 @@ pub enum Error { SigSlotStartIsNone, /// Failed to construct a LightClientFinalityUpdate from state. FailedConstructingUpdate, + /// Silently ignore this light client finality update + Ignore, } /// Wraps a `LightClientFinalityUpdate` that has been verified for propagation on the gossip network. @@ -57,16 +59,46 @@ impl VerifiedLightClientFinalityUpdate { return Err(Error::TooEarly); } + if let Some(latest_broadcasted_finality_update) = chain + .light_client_server_cache + .get_latest_broadcasted_finality_update() + { + // Ignore the incoming finality update if we've already broadcasted it + if latest_broadcasted_finality_update == rcv_finality_update { + return Err(Error::Ignore); + } + + // Ignore the incoming finality update if the latest broadcasted attested header slot + // is greater than the incoming attested header slot. + if latest_broadcasted_finality_update.get_attested_header_slot() + > rcv_finality_update.get_attested_header_slot() + { + return Err(Error::Ignore); + } + } + let latest_finality_update = chain .light_client_server_cache .get_latest_finality_update() .ok_or(Error::FailedConstructingUpdate)?; + // Ignore the incoming finality update if the latest constructed attested header slot + // is greater than the incoming attested header slot. + if latest_finality_update.get_attested_header_slot() + > rcv_finality_update.get_attested_header_slot() + { + return Err(Error::Ignore); + } + // verify that the gossiped finality update is the same as the locally constructed one. if latest_finality_update != rcv_finality_update { return Err(Error::InvalidLightClientFinalityUpdate); } + chain + .light_client_server_cache + .set_latest_broadcasted_finality_update(rcv_finality_update.clone()); + Ok(Self { light_client_finality_update: rcv_finality_update, seen_timestamp, diff --git a/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs b/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs index 5665adc3ed9..b1e1f7a2179 100644 --- a/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs +++ b/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs @@ -30,6 +30,8 @@ pub enum Error { FailedConstructingUpdate, /// Unknown block with parent root. UnknownBlockParentRoot(Hash256), + /// Silently ignore this light client optimistic update + Ignore, } /// Wraps a `LightClientOptimisticUpdate` that has been verified for propagation on the gossip network. @@ -61,6 +63,22 @@ impl VerifiedLightClientOptimisticUpdate { return Err(Error::TooEarly); } + if let Some(latest_broadcasted_optimistic_update) = chain + .light_client_server_cache + .get_latest_broadcasted_optimistic_update() + { + // Ignore the incoming optimistic update if we've already broadcasted it + if latest_broadcasted_optimistic_update == rcv_optimistic_update { + return Err(Error::Ignore); + } + + // Ignore the incoming optimistic update if the latest broadcasted slot + // is greater than the incoming slot. + if latest_broadcasted_optimistic_update.get_slot() > rcv_optimistic_update.get_slot() { + return Err(Error::Ignore); + } + } + let head = chain.canonical_head.cached_head(); let head_block = &head.snapshot.beacon_block; // check if we can process the optimistic update immediately @@ -76,11 +94,21 @@ impl VerifiedLightClientOptimisticUpdate { .get_latest_optimistic_update() .ok_or(Error::FailedConstructingUpdate)?; + // Ignore the incoming optimistic update if the latest constructed slot + // is greater than the incoming slot. + if latest_optimistic_update.get_slot() > rcv_optimistic_update.get_slot() { + return Err(Error::Ignore); + } + // verify that the gossiped optimistic update is the same as the locally constructed one. if latest_optimistic_update != rcv_optimistic_update { return Err(Error::InvalidLightClientOptimisticUpdate); } + chain + .light_client_server_cache + .set_latest_broadcasted_optimistic_update(rcv_optimistic_update.clone()); + let parent_root = rcv_optimistic_update.get_parent_root(); Ok(Self { light_client_optimistic_update: rcv_optimistic_update, diff --git a/beacon_node/beacon_chain/src/light_client_server_cache.rs b/beacon_node/beacon_chain/src/light_client_server_cache.rs index 74bc9faee4e..52b7fa16dbb 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -347,19 +347,16 @@ impl LightClientServerCache { &self, ) -> Option> { if let Some(latest_finality_update) = self.get_latest_finality_update() { - let latest_broadcasted_finality_update = - self.latest_broadcasted_finality_update.read().clone(); + let latest_broadcasted_finality_update = self.get_latest_broadcasted_finality_update(); match latest_broadcasted_finality_update { Some(latest_broadcasted_finality_update) => { if latest_broadcasted_finality_update != latest_finality_update { - *self.latest_broadcasted_finality_update.write() = - Some(latest_finality_update.clone()); + self.set_latest_broadcasted_finality_update(latest_finality_update.clone()); return Some(latest_finality_update); } } None => { - *self.latest_broadcasted_finality_update.write() = - Some(latest_finality_update.clone()); + self.set_latest_broadcasted_finality_update(latest_finality_update.clone()); return Some(latest_finality_update); } } @@ -372,6 +369,32 @@ impl LightClientServerCache { self.latest_finality_update.read().clone() } + pub fn get_latest_broadcasted_optimistic_update( + &self, + ) -> Option> { + self.latest_broadcasted_optimistic_update.read().clone() + } + + pub fn get_latest_broadcasted_finality_update( + &self, + ) -> Option> { + self.latest_broadcasted_finality_update.read().clone() + } + + pub fn set_latest_broadcasted_optimistic_update( + &self, + optimistic_update: LightClientOptimisticUpdate, + ) { + *self.latest_broadcasted_optimistic_update.write() = Some(optimistic_update.clone()); + } + + pub fn set_latest_broadcasted_finality_update( + &self, + finality_update: LightClientFinalityUpdate, + ) { + *self.latest_broadcasted_finality_update.write() = Some(finality_update.clone()); + } + /// Checks if we've already broadcasted the latest optimistic update. /// If we haven't, update the `latest_broadcasted_optimistic_update` cache /// and return the latest optimistic update for broadcasting, else return `None`. @@ -379,18 +402,19 @@ impl LightClientServerCache { &self, ) -> Option> { if let Some(latest_optimistic_update) = self.get_latest_optimistic_update() { - let latest_broadcasted_optimistic_update = self.latest_optimistic_update.read().clone(); + let latest_broadcasted_optimistic_update = + self.get_latest_broadcasted_optimistic_update(); match latest_broadcasted_optimistic_update { Some(latest_broadcasted_optimistic_update) => { if latest_broadcasted_optimistic_update != latest_optimistic_update { - *self.latest_broadcasted_optimistic_update.write() = - Some(latest_optimistic_update.clone()); + self.set_latest_broadcasted_optimistic_update( + latest_optimistic_update.clone(), + ); return Some(latest_optimistic_update); } } None => { - *self.latest_broadcasted_optimistic_update.write() = - Some(latest_optimistic_update.clone()); + self.set_latest_broadcasted_optimistic_update(latest_optimistic_update.clone()); return Some(latest_optimistic_update); } } 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 fcab34c93e6..b2ab5259bda 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -1998,6 +1998,7 @@ impl NetworkBeaconProcessor { error = ?e, "Light client error constructing finality update" ), + LightClientFinalityUpdateError::Ignore => {} } self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); } @@ -2118,6 +2119,7 @@ impl NetworkBeaconProcessor { "Light client error constructing optimistic update" ) } + LightClientOptimisticUpdateError::Ignore => {} } self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); } From 67785c7667ffbdaf22925e05a20dc2a67bafdfde Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 8 Jul 2025 11:39:51 +1000 Subject: [PATCH 7/7] Add more detail to LC errors --- ...ght_client_finality_update_verification.rs | 65 +++++++++++++++++-- ...t_client_optimistic_update_verification.rs | 45 +++++++++++-- beacon_node/http_api/src/lib.rs | 2 +- .../gossip_methods.rs | 9 ++- .../types/src/light_client_finality_update.rs | 17 ++++- .../src/light_client_optimistic_update.rs | 3 +- testing/simulator/src/checks.rs | 4 +- 7 files changed, 126 insertions(+), 19 deletions(-) diff --git a/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs b/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs index 220ecb4e1de..0d5a5425d5c 100644 --- a/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs +++ b/beacon_node/beacon_chain/src/light_client_finality_update_verification.rs @@ -3,7 +3,7 @@ use derivative::Derivative; use slot_clock::SlotClock; use std::time::Duration; use strum::AsRefStr; -use types::LightClientFinalityUpdate; +use types::{Hash256, LightClientFinalityUpdate, Slot}; /// Returned when a light client finality update was not successfully verified. It might not have been verified for /// two reasons: @@ -21,8 +21,31 @@ pub enum Error { /// /// Assuming the local clock is correct, the peer has sent an invalid message. TooEarly, - /// Light client finality update message does not match the locally constructed one. - InvalidLightClientFinalityUpdate, + /// Light client finalized update message does not match the locally constructed one, it has a + /// different signature slot. + MismatchedSignatureSlot { local: Slot, observed: Slot }, + /// Light client finalized update message does not match the locally constructed one, it has a + /// different finalized block header for the same signature slot. + MismatchedFinalizedHeader { + local_finalized_header_root: Hash256, + observed_finalized_header_root: Hash256, + signature_slot: Slot, + }, + /// Light client finalized update message does not match the locally constructed one, it has a + /// different attested block header for the same signature slot and finalized header. + MismatchedAttestedHeader { + local_attested_header_root: Hash256, + observed_attested_header_root: Hash256, + finalized_header_root: Hash256, + signature_slot: Slot, + }, + /// Light client finalized update message does not match the locally constructed one, it has a + /// different proof or sync aggregate for the same slot, attested header and finalized header. + MismatchedProofOrSyncAggregate { + attested_header_root: Hash256, + finalized_header_root: Hash256, + signature_slot: Slot, + }, /// Signature slot start time is none. SigSlotStartIsNone, /// Failed to construct a LightClientFinalityUpdate from state. @@ -50,7 +73,7 @@ impl VerifiedLightClientFinalityUpdate { // verify that enough time has passed for the block to have been propagated let start_time = chain .slot_clock - .start_of(*rcv_finality_update.signature_slot()) + .start_of(rcv_finality_update.signature_slot()) .ok_or(Error::SigSlotStartIsNone)?; let one_third_slot_duration = Duration::new(chain.spec.seconds_per_slot / 3, 0); if seen_timestamp + chain.spec.maximum_gossip_clock_disparity() @@ -90,9 +113,39 @@ impl VerifiedLightClientFinalityUpdate { return Err(Error::Ignore); } - // verify that the gossiped finality update is the same as the locally constructed one. + // Verify that the gossiped finality update is the same as the locally constructed one. if latest_finality_update != rcv_finality_update { - return Err(Error::InvalidLightClientFinalityUpdate); + let signature_slot = latest_finality_update.signature_slot(); + if signature_slot != rcv_finality_update.signature_slot() { + return Err(Error::MismatchedSignatureSlot { + local: signature_slot, + observed: rcv_finality_update.signature_slot(), + }); + } + let local_finalized_header_root = latest_finality_update.get_finalized_header_root(); + let observed_finalized_header_root = rcv_finality_update.get_finalized_header_root(); + if local_finalized_header_root != observed_finalized_header_root { + return Err(Error::MismatchedFinalizedHeader { + local_finalized_header_root, + observed_finalized_header_root, + signature_slot, + }); + } + let local_attested_header_root = latest_finality_update.get_attested_header_root(); + let observed_attested_header_root = rcv_finality_update.get_attested_header_root(); + if local_attested_header_root != observed_attested_header_root { + return Err(Error::MismatchedAttestedHeader { + local_attested_header_root, + observed_attested_header_root, + finalized_header_root: local_finalized_header_root, + signature_slot, + }); + } + return Err(Error::MismatchedProofOrSyncAggregate { + attested_header_root: local_attested_header_root, + finalized_header_root: local_finalized_header_root, + signature_slot, + }); } chain diff --git a/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs b/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs index b1e1f7a2179..4da69134433 100644 --- a/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs +++ b/beacon_node/beacon_chain/src/light_client_optimistic_update_verification.rs @@ -4,7 +4,7 @@ use eth2::types::Hash256; use slot_clock::SlotClock; use std::time::Duration; use strum::AsRefStr; -use types::LightClientOptimisticUpdate; +use types::{LightClientOptimisticUpdate, Slot}; /// Returned when a light client optimistic update was not successfully verified. It might not have been verified for /// two reasons: @@ -22,8 +22,22 @@ pub enum Error { /// /// Assuming the local clock is correct, the peer has sent an invalid message. TooEarly, - /// Light client optimistic update message does not match the locally constructed one. - InvalidLightClientOptimisticUpdate, + /// Light client optimistic update message does not match the locally constructed one, it has a + /// different signature slot. + MismatchedSignatureSlot { local: Slot, observed: Slot }, + /// Light client optimistic update message does not match the locally constructed one, it has a + /// different block header at the same slot. + MismatchedAttestedHeader { + local_attested_header_root: Hash256, + observed_attested_header_root: Hash256, + signature_slot: Slot, + }, + /// Light client optimistic update message does not match the locally constructed one, it has a + /// different sync aggregate for the same slot and attested header. + MismatchedSyncAggregate { + attested_header_root: Hash256, + signature_slot: Slot, + }, /// Signature slot start time is none. SigSlotStartIsNone, /// Failed to construct a LightClientOptimisticUpdate from state. @@ -54,7 +68,7 @@ impl VerifiedLightClientOptimisticUpdate { // verify that enough time has passed for the block to have been propagated let start_time = chain .slot_clock - .start_of(*rcv_optimistic_update.signature_slot()) + .start_of(rcv_optimistic_update.signature_slot()) .ok_or(Error::SigSlotStartIsNone)?; let one_third_slot_duration = Duration::new(chain.spec.seconds_per_slot / 3, 0); if seen_timestamp + chain.spec.maximum_gossip_clock_disparity() @@ -100,9 +114,28 @@ impl VerifiedLightClientOptimisticUpdate { return Err(Error::Ignore); } - // verify that the gossiped optimistic update is the same as the locally constructed one. + // Verify that the gossiped optimistic update is the same as the locally constructed one. if latest_optimistic_update != rcv_optimistic_update { - return Err(Error::InvalidLightClientOptimisticUpdate); + let signature_slot = latest_optimistic_update.signature_slot(); + if signature_slot != rcv_optimistic_update.signature_slot() { + return Err(Error::MismatchedSignatureSlot { + local: signature_slot, + observed: rcv_optimistic_update.signature_slot(), + }); + } + let local_attested_header_root = latest_optimistic_update.get_canonical_root(); + let observed_attested_header_root = rcv_optimistic_update.get_canonical_root(); + if local_attested_header_root != observed_attested_header_root { + return Err(Error::MismatchedAttestedHeader { + local_attested_header_root, + observed_attested_header_root, + signature_slot, + }); + } + return Err(Error::MismatchedSyncAggregate { + attested_header_root: local_attested_header_root, + signature_slot, + }); } chain diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 73b20197c13..bcdec6f49bb 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -2634,7 +2634,7 @@ pub fn serve( let fork_name = chain .spec - .fork_name_at_slot::(*update.signature_slot()); + .fork_name_at_slot::(update.signature_slot()); match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) 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 b2ab5259bda..2a78852428b 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -1966,7 +1966,10 @@ impl NetworkBeaconProcessor { Err(e) => { metrics::register_finality_update_error(&e); match e { - LightClientFinalityUpdateError::InvalidLightClientFinalityUpdate => { + LightClientFinalityUpdateError::MismatchedSignatureSlot { .. } + | LightClientFinalityUpdateError::MismatchedAttestedHeader { .. } + | LightClientFinalityUpdateError::MismatchedFinalizedHeader { .. } + | LightClientFinalityUpdateError::MismatchedProofOrSyncAggregate { .. } => { debug!( %peer_id, error = ?e, @@ -2080,7 +2083,9 @@ impl NetworkBeaconProcessor { } return; } - LightClientOptimisticUpdateError::InvalidLightClientOptimisticUpdate => { + LightClientOptimisticUpdateError::MismatchedSignatureSlot { .. } + | LightClientOptimisticUpdateError::MismatchedAttestedHeader { .. } + | LightClientOptimisticUpdateError::MismatchedSyncAggregate { .. } => { metrics::register_optimistic_update_error(&e); debug!( diff --git a/consensus/types/src/light_client_finality_update.rs b/consensus/types/src/light_client_finality_update.rs index 9189dcd0a0b..2125b4668b4 100644 --- a/consensus/types/src/light_client_finality_update.rs +++ b/consensus/types/src/light_client_finality_update.rs @@ -79,6 +79,7 @@ pub struct LightClientFinalityUpdate { /// current sync aggregate pub sync_aggregate: SyncAggregate, /// Slot of the sync aggregated signature + #[superstruct(getter(copy))] pub signature_slot: Slot, } @@ -179,6 +180,20 @@ impl LightClientFinalityUpdate { }) } + pub fn get_attested_header_root<'a>(&'a self) -> Hash256 { + map_light_client_finality_update_ref!(&'a _, self.to_ref(), |inner, cons| { + cons(inner); + inner.attested_header.beacon.canonical_root() + }) + } + + pub fn get_finalized_header_root<'a>(&'a self) -> Hash256 { + map_light_client_finality_update_ref!(&'a _, self.to_ref(), |inner, cons| { + cons(inner); + inner.finalized_header.beacon.canonical_root() + }) + } + pub fn from_ssz_bytes(bytes: &[u8], fork_name: ForkName) -> Result { let finality_update = match fork_name { ForkName::Altair | ForkName::Bellatrix => { @@ -227,7 +242,7 @@ impl LightClientFinalityUpdate { if attested_slot > prev_slot { true } else { - attested_slot == prev_slot && signature_slot > *self.signature_slot() + attested_slot == prev_slot && signature_slot > self.signature_slot() } } } diff --git a/consensus/types/src/light_client_optimistic_update.rs b/consensus/types/src/light_client_optimistic_update.rs index 5701ebd8751..13e308cd278 100644 --- a/consensus/types/src/light_client_optimistic_update.rs +++ b/consensus/types/src/light_client_optimistic_update.rs @@ -60,6 +60,7 @@ pub struct LightClientOptimisticUpdate { /// current sync aggregate pub sync_aggregate: SyncAggregate, /// Slot of the sync aggregated signature + #[superstruct(getter(copy))] pub signature_slot: Slot, } @@ -200,7 +201,7 @@ impl LightClientOptimisticUpdate { if attested_slot > prev_slot { true } else { - attested_slot == prev_slot && signature_slot > *self.signature_slot() + attested_slot == prev_slot && signature_slot > self.signature_slot() } } } diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index 1b2d4024d11..e7cc9b7a4ee 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -303,7 +303,7 @@ pub(crate) async fn verify_light_client_updates( } // Verify light client optimistic update. `signature_slot_distance` should be 1 in the ideal scenario. - let signature_slot = *client + let signature_slot = client .get_beacon_light_client_optimistic_update::() .await .map_err(|e| format!("Error while getting light client updates: {:?}", e))? @@ -332,7 +332,7 @@ pub(crate) async fn verify_light_client_updates( } continue; } - let signature_slot = *client + let signature_slot = client .get_beacon_light_client_finality_update::() .await .map_err(|e| format!("Error while getting light client updates: {:?}", e))?