diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index 8ea3c792f4d..92a3cad43e0 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -20,6 +20,7 @@ use std::iter; use std::marker::PhantomData; use std::sync::Arc; use std::time::Duration; +use store::DatabaseBlock; use tracing::{debug, instrument}; use tree_hash::TreeHash; use types::data::{ @@ -28,7 +29,7 @@ use types::data::{ }; use types::{ BeaconStateError, ChainSpec, DataColumnSidecar, DataColumnSidecarFulu, DataColumnSubnetId, - EthSpec, Hash256, PartialDataColumnSidecarRef, SignedBeaconBlockHeader, Slot, + EthSpec, Hash256, KzgCommitments, PartialDataColumnSidecarRef, SignedBeaconBlockHeader, Slot, }; /// An error occurred while validating a gossip data column. @@ -131,6 +132,25 @@ pub enum GossipDataColumnError { parent_root: Hash256, slot: Slot, }, + /// The block for this Gloas data column's `beacon_block_root` is not yet known. + /// The sidecar should be queued and retried when the block arrives. + /// + /// ## Peer scoring + /// + /// We cannot process the columns without validating its block, the peer isn't necessarily faulty. + BlockUnknown { + beacon_block_root: Hash256, + slot: Slot, + }, + /// The sidecar's slot does not match the block's slot. + /// + /// ## Peer scoring + /// + /// The data column sidecar is invalid and the peer is faulty. + SlotMismatch { + sidecar_slot: Slot, + block_slot: Slot, + }, /// The column conflicts with finalization, no need to propagate. /// /// ## Peer scoring @@ -319,8 +339,11 @@ impl GossipVerifiedDataColumn ) }) } - // TODO(gloas) support gloas data column variant - DataColumnSidecar::Gloas(_) => Err(GossipDataColumnError::InvalidVariant), + DataColumnSidecar::Gloas(_) => validate_data_column_sidecar_for_gossip_gloas::( + column_sidecar, + subnet_id, + chain, + ), } } @@ -329,9 +352,10 @@ impl GossipVerifiedDataColumn /// When publishing a block constructed externally, there will be no columns here. pub fn new_for_block_publishing( column_sidecar: Arc>, + commitments_len: usize, chain: &BeaconChain, ) -> Result { - verify_data_column_sidecar(&column_sidecar, &chain.spec)?; + verify_data_column_sidecar(&column_sidecar, commitments_len, &chain.spec)?; // Check if the data column is already in the DA checker cache. This happens when data columns // are made available through the `engine_getBlobs` method. If it exists in the cache, we know @@ -860,6 +884,28 @@ pub fn verify_kzg_for_data_column( }) } +/// Complete kzg verification for a `DataColumnSidecar` using externally provided commitments. +/// Used for Gloas where commitments come from the block's execution payload bid. +#[instrument(skip_all, level = "debug")] +pub fn verify_kzg_for_data_column_with_commitments( + data_column: Arc>, + cells_to_verify: PartialDataColumnSidecarRef, + kzg_commitments: &KzgCommitments, + kzg: &Kzg, + seen_timestamp: Duration, +) -> Result, (Option, KzgError)> { + let _timer = metrics::start_timer(&metrics::KZG_VERIFICATION_DATA_COLUMN_SINGLE_TIMES); + validate_partial_data_columns( + kzg, + iter::once((*data_column.index(), cells_to_verify)), + kzg_commitments, + )?; + Ok(KzgVerifiedDataColumn { + data: data_column, + seen_timestamp, + }) +} + /// Complete kzg verification for a `VerifiablePartialDataColumn`. /// /// Returns an error if the kzg verification check fails. @@ -916,7 +962,11 @@ pub fn validate_data_column_sidecar_for_gossip_fulu( + data_column: Arc>, + subnet: DataColumnSubnetId, + chain: &BeaconChain, +) -> Result, GossipDataColumnError> { + let DataColumnSidecar::Gloas(_) = data_column.as_ref() else { + return Err(GossipDataColumnError::InvalidVariant); + }; + + let column_slot = data_column.slot(); + let block_root = data_column.block_root(); + + verify_index_matches_subnet(&data_column, subnet, &chain.spec)?; + let bid_commitments = get_gloas_block_commitments(chain, &block_root, column_slot)?; + verify_data_column_sidecar(&data_column, bid_commitments.len(), &chain.spec)?; + verify_is_unknown_sidecar(chain, &data_column)?; + + // Check DA cache for already-processed columns. + let Some(cells_to_kzg_verify) = chain + .data_availability_checker + .missing_cells_for_column_sidecar(&data_column) + .map_err(|err| match err { + MissingCellsError::MismatchesCachedColumn => { + GossipDataColumnError::MismatchesCachedColumn + } + MissingCellsError::UnexpectedError(_) => todo!("handle unexpected error"), + })? + else { + if O::observe() { + observe_gossip_data_column(&data_column, chain)?; + } + return Err(GossipDataColumnError::PriorKnownUnpublished); + }; + + // [REJECT] KZG proof verification using commitments from the bid. + let kzg = &chain.kzg; + let seen_timestamp = chain.slot_clock.now_duration().unwrap_or_default(); + let kzg_verified_data_column = verify_kzg_for_data_column_with_commitments( + data_column.clone(), + cells_to_kzg_verify, + &bid_commitments, + kzg, + seen_timestamp, + ) + .map_err(|(_, e)| GossipDataColumnError::InvalidKzgProof(e))?; + + if O::observe() { + observe_gossip_data_column(&data_column, chain)?; + } + + Ok(GossipVerifiedDataColumn { + block_root, + data_column: kzg_verified_data_column, + _phantom: PhantomData, + }) +} + +/// Look up the block for a Gloas data column sidecar and extract the bid's `blob_kzg_commitments`. +/// Returns `BlockUnknown` if the block is not found, `SlotMismatch` if slots don't match. +fn get_gloas_block_commitments( + chain: &BeaconChain, + block_root: &Hash256, + sidecar_slot: Slot, +) -> Result, GossipDataColumnError> { + let block = chain + .store + .try_get_full_block(block_root) + .map_err(|e| GossipDataColumnError::BeaconChainError(Box::new(e.into())))?; + + // TODO(gloas): maybe also check DA cache? fork choice? + let block = match block { + Some(DatabaseBlock::Full(block)) => block, + Some(DatabaseBlock::Blinded(_)) | None => { + return Err(GossipDataColumnError::BlockUnknown { + beacon_block_root: *block_root, + slot: sidecar_slot, + }); + } + }; + + if sidecar_slot != block.slot() { + return Err(GossipDataColumnError::SlotMismatch { + sidecar_slot, + block_slot: block.slot(), + }); + } + + let bid = block + .message() + .body() + .signed_execution_payload_bid() + .map_err(|e| GossipDataColumnError::BeaconChainError(Box::new(e.into())))?; + Ok(bid.message.blob_kzg_commitments.clone()) +} + #[instrument(skip_all, level = "debug")] pub fn validate_partial_data_column_sidecar_for_gossip( mut column: Box>, @@ -1116,8 +1269,13 @@ pub enum PartialColumnVerificationResult { } /// Verify if the data column sidecar is valid. +/// +/// `commitments_len` is the number of KZG commitments associated with this block. +/// For Fulu, this comes from the sidecar's `kzg_commitments` field. +/// For Gloas, this comes from `block.body.signed_execution_payload_bid.message.blob_kzg_commitments`. fn verify_data_column_sidecar( data_column: &DataColumnSidecar, + commitments_len: usize, spec: &ChainSpec, ) -> Result<(), GossipDataColumnError> { if *data_column.index() >= E::number_of_columns() as u64 { @@ -1126,12 +1284,6 @@ fn verify_data_column_sidecar( )); } - // TODO(gloas): implement Gloas verification that takes kzg_commitments from block as parameter - let commitments_len = match data_column { - DataColumnSidecar::Fulu(dc) => dc.kzg_commitments.len(), - DataColumnSidecar::Gloas(_) => return Err(GossipDataColumnError::InvalidVariant), - }; - if commitments_len == 0 { return Err(GossipDataColumnError::UnexpectedDataColumn); } @@ -1444,7 +1596,7 @@ mod test { .build(); harness.advance_slot(); - let verify_fn = |column_sidecar: DataColumnSidecar| { + let verify_fn = |column_sidecar: DataColumnSidecar, _| { let col_index = *column_sidecar.index(); validate_data_column_sidecar_for_gossip_fulu::<_, Observe>( column_sidecar.into(), @@ -1469,9 +1621,10 @@ mod test { .build(); harness.advance_slot(); - let verify_fn = |column_sidecar: DataColumnSidecar| { + let verify_fn = |column_sidecar: DataColumnSidecar, commitments_len: usize| { GossipVerifiedDataColumn::<_>::new_for_block_publishing( column_sidecar.into(), + commitments_len, &harness.chain, ) }; @@ -1482,7 +1635,7 @@ mod test { // TODO(gloas) make this generic over gloas/fulu async fn empty_data_column_sidecars_fails_validation_fulu( harness: &BeaconChainHarness>, - verify_fn: &impl Fn(DataColumnSidecar) -> Result, + verify_fn: impl Fn(DataColumnSidecar, usize) -> Result, ) { let slot = harness.get_current_slot(); let state = harness.get_current_state(); @@ -1506,7 +1659,10 @@ mod test { .unwrap(), }); - let result = verify_fn(column_sidecar); + let result = verify_fn( + column_sidecar, + block.message().blob_kzg_commitments_len().unwrap(), + ); assert!(matches!( result.err(), Some(GossipDataColumnError::UnexpectedDataColumn) @@ -1515,7 +1671,7 @@ mod test { async fn data_column_sidecar_commitments_exceed_max_blobs_per_block( harness: &BeaconChainHarness>, - verify_fn: &impl Fn(DataColumnSidecar) -> Result, + verify_fn: &impl Fn(DataColumnSidecar, usize) -> Result, ) { let slot = harness.get_current_slot(); let epoch = slot.epoch(E::slots_per_epoch()); @@ -1545,7 +1701,10 @@ mod test { .next() .unwrap(); - let result = verify_fn(Arc::try_unwrap(column_sidecar).unwrap()); + let result = verify_fn( + Arc::try_unwrap(column_sidecar).unwrap(), + block.message().blob_kzg_commitments_len().unwrap(), + ); assert!(matches!( result.err(), Some(GossipDataColumnError::MaxBlobsPerBlockExceeded { .. }) diff --git a/beacon_node/http_api/src/beacon/execution_payload_envelope.rs b/beacon_node/http_api/src/beacon/execution_payload_envelope.rs index 06a5915c081..1fb55fa63ee 100644 --- a/beacon_node/http_api/src/beacon/execution_payload_envelope.rs +++ b/beacon_node/http_api/src/beacon/execution_payload_envelope.rs @@ -10,6 +10,7 @@ use beacon_chain::data_column_verification::{GossipDataColumnError, GossipVerifi use beacon_chain::{BeaconChain, BeaconChainTypes}; use bytes::Bytes; use eth2::types as api_types; +use eth2::types::FullPayloadContents; use eth2::{CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER}; use lighthouse_network::PubsubMessage; use network::NetworkMessage; @@ -18,6 +19,7 @@ use std::future::Future; use std::sync::Arc; use tokio::sync::mpsc::UnboundedSender; use tracing::{debug, error, info, warn}; +use tree_hash::TreeHash; use types::{EthSpec, SignedExecutionPayloadEnvelope}; use warp::{ Filter, Rejection, Reply, @@ -116,20 +118,33 @@ pub async fn publish_execution_payload_envelope( let blobs_and_proofs = chain.pending_payload_envelopes.write().take_blobs(slot); - // Spawn the column-build task (CPU-bound KZG cell-and-proof computation) before - // publishing the envelope so it runs in parallel with envelope gossip, narrowing - // the window in which peers see envelope-without-columns. If envelope publication - // fails below, dropping this future drops the spawned `JoinHandle` (the running - // closure on the blocking pool finishes and is then discarded — no work cancellation). - let column_build_future = match blobs_and_proofs { - Some(blobs) if !blobs.is_empty() => Some(spawn_build_gloas_data_columns_task( - &chain, - beacon_block_root, - slot, - blobs, - )?), - _ => None, - }; + let full_payload_contents = chain + .execution_layer + .as_ref() + .ok_or_else(|| warp_utils::reject::custom_bad_request("Execution layer missing".into()))? + .get_payload_by_root(&envelope.message.payload.tree_hash_root()) + .ok_or_else(|| warp_utils::reject::custom_bad_request("Payload missing".into()))?; + + let column_build_future = + if let FullPayloadContents::PayloadAndBlobs(payload_and_blobs) = full_payload_contents { + // Spawn the column-build task (CPU-bound KZG cell-and-proof computation) before + // publishing the envelope so it runs in parallel with envelope gossip, narrowing + // the window in which peers see envelope-without-columns. If envelope publication + // fails below, dropping this future drops the spawned `JoinHandle` (the running + // closure on the blocking pool finishes and is then discarded — no work cancellation). + match blobs_and_proofs { + Some(blobs) if !blobs.is_empty() => Some(spawn_build_gloas_data_columns_task( + &chain, + beacon_block_root, + payload_and_blobs.blobs_bundle.commitments.len(), + slot, + blobs, + )?), + _ => None, + } + } else { + None + }; // Publish the envelope to the network. crate::utils::publish_pubsub_message( @@ -196,6 +211,7 @@ pub async fn publish_execution_payload_envelope( fn spawn_build_gloas_data_columns_task( chain: &Arc>, beacon_block_root: types::Hash256, + commitments_len: usize, slot: types::Slot, blobs: types::BlobsList, ) -> Result>, Rejection>>, Rejection> { @@ -203,7 +219,15 @@ fn spawn_build_gloas_data_columns_task( let handle = chain .task_executor .spawn_blocking_handle( - move || build_gloas_data_columns(&chain_for_build, beacon_block_root, slot, &blobs), + move || { + build_gloas_data_columns( + &chain_for_build, + beacon_block_root, + commitments_len, + slot, + &blobs, + ) + }, "build_gloas_data_columns", ) .ok_or_else(|| warp_utils::reject::custom_server_error("runtime shutdown".to_string()))?; @@ -218,6 +242,7 @@ fn spawn_build_gloas_data_columns_task( fn build_gloas_data_columns( chain: &BeaconChain, beacon_block_root: types::Hash256, + commitments_len: usize, slot: types::Slot, blobs: &types::BlobsList, ) -> Result>, Rejection> { @@ -242,7 +267,7 @@ fn build_gloas_data_columns( .into_iter() .filter_map(|col| { let index = *col.index(); - match GossipVerifiedDataColumn::new_for_block_publishing(col, chain) { + match GossipVerifiedDataColumn::new_for_block_publishing(col, commitments_len, chain) { Ok(verified) => Some(verified), Err(GossipDataColumnError::PriorKnownUnpublished) => None, Err(e) => { diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 644ade956af..e5b4ee684b9 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -417,7 +417,12 @@ fn build_data_columns( let gossip_verified_data_columns = data_column_sidecars .into_iter() .filter_map(|data_column_sidecar| { - GossipVerifiedDataColumn::new_for_block_publishing(data_column_sidecar, chain).ok() + GossipVerifiedDataColumn::new_for_block_publishing( + data_column_sidecar, + block.num_expected_blobs(), + chain, + ) + .ok() }) .collect::>(); diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 29306c198dd..225007d60c9 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -727,7 +727,7 @@ impl NetworkBeaconProcessor { %parent_root, "Unknown parent hash for column" ); - self.send_sync_message(SyncMessage::UnknownParentDataColumn( + self.send_sync_message(SyncMessage::UnknownDataColumnParentOrBlock( peer_id, column_sidecar, )); @@ -739,6 +739,20 @@ impl NetworkBeaconProcessor { "Internal error when verifying column sidecar" ) } + GossipDataColumnError::BlockUnknown { + beacon_block_root, .. + } => { + debug!( + action = "queuing for block", + %block_root, + %beacon_block_root, + "Block not yet known for Gloas data column sidecar" + ); + self.send_sync_message(SyncMessage::UnknownDataColumnParentOrBlock( + peer_id, + column_sidecar, + )); + } GossipDataColumnError::ProposalSignatureInvalid | GossipDataColumnError::UnknownValidator(_) | GossipDataColumnError::ProposerIndexMismatch { .. } @@ -752,7 +766,8 @@ impl NetworkBeaconProcessor { | GossipDataColumnError::MaxBlobsPerBlockExceeded { .. } | GossipDataColumnError::InconsistentCommitmentsLength { .. } | GossipDataColumnError::InconsistentProofsLength { .. } - | GossipDataColumnError::NotFinalizedDescendant { .. } => { + | GossipDataColumnError::NotFinalizedDescendant { .. } + | GossipDataColumnError::SlotMismatch { .. } => { debug!( error = ?err, %slot, @@ -933,6 +948,14 @@ impl NetworkBeaconProcessor { slot, }); } + GossipDataColumnError::BlockUnknown { .. } => { + debug!( + %block_root, + %index, + "Block not yet known for Gloas partial data column" + ); + // TODO(gloas): send sync message + } GossipDataColumnError::PubkeyCacheTimeout | GossipDataColumnError::BeaconChainError(_) => { crit!( @@ -953,7 +976,8 @@ impl NetworkBeaconProcessor { | GossipDataColumnError::MaxBlobsPerBlockExceeded { .. } | GossipDataColumnError::InconsistentCommitmentsLength { .. } | GossipDataColumnError::InconsistentProofsLength { .. } - | GossipDataColumnError::NotFinalizedDescendant { .. } => { + | GossipDataColumnError::NotFinalizedDescendant { .. } + | GossipDataColumnError::SlotMismatch { .. } => { debug!( error = ?err, %block_root, diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index 3929f74aa04..6f5799cc497 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -232,10 +232,11 @@ impl BlockLookups { pub fn search_unknown_block( &mut self, block_root: Hash256, + block_component: Option>, peer_source: &[PeerId], cx: &mut SyncNetworkContext, ) -> bool { - self.new_current_lookup(block_root, None, None, peer_source, cx) + self.new_current_lookup(block_root, block_component, None, peer_source, cx) } /// A block or blob triggers the search of a parent. diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 734295ac1d3..83aa56dccd6 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -139,7 +139,8 @@ pub enum SyncMessage { UnknownParentBlob(PeerId, Arc>), /// A data column with an unknown parent has been received. - UnknownParentDataColumn(PeerId, Arc>), + /// For Fulu columns, this signifies a missing parent, for Gloas, a missing block with the bid. + UnknownDataColumnParentOrBlock(PeerId, Arc>), /// A partial data column with an unknown parent has been received. UnknownParentPartialDataColumn { @@ -426,7 +427,7 @@ impl SyncManager { // // TODO: This fork-choice check is potentially duplicated, review code if !self.chain.block_is_known_to_fork_choice(&remote.head_root) { - self.handle_unknown_block_root(peer_id, remote.head_root); + self.handle_unknown_block_root(peer_id, remote.head_root, None); } } } @@ -881,7 +882,7 @@ impl SyncManager { }), ); } - SyncMessage::UnknownParentDataColumn(peer_id, data_column) => { + SyncMessage::UnknownDataColumnParentOrBlock(peer_id, data_column) => { let data_column_slot = data_column.slot(); let block_root = data_column.block_root(); match data_column.as_ref() { @@ -905,9 +906,22 @@ impl SyncManager { }), ); } - // TODO(gloas) support gloas data column variant DataColumnSidecar::Gloas(_) => { - error!("Gloas variant not yet supported") + debug!(%block_root, "Received unknown block data column message"); + self.handle_unknown_block_root( + peer_id, + block_root, + Some(BlockComponent::DataColumn(DownloadResult { + value: block_root, + block_root, + seen_timestamp: self + .chain + .slot_clock + .now_duration() + .unwrap_or_default(), + peer_group: PeerGroup::from_single(peer_id), + })), + ); } } } @@ -935,7 +949,7 @@ impl SyncManager { if !self.notified_unknown_roots.contains(&(peer_id, block_root)) { self.notified_unknown_roots.insert((peer_id, block_root)); debug!(?block_root, ?peer_id, "Received unknown block hash message"); - self.handle_unknown_block_root(peer_id, block_root); + self.handle_unknown_block_root(peer_id, block_root, None); } } SyncMessage::Disconnect(peer_id) => { @@ -1036,11 +1050,17 @@ impl SyncManager { } } - fn handle_unknown_block_root(&mut self, peer_id: PeerId, block_root: Hash256) { + fn handle_unknown_block_root( + &mut self, + peer_id: PeerId, + block_root: Hash256, + block_component: Option>, + ) { match self.should_search_for_block(None, &peer_id) { Ok(_) => { if self.block_lookups.search_unknown_block( block_root, + block_component, &[peer_id], &mut self.network, ) { diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index a26996ec5ee..e1409a0b2d1 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -1474,7 +1474,7 @@ impl TestRig { peer_id: PeerId, column: Arc>, ) { - self.send_sync_message(SyncMessage::UnknownParentDataColumn(peer_id, column)); + self.send_sync_message(SyncMessage::UnknownDataColumnParentOrBlock(peer_id, column)); } fn trigger_unknown_block_from_attestation(&mut self, block_root: Hash256, peer_id: PeerId) {