From 61c9587a24f8b253196be21ef75ebfcb9a364945 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 18 Dec 2023 12:45:24 +0100 Subject: [PATCH] feat: add execution payload validator (#5811) --- Cargo.lock | 11 ++ Cargo.toml | 2 + bin/reth/Cargo.toml | 1 + bin/reth/src/lib.rs | 6 + crates/consensus/beacon/Cargo.toml | 2 + crates/consensus/beacon/src/engine/mod.rs | 148 +++++------------- crates/payload/validator/Cargo.toml | 15 ++ crates/payload/validator/src/lib.rs | 130 +++++++++++++++ crates/primitives/src/block.rs | 24 ++- .../rpc-types-compat/src/engine/payload.rs | 1 + crates/rpc/rpc-types/src/eth/engine/cancun.rs | 42 +++++ .../rpc/rpc-types/src/eth/engine/payload.rs | 9 +- 12 files changed, 279 insertions(+), 112 deletions(-) create mode 100644 crates/payload/validator/Cargo.toml create mode 100644 crates/payload/validator/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 63d66a491..8e0af142e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5610,6 +5610,7 @@ dependencies = [ "reth-network-api", "reth-nippy-jar", "reth-payload-builder", + "reth-payload-validator", "reth-primitives", "reth-provider", "reth-prune", @@ -5699,6 +5700,7 @@ dependencies = [ "reth-interfaces", "reth-metrics", "reth-payload-builder", + "reth-payload-validator", "reth-primitives", "reth-provider", "reth-prune", @@ -6212,6 +6214,15 @@ dependencies = [ "tracing", ] +[[package]] +name = "reth-payload-validator" +version = "0.1.0-alpha.13" +dependencies = [ + "reth-primitives", + "reth-rpc-types", + "reth-rpc-types-compat", +] + [[package]] name = "reth-primitives" version = "0.1.0-alpha.13" diff --git a/Cargo.toml b/Cargo.toml index cc048a6df..2d18c023d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ members = [ "crates/net/network-api/", "crates/payload/basic/", "crates/payload/builder/", + "crates/payload/validator/", "crates/primitives/", "crates/prune/", "crates/revm/", @@ -116,6 +117,7 @@ reth-network = { path = "crates/net/network" } reth-network-api = { path = "crates/net/network-api" } reth-nippy-jar = { path = "crates/storage/nippy-jar" } reth-payload-builder = { path = "crates/payload/builder" } +reth-payload-validator = { path = "crates/payload/validator" } reth-primitives = { path = "crates/primitives" } reth-provider = { path = "crates/storage/provider" } reth-prune = { path = "crates/prune" } diff --git a/bin/reth/Cargo.toml b/bin/reth/Cargo.toml index 1619c0cf8..4f2a2f1b3 100644 --- a/bin/reth/Cargo.toml +++ b/bin/reth/Cargo.toml @@ -43,6 +43,7 @@ reth-tracing.workspace = true reth-tasks.workspace = true reth-net-nat.workspace = true reth-payload-builder.workspace = true +reth-payload-validator.workspace = true reth-basic-payload-builder.workspace = true reth-discv4.workspace = true reth-prune.workspace = true diff --git a/bin/reth/src/lib.rs b/bin/reth/src/lib.rs index efb70dc36..973f988c0 100644 --- a/bin/reth/src/lib.rs +++ b/bin/reth/src/lib.rs @@ -46,6 +46,12 @@ pub mod test_vectors; pub mod utils; pub mod version; +/// Re-exported payload related types +pub mod payload { + pub use reth_payload_builder::*; + pub use reth_payload_validator::ExecutionPayloadValidator; +} + /// Re-exported from `reth_provider`. pub mod providers { pub use reth_provider::*; diff --git a/crates/consensus/beacon/Cargo.toml b/crates/consensus/beacon/Cargo.toml index 3f892778b..6986fd63a 100644 --- a/crates/consensus/beacon/Cargo.toml +++ b/crates/consensus/beacon/Cargo.toml @@ -18,10 +18,12 @@ reth-provider.workspace = true reth-rpc-types.workspace = true reth-tasks.workspace = true reth-payload-builder.workspace = true +reth-payload-validator.workspace = true reth-prune.workspace = true reth-snapshot.workspace = true reth-rpc-types-compat.workspace = true reth-tokio-util.workspace = true + # async tokio = { workspace = true, features = ["sync"] } tokio-stream.workspace = true diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index b9d9035ca..ccbdd1131 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -22,18 +22,18 @@ use reth_interfaces::{ }; use reth_payload_builder::{PayloadBuilderAttributes, PayloadBuilderHandle}; use reth_primitives::{ - constants::EPOCH_SLOTS, stage::StageId, BlockNumHash, BlockNumber, ChainSpec, Head, Header, - SealedBlock, SealedHeader, B256, U256, + constants::EPOCH_SLOTS, stage::StageId, BlockNumHash, BlockNumber, Head, Header, SealedBlock, + SealedHeader, B256, U256, }; use reth_provider::{ BlockIdReader, BlockReader, BlockSource, CanonChainTracker, ChainSpecProvider, ProviderError, StageCheckpointReader, }; use reth_rpc_types::engine::{ - CancunPayloadFields, ExecutionPayload, PayloadAttributes, PayloadError, PayloadStatus, - PayloadStatusEnum, PayloadValidationError, + CancunPayloadFields, ExecutionPayload, PayloadAttributes, PayloadStatus, PayloadStatusEnum, + PayloadValidationError, }; -use reth_rpc_types_compat::engine::payload::{try_into_block, validate_block_hash}; + use reth_stages::{ControlFlow, Pipeline, PipelineError}; use reth_tasks::TaskSpawner; use reth_tokio_util::EventListeners; @@ -73,6 +73,7 @@ mod forkchoice; use crate::hooks::{EngineHookEvent, EngineHooks, PolledHook}; pub use forkchoice::ForkchoiceStatus; use reth_interfaces::blockchain_tree::BlockValidationKind; +use reth_payload_validator::ExecutionPayloadValidator; mod metrics; @@ -187,6 +188,8 @@ where forkchoice_state_tracker: ForkchoiceStateTracker, /// The payload store. payload_builder: PayloadBuilderHandle, + /// Validator for execution payloads + payload_validator: ExecutionPayloadValidator, /// Listeners for engine events. listeners: EventListeners, /// Tracks the header of invalid payloads that were rejected by the engine because they're @@ -293,6 +296,7 @@ where ); let mut this = Self { sync, + payload_validator: ExecutionPayloadValidator::new(blockchain.chain_spec()), blockchain, sync_state_updater, engine_message_rx: UnboundedReceiverStream::new(rx), @@ -669,7 +673,7 @@ where // On Optimism, the proposers are allowed to reorg their own chain at will. cfg_if::cfg_if! { if #[cfg(feature = "optimism")] { - if self.chain_spec().is_optimism() { + if self.blockchain.chain_spec().is_optimism() { debug!( target: "consensus::engine", fcu_head_num=?header.number, @@ -1176,6 +1180,21 @@ where /// - 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 + // This validates the following engine API rule: + // + // 3. Given the expected array of blob versioned hashes client software **MUST** run its + // validation by taking the following steps: + // + // 1. Obtain the actual array by concatenating blob versioned hashes lists + // (`tx.blob_versioned_hashes`) of each [blob + // transaction](https://eips.ethereum.org/EIPS/eip-4844#new-transaction-type) included + // in the payload, respecting the order of inclusion. If the payload has no blob + // transactions the expected array **MUST** be `[]`. + // + // 2. Return `{status: INVALID, latestValidHash: null, validationError: errorMessage | null}` + // if the expected and the actual arrays don't match. + // + // This validation **MUST** be instantly run in all cases even during active sync process. fn ensure_well_formed_payload( &self, payload: ExecutionPayload, @@ -1183,117 +1202,26 @@ where ) -> Result { let parent_hash = payload.parent_hash(); - 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 - if !self.chain_spec().is_cancun_active_at_timestamp(block.timestamp) && - block.has_blob_transactions() - { - Err(PayloadError::PreCancunBlockWithBlobTransactions) - } else { - validate_block_hash(block_hash, block) - } - } - Err(error) => Err(error), - }; - - let block = match block_res { - Ok(block) => block, + match self.payload_validator.ensure_well_formed_payload(payload, cancun_fields.into()) { + Ok(block) => Ok(block), Err(error) => { error!(target: "consensus::engine", ?error, "Invalid payload"); + // we need to convert the error to a payload status (response to the CL) - let mut latest_valid_hash = None; - if !error.is_block_hash_mismatch() { - // Engine-API rule: - // > `latestValidHash: null` if the blockHash validation has failed - latest_valid_hash = - self.latest_valid_hash_for_invalid_payload(parent_hash, None); - } - let status = PayloadStatusEnum::from(error); - - return Err(PayloadStatus::new(status, latest_valid_hash)); - } - }; - - let block_versioned_hashes = block - .blob_transactions() - .iter() - .filter_map(|tx| tx.as_eip4844().map(|blob_tx| &blob_tx.blob_versioned_hashes)) - .flatten() - .collect::>(); - - self.validate_versioned_hashes(parent_hash, block_versioned_hashes, cancun_fields)?; - - 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 - /// [CancunPayloadFields], if the cancun payload fields are provided. If the payload fields are - /// not provided, but versioned hashes exist in the block, this returns a [PayloadStatus] with - /// the [PayloadError::InvalidVersionedHashes] error. - /// - /// This validates versioned hashes according to the Engine API Cancun spec: - /// - fn validate_versioned_hashes( - &self, - parent_hash: B256, - block_versioned_hashes: Vec<&B256>, - cancun_fields: Option, - ) -> Result<(), PayloadStatus> { - // This validates the following engine API rule: - // - // 3. Given the expected array of blob versioned hashes client software **MUST** run its - // validation by taking the following steps: - // - // 1. Obtain the actual array by concatenating blob versioned hashes lists - // (`tx.blob_versioned_hashes`) of each [blob - // transaction](https://eips.ethereum.org/EIPS/eip-4844#new-transaction-type) included - // in the payload, respecting the order of inclusion. If the payload has no blob - // transactions the expected array **MUST** be `[]`. - // - // 2. Return `{status: INVALID, latestValidHash: null, validationError: errorMessage | - // null}` if the expected and the actual arrays don't match. - // - // This validation **MUST** be instantly run in all cases even during active sync process. - if let Some(fields) = cancun_fields { - if block_versioned_hashes.len() != fields.versioned_hashes.len() { - // if the lengths don't match then we know that the payload is invalid let latest_valid_hash = - self.latest_valid_hash_for_invalid_payload(parent_hash, None); - let status = PayloadStatusEnum::from(PayloadError::InvalidVersionedHashes); - return Err(PayloadStatus::new(status, latest_valid_hash)); - } + if error.is_block_hash_mismatch() || error.is_invalid_versioned_hashes() { + // Engine-API rules: + // > `latestValidHash: null` if the blockHash validation has failed () + // > `latestValidHash: null` if the expected and the actual arrays don't match () + None + } else { + self.latest_valid_hash_for_invalid_payload(parent_hash, None) + }; - // we can use `zip` safely here because we already compared their length - let zipped_versioned_hashes = - fields.versioned_hashes.iter().zip(block_versioned_hashes); - for (payload_versioned_hash, block_versioned_hash) in zipped_versioned_hashes { - if payload_versioned_hash != block_versioned_hash { - // One of the hashes does not match - return invalid - let latest_valid_hash = - self.latest_valid_hash_for_invalid_payload(parent_hash, None); - let status = PayloadStatusEnum::from(PayloadError::InvalidVersionedHashes); - return Err(PayloadStatus::new(status, latest_valid_hash)); - } + let status = PayloadStatusEnum::from(error); + Err(PayloadStatus::new(status, latest_valid_hash)) } - } else if !block_versioned_hashes.is_empty() { - // there are versioned hashes in the block but no expected versioned hashes were - // provided in the new payload call, so the payload is invalid - let latest_valid_hash = self.latest_valid_hash_for_invalid_payload(parent_hash, None); - let status = PayloadStatusEnum::from(PayloadError::InvalidVersionedHashes); - return Err(PayloadStatus::new(status, latest_valid_hash)); } - - Ok(()) } /// When the pipeline is active, the tree is unable to commit any additional blocks since the diff --git a/crates/payload/validator/Cargo.toml b/crates/payload/validator/Cargo.toml new file mode 100644 index 000000000..f3003d62e --- /dev/null +++ b/crates/payload/validator/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "reth-payload-validator" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +homepage.workspace = true +repository.workspace = true +description = "Payload validation support" + +[dependencies] +# reth +reth-primitives.workspace = true +reth-rpc-types.workspace = true +reth-rpc-types-compat.workspace = true \ No newline at end of file diff --git a/crates/payload/validator/src/lib.rs b/crates/payload/validator/src/lib.rs new file mode 100644 index 000000000..c4c7d1379 --- /dev/null +++ b/crates/payload/validator/src/lib.rs @@ -0,0 +1,130 @@ +//! Payload Validation support. + +#![doc( + html_logo_url = "https://raw.githubusercontent.com/paradigmxyz/reth/main/assets/reth-docs.png", + html_favicon_url = "https://avatars0.githubusercontent.com/u/97369466?s=256", + issue_tracker_base_url = "https://github.com/paradigmxyz/reth/issues/" +)] +#![warn( + missing_debug_implementations, + missing_docs, + unreachable_pub, + unused_crate_dependencies, + rustdoc::all +)] +#![deny(unused_must_use, rust_2018_idioms)] +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] + +use reth_primitives::{ChainSpec, SealedBlock}; +use reth_rpc_types::{engine::MaybeCancunPayloadFields, ExecutionPayload, PayloadError}; +use reth_rpc_types_compat::engine::payload::{try_into_block, validate_block_hash}; +use std::sync::Arc; + +/// Execution payload validator. +#[derive(Clone, Debug)] +pub struct ExecutionPayloadValidator { + /// Chain spec to validate against. + chain_spec: Arc, +} + +impl ExecutionPayloadValidator { + /// Create a new validator. + pub fn new(chain_spec: Arc) -> Self { + Self { chain_spec } + } + + /// Returns the chain spec used by the validator. + #[inline] + pub fn chain_spec(&self) -> &ChainSpec { + &self.chain_spec + } + + /// Returns true if the Cancun hardfork is active at the given timestamp. + #[inline] + fn is_cancun_active_at_timestamp(&self, timestamp: u64) -> bool { + self.chain_spec().is_cancun_active_at_timestamp(timestamp) + } + + /// Cancun specific checks for EIP-4844 blob transactions. + /// + /// Ensures that the number of blob versioned hashes matches the number hashes included in the + /// _separate_ block_versioned_hashes of the cancun payload fields. + fn ensure_matching_blob_versioned_hashes( + &self, + sealed_block: &SealedBlock, + cancun_fields: &MaybeCancunPayloadFields, + ) -> Result<(), PayloadError> { + let num_blob_versioned_hashes = sealed_block.blob_versioned_hashes_iter().count(); + // Additional Cancun checks for blob transactions + if let Some(versioned_hashes) = cancun_fields.versioned_hashes() { + if num_blob_versioned_hashes != versioned_hashes.len() { + // Number of blob versioned hashes does not match + return Err(PayloadError::InvalidVersionedHashes) + } + // we can use `zip` safely here because we already compared their length + for (payload_versioned_hash, block_versioned_hash) in + versioned_hashes.iter().zip(sealed_block.blob_versioned_hashes_iter()) + { + if payload_versioned_hash != block_versioned_hash { + return Err(PayloadError::InvalidVersionedHashes) + } + } + } else { + // No Cancun fields, if block includes any blobs, this is an error + if num_blob_versioned_hashes > 0 { + return Err(PayloadError::InvalidVersionedHashes) + } + } + + Ok(()) + } + + /// Ensures that the given payload does not violate any consensus rules that concern the block's + /// layout, like: + /// - missing or invalid base fee + /// - invalid extra data + /// - invalid transactions + /// - 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 + /// + /// The checks are done in the order that conforms with the engine-API specification. + /// + /// This is intended to be invoked after receiving the payload from the CLI. + /// The additional [MaybeCancunPayloadFields] are not part of the payload, but are additional fields in the `engine_newPayloadV3` RPC call, See also + /// + /// If the cancun fields are provided this also validates that the versioned hashes in the block + /// match the versioned hashes passed in the + /// [CancunPayloadFields](reth_rpc_types::engine::CancunPayloadFields), if the cancun payload + /// fields are provided. If the payload fields are not provided, but versioned hashes exist + /// in the block, this is considered an error: [PayloadError::InvalidVersionedHashes]. + /// + /// This validates versioned hashes according to the Engine API Cancun spec: + /// + pub fn ensure_well_formed_payload( + &self, + payload: ExecutionPayload, + cancun_fields: MaybeCancunPayloadFields, + ) -> Result { + let block_hash = payload.block_hash(); + + // First parse the block + let block = try_into_block(payload, cancun_fields.parent_beacon_block_root())?; + + let cancun_active = self.is_cancun_active_at_timestamp(block.timestamp); + + if !cancun_active && block.has_blob_transactions() { + // cancun not active but blob transactions present + return Err(PayloadError::PreCancunBlockWithBlobTransactions); + } + + // Ensure the hash included in the payload matches the block hash + let sealed_block = validate_block_hash(block_hash, block)?; + + // EIP-4844 checks + self.ensure_matching_blob_versioned_hashes(&sealed_block, &cancun_fields)?; + + Ok(sealed_block) + } +} diff --git a/crates/primitives/src/block.rs b/crates/primitives/src/block.rs index 417167fa6..a1acbbfd7 100644 --- a/crates/primitives/src/block.rs +++ b/crates/primitives/src/block.rs @@ -86,6 +86,7 @@ impl Block { } /// Returns whether or not the block contains any blob transactions. + #[inline] pub fn has_blob_transactions(&self) -> bool { self.body.iter().any(|tx| tx.is_eip4844()) } @@ -233,9 +234,30 @@ impl SealedBlock { ) } + /// Returns an iterator over all blob transactions of the block + #[inline] + pub fn blob_transactions_iter(&self) -> impl Iterator + '_ { + self.body.iter().filter(|tx| tx.is_eip4844()) + } + /// Returns only the blob transactions, if any, from the block body. + #[inline] pub fn blob_transactions(&self) -> Vec<&TransactionSigned> { - self.body.iter().filter(|tx| tx.is_eip4844()).collect() + self.blob_transactions_iter().collect() + } + + /// Returns an iterator over all blob versioned hashes from the block body. + #[inline] + pub fn blob_versioned_hashes_iter(&self) -> impl Iterator + '_ { + self.blob_transactions_iter() + .filter_map(|tx| tx.as_eip4844().map(|blob_tx| &blob_tx.blob_versioned_hashes)) + .flatten() + } + + /// Returns all blob versioned hashes from the block body. + #[inline] + pub fn blob_versioned_hashes(&self) -> Vec<&B256> { + self.blob_versioned_hashes_iter().collect() } /// Expensive operation that recovers transaction signer. See [SealedBlockWithSenders]. diff --git a/crates/rpc/rpc-types-compat/src/engine/payload.rs b/crates/rpc/rpc-types-compat/src/engine/payload.rs index b007accc5..d5c21d2a3 100644 --- a/crates/rpc/rpc-types-compat/src/engine/payload.rs +++ b/crates/rpc/rpc-types-compat/src/engine/payload.rs @@ -304,6 +304,7 @@ pub fn try_into_sealed_block( /// /// If the provided block hash does not match the block hash computed from the provided block, this /// returns [PayloadError::BlockHash]. +#[inline] pub fn validate_block_hash( expected_block_hash: B256, block: Block, diff --git a/crates/rpc/rpc-types/src/eth/engine/cancun.rs b/crates/rpc/rpc-types/src/eth/engine/cancun.rs index 70ac709fe..3ddfb52f1 100644 --- a/crates/rpc/rpc-types/src/eth/engine/cancun.rs +++ b/crates/rpc/rpc-types/src/eth/engine/cancun.rs @@ -15,3 +15,45 @@ pub struct CancunPayloadFields { /// The expected blob versioned hashes. pub versioned_hashes: Vec, } + +/// A container type for [CancunPayloadFields] that may or may not be present. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Default)] +pub struct MaybeCancunPayloadFields { + fields: Option, +} + +impl MaybeCancunPayloadFields { + /// Returns a new `MaybeCancunPayloadFields` with no cancun fields. + pub const fn none() -> Self { + Self { fields: None } + } + + /// Returns a new `MaybeCancunPayloadFields` with the given cancun fields. + pub fn into_inner(self) -> Option { + self.fields + } + + /// Returns the parent beacon block root, if any. + pub fn parent_beacon_block_root(&self) -> Option { + self.fields.as_ref().map(|fields| fields.parent_beacon_block_root) + } + + /// Returns the blob versioned hashes, if any. + pub fn versioned_hashes(&self) -> Option<&Vec> { + self.fields.as_ref().map(|fields| &fields.versioned_hashes) + } +} + +impl From for MaybeCancunPayloadFields { + #[inline] + fn from(fields: CancunPayloadFields) -> Self { + Self { fields: Some(fields) } + } +} + +impl From> for MaybeCancunPayloadFields { + #[inline] + fn from(fields: Option) -> Self { + Self { fields } + } +} diff --git a/crates/rpc/rpc-types/src/eth/engine/payload.rs b/crates/rpc/rpc-types/src/eth/engine/payload.rs index 2790a78eb..3f0ff5c87 100644 --- a/crates/rpc/rpc-types/src/eth/engine/payload.rs +++ b/crates/rpc/rpc-types/src/eth/engine/payload.rs @@ -576,10 +576,17 @@ pub enum PayloadError { } impl PayloadError { - /// Returns `true` if the error is caused by invalid extra data. + /// Returns `true` if the error is caused by a block hash mismatch. + #[inline] pub fn is_block_hash_mismatch(&self) -> bool { matches!(self, PayloadError::BlockHash { .. }) } + + /// Returns `true` if the error is caused by invalid block hashes (Cancun). + #[inline] + pub fn is_invalid_versioned_hashes(&self) -> bool { + matches!(self, PayloadError::InvalidVersionedHashes) + } } /// This structure contains a body of an execution payload.