diff --git a/crates/consensus/beacon/src/engine/error.rs b/crates/consensus/beacon/src/engine/error.rs index 29db2c143d..d5f19b5363 100644 --- a/crates/consensus/beacon/src/engine/error.rs +++ b/crates/consensus/beacon/src/engine/error.rs @@ -80,6 +80,9 @@ pub enum BeaconOnNewPayloadError { /// Thrown when the engine task is unavailable/stopped. #[error("beacon consensus engine task stopped")] EngineUnavailable, + /// Thrown when a block has blob transactions, but is not after the Cancun fork. + #[error("block has blob transactions, but is not after the Cancun fork")] + PreCancunBlockWithBlobTransactions, /// An internal error occurred, not necessarily related to the payload. #[error(transparent)] Internal(Box), diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index b2df319b30..013fe3aac4 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -23,7 +23,7 @@ use reth_interfaces::{ use reth_payload_builder::{PayloadBuilderAttributes, PayloadBuilderHandle}; use reth_primitives::{ constants::EPOCH_SLOTS, listener::EventListeners, stage::StageId, BlockNumHash, BlockNumber, - Head, Header, SealedBlock, SealedHeader, H256, U256, + ChainSpec, Head, Header, SealedBlock, SealedHeader, H256, U256, }; use reth_provider::{ BlockIdReader, BlockReader, BlockSource, CanonChainTracker, ChainSpecProvider, ProviderError, @@ -33,7 +33,7 @@ use reth_rpc_types::engine::{ CancunPayloadFields, ExecutionPayload, PayloadAttributes, PayloadError, PayloadStatus, PayloadStatusEnum, PayloadValidationError, }; -use reth_rpc_types_compat::engine::payload::try_into_sealed_block; +use reth_rpc_types_compat::payload::{try_into_block, validate_block_hash}; use reth_stages::{ControlFlow, Pipeline, PipelineError}; use reth_tasks::TaskSpawner; use std::{ @@ -1079,7 +1079,7 @@ where payload: ExecutionPayload, cancun_fields: Option, ) -> Result { - let block = match self.ensure_well_formed_payload(payload, cancun_fields) { + let block = match self.ensure_well_formed_payload(payload, cancun_fields)? { Ok(block) => block, Err(status) => return Ok(status), }; @@ -1143,16 +1143,35 @@ where /// - incorrect hash /// - the versioned hashes passed with the payload do not exactly match transaction /// versioned hashes + /// - the block does not contain blob transactions if it is pre-cancun fn ensure_well_formed_payload( &self, payload: ExecutionPayload, cancun_fields: Option, - ) -> Result { + ) -> Result, BeaconOnNewPayloadError> { let parent_hash = payload.parent_hash(); - let block = match try_into_sealed_block( + + let block_hash = payload.block_hash(); + let block_res = match try_into_block( payload, cancun_fields.as_ref().map(|fields| fields.parent_beacon_block_root), ) { + Ok(block) => { + // make sure there are no blob transactions in the payload if it is pre-cancun + // we perform this check before validating the block hash because INVALID_PARAMS + // must be returned over an INVALID response. + if !self.chain_spec().is_cancun_active_at_timestamp(block.timestamp) && + block.has_blob_transactions() + { + return Err(BeaconOnNewPayloadError::PreCancunBlockWithBlobTransactions) + } + + validate_block_hash(block_hash, block) + } + Err(error) => Err(error), + }; + + let block = match block_res { Ok(block) => block, Err(error) => { error!(target: "consensus::engine", ?error, "Invalid payload"); @@ -1166,7 +1185,7 @@ where } let status = PayloadStatusEnum::from(error); - return Err(PayloadStatus::new(status, latest_valid_hash)) + return Ok(Err(PayloadStatus::new(status, latest_valid_hash))) } }; @@ -1177,9 +1196,18 @@ where .flatten() .collect::>(); - self.validate_versioned_hashes(parent_hash, block_versioned_hashes, cancun_fields)?; + if let Err(status) = + self.validate_versioned_hashes(parent_hash, block_versioned_hashes, cancun_fields) + { + return Ok(Err(status)) + } - Ok(block) + Ok(Ok(block)) + } + + /// Returns the currently configured [ChainSpec]. + fn chain_spec(&self) -> Arc { + self.blockchain.chain_spec() } /// Validates that the versioned hashes in the block match the versioned hashes passed in the diff --git a/crates/primitives/src/block.rs b/crates/primitives/src/block.rs index 199b088d06..c9be7194ad 100644 --- a/crates/primitives/src/block.rs +++ b/crates/primitives/src/block.rs @@ -60,6 +60,11 @@ impl Block { BlockWithSenders { block: self, senders } } + /// Returns whether or not the block contains any blob transactions. + pub fn has_blob_transactions(&self) -> bool { + self.body.iter().any(|tx| tx.is_eip4844()) + } + /// Calculates a heuristic for the in-memory size of the [Block]. #[inline] pub fn size(&self) -> usize { diff --git a/crates/rpc/rpc-engine-api/src/error.rs b/crates/rpc/rpc-engine-api/src/error.rs index a5bf6b111d..d42add66d2 100644 --- a/crates/rpc/rpc-engine-api/src/error.rs +++ b/crates/rpc/rpc-engine-api/src/error.rs @@ -112,6 +112,7 @@ impl From for jsonrpsee_types::error::ErrorObject<'static> { }, EngineApiError::NewPayload(ref err) => match err { BeaconOnNewPayloadError::Internal(_) | + BeaconOnNewPayloadError::PreCancunBlockWithBlobTransactions => INVALID_PARAMS_CODE, BeaconOnNewPayloadError::EngineUnavailable => INTERNAL_ERROR_CODE, }, // Any other server error diff --git a/crates/rpc/rpc-types-compat/src/engine/payload.rs b/crates/rpc/rpc-types-compat/src/engine/payload.rs index 4158e99901..e47f6ea290 100644 --- a/crates/rpc/rpc-types-compat/src/engine/payload.rs +++ b/crates/rpc/rpc-types-compat/src/engine/payload.rs @@ -257,19 +257,17 @@ pub fn convert_block_to_payload_input_v2(value: SealedBlock) -> ExecutionPayload } } -/// Tries to create a new block from the given payload and optional parent beacon block root. -/// Perform additional validation of `extra_data` and `base_fee_per_gas` fields. +/// Tries to create a new block (without a block hash) from the given payload and optional parent +/// beacon block root. +/// Performs additional validation of `extra_data` and `base_fee_per_gas` fields. /// /// NOTE: The log bloom is assumed to be validated during serialization. -/// NOTE: Empty ommers, nonce and difficulty values are validated upon computing block hash and -/// comparing the value with `payload.block_hash`. /// /// See -pub fn try_into_sealed_block( +pub fn try_into_block( value: ExecutionPayload, parent_beacon_block_root: Option, -) -> Result { - let block_hash = value.block_hash(); +) -> Result { let mut base_payload = match value { ExecutionPayload::V1(payload) => try_payload_v1_to_block(payload)?, ExecutionPayload::V2(payload) => try_payload_v2_to_block(payload)?, @@ -278,12 +276,48 @@ pub fn try_into_sealed_block( base_payload.header.parent_beacon_block_root = parent_beacon_block_root; - let payload = base_payload.seal_slow(); + Ok(base_payload) +} - if block_hash != payload.hash() { - return Err(PayloadError::BlockHash { execution: payload.hash(), consensus: block_hash }) +/// Tries to create a new block from the given payload and optional parent beacon block root. +/// +/// NOTE: Empty ommers, nonce and difficulty values are validated upon computing block hash and +/// comparing the value with `payload.block_hash`. +/// +/// Uses [try_into_block] to convert from the [ExecutionPayload] to [Block] and seals the block +/// with its hash. +/// +/// Uses [validate_block_hash] to validate the payload block hash and ultimately return the +/// [SealedBlock]. +pub fn try_into_sealed_block( + payload: ExecutionPayload, + parent_beacon_block_root: Option, +) -> Result { + let block_hash = payload.block_hash(); + let base_payload = try_into_block(payload, parent_beacon_block_root)?; + + // validate block hash and return + validate_block_hash(block_hash, base_payload) +} + +/// Takes the expected block hash and [Block], validating the block and converting it into a +/// [SealedBlock]. +/// +/// If the provided block hash does not match the block hash computed from the provided block, this +/// returns [PayloadError::BlockHash]. +pub fn validate_block_hash( + expected_block_hash: H256, + block: Block, +) -> Result { + let sealed_block = block.seal_slow(); + if expected_block_hash != sealed_block.hash() { + return Err(PayloadError::BlockHash { + execution: sealed_block.hash(), + consensus: expected_block_hash, + }) } - Ok(payload) + + Ok(sealed_block) } /// Converts [Withdrawal] to [reth_rpc_types::engine::payload::Withdrawal]