From 86e8a2a245490a1503c3c05c246f63f3d48e0175 Mon Sep 17 00:00:00 2001 From: Joseph Zhao <65984904+programskillforverification@users.noreply.github.com> Date: Fri, 21 Feb 2025 18:42:54 +0800 Subject: [PATCH] Return RecoveredBlock in ensure_well_formed_payload (#14625) --- crates/engine/primitives/src/lib.rs | 4 +- crates/engine/tree/src/tree/mod.rs | 31 ++---------- crates/ethereum/engine-primitives/src/lib.rs | 7 +-- crates/optimism/node/src/engine.rs | 7 +-- crates/rpc/rpc/src/validation.rs | 51 ++++++++------------ examples/custom-engine-types/src/main.rs | 7 +-- 6 files changed, 39 insertions(+), 68 deletions(-) diff --git a/crates/engine/primitives/src/lib.rs b/crates/engine/primitives/src/lib.rs index ed67a491c..e8d07df75 100644 --- a/crates/engine/primitives/src/lib.rs +++ b/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, NewPayloadError>; + ) -> Result, NewPayloadError>; } /// Type that validates the payloads processed by the engine. diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index ca36a69a9..0155bc852 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/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, - ) -> Result<(), InsertBlockError> { - 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, - ) -> Result> { - 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, @@ -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 { diff --git a/crates/ethereum/engine-primitives/src/lib.rs b/crates/ethereum/engine-primitives/src/lib.rs index a8eec97bd..644da880a 100644 --- a/crates/ethereum/engine-primitives/src/lib.rs +++ b/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 { - Ok(self.inner.ensure_well_formed_payload(payload)?) + ) -> Result, NewPayloadError> { + let sealed_block = self.inner.ensure_well_formed_payload(payload)?; + sealed_block.try_recover().map_err(|e| NewPayloadError::Other(e.into())) } } diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index 9fb9fc3c5..4b12008cd 100644 --- a/crates/optimism/node/src/engine.rs +++ b/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, NewPayloadError> { - Ok(self.inner.ensure_well_formed_payload(payload)?) + ) -> Result, NewPayloadError> { + let sealed_block = self.inner.ensure_well_formed_payload(payload)?; + sealed_block.try_recover().map_err(|e| NewPayloadError::Other(e.into())) } } diff --git a/crates/rpc/rpc/src/validation.rs b/crates/rpc/rpc/src/validation.rs index d1791984d..76d9ade3a 100644 --- a/crates/rpc/rpc/src/validation.rs +++ b/crates/rpc/rpc/src/validation.rs @@ -356,17 +356,13 @@ where &self, request: BuilderBlockValidationRequestV3, ) -> Result<(), ValidationApiError> { - let block = self - .payload_validator - .ensure_well_formed_payload(ExecutionData { - payload: ExecutionPayload::V3(request.request.execution_payload), - sidecar: ExecutionPayloadSidecar::v3(CancunPayloadFields { - 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)?; + let block = self.payload_validator.ensure_well_formed_payload(ExecutionData { + payload: ExecutionPayload::V3(request.request.execution_payload), + sidecar: ExecutionPayloadSidecar::v3(CancunPayloadFields { + parent_beacon_block_root: request.parent_beacon_block_root, + versioned_hashes: self.validate_blobs_bundle(request.request.blobs_bundle)?, + }), + })?; self.validate_message_against_block( block, @@ -381,25 +377,20 @@ where &self, request: BuilderBlockValidationRequestV4, ) -> Result<(), ValidationApiError> { - let block = self - .payload_validator - .ensure_well_formed_payload(ExecutionData { - payload: ExecutionPayload::V3(request.request.execution_payload), - sidecar: ExecutionPayloadSidecar::v4( - CancunPayloadFields { - parent_beacon_block_root: request.parent_beacon_block_root, - versioned_hashes: self - .validate_blobs_bundle(request.request.blobs_bundle)?, - }, - PraguePayloadFields { - requests: RequestsOrHash::Requests( - request.request.execution_requests.to_requests(), - ), - }, - ), - })? - .try_recover() - .map_err(|_| ValidationApiError::InvalidTransactionSignature)?; + let block = self.payload_validator.ensure_well_formed_payload(ExecutionData { + payload: ExecutionPayload::V3(request.request.execution_payload), + sidecar: ExecutionPayloadSidecar::v4( + CancunPayloadFields { + parent_beacon_block_root: request.parent_beacon_block_root, + versioned_hashes: self.validate_blobs_bundle(request.request.blobs_bundle)?, + }, + PraguePayloadFields { + requests: RequestsOrHash::Requests( + request.request.execution_requests.to_requests(), + ), + }, + ), + })?; self.validate_message_against_block( block, diff --git a/examples/custom-engine-types/src/main.rs b/examples/custom-engine-types/src/main.rs index 50bc60f80..efaa8de20 100644 --- a/examples/custom-engine-types/src/main.rs +++ b/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, NewPayloadError> { - Ok(self.inner.ensure_well_formed_payload(payload)?) + ) -> Result, NewPayloadError> { + let sealed_block = self.inner.ensure_well_formed_payload(payload)?; + sealed_block.try_recover().map_err(|e| NewPayloadError::Other(e.into())) } }