Get it working without check_payload_relevency

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -65,7 +65,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::check_envelope_relevancy;
 use crate::pending_payload_cache::PendingPayloadCache;
 use crate::pending_payload_cache::{
     Availability as PayloadAvailability,
@@ -3002,19 +3001,25 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 }
             }
 
-            // If the envelope is relevant we skip the duplicate import check in
+            // The envelope needs import only if it's a Gloas block with an envelope and
-            // `check_block_relevancy`. A verification error or an already-received payload
+            // the envelope isn't already in fork choice.
-            // both leave the envelope irrelevant.
+            let range_sync_envelope_needs_import = matches!(
-            let is_envelope_relevant = block
+                block,
-                .as_envelope()
+                RangeSyncBlock::Gloas {
-                .and_then(|envelope| {
+                    envelope: Some(_),
-                    check_envelope_relevancy(block.as_block(), envelope, self).ok()
+                    ..
-                })
+                }
-                .unwrap_or(false);
+            ) && !self
+                .canonical_head
+                .fork_choice_read_lock()
+                .is_payload_received(&block_root);
 
-            match check_block_relevancy(block.as_block(), block_root, is_envelope_relevant, self) {
+            match check_block_relevancy(block.as_block(), block_root, self) {
                 // If the block is relevant, add it to the filtered chain segment.
                 Ok(_) => filtered_chain_segment.push((block_root, block)),
+                Err(BlockError::DuplicateFullyImported(_)) if range_sync_envelope_needs_import => {
+                    filtered_chain_segment.push((block_root, block));
+                }
                 // If the block is already known, simply ignore this block.
                 //
                 // Note that `check_block_relevancy` is incapable of returning
@@ -3040,7 +3045,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 // However, we will potentially get a `ParentUnknown` on a later block. The sync
                 // protocol will need to ensure this is handled gracefully.
                 Err(BlockError::WouldRevertFinalizedSlot { .. }) => {
-                    if is_envelope_relevant
+                    if range_sync_envelope_needs_import
                         && self
                             .canonical_head
                             .cached_head()

beacon_node/beacon_chain/src/block_verification.rs

@@ -1349,7 +1349,7 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for RangeSyncBlock<T::Eth
     ) -> Result<ExecutionPendingBlock<T>, BlockSlashInfo<BlockError>> {
         // Perform an early check to prevent wasting time on irrelevant blocks.
         let header = self.signed_block_header();
-        let block_root = check_block_relevancy(self.as_block(), block_root, false, chain)
+        let block_root = check_block_relevancy(self.as_block(), block_root, chain)
             .map_err(|e| BlockSlashInfo::SignatureNotChecked(header.clone(), e))?;
 
         let (available_block, _envelope) = self.into_available_block().map_err(|e| {
@@ -1393,7 +1393,7 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for LookupBlock<T::EthSpe
         notify_execution_layer: NotifyExecutionLayer,
     ) -> Result<ExecutionPendingBlock<T>, BlockSlashInfo<BlockError>> {
         // Perform an early check to prevent wasting time on irrelevant blocks.
-        let block_root = check_block_relevancy(self.as_block(), block_root, false, chain)
+        let block_root = check_block_relevancy(self.as_block(), block_root, chain)
             .map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?;
 
         let maybe_available_block = MaybeAvailableBlock::AvailabilityPending {
@@ -1466,7 +1466,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
         /*
          *  Perform cursory checks to see if the block is even worth processing.
          */
-        check_block_relevancy(block.as_block(), block_root, false, chain)?;
+        check_block_relevancy(block.as_block(), block_root, chain)?;
 
         // Define a future that will verify the execution payload with an execution engine.
         //
@@ -1850,7 +1850,6 @@ pub fn check_block_is_finalized_checkpoint_or_descendant<
 pub fn check_block_relevancy<T: BeaconChainTypes>(
     signed_block: &SignedBeaconBlock<T::EthSpec>,
     block_root: Hash256,
-    skip_import_check: bool,
     chain: &BeaconChain<T>,
 ) -> Result<Hash256, BlockError> {
     let block = signed_block.message();
@@ -1880,12 +1879,11 @@ pub fn check_block_relevancy<T: BeaconChainTypes>(
     check_block_against_finalized_slot(block, block_root, chain)?;
 
     // Check if the block is already known. We know it is post-finalization, so it is
-    // sufficient to check the fork choice. This check can optionally be skipped.
+    // sufficient to check the fork choice.
-    if !skip_import_check
+    if chain
-        && chain
+        .canonical_head
-            .canonical_head
+        .fork_choice_read_lock()
-            .fork_choice_read_lock()
+        .contains_block(&block_root)
-            .contains_block(&block_root)
     {
         return Err(BlockError::DuplicateFullyImported(block_root));
     }

beacon_node/beacon_chain/src/block_verification_types.rs

@@ -12,7 +12,7 @@ use std::sync::Arc;
 use types::data::BlobIdentifier;
 use types::{
     BeaconBlockRef, BeaconState, BlindedPayload, ChainSpec, Epoch, EthSpec, Hash256,
-    SignedBeaconBlock, SignedBeaconBlockHeader, SignedExecutionPayloadEnvelope, Slot,
+    SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
 };
 
 /// A wrapper around a `SignedBeaconBlock`. This varaint is constructed
@@ -112,17 +112,6 @@ impl<E: EthSpec> RangeSyncBlock<E> {
         }
     }
 
-    pub fn as_envelope(&self) -> Option<&SignedExecutionPayloadEnvelope<E>> {
-        match self {
-            RangeSyncBlock::Base(_) => None,
-            RangeSyncBlock::Gloas { envelope, .. } => {
-                envelope.as_ref().map(|e| e.envelope().as_ref())
-            }
-        }
-    }
-}
-
-impl<E: EthSpec> RangeSyncBlock<E> {
     /// Constructs a `RangeSyncBlock` from a block and availability data (pre-Gloas).
     pub fn new<T>(
         block: Arc<SignedBeaconBlock<E>>,

beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs

@@ -25,13 +25,12 @@ use strum::AsRefStr;
 use tracing::instrument;
 use types::{
     BeaconState, BeaconStateError, DataColumnSidecarList, EthSpec, ExecutionBlockHash,
-    ExecutionPayloadEnvelope, Hash256, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot,
+    ExecutionPayloadEnvelope, Hash256, SignedExecutionPayloadEnvelope, Slot,
 };
 
 use crate::{
-    BeaconChain, BeaconChainError, BeaconChainTypes, BeaconStore, BlockError,
+    BeaconChainError, BeaconChainTypes, BeaconStore, BlockError, ExecutionPayloadError,
-    ExecutionPayloadError, PayloadVerificationError, PayloadVerificationOutcome,
+    PayloadVerificationError, PayloadVerificationOutcome,
-    payload_envelope_verification::gossip_verified_envelope::verify_envelope_consistency,
 };
 
 pub mod execution_pending_envelope;
@@ -293,41 +292,3 @@ pub(crate) fn load_snapshot_from_state_root<T: BeaconChainTypes>(
         beacon_block_root,
     })
 }
-
-/// Performs simple, cheap checks to ensure that the envelope is relevant to be imported.
-///
-/// Returns `Ok(true)` if the envelope passes these checks and should progress with verification,
-/// or `Ok(false)` if its payload has already been received and is no longer relevant.
-///
-/// Returns an error if a verification step fails.
-pub fn check_envelope_relevancy<T: BeaconChainTypes>(
-    block: &SignedBeaconBlock<T::EthSpec>,
-    signed_envelope: &SignedExecutionPayloadEnvelope<T::EthSpec>,
-    chain: &BeaconChain<T>,
-) -> Result<bool, EnvelopeError> {
-    let envelope = &signed_envelope.message;
-    let Ok(bid) = block.message().body().signed_execution_payload_bid() else {
-        return Err(EnvelopeError::InternalError(
-            "Block is pre-gloas".to_string(),
-        ));
-    };
-
-    let latest_finalized_slot = chain
-        .canonical_head
-        .cached_head()
-        .finalized_checkpoint()
-        .epoch
-        .start_slot(T::EthSpec::slots_per_epoch());
-
-    verify_envelope_consistency(envelope, block, &bid.message, latest_finalized_slot)?;
-
-    if chain
-        .canonical_head
-        .fork_choice_read_lock()
-        .is_payload_received(&envelope.beacon_block_root)
-    {
-        return Ok(false);
-    }
-
-    Ok(true)
-}