From 914bb1ed221353509640a6285879c885e285cf0a Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 2 Apr 2024 22:45:59 +0300 Subject: [PATCH 01/42] persist light client updates --- .../src/light_client_server_cache.rs | 57 +++++++++++++++++-- 1 file changed, 53 insertions(+), 4 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 d480a6b56b2..5259203aed8 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -7,8 +7,8 @@ use std::num::NonZeroUsize; use types::light_client_update::{FinalizedRootProofLen, FINALIZED_ROOT_INDEX}; use types::non_zero_usize::new_non_zero_usize; use types::{ - BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, - LightClientOptimisticUpdate, Slot, SyncAggregate, + BeaconBlock, BeaconBlockRef, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, Hash256, + LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, }; /// A prev block cache miss requires to re-generate the state of the post-parent block. Items in the @@ -32,6 +32,8 @@ pub struct LightClientServerCache { latest_optimistic_update: RwLock>>, /// Caches state proofs by block root prev_block_cache: Mutex>, + /// Caches `LightClientUpdate`'s by slot + light_client_updates_cache: Mutex>>, } impl LightClientServerCache { @@ -40,6 +42,8 @@ impl LightClientServerCache { latest_finality_update: None.into(), latest_optimistic_update: None.into(), prev_block_cache: lru::LruCache::new(PREV_BLOCK_CACHE_SIZE).into(), + // TODO(lightclient-network) + light_client_updates_cache: lru::LruCache::new(new_non_zero_usize(128)).into(), } } @@ -73,7 +77,7 @@ impl LightClientServerCache { &self, store: BeaconStore, block_parent_root: &Hash256, - block_slot: Slot, + block: BeaconBlock, sync_aggregate: &SyncAggregate, log: &Logger, chain_spec: &ChainSpec, @@ -81,7 +85,7 @@ impl LightClientServerCache { let _timer = metrics::start_timer(&metrics::LIGHT_CLIENT_SERVER_CACHE_RECOMPUTE_UPDATES_TIMES); - let signature_slot = block_slot; + let signature_slot = block.slot(); let attested_block_root = block_parent_root; let attested_block = @@ -140,6 +144,32 @@ impl LightClientServerCache { signature_slot, chain_spec, )?); + + let beacon_state = store + .get_state(&block.state_root(), Some(block.slot()))? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Beacon state not available {:?}", + attested_block.state_root() + )))?; + + let mut attested_state = store + .get_state(&attested_block.state_root(), Some(attested_block.slot()))? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Attested state not available {:?}", + attested_block.state_root() + )))?; + + self.light_client_updates_cache.lock().put( + block.slot().into(), + LightClientUpdate::new( + beacon_state, + block, + &mut attested_state, + &attested_block, + &finalized_block, + chain_spec, + )?, + ); } else { debug!( log, @@ -191,6 +221,25 @@ impl LightClientServerCache { pub fn get_latest_optimistic_update(&self) -> Option> { self.latest_optimistic_update.read().clone() } + + pub fn get_light_client_updates( + &self, + sync_committee_period: u64, + count: u64, + ) -> Vec> { + let epoch = Epoch::new(sync_committee_period); + let end_slot = epoch.start_slot(T::EthSpec::slots_per_epoch()); + let start_slot = end_slot - count; + let mut light_client_updates = vec![]; + let mut mutex = self.light_client_updates_cache.lock(); + for slot in start_slot.as_u64()..=end_slot.as_u64() { + if let Some(light_client_update) = mutex.get(&slot) { + light_client_updates.push(light_client_update.clone()); + } + } + + light_client_updates + } } impl Default for LightClientServerCache { From c308768c31136960639885aba596f718d45da37a Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 2 Apr 2024 22:49:01 +0300 Subject: [PATCH 02/42] update beacon chain to serve light client updates --- beacon_node/beacon_chain/src/beacon_chain.rs | 40 +++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index deeda491dca..e09f5742c27 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1346,16 +1346,54 @@ impl BeaconChain { &self, (parent_root, slot, sync_aggregate): LightClientProducerEvent, ) -> Result<(), Error> { + let block_root = self + .block_root_at_slot(slot, WhenSlotSkipped::Prev)? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Block root not available {:?}", + slot + )))?; + + let (block, _) = self + .store + .get_full_block(&block_root)? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Block not available {:?}", + slot + )))? + .deconstruct(); + self.light_client_server_cache.recompute_and_cache_updates( self.store.clone(), &parent_root, - slot, + block, &sync_aggregate, &self.log, &self.spec, ) } + pub fn get_latest_light_client_finality_update( + &self, + ) -> Option> { + self.light_client_server_cache.get_latest_finality_update() + } + + pub fn get_latest_light_client_optimistic_update( + &self, + ) -> Option> { + self.light_client_server_cache + .get_latest_optimistic_update() + } + + pub fn get_light_client_updates( + &self, + sync_committee_period: u64, + count: u64, + ) -> Vec> { + self.light_client_server_cache + .get_light_client_updates(sync_committee_period, count) + } + /// Returns the current heads of the `BeaconChain`. For the canonical head, see `Self::head`. /// /// Returns `(block_root, block_slot)`. From e332b009f546c0ee3701cd5e65c235dfa08a750f Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 2 Apr 2024 23:45:59 +0300 Subject: [PATCH 03/42] resolve todos --- beacon_node/beacon_chain/src/beacon_chain.rs | 23 ++++--------------- .../src/light_client_server_cache.rs | 17 ++++++++++---- consensus/types/src/light_client_update.rs | 1 + 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index e09f5742c27..6f6d5cbb854 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -339,7 +339,7 @@ struct PartialBeaconBlock { bls_to_execution_changes: Vec, } -pub type LightClientProducerEvent = (Hash256, Slot, SyncAggregate); +pub type LightClientProducerEvent = (Hash256, Hash256, Slot, SyncAggregate); pub type BeaconForkChoice = ForkChoice< BeaconForkChoiceStore< @@ -1344,28 +1344,12 @@ impl BeaconChain { pub fn recompute_and_cache_light_client_updates( &self, - (parent_root, slot, sync_aggregate): LightClientProducerEvent, + (block_root, parent_root, slot, sync_aggregate): LightClientProducerEvent, ) -> Result<(), Error> { - let block_root = self - .block_root_at_slot(slot, WhenSlotSkipped::Prev)? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Block root not available {:?}", - slot - )))?; - - let (block, _) = self - .store - .get_full_block(&block_root)? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Block not available {:?}", - slot - )))? - .deconstruct(); - self.light_client_server_cache.recompute_and_cache_updates( self.store.clone(), + &block_root, &parent_root, - block, &sync_aggregate, &self.log, &self.spec, @@ -3962,6 +3946,7 @@ impl BeaconChain { if let Some(mut light_client_server_tx) = self.light_client_server_tx.clone() { if let Ok(sync_aggregate) = block.body().sync_aggregate() { if let Err(e) = light_client_server_tx.try_send(( + block.body_root(), block.parent_root(), block.slot(), sync_aggregate.clone(), 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 5259203aed8..9e32221ff00 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -7,7 +7,7 @@ use std::num::NonZeroUsize; use types::light_client_update::{FinalizedRootProofLen, FINALIZED_ROOT_INDEX}; use types::non_zero_usize::new_non_zero_usize; use types::{ - BeaconBlock, BeaconBlockRef, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, Hash256, + BeaconBlockRef, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, }; @@ -16,6 +16,8 @@ use types::{ /// represents unlikely re-orgs, while keeping the cache very small. const PREV_BLOCK_CACHE_SIZE: NonZeroUsize = new_non_zero_usize(32); +pub const MAX_REQUEST_LIGHT_CLIENT_UPDATES: NonZeroUsize = new_non_zero_usize(128); + /// This cache computes light client messages ahead of time, required to satisfy p2p and API /// requests. These messages include proofs on historical states, so on-demand computation is /// expensive. @@ -42,8 +44,7 @@ impl LightClientServerCache { latest_finality_update: None.into(), latest_optimistic_update: None.into(), prev_block_cache: lru::LruCache::new(PREV_BLOCK_CACHE_SIZE).into(), - // TODO(lightclient-network) - light_client_updates_cache: lru::LruCache::new(new_non_zero_usize(128)).into(), + light_client_updates_cache: lru::LruCache::new(MAX_REQUEST_LIGHT_CLIENT_UPDATES).into(), } } @@ -76,8 +77,8 @@ impl LightClientServerCache { pub fn recompute_and_cache_updates( &self, store: BeaconStore, + block_root: &Hash256, block_parent_root: &Hash256, - block: BeaconBlock, sync_aggregate: &SyncAggregate, log: &Logger, chain_spec: &ChainSpec, @@ -85,6 +86,14 @@ impl LightClientServerCache { let _timer = metrics::start_timer(&metrics::LIGHT_CLIENT_SERVER_CACHE_RECOMPUTE_UPDATES_TIMES); + let (block, _) = store + .get_full_block(&block_root)? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Block not available {:?}", + block_root + )))? + .deconstruct(); + let signature_slot = block.slot(); let attested_block_root = block_parent_root; diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 09cc1950990..1274919222b 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -22,6 +22,7 @@ pub const FINALIZED_ROOT_INDEX: usize = 105; pub const CURRENT_SYNC_COMMITTEE_INDEX: usize = 54; pub const NEXT_SYNC_COMMITTEE_INDEX: usize = 55; pub const EXECUTION_PAYLOAD_INDEX: usize = 25; +pub const MAX_REQUEST_LIGHT_CLIENT_UPDATES: NonZeroUsize = new_non_zero_usize(128); pub type FinalizedRootProofLen = U6; pub type CurrentSyncCommitteeProofLen = U5; From 3a93933b20d2127ae00ed767bf040f865af2279c Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 3 Apr 2024 13:48:59 +0300 Subject: [PATCH 04/42] cache best update --- beacon_node/beacon_chain/src/beacon_chain.rs | 1 + .../src/light_client_server_cache.rs | 131 ++++++++++++------ consensus/types/src/light_client_update.rs | 1 - 3 files changed, 88 insertions(+), 45 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 6f6d5cbb854..756dc2eff6f 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1349,6 +1349,7 @@ impl BeaconChain { self.light_client_server_cache.recompute_and_cache_updates( self.store.clone(), &block_root, + slot, &parent_root, &sync_aggregate, &self.log, 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 9e32221ff00..5bf4d523cda 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -7,8 +7,8 @@ use std::num::NonZeroUsize; use types::light_client_update::{FinalizedRootProofLen, FINALIZED_ROOT_INDEX}; use types::non_zero_usize::new_non_zero_usize; use types::{ - BeaconBlockRef, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, Hash256, - LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, + BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, + LightClientOptimisticUpdate, LightClientUpdate, SignedBeaconBlock, Slot, SyncAggregate, }; /// A prev block cache miss requires to re-generate the state of the post-parent block. Items in the @@ -16,7 +16,8 @@ use types::{ /// represents unlikely re-orgs, while keeping the cache very small. const PREV_BLOCK_CACHE_SIZE: NonZeroUsize = new_non_zero_usize(32); -pub const MAX_REQUEST_LIGHT_CLIENT_UPDATES: NonZeroUsize = new_non_zero_usize(128); +// TODO(lightclientupdate) we want to cache 6 months worth of light client updates +pub const LIGHT_CLIENT_UPDATES_CACHE_SIZE: NonZeroUsize = new_non_zero_usize(180); /// This cache computes light client messages ahead of time, required to satisfy p2p and API /// requests. These messages include proofs on historical states, so on-demand computation is @@ -34,7 +35,7 @@ pub struct LightClientServerCache { latest_optimistic_update: RwLock>>, /// Caches state proofs by block root prev_block_cache: Mutex>, - /// Caches `LightClientUpdate`'s by slot + /// Caches `LightClientUpdate`'s by sync committee period light_client_updates_cache: Mutex>>, } @@ -44,7 +45,7 @@ impl LightClientServerCache { latest_finality_update: None.into(), latest_optimistic_update: None.into(), prev_block_cache: lru::LruCache::new(PREV_BLOCK_CACHE_SIZE).into(), - light_client_updates_cache: lru::LruCache::new(MAX_REQUEST_LIGHT_CLIENT_UPDATES).into(), + light_client_updates_cache: lru::LruCache::new(LIGHT_CLIENT_UPDATES_CACHE_SIZE).into(), } } @@ -72,12 +73,14 @@ impl LightClientServerCache { Ok(()) } - /// Given a block with a SyncAggregte computes better or more recent light client updates. The + /// Given a block with a SyncAggregate computes better or more recent light client updates. The /// results are cached either on disk or memory to be served via p2p and rest API + #[allow(clippy::too_many_arguments)] pub fn recompute_and_cache_updates( &self, store: BeaconStore, block_root: &Hash256, + block_slot: Slot, block_parent_root: &Hash256, sync_aggregate: &SyncAggregate, log: &Logger, @@ -86,15 +89,7 @@ impl LightClientServerCache { let _timer = metrics::start_timer(&metrics::LIGHT_CLIENT_SERVER_CACHE_RECOMPUTE_UPDATES_TIMES); - let (block, _) = store - .get_full_block(&block_root)? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Block not available {:?}", - block_root - )))? - .deconstruct(); - - let signature_slot = block.slot(); + let signature_slot = block_slot; let attested_block_root = block_parent_root; let attested_block = @@ -154,31 +149,13 @@ impl LightClientServerCache { chain_spec, )?); - let beacon_state = store - .get_state(&block.state_root(), Some(block.slot()))? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Beacon state not available {:?}", - attested_block.state_root() - )))?; - - let mut attested_state = store - .get_state(&attested_block.state_root(), Some(attested_block.slot()))? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Attested state not available {:?}", - attested_block.state_root() - )))?; - - self.light_client_updates_cache.lock().put( - block.slot().into(), - LightClientUpdate::new( - beacon_state, - block, - &mut attested_state, - &attested_block, - &finalized_block, - chain_spec, - )?, - ); + self.persist_best_light_client_update( + &store, + block_root, + attested_block, + finalized_block, + chain_spec, + )?; } else { debug!( log, @@ -191,6 +168,75 @@ impl LightClientServerCache { Ok(()) } + // - Is finalized + // - Has the most bits + // - Signed header at the oldest slot + fn persist_best_light_client_update( + &self, + store: &BeaconStore, + block_root: &Hash256, + attested_block: SignedBeaconBlock<::EthSpec>, + finalized_block: SignedBeaconBlock<::EthSpec>, + chain_spec: &ChainSpec, + ) -> Result<(), BeaconChainError> { + let (block, _) = store + .get_full_block(block_root)? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Block not available {:?}", + block_root + )))? + .deconstruct(); + + let beacon_state = store + .get_state(&block.state_root(), Some(block.slot()))? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Beacon state not available {:?}", + attested_block.state_root() + )))?; + + let mut attested_state = store + .get_state(&attested_block.state_root(), Some(attested_block.slot()))? + .ok_or(BeaconChainError::DBInconsistent(format!( + "Attested state not available {:?}", + attested_block.state_root() + )))?; + + let sync_period = block.epoch().sync_committee_period(chain_spec)?; + let new_light_client_update = LightClientUpdate::new( + beacon_state, + block, + &mut attested_state, + &attested_block, + &finalized_block, + chain_spec, + )?; + + let mut mutex = self.light_client_updates_cache.lock(); + + if let Some(existing_light_client_update) = mutex.get(&sync_period) { + let new_sync_aggregate_bits = new_light_client_update.sync_aggregate().num_set_bits(); + let new_signature_slot = new_light_client_update.signature_slot(); + + let existing_sync_aggregate_bits = + existing_light_client_update.sync_aggregate().num_set_bits(); + let existing_signature_slot = existing_light_client_update.signature_slot(); + + if new_sync_aggregate_bits > existing_sync_aggregate_bits + && new_signature_slot < existing_signature_slot + { + self.light_client_updates_cache + .lock() + .put(sync_period, new_light_client_update); + } + } else { + self.light_client_updates_cache + .lock() + .put(sync_period, new_light_client_update); + } + + Ok(()) + } + /// Retrieves prev block cached data from cache. If not present re-computes by retrieving the /// parent state, and inserts an entry to the cache. /// @@ -236,12 +282,9 @@ impl LightClientServerCache { sync_committee_period: u64, count: u64, ) -> Vec> { - let epoch = Epoch::new(sync_committee_period); - let end_slot = epoch.start_slot(T::EthSpec::slots_per_epoch()); - let start_slot = end_slot - count; let mut light_client_updates = vec![]; let mut mutex = self.light_client_updates_cache.lock(); - for slot in start_slot.as_u64()..=end_slot.as_u64() { + for slot in sync_committee_period..=sync_committee_period + count { if let Some(light_client_update) = mutex.get(&slot) { light_client_updates.push(light_client_update.clone()); } diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 1274919222b..09cc1950990 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -22,7 +22,6 @@ pub const FINALIZED_ROOT_INDEX: usize = 105; pub const CURRENT_SYNC_COMMITTEE_INDEX: usize = 54; pub const NEXT_SYNC_COMMITTEE_INDEX: usize = 55; pub const EXECUTION_PAYLOAD_INDEX: usize = 25; -pub const MAX_REQUEST_LIGHT_CLIENT_UPDATES: NonZeroUsize = new_non_zero_usize(128); pub type FinalizedRootProofLen = U6; pub type CurrentSyncCommitteeProofLen = U5; From b9bdd887ae6f9790256f8ebbf33f5900bb330ce1 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 10 Apr 2024 14:06:50 +0300 Subject: [PATCH 05/42] extend cache parts --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 +- .../src/light_client_server_cache.rs | 79 ++++++++++--------- consensus/types/src/light_client_update.rs | 70 +++++----------- 3 files changed, 64 insertions(+), 91 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 756dc2eff6f..9836351dc3c 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -339,7 +339,7 @@ struct PartialBeaconBlock { bls_to_execution_changes: Vec, } -pub type LightClientProducerEvent = (Hash256, Hash256, Slot, SyncAggregate); +pub type LightClientProducerEvent = (Hash256, Slot, SyncAggregate); pub type BeaconForkChoice = ForkChoice< BeaconForkChoiceStore< @@ -1344,11 +1344,10 @@ impl BeaconChain { pub fn recompute_and_cache_light_client_updates( &self, - (block_root, parent_root, slot, sync_aggregate): LightClientProducerEvent, + (parent_root, slot, sync_aggregate): LightClientProducerEvent, ) -> Result<(), Error> { self.light_client_server_cache.recompute_and_cache_updates( self.store.clone(), - &block_root, slot, &parent_root, &sync_aggregate, @@ -3947,7 +3946,6 @@ impl BeaconChain { if let Some(mut light_client_server_tx) = self.light_client_server_tx.clone() { if let Ok(sync_aggregate) = block.body().sync_aggregate() { if let Err(e) = light_client_server_tx.try_send(( - block.body_root(), block.parent_root(), block.slot(), sync_aggregate.clone(), 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 5bf4d523cda..9ea5ff004ff 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -4,16 +4,23 @@ use parking_lot::{Mutex, RwLock}; use slog::{debug, Logger}; use ssz_types::FixedVector; use std::num::NonZeroUsize; -use types::light_client_update::{FinalizedRootProofLen, FINALIZED_ROOT_INDEX}; +use std::sync::Arc; +use tree_hash::TreeHash; +use types::light_client_update::{ + FinalizedRootProofLen, NextSyncCommitteeProofLen, FINALIZED_ROOT_INDEX, + NEXT_SYNC_COMMITTEE_INDEX, +}; use types::non_zero_usize::new_non_zero_usize; use types::{ BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, SignedBeaconBlock, Slot, SyncAggregate, + SyncCommittee, }; /// A prev block cache miss requires to re-generate the state of the post-parent block. Items in the /// prev block cache are very small 32 * (6 + 1) = 224 bytes. 32 is an arbitrary number that /// represents unlikely re-orgs, while keeping the cache very small. +// TODO(lightclientupdate) this cache size has now increased const PREV_BLOCK_CACHE_SIZE: NonZeroUsize = new_non_zero_usize(32); // TODO(lightclientupdate) we want to cache 6 months worth of light client updates @@ -34,7 +41,7 @@ pub struct LightClientServerCache { /// Tracks a single global latest optimistic update out of all imported blocks. latest_optimistic_update: RwLock>>, /// Caches state proofs by block root - prev_block_cache: Mutex>, + prev_block_cache: Mutex>>, /// Caches `LightClientUpdate`'s by sync committee period light_client_updates_cache: Mutex>>, } @@ -79,7 +86,6 @@ impl LightClientServerCache { pub fn recompute_and_cache_updates( &self, store: BeaconStore, - block_root: &Hash256, block_slot: Slot, block_parent_root: &Hash256, sync_aggregate: &SyncAggregate, @@ -150,10 +156,11 @@ impl LightClientServerCache { )?); self.persist_best_light_client_update( - &store, - block_root, + store, + block_slot, attested_block, finalized_block, + sync_aggregate, chain_spec, )?; } else { @@ -173,39 +180,30 @@ impl LightClientServerCache { // - Signed header at the oldest slot fn persist_best_light_client_update( &self, - store: &BeaconStore, - block_root: &Hash256, + store: BeaconStore, + block_slot: Slot, attested_block: SignedBeaconBlock<::EthSpec>, finalized_block: SignedBeaconBlock<::EthSpec>, + sync_aggregate: &SyncAggregate, chain_spec: &ChainSpec, ) -> Result<(), BeaconChainError> { - let (block, _) = store - .get_full_block(block_root)? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Block not available {:?}", - block_root - )))? - .deconstruct(); - - let beacon_state = store - .get_state(&block.state_root(), Some(block.slot()))? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Beacon state not available {:?}", - attested_block.state_root() - )))?; - - let mut attested_state = store - .get_state(&attested_block.state_root(), Some(attested_block.slot()))? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Attested state not available {:?}", - attested_block.state_root() - )))?; - - let sync_period = block.epoch().sync_committee_period(chain_spec)?; + let cached_parts = self.get_or_compute_prev_block_cache( + store, + &attested_block.tree_hash_root(), + &attested_block.state_root(), + attested_block.slot(), + )?; + + let sync_period = block_slot + .epoch(T::EthSpec::slots_per_epoch()) + .sync_committee_period(chain_spec)?; + let new_light_client_update = LightClientUpdate::new( - beacon_state, - block, - &mut attested_state, + sync_aggregate, + block_slot, + cached_parts.next_sync_committee, + cached_parts.next_sync_committee_branch, + cached_parts.finality_branch, &attested_block, &finalized_block, chain_spec, @@ -247,7 +245,7 @@ impl LightClientServerCache { block_root: &Hash256, block_state_root: &Hash256, block_slot: Slot, - ) -> Result { + ) -> Result, BeaconChainError> { // Attempt to get the value from the cache first. if let Some(cached_parts) = self.prev_block_cache.lock().get(block_root) { return Ok(cached_parts.clone()); @@ -301,17 +299,24 @@ impl Default for LightClientServerCache { } type FinalityBranch = FixedVector; +type NextSyncCommitteeBranch = FixedVector; #[derive(Clone)] -struct LightClientCachedData { +struct LightClientCachedData { finality_branch: FinalityBranch, + next_sync_committee_branch: NextSyncCommitteeBranch, + next_sync_committee: Arc>, finalized_block_root: Hash256, } -impl LightClientCachedData { - fn from_state(state: &mut BeaconState) -> Result { +impl LightClientCachedData { + fn from_state(state: &mut BeaconState) -> Result { Ok(Self { finality_branch: state.compute_merkle_proof(FINALIZED_ROOT_INDEX)?.into(), + next_sync_committee: state.next_sync_committee()?.clone(), + next_sync_committee_branch: state + .compute_merkle_proof(NEXT_SYNC_COMMITTEE_INDEX)? + .into(), finalized_block_root: state.finalized_checkpoint().root, }) } diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 09cc1950990..fad8d538716 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -1,8 +1,7 @@ use super::{EthSpec, FixedVector, Hash256, Slot, SyncAggregate, SyncCommittee}; use crate::{ - beacon_state, test_utils::TestRandom, BeaconBlock, BeaconBlockHeader, BeaconState, ChainSpec, - ForkName, ForkVersionDeserialize, LightClientHeaderAltair, LightClientHeaderCapella, - LightClientHeaderDeneb, SignedBeaconBlock, + beacon_state, test_utils::TestRandom, ChainSpec, ForkName, ForkVersionDeserialize, + LightClientHeaderAltair, LightClientHeaderCapella, LightClientHeaderDeneb, SignedBeaconBlock, }; use derivative::Derivative; use safe_arith::ArithError; @@ -15,7 +14,6 @@ use ssz_types::typenum::{U4, U5, U6}; use std::sync::Arc; use superstruct::superstruct; use test_random_derive::TestRandom; -use tree_hash::TreeHash; use tree_hash_derive::TreeHash; pub const FINALIZED_ROOT_INDEX: usize = 105; @@ -142,45 +140,17 @@ impl ForkVersionDeserialize for LightClientUpdate { } impl LightClientUpdate { + #[allow(clippy::too_many_arguments)] pub fn new( - beacon_state: BeaconState, - block: BeaconBlock, - attested_state: &mut BeaconState, + sync_aggregate: &SyncAggregate, + block_slot: Slot, + next_sync_committee: Arc>, + next_sync_committee_branch: FixedVector, + finality_branch: FixedVector, attested_block: &SignedBeaconBlock, finalized_block: &SignedBeaconBlock, chain_spec: &ChainSpec, ) -> Result { - let sync_aggregate = block.body().sync_aggregate()?; - if sync_aggregate.num_set_bits() < chain_spec.min_sync_committee_participants as usize { - return Err(Error::NotEnoughSyncCommitteeParticipants); - } - - let signature_period = block.epoch().sync_committee_period(chain_spec)?; - // Compute and validate attested header. - let mut attested_header = attested_state.latest_block_header().clone(); - attested_header.state_root = attested_state.tree_hash_root(); - let attested_period = attested_header - .slot - .epoch(E::slots_per_epoch()) - .sync_committee_period(chain_spec)?; - if attested_period != signature_period { - return Err(Error::MismatchingPeriods); - } - // Build finalized header from finalized block - let finalized_header = BeaconBlockHeader { - slot: finalized_block.slot(), - proposer_index: finalized_block.message().proposer_index(), - parent_root: finalized_block.parent_root(), - state_root: finalized_block.state_root(), - body_root: finalized_block.message().body_root(), - }; - if finalized_header.tree_hash_root() != beacon_state.finalized_checkpoint().root { - return Err(Error::InvalidFinalizedBlock); - } - let next_sync_committee_branch = - attested_state.compute_merkle_proof(NEXT_SYNC_COMMITTEE_INDEX)?; - let finality_branch = attested_state.compute_merkle_proof(FINALIZED_ROOT_INDEX)?; - let light_client_update = match attested_block .fork_name(chain_spec) .map_err(|_| Error::InconsistentFork)? @@ -193,12 +163,12 @@ impl LightClientUpdate { LightClientHeaderAltair::block_to_light_client_header(finalized_block)?; Self::Altair(LightClientUpdateAltair { attested_header, - next_sync_committee: attested_state.next_sync_committee()?.clone(), - next_sync_committee_branch: FixedVector::new(next_sync_committee_branch)?, + next_sync_committee, + next_sync_committee_branch, finalized_header, - finality_branch: FixedVector::new(finality_branch)?, + finality_branch, sync_aggregate: sync_aggregate.clone(), - signature_slot: block.slot(), + signature_slot: block_slot, }) } ForkName::Capella => { @@ -208,12 +178,12 @@ impl LightClientUpdate { LightClientHeaderCapella::block_to_light_client_header(finalized_block)?; Self::Capella(LightClientUpdateCapella { attested_header, - next_sync_committee: attested_state.next_sync_committee()?.clone(), - next_sync_committee_branch: FixedVector::new(next_sync_committee_branch)?, + next_sync_committee, + next_sync_committee_branch, finalized_header, - finality_branch: FixedVector::new(finality_branch)?, + finality_branch, sync_aggregate: sync_aggregate.clone(), - signature_slot: block.slot(), + signature_slot: block_slot, }) } ForkName::Deneb => { @@ -223,12 +193,12 @@ impl LightClientUpdate { LightClientHeaderDeneb::block_to_light_client_header(finalized_block)?; Self::Deneb(LightClientUpdateDeneb { attested_header, - next_sync_committee: attested_state.next_sync_committee()?.clone(), - next_sync_committee_branch: FixedVector::new(next_sync_committee_branch)?, + next_sync_committee, + next_sync_committee_branch, finalized_header, - finality_branch: FixedVector::new(finality_branch)?, + finality_branch, sync_aggregate: sync_aggregate.clone(), - signature_slot: block.slot(), + signature_slot: block_slot, }) } }; From 9671423d492b66359216ab0b4cb1e0afabd5368a Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 10 Apr 2024 20:49:57 +0300 Subject: [PATCH 06/42] is better light client update --- .../src/light_client_server_cache.rs | 236 +++++++++++++----- 1 file changed, 167 insertions(+), 69 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 9ea5ff004ff..59b8c213b5c 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -1,19 +1,19 @@ use crate::errors::BeaconChainError; use crate::{metrics, BeaconChainTypes, BeaconStore}; use parking_lot::{Mutex, RwLock}; +use safe_arith::{ArithError, SafeArith}; use slog::{debug, Logger}; use ssz_types::FixedVector; use std::num::NonZeroUsize; use std::sync::Arc; -use tree_hash::TreeHash; use types::light_client_update::{ FinalizedRootProofLen, NextSyncCommitteeProofLen, FINALIZED_ROOT_INDEX, NEXT_SYNC_COMMITTEE_INDEX, }; use types::non_zero_usize::new_non_zero_usize; use types::{ - BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, - LightClientOptimisticUpdate, LightClientUpdate, SignedBeaconBlock, Slot, SyncAggregate, + BeaconBlockRef, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, Hash256, + LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, SyncCommittee, }; @@ -82,7 +82,6 @@ impl LightClientServerCache { /// Given a block with a SyncAggregate computes better or more recent light client updates. The /// results are cached either on disk or memory to be served via p2p and rest API - #[allow(clippy::too_many_arguments)] pub fn recompute_and_cache_updates( &self, store: BeaconStore, @@ -155,14 +154,38 @@ impl LightClientServerCache { chain_spec, )?); - self.persist_best_light_client_update( - store, - block_slot, - attested_block, - finalized_block, + let new_light_client_update = LightClientUpdate::new( sync_aggregate, + block_slot, + cached_parts.next_sync_committee, + cached_parts.next_sync_committee_branch, + cached_parts.finality_branch, + &attested_block, + &finalized_block, chain_spec, )?; + + let sync_period = block_slot + .epoch(T::EthSpec::slots_per_epoch()) + .sync_committee_period(chain_spec)?; + + let prev_light_client_update = self.get_light_client_update(sync_period); + + if let Some(prev_light_client_update) = prev_light_client_update { + if is_better_light_client_update( + &prev_light_client_update, + &new_light_client_update, + chain_spec, + )? { + self.light_client_updates_cache + .lock() + .put(sync_period, new_light_client_update); + } + } else { + self.light_client_updates_cache + .lock() + .put(sync_period, new_light_client_update); + } } else { debug!( log, @@ -175,66 +198,6 @@ impl LightClientServerCache { Ok(()) } - // - Is finalized - // - Has the most bits - // - Signed header at the oldest slot - fn persist_best_light_client_update( - &self, - store: BeaconStore, - block_slot: Slot, - attested_block: SignedBeaconBlock<::EthSpec>, - finalized_block: SignedBeaconBlock<::EthSpec>, - sync_aggregate: &SyncAggregate, - chain_spec: &ChainSpec, - ) -> Result<(), BeaconChainError> { - let cached_parts = self.get_or_compute_prev_block_cache( - store, - &attested_block.tree_hash_root(), - &attested_block.state_root(), - attested_block.slot(), - )?; - - let sync_period = block_slot - .epoch(T::EthSpec::slots_per_epoch()) - .sync_committee_period(chain_spec)?; - - let new_light_client_update = LightClientUpdate::new( - sync_aggregate, - block_slot, - cached_parts.next_sync_committee, - cached_parts.next_sync_committee_branch, - cached_parts.finality_branch, - &attested_block, - &finalized_block, - chain_spec, - )?; - - let mut mutex = self.light_client_updates_cache.lock(); - - if let Some(existing_light_client_update) = mutex.get(&sync_period) { - let new_sync_aggregate_bits = new_light_client_update.sync_aggregate().num_set_bits(); - let new_signature_slot = new_light_client_update.signature_slot(); - - let existing_sync_aggregate_bits = - existing_light_client_update.sync_aggregate().num_set_bits(); - let existing_signature_slot = existing_light_client_update.signature_slot(); - - if new_sync_aggregate_bits > existing_sync_aggregate_bits - && new_signature_slot < existing_signature_slot - { - self.light_client_updates_cache - .lock() - .put(sync_period, new_light_client_update); - } - } else { - self.light_client_updates_cache - .lock() - .put(sync_period, new_light_client_update); - } - - Ok(()) - } - /// Retrieves prev block cached data from cache. If not present re-computes by retrieving the /// parent state, and inserts an entry to the cache. /// @@ -275,6 +238,18 @@ impl LightClientServerCache { self.latest_optimistic_update.read().clone() } + pub fn get_light_client_update( + &self, + sync_committee_period: u64, + ) -> Option> { + let mut mutex = self.light_client_updates_cache.lock(); + if let Some(light_client_update) = mutex.get(&sync_committee_period) { + return Some(light_client_update.clone()); + } + + None + } + pub fn get_light_client_updates( &self, sync_committee_period: u64, @@ -355,3 +330,126 @@ fn is_latest_optimistic_update( attested_slot == prev_slot && signature_slot > *prev.signature_slot() } } + +// Implements spec prioritization rules: +// Full nodes SHOULD provide the best derivable LightClientUpdate for each sync committee period +// ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_update +fn is_better_light_client_update( + prev: &LightClientUpdate, + new: &LightClientUpdate, + chain_spec: &ChainSpec, +) -> Result { + // Compare super majority (> 2/3) sync committee participation + let max_active_participants = new.sync_aggregate().sync_committee_bits.len(); + let new_active_participants = new.sync_aggregate().sync_committee_bits.num_set_bits(); + let prev_active_participants = prev.sync_aggregate().sync_committee_bits.num_set_bits(); + + let new_has_super_majority = + new_active_participants.safe_mul(3)? >= max_active_participants.safe_mul(2)?; + let prev_has_super_majority = + prev_active_participants.safe_mul(3)? >= max_active_participants.safe_mul(2)?; + + if new_has_super_majority != prev_has_super_majority { + return Ok(new_has_super_majority & !prev_has_super_majority); + } + + if !new_has_super_majority && new_active_participants != prev_active_participants { + return Ok(new_active_participants > prev_active_participants); + } + + let new_attested_header_slot = match new { + LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, + LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.attested_header.beacon.slot, + }; + + let new_finalized_header_slot = match new { + LightClientUpdate::Altair(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Capella(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.finalized_header.beacon.slot, + }; + + let prev_attested_header_slot = match prev { + LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, + LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.attested_header.beacon.slot, + }; + + let prev_finalized_header_slot = match prev { + LightClientUpdate::Altair(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Capella(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.finalized_header.beacon.slot, + }; + + let new_attested_header_sync_committee_period = + compute_sync_committee_period_at_slot::(new_attested_header_slot, chain_spec)?; + + let new_signature_slot_sync_committee_period = + compute_sync_committee_period_at_slot::(*new.signature_slot(), chain_spec)?; + + let prev_attested_header_sync_committee_period = + compute_sync_committee_period_at_slot::(prev_attested_header_slot, chain_spec)?; + + let prev_signature_slot_sync_committee_period = + compute_sync_committee_period_at_slot::(*prev.signature_slot(), chain_spec)?; + + // Compare presence of relevant sync committee + let new_has_relevant_sync_committee = new.next_sync_committee_branch().is_empty() + && (new_attested_header_sync_committee_period == new_signature_slot_sync_committee_period); + + let prev_has_relevant_sync_committee = prev.next_sync_committee_branch().is_empty() + && (prev_attested_header_sync_committee_period + == prev_signature_slot_sync_committee_period); + + if new_has_relevant_sync_committee != prev_has_relevant_sync_committee { + return Ok(new_has_relevant_sync_committee); + } + + // Compare indication of any finality + let new_has_finality = new.finality_branch().is_empty(); + let prev_has_finality = prev.finality_branch().is_empty(); + if new_has_finality != prev_has_finality { + return Ok(new_has_finality); + } + + // Compare sync committee finality + if new_has_finality { + let new_has_sync_committee_finality = + compute_sync_committee_period_at_slot::(new_finalized_header_slot, chain_spec)? + == compute_sync_committee_period_at_slot::( + new_attested_header_slot, + chain_spec, + )?; + + let prev_has_sync_committee_finality = + compute_sync_committee_period_at_slot::(prev_finalized_header_slot, chain_spec)? + == compute_sync_committee_period_at_slot::( + prev_attested_header_slot, + chain_spec, + )?; + + if new_has_sync_committee_finality != prev_has_sync_committee_finality { + return Ok(new_has_sync_committee_finality); + } + } + + // Tiebreaker 1: Sync committee participation beyond super majority + if new_active_participants != prev_active_participants { + return Ok(new_active_participants > prev_active_participants); + } + + // Tiebreaker 2: Prefer older data (fewer changes to best) + if new_attested_header_slot != prev_attested_header_slot { + return Ok(new_attested_header_slot < prev_attested_header_slot); + } + + return Ok(new.signature_slot() < prev.signature_slot()); +} + +fn compute_sync_committee_period_at_slot( + slot: Slot, + chain_spec: &ChainSpec, +) -> Result { + slot.epoch(E::slots_per_epoch()) + .safe_div(chain_spec.epochs_per_sync_committee_period) +} From 6e0fa26de13f069bba5ad75da025cf8ec4407a0b Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 11 Apr 2024 10:35:52 +0300 Subject: [PATCH 07/42] initial api changes --- beacon_node/beacon_chain/src/beacon_chain.rs | 13 ----- beacon_node/http_api/src/lib.rs | 54 ++++++++++++++++++++ beacon_node/http_api/src/light_client.rs | 30 +++++++++++ common/eth2/src/types.rs | 6 +++ 4 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 beacon_node/http_api/src/light_client.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 6d8add9d292..327b0381054 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1362,19 +1362,6 @@ impl BeaconChain { ) } - pub fn get_latest_light_client_finality_update( - &self, - ) -> Option> { - self.light_client_server_cache.get_latest_finality_update() - } - - pub fn get_latest_light_client_optimistic_update( - &self, - ) -> Option> { - self.light_client_server_cache - .get_latest_optimistic_update() - } - pub fn get_light_client_updates( &self, sync_committee_period: u64, diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 9e6022dc954..e76baa4d861 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -13,6 +13,7 @@ mod block_rewards; mod build_block_contents; mod builder_states; mod database; +mod light_client; mod metrics; mod produce_block; mod proposer_duties; @@ -2407,6 +2408,55 @@ pub fn serve( }, ); + // GET beacon/light_client/updates + let get_beacon_light_client_updates = beacon_light_client_path + .clone() + .and(task_spawner_filter.clone()) + .and(warp::path("updates")) + .and(warp::path::end()) + .and(warp::query::()) + .and(warp::header::optional::("accept")) + .then( + |chain: Arc>, + task_spawner: TaskSpawner, + query: LightClientUpdatesQuery, + accept_header: Option| { + task_spawner.blocking_response_task(Priority::P1, move || { + let updates = chain + .light_client_server_cache + .get_light_client_updates(query.start_period, count) + .ok_or_else(|| { + warp_utils::reject::custom_not_found( + "No LightClientUpdates found".to_string(), + ) + })?; + + let fork_name = chain + .spec + .fork_name_at_slot::(*update.signature_slot()); + match accept_header { + Some(api_types::Accept::Ssz) => Response::builder() + .status(200) + .body(update.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) + .map_err(|e| { + warp_utils::reject::custom_server_error(format!( + "failed to create response: {}", + e + )) + }), + _ => Ok(warp::reply::json(&ForkVersionedResponse { + version: Some(fork_name), + metadata: EmptyMetadata {}, + data: update, + }) + .into_response()), + } + .map(|resp| add_consensus_version_header(resp, fork_name)) + }) + }, + ); + /* * beacon/rewards */ @@ -4529,6 +4579,10 @@ pub fn serve( enable(ctx.config.enable_light_client_server) .and(get_beacon_light_client_bootstrap), ) + .uor( + enable(ctx.config.enable_light_client_server) + .and(get_beacon_light_client_updates) + ) .uor(get_lighthouse_block_packing_efficiency) .uor(get_lighthouse_merge_readiness) .uor(get_events) diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs new file mode 100644 index 00000000000..fffa97cf0f8 --- /dev/null +++ b/beacon_node/http_api/src/light_client.rs @@ -0,0 +1,30 @@ +pub fn build_light_client_updates_response( + chain: Arc, + query: LightClientUpdatesQuery, + accept_header: Option +) { + let light_client_updates = chain + .get_beacon_light_client_updates(query.start_period, query.count) + .ok_or_else(|| { + warp_utils::reject::custom_not_found( + "No LightClientUpdates found".to_string(), + ) + })?; + + let fork_versioned_response = light_client_updates + .iter() + .map(|update| map_light_client_update_to_response(chain, update)) + .collect(); +} + +fn map_light_client_update_to_response(chain: Arc, light_client_update: &LightClientUpdate) -> ForkVersionedResponse{ + let fork_name = chain + .spec + .fork_name_at_slot::(*light_client_update.signature_slot()); + + ForkVersionedResponse { + version: Some(fork_name), + metadata: EmptyMetadata {}, + data: update, + } +} \ No newline at end of file diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index feff1d391a9..cf14de0a9ae 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -767,6 +767,12 @@ pub struct ValidatorAggregateAttestationQuery { pub slot: Slot, } +#[derive(Clone, Deserialize)] +pub struct LightClientUpdatesQuery { + pub start_period: u64, + pub count: u64, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)] pub struct BeaconCommitteeSubscription { #[serde(with = "serde_utils::quoted_u64")] From c8af8fbbd0f4416bcba57fc5a7adc838e8a765c0 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 15 Apr 2024 09:54:36 +0300 Subject: [PATCH 08/42] add lc update db column --- beacon_node/store/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index e86689b0cf1..02b0c4b480e 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -267,6 +267,9 @@ pub enum DBColumn { BeaconHistoricalSummaries, #[strum(serialize = "olc")] OverflowLRUCache, + /// For persisting eagerly computed light client data + #[strum(serialize = "lcu")] + LightClientUpdate, } /// A block from the database, which might have an execution payload or not. @@ -304,7 +307,8 @@ impl DBColumn { | Self::PubkeyCache | Self::BeaconRestorePoint | Self::DhtEnrs - | Self::OptimisticTransitionBlock => 32, + | Self::OptimisticTransitionBlock + | Self::LightClientUpdate => 32, Self::BeaconBlockRoots | Self::BeaconStateRoots | Self::BeaconHistoricalRoots From 45a5d8d8d6c7838ac3edfba8ac378d1622f4e7c0 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 16 Apr 2024 11:14:33 +0300 Subject: [PATCH 09/42] fmt --- beacon_node/http_api/src/lib.rs | 2 +- beacon_node/http_api/src/light_client.rs | 17 ++++++++------- beacon_node/store/src/hot_cold_store.rs | 27 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index e76baa4d861..de36b7d5131 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -4581,7 +4581,7 @@ pub fn serve( ) .uor( enable(ctx.config.enable_light_client_server) - .and(get_beacon_light_client_updates) + .and(get_beacon_light_client_updates), ) .uor(get_lighthouse_block_packing_efficiency) .uor(get_lighthouse_merge_readiness) diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index fffa97cf0f8..e555203eb55 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -1,14 +1,12 @@ pub fn build_light_client_updates_response( - chain: Arc, - query: LightClientUpdatesQuery, - accept_header: Option + chain: Arc, + query: LightClientUpdatesQuery, + accept_header: Option, ) { let light_client_updates = chain .get_beacon_light_client_updates(query.start_period, query.count) .ok_or_else(|| { - warp_utils::reject::custom_not_found( - "No LightClientUpdates found".to_string(), - ) + warp_utils::reject::custom_not_found("No LightClientUpdates found".to_string()) })?; let fork_versioned_response = light_client_updates @@ -17,7 +15,10 @@ pub fn build_light_client_updates_response( .collect(); } -fn map_light_client_update_to_response(chain: Arc, light_client_update: &LightClientUpdate) -> ForkVersionedResponse{ +fn map_light_client_update_to_response( + chain: Arc, + light_client_update: &LightClientUpdate, +) -> ForkVersionedResponse { let fork_name = chain .spec .fork_name_at_slot::(*light_client_update.signature_slot()); @@ -27,4 +28,4 @@ fn map_light_client_update_to_response(chain: Arc, light_client_upd metadata: EmptyMetadata {}, data: update, } -} \ No newline at end of file +} diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 70e02164e08..07c0ad1f285 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -1933,6 +1933,33 @@ impl, Cold: ItemStore> HotColdDB Ok(ops) } + pub fn store_light_client_update( + &self, + sync_committee_period: u64, + light_client_update: &LightClientUpdate, + ) -> Result<(), Error> { + let column = DBColumn::LightClientUpdate; + + self.hot_db.put_bytes( + column.into(), + &sync_committee_period.to_le_bytes(), + &light_client_update.as_ssz_bytes(), + ) + } + + pub fn get_light_client_updates( + &self, + start_period: u64, + count: u64 + ) -> Result<(), Error> { + let column = DBColumn::LightClientUpdate; + for res in self.hot_db.iter_raw_entries(column, &start_period.to_le_bytes()) { + let (key_bytes, value_bytes) = res?; + } + + Ok(()) + } + /// Try to prune all execution payloads, returning early if there is no need to prune. pub fn try_prune_execution_payloads(&self, force: bool) -> Result<(), Error> { let split = self.get_split_info(); From 2545655f807e6dcbc4e6f7564d08be3e21b20dde Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 29 Apr 2024 00:42:25 +0300 Subject: [PATCH 10/42] added tests --- beacon_node/beacon_chain/src/beacon_chain.rs | 10 +- .../src/light_client_server_cache.rs | 131 +++++++++++----- beacon_node/http_api/src/lib.rs | 37 +---- beacon_node/http_api/src/light_client.rs | 140 +++++++++++++++--- beacon_node/http_api/tests/tests.rs | 35 +++++ beacon_node/store/src/hot_cold_store.rs | 27 ---- common/eth2/src/lib.rs | 25 ++++ common/eth2/src/types.rs | 12 ++ 8 files changed, 296 insertions(+), 121 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 327b0381054..eded533cf9b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1366,9 +1366,13 @@ impl BeaconChain { &self, sync_committee_period: u64, count: u64, - ) -> Vec> { - self.light_client_server_cache - .get_light_client_updates(sync_committee_period, count) + ) -> Result>, Error> { + self.light_client_server_cache.get_light_client_updates( + &self.store, + sync_committee_period, + count, + &self.spec, + ) } /// Returns the current heads of the `BeaconChain`. For the canonical head, see `Self::head`. 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 29951507d87..81c9f6fa1d0 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -3,9 +3,13 @@ use crate::{metrics, BeaconChainTypes, BeaconStore}; use parking_lot::{Mutex, RwLock}; use safe_arith::{ArithError, SafeArith}; use slog::{debug, Logger}; +use ssz::Decode; +use ssz::Encode; use ssz_types::FixedVector; use std::num::NonZeroUsize; use std::sync::Arc; +use store::DBColumn; +use store::KeyValueStore; use types::light_client_update::{ FinalizedRootProofLen, NextSyncCommitteeProofLen, FINALIZED_ROOT_INDEX, NEXT_SYNC_COMMITTEE_INDEX, @@ -23,9 +27,6 @@ use types::{ // TODO(lightclientupdate) this cache size has now increased const PREV_BLOCK_CACHE_SIZE: NonZeroUsize = new_non_zero_usize(32); -// TODO(lightclientupdate) we want to cache 6 months worth of light client updates -pub const LIGHT_CLIENT_UPDATES_CACHE_SIZE: NonZeroUsize = new_non_zero_usize(180); - /// This cache computes light client messages ahead of time, required to satisfy p2p and API /// requests. These messages include proofs on historical states, so on-demand computation is /// expensive. @@ -42,8 +43,6 @@ pub struct LightClientServerCache { latest_optimistic_update: RwLock>>, /// Caches state proofs by block root prev_block_cache: Mutex>>, - /// Caches `LightClientUpdate`'s by sync committee period - light_client_updates_cache: Mutex>>, } impl LightClientServerCache { @@ -52,7 +51,6 @@ impl LightClientServerCache { latest_finality_update: None.into(), latest_optimistic_update: None.into(), prev_block_cache: lru::LruCache::new(PREV_BLOCK_CACHE_SIZE).into(), - light_client_updates_cache: lru::LruCache::new(LIGHT_CLIENT_UPDATES_CACHE_SIZE).into(), } } @@ -169,7 +167,8 @@ impl LightClientServerCache { .epoch(T::EthSpec::slots_per_epoch()) .sync_committee_period(chain_spec)?; - let prev_light_client_update = self.get_light_client_update(sync_period); + let prev_light_client_update = + self.get_light_client_update(&store, sync_period, chain_spec)?; if let Some(prev_light_client_update) = prev_light_client_update { if is_better_light_client_update( @@ -177,14 +176,14 @@ impl LightClientServerCache { &new_light_client_update, chain_spec, )? { - self.light_client_updates_cache - .lock() - .put(sync_period, new_light_client_update); + self.store_light_client_update( + &store, + sync_period, + &new_light_client_update, + )?; } } else { - self.light_client_updates_cache - .lock() - .put(sync_period, new_light_client_update); + self.store_light_client_update(&store, sync_period, &new_light_client_update)?; } } else { debug!( @@ -198,6 +197,84 @@ impl LightClientServerCache { Ok(()) } + pub fn store_light_client_update( + &self, + store: &BeaconStore, + sync_committee_period: u64, + light_client_update: &LightClientUpdate, + ) -> Result<(), BeaconChainError> { + let column = DBColumn::LightClientUpdate; + + store.hot_db.put_bytes( + column.into(), + &sync_committee_period.to_le_bytes(), + &light_client_update.as_ssz_bytes(), + )?; + + Ok(()) + } + + pub fn get_light_client_update( + &self, + store: &BeaconStore, + sync_committee_period: u64, + chain_spec: &ChainSpec, + ) -> Result>, BeaconChainError> { + let column = DBColumn::LightClientUpdate; + let res = store + .hot_db + .get_bytes(column.into(), &sync_committee_period.to_le_bytes())?; + + if let Some(light_client_update_bytes) = res { + let epoch = sync_committee_period + .safe_mul(chain_spec.epochs_per_sync_committee_period.into())?; + + let fork_name = chain_spec.fork_name_at_epoch(epoch.into()); + + let light_client_update = + LightClientUpdate::from_ssz_bytes(&light_client_update_bytes, fork_name) + .map_err(store::errors::Error::SszDecodeError)?; + + return Ok(Some(light_client_update)); + } + + Ok(None) + } + + pub fn get_light_client_updates( + &self, + store: &BeaconStore, + start_period: u64, + count: u64, + chain_spec: &ChainSpec, + ) -> Result>, BeaconChainError> { + let column = DBColumn::LightClientUpdate; + let mut light_client_updates = vec![]; + for res in store + .hot_db + .iter_column_from::>(column, &start_period.to_le_bytes()) + { + let (sync_committee_bytes, light_client_update_bytes) = res?; + let sync_committee_period = u64::from_ssz_bytes(&sync_committee_bytes) + .map_err(store::errors::Error::SszDecodeError)?; + let epoch = sync_committee_period + .safe_mul(chain_spec.epochs_per_sync_committee_period.into())?; + + let fork_name = chain_spec.fork_name_at_epoch(epoch.into()); + + let light_client_update = + LightClientUpdate::from_ssz_bytes(&light_client_update_bytes, fork_name) + .map_err(store::errors::Error::SszDecodeError)?; + + light_client_updates.push(light_client_update); + + if sync_committee_period >= start_period + count { + break; + } + } + Ok(light_client_updates) + } + /// Retrieves prev block cached data from cache. If not present re-computes by retrieving the /// parent state, and inserts an entry to the cache. /// @@ -237,34 +314,6 @@ impl LightClientServerCache { pub fn get_latest_optimistic_update(&self) -> Option> { self.latest_optimistic_update.read().clone() } - - pub fn get_light_client_update( - &self, - sync_committee_period: u64, - ) -> Option> { - let mut mutex = self.light_client_updates_cache.lock(); - if let Some(light_client_update) = mutex.get(&sync_committee_period) { - return Some(light_client_update.clone()); - } - - None - } - - pub fn get_light_client_updates( - &self, - sync_committee_period: u64, - count: u64, - ) -> Vec> { - let mut light_client_updates = vec![]; - let mut mutex = self.light_client_updates_cache.lock(); - for slot in sync_committee_period..=sync_committee_period + count { - if let Some(light_client_update) = mutex.get(&slot) { - light_client_updates.push(light_client_update.clone()); - } - } - - light_client_updates - } } impl Default for LightClientServerCache { diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index de36b7d5131..34cf6cc960d 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -31,6 +31,7 @@ mod validator_inclusion; mod validators; mod version; +use crate::light_client::get_light_client_updates; use crate::produce_block::{produce_blinded_block_v2, produce_block_v2, produce_block_v3}; use beacon_chain::{ attestation_verification::VerifiedAttestation, observed_operations::ObservationOutcome, @@ -44,8 +45,8 @@ use bytes::Bytes; use directory::DEFAULT_ROOT_DIR; use eth2::types::{ self as api_types, BroadcastValidation, EndpointVersion, ForkChoice, ForkChoiceNode, - PublishBlockRequest, ValidatorBalancesRequestBody, ValidatorId, ValidatorStatus, - ValidatorsRequestBody, + LightClientUpdatesQuery, PublishBlockRequest, ValidatorBalancesRequestBody, ValidatorId, + ValidatorStatus, ValidatorsRequestBody, }; use eth2::{CONSENSUS_VERSION_HEADER, CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER}; use lighthouse_network::{types::SyncState, EnrExt, NetworkGlobals, PeerId, PubsubMessage}; @@ -2422,37 +2423,7 @@ pub fn serve( query: LightClientUpdatesQuery, accept_header: Option| { task_spawner.blocking_response_task(Priority::P1, move || { - let updates = chain - .light_client_server_cache - .get_light_client_updates(query.start_period, count) - .ok_or_else(|| { - warp_utils::reject::custom_not_found( - "No LightClientUpdates found".to_string(), - ) - })?; - - let fork_name = chain - .spec - .fork_name_at_slot::(*update.signature_slot()); - match accept_header { - Some(api_types::Accept::Ssz) => Response::builder() - .status(200) - .body(update.as_ssz_bytes().into()) - .map(|res: Response| add_ssz_content_type_header(res)) - .map_err(|e| { - warp_utils::reject::custom_server_error(format!( - "failed to create response: {}", - e - )) - }), - _ => Ok(warp::reply::json(&ForkVersionedResponse { - version: Some(fork_name), - metadata: EmptyMetadata {}, - data: update, - }) - .into_response()), - } - .map(|resp| add_consensus_version_header(resp, fork_name)) + get_light_client_updates::(chain, query, accept_header) }) }, ); diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index e555203eb55..992df334439 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -1,31 +1,137 @@ -pub fn build_light_client_updates_response( - chain: Arc, +use beacon_chain::{BeaconChain, BeaconChainTypes}; +use eth2::types::{ + self as api_types, ChainSpec, ForkVersionedResponse, LightClientUpdate, + LightClientUpdateResponseChunk, LightClientUpdateSszResponse, LightClientUpdatesQuery, +}; +use ssz::Encode; +use std::sync::Arc; +use warp::{ + hyper::{Body, Response}, + reply::Reply, + Rejection, +}; + +use crate::version::{add_ssz_content_type_header, fork_versioned_response, V1}; + +const MAX_REQUEST_LIGHT_CLIENT_UPDATES: u64 = 128; + +pub fn get_light_client_updates( + chain: Arc>, query: LightClientUpdatesQuery, accept_header: Option, -) { +) -> Result, Rejection> { + validate_light_client_updates_request(&chain, &query)?; + let light_client_updates = chain - .get_beacon_light_client_updates(query.start_period, query.count) - .ok_or_else(|| { + .get_light_client_updates(query.start_period, query.count) + .map_err(|_| { warp_utils::reject::custom_not_found("No LightClientUpdates found".to_string()) })?; - let fork_versioned_response = light_client_updates - .iter() - .map(|update| map_light_client_update_to_response(chain, update)) - .collect(); + match accept_header { + Some(api_types::Accept::Ssz) => { + let response_chunks = light_client_updates + .iter() + .map(|update| map_light_client_update_to_ssz_chunk::(&chain, update)) + .collect::>(); + + let ssz_response = LightClientUpdateSszResponse { + response_chunk_len: (light_client_updates.len() as u64).to_le_bytes().to_vec(), + response_chunk: response_chunks.as_ssz_bytes(), + } + .as_ssz_bytes(); + + Response::builder() + .status(200) + .body(ssz_response) + .map(|res: Response>| add_ssz_content_type_header(res)) + .map_err(|e| { + warp_utils::reject::custom_server_error(format!( + "failed to create response: {}", + e + )) + }) + } + _ => { + let fork_versioned_response = light_client_updates + .iter() + .map(|update| map_light_client_update_to_json_response::(&chain, update.clone())) + .collect::>>>(); + Ok(warp::reply::json(&fork_versioned_response).into_response()) + } + } +} + +pub fn validate_light_client_updates_request( + chain: &BeaconChain, + query: &LightClientUpdatesQuery, +) -> Result<(), Rejection> { + if query.count > MAX_REQUEST_LIGHT_CLIENT_UPDATES { + return Err(warp_utils::reject::custom_bad_request("Invalid count requested".to_string())) + } + + let current_sync_period = chain.epoch().unwrap().sync_committee_period(&chain.spec).unwrap(); + println!("start period {:?}", query.start_period); + if query.start_period > current_sync_period { + return Err(warp_utils::reject::custom_bad_request("Invalid sync committee period requested 1".to_string())) + } + + let earliest_altair_sync_committee = chain.spec.altair_fork_epoch.unwrap().sync_committee_period(&chain.spec).unwrap(); + if query.start_period < earliest_altair_sync_committee { + return Err(warp_utils::reject::custom_bad_request("Invalid sync committee period requested 2".to_string())) + } + + Ok(()) } -fn map_light_client_update_to_response( - chain: Arc, - light_client_update: &LightClientUpdate, -) -> ForkVersionedResponse { +fn map_light_client_update_to_ssz_chunk( + chain: &BeaconChain, + light_client_update: &LightClientUpdate, +) -> LightClientUpdateResponseChunk { let fork_name = chain .spec .fork_name_at_slot::(*light_client_update.signature_slot()); - ForkVersionedResponse { - version: Some(fork_name), - metadata: EmptyMetadata {}, - data: update, + let fork_digest = match fork_name { + types::ForkName::Base => ChainSpec::compute_fork_digest( + chain.spec.genesis_fork_version, + chain.genesis_validators_root, + ), + types::ForkName::Altair => ChainSpec::compute_fork_digest( + chain.spec.altair_fork_version, + chain.genesis_validators_root, + ), + types::ForkName::Merge => ChainSpec::compute_fork_digest( + chain.spec.bellatrix_fork_version, + chain.genesis_validators_root, + ), + types::ForkName::Capella => ChainSpec::compute_fork_digest( + chain.spec.capella_fork_version, + chain.genesis_validators_root, + ), + types::ForkName::Deneb => ChainSpec::compute_fork_digest( + chain.spec.deneb_fork_version, + chain.genesis_validators_root, + ), + types::ForkName::Electra => ChainSpec::compute_fork_digest( + chain.spec.electra_fork_version, + chain.genesis_validators_root, + ), + }; + + LightClientUpdateResponseChunk { + context: fork_digest, + payload: light_client_update.as_ssz_bytes(), } } + +fn map_light_client_update_to_json_response( + chain: &BeaconChain, + light_client_update: LightClientUpdate, +) -> ForkVersionedResponse> { + let fork_name = chain + .spec + .fork_name_at_slot::(*light_client_update.signature_slot()); + + fork_versioned_response(V1, fork_name, light_client_update).unwrap() +} diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index e4580e4ffdb..c67a0ac0056 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1702,6 +1702,29 @@ impl ApiTester { self } + pub async fn test_get_beacon_light_client_updates(self) -> Self { + let current_epoch = self.chain.epoch().unwrap(); + let current_sync_committee_period = current_epoch.sync_committee_period(&self.chain.spec).unwrap(); + + let result = match self + .client + .get_light_client_updates::(current_sync_committee_period as u64, 1) + .await + { + Ok(result) => result.map(|res| res.data).collect(), + Err(e) => panic!("query failed incorrectly: {e:?}"), + }; + + let expected = self + .chain + .light_client_server_cache + .get_light_client_updates(&self.chain.store, current_sync_committee_period as u64, 1, &self.chain.spec).unwrap(); + + assert_eq!(result, expected); + + self + } + pub async fn test_get_beacon_light_client_bootstrap(self) -> Self { let block_id = BlockId(CoreBlockId::Finalized); let (block_root, _, _) = block_id.root(&self.chain).unwrap(); @@ -5780,6 +5803,18 @@ async fn node_get() { .await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_light_client_updates() { + let config = ApiTesterConfig { + spec: ForkName::Altair.make_genesis_spec(E::default_spec()), + ..<_>::default() + }; + ApiTester::new_from_config(config) + .await + .test_get_beacon_light_client_updates() + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_light_client_bootstrap() { let config = ApiTesterConfig { diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 07c0ad1f285..70e02164e08 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -1933,33 +1933,6 @@ impl, Cold: ItemStore> HotColdDB Ok(ops) } - pub fn store_light_client_update( - &self, - sync_committee_period: u64, - light_client_update: &LightClientUpdate, - ) -> Result<(), Error> { - let column = DBColumn::LightClientUpdate; - - self.hot_db.put_bytes( - column.into(), - &sync_committee_period.to_le_bytes(), - &light_client_update.as_ssz_bytes(), - ) - } - - pub fn get_light_client_updates( - &self, - start_period: u64, - count: u64 - ) -> Result<(), Error> { - let column = DBColumn::LightClientUpdate; - for res in self.hot_db.iter_raw_entries(column, &start_period.to_le_bytes()) { - let (key_bytes, value_bytes) = res?; - } - - Ok(()) - } - /// Try to prune all execution payloads, returning early if there is no need to prune. pub fn try_prune_execution_payloads(&self, force: bool) -> Result<(), Error> { let split = self.get_split_info(); diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index d8b2c8ef2d1..a982d4d47e6 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -732,6 +732,31 @@ impl BeaconNodeHttpClient { self.get_opt(path).await } + /// `GET beacon/light_client/updates` + /// + /// Returns `Ok(None)` on a 404 error. + pub async fn get_light_client_updates( + &self, + start_period: u64, + count: u64, + ) -> Result>>>, Error> { + let mut path = self.eth_path(V1)?; + + path.path_segments_mut() + .map_err(|()| Error::InvalidUrl(self.server.clone()))? + .push("beacon") + .push("light_client") + .push("updates"); + + path.query_pairs_mut() + .append_pair("start_period", &start_period.to_string()); + + path.query_pairs_mut() + .append_pair("count", &count.to_string()); + + self.get_opt(path).await + } + /// `GET beacon/light_client/bootstrap` /// /// Returns `Ok(None)` on a 404 error. diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index cf14de0a9ae..e930a8d0f71 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -773,6 +773,18 @@ pub struct LightClientUpdatesQuery { pub count: u64, } +#[derive(Encode, Decode)] +pub struct LightClientUpdateSszResponse { + pub response_chunk_len: Vec, + pub response_chunk: Vec, +} + +#[derive(Encode, Decode)] +pub struct LightClientUpdateResponseChunk { + pub context: [u8; 4], + pub payload: Vec, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)] pub struct BeaconCommitteeSubscription { #[serde(with = "serde_utils::quoted_u64")] From 3029f2c2c5c0e3f7d1f33f37e0616f886be5520b Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 29 Apr 2024 01:06:47 +0300 Subject: [PATCH 11/42] add sim --- beacon_node/http_api/tests/tests.rs | 6 +++--- common/eth2/src/lib.rs | 2 +- testing/simulator/src/checks.rs | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index c67a0ac0056..382d9c2d66e 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1708,10 +1708,10 @@ impl ApiTester { let result = match self .client - .get_light_client_updates::(current_sync_committee_period as u64, 1) + .get_beacon_light_client_updates::(current_sync_committee_period as u64, 1) .await { - Ok(result) => result.map(|res| res.data).collect(), + Ok(result) => result, Err(e) => panic!("query failed incorrectly: {e:?}"), }; @@ -1720,7 +1720,7 @@ impl ApiTester { .light_client_server_cache .get_light_client_updates(&self.chain.store, current_sync_committee_period as u64, 1, &self.chain.spec).unwrap(); - assert_eq!(result, expected); + // assert_eq!(result, expected); self } diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index a982d4d47e6..f9e93f64a9e 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -735,7 +735,7 @@ impl BeaconNodeHttpClient { /// `GET beacon/light_client/updates` /// /// Returns `Ok(None)` on a 404 error. - pub async fn get_light_client_updates( + pub async fn get_beacon_light_client_updates( &self, start_period: u64, count: u64, diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index d30e44a1174..e2cb8065e4b 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -329,6 +329,10 @@ pub(crate) async fn verify_light_client_updates( "Existing finality update too old: signature slot {signature_slot}, current slot {slot:?}" )); } + + client + .get_beacon_light_client_updates::(1, 1) + .await; } Ok(()) From 49a7e615a99d6fa74cfad14218ff138fc668cfce Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 15 May 2024 10:14:06 +0300 Subject: [PATCH 12/42] fix some weird issues with the simulator --- beacon_node/http_api/src/light_client.rs | 29 ++++++++++++++++++------ beacon_node/http_api/tests/tests.rs | 12 ++++++++-- common/eth2/src/lib.rs | 2 +- testing/simulator/src/checks.rs | 28 ++++++++++++++++++++--- testing/simulator/src/cli.rs | 3 --- 5 files changed, 58 insertions(+), 16 deletions(-) diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index 992df334439..339978d3de6 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -67,18 +67,33 @@ pub fn validate_light_client_updates_request( query: &LightClientUpdatesQuery, ) -> Result<(), Rejection> { if query.count > MAX_REQUEST_LIGHT_CLIENT_UPDATES { - return Err(warp_utils::reject::custom_bad_request("Invalid count requested".to_string())) + return Err(warp_utils::reject::custom_bad_request( + "Invalid count requested".to_string(), + )); } - let current_sync_period = chain.epoch().unwrap().sync_committee_period(&chain.spec).unwrap(); + let current_sync_period = chain + .epoch() + .unwrap() + .sync_committee_period(&chain.spec) + .unwrap(); println!("start period {:?}", query.start_period); if query.start_period > current_sync_period { - return Err(warp_utils::reject::custom_bad_request("Invalid sync committee period requested 1".to_string())) + return Err(warp_utils::reject::custom_bad_request( + "Invalid sync committee period requested 1".to_string(), + )); } - let earliest_altair_sync_committee = chain.spec.altair_fork_epoch.unwrap().sync_committee_period(&chain.spec).unwrap(); + let earliest_altair_sync_committee = chain + .spec + .altair_fork_epoch + .unwrap() + .sync_committee_period(&chain.spec) + .unwrap(); if query.start_period < earliest_altair_sync_committee { - return Err(warp_utils::reject::custom_bad_request("Invalid sync committee period requested 2".to_string())) + return Err(warp_utils::reject::custom_bad_request( + "Invalid sync committee period requested 2".to_string(), + )); } Ok(()) @@ -93,7 +108,7 @@ fn map_light_client_update_to_ssz_chunk( .fork_name_at_slot::(*light_client_update.signature_slot()); let fork_digest = match fork_name { - types::ForkName::Base => ChainSpec::compute_fork_digest( + types::ForkName::Base => ChainSpec::compute_fork_digest( chain.spec.genesis_fork_version, chain.genesis_validators_root, ), @@ -101,7 +116,7 @@ fn map_light_client_update_to_ssz_chunk( chain.spec.altair_fork_version, chain.genesis_validators_root, ), - types::ForkName::Merge => ChainSpec::compute_fork_digest( + types::ForkName::Bellatrix => ChainSpec::compute_fork_digest( chain.spec.bellatrix_fork_version, chain.genesis_validators_root, ), diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 4da0436ef28..d84b36272c8 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1704,7 +1704,9 @@ impl ApiTester { pub async fn test_get_beacon_light_client_updates(self) -> Self { let current_epoch = self.chain.epoch().unwrap(); - let current_sync_committee_period = current_epoch.sync_committee_period(&self.chain.spec).unwrap(); + let current_sync_committee_period = current_epoch + .sync_committee_period(&self.chain.spec) + .unwrap(); let result = match self .client @@ -1718,7 +1720,13 @@ impl ApiTester { let expected = self .chain .light_client_server_cache - .get_light_client_updates(&self.chain.store, current_sync_committee_period as u64, 1, &self.chain.spec).unwrap(); + .get_light_client_updates( + &self.chain.store, + current_sync_committee_period as u64, + 1, + &self.chain.spec, + ) + .unwrap(); // assert_eq!(result, expected); diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index f9e93f64a9e..ed0e33a022a 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -739,7 +739,7 @@ impl BeaconNodeHttpClient { &self, start_period: u64, count: u64, - ) -> Result>>>, Error> { + ) -> Result>>>, Error> { let mut path = self.eth_path(V1)?; path.path_segments_mut() diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index abcd2aabbcb..37151a226d3 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -263,6 +263,10 @@ pub(crate) async fn verify_light_client_updates( slot_delay(Slot::new(1), slot_duration).await; let slot = Slot::new(slot); let previous_slot = slot - 1; + let sync_committee_period = slot + .epoch(E::slots_per_epoch()) + .sync_committee_period(&E::default_spec()) + .unwrap(); let previous_slot_block = client .get_beacon_blocks::(BlockId::Slot(previous_slot)) @@ -330,9 +334,27 @@ pub(crate) async fn verify_light_client_updates( )); } - client - .get_beacon_light_client_updates::(1, 1) - .await; + // Verify light client update. `signature_slot_distance` should be 1 in the ideal scenario. + let light_client_updates = client + .get_beacon_light_client_updates::(sync_committee_period, 1) + .await + .map_err(|e| format!("Error while getting light client update: {:?}", e))? + .ok_or(format!("Light client update not found {slot:?}"))?; + + if light_client_updates.len() != 1 { + return Err(format!( + "{} light client update(s) was returned when only one was expected.", light_client_updates.len() + )); + } + + if let Some(light_client_update) = light_client_updates.first() { + let signature_slot_distance = slot - *light_client_update.data.signature_slot(); + if signature_slot_distance > light_client_update_slot_tolerance { + return Err(format!( + "Existing light client update too old: signature slot {signature_slot}, current slot {slot:?}" + )); + } + }; } Ok(()) diff --git a/testing/simulator/src/cli.rs b/testing/simulator/src/cli.rs index 00af7e560ce..79e575166d2 100644 --- a/testing/simulator/src/cli.rs +++ b/testing/simulator/src/cli.rs @@ -57,7 +57,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name("continue-after-checks") - .short("c") .long("continue_after_checks") .takes_value(false) .help("Continue after checks (default false)"), @@ -77,7 +76,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name("vc-count") - .short("c") .long("vc-count") .takes_value(true) .default_value("3") @@ -117,7 +115,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name("continue-after-checks") - .short("c") .long("continue_after_checks") .takes_value(false) .help("Continue after checks (default false)"), From 00d4fd0cfe3995d7b21f1944ce2909f2f1285bb3 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 27 May 2024 17:46:23 +0200 Subject: [PATCH 13/42] tests --- beacon_node/http_api/tests/tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index d84b36272c8..f986ea203f6 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1728,8 +1728,7 @@ impl ApiTester { ) .unwrap(); - // assert_eq!(result, expected); - + assert_eq!(result.clone().unwrap().len(), expected.len()); self } From 6f12133bc80e102a12810d5a15d5c5990c70d3a1 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 29 May 2024 14:31:02 +0200 Subject: [PATCH 14/42] test changes --- beacon_node/beacon_chain/tests/store_tests.rs | 132 ++++++++++++++++++ slasher/src/database/interface.rs | 1 + testing/simulator/src/checks.rs | 7 +- 3 files changed, 138 insertions(+), 2 deletions(-) diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 5da92573f77..0f5087f5537 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -106,6 +106,138 @@ fn get_harness_generic( harness } +#[tokio::test] +async fn light_client_test() { + let num_blocks_produced = E::slots_per_epoch() * 20; + let db_path = tempdir().unwrap(); + let log = test_logger(); + let spec = test_spec::(); + let seconds_per_slot = spec.seconds_per_slot; + let store = get_store_generic( + &db_path, + StoreConfig { + slots_per_restore_point: 2 * E::slots_per_epoch(), + ..Default::default() + }, + test_spec::(), + ); + let harness = get_harness(store.clone(), LOW_VALIDATOR_COUNT); + let all_validators = (0..LOW_VALIDATOR_COUNT).collect::>(); + let checkpoint_slot = Slot::new(E::slots_per_epoch() * 2); + let num_initial_slots = E::slots_per_epoch() * 11; + let slots: Vec = (1..num_initial_slots).map(Slot::new).collect(); + + let (genesis_state, genesis_state_root) = harness.get_current_state_and_root(); + harness + .add_attested_blocks_at_slots( + genesis_state.clone(), + genesis_state_root, + &slots, + &all_validators, + ) + .await; + + let wss_block_root = harness + .chain + .block_root_at_slot(Slot::new(0), WhenSlotSkipped::Prev) + .unwrap() + .unwrap(); + let wss_state_root = harness + .chain + .state_root_at_slot(checkpoint_slot) + .unwrap() + .unwrap(); + + let wss_block = harness + .chain + .store + .get_full_block(&wss_block_root) + .unwrap() + .unwrap(); + let wss_blobs_opt = harness.chain.store.get_blobs(&wss_block_root).unwrap(); + let wss_state = store + .get_state(&wss_state_root, Some(checkpoint_slot)) + .unwrap() + .unwrap(); + + let kzg = spec.deneb_fork_epoch.map(|_| KZG.clone()); + + let mock = + mock_execution_layer_from_parts(&harness.spec, harness.runtime.task_executor.clone()); + + harness + .extend_chain( + num_blocks_produced as usize, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + // Initialise a new beacon chain from the finalized checkpoint. + // The slot clock must be set to a time ahead of the checkpoint state. + let slot_clock = TestingSlotClock::new( + Slot::new(0), + Duration::from_secs(harness.chain.genesis_time), + Duration::from_secs(seconds_per_slot), + ); + slot_clock.set_slot(harness.get_current_slot().as_u64()); + + let (shutdown_tx, _shutdown_rx) = futures::channel::mpsc::channel(1); + + let beacon_chain = BeaconChainBuilder::>::new(MinimalEthSpec) + .store(store.clone()) + .custom_spec(test_spec::()) + .task_executor(harness.chain.task_executor.clone()) + .logger(log.clone()) + .weak_subjectivity_state( + wss_state, + wss_block.clone(), + wss_blobs_opt.clone(), + genesis_state, + ) + .unwrap() + .store_migrator_config(MigratorConfig::default().blocking()) + .dummy_eth1_backend() + .expect("should build dummy backend") + .slot_clock(slot_clock) + .shutdown_sender(shutdown_tx) + .chain_config(ChainConfig::default()) + .event_handler(Some(ServerSentEventHandler::new_with_capacity( + log.clone(), + 1, + ))) + .execution_layer(Some(mock.el)) + .kzg(kzg) + .build() + .expect("should build"); + + let beacon_chain = Arc::new(beacon_chain); + + let current_state = harness.get_current_state(); + + let sync_aggregate = beacon_chain + .op_pool + .get_sync_aggregate(¤t_state) + .unwrap() + .unwrap(); + + let block_root = harness + .chain + .block_root_at_slot(Slot::new(0), WhenSlotSkipped::Prev) + .unwrap() + .unwrap(); + + beacon_chain + .light_client_server_cache + .recompute_and_cache_updates( + store, + Slot::new(1), + &block_root, + &sync_aggregate, + &log, + &spec + ).unwrap(); +} + /// Tests that `store.heal_freezer_block_roots_at_split` inserts block roots between last restore point /// slot and the split slot. #[tokio::test] diff --git a/slasher/src/database/interface.rs b/slasher/src/database/interface.rs index 5bb920383c3..89e25c90816 100644 --- a/slasher/src/database/interface.rs +++ b/slasher/src/database/interface.rs @@ -227,4 +227,5 @@ impl<'env> Cursor<'env> { _ => Err(Error::MismatchedDatabaseVariant), } } + } diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index 37151a226d3..bc3bfb13f0b 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -343,7 +343,8 @@ pub(crate) async fn verify_light_client_updates( if light_client_updates.len() != 1 { return Err(format!( - "{} light client update(s) was returned when only one was expected.", light_client_updates.len() + "{} light client update(s) was returned when only one was expected.", + light_client_updates.len() )); } @@ -356,7 +357,9 @@ pub(crate) async fn verify_light_client_updates( } }; } - + return Err(format!( + "we succeeded in returning light client data :)" + )); Ok(()) } From 22e9201e1ac2bd97cb35795da35e212042536bc0 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 31 May 2024 13:45:19 +0200 Subject: [PATCH 15/42] testing --- .../src/light_client_server_cache.rs | 217 ++++-------------- .../beacon_chain/tests/light_client_tests.rs | 0 beacon_node/beacon_chain/tests/main.rs | 1 + beacon_node/beacon_chain/tests/store_tests.rs | 126 +++++++++- beacon_node/store/src/leveldb_store.rs | 1 - beacon_node/store/src/lib.rs | 6 +- .../src/light_client_optimistic_update.rs | 17 ++ consensus/types/src/light_client_update.rs | 126 ++++++++++ testing/ef_tests/src/cases/light_client.rs | 0 9 files changed, 303 insertions(+), 191 deletions(-) create mode 100644 beacon_node/beacon_chain/tests/light_client_tests.rs create mode 100644 testing/ef_tests/src/cases/light_client.rs 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 81c9f6fa1d0..5484bd4bd0a 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -116,7 +116,7 @@ impl LightClientServerCache { // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice let is_latest_optimistic = match &self.latest_optimistic_update.read().clone() { Some(latest_optimistic_update) => { - is_latest_optimistic_update(latest_optimistic_update, attested_slot, signature_slot) + latest_optimistic_update.is_latest_optimistic_update(attested_slot, signature_slot) } None => true, }; @@ -151,40 +151,6 @@ impl LightClientServerCache { signature_slot, chain_spec, )?); - - let new_light_client_update = LightClientUpdate::new( - sync_aggregate, - block_slot, - cached_parts.next_sync_committee, - cached_parts.next_sync_committee_branch, - cached_parts.finality_branch, - &attested_block, - &finalized_block, - chain_spec, - )?; - - let sync_period = block_slot - .epoch(T::EthSpec::slots_per_epoch()) - .sync_committee_period(chain_spec)?; - - let prev_light_client_update = - self.get_light_client_update(&store, sync_period, chain_spec)?; - - if let Some(prev_light_client_update) = prev_light_client_update { - if is_better_light_client_update( - &prev_light_client_update, - &new_light_client_update, - chain_spec, - )? { - self.store_light_client_update( - &store, - sync_period, - &new_light_client_update, - )?; - } - } else { - self.store_light_client_update(&store, sync_period, &new_light_client_update)?; - } } else { debug!( log, @@ -194,6 +160,44 @@ impl LightClientServerCache { } } + if let Some(finalized_block) = + store.get_full_block(&cached_parts.finalized_block_root)? + { + + let new_light_client_update = LightClientUpdate::new( + sync_aggregate, + block_slot, + cached_parts.next_sync_committee, + cached_parts.next_sync_committee_branch, + cached_parts.finality_branch, + &attested_block, + &finalized_block, + chain_spec, + )?; + + let sync_period = block_slot + .epoch(T::EthSpec::slots_per_epoch()) + .sync_committee_period(chain_spec)?; + + let prev_light_client_update = + self.get_light_client_update(&store, sync_period, chain_spec)?; + + if let Some(prev_light_client_update) = prev_light_client_update { + if prev_light_client_update.is_better_light_client_update( + &new_light_client_update, + chain_spec, + )? { + self.store_light_client_update( + &store, + sync_period, + &new_light_client_update, + )?; + } + } else { + self.store_light_client_update(&store, sync_period, &new_light_client_update)?; + } + } + Ok(()) } @@ -252,8 +256,8 @@ impl LightClientServerCache { let mut light_client_updates = vec![]; for res in store .hot_db - .iter_column_from::>(column, &start_period.to_le_bytes()) - { + .iter_column_from::>(column.into(), &start_period.to_le_bytes()) + { let (sync_committee_bytes, light_client_update_bytes) = res?; let sync_committee_period = u64::from_ssz_bytes(&sync_committee_bytes) .map_err(store::errors::Error::SszDecodeError)?; @@ -363,142 +367,3 @@ fn is_latest_finality_update( } } -// Implements spec prioritization rules: -// > Full nodes SHOULD provide the LightClientOptimisticUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) -// -// ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_optimistic_update -fn is_latest_optimistic_update( - prev: &LightClientOptimisticUpdate, - attested_slot: Slot, - signature_slot: Slot, -) -> bool { - let prev_slot = prev.get_slot(); - if attested_slot > prev_slot { - true - } else { - attested_slot == prev_slot && signature_slot > *prev.signature_slot() - } -} - -// Implements spec prioritization rules: -// Full nodes SHOULD provide the best derivable LightClientUpdate for each sync committee period -// ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_update -fn is_better_light_client_update( - prev: &LightClientUpdate, - new: &LightClientUpdate, - chain_spec: &ChainSpec, -) -> Result { - // Compare super majority (> 2/3) sync committee participation - let max_active_participants = new.sync_aggregate().sync_committee_bits.len(); - let new_active_participants = new.sync_aggregate().sync_committee_bits.num_set_bits(); - let prev_active_participants = prev.sync_aggregate().sync_committee_bits.num_set_bits(); - - let new_has_super_majority = - new_active_participants.safe_mul(3)? >= max_active_participants.safe_mul(2)?; - let prev_has_super_majority = - prev_active_participants.safe_mul(3)? >= max_active_participants.safe_mul(2)?; - - if new_has_super_majority != prev_has_super_majority { - return Ok(new_has_super_majority & !prev_has_super_majority); - } - - if !new_has_super_majority && new_active_participants != prev_active_participants { - return Ok(new_active_participants > prev_active_participants); - } - - let new_attested_header_slot = match new { - LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, - LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, - LightClientUpdate::Deneb(update) => update.attested_header.beacon.slot, - }; - - let new_finalized_header_slot = match new { - LightClientUpdate::Altair(update) => update.finalized_header.beacon.slot, - LightClientUpdate::Capella(update) => update.finalized_header.beacon.slot, - LightClientUpdate::Deneb(update) => update.finalized_header.beacon.slot, - }; - - let prev_attested_header_slot = match prev { - LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, - LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, - LightClientUpdate::Deneb(update) => update.attested_header.beacon.slot, - }; - - let prev_finalized_header_slot = match prev { - LightClientUpdate::Altair(update) => update.finalized_header.beacon.slot, - LightClientUpdate::Capella(update) => update.finalized_header.beacon.slot, - LightClientUpdate::Deneb(update) => update.finalized_header.beacon.slot, - }; - - let new_attested_header_sync_committee_period = - compute_sync_committee_period_at_slot::(new_attested_header_slot, chain_spec)?; - - let new_signature_slot_sync_committee_period = - compute_sync_committee_period_at_slot::(*new.signature_slot(), chain_spec)?; - - let prev_attested_header_sync_committee_period = - compute_sync_committee_period_at_slot::(prev_attested_header_slot, chain_spec)?; - - let prev_signature_slot_sync_committee_period = - compute_sync_committee_period_at_slot::(*prev.signature_slot(), chain_spec)?; - - // Compare presence of relevant sync committee - let new_has_relevant_sync_committee = new.next_sync_committee_branch().is_empty() - && (new_attested_header_sync_committee_period == new_signature_slot_sync_committee_period); - - let prev_has_relevant_sync_committee = prev.next_sync_committee_branch().is_empty() - && (prev_attested_header_sync_committee_period - == prev_signature_slot_sync_committee_period); - - if new_has_relevant_sync_committee != prev_has_relevant_sync_committee { - return Ok(new_has_relevant_sync_committee); - } - - // Compare indication of any finality - let new_has_finality = new.finality_branch().is_empty(); - let prev_has_finality = prev.finality_branch().is_empty(); - if new_has_finality != prev_has_finality { - return Ok(new_has_finality); - } - - // Compare sync committee finality - if new_has_finality { - let new_has_sync_committee_finality = - compute_sync_committee_period_at_slot::(new_finalized_header_slot, chain_spec)? - == compute_sync_committee_period_at_slot::( - new_attested_header_slot, - chain_spec, - )?; - - let prev_has_sync_committee_finality = - compute_sync_committee_period_at_slot::(prev_finalized_header_slot, chain_spec)? - == compute_sync_committee_period_at_slot::( - prev_attested_header_slot, - chain_spec, - )?; - - if new_has_sync_committee_finality != prev_has_sync_committee_finality { - return Ok(new_has_sync_committee_finality); - } - } - - // Tiebreaker 1: Sync committee participation beyond super majority - if new_active_participants != prev_active_participants { - return Ok(new_active_participants > prev_active_participants); - } - - // Tiebreaker 2: Prefer older data (fewer changes to best) - if new_attested_header_slot != prev_attested_header_slot { - return Ok(new_attested_header_slot < prev_attested_header_slot); - } - - return Ok(new.signature_slot() < prev.signature_slot()); -} - -fn compute_sync_committee_period_at_slot( - slot: Slot, - chain_spec: &ChainSpec, -) -> Result { - slot.epoch(E::slots_per_epoch()) - .safe_div(chain_spec.epochs_per_sync_committee_period) -} diff --git a/beacon_node/beacon_chain/tests/light_client_tests.rs b/beacon_node/beacon_chain/tests/light_client_tests.rs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/beacon_node/beacon_chain/tests/main.rs b/beacon_node/beacon_chain/tests/main.rs index 942ce816846..dd891d2baf3 100644 --- a/beacon_node/beacon_chain/tests/main.rs +++ b/beacon_node/beacon_chain/tests/main.rs @@ -11,3 +11,4 @@ mod store_tests; mod sync_committee_verification; mod tests; mod validator_monitor; +mod light_client_tests; diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 0f5087f5537..137dcbae78a 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -9,6 +9,7 @@ use beacon_chain::test_utils::{ mock_execution_layer_from_parts, test_spec, AttestationStrategy, BeaconChainHarness, BlockStrategy, DiskHarnessType, KZG, }; +use beacon_chain::test_utils::RelativeSyncCommittee; use beacon_chain::{ data_availability_checker::MaybeAvailableBlock, historical_blocks::HistoricalBlockError, migrate::MigratorConfig, BeaconChain, BeaconChainError, BeaconChainTypes, BeaconSnapshot, @@ -109,6 +110,8 @@ fn get_harness_generic( #[tokio::test] async fn light_client_test() { let num_blocks_produced = E::slots_per_epoch() * 20; + let num_final_blocks = E::slots_per_epoch() * 2; + let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9); let db_path = tempdir().unwrap(); let log = test_logger(); let spec = test_spec::(); @@ -123,8 +126,7 @@ async fn light_client_test() { ); let harness = get_harness(store.clone(), LOW_VALIDATOR_COUNT); let all_validators = (0..LOW_VALIDATOR_COUNT).collect::>(); - let checkpoint_slot = Slot::new(E::slots_per_epoch() * 2); - let num_initial_slots = E::slots_per_epoch() * 11; + let num_initial_slots = E::slots_per_epoch() * 10; let slots: Vec = (1..num_initial_slots).map(Slot::new).collect(); let (genesis_state, genesis_state_root) = harness.get_current_state_and_root(); @@ -136,10 +138,11 @@ async fn light_client_test() { &all_validators, ) .await; + let (genesis_state, genesis_state_root) = harness.get_current_state_and_root(); let wss_block_root = harness .chain - .block_root_at_slot(Slot::new(0), WhenSlotSkipped::Prev) + .block_root_at_slot(checkpoint_slot, WhenSlotSkipped::Prev) .unwrap() .unwrap(); let wss_state_root = harness @@ -147,7 +150,6 @@ async fn light_client_test() { .state_root_at_slot(checkpoint_slot) .unwrap() .unwrap(); - let wss_block = harness .chain .store @@ -164,14 +166,16 @@ async fn light_client_test() { let mock = mock_execution_layer_from_parts(&harness.spec, harness.runtime.task_executor.clone()); - + + harness.advance_slot(); harness .extend_chain( - num_blocks_produced as usize, + num_final_blocks as usize, BlockStrategy::OnCanonicalHead, AttestationStrategy::AllValidators, ) .await; + // Initialise a new beacon chain from the finalized checkpoint. // The slot clock must be set to a time ahead of the checkpoint state. let slot_clock = TestingSlotClock::new( @@ -213,6 +217,28 @@ async fn light_client_test() { let beacon_chain = Arc::new(beacon_chain); let current_state = harness.get_current_state(); + + let block_root = *current_state + .get_block_root(current_state.slot() - Slot::new(1)) + .unwrap(); + + let contributions = harness.make_sync_contributions( + ¤t_state, + block_root, + current_state.slot() - Slot::new(1), + RelativeSyncCommittee::Current, + ); + + for (_, contribution_and_proof) in contributions { + let contribution = contribution_and_proof + .expect("contribution exists for committee") + .message + .contribution; + beacon_chain.op_pool + .insert_sync_contribution(contribution.clone()) + .unwrap(); + beacon_chain.op_pool.insert_sync_contribution(contribution).unwrap(); + } let sync_aggregate = beacon_chain .op_pool @@ -220,22 +246,100 @@ async fn light_client_test() { .unwrap() .unwrap(); - let block_root = harness - .chain - .block_root_at_slot(Slot::new(0), WhenSlotSkipped::Prev) + beacon_chain + .light_client_server_cache + .recompute_and_cache_updates( + store.clone(), + current_state.slot() - Slot::new(1), + &block_root, + &sync_aggregate, + &log, + &spec + ).unwrap(); + + let sync_period = (current_state.slot() - Slot::new(1)) + .epoch(E::slots_per_epoch()) + .sync_committee_period(&spec).unwrap(); + + let res = beacon_chain.light_client_server_cache.get_light_client_update( + &store, + sync_period, + &spec + ).unwrap(); + + let other = beacon_chain.get_light_client_updates( + sync_period, + 1 + ).unwrap(); + + for i in 0..1000 { + harness.advance_slot(); + + } + + + harness + .extend_chain( + num_final_blocks as usize, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + + + let current_state = harness.get_current_state(); + + let block_root = *current_state + .get_block_root(current_state.slot() - Slot::new(1)) + .unwrap(); + + let contributions = harness.make_sync_contributions( + ¤t_state, + block_root, + current_state.slot() - Slot::new(1), + RelativeSyncCommittee::Current, + ); + + for (_, contribution_and_proof) in contributions { + let contribution = contribution_and_proof + .expect("contribution exists for committee") + .message + .contribution; + beacon_chain.op_pool + .insert_sync_contribution(contribution.clone()) + .unwrap(); + beacon_chain.op_pool.insert_sync_contribution(contribution).unwrap(); + } + + let sync_aggregate = beacon_chain + .op_pool + .get_sync_aggregate(¤t_state) .unwrap() .unwrap(); beacon_chain .light_client_server_cache .recompute_and_cache_updates( - store, - Slot::new(1), + store.clone(), + current_state.slot() - Slot::new(1), &block_root, &sync_aggregate, &log, &spec ).unwrap(); + + let res = beacon_chain.light_client_server_cache.get_light_client_update( + &store, + sync_period, + &spec + ).unwrap(); + + let other = beacon_chain.get_light_client_updates( + sync_period, + 2 + ).unwrap(); + + println!("other len {}", other.len()); } /// Tests that `store.heal_freezer_block_roots_at_split` inserts block roots between last restore point diff --git a/beacon_node/store/src/leveldb_store.rs b/beacon_node/store/src/leveldb_store.rs index ffd55c16a04..98124ffd993 100644 --- a/beacon_node/store/src/leveldb_store.rs +++ b/beacon_node/store/src/leveldb_store.rs @@ -167,7 +167,6 @@ impl KeyValueStore for LevelDB { fn iter_column_from(&self, column: DBColumn, from: &[u8]) -> ColumnIter { let start_key = BytesKey::from_vec(get_key_for_col(column.into(), from)); - let iter = self.db.iter(self.read_options()); iter.seek(&start_key); diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index 1f96f1fc1d3..0f9ae3d2172 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -310,13 +310,13 @@ impl DBColumn { | Self::PubkeyCache | Self::BeaconRestorePoint | Self::DhtEnrs - | Self::OptimisticTransitionBlock - | Self::LightClientUpdate => 32, + | Self::OptimisticTransitionBlock => 32, Self::BeaconBlockRoots | Self::BeaconStateRoots | Self::BeaconHistoricalRoots | Self::BeaconHistoricalSummaries - | Self::BeaconRandaoMixes => 8, + | Self::BeaconRandaoMixes + | Self::LightClientUpdate => 8, } } } diff --git a/consensus/types/src/light_client_optimistic_update.rs b/consensus/types/src/light_client_optimistic_update.rs index 4727673f6c0..552c8f3e867 100644 --- a/consensus/types/src/light_client_optimistic_update.rs +++ b/consensus/types/src/light_client_optimistic_update.rs @@ -167,6 +167,23 @@ impl LightClientOptimisticUpdate { } } } + + // Implements spec prioritization rules: + // > Full nodes SHOULD provide the LightClientOptimisticUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) + // + // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_optimistic_update + pub fn is_latest_optimistic_update( + &self, + attested_slot: Slot, + signature_slot: Slot, + ) -> bool { + let prev_slot = self.get_slot(); + if attested_slot > prev_slot { + true + } else { + attested_slot == prev_slot && signature_slot > *self.signature_slot() + } + } } impl ForkVersionDeserialize for LightClientOptimisticUpdate { diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 968bc4f0b05..3815b8c8bda 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -2,7 +2,9 @@ use super::{EthSpec, FixedVector, Hash256, Slot, SyncAggregate, SyncCommittee}; use crate::{ beacon_state, test_utils::TestRandom, ChainSpec, ForkName, ForkVersionDeserialize, LightClientHeaderAltair, LightClientHeaderCapella, LightClientHeaderDeneb, SignedBeaconBlock, + Epoch, }; +use safe_arith::SafeArith; use derivative::Derivative; use safe_arith::ArithError; use serde::{Deserialize, Deserializer, Serialize}; @@ -229,8 +231,132 @@ impl LightClientUpdate { Ok(update) } + + // Implements spec prioritization rules: + // Full nodes SHOULD provide the best derivable LightClientUpdate for each sync committee period + // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_update + pub fn is_better_light_client_update( + &self, + new: &LightClientUpdate, + chain_spec: &ChainSpec, + ) -> Result { + // Compare super majority (> 2/3) sync committee participation + let max_active_participants = new.sync_aggregate().sync_committee_bits.len(); + let new_active_participants = new.sync_aggregate().sync_committee_bits.num_set_bits(); + let prev_active_participants = self.sync_aggregate().sync_committee_bits.num_set_bits(); + + let new_has_super_majority = + new_active_participants.safe_mul(3)? >= max_active_participants.safe_mul(2)?; + let prev_has_super_majority = + prev_active_participants.safe_mul(3)? >= max_active_participants.safe_mul(2)?; + + if new_has_super_majority != prev_has_super_majority { + return Ok(new_has_super_majority & !prev_has_super_majority); + } + + if !new_has_super_majority && new_active_participants != prev_active_participants { + return Ok(new_active_participants > prev_active_participants); + } + + let new_attested_header_slot = match new { + LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, + LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.attested_header.beacon.slot, + }; + + let new_finalized_header_slot = match new { + LightClientUpdate::Altair(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Capella(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.finalized_header.beacon.slot, + }; + + let prev_attested_header_slot = match self { + LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, + LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.attested_header.beacon.slot, + }; + + let prev_finalized_header_slot = match self { + LightClientUpdate::Altair(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Capella(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Deneb(update) => update.finalized_header.beacon.slot, + }; + + let new_attested_header_sync_committee_period = + compute_sync_committee_period_at_slot::(new_attested_header_slot, chain_spec)?; + + let new_signature_slot_sync_committee_period = + compute_sync_committee_period_at_slot::(*new.signature_slot(), chain_spec)?; + + let prev_attested_header_sync_committee_period = + compute_sync_committee_period_at_slot::(prev_attested_header_slot, chain_spec)?; + + let prev_signature_slot_sync_committee_period = + compute_sync_committee_period_at_slot::(*self.signature_slot(), chain_spec)?; + + // Compare presence of relevant sync committee + let new_has_relevant_sync_committee = new.next_sync_committee_branch().is_empty() + && (new_attested_header_sync_committee_period == new_signature_slot_sync_committee_period); + + let prev_has_relevant_sync_committee = self.next_sync_committee_branch().is_empty() + && (prev_attested_header_sync_committee_period + == prev_signature_slot_sync_committee_period); + + if new_has_relevant_sync_committee != prev_has_relevant_sync_committee { + return Ok(new_has_relevant_sync_committee); + } + + // Compare indication of any finality + let new_has_finality = new.finality_branch().is_empty(); + let prev_has_finality = self.finality_branch().is_empty(); + if new_has_finality != prev_has_finality { + return Ok(new_has_finality); + } + + // Compare sync committee finality + if new_has_finality { + let new_has_sync_committee_finality = + compute_sync_committee_period_at_slot::(new_finalized_header_slot, chain_spec)? + == compute_sync_committee_period_at_slot::( + new_attested_header_slot, + chain_spec, + )?; + + let prev_has_sync_committee_finality = + compute_sync_committee_period_at_slot::(prev_finalized_header_slot, chain_spec)? + == compute_sync_committee_period_at_slot::( + prev_attested_header_slot, + chain_spec, + )?; + + if new_has_sync_committee_finality != prev_has_sync_committee_finality { + return Ok(new_has_sync_committee_finality); + } + } + + // Tiebreaker 1: Sync committee participation beyond super majority + if new_active_participants != prev_active_participants { + return Ok(new_active_participants > prev_active_participants); + } + + // Tiebreaker 2: Prefer older data (fewer changes to best) + if new_attested_header_slot != prev_attested_header_slot { + return Ok(new_attested_header_slot < prev_attested_header_slot); + } + + return Ok(new.signature_slot() < self.signature_slot()); + } +} + +fn compute_sync_committee_period_at_slot( + slot: Slot, + chain_spec: &ChainSpec, +) -> Result { + slot.epoch(E::slots_per_epoch()) + .safe_div(chain_spec.epochs_per_sync_committee_period) } + #[cfg(test)] mod tests { use super::*; diff --git a/testing/ef_tests/src/cases/light_client.rs b/testing/ef_tests/src/cases/light_client.rs new file mode 100644 index 00000000000..e69de29bb2d From 194610082784547805a5b611845aa3030aeada8b Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 3 Jun 2024 18:13:19 +0200 Subject: [PATCH 16/42] started work on ef tests and some code clean up --- .../src/light_client_server_cache.rs | 33 +++---- .../beacon_chain/tests/light_client_tests.rs | 0 beacon_node/beacon_chain/tests/main.rs | 1 - beacon_node/beacon_chain/tests/store_tests.rs | 98 +++++++++++-------- beacon_node/store/src/lib.rs | 2 +- .../src/light_client_optimistic_update.rs | 6 +- consensus/types/src/light_client_update.rs | 41 ++++---- slasher/src/database/interface.rs | 1 - testing/ef_tests/check_all_files_accessed.py | 3 +- testing/ef_tests/src/cases.rs | 2 + testing/ef_tests/src/cases/light_client.rs | 0 .../light_client_verify_is_better_update.rs | 62 ++++++++++++ testing/ef_tests/src/decode.rs | 12 ++- testing/ef_tests/src/error.rs | 3 + testing/ef_tests/src/handler.rs | 25 +++++ testing/ef_tests/tests/tests.rs | 5 + testing/simulator/src/checks.rs | 4 +- testing/simulator/src/cli.rs | 4 +- 18 files changed, 203 insertions(+), 99 deletions(-) delete mode 100644 beacon_node/beacon_chain/tests/light_client_tests.rs delete mode 100644 testing/ef_tests/src/cases/light_client.rs create mode 100644 testing/ef_tests/src/cases/light_client_verify_is_better_update.rs 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 5484bd4bd0a..9fe48a0f9bf 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -1,7 +1,7 @@ use crate::errors::BeaconChainError; use crate::{metrics, BeaconChainTypes, BeaconStore}; use parking_lot::{Mutex, RwLock}; -use safe_arith::{ArithError, SafeArith}; +use safe_arith::SafeArith; use slog::{debug, Logger}; use ssz::Decode; use ssz::Encode; @@ -16,7 +16,7 @@ use types::light_client_update::{ }; use types::non_zero_usize::new_non_zero_usize; use types::{ - BeaconBlockRef, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, Hash256, + BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, SyncCommittee, }; @@ -160,10 +160,7 @@ impl LightClientServerCache { } } - if let Some(finalized_block) = - store.get_full_block(&cached_parts.finalized_block_root)? - { - + if let Some(finalized_block) = store.get_full_block(&cached_parts.finalized_block_root)? { let new_light_client_update = LightClientUpdate::new( sync_aggregate, block_slot, @@ -183,19 +180,14 @@ impl LightClientServerCache { self.get_light_client_update(&store, sync_period, chain_spec)?; if let Some(prev_light_client_update) = prev_light_client_update { - if prev_light_client_update.is_better_light_client_update( - &new_light_client_update, - chain_spec, - )? { - self.store_light_client_update( - &store, - sync_period, - &new_light_client_update, - )?; + if prev_light_client_update + .is_better_light_client_update(&new_light_client_update, chain_spec)? + { + self.store_light_client_update(&store, sync_period, &new_light_client_update)?; } } else { self.store_light_client_update(&store, sync_period, &new_light_client_update)?; - } + } } Ok(()) @@ -236,7 +228,7 @@ impl LightClientServerCache { let fork_name = chain_spec.fork_name_at_epoch(epoch.into()); let light_client_update = - LightClientUpdate::from_ssz_bytes(&light_client_update_bytes, fork_name) + LightClientUpdate::from_ssz_bytes(&light_client_update_bytes, &fork_name) .map_err(store::errors::Error::SszDecodeError)?; return Ok(Some(light_client_update)); @@ -256,8 +248,8 @@ impl LightClientServerCache { let mut light_client_updates = vec![]; for res in store .hot_db - .iter_column_from::>(column.into(), &start_period.to_le_bytes()) - { + .iter_column_from::>(column, &start_period.to_le_bytes()) + { let (sync_committee_bytes, light_client_update_bytes) = res?; let sync_committee_period = u64::from_ssz_bytes(&sync_committee_bytes) .map_err(store::errors::Error::SszDecodeError)?; @@ -267,7 +259,7 @@ impl LightClientServerCache { let fork_name = chain_spec.fork_name_at_epoch(epoch.into()); let light_client_update = - LightClientUpdate::from_ssz_bytes(&light_client_update_bytes, fork_name) + LightClientUpdate::from_ssz_bytes(&light_client_update_bytes, &fork_name) .map_err(store::errors::Error::SszDecodeError)?; light_client_updates.push(light_client_update); @@ -366,4 +358,3 @@ fn is_latest_finality_update( attested_slot == prev_slot && signature_slot > *prev.signature_slot() } } - diff --git a/beacon_node/beacon_chain/tests/light_client_tests.rs b/beacon_node/beacon_chain/tests/light_client_tests.rs deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/beacon_node/beacon_chain/tests/main.rs b/beacon_node/beacon_chain/tests/main.rs index dd891d2baf3..942ce816846 100644 --- a/beacon_node/beacon_chain/tests/main.rs +++ b/beacon_node/beacon_chain/tests/main.rs @@ -11,4 +11,3 @@ mod store_tests; mod sync_committee_verification; mod tests; mod validator_monitor; -mod light_client_tests; diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 137dcbae78a..4820f89a21a 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -5,11 +5,11 @@ use beacon_chain::block_verification_types::RpcBlock; use beacon_chain::builder::BeaconChainBuilder; use beacon_chain::data_availability_checker::AvailableBlock; use beacon_chain::schema_change::migrate_schema; +use beacon_chain::test_utils::RelativeSyncCommittee; use beacon_chain::test_utils::{ mock_execution_layer_from_parts, test_spec, AttestationStrategy, BeaconChainHarness, BlockStrategy, DiskHarnessType, KZG, }; -use beacon_chain::test_utils::RelativeSyncCommittee; use beacon_chain::{ data_availability_checker::MaybeAvailableBlock, historical_blocks::HistoricalBlockError, migrate::MigratorConfig, BeaconChain, BeaconChainError, BeaconChainTypes, BeaconSnapshot, @@ -108,10 +108,9 @@ fn get_harness_generic( } #[tokio::test] -async fn light_client_test() { - let num_blocks_produced = E::slots_per_epoch() * 20; +async fn light_client_updates_test() { let num_final_blocks = E::slots_per_epoch() * 2; - let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9); + let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9); let db_path = tempdir().unwrap(); let log = test_logger(); let spec = test_spec::(); @@ -165,8 +164,8 @@ async fn light_client_test() { let kzg = spec.deneb_fork_epoch.map(|_| KZG.clone()); let mock = - mock_execution_layer_from_parts(&harness.spec, harness.runtime.task_executor.clone()); - + mock_execution_layer_from_parts(&harness.spec, harness.runtime.task_executor.clone()); + harness.advance_slot(); harness .extend_chain( @@ -217,7 +216,7 @@ async fn light_client_test() { let beacon_chain = Arc::new(beacon_chain); let current_state = harness.get_current_state(); - + let block_root = *current_state .get_block_root(current_state.slot() - Slot::new(1)) .unwrap(); @@ -229,23 +228,30 @@ async fn light_client_test() { RelativeSyncCommittee::Current, ); + // generate sync aggregates for (_, contribution_and_proof) in contributions { let contribution = contribution_and_proof .expect("contribution exists for committee") .message .contribution; - beacon_chain.op_pool + beacon_chain + .op_pool .insert_sync_contribution(contribution.clone()) .unwrap(); - beacon_chain.op_pool.insert_sync_contribution(contribution).unwrap(); + beacon_chain + .op_pool + .insert_sync_contribution(contribution) + .unwrap(); } + // check that we can fetch the newly generated sync aggregate let sync_aggregate = beacon_chain .op_pool .get_sync_aggregate(¤t_state) .unwrap() .unwrap(); + // cache light client data beacon_chain .light_client_server_cache .recompute_and_cache_updates( @@ -254,30 +260,35 @@ async fn light_client_test() { &block_root, &sync_aggregate, &log, - &spec - ).unwrap(); + &spec, + ) + .unwrap(); + // calculate the sync period from the previous slot let sync_period = (current_state.slot() - Slot::new(1)) .epoch(E::slots_per_epoch()) - .sync_committee_period(&spec).unwrap(); + .sync_committee_period(&spec) + .unwrap(); + + // fetch a single light client update directly from the db + let res = beacon_chain + .light_client_server_cache + .get_light_client_update(&store, sync_period, &spec) + .unwrap(); - let res = beacon_chain.light_client_server_cache.get_light_client_update( - &store, - sync_period, - &spec - ).unwrap(); + // fetch a range of light client updates. right now there should only be one light client update + // in the db. + let other = beacon_chain + .get_light_client_updates(sync_period, 100) + .unwrap(); - let other = beacon_chain.get_light_client_updates( - sync_period, - 1 - ).unwrap(); + assert_eq!(other.len(), 1); + // advance a bunch of slots & extend the chain for i in 0..1000 { harness.advance_slot(); - } - harness .extend_chain( num_final_blocks as usize, @@ -286,13 +297,12 @@ async fn light_client_test() { ) .await; - let current_state = harness.get_current_state(); let block_root = *current_state .get_block_root(current_state.slot() - Slot::new(1)) .unwrap(); - + let contributions = harness.make_sync_contributions( ¤t_state, block_root, @@ -300,15 +310,20 @@ async fn light_client_test() { RelativeSyncCommittee::Current, ); + // generate new sync aggregates from this new state for (_, contribution_and_proof) in contributions { let contribution = contribution_and_proof .expect("contribution exists for committee") .message .contribution; - beacon_chain.op_pool + beacon_chain + .op_pool .insert_sync_contribution(contribution.clone()) .unwrap(); - beacon_chain.op_pool.insert_sync_contribution(contribution).unwrap(); + beacon_chain + .op_pool + .insert_sync_contribution(contribution) + .unwrap(); } let sync_aggregate = beacon_chain @@ -317,6 +332,7 @@ async fn light_client_test() { .unwrap() .unwrap(); + // cache new light client data beacon_chain .light_client_server_cache .recompute_and_cache_updates( @@ -325,21 +341,25 @@ async fn light_client_test() { &block_root, &sync_aggregate, &log, - &spec - ).unwrap(); + &spec, + ) + .unwrap(); - let res = beacon_chain.light_client_server_cache.get_light_client_update( - &store, - sync_period, - &spec - ).unwrap(); + let res = beacon_chain + .light_client_server_cache + .get_light_client_update(&store, sync_period, &spec) + .unwrap(); - let other = beacon_chain.get_light_client_updates( - sync_period, - 2 - ).unwrap(); + // we should now have two light client updates in the db + let other = beacon_chain + .get_light_client_updates(sync_period, 100) + .unwrap(); + + for o in other.iter() { + println!("{:?}", o.signature_slot()); + } - println!("other len {}", other.len()); + assert_eq!(other.len(), 2); } /// Tests that `store.heal_freezer_block_roots_at_split` inserts block roots between last restore point diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index 0f9ae3d2172..240bd711959 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -315,7 +315,7 @@ impl DBColumn { | Self::BeaconStateRoots | Self::BeaconHistoricalRoots | Self::BeaconHistoricalSummaries - | Self::BeaconRandaoMixes + | Self::BeaconRandaoMixes | Self::LightClientUpdate => 8, } } diff --git a/consensus/types/src/light_client_optimistic_update.rs b/consensus/types/src/light_client_optimistic_update.rs index 552c8f3e867..47c1bd78436 100644 --- a/consensus/types/src/light_client_optimistic_update.rs +++ b/consensus/types/src/light_client_optimistic_update.rs @@ -172,11 +172,7 @@ impl LightClientOptimisticUpdate { // > Full nodes SHOULD provide the LightClientOptimisticUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) // // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_optimistic_update - pub fn is_latest_optimistic_update( - &self, - attested_slot: Slot, - signature_slot: Slot, - ) -> bool { + pub fn is_latest_optimistic_update(&self, attested_slot: Slot, signature_slot: Slot) -> bool { let prev_slot = self.get_slot(); if attested_slot > prev_slot { true diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 3815b8c8bda..13e4a0af8a3 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -1,12 +1,11 @@ use super::{EthSpec, FixedVector, Hash256, Slot, SyncAggregate, SyncCommittee}; use crate::{ - beacon_state, test_utils::TestRandom, ChainSpec, ForkName, ForkVersionDeserialize, + beacon_state, test_utils::TestRandom, ChainSpec, Epoch, ForkName, ForkVersionDeserialize, LightClientHeaderAltair, LightClientHeaderCapella, LightClientHeaderDeneb, SignedBeaconBlock, - Epoch, }; -use safe_arith::SafeArith; use derivative::Derivative; use safe_arith::ArithError; +use safe_arith::SafeArith; use serde::{Deserialize, Deserializer, Serialize}; use serde_json::Value; use ssz::Decode; @@ -213,7 +212,7 @@ impl LightClientUpdate { Ok(light_client_update) } - pub fn from_ssz_bytes(bytes: &[u8], fork_name: ForkName) -> Result { + pub fn from_ssz_bytes(bytes: &[u8], fork_name: &ForkName) -> Result { let update = match fork_name { ForkName::Altair | ForkName::Bellatrix => { Self::Altair(LightClientUpdateAltair::from_ssz_bytes(bytes)?) @@ -271,15 +270,15 @@ impl LightClientUpdate { }; let prev_attested_header_slot = match self { - LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, - LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, - LightClientUpdate::Deneb(update) => update.attested_header.beacon.slot, + LightClientUpdate::Altair(prev) => prev.attested_header.beacon.slot, + LightClientUpdate::Capella(prev) => prev.attested_header.beacon.slot, + LightClientUpdate::Deneb(prev) => prev.attested_header.beacon.slot, }; let prev_finalized_header_slot = match self { - LightClientUpdate::Altair(update) => update.finalized_header.beacon.slot, - LightClientUpdate::Capella(update) => update.finalized_header.beacon.slot, - LightClientUpdate::Deneb(update) => update.finalized_header.beacon.slot, + LightClientUpdate::Altair(prev) => prev.finalized_header.beacon.slot, + LightClientUpdate::Capella(prev) => prev.finalized_header.beacon.slot, + LightClientUpdate::Deneb(prev) => prev.finalized_header.beacon.slot, }; let new_attested_header_sync_committee_period = @@ -295,10 +294,11 @@ impl LightClientUpdate { compute_sync_committee_period_at_slot::(*self.signature_slot(), chain_spec)?; // Compare presence of relevant sync committee - let new_has_relevant_sync_committee = new.next_sync_committee_branch().is_empty() - && (new_attested_header_sync_committee_period == new_signature_slot_sync_committee_period); + let new_has_relevant_sync_committee = !new.next_sync_committee_branch().is_empty() + && (new_attested_header_sync_committee_period + == new_signature_slot_sync_committee_period); - let prev_has_relevant_sync_committee = self.next_sync_committee_branch().is_empty() + let prev_has_relevant_sync_committee = !self.next_sync_committee_branch().is_empty() && (prev_attested_header_sync_committee_period == prev_signature_slot_sync_committee_period); @@ -307,8 +307,8 @@ impl LightClientUpdate { } // Compare indication of any finality - let new_has_finality = new.finality_branch().is_empty(); - let prev_has_finality = self.finality_branch().is_empty(); + let new_has_finality = !new.finality_branch().is_empty(); + let prev_has_finality = !self.finality_branch().is_empty(); if new_has_finality != prev_has_finality { return Ok(new_has_finality); } @@ -317,17 +317,11 @@ impl LightClientUpdate { if new_has_finality { let new_has_sync_committee_finality = compute_sync_committee_period_at_slot::(new_finalized_header_slot, chain_spec)? - == compute_sync_committee_period_at_slot::( - new_attested_header_slot, - chain_spec, - )?; + == new_attested_header_sync_committee_period; let prev_has_sync_committee_finality = compute_sync_committee_period_at_slot::(prev_finalized_header_slot, chain_spec)? - == compute_sync_committee_period_at_slot::( - prev_attested_header_slot, - chain_spec, - )?; + == prev_attested_header_sync_committee_period; if new_has_sync_committee_finality != prev_has_sync_committee_finality { return Ok(new_has_sync_committee_finality); @@ -356,7 +350,6 @@ fn compute_sync_committee_period_at_slot( .safe_div(chain_spec.epochs_per_sync_committee_period) } - #[cfg(test)] mod tests { use super::*; diff --git a/slasher/src/database/interface.rs b/slasher/src/database/interface.rs index 89e25c90816..5bb920383c3 100644 --- a/slasher/src/database/interface.rs +++ b/slasher/src/database/interface.rs @@ -227,5 +227,4 @@ impl<'env> Cursor<'env> { _ => Err(Error::MismatchedDatabaseVariant), } } - } diff --git a/testing/ef_tests/check_all_files_accessed.py b/testing/ef_tests/check_all_files_accessed.py index 7629d61827f..c4562d9c736 100755 --- a/testing/ef_tests/check_all_files_accessed.py +++ b/testing/ef_tests/check_all_files_accessed.py @@ -26,7 +26,8 @@ "tests/.*/.*/ssz_static/Eth1Block/", "tests/.*/.*/ssz_static/PowBlock/", # light_client - "tests/.*/.*/light_client", + # "tests/.*/.*/light_client", + "tests/.*/mainnet/.*/light_client/update_ranking", # LightClientStore "tests/.*/.*/ssz_static/LightClientStore", # LightClientSnapshot diff --git a/testing/ef_tests/src/cases.rs b/testing/ef_tests/src/cases.rs index f328fa64047..2d6f661f0e4 100644 --- a/testing/ef_tests/src/cases.rs +++ b/testing/ef_tests/src/cases.rs @@ -24,6 +24,7 @@ mod kzg_compute_kzg_proof; mod kzg_verify_blob_kzg_proof; mod kzg_verify_blob_kzg_proof_batch; mod kzg_verify_kzg_proof; +mod light_client_verify_is_better_update; mod merkle_proof_validity; mod operations; mod rewards; @@ -54,6 +55,7 @@ pub use kzg_compute_kzg_proof::*; pub use kzg_verify_blob_kzg_proof::*; pub use kzg_verify_blob_kzg_proof_batch::*; pub use kzg_verify_kzg_proof::*; +pub use light_client_verify_is_better_update::*; pub use merkle_proof_validity::*; pub use operations::*; pub use rewards::RewardsTest; diff --git a/testing/ef_tests/src/cases/light_client.rs b/testing/ef_tests/src/cases/light_client.rs deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs b/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs new file mode 100644 index 00000000000..948352ffd2e --- /dev/null +++ b/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs @@ -0,0 +1,62 @@ +use super::*; +use decode::ssz_decode_light_client_update; +use serde::Deserialize; +use types::LightClientUpdate; + +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct LightClientVerifyIsBetterUpdate { + light_client_updates: Vec>, +} + +#[derive(Debug, Clone, Default, Deserialize)] +pub struct Metadata { + updates_count: u64, +} + +impl LoadCase for LightClientVerifyIsBetterUpdate { + fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { + let mut light_client_updates = vec![]; + let metadata: Metadata = decode::yaml_decode_file(path.join("meta.yaml").as_path())?; + for index in 0..metadata.updates_count { + let light_client_update = ssz_decode_light_client_update( + &path.join(format!("updates_{}.ssz_snappy", index)), + &fork_name, + )?; + light_client_updates.push(light_client_update); + } + + Ok(Self { + light_client_updates, + }) + } +} + +impl Case for LightClientVerifyIsBetterUpdate { + // Light client updates in `self.light_client_updates` are ordered in descending precedence + // where the update at index = 0 is considered the best update. This test iterates through + // all light client updates in a nested loop to make all possible comparisons. If a light client update + // at index `i`` is considered 'better' than a light client update at index `j`` when `i > j`, this test fails. + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + let spec = fork_name.make_genesis_spec(E::default_spec()); + for (i, ith_light_client_update) in self.light_client_updates.iter().enumerate() { + for (j, jth_light_client_update) in self.light_client_updates.iter().enumerate() { + if i == j { + continue; + } + + let is_better_update = ith_light_client_update + .is_better_light_client_update(jth_light_client_update, &spec) + .unwrap(); + + if (is_better_update && (i < j)) || (!is_better_update && (i > j)) { + return Err(Error::FailedComparison( + format!("Light client update at index {} should not be considered a better update than the light client update at index {}", i, j) + )); + } + } + } + + Ok(()) + } +} diff --git a/testing/ef_tests/src/decode.rs b/testing/ef_tests/src/decode.rs index 51ab682f3dc..757b9bf3c43 100644 --- a/testing/ef_tests/src/decode.rs +++ b/testing/ef_tests/src/decode.rs @@ -5,7 +5,7 @@ use std::fs::{self}; use std::io::Write; use std::path::Path; use std::path::PathBuf; -use types::BeaconState; +use types::{BeaconState, LightClientUpdate}; /// See `log_file_access` for details. const ACCESSED_FILE_LOG_FILENAME: &str = ".accessed_file_log.txt"; @@ -95,3 +95,13 @@ pub fn ssz_decode_state( log_file_access(path); ssz_decode_file_with(path, |bytes| BeaconState::from_ssz_bytes(bytes, spec)) } + +pub fn ssz_decode_light_client_update( + path: &Path, + fork_name: &ForkName, +) -> Result, Error> { + log_file_access(path); + ssz_decode_file_with(path, |bytes| { + LightClientUpdate::from_ssz_bytes(bytes, fork_name) + }) +} diff --git a/testing/ef_tests/src/error.rs b/testing/ef_tests/src/error.rs index c5795777ada..389308377c7 100644 --- a/testing/ef_tests/src/error.rs +++ b/testing/ef_tests/src/error.rs @@ -14,6 +14,8 @@ pub enum Error { SkippedKnownFailure, /// The test failed due to some internal error preventing the test from running. InternalError(String), + /// The test failed while making some comparison. + FailedComparison(String), } impl Error { @@ -26,6 +28,7 @@ impl Error { Error::SkippedBls => "SkippedBls", Error::SkippedKnownFailure => "SkippedKnownFailure", Error::InternalError(_) => "InternalError", + Error::FailedComparison(_) => "FailedComparison", } } diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index 2d5ea4149ef..5aeb910834e 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -828,6 +828,31 @@ impl Handler for KzgInclusionMerkleProofValidityHandler(PhantomData); + +impl Handler for LightClientUpdateHandler { + type Case = cases::LightClientVerifyIsBetterUpdate; + + fn config_name() -> &'static str { + E::name() + } + + fn runner_name() -> &'static str { + "light_client" + } + + fn handler_name(&self) -> String { + "update_ranking".into() + } + + fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { + // Enabled in Altair + fork_name != ForkName::Base + } +} + #[derive(Derivative)] #[derivative(Default(bound = ""))] pub struct OperationsHandler(PhantomData<(E, O)>); diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 5226c7ac2b0..1f7cee439d0 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -731,6 +731,11 @@ fn merkle_proof_validity() { MerkleProofValidityHandler::::default().run(); } +#[test] +fn light_client_update() { + LightClientUpdateHandler::::default().run(); +} + #[test] #[cfg(feature = "fake_crypto")] fn kzg_inclusion_merkle_proof_validity() { diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index bc3bfb13f0b..5ea03286001 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -357,9 +357,7 @@ pub(crate) async fn verify_light_client_updates( } }; } - return Err(format!( - "we succeeded in returning light client data :)" - )); + Ok(()) } diff --git a/testing/simulator/src/cli.rs b/testing/simulator/src/cli.rs index ce39d9b249a..191ce449497 100644 --- a/testing/simulator/src/cli.rs +++ b/testing/simulator/src/cli.rs @@ -56,7 +56,7 @@ pub fn cli_app() -> Command { .help("Set the severity level of the logs."), ) .arg( - Arg::with_name("continue-after-checks") + Arg::new("continue-after-checks") .long("continue_after_checks") .action(ArgAction::SetTrue) .help("Continue after checks (default false)"), @@ -114,7 +114,7 @@ pub fn cli_app() -> Command { .help("Set the severity level of the logs."), ) .arg( - Arg::with_name("continue-after-checks") + Arg::new("continue-after-checks") .long("continue_after_checks") .action(ArgAction::SetTrue) .help("Continue after checks (default false)"), From 2b1e47de8aab964b56fb0d073fc56d2984e0e206 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 4 Jun 2024 17:08:05 +0200 Subject: [PATCH 17/42] update tests --- consensus/types/src/light_client_update.rs | 43 ++++++++++++++++---- testing/ef_tests/check_all_files_accessed.py | 3 +- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 13e4a0af8a3..f6a2f53f744 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -33,6 +33,9 @@ pub const CURRENT_SYNC_COMMITTEE_PROOF_LEN: usize = 5; pub const NEXT_SYNC_COMMITTEE_PROOF_LEN: usize = 5; pub const EXECUTION_PAYLOAD_PROOF_LEN: usize = 4; +type FinalityBranch = FixedVector; +type NextSyncCommitteeBranch = FixedVector; + #[derive(Debug, PartialEq, Clone)] pub enum Error { SszTypesError(ssz_types::Error), @@ -113,7 +116,7 @@ pub struct LightClientUpdate { /// The `SyncCommittee` used in the next period. pub next_sync_committee: Arc>, /// Merkle proof for next sync committee - pub next_sync_committee_branch: FixedVector, + pub next_sync_committee_branch: NextSyncCommitteeBranch, /// The last `BeaconBlockHeader` from the last attested finalized block (end of epoch). #[superstruct(only(Altair), partial_getter(rename = "finalized_header_altair"))] pub finalized_header: LightClientHeaderAltair, @@ -122,7 +125,7 @@ pub struct LightClientUpdate { #[superstruct(only(Deneb), partial_getter(rename = "finalized_header_deneb"))] pub finalized_header: LightClientHeaderDeneb, /// Merkle proof attesting finalized header. - pub finality_branch: FixedVector, + pub finality_branch: FinalityBranch, /// current sync aggreggate pub sync_aggregate: SyncAggregate, /// Slot of the sync aggregated signature @@ -241,6 +244,7 @@ impl LightClientUpdate { ) -> Result { // Compare super majority (> 2/3) sync committee participation let max_active_participants = new.sync_aggregate().sync_committee_bits.len(); + let new_active_participants = new.sync_aggregate().sync_committee_bits.num_set_bits(); let prev_active_participants = self.sync_aggregate().sync_committee_bits.num_set_bits(); @@ -294,11 +298,11 @@ impl LightClientUpdate { compute_sync_committee_period_at_slot::(*self.signature_slot(), chain_spec)?; // Compare presence of relevant sync committee - let new_has_relevant_sync_committee = !new.next_sync_committee_branch().is_empty() + let new_has_relevant_sync_committee = !new.is_next_sync_committee_branch_empty() && (new_attested_header_sync_committee_period == new_signature_slot_sync_committee_period); - let prev_has_relevant_sync_committee = !self.next_sync_committee_branch().is_empty() + let prev_has_relevant_sync_committee = !self.is_next_sync_committee_branch_empty() && (prev_attested_header_sync_committee_period == prev_signature_slot_sync_committee_period); @@ -307,8 +311,8 @@ impl LightClientUpdate { } // Compare indication of any finality - let new_has_finality = !new.finality_branch().is_empty(); - let prev_has_finality = !self.finality_branch().is_empty(); + let new_has_finality = !new.is_finality_branch_empty(); + let prev_has_finality = !self.is_finality_branch_empty(); if new_has_finality != prev_has_finality { return Ok(new_has_finality); } @@ -340,14 +344,37 @@ impl LightClientUpdate { return Ok(new.signature_slot() < self.signature_slot()); } + + fn is_next_sync_committee_branch_empty(&self) -> bool { + for index in self.next_sync_committee_branch().iter() { + if index.clone() != Hash256::default() { + return false + } + } + true + } + + fn is_finality_branch_empty(&self) -> bool { + for index in self.finality_branch().iter() { + if index.clone() != Hash256::default() { + return false + } + } + true + } } fn compute_sync_committee_period_at_slot( slot: Slot, chain_spec: &ChainSpec, ) -> Result { - slot.epoch(E::slots_per_epoch()) - .safe_div(chain_spec.epochs_per_sync_committee_period) + println!("slot {}", slot); + let x = slot.epoch(E::slots_per_epoch()) + .safe_div(chain_spec.epochs_per_sync_committee_period).unwrap(); + + println!("sync comm period {}", x); + + Ok(x) } #[cfg(test)] diff --git a/testing/ef_tests/check_all_files_accessed.py b/testing/ef_tests/check_all_files_accessed.py index c4562d9c736..7772618bc84 100755 --- a/testing/ef_tests/check_all_files_accessed.py +++ b/testing/ef_tests/check_all_files_accessed.py @@ -27,7 +27,8 @@ "tests/.*/.*/ssz_static/PowBlock/", # light_client # "tests/.*/.*/light_client", - "tests/.*/mainnet/.*/light_client/update_ranking", + "tests/.*/.*/light_client/single_merkle_proof", + "tests/.*/.*/light_client/sync", # LightClientStore "tests/.*/.*/ssz_static/LightClientStore", # LightClientSnapshot From 211601cac318a2d0eb5cc25c6dba327114fd3ae0 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 4 Jun 2024 17:25:02 +0200 Subject: [PATCH 18/42] linting --- .../src/light_client_server_cache.rs | 5 ++--- consensus/types/src/light_client_update.rs | 16 +++++++++------- 2 files changed, 11 insertions(+), 10 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 9fe48a0f9bf..cdd275ff1eb 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -16,9 +16,8 @@ use types::light_client_update::{ }; use types::non_zero_usize::new_non_zero_usize; use types::{ - BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, - LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, - SyncCommittee, + BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, + LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, SyncCommittee, }; /// A prev block cache miss requires to re-generate the state of the post-parent block. Items in the diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index f6a2f53f744..7af698d3701 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -244,7 +244,7 @@ impl LightClientUpdate { ) -> Result { // Compare super majority (> 2/3) sync committee participation let max_active_participants = new.sync_aggregate().sync_committee_bits.len(); - + let new_active_participants = new.sync_aggregate().sync_committee_bits.num_set_bits(); let prev_active_participants = self.sync_aggregate().sync_committee_bits.num_set_bits(); @@ -347,8 +347,8 @@ impl LightClientUpdate { fn is_next_sync_committee_branch_empty(&self) -> bool { for index in self.next_sync_committee_branch().iter() { - if index.clone() != Hash256::default() { - return false + if *index != Hash256::default() { + return false; } } true @@ -356,8 +356,8 @@ impl LightClientUpdate { fn is_finality_branch_empty(&self) -> bool { for index in self.finality_branch().iter() { - if index.clone() != Hash256::default() { - return false + if *index != Hash256::default() { + return false; } } true @@ -369,8 +369,10 @@ fn compute_sync_committee_period_at_slot( chain_spec: &ChainSpec, ) -> Result { println!("slot {}", slot); - let x = slot.epoch(E::slots_per_epoch()) - .safe_div(chain_spec.epochs_per_sync_committee_period).unwrap(); + let x = slot + .epoch(E::slots_per_epoch()) + .safe_div(chain_spec.epochs_per_sync_committee_period) + .unwrap(); println!("sync comm period {}", x); From 891d67716070312b5f01ff7e12909aa09bd969f4 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 5 Jun 2024 11:41:03 +0200 Subject: [PATCH 19/42] noop pre altair, were still failing on electra though --- beacon_node/beacon_chain/tests/store_tests.rs | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 4820f89a21a..3cd04f96984 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -109,11 +109,17 @@ fn get_harness_generic( #[tokio::test] async fn light_client_updates_test() { + let spec = test_spec::(); + let Some(_) = spec.altair_fork_epoch else { + // No-op prior to Altair. + return; + }; + let num_final_blocks = E::slots_per_epoch() * 2; let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9); let db_path = tempdir().unwrap(); let log = test_logger(); - let spec = test_spec::(); + let seconds_per_slot = spec.seconds_per_slot; let store = get_store_generic( &db_path, @@ -137,7 +143,6 @@ async fn light_client_updates_test() { &all_validators, ) .await; - let (genesis_state, genesis_state_root) = harness.get_current_state_and_root(); let wss_block_root = harness .chain @@ -271,21 +276,23 @@ async fn light_client_updates_test() { .unwrap(); // fetch a single light client update directly from the db - let res = beacon_chain + let lc_update = beacon_chain .light_client_server_cache .get_light_client_update(&store, sync_period, &spec) + .unwrap() .unwrap(); // fetch a range of light client updates. right now there should only be one light client update // in the db. - let other = beacon_chain + let lc_updates = beacon_chain .get_light_client_updates(sync_period, 100) .unwrap(); - assert_eq!(other.len(), 1); + assert_eq!(lc_updates.len(), 1); + assert_eq!(lc_updates.first().unwrap().clone(), lc_update); // advance a bunch of slots & extend the chain - for i in 0..1000 { + for _i in 0..1000 { harness.advance_slot(); } @@ -345,21 +352,12 @@ async fn light_client_updates_test() { ) .unwrap(); - let res = beacon_chain - .light_client_server_cache - .get_light_client_update(&store, sync_period, &spec) - .unwrap(); - // we should now have two light client updates in the db - let other = beacon_chain + let lc_updates = beacon_chain .get_light_client_updates(sync_period, 100) .unwrap(); - for o in other.iter() { - println!("{:?}", o.signature_slot()); - } - - assert_eq!(other.len(), 2); + assert_eq!(lc_updates.len(), 2); } /// Tests that `store.heal_freezer_block_roots_at_split` inserts block roots between last restore point From d729feab40767feddb6eb1e3db4b0b01e9d7b277 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 10 Jun 2024 18:51:07 +0200 Subject: [PATCH 20/42] allow for zeroed light client header --- .../src/light_client_server_cache.rs | 92 ++++++++----------- beacon_node/beacon_chain/tests/store_tests.rs | 5 + .../types/src/light_client_finality_update.rs | 17 ++++ consensus/types/src/light_client_header.rs | 31 +++++++ consensus/types/src/light_client_update.rs | 29 ++++-- 5 files changed, 114 insertions(+), 60 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 cdd275ff1eb..e8d9857c704 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -17,7 +17,8 @@ use types::light_client_update::{ use types::non_zero_usize::new_non_zero_usize; use types::{ BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, - LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, SyncCommittee, + LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, + SyncCommittee, }; /// A prev block cache miss requires to re-generate the state of the post-parent block. Items in the @@ -129,23 +130,55 @@ impl LightClientServerCache { )?); }; + let maybe_finalized_block_root = + store.get_full_block(&cached_parts.finalized_block_root)?; + + let new_light_client_update = LightClientUpdate::new( + sync_aggregate, + block_slot, + cached_parts.next_sync_committee, + cached_parts.next_sync_committee_branch, + cached_parts.finality_branch.clone(), + &attested_block, + maybe_finalized_block_root.as_ref(), + chain_spec, + )?; + + let sync_period = block_slot + .epoch(T::EthSpec::slots_per_epoch()) + .sync_committee_period(chain_spec)?; + + let prev_light_client_update = + self.get_light_client_update(&store, sync_period, chain_spec)?; + + let should_persist_light_client_update = + if let Some(prev_light_client_update) = prev_light_client_update { + prev_light_client_update + .is_better_light_client_update(&new_light_client_update, chain_spec)? + } else { + true + }; + + if should_persist_light_client_update { + self.store_light_client_update(&store, sync_period, &new_light_client_update)?; + } + // Spec: Full nodes SHOULD provide the LightClientFinalityUpdate with the highest // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice let is_latest_finality = match &self.latest_finality_update.read().clone() { Some(latest_finality_update) => { - is_latest_finality_update(latest_finality_update, attested_slot, signature_slot) + latest_finality_update.is_latest_finality_update(attested_slot, signature_slot) } None => true, }; + if is_latest_finality & !cached_parts.finalized_block_root.is_zero() { // Immediately after checkpoint sync the finalized block may not be available yet. - if let Some(finalized_block) = - store.get_full_block(&cached_parts.finalized_block_root)? - { + if let Some(finalized_block) = maybe_finalized_block_root { *self.latest_finality_update.write() = Some(LightClientFinalityUpdate::new( &attested_block, &finalized_block, - cached_parts.finality_branch.clone(), + cached_parts.finality_branch, sync_aggregate.clone(), signature_slot, chain_spec, @@ -159,36 +192,6 @@ impl LightClientServerCache { } } - if let Some(finalized_block) = store.get_full_block(&cached_parts.finalized_block_root)? { - let new_light_client_update = LightClientUpdate::new( - sync_aggregate, - block_slot, - cached_parts.next_sync_committee, - cached_parts.next_sync_committee_branch, - cached_parts.finality_branch, - &attested_block, - &finalized_block, - chain_spec, - )?; - - let sync_period = block_slot - .epoch(T::EthSpec::slots_per_epoch()) - .sync_committee_period(chain_spec)?; - - let prev_light_client_update = - self.get_light_client_update(&store, sync_period, chain_spec)?; - - if let Some(prev_light_client_update) = prev_light_client_update { - if prev_light_client_update - .is_better_light_client_update(&new_light_client_update, chain_spec)? - { - self.store_light_client_update(&store, sync_period, &new_light_client_update)?; - } - } else { - self.store_light_client_update(&store, sync_period, &new_light_client_update)?; - } - } - Ok(()) } @@ -340,20 +343,3 @@ impl LightClientCachedData { }) } } - -// Implements spec prioritization rules: -// > Full nodes SHOULD provide the LightClientFinalityUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) -// -// ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_finality_update -fn is_latest_finality_update( - prev: &LightClientFinalityUpdate, - attested_slot: Slot, - signature_slot: Slot, -) -> bool { - let prev_slot = prev.get_attested_header_slot(); - if attested_slot > prev_slot { - true - } else { - attested_slot == prev_slot && signature_slot > *prev.signature_slot() - } -} diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 3cd04f96984..718ba7a221c 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -222,6 +222,11 @@ async fn light_client_updates_test() { let current_state = harness.get_current_state(); + if ForkName::Electra == current_state.fork_name_unchecked() { + // TODO(electra) fix beacon state `compute_merkle_proof` + return; + } + let block_root = *current_state .get_block_root(current_state.slot() - Slot::new(1)) .unwrap(); diff --git a/consensus/types/src/light_client_finality_update.rs b/consensus/types/src/light_client_finality_update.rs index 29c526e2916..d4d009fb2ce 100644 --- a/consensus/types/src/light_client_finality_update.rs +++ b/consensus/types/src/light_client_finality_update.rs @@ -181,6 +181,23 @@ impl LightClientFinalityUpdate { } } } + + // Implements spec prioritization rules: + // > Full nodes SHOULD provide the LightClientFinalityUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) + // + // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_finality_update + pub fn is_latest_finality_update( + &self, + attested_slot: Slot, + signature_slot: Slot, + ) -> bool { + let prev_slot = self.get_attested_header_slot(); + if attested_slot > prev_slot { + true + } else { + attested_slot == prev_slot && signature_slot > *self.signature_slot() + } + } } impl ForkVersionDeserialize for LightClientFinalityUpdate { diff --git a/consensus/types/src/light_client_header.rs b/consensus/types/src/light_client_header.rs index 213ec90f955..610e5cebdb3 100644 --- a/consensus/types/src/light_client_header.rs +++ b/consensus/types/src/light_client_header.rs @@ -136,6 +136,15 @@ impl LightClientHeaderAltair { } } +impl Default for LightClientHeaderAltair { + fn default() -> Self { + Self { + beacon: BeaconBlockHeader::empty(), + _phantom_data: PhantomData, + } + } +} + impl LightClientHeaderCapella { pub fn block_to_light_client_header(block: &SignedBeaconBlock) -> Result { let payload = block @@ -164,6 +173,17 @@ impl LightClientHeaderCapella { } } +impl Default for LightClientHeaderCapella { + fn default() -> Self { + Self { + beacon: BeaconBlockHeader::empty(), + execution: ExecutionPayloadHeaderCapella::default(), + execution_branch: FixedVector::default(), + _phantom_data: PhantomData, + } + } +} + impl LightClientHeaderDeneb { pub fn block_to_light_client_header(block: &SignedBeaconBlock) -> Result { let payload = block @@ -192,6 +212,17 @@ impl LightClientHeaderDeneb { } } +impl Default for LightClientHeaderDeneb { + fn default() -> Self { + Self { + beacon: BeaconBlockHeader::empty(), + execution: ExecutionPayloadHeaderDeneb::default(), + execution_branch: FixedVector::default(), + _phantom_data: PhantomData, + } + } +} + impl ForkVersionDeserialize for LightClientHeader { fn deserialize_by_fork<'de, D: serde::Deserializer<'de>>( value: serde_json::value::Value, diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 7af698d3701..72c49facdfd 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -157,7 +157,7 @@ impl LightClientUpdate { next_sync_committee_branch: FixedVector, finality_branch: FixedVector, attested_block: &SignedBeaconBlock, - finalized_block: &SignedBeaconBlock, + finalized_block: Option<&SignedBeaconBlock>, chain_spec: &ChainSpec, ) -> Result { let light_client_update = match attested_block @@ -168,8 +168,13 @@ impl LightClientUpdate { ForkName::Altair | ForkName::Bellatrix => { let attested_header = LightClientHeaderAltair::block_to_light_client_header(attested_block)?; - let finalized_header = - LightClientHeaderAltair::block_to_light_client_header(finalized_block)?; + + let finalized_header = if let Some(finalized_block) = finalized_block { + LightClientHeaderAltair::block_to_light_client_header(finalized_block)? + } else { + LightClientHeaderAltair::default() + }; + Self::Altair(LightClientUpdateAltair { attested_header, next_sync_committee, @@ -183,8 +188,13 @@ impl LightClientUpdate { ForkName::Capella => { let attested_header = LightClientHeaderCapella::block_to_light_client_header(attested_block)?; - let finalized_header = - LightClientHeaderCapella::block_to_light_client_header(finalized_block)?; + + let finalized_header = if let Some(finalized_block) = finalized_block { + LightClientHeaderCapella::block_to_light_client_header(finalized_block)? + } else { + LightClientHeaderCapella::default() + }; + Self::Capella(LightClientUpdateCapella { attested_header, next_sync_committee, @@ -198,8 +208,13 @@ impl LightClientUpdate { ForkName::Deneb | ForkName::Electra => { let attested_header = LightClientHeaderDeneb::block_to_light_client_header(attested_block)?; - let finalized_header = - LightClientHeaderDeneb::block_to_light_client_header(finalized_block)?; + + let finalized_header = if let Some(finalized_block) = finalized_block { + LightClientHeaderDeneb::block_to_light_client_header(finalized_block)? + } else { + LightClientHeaderDeneb::default() + }; + Self::Deneb(LightClientUpdateDeneb { attested_header, next_sync_committee, From fe2b43d6b7d8a0d8b2b973635713ea534b729ff7 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 10 Jun 2024 18:52:38 +0200 Subject: [PATCH 21/42] merge unstable --- beacon_node/beacon_chain/src/light_client_server_cache.rs | 1 - 1 file changed, 1 deletion(-) 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 e8d9857c704..968f4b396fd 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -24,7 +24,6 @@ use types::{ /// A prev block cache miss requires to re-generate the state of the post-parent block. Items in the /// prev block cache are very small 32 * (6 + 1) = 224 bytes. 32 is an arbitrary number that /// represents unlikely re-orgs, while keeping the cache very small. -// TODO(lightclientupdate) this cache size has now increased const PREV_BLOCK_CACHE_SIZE: NonZeroUsize = new_non_zero_usize(32); /// This cache computes light client messages ahead of time, required to satisfy p2p and API From 1774e63aa053f23457f247f307758cba27c42a3d Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 10 Jun 2024 19:06:40 +0200 Subject: [PATCH 22/42] remove unwraps --- .../src/light_client_server_cache.rs | 3 +- beacon_node/http_api/src/light_client.rs | 29 ++++++++++++++----- .../types/src/light_client_finality_update.rs | 6 +--- 3 files changed, 23 insertions(+), 15 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 968f4b396fd..3f967acf88a 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -17,8 +17,7 @@ use types::light_client_update::{ use types::non_zero_usize::new_non_zero_usize; use types::{ BeaconBlockRef, BeaconState, ChainSpec, EthSpec, ForkName, Hash256, LightClientFinalityUpdate, - LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, - SyncCommittee, + LightClientOptimisticUpdate, LightClientUpdate, Slot, SyncAggregate, SyncCommittee, }; /// A prev block cache miss requires to re-generate the state of the post-parent block. Items in the diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index 339978d3de6..856d71ed1cb 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -56,7 +56,7 @@ pub fn get_light_client_updates( let fork_versioned_response = light_client_updates .iter() .map(|update| map_light_client_update_to_json_response::(&chain, update.clone())) - .collect::>>>(); + .collect::>>, Rejection>>()?; Ok(warp::reply::json(&fork_versioned_response).into_response()) } } @@ -74,10 +74,16 @@ pub fn validate_light_client_updates_request( let current_sync_period = chain .epoch() - .unwrap() + .map_err(|_| { + warp_utils::reject::custom_server_error("failed to get current epoch".to_string()) + })? .sync_committee_period(&chain.spec) - .unwrap(); - println!("start period {:?}", query.start_period); + .map_err(|_| { + warp_utils::reject::custom_server_error( + "failed to get current sync committee period".to_string(), + ) + })?; + if query.start_period > current_sync_period { return Err(warp_utils::reject::custom_bad_request( "Invalid sync committee period requested 1".to_string(), @@ -87,9 +93,16 @@ pub fn validate_light_client_updates_request( let earliest_altair_sync_committee = chain .spec .altair_fork_epoch - .unwrap() + .ok_or(warp_utils::reject::custom_server_error( + "failed to get altair fork epoch".to_string(), + ))? .sync_committee_period(&chain.spec) - .unwrap(); + .map_err(|_| { + warp_utils::reject::custom_server_error( + "failed to get earliest altair sync committee".to_string(), + ) + })?; + if query.start_period < earliest_altair_sync_committee { return Err(warp_utils::reject::custom_bad_request( "Invalid sync committee period requested 2".to_string(), @@ -143,10 +156,10 @@ fn map_light_client_update_to_ssz_chunk( fn map_light_client_update_to_json_response( chain: &BeaconChain, light_client_update: LightClientUpdate, -) -> ForkVersionedResponse> { +) -> Result>, Rejection> { let fork_name = chain .spec .fork_name_at_slot::(*light_client_update.signature_slot()); - fork_versioned_response(V1, fork_name, light_client_update).unwrap() + fork_versioned_response(V1, fork_name, light_client_update) } diff --git a/consensus/types/src/light_client_finality_update.rs b/consensus/types/src/light_client_finality_update.rs index d4d009fb2ce..39aacac9919 100644 --- a/consensus/types/src/light_client_finality_update.rs +++ b/consensus/types/src/light_client_finality_update.rs @@ -186,11 +186,7 @@ impl LightClientFinalityUpdate { // > Full nodes SHOULD provide the LightClientFinalityUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) // // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_finality_update - pub fn is_latest_finality_update( - &self, - attested_slot: Slot, - signature_slot: Slot, - ) -> bool { + pub fn is_latest_finality_update(&self, attested_slot: Slot, signature_slot: Slot) -> bool { let prev_slot = self.get_attested_header_slot(); if attested_slot > prev_slot { true From af9b20d8f59b889399b3073b671aa86453380f1a Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 10 Jun 2024 19:07:49 +0200 Subject: [PATCH 23/42] remove unwraps --- beacon_node/http_api/src/light_client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index 856d71ed1cb..34468079103 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -86,7 +86,7 @@ pub fn validate_light_client_updates_request( if query.start_period > current_sync_period { return Err(warp_utils::reject::custom_bad_request( - "Invalid sync committee period requested 1".to_string(), + "Invalid sync committee period requested".to_string(), )); } @@ -105,7 +105,7 @@ pub fn validate_light_client_updates_request( if query.start_period < earliest_altair_sync_committee { return Err(warp_utils::reject::custom_bad_request( - "Invalid sync committee period requested 2".to_string(), + "Invalid sync committee period requested".to_string(), )); } From d8e6891e231a1102a26291d6a97d236db90fc69b Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 25 Jun 2024 10:48:40 +0200 Subject: [PATCH 24/42] Update light_client_update.rs --- consensus/types/src/light_client_update.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 72c49facdfd..d2601bd8a80 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -302,13 +302,10 @@ impl LightClientUpdate { let new_attested_header_sync_committee_period = compute_sync_committee_period_at_slot::(new_attested_header_slot, chain_spec)?; - let new_signature_slot_sync_committee_period = compute_sync_committee_period_at_slot::(*new.signature_slot(), chain_spec)?; - let prev_attested_header_sync_committee_period = compute_sync_committee_period_at_slot::(prev_attested_header_slot, chain_spec)?; - let prev_signature_slot_sync_committee_period = compute_sync_committee_period_at_slot::(*self.signature_slot(), chain_spec)?; From fa7cd252268da7ad77c92ff87ed726a69a6271d0 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 25 Jun 2024 16:12:28 +0200 Subject: [PATCH 25/42] move functionality to helper methods --- consensus/types/src/light_client_update.rs | 44 +++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index f26b46e01b8..4c3b9e33975 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -293,6 +293,24 @@ impl LightClientUpdate { } } + pub fn attested_header_sync_committee_period(&self, chain_spec: &ChainSpec) -> Result { + compute_sync_committee_period_at_slot::(self.attested_header_slot(), chain_spec) + .map_err(Error::ArithError) + } + + pub fn signature_slot_sync_committee_period(&self, chain_spec: &ChainSpec) -> Result { + compute_sync_committee_period_at_slot::(*self.signature_slot(), chain_spec) + .map_err(Error::ArithError) + } + + pub fn is_sync_committee_update(&self, chain_spec: &ChainSpec) -> Result { + let new_has_relevant_sync_committee = !self.is_next_sync_committee_branch_empty() + && (self.attested_header_sync_committee_period(chain_spec)? + == self.signature_slot_sync_committee_period(chain_spec)?); + + Ok(new_has_relevant_sync_committee) + } + // Implements spec prioritization rules: // Full nodes SHOULD provide the best derivable LightClientUpdate for each sync committee period @@ -314,7 +332,7 @@ impl LightClientUpdate { prev_active_participants.safe_mul(3)? >= max_active_participants.safe_mul(2)?; if new_has_super_majority != prev_has_super_majority { - return Ok(new_has_super_majority & !prev_has_super_majority); + return Ok(new_has_super_majority); } if !new_has_super_majority && new_active_participants != prev_active_participants { @@ -326,26 +344,10 @@ impl LightClientUpdate { let prev_attested_header_slot = self.attested_header_slot(); let prev_finalized_header_slot = self.finalized_header_slot(); - let new_attested_header_sync_committee_period = - compute_sync_committee_period_at_slot::(new_attested_header_slot, chain_spec)?; - - let new_signature_slot_sync_committee_period = - compute_sync_committee_period_at_slot::(*new.signature_slot(), chain_spec)?; - - let prev_attested_header_sync_committee_period = - compute_sync_committee_period_at_slot::(prev_attested_header_slot, chain_spec)?; - - let prev_signature_slot_sync_committee_period = - compute_sync_committee_period_at_slot::(*self.signature_slot(), chain_spec)?; - // Compare presence of relevant sync committee - let new_has_relevant_sync_committee = !new.is_next_sync_committee_branch_empty() - && (new_attested_header_sync_committee_period - == new_signature_slot_sync_committee_period); + let new_has_relevant_sync_committee = new.is_sync_committee_update(chain_spec)?; - let prev_has_relevant_sync_committee = !self.is_next_sync_committee_branch_empty() - && (prev_attested_header_sync_committee_period - == prev_signature_slot_sync_committee_period); + let prev_has_relevant_sync_committee = !self.is_sync_committee_update(chain_spec)?; if new_has_relevant_sync_committee != prev_has_relevant_sync_committee { return Ok(new_has_relevant_sync_committee); @@ -362,11 +364,11 @@ impl LightClientUpdate { if new_has_finality { let new_has_sync_committee_finality = compute_sync_committee_period_at_slot::(new_finalized_header_slot, chain_spec)? - == new_attested_header_sync_committee_period; + == new.attested_header_sync_committee_period(chain_spec)?; let prev_has_sync_committee_finality = compute_sync_committee_period_at_slot::(prev_finalized_header_slot, chain_spec)? - == prev_attested_header_sync_committee_period; + == self.attested_header_sync_committee_period(chain_spec)?; if new_has_sync_committee_finality != prev_has_sync_committee_finality { return Ok(new_has_sync_committee_finality); From bb69c482fd46a380108ac68674d066a607600580 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 25 Jun 2024 16:39:16 +0200 Subject: [PATCH 26/42] refactor is best update fn --- .../src/light_client_server_cache.rs | 9 ++-- consensus/types/src/light_client_update.rs | 43 ++++++++++--------- 2 files changed, 26 insertions(+), 26 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 90da2b51e08..d9818d924f1 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -131,8 +131,7 @@ impl LightClientServerCache { )?); }; - let maybe_finalized_block = - store.get_full_block(&cached_parts.finalized_block_root)?; + let maybe_finalized_block = store.get_full_block(&cached_parts.finalized_block_root)?; let new_light_client_update = LightClientUpdate::new( sync_aggregate, @@ -150,12 +149,10 @@ impl LightClientServerCache { .sync_committee_period(chain_spec)?; let prev_light_client_update = match &self.latest_light_client_update.read().clone() { - Some(prev_light_client_update) => { - Some(prev_light_client_update.clone()) - }, + Some(prev_light_client_update) => Some(prev_light_client_update.clone()), None => self.get_light_client_update(&store, sync_period, chain_spec)?, }; - + let should_persist_light_client_update = if let Some(prev_light_client_update) = prev_light_client_update { prev_light_client_update diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 4c3b9e33975..c9f64de91ef 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -232,7 +232,7 @@ impl LightClientUpdate { } ForkName::Electra => { let attested_header = - LightClientHeaderElectra::block_to_light_client_header(attested_block)?; + LightClientHeaderElectra::block_to_light_client_header(attested_block)?; let finalized_header = if let Some(finalized_block) = finalized_block { LightClientHeaderElectra::block_to_light_client_header(finalized_block)? @@ -240,7 +240,7 @@ impl LightClientUpdate { LightClientHeaderElectra::default() }; - Self::Electra(LightClientUpdateElectra { + Self::Electra(LightClientUpdateElectra { attested_header, next_sync_committee, next_sync_committee_branch, @@ -293,31 +293,41 @@ impl LightClientUpdate { } } - pub fn attested_header_sync_committee_period(&self, chain_spec: &ChainSpec) -> Result { + fn attested_header_sync_committee_period( + &self, + chain_spec: &ChainSpec, + ) -> Result { compute_sync_committee_period_at_slot::(self.attested_header_slot(), chain_spec) .map_err(Error::ArithError) } - pub fn signature_slot_sync_committee_period(&self, chain_spec: &ChainSpec) -> Result { + fn signature_slot_sync_committee_period(&self, chain_spec: &ChainSpec) -> Result { compute_sync_committee_period_at_slot::(*self.signature_slot(), chain_spec) .map_err(Error::ArithError) } - pub fn is_sync_committee_update(&self, chain_spec: &ChainSpec) -> Result { + fn is_sync_committee_update(&self, chain_spec: &ChainSpec) -> Result { let new_has_relevant_sync_committee = !self.is_next_sync_committee_branch_empty() - && (self.attested_header_sync_committee_period(chain_spec)? - == self.signature_slot_sync_committee_period(chain_spec)?); + && (self.attested_header_sync_committee_period(chain_spec)? + == self.signature_slot_sync_committee_period(chain_spec)?); Ok(new_has_relevant_sync_committee) } + fn has_sync_committee_finality(&self, chain_spec: &ChainSpec) -> Result { + let has_sync_committee_finality = + compute_sync_committee_period_at_slot::(self.finalized_header_slot(), chain_spec)? + == self.attested_header_sync_committee_period(chain_spec)?; + + Ok(has_sync_committee_finality) + } // Implements spec prioritization rules: // Full nodes SHOULD provide the best derivable LightClientUpdate for each sync committee period // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_update pub fn is_better_light_client_update( &self, - new: &LightClientUpdate, + new: &Self, chain_spec: &ChainSpec, ) -> Result { // Compare super majority (> 2/3) sync committee participation @@ -339,14 +349,8 @@ impl LightClientUpdate { return Ok(new_active_participants > prev_active_participants); } - let new_attested_header_slot = new.attested_header_slot(); - let new_finalized_header_slot = new.finalized_header_slot(); - let prev_attested_header_slot = self.attested_header_slot(); - let prev_finalized_header_slot = self.finalized_header_slot(); - // Compare presence of relevant sync committee let new_has_relevant_sync_committee = new.is_sync_committee_update(chain_spec)?; - let prev_has_relevant_sync_committee = !self.is_sync_committee_update(chain_spec)?; if new_has_relevant_sync_committee != prev_has_relevant_sync_committee { @@ -362,13 +366,9 @@ impl LightClientUpdate { // Compare sync committee finality if new_has_finality { - let new_has_sync_committee_finality = - compute_sync_committee_period_at_slot::(new_finalized_header_slot, chain_spec)? - == new.attested_header_sync_committee_period(chain_spec)?; + let new_has_sync_committee_finality = new.has_sync_committee_finality(chain_spec)?; - let prev_has_sync_committee_finality = - compute_sync_committee_period_at_slot::(prev_finalized_header_slot, chain_spec)? - == self.attested_header_sync_committee_period(chain_spec)?; + let prev_has_sync_committee_finality = self.has_sync_committee_finality(chain_spec)?; if new_has_sync_committee_finality != prev_has_sync_committee_finality { return Ok(new_has_sync_committee_finality); @@ -380,6 +380,9 @@ impl LightClientUpdate { return Ok(new_active_participants > prev_active_participants); } + let new_attested_header_slot = new.attested_header_slot(); + let prev_attested_header_slot = self.attested_header_slot(); + // Tiebreaker 2: Prefer older data (fewer changes to best) if new_attested_header_slot != prev_attested_header_slot { return Ok(new_attested_header_slot < prev_attested_header_slot); From 89ae007498c0e394a0524043798489d0be20883a Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 25 Jun 2024 16:45:33 +0200 Subject: [PATCH 27/42] improve organization of light client server cache logic --- .../src/light_client_server_cache.rs | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 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 d9818d924f1..ee6da136c6b 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -111,25 +111,7 @@ impl LightClientServerCache { attested_block.slot(), )?; - let attested_slot = attested_block.slot(); - - // Spec: Full nodes SHOULD provide the LightClientOptimisticUpdate with the highest - // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice - let is_latest_optimistic = match &self.latest_optimistic_update.read().clone() { - Some(latest_optimistic_update) => { - latest_optimistic_update.is_latest_optimistic_update(attested_slot, signature_slot) - } - None => true, - }; - if is_latest_optimistic { - // can create an optimistic update, that is more recent - *self.latest_optimistic_update.write() = Some(LightClientOptimisticUpdate::new( - &attested_block, - sync_aggregate.clone(), - signature_slot, - chain_spec, - )?); - }; + let attested_slot = attested_block.slot(); let maybe_finalized_block = store.get_full_block(&cached_parts.finalized_block_root)?; @@ -148,6 +130,8 @@ impl LightClientServerCache { .epoch(T::EthSpec::slots_per_epoch()) .sync_committee_period(chain_spec)?; + // Spec: Full nodes SHOULD provide the best derivable LightClientUpdate (according to is_better_update) + // for each sync committee period let prev_light_client_update = match &self.latest_light_client_update.read().clone() { Some(prev_light_client_update) => Some(prev_light_client_update.clone()), None => self.get_light_client_update(&store, sync_period, chain_spec)?, @@ -165,6 +149,24 @@ impl LightClientServerCache { self.store_light_client_update(&store, sync_period, &new_light_client_update)?; } + // Spec: Full nodes SHOULD provide the LightClientOptimisticUpdate with the highest + // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice + let is_latest_optimistic = match &self.latest_optimistic_update.read().clone() { + Some(latest_optimistic_update) => { + latest_optimistic_update.is_latest_optimistic_update(attested_slot, signature_slot) + } + None => true, + }; + if is_latest_optimistic { + // can create an optimistic update, that is more recent + *self.latest_optimistic_update.write() = Some(LightClientOptimisticUpdate::new( + &attested_block, + sync_aggregate.clone(), + signature_slot, + chain_spec, + )?); + }; + // Spec: Full nodes SHOULD provide the LightClientFinalityUpdate with the highest // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice let is_latest_finality = match &self.latest_finality_update.read().clone() { From 7fcf032cd6e1aacf8d52ac0bff3bf5f2950c09e9 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 25 Jun 2024 17:41:54 +0200 Subject: [PATCH 28/42] fork diget calc, and only spawn as many blcoks as we need for the lc update test --- beacon_node/beacon_chain/tests/store_tests.rs | 4 +-- beacon_node/http_api/src/light_client.rs | 27 +------------------ 2 files changed, 3 insertions(+), 28 deletions(-) diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 569db922d50..c093bc68688 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -296,8 +296,8 @@ async fn light_client_updates_test() { assert_eq!(lc_updates.len(), 1); assert_eq!(lc_updates.first().unwrap().clone(), lc_update); - // advance a bunch of slots & extend the chain - for _i in 0..1000 { + // Advance to the next sync committee period + for _i in 0..(E::slots_per_epoch() * u64::from(spec.epochs_per_sync_committee_period)) { harness.advance_slot(); } diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index 34468079103..86899ebe31e 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -120,32 +120,7 @@ fn map_light_client_update_to_ssz_chunk( .spec .fork_name_at_slot::(*light_client_update.signature_slot()); - let fork_digest = match fork_name { - types::ForkName::Base => ChainSpec::compute_fork_digest( - chain.spec.genesis_fork_version, - chain.genesis_validators_root, - ), - types::ForkName::Altair => ChainSpec::compute_fork_digest( - chain.spec.altair_fork_version, - chain.genesis_validators_root, - ), - types::ForkName::Bellatrix => ChainSpec::compute_fork_digest( - chain.spec.bellatrix_fork_version, - chain.genesis_validators_root, - ), - types::ForkName::Capella => ChainSpec::compute_fork_digest( - chain.spec.capella_fork_version, - chain.genesis_validators_root, - ), - types::ForkName::Deneb => ChainSpec::compute_fork_digest( - chain.spec.deneb_fork_version, - chain.genesis_validators_root, - ), - types::ForkName::Electra => ChainSpec::compute_fork_digest( - chain.spec.electra_fork_version, - chain.genesis_validators_root, - ), - }; + let fork_digest =ChainSpec::compute_fork_digest(chain.spec.fork_version_for_name(fork_name), chain.genesis_validators_root); LightClientUpdateResponseChunk { context: fork_digest, From 8bf1bc62bc44e6b3e7631c5d293972184dd941c6 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 26 Jun 2024 13:46:13 +0200 Subject: [PATCH 29/42] fetch lc update from the cache if it exists --- .../beacon_chain/src/light_client_server_cache.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 ee6da136c6b..fa2c0e3da08 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -218,12 +218,24 @@ impl LightClientServerCache { Ok(()) } - pub fn get_light_client_update( + // Used to fetch the most recently persisted "best" light client update. + // Should not be used outside the light client server, as it also caches the fetched + // light client update. + fn get_light_client_update( &self, store: &BeaconStore, sync_committee_period: u64, chain_spec: &ChainSpec, ) -> Result>, BeaconChainError> { + if let Some(latest_light_client_update) = self.latest_light_client_update.read().clone() { + let latest_lc_update_sync_committee_period = latest_light_client_update.signature_slot() + .epoch(T::EthSpec::slots_per_epoch()) + .sync_committee_period(chain_spec)?; + if latest_lc_update_sync_committee_period == sync_committee_period { + return Ok(Some(latest_light_client_update)); + } + } + let column = DBColumn::LightClientUpdate; let res = store .hot_db @@ -239,6 +251,7 @@ impl LightClientServerCache { LightClientUpdate::from_ssz_bytes(&light_client_update_bytes, &fork_name) .map_err(store::errors::Error::SszDecodeError)?; + *self.latest_light_client_update.write() = Some(light_client_update.clone()); return Ok(Some(light_client_update)); } From f70b93e797e195253bde423a496bd958e03c96b7 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 26 Jun 2024 13:54:14 +0200 Subject: [PATCH 30/42] fmt --- .../beacon_chain/src/light_client_server_cache.rs | 11 ++++++----- beacon_node/http_api/src/light_client.rs | 5 ++++- 2 files changed, 10 insertions(+), 6 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 fa2c0e3da08..401c7cb133e 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -111,7 +111,7 @@ impl LightClientServerCache { attested_block.slot(), )?; - let attested_slot = attested_block.slot(); + let attested_slot = attested_block.slot(); let maybe_finalized_block = store.get_full_block(&cached_parts.finalized_block_root)?; @@ -130,7 +130,7 @@ impl LightClientServerCache { .epoch(T::EthSpec::slots_per_epoch()) .sync_committee_period(chain_spec)?; - // Spec: Full nodes SHOULD provide the best derivable LightClientUpdate (according to is_better_update) + // Spec: Full nodes SHOULD provide the best derivable LightClientUpdate (according to is_better_update) // for each sync committee period let prev_light_client_update = match &self.latest_light_client_update.read().clone() { Some(prev_light_client_update) => Some(prev_light_client_update.clone()), @@ -219,7 +219,7 @@ impl LightClientServerCache { } // Used to fetch the most recently persisted "best" light client update. - // Should not be used outside the light client server, as it also caches the fetched + // Should not be used outside the light client server, as it also caches the fetched // light client update. fn get_light_client_update( &self, @@ -228,14 +228,15 @@ impl LightClientServerCache { chain_spec: &ChainSpec, ) -> Result>, BeaconChainError> { if let Some(latest_light_client_update) = self.latest_light_client_update.read().clone() { - let latest_lc_update_sync_committee_period = latest_light_client_update.signature_slot() + let latest_lc_update_sync_committee_period = latest_light_client_update + .signature_slot() .epoch(T::EthSpec::slots_per_epoch()) .sync_committee_period(chain_spec)?; if latest_lc_update_sync_committee_period == sync_committee_period { return Ok(Some(latest_light_client_update)); } } - + let column = DBColumn::LightClientUpdate; let res = store .hot_db diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index 86899ebe31e..a6543114b85 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -120,7 +120,10 @@ fn map_light_client_update_to_ssz_chunk( .spec .fork_name_at_slot::(*light_client_update.signature_slot()); - let fork_digest =ChainSpec::compute_fork_digest(chain.spec.fork_version_for_name(fork_name), chain.genesis_validators_root); + let fork_digest = ChainSpec::compute_fork_digest( + chain.spec.fork_version_for_name(fork_name), + chain.genesis_validators_root, + ); LightClientUpdateResponseChunk { context: fork_digest, From eb19b0d2225de3cdd9c7e4137abd22ad6f814c1f Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 5 Jul 2024 10:24:04 +0200 Subject: [PATCH 31/42] Fix beacon_chain tests --- beacon_node/beacon_chain/tests/store_tests.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index c093bc68688..18638e36d92 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -280,13 +280,6 @@ async fn light_client_updates_test() { .sync_committee_period(&spec) .unwrap(); - // fetch a single light client update directly from the db - let lc_update = beacon_chain - .light_client_server_cache - .get_light_client_update(&store, sync_period, &spec) - .unwrap() - .unwrap(); - // fetch a range of light client updates. right now there should only be one light client update // in the db. let lc_updates = beacon_chain @@ -294,7 +287,6 @@ async fn light_client_updates_test() { .unwrap(); assert_eq!(lc_updates.len(), 1); - assert_eq!(lc_updates.first().unwrap().clone(), lc_update); // Advance to the next sync committee period for _i in 0..(E::slots_per_epoch() * u64::from(spec.epochs_per_sync_committee_period)) { From bcda61b5a2996a9d518194811181dbb90b3b6211 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 5 Jul 2024 11:35:05 +0200 Subject: [PATCH 32/42] Add debug code to update ranking_order ef test --- consensus/types/src/light_client_update.rs | 30 ++++------ .../light_client_verify_is_better_update.rs | 57 +++++++++++++++++-- 2 files changed, 62 insertions(+), 25 deletions(-) diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index c9f64de91ef..769b1a3566d 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -275,7 +275,7 @@ impl LightClientUpdate { Ok(update) } - fn attested_header_slot(&self) -> Slot { + pub fn attested_header_slot(&self) -> Slot { match self { LightClientUpdate::Altair(update) => update.attested_header.beacon.slot, LightClientUpdate::Capella(update) => update.attested_header.beacon.slot, @@ -306,20 +306,17 @@ impl LightClientUpdate { .map_err(Error::ArithError) } - fn is_sync_committee_update(&self, chain_spec: &ChainSpec) -> Result { - let new_has_relevant_sync_committee = !self.is_next_sync_committee_branch_empty() + pub fn is_sync_committee_update(&self, chain_spec: &ChainSpec) -> Result { + Ok(!self.is_next_sync_committee_branch_empty() && (self.attested_header_sync_committee_period(chain_spec)? - == self.signature_slot_sync_committee_period(chain_spec)?); - - Ok(new_has_relevant_sync_committee) + == self.signature_slot_sync_committee_period(chain_spec)?)) } - fn has_sync_committee_finality(&self, chain_spec: &ChainSpec) -> Result { - let has_sync_committee_finality = + pub fn has_sync_committee_finality(&self, chain_spec: &ChainSpec) -> Result { + Ok( compute_sync_committee_period_at_slot::(self.finalized_header_slot(), chain_spec)? - == self.attested_header_sync_committee_period(chain_spec)?; - - Ok(has_sync_committee_finality) + == self.attested_header_sync_committee_period(chain_spec)?, + ) } // Implements spec prioritization rules: @@ -400,7 +397,7 @@ impl LightClientUpdate { true } - fn is_finality_branch_empty(&self) -> bool { + pub fn is_finality_branch_empty(&self) -> bool { for index in self.finality_branch().iter() { if *index != Hash256::default() { return false; @@ -414,15 +411,8 @@ fn compute_sync_committee_period_at_slot( slot: Slot, chain_spec: &ChainSpec, ) -> Result { - println!("slot {}", slot); - let x = slot - .epoch(E::slots_per_epoch()) + slot.epoch(E::slots_per_epoch()) .safe_div(chain_spec.epochs_per_sync_committee_period) - .unwrap(); - - println!("sync comm period {}", x); - - Ok(x) } #[cfg(test)] diff --git a/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs b/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs index 948352ffd2e..337ffba1ed7 100644 --- a/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs +++ b/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs @@ -1,7 +1,7 @@ use super::*; use decode::ssz_decode_light_client_update; use serde::Deserialize; -use types::LightClientUpdate; +use types::{LightClientUpdate, Slot}; #[derive(Debug, Clone, Deserialize)] #[serde(deny_unknown_fields)] @@ -49,10 +49,29 @@ impl Case for LightClientVerifyIsBetterUpdate { .is_better_light_client_update(jth_light_client_update, &spec) .unwrap(); - if (is_better_update && (i < j)) || (!is_better_update && (i > j)) { - return Err(Error::FailedComparison( - format!("Light client update at index {} should not be considered a better update than the light client update at index {}", i, j) - )); + let ith_summary = + LightClientUpdateSummary::from_update(ith_light_client_update, &spec); + let jth_summary = + LightClientUpdateSummary::from_update(jth_light_client_update, &spec); + + let (best_index, other_index, best_update, other_update, failed) = if i < j { + // i is better, so is_better_update must return false + (i, j, ith_summary, jth_summary, is_better_update) + } else { + // j is better, so is_better must return true + (j, i, jth_summary, ith_summary, !is_better_update) + }; + + if failed { + eprintln!("is_better_update: {is_better_update}"); + eprintln!("index {best_index} update {best_update:?}"); + eprintln!("index {other_index} update {other_update:?}"); + eprintln!( + "update at index {best_index} must be considered better than update at index {other_index}" + ); + return Err(Error::FailedComparison(format!( + "update at index {best_index} must be considered better than update at index {other_index}" + ))); } } } @@ -60,3 +79,31 @@ impl Case for LightClientVerifyIsBetterUpdate { Ok(()) } } + +#[derive(Debug)] +#[allow(dead_code)] +struct LightClientUpdateSummary { + participants: usize, + supermajority: bool, + relevant_sync_committee: bool, + has_finality: bool, + has_sync_committee_finality: bool, + header_slot: Slot, + signature_slot: Slot, +} + +impl LightClientUpdateSummary { + fn from_update(update: &LightClientUpdate, spec: &ChainSpec) -> Self { + let max_participants = update.sync_aggregate().sync_committee_bits.len(); + let participants = update.sync_aggregate().sync_committee_bits.num_set_bits(); + Self { + participants, + supermajority: participants * 3 > max_participants * 2, + relevant_sync_committee: update.is_sync_committee_update(spec).unwrap(), + has_finality: !update.is_finality_branch_empty(), + has_sync_committee_finality: update.has_sync_committee_finality(spec).unwrap(), + header_slot: update.attested_header_slot(), + signature_slot: *update.signature_slot(), + } + } +} From e8691dba041f057f51a305f1b90748438bd382f9 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 5 Jul 2024 11:44:44 +0200 Subject: [PATCH 33/42] Fix compare code --- consensus/types/src/light_client_update.rs | 2 +- .../ef_tests/src/cases/light_client_verify_is_better_update.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 769b1a3566d..6159003a6a3 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -348,7 +348,7 @@ impl LightClientUpdate { // Compare presence of relevant sync committee let new_has_relevant_sync_committee = new.is_sync_committee_update(chain_spec)?; - let prev_has_relevant_sync_committee = !self.is_sync_committee_update(chain_spec)?; + let prev_has_relevant_sync_committee = self.is_sync_committee_update(chain_spec)?; if new_has_relevant_sync_committee != prev_has_relevant_sync_committee { return Ok(new_has_relevant_sync_committee); diff --git a/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs b/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs index 337ffba1ed7..de281d906c1 100644 --- a/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs +++ b/testing/ef_tests/src/cases/light_client_verify_is_better_update.rs @@ -41,6 +41,7 @@ impl Case for LightClientVerifyIsBetterUpdate { let spec = fork_name.make_genesis_spec(E::default_spec()); for (i, ith_light_client_update) in self.light_client_updates.iter().enumerate() { for (j, jth_light_client_update) in self.light_client_updates.iter().enumerate() { + eprintln!("{i} {j}"); if i == j { continue; } From b8741afc588a96e8230f0a32c0368fb3694b16a1 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 29 Jul 2024 15:58:37 -0700 Subject: [PATCH 34/42] fix test --- beacon_node/beacon_chain/src/light_client_server_cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 401c7cb133e..e70454b6daa 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -221,7 +221,7 @@ impl LightClientServerCache { // Used to fetch the most recently persisted "best" light client update. // Should not be used outside the light client server, as it also caches the fetched // light client update. - fn get_light_client_update( + pub fn get_light_client_update( &self, store: &BeaconStore, sync_committee_period: u64, From a1f2408beaf65a46644685e66602622c67d95269 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 30 Jul 2024 10:59:58 +1000 Subject: [PATCH 35/42] Use blinded blocks for light client proofs --- beacon_node/beacon_chain/src/beacon_chain.rs | 7 +- .../src/light_client_server_cache.rs | 15 ++- consensus/types/src/beacon_block_body.rs | 94 ++++++------------- consensus/types/src/light_client_bootstrap.rs | 6 +- .../types/src/light_client_finality_update.rs | 6 +- consensus/types/src/light_client_header.rs | 41 +++++--- .../src/light_client_optimistic_update.rs | 4 +- consensus/types/src/light_client_update.rs | 6 +- 8 files changed, 72 insertions(+), 107 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index c6ed979d681..4bc98a98da0 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -6766,12 +6766,7 @@ impl BeaconChain { &self, block_root: &Hash256, ) -> Result, ForkName)>, Error> { - let handle = self - .task_executor - .handle() - .ok_or(BeaconChainError::RuntimeShutdown)?; - - let Some(block) = handle.block_on(async { self.get_block(block_root).await })? else { + let Some(block) = self.get_blinded_block(block_root)? else { return Ok(None); }; 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 ca029057373..87513885f77 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -84,13 +84,12 @@ impl LightClientServerCache { let signature_slot = block_slot; let attested_block_root = block_parent_root; - let attested_block = - store - .get_full_block(attested_block_root)? - .ok_or(BeaconChainError::DBInconsistent(format!( - "Block not available {:?}", - attested_block_root - )))?; + let attested_block = store.get_blinded_block(attested_block_root)?.ok_or( + BeaconChainError::DBInconsistent(format!( + "Block not available {:?}", + attested_block_root + )), + )?; let cached_parts = self.get_or_compute_prev_block_cache( store.clone(), @@ -130,7 +129,7 @@ impl LightClientServerCache { if is_latest_finality & !cached_parts.finalized_block_root.is_zero() { // Immediately after checkpoint sync the finalized block may not be available yet. if let Some(finalized_block) = - store.get_full_block(&cached_parts.finalized_block_root)? + store.get_blinded_block(&cached_parts.finalized_block_root)? { *self.latest_finality_update.write() = Some(LightClientFinalityUpdate::new( &attested_block, diff --git a/consensus/types/src/beacon_block_body.rs b/consensus/types/src/beacon_block_body.rs index 363ba08f7d5..373e165e0bb 100644 --- a/consensus/types/src/beacon_block_body.rs +++ b/consensus/types/src/beacon_block_body.rs @@ -129,6 +129,11 @@ impl> BeaconBlockBody { pub fn execution_payload(&self) -> Result, Error> { self.to_ref().execution_payload() } + + /// Returns the name of the fork pertaining to `self`. + pub fn fork_name(&self) -> ForkName { + self.to_ref().fork_name() + } } impl<'a, E: EthSpec, Payload: AbstractExecPayload> BeaconBlockBodyRef<'a, E, Payload> { @@ -239,6 +244,28 @@ impl<'a, E: EthSpec, Payload: AbstractExecPayload> BeaconBlockBodyRef<'a, E, Ok(proof.into()) } + pub fn block_body_merkle_proof(&self, generalized_index: usize) -> Result, Error> { + let field_index = match generalized_index { + light_client_update::EXECUTION_PAYLOAD_INDEX => { + // Execution payload is a top-level field, subtract off the generalized indices + // for the internal nodes. Result should be 9, the field offset of the execution + // payload in the `BeaconBlockBody`: + // https://github.com/ethereum/consensus-specs/blob/dev/specs/deneb/beacon-chain.md#beaconblockbody + generalized_index + .checked_sub(NUM_BEACON_BLOCK_BODY_HASH_TREE_ROOT_LEAVES) + .ok_or(Error::IndexNotSupported(generalized_index))? + } + _ => return Err(Error::IndexNotSupported(generalized_index)), + }; + + let leaves = self.body_merkle_leaves(); + let depth = light_client_update::EXECUTION_PAYLOAD_PROOF_LEN; + let tree = merkle_proof::MerkleTree::create(&leaves, depth); + let (_, proof) = tree.generate_proof(field_index, depth)?; + + Ok(proof) + } + /// Return `true` if this block body has a non-zero number of blobs. pub fn has_blobs(self) -> bool { self.blob_kzg_commitments() @@ -832,73 +859,6 @@ impl From>> } } -impl BeaconBlockBody { - /// Returns the name of the fork pertaining to `self`. - pub fn fork_name(&self) -> ForkName { - self.to_ref().fork_name() - } - - pub fn block_body_merkle_proof(&self, generalized_index: usize) -> Result, Error> { - let field_index = match generalized_index { - light_client_update::EXECUTION_PAYLOAD_INDEX => { - // Execution payload is a top-level field, subtract off the generalized indices - // for the internal nodes. Result should be 9, the field offset of the execution - // payload in the `BeaconBlockBody`: - // https://github.com/ethereum/consensus-specs/blob/dev/specs/deneb/beacon-chain.md#beaconblockbody - generalized_index - .checked_sub(NUM_BEACON_BLOCK_BODY_HASH_TREE_ROOT_LEAVES) - .ok_or(Error::IndexNotSupported(generalized_index))? - } - _ => return Err(Error::IndexNotSupported(generalized_index)), - }; - - let attestations_root = if self.fork_name() > ForkName::Electra { - self.attestations_electra()?.tree_hash_root() - } else { - self.attestations_base()?.tree_hash_root() - }; - - let attester_slashings_root = if self.fork_name() > ForkName::Electra { - self.attester_slashings_electra()?.tree_hash_root() - } else { - self.attester_slashings_base()?.tree_hash_root() - }; - - let mut leaves = vec![ - self.randao_reveal().tree_hash_root(), - self.eth1_data().tree_hash_root(), - self.graffiti().tree_hash_root(), - self.proposer_slashings().tree_hash_root(), - attester_slashings_root, - attestations_root, - self.deposits().tree_hash_root(), - self.voluntary_exits().tree_hash_root(), - ]; - - if let Ok(sync_aggregate) = self.sync_aggregate() { - leaves.push(sync_aggregate.tree_hash_root()) - } - - if let Ok(execution_payload) = self.execution_payload() { - leaves.push(execution_payload.tree_hash_root()) - } - - if let Ok(bls_to_execution_changes) = self.bls_to_execution_changes() { - leaves.push(bls_to_execution_changes.tree_hash_root()) - } - - if let Ok(blob_kzg_commitments) = self.blob_kzg_commitments() { - leaves.push(blob_kzg_commitments.tree_hash_root()) - } - - let depth = light_client_update::EXECUTION_PAYLOAD_PROOF_LEN; - let tree = merkle_proof::MerkleTree::create(&leaves, depth); - let (_, proof) = tree.generate_proof(field_index, depth)?; - - Ok(proof) - } -} - /// Util method helpful for logging. pub fn format_kzg_commitments(commitments: &[KzgCommitment]) -> String { let commitment_strings: Vec = commitments.iter().map(|x| x.to_string()).collect(); diff --git a/consensus/types/src/light_client_bootstrap.rs b/consensus/types/src/light_client_bootstrap.rs index e3a85744ded..f06a94adce9 100644 --- a/consensus/types/src/light_client_bootstrap.rs +++ b/consensus/types/src/light_client_bootstrap.rs @@ -1,8 +1,8 @@ use crate::{ light_client_update::*, test_utils::TestRandom, BeaconState, ChainSpec, EthSpec, FixedVector, ForkName, ForkVersionDeserialize, Hash256, LightClientHeader, LightClientHeaderAltair, - LightClientHeaderCapella, LightClientHeaderDeneb, LightClientHeaderElectra, SignedBeaconBlock, - Slot, SyncCommittee, + LightClientHeaderCapella, LightClientHeaderDeneb, LightClientHeaderElectra, + SignedBlindedBeaconBlock, Slot, SyncCommittee, }; use derivative::Derivative; use serde::{Deserialize, Deserializer, Serialize}; @@ -114,7 +114,7 @@ impl LightClientBootstrap { pub fn from_beacon_state( beacon_state: &mut BeaconState, - block: &SignedBeaconBlock, + block: &SignedBlindedBeaconBlock, chain_spec: &ChainSpec, ) -> Result { let mut header = beacon_state.latest_block_header().clone(); diff --git a/consensus/types/src/light_client_finality_update.rs b/consensus/types/src/light_client_finality_update.rs index a9e24e03db1..e65b0572923 100644 --- a/consensus/types/src/light_client_finality_update.rs +++ b/consensus/types/src/light_client_finality_update.rs @@ -3,7 +3,7 @@ use crate::ChainSpec; use crate::{ light_client_update::*, test_utils::TestRandom, ForkName, ForkVersionDeserialize, LightClientHeaderAltair, LightClientHeaderCapella, LightClientHeaderDeneb, - LightClientHeaderElectra, SignedBeaconBlock, + LightClientHeaderElectra, SignedBlindedBeaconBlock, }; use derivative::Derivative; use serde::{Deserialize, Deserializer, Serialize}; @@ -73,8 +73,8 @@ pub struct LightClientFinalityUpdate { impl LightClientFinalityUpdate { pub fn new( - attested_block: &SignedBeaconBlock, - finalized_block: &SignedBeaconBlock, + attested_block: &SignedBlindedBeaconBlock, + finalized_block: &SignedBlindedBeaconBlock, finality_branch: FixedVector, sync_aggregate: SyncAggregate, signature_slot: Slot, diff --git a/consensus/types/src/light_client_header.rs b/consensus/types/src/light_client_header.rs index 1d6432ed6f3..1feb748fae1 100644 --- a/consensus/types/src/light_client_header.rs +++ b/consensus/types/src/light_client_header.rs @@ -4,7 +4,7 @@ use crate::ForkVersionDeserialize; use crate::{light_client_update::*, BeaconBlockBody}; use crate::{ test_utils::TestRandom, EthSpec, ExecutionPayloadHeaderCapella, ExecutionPayloadHeaderDeneb, - ExecutionPayloadHeaderElectra, FixedVector, Hash256, SignedBeaconBlock, + ExecutionPayloadHeaderElectra, FixedVector, Hash256, SignedBlindedBeaconBlock, }; use crate::{BeaconBlockHeader, ExecutionPayloadHeader}; use derivative::Derivative; @@ -72,7 +72,7 @@ pub struct LightClientHeader { impl LightClientHeader { pub fn block_to_light_client_header( - block: &SignedBeaconBlock, + block: &SignedBlindedBeaconBlock, chain_spec: &ChainSpec, ) -> Result { let header = match block @@ -139,7 +139,9 @@ impl LightClientHeader { } impl LightClientHeaderAltair { - pub fn block_to_light_client_header(block: &SignedBeaconBlock) -> Result { + pub fn block_to_light_client_header( + block: &SignedBlindedBeaconBlock, + ) -> Result { Ok(LightClientHeaderAltair { beacon: block.message().block_header(), _phantom_data: PhantomData, @@ -148,7 +150,9 @@ impl LightClientHeaderAltair { } impl LightClientHeaderCapella { - pub fn block_to_light_client_header(block: &SignedBeaconBlock) -> Result { + pub fn block_to_light_client_header( + block: &SignedBlindedBeaconBlock, + ) -> Result { let payload = block .message() .execution_payload()? @@ -163,8 +167,9 @@ impl LightClientHeaderCapella { .to_owned(), ); - let execution_branch = - beacon_block_body.block_body_merkle_proof(EXECUTION_PAYLOAD_INDEX)?; + let execution_branch = beacon_block_body + .to_ref() + .block_body_merkle_proof(EXECUTION_PAYLOAD_INDEX)?; return Ok(LightClientHeaderCapella { beacon: block.message().block_header(), @@ -176,13 +181,15 @@ impl LightClientHeaderCapella { } impl LightClientHeaderDeneb { - pub fn block_to_light_client_header(block: &SignedBeaconBlock) -> Result { - let payload = block + pub fn block_to_light_client_header( + block: &SignedBlindedBeaconBlock, + ) -> Result { + let header = block .message() .execution_payload()? - .execution_payload_deneb()?; + .execution_payload_deneb()? + .clone(); - let header = ExecutionPayloadHeaderDeneb::from(payload); let beacon_block_body = BeaconBlockBody::from( block .message() @@ -191,8 +198,9 @@ impl LightClientHeaderDeneb { .to_owned(), ); - let execution_branch = - beacon_block_body.block_body_merkle_proof(EXECUTION_PAYLOAD_INDEX)?; + let execution_branch = beacon_block_body + .to_ref() + .block_body_merkle_proof(EXECUTION_PAYLOAD_INDEX)?; Ok(LightClientHeaderDeneb { beacon: block.message().block_header(), @@ -204,7 +212,9 @@ impl LightClientHeaderDeneb { } impl LightClientHeaderElectra { - pub fn block_to_light_client_header(block: &SignedBeaconBlock) -> Result { + pub fn block_to_light_client_header( + block: &SignedBlindedBeaconBlock, + ) -> Result { let payload = block .message() .execution_payload()? @@ -219,8 +229,9 @@ impl LightClientHeaderElectra { .to_owned(), ); - let execution_branch = - beacon_block_body.block_body_merkle_proof(EXECUTION_PAYLOAD_INDEX)?; + let execution_branch = beacon_block_body + .to_ref() + .block_body_merkle_proof(EXECUTION_PAYLOAD_INDEX)?; Ok(LightClientHeaderElectra { beacon: block.message().block_header(), diff --git a/consensus/types/src/light_client_optimistic_update.rs b/consensus/types/src/light_client_optimistic_update.rs index 708f24e7701..f5b749be706 100644 --- a/consensus/types/src/light_client_optimistic_update.rs +++ b/consensus/types/src/light_client_optimistic_update.rs @@ -2,7 +2,7 @@ use super::{EthSpec, ForkName, ForkVersionDeserialize, LightClientHeader, Slot, use crate::test_utils::TestRandom; use crate::{ light_client_update::*, ChainSpec, LightClientHeaderAltair, LightClientHeaderCapella, - LightClientHeaderDeneb, LightClientHeaderElectra, SignedBeaconBlock, + LightClientHeaderDeneb, LightClientHeaderElectra, SignedBlindedBeaconBlock, }; use derivative::Derivative; use serde::{Deserialize, Deserializer, Serialize}; @@ -63,7 +63,7 @@ pub struct LightClientOptimisticUpdate { impl LightClientOptimisticUpdate { pub fn new( - attested_block: &SignedBeaconBlock, + attested_block: &SignedBlindedBeaconBlock, sync_aggregate: SyncAggregate, signature_slot: Slot, chain_spec: &ChainSpec, diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index 210fa0eeeb3..8a3eaff487f 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -3,7 +3,7 @@ use crate::light_client_header::LightClientHeaderElectra; use crate::{ beacon_state, test_utils::TestRandom, BeaconBlock, BeaconBlockHeader, BeaconState, ChainSpec, ForkName, ForkVersionDeserialize, LightClientHeaderAltair, LightClientHeaderCapella, - LightClientHeaderDeneb, SignedBeaconBlock, + LightClientHeaderDeneb, SignedBlindedBeaconBlock, }; use derivative::Derivative; use safe_arith::ArithError; @@ -156,8 +156,8 @@ impl LightClientUpdate { beacon_state: BeaconState, block: BeaconBlock, attested_state: &mut BeaconState, - attested_block: &SignedBeaconBlock, - finalized_block: &SignedBeaconBlock, + attested_block: &SignedBlindedBeaconBlock, + finalized_block: &SignedBlindedBeaconBlock, chain_spec: &ChainSpec, ) -> Result { let sync_aggregate = block.body().sync_aggregate()?; From 42e5f236cc709f6beb6e82c9381011a13485069a Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 29 Jul 2024 18:31:02 -0700 Subject: [PATCH 36/42] fix ef test --- beacon_node/beacon_chain/tests/store_tests.rs | 2 +- testing/ef_tests/src/handler.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 8ca0a852861..c4f9ad532c6 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -294,7 +294,7 @@ async fn light_client_updates_test() { harness .extend_chain( - num_final_blocks as usize, + (num_final_blocks + 1) as usize, BlockStrategy::OnCanonicalHead, AttestationStrategy::AllValidators, ) diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index fb58e25d3f8..52fc58f3d8c 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -858,7 +858,8 @@ impl Handler for LightClientUpdateHandler { fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { // Enabled in Altair - fork_name != ForkName::Base + // TODO(electra) re-enable once https://github.com/sigp/lighthouse/issues/6002 is resolved + fork_name != ForkName::Base && fork_name != ForkName::Electra } } From 25985d9f67387ff7def0b63fe3918e31f8fcca76 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 29 Jul 2024 21:52:51 -0700 Subject: [PATCH 37/42] fix lc update check --- .../src/light_client_server_cache.rs | 22 ++++++++++++------- consensus/types/src/light_client_update.rs | 3 ++- 2 files changed, 16 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 2b46b39ca6d..fc342255bce 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -112,7 +112,7 @@ impl LightClientServerCache { let attested_slot = attested_block.slot(); - let maybe_finalized_block = store.get_blinded_block(&cached_parts.finalized_block_root)?; + let maybe_finalized_block = store.get_blinded_block(&cached_parts.finalized_block_root)?; let new_light_client_update = LightClientUpdate::new( sync_aggregate, @@ -138,15 +138,23 @@ impl LightClientServerCache { let should_persist_light_client_update = if let Some(prev_light_client_update) = prev_light_client_update { - prev_light_client_update - .is_better_light_client_update(&new_light_client_update, chain_spec)? + let prev_sync_period = prev_light_client_update + .signature_slot() + .epoch(T::EthSpec::slots_per_epoch()) + .sync_committee_period(chain_spec)?; + + let should_persist = if sync_period != prev_sync_period { + true + } else { + prev_light_client_update + .is_better_light_client_update(&new_light_client_update, chain_spec)? + }; + + should_persist } else { true }; - println!("{}", sync_period); - println!("{}", should_persist_light_client_update); - if should_persist_light_client_update { self.store_light_client_update(&store, sync_period, &new_light_client_update)?; } @@ -207,8 +215,6 @@ impl LightClientServerCache { sync_committee_period: u64, light_client_update: &LightClientUpdate, ) -> Result<(), BeaconChainError> { - - println!("STORE LC UPDATE FOR SYNC COMMITTEE PERIOD {}", sync_committee_period); let column = DBColumn::LightClientUpdate; store.hot_db.put_bytes( diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index f7158435cde..faa9ddc782a 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -2,7 +2,8 @@ use super::{EthSpec, FixedVector, Hash256, Slot, SyncAggregate, SyncCommittee}; use crate::light_client_header::LightClientHeaderElectra; use crate::{ beacon_state, test_utils::TestRandom, ChainSpec, Epoch, ForkName, ForkVersionDeserialize, - LightClientHeaderAltair, LightClientHeaderCapella, LightClientHeaderDeneb, SignedBlindedBeaconBlock, + LightClientHeaderAltair, LightClientHeaderCapella, LightClientHeaderDeneb, + SignedBlindedBeaconBlock, }; use derivative::Derivative; use safe_arith::ArithError; From d7d097cddf3dcbe80bd31d2bb2ae8a01a9d1d7b4 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 30 Jul 2024 13:02:55 +0200 Subject: [PATCH 38/42] Lint --- beacon_node/beacon_chain/src/light_client_server_cache.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 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 fc342255bce..4635fbc4fed 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -143,14 +143,12 @@ impl LightClientServerCache { .epoch(T::EthSpec::slots_per_epoch()) .sync_committee_period(chain_spec)?; - let should_persist = if sync_period != prev_sync_period { + if sync_period != prev_sync_period { true } else { prev_light_client_update .is_better_light_client_update(&new_light_client_update, chain_spec)? - }; - - should_persist + } } else { true }; From 42dec6216dea7a3714bf6a9e1421149fd293d2b7 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 30 Jul 2024 13:52:43 -0700 Subject: [PATCH 39/42] revert basic sim --- .../src/light_client_server_cache.rs | 4 ++- testing/simulator/src/checks.rs | 27 ------------------- testing/simulator/src/cli.rs | 2 ++ 3 files changed, 5 insertions(+), 28 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 4635fbc4fed..a8cfb9d041b 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -2,7 +2,7 @@ use crate::errors::BeaconChainError; use crate::{metrics, BeaconChainTypes, BeaconStore}; use parking_lot::{Mutex, RwLock}; use safe_arith::SafeArith; -use slog::{debug, Logger}; +use slog::{debug, info, Logger}; use ssz::Decode; use ssz::Encode; use ssz_types::FixedVector; @@ -165,6 +165,8 @@ impl LightClientServerCache { } None => true, }; + info!(log, "is_latest_optimistic: {}", is_latest_optimistic); + info!(log, "signature slot: {}", signature_slot); if is_latest_optimistic { // can create an optimistic update, that is more recent *self.latest_optimistic_update.write() = Some(LightClientOptimisticUpdate::new( diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index 5ea03286001..03cc17fab3e 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -263,10 +263,6 @@ pub(crate) async fn verify_light_client_updates( slot_delay(Slot::new(1), slot_duration).await; let slot = Slot::new(slot); let previous_slot = slot - 1; - let sync_committee_period = slot - .epoch(E::slots_per_epoch()) - .sync_committee_period(&E::default_spec()) - .unwrap(); let previous_slot_block = client .get_beacon_blocks::(BlockId::Slot(previous_slot)) @@ -333,29 +329,6 @@ pub(crate) async fn verify_light_client_updates( "Existing finality update too old: signature slot {signature_slot}, current slot {slot:?}" )); } - - // Verify light client update. `signature_slot_distance` should be 1 in the ideal scenario. - let light_client_updates = client - .get_beacon_light_client_updates::(sync_committee_period, 1) - .await - .map_err(|e| format!("Error while getting light client update: {:?}", e))? - .ok_or(format!("Light client update not found {slot:?}"))?; - - if light_client_updates.len() != 1 { - return Err(format!( - "{} light client update(s) was returned when only one was expected.", - light_client_updates.len() - )); - } - - if let Some(light_client_update) = light_client_updates.first() { - let signature_slot_distance = slot - *light_client_update.data.signature_slot(); - if signature_slot_distance > light_client_update_slot_tolerance { - return Err(format!( - "Existing light client update too old: signature slot {signature_slot}, current slot {slot:?}" - )); - } - }; } Ok(()) diff --git a/testing/simulator/src/cli.rs b/testing/simulator/src/cli.rs index 191ce449497..3d61dcde74a 100644 --- a/testing/simulator/src/cli.rs +++ b/testing/simulator/src/cli.rs @@ -57,6 +57,7 @@ pub fn cli_app() -> Command { ) .arg( Arg::new("continue-after-checks") + .short('c') .long("continue_after_checks") .action(ArgAction::SetTrue) .help("Continue after checks (default false)"), @@ -115,6 +116,7 @@ pub fn cli_app() -> Command { ) .arg( Arg::new("continue-after-checks") + .short('c') .long("continue_after_checks") .action(ArgAction::SetTrue) .help("Continue after checks (default false)"), From 2f0837aeaf590f15e38cdf121bfc5458c9979ded Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 31 Jul 2024 14:14:57 -0700 Subject: [PATCH 40/42] small fix --- .../src/light_client_server_cache.rs | 84 +++++++++---------- testing/simulator/src/checks.rs | 28 +++++++ 2 files changed, 70 insertions(+), 42 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 a8cfb9d041b..58e7729c860 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -114,49 +114,10 @@ impl LightClientServerCache { let maybe_finalized_block = store.get_blinded_block(&cached_parts.finalized_block_root)?; - let new_light_client_update = LightClientUpdate::new( - sync_aggregate, - block_slot, - cached_parts.next_sync_committee, - cached_parts.next_sync_committee_branch, - cached_parts.finality_branch.clone(), - &attested_block, - maybe_finalized_block.as_ref(), - chain_spec, - )?; - let sync_period = block_slot .epoch(T::EthSpec::slots_per_epoch()) .sync_committee_period(chain_spec)?; - // Spec: Full nodes SHOULD provide the best derivable LightClientUpdate (according to is_better_update) - // for each sync committee period - let prev_light_client_update = match &self.latest_light_client_update.read().clone() { - Some(prev_light_client_update) => Some(prev_light_client_update.clone()), - None => self.get_light_client_update(&store, sync_period, chain_spec)?, - }; - - let should_persist_light_client_update = - if let Some(prev_light_client_update) = prev_light_client_update { - let prev_sync_period = prev_light_client_update - .signature_slot() - .epoch(T::EthSpec::slots_per_epoch()) - .sync_committee_period(chain_spec)?; - - if sync_period != prev_sync_period { - true - } else { - prev_light_client_update - .is_better_light_client_update(&new_light_client_update, chain_spec)? - } - } else { - true - }; - - if should_persist_light_client_update { - self.store_light_client_update(&store, sync_period, &new_light_client_update)?; - } - // Spec: Full nodes SHOULD provide the LightClientOptimisticUpdate with the highest // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice let is_latest_optimistic = match &self.latest_optimistic_update.read().clone() { @@ -188,11 +149,11 @@ impl LightClientServerCache { if is_latest_finality & !cached_parts.finalized_block_root.is_zero() { // Immediately after checkpoint sync the finalized block may not be available yet. - if let Some(finalized_block) = maybe_finalized_block { + if let Some(finalized_block) = maybe_finalized_block.as_ref() { *self.latest_finality_update.write() = Some(LightClientFinalityUpdate::new( &attested_block, - &finalized_block, - cached_parts.finality_branch, + finalized_block, + cached_parts.finality_branch.clone(), sync_aggregate.clone(), signature_slot, chain_spec, @@ -206,6 +167,45 @@ impl LightClientServerCache { } } + let new_light_client_update = LightClientUpdate::new( + sync_aggregate, + block_slot, + cached_parts.next_sync_committee, + cached_parts.next_sync_committee_branch, + cached_parts.finality_branch, + &attested_block, + maybe_finalized_block.as_ref(), + chain_spec, + )?; + + // Spec: Full nodes SHOULD provide the best derivable LightClientUpdate (according to is_better_update) + // for each sync committee period + let prev_light_client_update = match &self.latest_light_client_update.read().clone() { + Some(prev_light_client_update) => Some(prev_light_client_update.clone()), + None => self.get_light_client_update(&store, sync_period, chain_spec)?, + }; + + let should_persist_light_client_update = + if let Some(prev_light_client_update) = prev_light_client_update { + let prev_sync_period = prev_light_client_update + .signature_slot() + .epoch(T::EthSpec::slots_per_epoch()) + .sync_committee_period(chain_spec)?; + + if sync_period != prev_sync_period { + true + } else { + prev_light_client_update + .is_better_light_client_update(&new_light_client_update, chain_spec)? + } + } else { + true + }; + + if should_persist_light_client_update { + self.store_light_client_update(&store, sync_period, &new_light_client_update)?; + } + Ok(()) } diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index 03cc17fab3e..c282639b16f 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -264,6 +264,11 @@ pub(crate) async fn verify_light_client_updates( let slot = Slot::new(slot); let previous_slot = slot - 1; + let sync_committee_period = slot + .epoch(E::slots_per_epoch()) + .sync_committee_period(&E::default_spec()) + .unwrap(); + let previous_slot_block = client .get_beacon_blocks::(BlockId::Slot(previous_slot)) .await @@ -329,6 +334,29 @@ pub(crate) async fn verify_light_client_updates( "Existing finality update too old: signature slot {signature_slot}, current slot {slot:?}" )); } + + // Verify light client update. `signature_slot_distance` should be 1 in the ideal scenario. + let light_client_updates = client + .get_beacon_light_client_updates::(sync_committee_period, 1) + .await + .map_err(|e| format!("Error while getting light client update: {:?}", e))? + .ok_or(format!("Light client update not found {slot:?}"))?; + + if light_client_updates.len() != 1 { + return Err(format!( + "{} light client update(s) was returned when only one was expected.", + light_client_updates.len() + )); + } + + if let Some(light_client_update) = light_client_updates.first() { + let signature_slot_distance = slot - *light_client_update.data.signature_slot(); + if signature_slot_distance > light_client_update_slot_tolerance { + return Err(format!( + "Existing light client update too old: signature slot {signature_slot}, current slot {slot:?}" + )); + } + }; } Ok(()) From e6555f363bc1518046015338ba70bf88f2f9423a Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 31 Jul 2024 16:29:07 -0700 Subject: [PATCH 41/42] revert sim --- testing/simulator/src/checks.rs | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/testing/simulator/src/checks.rs b/testing/simulator/src/checks.rs index c282639b16f..03cc17fab3e 100644 --- a/testing/simulator/src/checks.rs +++ b/testing/simulator/src/checks.rs @@ -264,11 +264,6 @@ pub(crate) async fn verify_light_client_updates( let slot = Slot::new(slot); let previous_slot = slot - 1; - let sync_committee_period = slot - .epoch(E::slots_per_epoch()) - .sync_committee_period(&E::default_spec()) - .unwrap(); - let previous_slot_block = client .get_beacon_blocks::(BlockId::Slot(previous_slot)) .await @@ -334,29 +329,6 @@ pub(crate) async fn verify_light_client_updates( "Existing finality update too old: signature slot {signature_slot}, current slot {slot:?}" )); } - - // Verify light client update. `signature_slot_distance` should be 1 in the ideal scenario. - let light_client_updates = client - .get_beacon_light_client_updates::(sync_committee_period, 1) - .await - .map_err(|e| format!("Error while getting light client update: {:?}", e))? - .ok_or(format!("Light client update not found {slot:?}"))?; - - if light_client_updates.len() != 1 { - return Err(format!( - "{} light client update(s) was returned when only one was expected.", - light_client_updates.len() - )); - } - - if let Some(light_client_update) = light_client_updates.first() { - let signature_slot_distance = slot - *light_client_update.data.signature_slot(); - if signature_slot_distance > light_client_update_slot_tolerance { - return Err(format!( - "Existing light client update too old: signature slot {signature_slot}, current slot {slot:?}" - )); - } - }; } Ok(()) From 1835b2945d16c90a055061152abd03047c4a390e Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 1 Aug 2024 17:09:41 +0200 Subject: [PATCH 42/42] Review PR --- .../beacon_chain/src/light_client_server_cache.rs | 14 ++++++-------- .../types/src/light_client_finality_update.rs | 2 +- .../types/src/light_client_optimistic_update.rs | 2 +- consensus/types/src/light_client_update.rs | 3 --- 4 files changed, 8 insertions(+), 13 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 58e7729c860..efc746675dc 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -2,7 +2,7 @@ use crate::errors::BeaconChainError; use crate::{metrics, BeaconChainTypes, BeaconStore}; use parking_lot::{Mutex, RwLock}; use safe_arith::SafeArith; -use slog::{debug, info, Logger}; +use slog::{debug, Logger}; use ssz::Decode; use ssz::Encode; use ssz_types::FixedVector; @@ -57,7 +57,7 @@ impl LightClientServerCache { /// Compute and cache state proofs for latter production of light-client messages. Does not /// trigger block replay. - pub fn cache_state_data( + pub(crate) fn cache_state_data( &self, spec: &ChainSpec, block: BeaconBlockRef, @@ -122,12 +122,10 @@ impl LightClientServerCache { // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice let is_latest_optimistic = match &self.latest_optimistic_update.read().clone() { Some(latest_optimistic_update) => { - latest_optimistic_update.is_latest_optimistic_update(attested_slot, signature_slot) + latest_optimistic_update.is_latest(attested_slot, signature_slot) } None => true, }; - info!(log, "is_latest_optimistic: {}", is_latest_optimistic); - info!(log, "signature slot: {}", signature_slot); if is_latest_optimistic { // can create an optimistic update, that is more recent *self.latest_optimistic_update.write() = Some(LightClientOptimisticUpdate::new( @@ -142,7 +140,7 @@ impl LightClientServerCache { // attested_header.beacon.slot (if multiple, highest signature_slot) as selected by fork choice let is_latest_finality = match &self.latest_finality_update.read().clone() { Some(latest_finality_update) => { - latest_finality_update.is_latest_finality_update(attested_slot, signature_slot) + latest_finality_update.is_latest(attested_slot, signature_slot) } None => true, }; @@ -209,7 +207,7 @@ impl LightClientServerCache { Ok(()) } - pub fn store_light_client_update( + fn store_light_client_update( &self, store: &BeaconStore, sync_committee_period: u64, @@ -231,7 +229,7 @@ impl LightClientServerCache { // Used to fetch the most recently persisted "best" light client update. // Should not be used outside the light client server, as it also caches the fetched // light client update. - pub fn get_light_client_update( + fn get_light_client_update( &self, store: &BeaconStore, sync_committee_period: u64, diff --git a/consensus/types/src/light_client_finality_update.rs b/consensus/types/src/light_client_finality_update.rs index c3d72d80291..dc7561f5fcc 100644 --- a/consensus/types/src/light_client_finality_update.rs +++ b/consensus/types/src/light_client_finality_update.rs @@ -197,7 +197,7 @@ impl LightClientFinalityUpdate { // > Full nodes SHOULD provide the LightClientFinalityUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) // // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_finality_update - pub fn is_latest_finality_update(&self, attested_slot: Slot, signature_slot: Slot) -> bool { + pub fn is_latest(&self, attested_slot: Slot, signature_slot: Slot) -> bool { let prev_slot = self.get_attested_header_slot(); if attested_slot > prev_slot { true diff --git a/consensus/types/src/light_client_optimistic_update.rs b/consensus/types/src/light_client_optimistic_update.rs index 6c56882dbb7..3cae31edf80 100644 --- a/consensus/types/src/light_client_optimistic_update.rs +++ b/consensus/types/src/light_client_optimistic_update.rs @@ -183,7 +183,7 @@ impl LightClientOptimisticUpdate { // > Full nodes SHOULD provide the LightClientOptimisticUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) // // ref: https://github.com/ethereum/consensus-specs/blob/113c58f9bf9c08867f6f5f633c4d98e0364d612a/specs/altair/light-client/full-node.md#create_light_client_optimistic_update - pub fn is_latest_optimistic_update(&self, attested_slot: Slot, signature_slot: Slot) -> bool { + pub fn is_latest(&self, attested_slot: Slot, signature_slot: Slot) -> bool { let prev_slot = self.get_slot(); if attested_slot > prev_slot { true diff --git a/consensus/types/src/light_client_update.rs b/consensus/types/src/light_client_update.rs index faa9ddc782a..3b48a68df31 100644 --- a/consensus/types/src/light_client_update.rs +++ b/consensus/types/src/light_client_update.rs @@ -350,7 +350,6 @@ impl LightClientUpdate { // Compare presence of relevant sync committee let new_has_relevant_sync_committee = new.is_sync_committee_update(chain_spec)?; let prev_has_relevant_sync_committee = self.is_sync_committee_update(chain_spec)?; - if new_has_relevant_sync_committee != prev_has_relevant_sync_committee { return Ok(new_has_relevant_sync_committee); } @@ -365,9 +364,7 @@ impl LightClientUpdate { // Compare sync committee finality if new_has_finality { let new_has_sync_committee_finality = new.has_sync_committee_finality(chain_spec)?; - let prev_has_sync_committee_finality = self.has_sync_committee_finality(chain_spec)?; - if new_has_sync_committee_finality != prev_has_sync_committee_finality { return Ok(new_has_sync_committee_finality); }