Return RecoveredBlock in ensure_well_formed_payload (#14625)

crates/engine/primitives/src/lib.rs

@@ -21,7 +21,7 @@ use reth_payload_primitives::{
     EngineObjectValidationError, InvalidPayloadAttributesError, NewPayloadError, PayloadAttributes,
     PayloadOrAttributes, PayloadTypes,
 };
-use reth_primitives::{NodePrimitives, SealedBlock};
+use reth_primitives::{NodePrimitives, RecoveredBlock, SealedBlock};
 use reth_primitives_traits::Block;
 use serde::{de::DeserializeOwned, Serialize};
 
@@ -145,7 +145,7 @@ pub trait PayloadValidator: fmt::Debug + Send + Sync + Unpin + 'static {
     fn ensure_well_formed_payload(
         &self,
         payload: Self::ExecutionData,
-    ) -> Result<SealedBlock<Self::Block>, NewPayloadError>;
+    ) -> Result<RecoveredBlock<Self::Block>, NewPayloadError>;
 }
 
 /// Type that validates the payloads processed by the engine.

crates/engine/tree/src/tree/mod.rs

@@ -919,7 +919,7 @@ where
         let status = if self.backfill_sync_state.is_idle() {
             let mut latest_valid_hash = None;
             let num_hash = block.num_hash();
-            match self.insert_block_without_senders(block) {
+            match self.insert_block(block) {
                 Ok(status) => {
                     let status = match status {
                         InsertPayloadOk::Inserted(BlockStatus::Valid) => {
@@ -942,7 +942,7 @@ where
                 }
                 Err(error) => self.on_insert_block_error(error)?,
             }
-        } else if let Err(error) = self.buffer_block_without_senders(block) {
+        } else if let Err(error) = self.buffer_block(block) {
             self.on_insert_block_error(error)?
         } else {
             PayloadStatus::from_status(PayloadStatusEnum::Syncing)
@@ -1991,19 +1991,6 @@ where
         Ok(())
     }
 
-    /// Attempts to recover the block's senders and then buffers it.
-    ///
-    /// Returns an error if sender recovery failed or inserting into the buffer failed.
-    fn buffer_block_without_senders(
-        &mut self,
-        block: SealedBlock<N::Block>,
-    ) -> Result<(), InsertBlockError<N::Block>> {
-        match block.try_recover() {
-            Ok(block) => self.buffer_block(block),
-            Err(err) => Err(InsertBlockError::sender_recovery_error(err.into_inner())),
-        }
-    }
-
     /// Pre-validates the block and inserts it into the buffer.
     fn buffer_block(
         &mut self,
@@ -2350,16 +2337,6 @@ where
         self.most_recent_cache.take_if(|cache| cache.executed_block_hash() == parent_hash)
     }
 
-    fn insert_block_without_senders(
-        &mut self,
-        block: SealedBlock<N::Block>,
-    ) -> Result<InsertPayloadOk, InsertBlockError<N::Block>> {
-        match block.try_recover() {
-            Ok(block) => self.insert_block(block),
-            Err(err) => Err(InsertBlockError::sender_recovery_error(err.into_inner())),
-        }
-    }
-
     fn insert_block(
         &mut self,
         block: RecoveredBlock<N::Block>,
@@ -3833,11 +3810,11 @@ mod tests {
         let s = include_str!("../../test-data/holesky/2.rlp");
         let data = Bytes::from_str(s).unwrap();
         let block = Block::decode(&mut data.as_ref()).unwrap();
-        let sealed = block.seal_slow();
+        let sealed = block.seal_slow().try_recover().unwrap();
 
         let mut test_harness = TestHarness::new(HOLESKY.clone());
 
-        let outcome = test_harness.tree.insert_block_without_senders(sealed.clone()).unwrap();
+        let outcome = test_harness.tree.insert_block(sealed.clone()).unwrap();
         assert_eq!(
             outcome,
             InsertPayloadOk::Inserted(BlockStatus::Disconnected {

crates/ethereum/engine-primitives/src/lib.rs

@@ -26,7 +26,7 @@ use reth_payload_primitives::{
     EngineObjectValidationError, NewPayloadError, PayloadOrAttributes, PayloadTypes,
 };
 use reth_payload_validator::ExecutionPayloadValidator;
-use reth_primitives::{Block, NodePrimitives, SealedBlock};
+use reth_primitives::{Block, NodePrimitives, RecoveredBlock, SealedBlock};
 
 /// The types used in the default mainnet ethereum beacon consensus engine.
 #[derive(Debug, Default, Clone, serde::Deserialize, serde::Serialize)]
@@ -104,8 +104,9 @@ impl PayloadValidator for EthereumEngineValidator {
     fn ensure_well_formed_payload(
         &self,
         payload: ExecutionData,
-    ) -> Result<SealedBlock, NewPayloadError> {
+    ) -> Result<RecoveredBlock<Self::Block>, NewPayloadError> {
-        Ok(self.inner.ensure_well_formed_payload(payload)?)
+        let sealed_block = self.inner.ensure_well_formed_payload(payload)?;
+        sealed_block.try_recover().map_err(|e| NewPayloadError::Other(e.into()))
     }
 }
 

crates/optimism/node/src/engine.rs

@@ -19,7 +19,7 @@ use reth_optimism_forks::{OpHardfork, OpHardforks};
 use reth_optimism_payload_builder::{OpBuiltPayload, OpPayloadBuilderAttributes};
 use reth_optimism_primitives::{OpBlock, OpPrimitives};
 use reth_payload_validator::ExecutionPayloadValidator;
-use reth_primitives::SealedBlock;
+use reth_primitives::{RecoveredBlock, SealedBlock};
 use std::sync::Arc;
 
 /// The types used in the optimism beacon consensus engine.
@@ -97,8 +97,9 @@ impl PayloadValidator for OpEngineValidator {
     fn ensure_well_formed_payload(
         &self,
         payload: ExecutionData,
-    ) -> Result<SealedBlock<Self::Block>, NewPayloadError> {
+    ) -> Result<RecoveredBlock<Self::Block>, NewPayloadError> {
-        Ok(self.inner.ensure_well_formed_payload(payload)?)
+        let sealed_block = self.inner.ensure_well_formed_payload(payload)?;
+        sealed_block.try_recover().map_err(|e| NewPayloadError::Other(e.into()))
     }
 }
 

crates/rpc/rpc/src/validation.rs

@@ -356,17 +356,13 @@ where
         &self,
         request: BuilderBlockValidationRequestV3,
     ) -> Result<(), ValidationApiError> {
-        let block = self
+        let block = self.payload_validator.ensure_well_formed_payload(ExecutionData {
-            .payload_validator
+            payload: ExecutionPayload::V3(request.request.execution_payload),
-            .ensure_well_formed_payload(ExecutionData {
+            sidecar: ExecutionPayloadSidecar::v3(CancunPayloadFields {
-                payload: ExecutionPayload::V3(request.request.execution_payload),
+                parent_beacon_block_root: request.parent_beacon_block_root,
-                sidecar: ExecutionPayloadSidecar::v3(CancunPayloadFields {
+                versioned_hashes: self.validate_blobs_bundle(request.request.blobs_bundle)?,
-                    parent_beacon_block_root: request.parent_beacon_block_root,
+            }),
-                    versioned_hashes: self.validate_blobs_bundle(request.request.blobs_bundle)?,
+        })?;
-                }),
-            })?
-            .try_recover()
-            .map_err(|_| ValidationApiError::InvalidTransactionSignature)?;
 
         self.validate_message_against_block(
             block,
@@ -381,25 +377,20 @@ where
         &self,
         request: BuilderBlockValidationRequestV4,
     ) -> Result<(), ValidationApiError> {
-        let block = self
+        let block = self.payload_validator.ensure_well_formed_payload(ExecutionData {
-            .payload_validator
+            payload: ExecutionPayload::V3(request.request.execution_payload),
-            .ensure_well_formed_payload(ExecutionData {
+            sidecar: ExecutionPayloadSidecar::v4(
-                payload: ExecutionPayload::V3(request.request.execution_payload),
+                CancunPayloadFields {
-                sidecar: ExecutionPayloadSidecar::v4(
+                    parent_beacon_block_root: request.parent_beacon_block_root,
-                    CancunPayloadFields {
+                    versioned_hashes: self.validate_blobs_bundle(request.request.blobs_bundle)?,
-                        parent_beacon_block_root: request.parent_beacon_block_root,
+                },
-                        versioned_hashes: self
+                PraguePayloadFields {
-                            .validate_blobs_bundle(request.request.blobs_bundle)?,
+                    requests: RequestsOrHash::Requests(
-                    },
+                        request.request.execution_requests.to_requests(),
-                    PraguePayloadFields {
+                    ),
-                        requests: RequestsOrHash::Requests(
+                },
-                            request.request.execution_requests.to_requests(),
+            ),
-                        ),
+        })?;
-                    },
-                ),
-            })?
-            .try_recover()
-            .map_err(|_| ValidationApiError::InvalidTransactionSignature)?;
 
         self.validate_message_against_block(
             block,

examples/custom-engine-types/src/main.rs

@@ -38,7 +38,7 @@ use reth::{
     },
     network::NetworkHandle,
     payload::ExecutionPayloadValidator,
-    primitives::{Block, EthPrimitives, SealedBlock, TransactionSigned},
+    primitives::{Block, EthPrimitives, RecoveredBlock, SealedBlock, TransactionSigned},
     providers::{EthStorage, StateProviderFactory},
     rpc::{eth::EthApi, types::engine::ExecutionPayload},
     tasks::TaskManager,
@@ -205,8 +205,9 @@ impl PayloadValidator for CustomEngineValidator {
     fn ensure_well_formed_payload(
         &self,
         payload: ExecutionData,
-    ) -> Result<SealedBlock<Self::Block>, NewPayloadError> {
+    ) -> Result<RecoveredBlock<Self::Block>, NewPayloadError> {
-        Ok(self.inner.ensure_well_formed_payload(payload)?)
+        let sealed_block = self.inner.ensure_well_formed_payload(payload)?;
+        sealed_block.try_recover().map_err(|e| NewPayloadError::Other(e.into()))
     }
 }