Cleanup

beacon_node/beacon_chain/src/block_verification.rs

@@ -60,6 +60,7 @@ use crate::execution_payload::{
 };
 use crate::kzg_utils::blobs_to_data_column_sidecars;
 use crate::observed_block_producers::SeenBlock;
+use crate::payload_envelope_verification::EnvelopeError;
 use crate::validator_monitor::HISTORIC_EPOCHS as VALIDATOR_MONITOR_HISTORIC_EPOCHS;
 use crate::validator_pubkey_cache::ValidatorPubkeyCache;
 use crate::{
@@ -321,13 +322,18 @@ pub enum BlockError {
         bid_parent_root: Hash256,
         block_parent_root: Hash256,
     },
-    /// The parent block is known but its execution payload envelope has not been received yet.
+    /// The child block is known but its parent execution payload envelope has not been received yet.
     ///
     /// ## Peer scoring
     ///
     /// It's unclear if this block is valid, but it cannot be fully verified without the parent's
     /// execution payload envelope.
     ParentEnvelopeUnknown { parent_root: Hash256 },
+
+    PayloadEnvelopeError {
+        e: Box<EnvelopeError>,
+        penalize_peer: bool,
+    },
 }
 
 /// Which specific signature(s) are invalid in a SignedBeaconBlock
@@ -494,6 +500,35 @@ impl From<ArithError> for BlockError {
     }
 }
 
+impl From<EnvelopeError> for BlockError {
+    fn from(e: EnvelopeError) -> Self {
+        let penalize_peer = match &e {
+            // REJECT per spec: peer sent invalid envelope data
+            EnvelopeError::BadSignature
+            | EnvelopeError::BuilderIndexMismatch { .. }
+            | EnvelopeError::BlockHashMismatch { .. }
+            | EnvelopeError::SlotMismatch { .. }
+            | EnvelopeError::IncorrectBlockProposer { .. } => true,
+            // IGNORE per spec: not the peer's fault
+            EnvelopeError::BlockRootUnknown { .. }
+            | EnvelopeError::PriorToFinalization { .. }
+            | EnvelopeError::UnknownValidator { .. } => false,
+            // Internal errors: not the peer's fault
+            EnvelopeError::BeaconChainError(_)
+            | EnvelopeError::BeaconStateError(_)
+            | EnvelopeError::BlockProcessingError(_)
+            | EnvelopeError::EnvelopeProcessingError(_)
+            | EnvelopeError::ExecutionPayloadError(_)
+            | EnvelopeError::BlockError(_)
+            | EnvelopeError::InternalError(_) => false,
+        };
+        BlockError::PayloadEnvelopeError {
+            e: Box::new(e),
+            penalize_peer,
+        }
+    }
+}
+
 /// Stores information about verifying a payload against an execution engine.
 #[derive(Debug, PartialEq, Clone, Encode, Decode)]
 pub struct PayloadVerificationOutcome {

beacon_node/http_api/src/beacon/execution_payload_envelope.rs

@@ -90,7 +90,6 @@ pub(crate) fn post_beacon_execution_payload_envelope<T: BeaconChainTypes>(
         .boxed()
 }
 /// Publishes a signed execution payload envelope to the network.
-/// TODO(gloas): Add gossip verification (BroadcastValidation::Gossip) before import.
 pub async fn publish_execution_payload_envelope<T: BeaconChainTypes>(
     envelope: SignedExecutionPayloadEnvelope<T::EthSpec>,
     chain: Arc<BeaconChain<T>>,

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -1390,7 +1390,9 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                 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::PayloadEnvelopeError { .. }) => {
                 error!(error = %e, "Internal block gossip validation error");
                 return None;
             }

beacon_node/network/src/network_beacon_processor/sync_methods.rs

@@ -109,9 +109,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                 );
                 self.send_sync_message(SyncMessage::BlockComponentProcessed {
                     process_type,
-                    result: BlockProcessingResult::Err(BlockError::InternalError(format!(
-                        "Envelope verification failed: {e:?}"
-                    ))),
+                    result: BlockProcessingResult::Err(e.into()),
                 });
                 return;
             }
@@ -138,9 +136,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                     ?beacon_block_root,
                     "RPC payload envelope processing failed"
                 );
-                BlockProcessingResult::Err(BlockError::InternalError(format!(
-                    "Envelope processing failed: {e:?}"
-                )))
+                BlockProcessingResult::Err(e.into())
             }
         };
 

beacon_node/network/src/sync/block_lookups/mod.rs

@@ -217,6 +217,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                 block_root,
                 Some(block_component),
                 Some(parent_root),
+                None,
                 // On a `UnknownParentBlock` or `UnknownParentBlob` event the peer is not required
                 // to have the rest of the block components (refer to decoupled blob gossip). Create
                 // the lookup with zero peers to house the block components.
@@ -246,30 +247,20 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
         if envelope_lookup_exists {
             // Create child lookup that waits for the parent envelope (not parent block).
             // The child block itself is available, so we pass it as a component.
-            let child_created = self.new_current_lookup(
+            self.new_current_lookup(
                 block_root,
                 Some(block_component),
                 None, // not awaiting parent block
+                Some(parent_root),
                 &[],
                 cx,
-            );
-            // Set awaiting_parent_envelope on the child lookup
-            if child_created {
-                if let Some((_, lookup)) = self
-                    .single_block_lookups
-                    .iter_mut()
-                    .find(|(_, l)| l.is_for_block(block_root))
-                {
-                    lookup.set_awaiting_parent_envelope(parent_root);
-                }
-            }
-            child_created
+            )
         } else {
             false
         }
     }
 
-    /// Seach a block whose parent root is unknown.
+    /// Search a block whose parent root is unknown.
     ///
     /// Returns true if the lookup is created or already exists
     #[must_use = "only reference the new lookup if returns true"]
@@ -279,7 +270,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
         peer_source: &[PeerId],
         cx: &mut SyncNetworkContext<T>,
     ) -> bool {
-        self.new_current_lookup(block_root, None, None, peer_source, cx)
+        self.new_current_lookup(block_root, None, None, None, peer_source, cx)
     }
 
     /// A block or blob triggers the search of a parent.
@@ -384,7 +375,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
         }
 
         // `block_root_to_search` is a failed chain check happens inside new_current_lookup
-        self.new_current_lookup(block_root_to_search, None, None, peers, cx)
+        self.new_current_lookup(block_root_to_search, None, None, None, peers, cx)
     }
 
     /// A block triggers the search of a parent envelope.
@@ -447,6 +438,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
         block_root: Hash256,
         block_component: Option<BlockComponent<T::EthSpec>>,
         awaiting_parent: Option<Hash256>,
+        awaiting_parent_envelope: Option<Hash256>,
         peers: &[PeerId],
         cx: &mut SyncNetworkContext<T>,
     ) -> bool {
@@ -501,6 +493,9 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
         // If we know that this lookup has unknown parent (is awaiting a parent lookup to resolve),
         // signal here to hold processing downloaded data.
         let mut lookup = SingleBlockLookup::new(block_root, peers, cx.next_id(), awaiting_parent);
+        if let Some(parent_root) = awaiting_parent_envelope {
+            lookup.set_awaiting_parent_envelope(parent_root);
+        }
         let _guard = lookup.span.clone().entered();
 
         // Add block components to the new request
@@ -777,6 +772,26 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                         // We opt to drop the lookup instead.
                         Action::Drop(format!("{e:?}"))
                     }
+                    BlockError::PayloadEnvelopeError { e, penalize_peer } => {
+                        debug!(
+                            ?block_root,
+                            error = ?e,
+                            "Payload envelope processing error"
+                        );
+                        if penalize_peer {
+                            let peer_group = request_state.on_processing_failure()?;
+                            for peer in peer_group.all() {
+                                cx.report_peer(
+                                    *peer,
+                                    PeerAction::MidToleranceError,
+                                    "lookup_envelope_processing_failure",
+                                );
+                            }
+                            Action::Retry
+                        } else {
+                            Action::Drop(format!("{e:?}"))
+                        }
+                    }
                     other => {
                         debug!(
                             ?block_root,