diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 8456bbbc02..76e7b217ce 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -32,7 +32,7 @@ use crate::data_column_verification::{ }; use crate::early_attester_cache::EarlyAttesterCache; use crate::envelope_times_cache::EnvelopeTimesCache; -use crate::errors::{BeaconChainError as Error, BlockOrEnvelopeError, BlockProductionError}; +use crate::errors::{BeaconChainError as Error, BlockProductionError}; use crate::events::ServerSentEventHandler; use crate::execution_payload::{NotifyExecutionLayer, PreparePayloadHandle, get_execution_payload}; use crate::fetch_blobs::EngineGetBlobsOutput; @@ -67,7 +67,6 @@ use crate::payload_attestation_verification::VerifiedPayloadAttestationMessage; use crate::payload_bid_verification::payload_bid_cache::GossipVerifiedPayloadBidCache; #[cfg(not(test))] use crate::payload_envelope_streamer::{EnvelopeRequestSource, launch_payload_envelope_stream}; -use crate::payload_envelope_verification::EnvelopeError; use crate::pending_payload_cache::PendingPayloadCache; use crate::pending_payload_cache::{ Availability as PayloadAvailability, @@ -3304,7 +3303,7 @@ impl BeaconChain { self: &Arc, data_columns: Vec>, publish_fn: impl FnOnce() -> Result<(), BlockError>, - ) -> Result { + ) -> Result { let Ok((slot, block_root)) = data_columns .iter() .map(|c| (c.slot(), c.block_root())) @@ -3313,8 +3312,7 @@ impl BeaconChain { else { return Err(BlockError::InternalError( "Columns should be from the same block".to_string(), - ) - .into()); + )); }; let is_gloas = self @@ -3331,7 +3329,7 @@ impl BeaconChain { .fork_choice_read_lock() .contains_block(&block_root) { - return Err(BlockError::DuplicateFullyImported(block_root).into()); + return Err(BlockError::DuplicateFullyImported(block_root)); } self.emit_sse_data_column_sidecar_events( @@ -3357,7 +3355,7 @@ impl BeaconChain { verified_partial: KzgVerifiedPartialDataColumn, verified_header: GossipVerifiedPartialDataColumnHeader, slot: Slot, - ) -> Result, BlockOrEnvelopeError> { + ) -> Result, BlockError> { let block_root = verified_partial.block_root(); let partial = verified_partial.as_data_column(); let index_str = partial.index.to_string(); @@ -3382,7 +3380,7 @@ impl BeaconChain { .fork_choice_read_lock() .contains_block(&block_root) { - return Err(BlockError::DuplicateFullyImported(block_root).into()); + return Err(BlockError::DuplicateFullyImported(block_root)); } let Some(assembler) = self.data_availability_checker.partial_assembler() else { @@ -3426,11 +3424,8 @@ impl BeaconChain { .fork_name_at_slot::(slot) .gloas_enabled() { - let Some(bid) = - load_gloas_payload_bid(block_root, self).map_err(EnvelopeError::from)? - else { - return Err(EnvelopeError::BlockRootUnknown { block_root }.into()); - }; + let bid = load_gloas_payload_bid(block_root, self)? + .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; let availability = self .pending_payload_cache .put_kzg_verified_custody_data_columns( @@ -3438,7 +3433,7 @@ impl BeaconChain { bid, merge_result.full_columns.clone(), ) - .map_err(EnvelopeError::from)?; + .map_err(BlockError::from)?; self.process_payload_availability(slot, availability, || Ok(())) .await? } else { @@ -3505,7 +3500,7 @@ impl BeaconChain { slot: Slot, block_root: Hash256, engine_get_blobs_output: EngineGetBlobsOutput, - ) -> Result { + ) -> Result { // If this block has already been imported to forkchoice it must have been available, so // we don't need to process its blobs again. if self @@ -3513,7 +3508,7 @@ impl BeaconChain { .fork_choice_read_lock() .contains_block(&block_root) { - return Err(BlockError::DuplicateFullyImported(block_root).into()); + return Err(BlockError::DuplicateFullyImported(block_root)); } match &engine_get_blobs_output { @@ -3594,7 +3589,7 @@ impl BeaconChain { pub async fn process_rpc_custody_columns( self: &Arc, custody_columns: DataColumnSidecarList, - ) -> Result { + ) -> Result { let Ok((slot, block_root)) = custody_columns .iter() .map(|c| (c.slot(), c.block_root())) @@ -3603,8 +3598,7 @@ impl BeaconChain { else { return Err(BlockError::InternalError( "Columns should be from the same block".to_string(), - ) - .into()); + )); }; // If this block has already been imported to forkchoice it must have been available, so @@ -3616,7 +3610,7 @@ impl BeaconChain { .fork_choice_read_lock() .contains_block(&block_root) { - return Err(BlockError::DuplicateFullyImported(block_root).into()); + return Err(BlockError::DuplicateFullyImported(block_root)); } // Reject RPC columns referencing unknown parents. Otherwise we allow potentially invalid data @@ -3635,7 +3629,7 @@ impl BeaconChain { .fork_choice_read_lock() .contains_block(&parent_root) { - return Err(BlockError::ParentUnknown { parent_root }.into()); + return Err(BlockError::ParentUnknown { parent_root }); } self.emit_sse_data_column_sidecar_events( @@ -3644,9 +3638,8 @@ impl BeaconChain { custody_columns.iter().map(|column| column.as_ref()), ); - Ok(self - .check_rpc_custody_columns_availability_and_import(slot, block_root, custody_columns) - .await?) + self.check_rpc_custody_columns_availability_and_import(slot, block_root, custody_columns) + .await } pub async fn reconstruct_data_columns( @@ -3658,7 +3651,7 @@ impl BeaconChain { AvailabilityProcessingStatus, DataColumnSidecarList, )>, - BlockOrEnvelopeError, + BlockError, > { // As of now we only reconstruct data columns on supernodes, so if the block is already // available on a supernode, there's no need to reconstruct as the node must already have @@ -3677,11 +3670,8 @@ impl BeaconChain { .gloas_enabled(); if is_gloas { - let Some(bid) = - load_gloas_payload_bid(block_root, self).map_err(EnvelopeError::from)? - else { - return Err(EnvelopeError::BlockRootUnknown { block_root }.into()); - }; + let bid = load_gloas_payload_bid(block_root, self)? + .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; let pending_payload_cache = self.pending_payload_cache.clone(); let result = self .task_executor @@ -3689,8 +3679,8 @@ impl BeaconChain { pending_payload_cache.reconstruct_data_columns(&block_root, bid) }) .await - .map_err(|_| EnvelopeError::from(BeaconChainError::RuntimeShutdown))? - .map_err(EnvelopeError::from)?; + .map_err(|_| BlockError::from(BeaconChainError::RuntimeShutdown))? + .map_err(BlockError::from)?; match result { DataColumnReconstructionResultGloas::Success(( @@ -3991,7 +3981,7 @@ impl BeaconChain { block_root: Hash256, data_columns: Vec>, publish_fn: impl FnOnce() -> Result<(), BlockError>, - ) -> Result { + ) -> Result { if let Some(slasher) = self.slasher.as_ref() { for data_column in &data_columns { // TODO(gloas) different gossip checks in gloas @@ -4007,25 +3997,18 @@ impl BeaconChain { .fork_name_at_slot::(slot) .gloas_enabled() { - let Some(bid) = - load_gloas_payload_bid(block_root, self).map_err(EnvelopeError::from)? - else { - return Ok(AvailabilityProcessingStatus::MissingComponents( - slot, block_root, - )); - }; + let bid = load_gloas_payload_bid(block_root, self)? + .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; let availability = self .pending_payload_cache - .put_gossip_verified_data_columns(block_root, bid, data_columns) - .map_err(EnvelopeError::from)?; + .put_gossip_verified_data_columns(block_root, bid, data_columns)?; Ok(self .process_payload_availability(slot, availability, publish_fn) .await?) } else { let availability = self .data_availability_checker - .put_gossip_verified_data_columns(block_root, slot, data_columns) - .map_err(BlockError::from)?; + .put_gossip_verified_data_columns(block_root, slot, data_columns)?; Ok(self .process_availability(slot, availability, publish_fn) .await?) @@ -4088,7 +4071,7 @@ impl BeaconChain { slot: Slot, block_root: Hash256, engine_get_blobs_output: EngineGetBlobsOutput, - ) -> Result { + ) -> Result { match engine_get_blobs_output { EngineGetBlobsOutput::Blobs(blobs) => { self.check_blob_header_signature_and_slashability( @@ -4120,17 +4103,12 @@ impl BeaconChain { .fork_name_at_slot::(slot) .gloas_enabled() { - let Some(bid) = - load_gloas_payload_bid(block_root, self).map_err(EnvelopeError::from)? - else { - return Ok(AvailabilityProcessingStatus::MissingComponents( - slot, block_root, - )); - }; + let bid = load_gloas_payload_bid(block_root, self)? + .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; let availability = self .pending_payload_cache .put_kzg_verified_custody_data_columns(block_root, bid, data_columns) - .map_err(EnvelopeError::from)?; + .map_err(BlockError::from)?; Ok(self .process_payload_availability(slot, availability, || Ok(())) .await?) @@ -4169,23 +4147,15 @@ impl BeaconChain { .fork_name_at_slot::(slot) .gloas_enabled() { - let Some(bid) = load_gloas_payload_bid(block_root, self).map_err(BlockError::from)? - else { - return Ok(AvailabilityProcessingStatus::MissingComponents( - slot, block_root, - )); - }; + let bid = load_gloas_payload_bid(block_root, self)? + .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; let availability = self .pending_payload_cache .put_rpc_custody_columns(block_root, bid, custody_columns) .map_err(BlockError::from)?; Ok(self .process_payload_availability(slot, availability, || Ok(())) - .await - .map_err(|error| match error { - EnvelopeError::BlockError(error) => error, - error => BlockError::InternalError(error.to_string()), - })?) + .await?) } else { let availability = self .data_availability_checker @@ -4256,7 +4226,7 @@ impl BeaconChain { slot: Slot, availability: PayloadAvailability, publish_fn: impl FnOnce() -> Result<(), BlockError>, - ) -> Result { + ) -> Result { match availability { PayloadAvailability::Available(available_envelope) => { publish_fn()?; diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index b70730d047..eeee562e9c 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -286,6 +286,10 @@ pub enum BlockError { /// TODO: We may need to penalize the peer that gave us a potentially invalid rpc blob. /// https://github.com/sigp/lighthouse/issues/4546 AvailabilityCheck(AvailabilityCheckError), + /// The payload envelope's block root is unknown. + EnvelopeBlockRootUnknown { block_root: 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. /// This can happen because we stay subscribed to the blob subnet after 2 epochs, as we could /// still receive valid blobs from a Deneb epoch after PeerDAS is activated. diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index e342a05798..7036ecdc70 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -1307,9 +1307,11 @@ pub(crate) fn load_gloas_payload_bid( block_root: Hash256, chain: &BeaconChain, ) -> Result>, BeaconChainError> { - let bid = if let Some(bid) = chain.pending_payload_cache.get_bid(&block_root) { - bid - } else if let Some(block) = chain.early_attester_cache.get_block(block_root) { + if let Some(bid) = chain.pending_payload_cache.get_bid(&block_root) { + return Ok(Some(bid)); + } + + let bid = if let Some(block) = chain.early_attester_cache.get_block(block_root) { PendingPayloadBid::from_block(block.as_ref()).map_err(BeaconChainError::BeaconStateError)? } else { match chain diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 361521cea5..9802f091e0 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -1,4 +1,3 @@ -use crate::BlockError; use crate::beacon_block_streamer::Error as BlockStreamerError; use crate::beacon_chain::ForkChoiceError; use crate::beacon_fork_choice_store::Error as ForkChoiceStoreError; @@ -10,7 +9,6 @@ use crate::observed_attesters::Error as ObservedAttestersError; use crate::observed_block_producers::Error as ObservedBlockProducersError; use crate::observed_data_sidecars::Error as ObservedDataSidecarsError; use crate::payload_envelope_streamer::Error as EnvelopeStreamerError; -use crate::payload_envelope_verification::EnvelopeError; use bls::PublicKeyBytes; use execution_layer::PayloadStatus; use fork_choice::ExecutionStatus; @@ -336,21 +334,3 @@ easy_from_to!(SlotProcessingError, BlockProductionError); easy_from_to!(StateAdvanceError, BlockProductionError); easy_from_to!(ForkChoiceError, BlockProductionError); easy_from_to!(EpochCacheError, BlockProductionError); - -#[derive(Debug)] -pub enum BlockOrEnvelopeError { - BlockError(BlockError), - EnvelopeError(EnvelopeError), -} - -easy_from_to!(BlockError, BlockOrEnvelopeError); -easy_from_to!(EnvelopeError, BlockOrEnvelopeError); - -impl AsRef for BlockOrEnvelopeError { - fn as_ref(&self) -> &str { - match self { - BlockOrEnvelopeError::BlockError(e) => e.as_ref(), - BlockOrEnvelopeError::EnvelopeError(e) => e.as_ref(), - } - } -} diff --git a/beacon_node/beacon_chain/src/fetch_blobs/mod.rs b/beacon_node/beacon_chain/src/fetch_blobs/mod.rs index 7d26a603c8..335d76b410 100644 --- a/beacon_node/beacon_chain/src/fetch_blobs/mod.rs +++ b/beacon_node/beacon_chain/src/fetch_blobs/mod.rs @@ -16,13 +16,13 @@ use crate::blob_verification::{GossipBlobError, KzgVerifiedBlob}; use crate::data_column_verification::{ KzgVerifiedCustodyDataColumn, KzgVerifiedCustodyPartialDataColumn, KzgVerifiedPartialDataColumn, }; -use crate::errors::BlockOrEnvelopeError; #[cfg_attr(test, double)] use crate::fetch_blobs::fetch_blobs_beacon_adapter::FetchBlobsBeaconAdapter; use crate::kzg_utils::blobs_to_partial_data_columns; use crate::observed_data_sidecars::ObservationKey; use crate::{ - AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, metrics, + AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, + metrics, }; use execution_layer::Error as ExecutionLayerError; use execution_layer::json_structures::{BlobAndProofV1, BlobAndProofV2, BlobAndProofV3}; @@ -50,7 +50,7 @@ pub enum EngineGetBlobsOutput { pub enum FetchEngineBlobError { BeaconStateError(BeaconStateError), BeaconChainError(Box), - BlobProcessingError(Box), + BlobProcessingError(Box), BlobSidecarError(BlobSidecarError), DataColumnSidecarError(DataColumnSidecarError), ExecutionLayerMissing, diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index bd9c4a7c12..804268a613 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -76,7 +76,7 @@ pub use self::beacon_chain::{ }; pub use self::beacon_snapshot::BeaconSnapshot; pub use self::chain_config::ChainConfig; -pub use self::errors::{BeaconChainError, BlockOrEnvelopeError, BlockProductionError}; +pub use self::errors::{BeaconChainError, BlockProductionError}; pub use self::historical_blocks::HistoricalBlockError; pub use attestation_verification::Error as AttestationError; pub use beacon_fork_choice_store::{ 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 8b7c7eb4d7..975a684e95 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -15,7 +15,7 @@ use super::{ use crate::data_column_verification::load_gloas_payload_bid; use crate::pending_payload_cache::Availability as PayloadAvailability; use crate::{ - AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, + AvailabilityProcessingStatus, BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, NotifyExecutionLayer, block_verification_types::AvailableBlockData, metrics, @@ -85,7 +85,13 @@ impl BeaconChain { // about what the function actually does. let executed_envelope = chain .into_executed_payload_envelope(execution_pending) - .await?; + .await + .map_err(|error| match error { + BlockError::ExecutionPayloadError(error) => { + EnvelopeError::ExecutionPayloadError(error) + } + error => EnvelopeError::ImportError(error), + })?; // Record the time it took to wait for execution layer verification. if let Some(timestamp) = slot_clock.now_duration() { @@ -96,6 +102,7 @@ impl BeaconChain { self.check_envelope_availability_and_import(executed_envelope) .await + .map_err(EnvelopeError::ImportError) }; // Verify and import the payload envelope. @@ -131,6 +138,14 @@ impl BeaconChain { } Err(EnvelopeError::BeaconChainError(e)) } + Err(EnvelopeError::ImportError(BlockError::BeaconChainError(e))) => { + if matches!(e.as_ref(), BeaconChainError::TokioJoin(_)) { + debug!(error = ?e, "Envelope processing cancelled"); + } else { + warn!(error = ?e, "Execution payload envelope rejected"); + } + Err(EnvelopeError::ImportError(BlockError::BeaconChainError(e))) + } Err(other) => { warn!( reason = other.to_string(), @@ -149,8 +164,8 @@ impl BeaconChain { self: &Arc, slot: Slot, availability: PayloadAvailability, - publish_fn: impl FnOnce() -> Result<(), EnvelopeError>, - ) -> Result { + publish_fn: impl FnOnce() -> Result<(), BlockError>, + ) -> Result { match availability { PayloadAvailability::Available(available_envelope) => { publish_fn()?; @@ -167,11 +182,11 @@ impl BeaconChain { async fn check_envelope_availability_and_import( self: &Arc, envelope: AvailabilityPendingExecutedEnvelope, - ) -> Result { + ) -> Result { let slot = envelope.envelope.slot(); let block_root = envelope.block_root; let bid = load_gloas_payload_bid(block_root, self)? - .ok_or(EnvelopeError::BlockRootUnknown { block_root })?; + .ok_or(BlockError::EnvelopeBlockRootUnknown { block_root })?; let availability = self .pending_payload_cache .put_executed_payload_envelope(bid, envelope)?; @@ -187,7 +202,7 @@ impl BeaconChain { async fn into_executed_payload_envelope( self: Arc, pending_envelope: ExecutionPendingEnvelope, - ) -> Result, EnvelopeError> { + ) -> Result, BlockError> { let ExecutionPendingEnvelope { signed_envelope, block_root, @@ -204,7 +219,7 @@ impl BeaconChain { .payload_verification_status .is_optimistic() { - return Err(EnvelopeError::OptimisticSyncNotSupported { block_root }); + return Err(BlockError::OptimisticSyncNotSupported { block_root }); } Ok(AvailabilityPendingExecutedEnvelope::new( @@ -218,7 +233,7 @@ impl BeaconChain { pub async fn import_available_execution_payload_envelope( self: &Arc, envelope: Box>, - ) -> Result { + ) -> Result { let AvailableExecutedEnvelope { envelope, block_root, @@ -255,13 +270,13 @@ impl BeaconChain { signed_envelope: AvailableEnvelope, block_root: Hash256, payload_verification_status: PayloadVerificationStatus, - ) -> Result { + ) -> Result { // Everything in this initial section is on the hot path for processing the envelope. // Take an upgradable read lock on fork choice so we can check if this block has already // 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(EnvelopeError::BlockRootUnknown { block_root }); + return Err(BlockError::EnvelopeBlockRootUnknown { block_root }); } // TODO(gloas) add defensive check to see if payload envelope is already in fork choice @@ -276,7 +291,7 @@ impl BeaconChain { // node which can be eligible for head. fork_choice .on_valid_payload_envelope_received(block_root) - .map_err(|e| EnvelopeError::InternalError(format!("{e:?}")))?; + .map_err(|e| BlockError::InternalError(format!("{e:?}")))?; // TODO(gloas) emit SSE event if the payload became the new head payload diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index 56f2c2c22c..43dd23f112 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -18,7 +18,7 @@ //! //! ``` -use state_processing::{BlockProcessingError, envelope_processing::EnvelopeProcessingError}; +use state_processing::envelope_processing::EnvelopeProcessingError; use std::sync::Arc; use store::Error as DBError; use strum::AsRefStr; @@ -38,7 +38,6 @@ pub mod gossip_verified_envelope; pub mod import; mod payload_notifier; -use crate::data_availability_checker::AvailabilityCheckError; pub use execution_pending_envelope::ExecutionPendingEnvelope; #[derive(Debug)] @@ -173,25 +172,16 @@ pub enum EnvelopeError { payload_slot: Slot, latest_finalized_slot: Slot, }, - /// Optimistic sync is not supported for Gloas payload envelopes. - OptimisticSyncNotSupported { block_root: Hash256 }, /// Some Beacon Chain Error BeaconChainError(Arc), /// Some Beacon State error BeaconStateError(BeaconStateError), - /// Some BlockProcessingError (for electra operations) - BlockProcessingError(BlockProcessingError), /// Some EnvelopeProcessingError EnvelopeProcessingError(EnvelopeProcessingError), /// Error verifying the execution payload ExecutionPayloadError(ExecutionPayloadError), - /// An error from block-level checks reused during envelope import - BlockError(BlockError), - /// The envelope satisfied all validity conditions except consistency - /// with the corresponding columns that we received over gossip/rpc. - AvailabilityCheck(AvailabilityCheckError), - /// Internal error - InternalError(String), + /// An error from importing the envelope. + ImportError(BlockError), } impl std::fmt::Display for EnvelopeError { @@ -224,18 +214,6 @@ impl From for EnvelopeError { } } -impl From for EnvelopeError { - fn from(e: BlockError) -> Self { - EnvelopeError::BlockError(e) - } -} - -impl From for EnvelopeError { - fn from(e: AvailabilityCheckError) -> Self { - EnvelopeError::AvailabilityCheck(e) - } -} - impl From for EnvelopeError { fn from(e: EnvelopeProcessingError) -> Self { match e { diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs index eb5e13b0cc..0bbe32525a 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/payload_notifier.rs @@ -31,7 +31,8 @@ impl PayloadNotifier { match notify_execution_layer { NotifyExecutionLayer::No if chain.config.optimistic_finalized_sync => { - let new_payload_request = Self::build_new_payload_request(&envelope, &block)?; + let new_payload_request = Self::build_new_payload_request(&envelope, &block) + .map_err(EnvelopeError::ImportError)?; // TODO(gloas): check and test RLP block hash calculation post-Gloas if let Err(e) = new_payload_request.perform_optimistic_sync_verifications() { warn!( diff --git a/beacon_node/beacon_chain/tests/column_verification.rs b/beacon_node/beacon_chain/tests/column_verification.rs index e779135e3f..06a5f44e5f 100644 --- a/beacon_node/beacon_chain/tests/column_verification.rs +++ b/beacon_node/beacon_chain/tests/column_verification.rs @@ -6,8 +6,7 @@ use beacon_chain::test_utils::{ generate_data_column_sidecars_from_block, test_spec, }; use beacon_chain::{ - AvailabilityProcessingStatus, BlockError, BlockOrEnvelopeError, ChainConfig, InvalidSignature, - NotifyExecutionLayer, + AvailabilityProcessingStatus, BlockError, ChainConfig, InvalidSignature, NotifyExecutionLayer, block_verification_types::{AsBlock, LookupBlock}, }; use bls::{Keypair, Signature}; @@ -112,9 +111,7 @@ async fn rpc_columns_with_invalid_header_signature() { .unwrap_err(); assert!(matches!( err, - BlockOrEnvelopeError::BlockError(BlockError::InvalidSignature( - InvalidSignature::ProposerSignature - )) + BlockError::InvalidSignature(InvalidSignature::ProposerSignature) )); } 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 65e1a83840..0eb63d8dfa 100644 --- a/beacon_node/http_api/src/beacon/execution_payload_envelope.rs +++ b/beacon_node/http_api/src/beacon/execution_payload_envelope.rs @@ -7,6 +7,7 @@ use crate::version::{ execution_optimistic_finalized_beacon_response, }; use beacon_chain::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn}; +use beacon_chain::payload_envelope_verification::EnvelopeError; use beacon_chain::{BeaconChain, BeaconChainTypes, NotifyExecutionLayer}; use bytes::Bytes; use eth2::types as api_types; @@ -148,7 +149,7 @@ pub async fn publish_execution_payload_envelope( PubsubMessage::ExecutionPayload(Box::new(envelope_for_gossip)), ) .map_err(|_| { - beacon_chain::payload_envelope_verification::EnvelopeError::BeaconChainError(Arc::new( + EnvelopeError::BeaconChainError(Arc::new( beacon_chain::BeaconChainError::UnableToPublish, )) }) 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 6bc3c03b8b..aec2b1fcb8 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -14,8 +14,8 @@ use beacon_chain::payload_bid_verification::PayloadBidError; use beacon_chain::proposer_preferences_verification::ProposerPreferencesError; use beacon_chain::store::Error; use beacon_chain::{ - AvailabilityProcessingStatus, BeaconChainError, BeaconChainTypes, BlockError, - BlockOrEnvelopeError, ForkChoiceError, GossipVerifiedBlock, NotifyExecutionLayer, + AvailabilityProcessingStatus, BeaconChainError, BeaconChainTypes, BlockError, ForkChoiceError, + GossipVerifiedBlock, NotifyExecutionLayer, attestation_verification::{self, Error as AttnError, VerifiedAttestation}, data_availability_checker::AvailabilityCheckErrorCategory, light_client_finality_update_verification::Error as LightClientFinalityUpdateError, @@ -1407,7 +1407,7 @@ impl NetworkBeaconProcessor { self.check_reconstruction_trigger(slot, &block_root).await; } }, - Err(BlockOrEnvelopeError::BlockError(BlockError::DuplicateFullyImported(_))) => { + Err(BlockError::DuplicateFullyImported(_)) => { debug!( ?block_root, data_column_index, "Ignoring gossip column already imported" @@ -1538,7 +1538,7 @@ impl NetworkBeaconProcessor { self.check_reconstruction_trigger(*slot, block_root).await; } }, - Err(BlockOrEnvelopeError::BlockError(BlockError::DuplicateFullyImported(_))) => { + Err(BlockError::DuplicateFullyImported(_)) => { debug!( ?block_root, data_column_index, "Ignoring completed gossip column already imported" @@ -1846,7 +1846,10 @@ impl NetworkBeaconProcessor { return None; } // BlobNotRequired is unreachable. Only constructed in `process_gossip_blob` - Err(e @ BlockError::InternalError(_)) | Err(e @ BlockError::BlobNotRequired(_)) => { + Err(e @ BlockError::InternalError(_)) + | Err(e @ BlockError::BlobNotRequired(_)) + | Err(e @ BlockError::EnvelopeBlockRootUnknown { .. }) + | Err(e @ BlockError::OptimisticSyncNotSupported { .. }) => { error!(error = %e, "Internal block gossip validation error"); return None; } @@ -3833,8 +3836,7 @@ impl NetworkBeaconProcessor { | EnvelopeError::UnknownValidator { .. } | EnvelopeError::IncorrectBlockProposer { .. } | EnvelopeError::ExecutionPayloadError(_) - | EnvelopeError::EnvelopeProcessingError(_) - | EnvelopeError::BlockError(_) => { + | EnvelopeError::EnvelopeProcessingError(_) => { self.propagate_validation_result( message_id, peer_id, @@ -3914,12 +3916,9 @@ impl NetworkBeaconProcessor { } EnvelopeError::PriorToFinalization { .. } - | EnvelopeError::OptimisticSyncNotSupported { .. } | EnvelopeError::BeaconChainError(_) | EnvelopeError::BeaconStateError(_) - | EnvelopeError::BlockProcessingError(_) - | EnvelopeError::InternalError(_) - | EnvelopeError::AvailabilityCheck(_) => { + | EnvelopeError::ImportError(_) => { self.propagate_validation_result( message_id, peer_id, @@ -4010,7 +4009,8 @@ impl NetworkBeaconProcessor { | EnvelopeError::BlockHashMismatch { .. } | EnvelopeError::UnknownValidator { .. } | EnvelopeError::IncorrectBlockProposer { .. } - | EnvelopeError::ExecutionPayloadError(_) => { + | EnvelopeError::ExecutionPayloadError(_) + | EnvelopeError::EnvelopeProcessingError(_) => { self.gossip_penalize_peer( peer_id, PeerAction::LowToleranceError, @@ -4018,23 +4018,11 @@ impl NetworkBeaconProcessor { ); } - EnvelopeError::EnvelopeProcessingError(_) - | EnvelopeError::BlockError(_) - | EnvelopeError::BlockRootUnknown { .. } => { - self.gossip_penalize_peer( - peer_id, - PeerAction::LowToleranceError, - "gossip_envelope_processing_error", - ); - } - - EnvelopeError::PriorToFinalization { .. } - | EnvelopeError::OptimisticSyncNotSupported { .. } + EnvelopeError::BlockRootUnknown { .. } + | EnvelopeError::PriorToFinalization { .. } | EnvelopeError::BeaconChainError(_) | EnvelopeError::BeaconStateError(_) - | EnvelopeError::BlockProcessingError(_) - | EnvelopeError::InternalError(_) - | EnvelopeError::AvailabilityCheck(_) => {} + | EnvelopeError::ImportError(_) => {} }, } } diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index c1980215d1..817d4bd5ca 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -7,9 +7,7 @@ use beacon_chain::data_column_verification::{GossipDataColumnError, observe_goss use beacon_chain::fetch_blobs::{ EngineGetBlobsOutput, FetchEngineBlobError, fetch_and_process_engine_blobs, }; -use beacon_chain::{ - AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, BlockError, BlockOrEnvelopeError, -}; +use beacon_chain::{AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, BlockError}; use beacon_processor::{ BeaconProcessorSend, DuplicateCache, GossipAggregatePackage, GossipAttestationPackage, Work, WorkEvent as BeaconWorkEvent, @@ -983,10 +981,7 @@ impl NetworkBeaconProcessor { ); } Err(FetchEngineBlobError::BlobProcessingError(e)) - if matches!( - *e, - BlockOrEnvelopeError::BlockError(BlockError::DuplicateFullyImported(..)) - ) => + if matches!(*e, BlockError::DuplicateFullyImported(..)) => { debug!( %block_root, @@ -1055,7 +1050,7 @@ impl NetworkBeaconProcessor { "Reconstruction not required for block" ); } - Err(BlockOrEnvelopeError::BlockError(BlockError::DuplicateFullyImported(_))) => { + Err(BlockError::DuplicateFullyImported(_)) => { debug!("Block already imported in parallel with reconstruction"); } Err(e) => { 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 03be073e7d..61d1b84950 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -11,9 +11,8 @@ use beacon_chain::block_verification_types::{AsBlock, RangeSyncBlock}; use beacon_chain::data_availability_checker::AvailabilityCheckError; use beacon_chain::historical_data_columns::HistoricalDataColumnError; use beacon_chain::{ - AvailabilityProcessingStatus, BeaconChainTypes, BlockError, BlockOrEnvelopeError, - ChainSegmentResult, HistoricalBlockError, NotifyExecutionLayer, - validator_monitor::get_slot_delay_ms, + AvailabilityProcessingStatus, BeaconChainTypes, BlockError, ChainSegmentResult, + HistoricalBlockError, NotifyExecutionLayer, validator_monitor::get_slot_delay_ms, }; use beacon_processor::{ AsyncFn, BlockingFn, DuplicateCache, @@ -346,7 +345,7 @@ impl NetworkBeaconProcessor { // Sync handles these results self.send_sync_message(SyncMessage::BlockComponentProcessed { process_type, - result: result.map_err(Into::into).into(), + result: result.into(), }); } @@ -411,7 +410,7 @@ impl NetworkBeaconProcessor { ); } }, - Err(BlockOrEnvelopeError::BlockError(BlockError::DuplicateFullyImported(_))) => { + Err(BlockError::DuplicateFullyImported(_)) => { debug!( block_hash = %block_root, "Custody columns have already been imported" diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index ab9c7bbe38..3929f74aa0 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -33,9 +33,7 @@ use beacon_chain::block_verification_types::AsBlock; use beacon_chain::data_availability_checker::{ AvailabilityCheckError, AvailabilityCheckErrorCategory, }; -use beacon_chain::{ - AvailabilityProcessingStatus, BeaconChainTypes, BlockError, BlockOrEnvelopeError, -}; +use beacon_chain::{AvailabilityProcessingStatus, BeaconChainTypes, BlockError}; pub use common::RequestState; use fnv::FnvHashMap; use lighthouse_network::service::api_types::SingleLookupReqId; @@ -591,12 +589,8 @@ impl BlockLookups { let action = match result { BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(_)) - | BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError( - BlockError::DuplicateFullyImported(..), - )) - | BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError( - BlockError::GenesisBlock, - )) => { + | BlockProcessingResult::Err(BlockError::DuplicateFullyImported(..)) + | BlockProcessingResult::Err(BlockError::GenesisBlock) => { // Successfully imported request_state.on_processing_success()?; Action::Continue @@ -620,9 +614,7 @@ impl BlockLookups { Action::Retry } } - BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError( - BlockError::DuplicateImportStatusUnknown(..), - )) => { + BlockProcessingResult::Err(BlockError::DuplicateImportStatusUnknown(..)) => { // This is unreachable because RPC blocks do not undergo gossip verification, and // this error can *only* come from gossip verification. error!(?block_root, "Single block lookup hit unreachable condition"); @@ -638,11 +630,6 @@ impl BlockLookups { Action::Drop("Block processing ignored".to_owned()) } BlockProcessingResult::Err(e) => { - let BlockOrEnvelopeError::BlockError(e) = e else { - // TODO(gloas): handle properly - return Err(LookupRequestError::Failed(format!("{e:?}"))); - }; - match e { BlockError::BeaconChainError(e) => { // Internal error diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 1b45ea7052..8eba8300cd 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -50,8 +50,7 @@ use crate::sync::custody_backfill_sync::CustodyBackFillSync; use crate::sync::network_context::{PeerGroup, RpcResponseResult}; use beacon_chain::block_verification_types::AsBlock; use beacon_chain::{ - AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, BlockError, BlockOrEnvelopeError, - EngineState, + AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, BlockError, EngineState, }; use futures::StreamExt; use lighthouse_network::SyncInfo; @@ -207,7 +206,7 @@ impl BlockProcessType { #[derive(Debug)] pub enum BlockProcessingResult { Ok(AvailabilityProcessingStatus), - Err(BlockOrEnvelopeError), + Err(BlockError), Ignored, } @@ -1450,8 +1449,8 @@ impl SyncManager { } } -impl From> for BlockProcessingResult { - fn from(result: Result) -> Self { +impl From> for BlockProcessingResult { + fn from(result: Result) -> Self { match result { Ok(status) => BlockProcessingResult::Ok(status), Err(e) => BlockProcessingResult::Err(e), @@ -1459,14 +1458,8 @@ impl From> for BlockP } } -impl From for BlockProcessingResult { - fn from(e: BlockOrEnvelopeError) -> Self { +impl From for BlockProcessingResult { + fn from(e: BlockError) -> Self { BlockProcessingResult::Err(e) } } - -impl From for BlockProcessingResult { - fn from(e: BlockError) -> Self { - BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError(e)) - } -}