From dac8a6ec8d4c22446daf68294b852e0c3ca873f0 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 1 May 2026 03:46:10 +0200 Subject: [PATCH] Gloas: fix test failures (KZG verifier wiring, harness columns, WSS sync) Brings the FORK_NAME=gloas beacon_chain test suite from 31 failures to green: - v1 KZG batch verifier couldn't verify Gloas columns. Added verify_columns_against_block helper that picks commitments per fork (Fulu: inline on column; Gloas: signed_execution_payload_bid). - BeaconChainHarness::process_envelope didn't persist columns. Now mirrors what production does in import_available_execution_payload_envelope. - get_or_reconstruct_blobs returned an error for Gloas. Now short-circuits to Ok(None); WSS test copies columns from source to dest directly. - update_data_column_signed_header (block_verification tests) only handled Fulu shape. Added a Gloas branch that re-keys to canonical_root. - BlockError::EnvelopeBlockRootUnknown changed to tuple variant. - Removed duplicate process_payload_envelope_availability. --- beacon_node/beacon_chain/src/beacon_chain.rs | 20 +++++-- .../beacon_chain/src/block_verification.rs | 2 +- .../src/data_availability_checker.rs | 50 ++++++++++++---- beacon_node/beacon_chain/src/kzg_utils.rs | 19 ++++-- .../payload_envelope_verification/import.rs | 4 +- .../pending_payload_cache/pending_column.rs | 14 +++-- .../pending_components.rs | 6 +- beacon_node/beacon_chain/src/test_utils.rs | 46 ++++++++++++--- .../beacon_chain/tests/block_verification.rs | 40 +++++++++---- beacon_node/beacon_chain/tests/store_tests.rs | 58 ++++++++++++++++++- .../gossip_methods.rs | 2 +- 11 files changed, 205 insertions(+), 56 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a7f289b3f1..0a48a3b168 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1331,6 +1331,14 @@ impl BeaconChain { return Ok(None); }; + // Gloas removes the standalone `BlobSidecar` shape — KZG commitments live in the bid and + // there's no signed-block-header / inclusion-proof to populate a `BlobSidecar` from. The + // canonical data is the column sidecar set on disk; callers needing data for a Gloas + // block should consume columns directly via `get_data_columns`. + if block.fork_name_unchecked().gloas_enabled() { + return Ok(None); + } + if self.spec.is_peer_das_enabled_for_epoch(block.epoch()) { let fork_name = self.spec.fork_name_at_epoch(block.epoch()); if let Some(columns) = self.store.get_data_columns(block_root, fork_name)? { @@ -3442,7 +3450,7 @@ impl BeaconChain { .gloas_enabled() { let bid = load_gloas_payload_bid(block_root, self)? - .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; + .ok_or(BlockError::EnvelopeBlockRootUnknown(block_root))?; let availability = self .pending_payload_cache .put_kzg_verified_custody_data_columns( @@ -3680,7 +3688,7 @@ impl BeaconChain { if is_gloas { let bid = load_gloas_payload_bid(block_root, self)? - .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; + .ok_or(BlockError::EnvelopeBlockRootUnknown(block_root))?; let pending_payload_cache = self.pending_payload_cache.clone(); let result = self .task_executor @@ -4005,7 +4013,7 @@ impl BeaconChain { .gloas_enabled() { let bid = load_gloas_payload_bid(block_root, self)? - .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; + .ok_or(BlockError::EnvelopeBlockRootUnknown(block_root))?; let availability = self .pending_payload_cache .put_gossip_verified_data_columns(block_root, bid, data_columns)?; @@ -4111,7 +4119,7 @@ impl BeaconChain { .gloas_enabled() { let bid = load_gloas_payload_bid(block_root, self)? - .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; + .ok_or(BlockError::EnvelopeBlockRootUnknown(block_root))?; let availability = self .pending_payload_cache .put_kzg_verified_custody_data_columns(block_root, bid, &data_columns) @@ -4155,7 +4163,7 @@ impl BeaconChain { .gloas_enabled() { let bid = load_gloas_payload_bid(block_root, self)? - .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; + .ok_or(BlockError::EnvelopeBlockRootUnknown(block_root))?; let availability = self .pending_payload_cache .put_rpc_custody_columns(block_root, bid, custody_columns) @@ -7750,7 +7758,7 @@ impl BeaconChain { ) } - pub(crate) fn get_blobs_or_columns_store_op( + pub fn get_blobs_or_columns_store_op( &self, block_root: Hash256, block_slot: Slot, diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index eeee562e9c..24f971f736 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -287,7 +287,7 @@ pub enum BlockError { /// https://github.com/sigp/lighthouse/issues/4546 AvailabilityCheck(AvailabilityCheckError), /// The payload envelope's block root is unknown. - EnvelopeBlockRootUnknown { block_root: Hash256 }, + EnvelopeBlockRootUnknown(Hash256), /// Optimistic sync is not supported for Gloas payload envelopes. OptimisticSyncNotSupported { block_root: Hash256 }, /// A Blob with a slot after PeerDAS is received and is not required to be imported. diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 928d7a1bad..bdf16ec52c 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -33,6 +33,7 @@ use crate::data_column_verification::{ GossipVerifiedDataColumn, KzgVerifiedCustodyDataColumn, KzgVerifiedDataColumn, verify_kzg_for_data_column_list, }; +use crate::kzg_utils::validate_full_data_columns_with_commitments; use crate::metrics::{ KZG_DATA_COLUMN_RECONSTRUCTION_ATTEMPTS, KZG_DATA_COLUMN_RECONSTRUCTION_FAILURES, }; @@ -490,8 +491,7 @@ impl DataAvailabilityChecker { AvailableBlockData::Blobs(blobs) => verify_kzg_for_blob_list(blobs.iter(), &self.kzg) .map_err(AvailabilityCheckError::InvalidBlobs), AvailableBlockData::DataColumns(columns) => { - verify_kzg_for_data_column_list(columns.iter(), &self.kzg) - .map_err(AvailabilityCheckError::InvalidColumn) + verify_columns_against_block(&self.kzg, available_block.block(), columns) } } } @@ -504,13 +504,17 @@ impl DataAvailabilityChecker { available_blocks: &[AvailableBlock], ) -> Result<(), AvailabilityCheckError> { let mut all_blobs = Vec::new(); - let mut all_data_columns = Vec::new(); for available_block in available_blocks { - match available_block.data().to_owned() { + match available_block.data() { AvailableBlockData::NoData => {} - AvailableBlockData::Blobs(blobs) => all_blobs.extend(blobs), - AvailableBlockData::DataColumns(columns) => all_data_columns.extend(columns), + AvailableBlockData::Blobs(blobs) => all_blobs.extend(blobs.iter().cloned()), + AvailableBlockData::DataColumns(columns) => { + // Each block has its own commitments. For Gloas they live in the bid; for + // Fulu they live inline on the column. Verify per block and let the helper + // pick the right path. + verify_columns_against_block(&self.kzg, available_block.block(), columns)?; + } } } @@ -519,11 +523,6 @@ impl DataAvailabilityChecker { .map_err(AvailabilityCheckError::InvalidBlobs)?; } - if !all_data_columns.is_empty() { - verify_kzg_for_data_column_list(all_data_columns.iter(), &self.kzg) - .map_err(AvailabilityCheckError::InvalidColumn)?; - } - Ok(()) } @@ -679,6 +678,35 @@ impl DataAvailabilityChecker { } } +/// Verify a batch of data columns belonging to a single block, picking the right commitment +/// source for the block's fork (Fulu: inline on column; Gloas: from the embedded payload bid). +fn verify_columns_against_block( + kzg: &Kzg, + block: &SignedBeaconBlock, + columns: &[Arc>], +) -> Result<(), AvailabilityCheckError> { + if columns.is_empty() { + return Ok(()); + } + if block.fork_name_unchecked().gloas_enabled() { + let commitments = block + .message() + .body() + .signed_execution_payload_bid() + .map(|bid| bid.message.blob_kzg_commitments.clone()) + .map_err(|_| { + AvailabilityCheckError::Unexpected( + "Gloas block missing signed_execution_payload_bid".to_string(), + ) + })?; + validate_full_data_columns_with_commitments(kzg, columns.iter(), commitments.as_ref()) + .map_err(AvailabilityCheckError::InvalidColumn) + } else { + verify_kzg_for_data_column_list(columns.iter(), kzg) + .map_err(AvailabilityCheckError::InvalidColumn) + } +} + /// Helper struct to group data availability checker metrics. pub struct DataAvailabilityCheckerMetrics { pub block_cache_size: usize, diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 5ad1cc115d..e1558f050b 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -676,12 +676,19 @@ pub fn reconstruct_blobs( let blob_indices: Vec = match blob_indices_opt { Some(indices) => indices.into_iter().map(|i| i as usize).collect(), None => { - // TODO(gloas): support blob reconstruction for Gloas - // https://github.com/sigp/lighthouse/issues/7413 - let num_of_blobs = first_data_column - .kzg_commitments() - .map_err(|_| "Gloas blob reconstruction not yet supported".to_string())? - .len(); + // Fulu columns carry commitments inline; Gloas columns don't, so fall back to + // the block's payload bid commitments. + let num_of_blobs = match first_data_column.kzg_commitments() { + Ok(commitments) => commitments.len(), + Err(_) => signed_block + .message() + .body() + .signed_execution_payload_bid() + .map(|bid| bid.message.blob_kzg_commitments.len()) + .map_err(|_| { + "Gloas blob reconstruction: block missing payload bid".to_string() + })?, + }; (0..num_of_blobs).collect() } }; diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 10d97add1d..e7c900dcd7 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -163,7 +163,7 @@ impl BeaconChain { let slot = envelope.envelope.slot(); let block_root = envelope.block_root; let bid = load_gloas_payload_bid(block_root, self)? - .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; + .ok_or(BlockError::EnvelopeBlockRootUnknown(block_root))?; let availability = self .pending_payload_cache .put_executed_payload_envelope(bid, envelope)?; @@ -253,7 +253,7 @@ impl BeaconChain { // been imported. We don't want to repeat work importing a block that is already imported. let fork_choice_reader = self.canonical_head.fork_choice_upgradable_read_lock(); if !fork_choice_reader.contains_block(&block_root) { - return Err(BlockError::EnvelopeBlockRootUnknown { block_root }); + return Err(BlockError::EnvelopeBlockRootUnknown(block_root)); } // TODO(gloas) add defensive check to see if payload envelope is already in fork choice diff --git a/beacon_node/beacon_chain/src/pending_payload_cache/pending_column.rs b/beacon_node/beacon_chain/src/pending_payload_cache/pending_column.rs index 1c7a0389c3..0844bbf0f9 100644 --- a/beacon_node/beacon_chain/src/pending_payload_cache/pending_column.rs +++ b/beacon_node/beacon_chain/src/pending_payload_cache/pending_column.rs @@ -8,13 +8,15 @@ pub struct PendingColumn { cells: Vec, KzgProof)>>, } -impl Default for PendingColumn { - fn default() -> Self { - Self { cells: Vec::new() } - } -} - impl PendingColumn { + /// Allocate a `PendingColumn` whose `cells` vec has space for `blob_count` entries, all + /// initialised to `None`. Required so that `insert(idx, ...)` can write into `cells[idx]`. + pub fn new_with_capacity(blob_count: usize) -> Self { + Self { + cells: vec![None; blob_count], + } + } + pub fn insert(&mut self, index: usize, cell: &Cell, proof: &KzgProof) { if let Some(existing_cell) = self.cells.get_mut(index) && existing_cell.is_none() diff --git a/beacon_node/beacon_chain/src/pending_payload_cache/pending_components.rs b/beacon_node/beacon_chain/src/pending_payload_cache/pending_components.rs index 20e6d1b52f..0516afd4e9 100644 --- a/beacon_node/beacon_chain/src/pending_payload_cache/pending_components.rs +++ b/beacon_node/beacon_chain/src/pending_payload_cache/pending_components.rs @@ -79,12 +79,16 @@ impl PendingComponents { &mut self, kzg_verified_data_columns: &[KzgVerifiedCustodyDataColumn], ) -> Result<(), AvailabilityCheckError> { + let num_blobs_expected = self.num_blobs_expected(); for data_column in kzg_verified_data_columns { let data_column = data_column.as_data_column(); + // The Vec-backed `PendingColumn` keys cells by index, so we have to allocate up to + // `num_blobs_expected` entries before inserting; otherwise `cells.get_mut(idx)` returns + // None and the insert is a no-op. let col = self .verified_data_columns .entry(*data_column.index()) - .or_default(); + .or_insert_with(|| PendingColumn::new_with_capacity(num_blobs_expected)); for (cell_idx, (cell, proof)) in data_column .column() .iter() diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 4a9b7f7fa0..68cec11b76 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2832,11 +2832,42 @@ where .await .expect("newPayload should succeed"); - // Store the envelope. + // Store the envelope and the data columns derived from the block. + // + // Production stores columns inside `import_available_execution_payload_envelope` after + // the cache is satisfied. The harness sidesteps that flow but must still persist columns + // or the `DataColumnMissing` invariant fires for any block with `num_expected_blobs > 0`. + let block = self + .chain + .store + .get_blinded_block(&block_root) + .expect("should read block from store") + .expect("block should exist in store"); + let mut ops = vec![]; + let block_with_full_payload = self + .chain + .store + .make_full_block(&block_root, block.clone()) + .expect("should reconstruct full block"); + let columns = + generate_data_column_sidecars_from_block(&block_with_full_payload, &self.spec); + if !columns.is_empty() + && let Some(store_op) = self.chain.get_blobs_or_columns_store_op( + block_root, + block.slot(), + AvailableBlockData::DataColumns(columns), + ) + { + ops.push(store_op); + } + ops.push(store::StoreOp::PutPayloadEnvelope( + block_root, + std::sync::Arc::new(signed_envelope), + )); self.chain .store - .put_payload_envelope(&block_root, &signed_envelope) - .expect("should store envelope"); + .do_atomically_with_block_and_blobs_cache(ops) + .expect("should persist envelope and columns"); // Update fork choice so it knows the payload was received. self.chain @@ -2857,11 +2888,10 @@ where block: Arc>, ) -> RangeSyncBlock { let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); - let has_blobs = block - .message() - .body() - .blob_kzg_commitments() - .is_ok_and(|c| !c.is_empty()); + // For Gloas, kzg commitments live in the bid (`signed_execution_payload_bid`), so the + // body's `blob_kzg_commitments()` accessor returns Err. `num_expected_blobs` already + // handles both shapes. + let has_blobs = block.num_expected_blobs() > 0; if !has_blobs { return RangeSyncBlock::new( block, diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index af858874b2..38636540a1 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -323,18 +323,34 @@ fn update_data_column_signed_header( ) { for old_custody_column_sidecar in data_columns.as_mut_slice() { let old_column_sidecar = old_custody_column_sidecar.as_data_column(); - let new_column_sidecar = Arc::new(DataColumnSidecar::Fulu(DataColumnSidecarFulu { - index: *old_column_sidecar.index(), - column: old_column_sidecar.column().clone(), - kzg_commitments: old_column_sidecar.kzg_commitments().unwrap().clone(), - kzg_proofs: old_column_sidecar.kzg_proofs().clone(), - signed_block_header: signed_block.signed_block_header(), - kzg_commitments_inclusion_proof: signed_block - .message() - .body() - .kzg_commitments_merkle_proof() - .unwrap(), - })); + let new_column_sidecar = match old_column_sidecar.as_ref() { + DataColumnSidecar::Fulu(_) => { + Arc::new(DataColumnSidecar::Fulu(DataColumnSidecarFulu { + index: *old_column_sidecar.index(), + column: old_column_sidecar.column().clone(), + kzg_commitments: old_column_sidecar.kzg_commitments().unwrap().clone(), + kzg_proofs: old_column_sidecar.kzg_proofs().clone(), + signed_block_header: signed_block.signed_block_header(), + kzg_commitments_inclusion_proof: signed_block + .message() + .body() + .kzg_commitments_merkle_proof() + .unwrap(), + })) + } + // Gloas columns reference the block by `beacon_block_root` instead of holding the + // block header inline, so updating the parent root just means re-keying the column to + // the new canonical root. + DataColumnSidecar::Gloas(g) => { + Arc::new(DataColumnSidecar::Gloas(types::DataColumnSidecarGloas { + index: g.index, + column: g.column.clone(), + kzg_proofs: g.kzg_proofs.clone(), + slot: g.slot, + beacon_block_root: signed_block.canonical_root(), + })) + } + }; *old_custody_column_sidecar = CustodyDataColumn::from_asserted_custody(new_column_sidecar); } } diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 86adf50995..171c83b2a9 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -3133,6 +3133,29 @@ async fn weak_subjectivity_sync_test( let beacon_chain = Arc::new(beacon_chain); let wss_block_root = wss_block.canonical_root(); + + // For Gloas, blobs aren't a standalone shape — the WSS data is the column sidecar set, which + // `get_or_reconstruct_blobs` returns `None` for. Copy the WSS block's columns straight from + // the source store so that the destination has them after checkpoint sync, matching what + // network-driven WSS would produce in production. + if wss_block.fork_name_unchecked().gloas_enabled() + && let Ok(Some(source_columns)) = harness + .chain + .store + .get_data_columns(&wss_block_root, ForkName::Gloas) + && !source_columns.is_empty() + && let Some(store_op) = beacon_chain.get_blobs_or_columns_store_op( + wss_block_root, + wss_block.slot(), + beacon_chain::block_verification_types::AvailableBlockData::DataColumns(source_columns), + ) + { + beacon_chain + .store + .do_atomically_with_block_and_blobs_cache(vec![store_op]) + .unwrap(); + } + let store_wss_block = harness .chain .get_block(&wss_block_root) @@ -3200,12 +3223,43 @@ async fn weak_subjectivity_sync_test( .await .unwrap(); - // Store the envelope and apply it to fork choice. + // Store the envelope, its columns, and apply to fork choice. if let Some(envelope) = &snapshot.execution_envelope { + // Persist data columns for Gloas blocks. This mirrors what production does in + // `import_available_execution_payload_envelope` and what the harness now does in + // `process_envelope` — the WSS forward-sync loop bypasses both, so do it directly. + let mut ops = vec![]; + let columns_block = beacon_chain + .store + .get_blinded_block(&block_root) + .unwrap() + .and_then(|b| beacon_chain.store.make_full_block(&block_root, b).ok()); + if let Some(full_block) = columns_block { + let columns = beacon_chain::test_utils::generate_data_column_sidecars_from_block( + &full_block, + &beacon_chain.spec, + ); + if !columns.is_empty() + && let Some(store_op) = beacon_chain.get_blobs_or_columns_store_op( + block_root, + full_block.slot(), + beacon_chain::block_verification_types::AvailableBlockData::DataColumns( + columns, + ), + ) + { + ops.push(store_op); + } + } + ops.push(store::StoreOp::PutPayloadEnvelope( + block_root, + std::sync::Arc::new(envelope.as_ref().clone()), + )); beacon_chain .store - .put_payload_envelope(&block_root, envelope) + .do_atomically_with_block_and_blobs_cache(ops) .unwrap(); + // Update fork choice so head selection accounts for Full payload status. beacon_chain .canonical_head 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 aec2b1fcb8..4e636060e0 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -1848,7 +1848,7 @@ impl NetworkBeaconProcessor { // BlobNotRequired is unreachable. Only constructed in `process_gossip_blob` Err(e @ BlockError::InternalError(_)) | Err(e @ BlockError::BlobNotRequired(_)) - | Err(e @ BlockError::EnvelopeBlockRootUnknown { .. }) + | Err(e @ BlockError::EnvelopeBlockRootUnknown(_)) | Err(e @ BlockError::OptimisticSyncNotSupported { .. }) => { error!(error = %e, "Internal block gossip validation error"); return None;