diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 3a55e96be5..b769a40d5a 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -671,7 +671,8 @@ pub fn signature_verify_chain_segment( } } } - // TODO(gloas) make this work across both v1 and v2 + // TODO(gloas) When implementing range and backfill sync for gloas + // we need a batch verify kzg function in the new da checker as well. chain .data_availability_checker .v1() @@ -1311,7 +1312,8 @@ impl IntoExecutionPendingBlock for RpcBlock let maybe_available_block = match &self { RpcBlock::FullyAvailable(available_block) => { - // TODO(gloas) make this work across both v1 and v2 + // TODO(gloas) when implementing sync for gloas we need a verify kzg function + // added to the new da checker as well. chain .data_availability_checker .v1() diff --git a/beacon_node/beacon_chain/src/custody_context.rs b/beacon_node/beacon_chain/src/custody_context.rs index cebb256a02..c512ce616a 100644 --- a/beacon_node/beacon_chain/src/custody_context.rs +++ b/beacon_node/beacon_chain/src/custody_context.rs @@ -7,7 +7,7 @@ use std::{ sync::atomic::{AtomicU64, Ordering}, }; use tracing::{debug, warn}; -use types::{ChainSpec, ColumnIndex, Epoch, EthSpec, SignedExecutionPayloadEnvelope, Slot}; +use types::{ChainSpec, ColumnIndex, Epoch, EthSpec, Slot}; /// A delay before making the CGC change effective to the data availability checker. pub const CUSTODY_CHANGE_DA_EFFECTIVE_DELAY_SECONDS: u64 = 30; @@ -527,13 +527,6 @@ impl CustodyContext { .write() .reset_validator_custody_requirements(effective_epoch); } - - pub fn data_columns_required_for_payload( - &self, - _payload: &SignedExecutionPayloadEnvelope, - ) -> bool { - todo!() - } } /// Indicates that the custody group count (CGC) has increased. diff --git a/beacon_node/beacon_chain/src/data_availability_checker_v2.rs b/beacon_node/beacon_chain/src/data_availability_checker_v2.rs index 63629dbfb7..fcff5605a4 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker_v2.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker_v2.rs @@ -15,7 +15,7 @@ use task_executor::TaskExecutor; use tracing::{debug, error, instrument}; use types::{ ChainSpec, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, EthSpec, Hash256, - SignedBeaconBlock, Slot, + SignedExecutionPayloadBid, Slot, }; mod overflow_lru_cache; @@ -30,19 +30,20 @@ use crate::metrics::{ use crate::observed_data_sidecars::ObservationStrategy; use types::new_non_zero_usize; -/// The LRU Cache stores `PendingComponents`, which store block and its associated column data. +/// The LRU Cache stores `PendingComponents`, which store the block root, the execution payload bid, and its associated column data. +/// The execution payload bid stores the kzg commitments which we use to verify against incoming column data. /// Setting this to 32 keeps memory usage reasonable. /// /// `PendingComponents` are now never removed from the cache manually and are only removed via LRU /// eviction to prevent race conditions (#7961), so we expect this cache to be full all the time. const OVERFLOW_LRU_CAPACITY_NON_ZERO: NonZeroUsize = new_non_zero_usize(32); -/// Represents available data for a block - the block root and its data columns. +/// Represents available data for a payload - its block root and its data columns. pub type AvailableData = (Hash256, DataColumnSidecarList); -/// This type is returned after adding a block / column to the `DataAvailabilityChecker`. +/// This type is returned after adding a bid / column to the `DataAvailabilityChecker`. /// -/// Indicates if the block's data is fully `Available` or if we need more columns. +/// Indicates if the payloads data is fully `Available` or if we need more columns. pub enum Availability { MissingComponents(Hash256), Available(Box>), @@ -68,10 +69,10 @@ pub enum DataColumnReconstructionResult { RecoveredColumnsNotImported(&'static str), } -/// Cache to hold data columns for blocks pending data availability. +/// Cache to hold data columns for payloads pending data availability. /// /// In Gloas, beacon blocks can be immediately imported into fork choice. The execution payload -/// is separated from the beacon block. This cache tracks data columns for payloads until all +/// bid contains the payloads kzg commitments. This cache tracks data columns for payloads until all /// required columns are received. /// /// Usually data becomes available on its slot within a second of receiving its first component @@ -126,7 +127,7 @@ impl AvailabilityCache for DataAvailabilityChecker { }) } - /// Insert RPC custody columns and check if the block becomes available. + /// Insert RPC custody columns and check if the payload becomes available. #[instrument(skip_all, level = "trace")] fn put_rpc_custody_columns( &self, @@ -154,9 +155,8 @@ impl AvailabilityCache for DataAvailabilityChecker { .put_kzg_verified_data_columns(block_root, verified_custody_columns) } - /// Check if we've cached other data columns for this block. If it satisfies the custody - /// requirement and we also have the block cached, return the `Availability` variant - /// triggering import. Otherwise cache the data column sidecar. + /// Check if we've cached other data columns for this block root. If it satisfies the custody + /// requirement, return the `Availability::Available` variant. Otherwise cache the data column sidecar. #[instrument(skip_all, level = "trace")] fn put_gossip_verified_data_columns( &self, @@ -314,13 +314,13 @@ impl DataAvailabilityChecker { &self.custody_context } - /// Insert a block into the cache and check if data becomes available. - pub fn put_block( + /// Insert an execution payload bid into the cache and check if data becomes available. + pub fn put_bid( &self, block_root: Hash256, - block: Arc>, + bid: Arc>, ) -> Result, AvailabilityCheckError> { - self.availability_cache.put_block(block_root, block) + self.availability_cache.put_bid(block_root, bid) } /// Collects metrics from the data availability checker. diff --git a/beacon_node/beacon_chain/src/data_availability_checker_v2/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker_v2/overflow_lru_cache.rs index fa7ac913c1..51a19554dd 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker_v2/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker_v2/overflow_lru_cache.rs @@ -11,7 +11,7 @@ use std::sync::Arc; use tracing::{Span, debug, debug_span}; use types::{ ChainSpec, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, Epoch, EthSpec, Hash256, - SignedBeaconBlock, + SignedExecutionPayloadBid, }; /// This represents the components of a payload pending data availability. @@ -22,7 +22,8 @@ pub struct PendingComponents { /// The block root is stored for tracing context in the span. #[allow(dead_code)] pub block_root: Hash256, - pub block: Option>>, + /// The execution payload bid containing blob_kzg_commitments. + pub bid: Option>>, pub verified_data_columns: Vec>, pub reconstruction_started: bool, span: Span, @@ -62,50 +63,40 @@ impl PendingComponents { Ok(()) } - /// Inserts a block into the cache. - pub fn insert_block(&mut self, block: Arc>) { - self.block = Some(block); + /// Inserts an execution payload bid into the cache. + pub fn insert_bid(&mut self, bid: Arc>) { + self.bid = Some(bid); } - /// Returns the number of blobs expected for this block by reading the bid's kzg commitments. - /// Returns an error if the block is not cached or not a Gloas block. + /// Returns the number of blobs expected by reading the bid's kzg commitments. + /// Returns an error if the bid is not cached. This function should only be called + /// after ensuring that the bid has been cached. pub fn num_blobs_expected(&self) -> Result { - let block = self - .block + let bid = self + .bid .as_ref() - .ok_or_else(|| AvailabilityCheckError::Unexpected("No block available".to_string()))?; - - let bid = block - .message() - .body() - .signed_execution_payload_bid() - .map_err(|_| { - AvailabilityCheckError::Unexpected( - "Block does not have execution payload bid (not a Gloas block?)".to_string(), - ) - })?; + .ok_or_else(|| AvailabilityCheckError::Unexpected("No bid available".to_string()))?; Ok(bid.message.blob_kzg_commitments.len()) } - /// Returns Some if all required data columns have been received. + /// Returns `Some` if the bid and all required data columns have been received. pub fn make_available( &self, num_expected_columns: usize, ) -> Result>, AvailabilityCheckError> { - // Check if we have a block - if not, still waiting - if self.block.is_none() { + // Check if we have a bid - if not, still waiting + if self.bid.is_none() { return Ok(None); } - // Get the number of blobs expected from the block's bid - // This will error if the block doesn't have a bid (not Gloas) + // Get the number of blobs expected from the bid let num_expected_blobs = self.num_blobs_expected()?; if num_expected_blobs == 0 { // No blobs expected, data is available (empty) self.span.in_scope(|| { - debug!("Block has no blobs, data is available"); + debug!("Bid has no blobs, data is available"); }); return Ok(Some(vec![])); } @@ -145,18 +136,18 @@ impl PendingComponents { let _guard = span.clone().entered(); Self { block_root, - block: None, + bid: None, verified_data_columns: vec![], reconstruction_started: false, span, } } - /// Returns the epoch of the block or first data column, if available. + /// Returns the epoch of the bid or first data column, if available. pub fn epoch(&self) -> Option { - // Get epoch from block - if let Some(block) = &self.block { - return Some(block.slot().epoch(E::slots_per_epoch())); + // Get epoch from bid + if let Some(bid) = &self.bid { + return Some(bid.message.slot.epoch(E::slots_per_epoch())); } // Or, get epoch from first data column @@ -232,17 +223,17 @@ impl DataAvailabilityCheckerInner { f(self.critical.read().peek(block_root)) } - /// Insert a block into the cache and check if data becomes available. - pub fn put_block( + /// Insert an execution payload bid into the cache and check if data becomes available. + pub fn put_bid( &self, block_root: Hash256, - block: Arc>, + bid: Arc>, ) -> Result, AvailabilityCheckError> { - let epoch = block.slot().epoch(T::EthSpec::slots_per_epoch()); + let epoch = bid.message.slot.epoch(T::EthSpec::slots_per_epoch()); let pending_components = self.update_or_insert_pending_components(block_root, |pending_components| { - pending_components.insert_block(block); + pending_components.insert_bid(bid); Ok(()) })?; @@ -250,7 +241,7 @@ impl DataAvailabilityCheckerInner { pending_components.span.in_scope(|| { debug!( - component = "block", + component = "bid", status = pending_components.status_str(num_expected_columns), "Component added to data availability checker" ); @@ -312,8 +303,8 @@ impl DataAvailabilityCheckerInner { } // We never remove the pending components manually to avoid race conditions. - // This ensures components remain available during and right after block import, - // preventing a race condition where a component was removed after the block was + // This ensures components remain available during and right after payload import, + // preventing a race condition where a component was removed after the payload was // imported, but re-inserted immediately, causing partial pending components to be // stored and served to peers. // Components are only removed via LRU eviction as finality advances. @@ -453,7 +444,7 @@ mod pending_components_tests { let components = PendingComponents::::empty(block_root); assert_eq!(components.block_root, block_root); - assert!(components.block.is_none()); + assert!(components.bid.is_none()); assert!(components.verified_data_columns.is_empty()); assert!(!components.reconstruction_started); assert!(components.epoch().is_none()); @@ -469,7 +460,7 @@ mod pending_components_tests { } #[test] - fn test_status_str_no_block() { + fn test_status_str_no_bid() { let block_root = Hash256::random(); let components = PendingComponents::::empty(block_root); @@ -478,7 +469,7 @@ mod pending_components_tests { } #[test] - fn test_num_blobs_expected_no_block() { + fn test_num_blobs_expected_no_bid() { let block_root = Hash256::random(); let components = PendingComponents::::empty(block_root); @@ -492,11 +483,11 @@ mod pending_components_tests { } #[test] - fn test_make_available_no_block_returns_none() { + fn test_make_available_no_bid_returns_none() { let block_root = Hash256::random(); let components = PendingComponents::::empty(block_root); - // Without a block, make_available should return Ok(None) + // Without a bid, make_available should return Ok(None) let result = components.make_available(10); assert!(result.is_ok()); assert!(result.unwrap().is_none()); @@ -760,7 +751,7 @@ mod data_availability_checker_tests { .put_kzg_verified_data_columns(block_root, verified_columns) .expect("should put columns"); - // Without a block, should still be missing components + // Without a bid, should still be missing components assert!(matches!(result, Availability::MissingComponents(_))); } diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index c4476bb508..dff8fbabff 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -734,7 +734,8 @@ impl NetworkBeaconProcessor { } } - // TODO(gloas) make this work across both v1 and v2 + // TODO(gloas) when implementing backfill sync for gloas + // we need a batch verify kzg function in the new da checker match self .chain .data_availability_checker